We changed the pcie controller driver to platform driver so that the PCI
of platform devices need to be created earlier in the arch_initcall stage
according to the original timing of calling fsl_add_bridge(). So we do PCI
probing separately from other devices. But probing more than once could
cause duplication warning. We add check if the devices have already probed
before probing any devices to avoid duplication warning.
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---
drivers/of/platform.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
From: Rob Herring <hidden> Date: 2012-06-11 15:33:12
[adding lkml and Greg KH]
On 06/08/2012 04:43 AM, Jia Hongtao wrote:
We changed the pcie controller driver to platform driver so that the PCI
of platform devices need to be created earlier in the arch_initcall stage
according to the original timing of calling fsl_add_bridge(). So we do PCI
probing separately from other devices. But probing more than once could
cause duplication warning. We add check if the devices have already probed
before probing any devices to avoid duplication warning.
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---
Is this a fast operation? I worry that we'd be doing this for every
device creation. Looking at kobj's seems too low level as well.
Perhaps moving the warning up the call stack and making the relevant
functions __must_check would work.
Rob
quoted hunk
+ kfree(dev);
+ of_node_put(np);
+ return NULL;
+ }
+
/* count the io and irq resources */
while (of_address_to_resource(np, num_reg, &temp_res) == 0)
num_reg++;
-----Original Message-----
From: Rob Herring [mailto:robherring2@gmail.com]
Sent: Monday, June 11, 2012 11:33 PM
To: Jia Hongtao-B38951
Cc: devicetree-discuss@lists.ozlabs.org; linux-kernel@vger.kernel.org;
Greg KH
Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices
[adding lkml and Greg KH]
On 06/08/2012 04:43 AM, Jia Hongtao wrote:
quoted
We changed the pcie controller driver to platform driver so that the
PCI
quoted
of platform devices need to be created earlier in the arch_initcall
stage
quoted
according to the original timing of calling fsl_add_bridge(). So we do
PCI
quoted
probing separately from other devices. But probing more than once could
cause duplication warning. We add check if the devices have already
probed
quoted
before probing any devices to avoid duplication warning.
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---
Where's v1 and v2 history?
Sorry, there is no V1 and V2.
These are a series of patches and only this one related to device-tree operation.
V3 are for other patches.
I will aware of that in the future.
Thanks for mentioning.
if (!dev)
return NULL;
+ dev->dev.of_node = of_node_get(np);
+ if (bus_id)
+ dev_set_name(&dev->dev, "%s", bus_id);
+ else
+ of_device_make_bus_id(&dev->dev);
+
+ if (kset_find_obj(dev->dev.kobj.kset, kobject_name(&dev->dev.kobj)))
{
Is this a fast operation? I worry that we'd be doing this for every
device creation. Looking at kobj's seems too low level as well.
Perhaps moving the warning up the call stack and making the relevant
functions __must_check would work.
Rob
quoted
+ kfree(dev);
+ of_node_put(np);
+ return NULL;
+ }
+
/* count the io and irq resources */
while (of_address_to_resource(np, num_reg, &temp_res) == 0)
num_reg++;
if (!dev)
return NULL;
+ dev->dev.of_node = of_node_get(np);
+ if (bus_id)
+ dev_set_name(&dev->dev, "%s", bus_id);
+ else
+ of_device_make_bus_id(&dev->dev);
+
+ if (kset_find_obj(dev->dev.kobj.kset, kobject_name(&dev->dev.kobj)))
Whatever you are trying to do here, odds are, it's wrong :)
What is happening that is causing the problem? What is causing the
platform core to be calling probe on a device more than once?
greg k-h
if (!dev)
return NULL;
+ dev->dev.of_node = of_node_get(np);
+ if (bus_id)
+ dev_set_name(&dev->dev, "%s", bus_id);
+ else
+ of_device_make_bus_id(&dev->dev);
+
+ if (kset_find_obj(dev->dev.kobj.kset, kobject_name(&dev-
dev.kobj)))
Whatever you are trying to do here, odds are, it's wrong :)
What is happening that is causing the problem? What is causing the
platform core to be calling probe on a device more than once?
greg k-h
This is why in some cases we may probe twice:
Firstly, we need to probe pci controller separately. But in KVM different
topology of device tree is used in which pci nodes are not under root node
but under "simple bus". Unfortunately, of_platform_bus_probe() will probe
all the first level nodes under "simple bus" so pci nodes will probe again.
Two ways to solve this problem:
1. All the nodes that need to be probed should just under root.
2. Avoid duplication probe at runtime like this patch did.
I do like the first one but it's hard to changing the traditional way that
exist for a long time.
Also, I think this patch as a workaround is not a perfect way to solve this
problem. So I wonder if you or others have any better solutions.
Thanks.
-Jia Hongtao.
if (!dev)
return NULL;
+ dev->dev.of_node = of_node_get(np);
+ if (bus_id)
+ dev_set_name(&dev->dev, "%s", bus_id);
+ else
+ of_device_make_bus_id(&dev->dev);
+
+ if (kset_find_obj(dev->dev.kobj.kset, kobject_name(&dev-
dev.kobj)))
Whatever you are trying to do here, odds are, it's wrong :)
What is happening that is causing the problem? What is causing the
platform core to be calling probe on a device more than once?
greg k-h
This is why in some cases we may probe twice:
Firstly, we need to probe pci controller separately. But in KVM different
topology of device tree is used in which pci nodes are not under root node
but under "simple bus". Unfortunately, of_platform_bus_probe() will probe
all the first level nodes under "simple bus" so pci nodes will probe again.
Sounds like a bug :)
Two ways to solve this problem:
1. All the nodes that need to be probed should just under root.
That seems reasonable.
2. Avoid duplication probe at runtime like this patch did.
No, don't paper over the real problem in the platform core code like
this. Fix the real issue here instead.
I do like the first one but it's hard to changing the traditional way that
exist for a long time.
I don't understand, why is this just showing up now? What changed to
cause this? Couldn't that be the real problem here?
greg k-h
if (!dev)
return NULL;
+ dev->dev.of_node = of_node_get(np);
+ if (bus_id)
+ dev_set_name(&dev->dev, "%s", bus_id);
+ else
+ of_device_make_bus_id(&dev->dev);
+
+ if (kset_find_obj(dev->dev.kobj.kset, kobject_name(&dev-
dev.kobj)))
Whatever you are trying to do here, odds are, it's wrong :)
What is happening that is causing the problem? What is causing the
platform core to be calling probe on a device more than once?
greg k-h
This is why in some cases we may probe twice:
Firstly, we need to probe pci controller separately. But in KVM
different
quoted
topology of device tree is used in which pci nodes are not under root
node
quoted
but under "simple bus". Unfortunately, of_platform_bus_probe() will
probe
quoted
all the first level nodes under "simple bus" so pci nodes will probe
again.
Sounds like a bug :)
quoted
Two ways to solve this problem:
1. All the nodes that need to be probed should just under root.
That seems reasonable.
quoted
2. Avoid duplication probe at runtime like this patch did.
No, don't paper over the real problem in the platform core code like
this. Fix the real issue here instead.
quoted
I do like the first one but it's hard to changing the traditional way
that
quoted
exist for a long time.
I don't understand, why is this just showing up now? What changed to
cause this? Couldn't that be the real problem here?
The issue is showing up because we now probe devices twice.
Previously, we just probe devices once. But now we changed the way of pci
init which makes pci controllers should be probed earlier than other devices.
So we have to probe pci nodes separately. Probe more than once is the root
cause of this issue.
The pci patchset I mentioned please refer to:
http://patchwork.ozlabs.org/patch/163742/
- Jia Hongtao.
From: Li Yang-R58472 <hidden> Date: 2012-07-09 04:02:10
quoted
I don't understand, why is this just showing up now? What changed to
cause this? Couldn't that be the real problem here?
The issue is showing up because we now probe devices twice.
Previously, we just probe devices once. But now we changed the way of pci
init which makes pci controllers should be probed earlier than other
devices.
So we have to probe pci nodes separately. Probe more than once is the
root
cause of this issue.
The pci patchset I mentioned please refer to:
http://patchwork.ozlabs.org/patch/163742/
Let me try to clarify a little bit. The of platform bus normally traverse the device tree to add all the devices. The change which caused problem is that we need to probe PCIe RC devices at a earlier stage of initialization. So we added these PCIe RC devices earlier than the normal device tree traversal process. These PCIe RC devices will be scanned again during the normal traversal and cause duplicated devices being added. Our proposal is to deal with duplicated devices automatically and make it possible to scan the device tree multiple times for devices to be added.
Regards,
Leo
From: Rob Herring <hidden> Date: 2012-07-09 13:58:16
On 07/08/2012 10:46 PM, Li Yang-R58472 wrote:
quoted
quoted
I don't understand, why is this just showing up now? What
changed to cause this? Couldn't that be the real problem here?
The issue is showing up because we now probe devices twice.
Previously, we just probe devices once. But now we changed the way
of pci init which makes pci controllers should be probed earlier
than other devices. So we have to probe pci nodes separately. Probe
more than once is the root cause of this issue.
The pci patchset I mentioned please refer to:
http://patchwork.ozlabs.org/patch/163742/
Let me try to clarify a little bit. The of platform bus normally
traverse the device tree to add all the devices. The change which
caused problem is that we need to probe PCIe RC devices at a earlier
stage of initialization. So we added these PCIe RC devices earlier
than the normal device tree traversal process. These PCIe RC devices
will be scanned again during the normal traversal and cause
duplicated devices being added. Our proposal is to deal with
duplicated devices automatically and make it possible to scan the
device tree multiple times for devices to be added.
What is making PCI need to be probed earlier? Perhaps deferred probe
would work?
Perhaps giving of_platform_populate an exclude list of compatible
strings to skip would work.
Rob
On Mon, Jul 09, 2012 at 03:46:59AM +0000, Li Yang-R58472 wrote:
quoted
quoted
I don't understand, why is this just showing up now? What changed to
cause this? Couldn't that be the real problem here?
The issue is showing up because we now probe devices twice.
Previously, we just probe devices once. But now we changed the way of pci
init which makes pci controllers should be probed earlier than other
devices.
So we have to probe pci nodes separately. Probe more than once is the
root
cause of this issue.
The pci patchset I mentioned please refer to:
http://patchwork.ozlabs.org/patch/163742/
Let me try to clarify a little bit. The of platform bus normally
traverse the device tree to add all the devices. The change which
caused problem is that we need to probe PCIe RC devices at a earlier
stage of initialization.
That sounds, wrong.
So we added these PCIe RC devices earlier than the normal device tree
traversal process. These PCIe RC devices will be scanned again during
the normal traversal and cause duplicated devices being added. Our
proposal is to deal with duplicated devices automatically and make it
possible to scan the device tree multiple times for devices to be
added.
Then you need to put something in your own tree scanning logic to not
try to register devices multiple times. How about a simple flag in your
device structure instead of having to muck around in the driver core
internals?
Although one should seriously question the need to want to recan the bus
and register devices at different times of the boot process...
thanks,
greg k-h
-----Original Message-----
From: Rob Herring [mailto:robherring2@gmail.com]
Sent: Monday, July 09, 2012 9:58 PM
To: Li Yang-R58472
Cc: Jia Hongtao-B38951; Greg KH; devicetree-discuss@lists.ozlabs.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices
On 07/08/2012 10:46 PM, Li Yang-R58472 wrote:
quoted
quoted
quoted
I don't understand, why is this just showing up now? What
changed to cause this? Couldn't that be the real problem here?
The issue is showing up because we now probe devices twice.
Previously, we just probe devices once. But now we changed the way
of pci init which makes pci controllers should be probed earlier
than other devices. So we have to probe pci nodes separately. Probe
more than once is the root cause of this issue.
The pci patchset I mentioned please refer to:
http://patchwork.ozlabs.org/patch/163742/
Let me try to clarify a little bit. The of platform bus normally
traverse the device tree to add all the devices. The change which
caused problem is that we need to probe PCIe RC devices at a earlier
stage of initialization. So we added these PCIe RC devices earlier
than the normal device tree traversal process. These PCIe RC devices
will be scanned again during the normal traversal and cause
duplicated devices being added. Our proposal is to deal with
duplicated devices automatically and make it possible to scan the
device tree multiple times for devices to be added.
What is making PCI need to be probed earlier? Perhaps deferred probe
would work?
Pci initialization fsl_add_bridge() should be called earlier as they used
to be called in board specific code xx_setup_arch(). Now we put this init
part in the probe function of pci controller to make the code infrastructure
better. That make pci bus should be probed earlier.
Perhaps giving of_platform_populate an exclude list of compatible
strings to skip would work.
Rob
Yes, this would work and we actually already implemented this solution.
But I think it looks hackish and will influence the other API users.
Thanks.
-Jia Hongtao.
On Mon, Jul 09, 2012 at 03:46:59AM +0000, Li Yang-R58472 wrote:
quoted
quoted
quoted
I don't understand, why is this just showing up now? What changed to
cause this? Couldn't that be the real problem here?
The issue is showing up because we now probe devices twice.
Previously, we just probe devices once. But now we changed the way of pci
init which makes pci controllers should be probed earlier than other
devices.
So we have to probe pci nodes separately. Probe more than once is the
root
cause of this issue.
The pci patchset I mentioned please refer to:
http://patchwork.ozlabs.org/patch/163742/
Let me try to clarify a little bit. The of platform bus normally
traverse the device tree to add all the devices. The change which
caused problem is that we need to probe PCIe RC devices at a earlier
stage of initialization.
That sounds, wrong.
Yes, really really wrong; starting with terminology...
quoted
So we added these PCIe RC devices earlier than the normal device tree
traversal process. These PCIe RC devices will be scanned again during
the normal traversal and cause duplicated devices being added. Our
proposal is to deal with duplicated devices automatically and make it
possible to scan the device tree multiple times for devices to be
added.
... This isn't *probing* twice; it is *registration*. That's cause
confusion on this thread.
Then you need to put something in your own tree scanning logic to not
try to register devices multiple times. How about a simple flag in your
device structure instead of having to muck around in the driver core
internals?
Right. If you're going to create the pci bus devices early, then you
need to explicitly inhibit creation of them later... but still; why do
the PCI bus devices need to be registered separately from the rest of the
devices on the simple-bus? Why not just move *all* device registration
earlier?
Although one should seriously question the need to want to recan the bus
and register devices at different times of the boot process...
Yes; the model they're trying to use sounds wrong.
g.