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

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Rob Herring [mailto:robherring2@xxxxxxxxx]
> Sent: Monday, June 11, 2012 11:33 PM
> To: Jia Hongtao-B38951
> Cc: devicetree-discuss@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> 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@xxxxxxxxxxxxx>
> > Signed-off-by: Li Yang <leoli@xxxxxxxxxxxxx>
> > ---
> 
> 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);
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/