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:
signature.asc
Description: PGP signature