Re: Dynamically adding SPI buses (was: Re: [PATCH v8 0/8] Device Tree Overlays - 8th time's the charm)

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

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

On Fri, Nov 27, 2015 at 02:06:41PM +0100, Geert Uytterhoeven wrote:

> (replaying to an old mail, as I'm seeing the exact same behavior with current
>  overlay code)

I don't seem to have this mail...  I'm not even sure that mailing list
archive does (at least

   http://thread.gmane.org/gmane.linux.kernel.spi.devel/18597

anyway).

> When adding an SPI master device node to DT, or changing its status to "okay",
> of_register_spi_devices() will scan for SPI slaves, and will add all of them.
> Later, the notifier kicks:

>     static int of_spi_notify(struct notifier_block *nb, unsigned long action,
>                              void *arg)
>     {
>             ...
> 
>             case OF_RECONFIG_CHANGE_ADD:
>                     master = of_find_spi_master_by_node(rd->dn->parent);
>                     if (master == NULL)
>                             return NOTIFY_OK;       /* not for us */
> 
>                     spi = of_register_spi_device(master, rd->dn);

> Woops, this fails, as the device has been added before!
> Should the code just check for the existence of the SPI slave first, or would
> that cause problems when just modifying an existing SPI slave node in DT?

Looking at the I2C code it seems like it's using
of_node_test_and_set_flag() to check for OF_POPULATED before
instantiating, doing that manually in every place where it might
instantiate a device and manually clearing when deinstantiating.

I have to say I don't entirely understand the logic behind how this
stuff is supposed to work, it's confusing to me why we have separate
dynamic and static instantiation paths in the first place or why this is
open coded in the buses.  It seems like a more robust thing here is to
always use the dynamic path even on static instantiation, or to move it
more into the driver core.

Ideally I'd like to see the I2C code factored out into the DT core but
otherwise probably we have to cut'n'paste it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]