Re: [RFC] early init and DT platform devices allocation/registration

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3288 bytes --]

On Wed, Jun 26, 2013 at 02:12:06PM +0100, Grant Likely wrote:
> On Wed, Jun 26, 2013 at 1:44 PM, Sebastian Hesselbarth
> <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On 06/26/13 12:03, Grant Likely wrote:
> >>
> >> On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>>
> >>> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Tue, 25 Jun 2013
> >>> 19:52:33 +0200:
> >>>
> >>>>> Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC
> >>>>> to avoid duplicated device registration.
> >>>>
> >>>>
> >>>> Gah! my eyes!
> >>>>
> >>>> Don't do that. It is incredibly problematic. Look at inhibiting
> >>>> duplicate device creation instead.
> >>>
> >>>
> >>> I may not follow this thread correctly, but could anyone point out the
> >>> above "inhibiting duplicate device creation" if there's already such
> >>> solution?
> >>
> >>
> >> No, the solution doesn't exist yet, but it wouldn't be hard to
> >> implement. What you need to do is to add a struct device pointer to
> >> struct device_node, and set the pointer to the struct device when
> >> of_platform_device_create creates a device. (it would also need to be
> >> set for early_platform_device creation, but that's not something that
> >> should affect you). You would also add a check to
> >> of_platform_device_create to check if the device pointer is already
> >> set. If it is, then skip creation of the device.
> >
> >
> > Grant,
> >
> > What about the other way round, i.e. check if there is a device with
> > .of_node pointed to the struct device_node currently at
> > of_platform_device_create?
> >
> > That will avoid adding struct device to struct device_node which you
> > fought against for good reasons.
> 
> The main thing is that it means searching through the entire list of
> platform devices every time a new platform device is created. That
> seems unnecessarily expensive to me.

There's not really much reason to not add a pointer to the struct device
of a device_node because you can already obtain the platform_device by
calling of_find_device_by_node(). That doesn't work early, but if that
gets fixed, then of_find_device_by_node() could also be used. And since
adding a struct device * to device_node is pretty much required to fix
of_platform_device_create() for early, of_find_device_by_node() can be
implemented much more efficiently.

> > Also, I guess of_platform_device_create could be exported and used
> > by anyone who wants to create platform_devices early.
> 
> Yes, but in that case it is probably better for them to call
> of_platform_populate early if of_platform_device_create is fixed to
> support early calling. Then you'd just set up all the devices earlier
> in init, allow drivers that support early probing to do so, and then
> everything else uses the normal initcall path.

I've been bugged by the recent additions in drivers/irqchip lately
because they all rely on only the device_node and therefore none of the
functions that require a struct device can be used. If things can indeed
be fixed to call of_platform_populate() early that would restore a whole
lot of consistency.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]