Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Rob Herring]

[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 <B38951@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---

Where's v1 and v2 history?

>  drivers/of/platform.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index a37330e..3aab01f 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct device_node *np,
>  	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

> +		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++;
> @@ -161,17 +173,11 @@ struct platform_device *of_device_alloc(struct device_node *np,
>  		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
>  	}
>  
> -	dev->dev.of_node = of_node_get(np);
>  #if defined(CONFIG_MICROBLAZE)
>  	dev->dev.dma_mask = &dev->archdata.dma_mask;
>  #endif
>  	dev->dev.parent = parent;
>  
> -	if (bus_id)
> -		dev_set_name(&dev->dev, "%s", bus_id);
> -	else
> -		of_device_make_bus_id(&dev->dev);
> -
>  	return dev;
>  }
>  EXPORT_SYMBOL(of_device_alloc);

RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Jia Hongtao-B38951]

> -----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:
> > 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 <B38951@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> 
> 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.

> 
> >  drivers/of/platform.c |   18 ++++++++++++------
> >  1 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index a37330e..3aab01f 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct
> device_node *np,
> >  	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.
>

To us this is also the main concern.

-Jia Hongtao.
 
> Perhaps moving the warning up the call stack and making the relevant
> functions __must_check would work.
> 
> Rob
> 
> > +		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++;
> > @@ -161,17 +173,11 @@ struct platform_device *of_device_alloc(struct
> device_node *np,
> >  		WARN_ON(of_irq_to_resource_table(np, res, num_irq) !=
> num_irq);
> >  	}
> >
> > -	dev->dev.of_node = of_node_get(np);
> >  #if defined(CONFIG_MICROBLAZE)
> >  	dev->dev.dma_mask = &dev->archdata.dma_mask;
> >  #endif
> >  	dev->dev.parent = parent;
> >
> > -	if (bus_id)
> > -		dev_set_name(&dev->dev, "%s", bus_id);
> > -	else
> > -		of_device_make_bus_id(&dev->dev);
> > -
> >  	return dev;
> >  }
> >  EXPORT_SYMBOL(of_device_alloc);
> 

RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Jia Hongtao-B38951]

Hi Rob and Greg KH,

Do you have any better idea to avoid duplication probe warning?

Thanks.
-Jia Hongtao.

> -----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:
> > 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 <B38951@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> 
> Where's v1 and v2 history?
> 
> >  drivers/of/platform.c |   18 ++++++++++++------
> >  1 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index a37330e..3aab01f 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct
> device_node *np,
> >  	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
> 
> > +		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++;
> > @@ -161,17 +173,11 @@ struct platform_device *of_device_alloc(struct
> device_node *np,
> >  		WARN_ON(of_irq_to_resource_table(np, res, num_irq) !=
> num_irq);
> >  	}
> >
> > -	dev->dev.of_node = of_node_get(np);
> >  #if defined(CONFIG_MICROBLAZE)
> >  	dev->dev.dma_mask = &dev->archdata.dma_mask;
> >  #endif
> >  	dev->dev.parent = parent;
> >
> > -	if (bus_id)
> > -		dev_set_name(&dev->dev, "%s", bus_id);
> > -	else
> > -		of_device_make_bus_id(&dev->dev);
> > -
> >  	return dev;
> >  }
> >  EXPORT_SYMBOL(of_device_alloc);
> 

Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Greg KH]

On Fri, Jul 06, 2012 at 02:05:05AM +0000, Jia Hongtao-B38951 wrote:
> Hi Rob and Greg KH,
> 
> Do you have any better idea to avoid duplication probe warning?

I have no idea what the problem is that you are trying to solve.

> > 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.

Ick, something else is going wrong here, how could you ever have the
same device probed more than once?

> > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > ---
> > 
> > Where's v1 and v2 history?
> > 
> > >  drivers/of/platform.c |   18 ++++++++++++------
> > >  1 files changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > index a37330e..3aab01f 100644
> > > --- a/drivers/of/platform.c
> > > +++ b/drivers/of/platform.c
> > > @@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct
> > device_node *np,
> > >  	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

RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Friday, July 06, 2012 12:26 PM
> To: Jia Hongtao-B38951
> Cc: Rob Herring; devicetree-discuss@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; Li Yang-R58472
> Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices
> 
> On Fri, Jul 06, 2012 at 02:05:05AM +0000, Jia Hongtao-B38951 wrote:
> > Hi Rob and Greg KH,
> >
> > Do you have any better idea to avoid duplication probe warning?
> 
> I have no idea what the problem is that you are trying to solve.
> 
> > > 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.
> 
> Ick, something else is going wrong here, how could you ever have the
> same device probed more than once?
> 
> > > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > ---
> > >
> > > Where's v1 and v2 history?
> > >
> > > >  drivers/of/platform.c |   18 ++++++++++++------
> > > >  1 files changed, 12 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > > index a37330e..3aab01f 100644
> > > > --- a/drivers/of/platform.c
> > > > +++ b/drivers/of/platform.c
> > > > @@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct
> > > device_node *np,
> > > >  	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. 

Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Greg KH]

On Fri, Jul 06, 2012 at 06:07:43AM +0000, Jia Hongtao-B38951 wrote:
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Friday, July 06, 2012 12:26 PM
> > To: Jia Hongtao-B38951
> > Cc: Rob Herring; devicetree-discuss@lists.ozlabs.org; linux-
> > kernel@vger.kernel.org; Li Yang-R58472
> > Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices
> > 
> > On Fri, Jul 06, 2012 at 02:05:05AM +0000, Jia Hongtao-B38951 wrote:
> > > Hi Rob and Greg KH,
> > >
> > > Do you have any better idea to avoid duplication probe warning?
> > 
> > I have no idea what the problem is that you are trying to solve.
> > 
> > > > 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.
> > 
> > Ick, something else is going wrong here, how could you ever have the
> > same device probed more than once?
> > 
> > > > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > > ---
> > > >
> > > > Where's v1 and v2 history?
> > > >
> > > > >  drivers/of/platform.c |   18 ++++++++++++------
> > > > >  1 files changed, 12 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > > > index a37330e..3aab01f 100644
> > > > > --- a/drivers/of/platform.c
> > > > > +++ b/drivers/of/platform.c
> > > > > @@ -139,6 +139,18 @@ struct platform_device *of_device_alloc(struct
> > > > device_node *np,
> > > > >  	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

RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Saturday, July 07, 2012 12:17 AM
> To: Jia Hongtao-B38951
> Cc: Rob Herring; devicetree-discuss@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; Li Yang-R58472
> Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices
> 
> On Fri, Jul 06, 2012 at 06:07:43AM +0000, Jia Hongtao-B38951 wrote:
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Friday, July 06, 2012 12:26 PM
> > > To: Jia Hongtao-B38951
> > > Cc: Rob Herring; devicetree-discuss@lists.ozlabs.org; linux-
> > > kernel@vger.kernel.org; Li Yang-R58472
> > > Subject: Re: [PATCH V3 5/6] Avoid duplicate probe for of platform
> devices
> > >
> > > On Fri, Jul 06, 2012 at 02:05:05AM +0000, Jia Hongtao-B38951 wrote:
> > > > Hi Rob and Greg KH,
> > > >
> > > > Do you have any better idea to avoid duplication probe warning?
> > >
> > > I have no idea what the problem is that you are trying to solve.
> > >
> > > > > 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.
> > >
> > > Ick, something else is going wrong here, how could you ever have the
> > > same device probed more than once?
> > >
> > > > > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > > > ---
> > > > >
> > > > > Where's v1 and v2 history?
> > > > >
> > > > > >  drivers/of/platform.c |   18 ++++++++++++------
> > > > > >  1 files changed, 12 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > > > > index a37330e..3aab01f 100644
> > > > > > --- a/drivers/of/platform.c
> > > > > > +++ b/drivers/of/platform.c
> > > > > > @@ -139,6 +139,18 @@ struct platform_device
> *of_device_alloc(struct
> > > > > device_node *np,
> > > > > >  	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?
> 

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.

RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Li Yang-R58472]

> > 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

Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Rob Herring]

On 07/08/2012 10:46 PM, Li Yang-R58472 wrote:
>>> 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

RE: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Jia Hongtao-B38951]

> -----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:
> >>> 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.

Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Greg KH]

On Mon, Jul 09, 2012 at 03:46:59AM +0000, Li Yang-R58472 wrote:
> > > 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

Re: [PATCH V3 5/6] Avoid duplicate probe for of platform devices

[Grant Likely]

On Mon, 9 Jul 2012 07:58:31 -0700, Greg KH <greg@kroah.com> wrote:
> On Mon, Jul 09, 2012 at 03:46:59AM +0000, Li Yang-R58472 wrote:
> > > > 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...

> > 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.