publishing OF platform devices

publishing OF platform devices

[John Rigby]

I have a question about the "right" way to publish platform
devices via of_platform_bus_probe.

On 5200 platforms it gets called from
mpc52xx_declare_of_platform_devices which is registered as the
machdep_calls init routine.

On other platforms it is called via a device_initcall

I assume the main difference between these two is one would happen
later than the other?

Also I noticed that some 83xx platforms don't call
of_platform_bus_probe at all.  Thats ok because most of those devices
are register various routines in fsl_soc.c.  The one I have not
figured out yet is spi, I don't see anyone registering mpc83xx_spi?

Any enlightenment would be appreciated

Thanks
John

Re: publishing OF platform devices

[Sylvain Munaut]

John Rigby wrote:
> I have a question about the "right" way to publish platform
> devices via of_platform_bus_probe.
>
> On 5200 platforms it gets called from
> mpc52xx_declare_of_platform_devices which is registered as the
> machdep_calls init routine.
>   
It's called by machdep calls so that it's only called if the platform is
detected ...
> On other platforms it is called via a device_initcall
>   
That will always be called which is bad.

Because if you have a kernel will multiple platform support built in, it
will
be called everytime, even if it's not running on the good platform.
> Also I noticed that some 83xx platforms don't call
> of_platform_bus_probe at all.  Thats ok because most of those devices
> are register various routines in fsl_soc.c.  The one I have not
> figured out yet is spi, I don't see anyone registering mpc83xx_spi?
>   
On theses I have no idea ...

    Sylvain

Re: publishing OF platform devices

[Kim Phillips]

On Thu, 29 Mar 2007 23:26:39 +0200
Sylvain Munaut <tnt@246tNt.com> wrote:

> John Rigby wrote:
> > I have a question about the "right" way to publish platform
> > devices via of_platform_bus_probe.
> >
> > On 5200 platforms it gets called from
> > mpc52xx_declare_of_platform_devices which is registered as the
> > machdep_calls init routine.
> >   
> It's called by machdep calls so that it's only called if the platform
> is detected ...
> > On other platforms it is called via a device_initcall
> >   
> That will always be called which is bad.
> 
> Because if you have a kernel will multiple platform support built in,
> it will
> be called everytime, even if it's not running on the good platform.
> > Also I noticed that some 83xx platforms don't call
> > of_platform_bus_probe at all.  Thats ok because most of those
> > devices are register various routines in fsl_soc.c.  The one I have
> > not figured out yet is spi, I don't see anyone registering
> > mpc83xx_spi?
> >   
> On theses I have no idea ...
> 
spi_mpc83xx is a platform_driver.  You either have to add spi code to
fsl_soc.c, or, probably better, make it an of_platform_driver (see
drivers/net/ucc_geth.c).  Hopefully an of_platform_driver patch will
be accepted now that cuimage is here, and 83xx can be removed from arch/ppc.

Kim

Re: publishing OF platform devices

[Arnd Bergmann]

On Thursday 29 March 2007, Sylvain Munaut wrote:
> John Rigby wrote:
> > I have a question about the "right" way to publish platform
> > devices via of_platform_bus_probe.
> >
> > On 5200 platforms it gets called from
> > mpc52xx_declare_of_platform_devices which is registered as the
> > machdep_calls init routine.
> > =A0=20
> It's called by machdep calls so that it's only called if the platform is
> detected ...
> > On other platforms it is called via a device_initcall
> > =A0=20
> That will always be called which is bad.
>=20
> Because if you have a kernel will multiple platform support built in, it
> will
> be called everytime, even if it's not running on the good platform.

The ppc_md->init() function is currently only present on 32 bit,
if we decide that this is the way to go, we should make it common
to both 32 and 64 bit platforms.

> > Also I noticed that some 83xx platforms don't call
> > of_platform_bus_probe at all. =A0Thats ok because most of those devices
> > are register various routines in fsl_soc.c. =A0The one I have not
> > figured out yet is spi, I don't see anyone registering mpc83xx_spi?
> >
> On theses I have no idea ...

They should be converted to use of_platform_bus_probe in the future. As soon
as we have of_platform_drivers for the devices probed in fsl_soc.c, we can
get rid of that file.

	Arnd <><

Re: publishing OF platform devices

[Benjamin Herrenschmidt]

> The ppc_md->init() function is currently only present on 32 bit,
> if we decide that this is the way to go, we should make it common
> to both 32 and 64 bit platforms.

I still think we should do something like platform_xxx_initcall(); with
xxx being {core,subsys,device,...} ... I'll look into it. ppc_md.init()
is a bit gross.

Ben.

Re: publishing OF platform devices

[Arnd Bergmann]

On Friday 30 March 2007, Benjamin Herrenschmidt wrote:
> 
> > The ppc_md->init() function is currently only present on 32 bit,
> > if we decide that this is the way to go, we should make it common
> > to both 32 and 64 bit platforms.
> 
> I still think we should do something like platform_xxx_initcall(); with
> xxx being {core,subsys,device,...} ... I'll look into it. ppc_md.init()
> is a bit gross.

Yes, that sounds like a good idea, but I'd much prefer adding these only
if they actually simplify the existing code, rather than having them
for completeness.

We also might want to have something a lot simpler at first. Since
many platforms just want to call

of_platform_bus_probe(NULL, foo_ids, NULL);

There could simply be an entry in ppc_md that gets set to foo_ids
and we call of_platform_bus_probe() with that, and get rid of the
unused root and parent arguments.

	Arnd <><

Re: publishing OF platform devices

[Benjamin Herrenschmidt]

On Fri, 2007-03-30 at 00:18 +0200, Arnd Bergmann wrote:
> On Friday 30 March 2007, Benjamin Herrenschmidt wrote:
> > 
> > > The ppc_md->init() function is currently only present on 32 bit,
> > > if we decide that this is the way to go, we should make it common
> > > to both 32 and 64 bit platforms.
> > 
> > I still think we should do something like platform_xxx_initcall(); with
> > xxx being {core,subsys,device,...} ... I'll look into it. ppc_md.init()
> > is a bit gross.
> 
> Yes, that sounds like a good idea, but I'd much prefer adding these only
> if they actually simplify the existing code, rather than having them
> for completeness.

They will at first generate something like:

static int func(void)
{
	if (!machine_is(platform))
		return 0;
	return user_func();
}

xxx_initcall(func);

And thus will be equivalent in generated code to the open coded
versions, but that is already enough to avoid, I hope, quite a bit of
mistakes and simplify the source.

Later on, we can look into doing some smart ELF section tricks to also
reduce the generated code.

> We also might want to have something a lot simpler at first. Since
> many platforms just want to call
> 
> of_platform_bus_probe(NULL, foo_ids, NULL);
> 
> There could simply be an entry in ppc_md that gets set to foo_ids
> and we call of_platform_bus_probe() with that, and get rid of the
> unused root and parent arguments.

Might not be unused forever... oh well, we'll see.

Ben.