Hello, On 11/18/2015 05:07 PM, Javier Martinez Canillas wrote: > Hello Brian and Mark, > > On 11/16/2015 06:32 PM, Javier Martinez Canillas wrote: >> On 11/16/2015 05:47 PM, Brian Norris wrote: >>> On Mon, Nov 16, 2015 at 05:00:43PM -0300, Javier Martinez Canillas wrote: >>>> On 11/16/2015 04:24 PM, Brian Norris wrote: > > [snip] > >>>>> I don't think you have problems only with bad FDTs. I think you have a >>>>> problem with perfectly valid DTs. >>>>> >>>> >>>> Agreed, I wonder if spi_uevent() shouldn't be changed to report both the >>>> OF and old SPI modaliases to avoid breaking a lot of drivers but at the >>>> same time allowing DT-only drivers to not need an SPI ID table. >>> >>> That's the solution I imagined, though I haven't tested it yet. I don't >>> see much precedent for reporting multiple modaliases, so there could be >>> some kind of ABI issues around that too. >>> >> >> Ok, I'm glad that we agree. This definitely needs to be discussed with more >> people. I'll re-spin my patch and post a v2 reporting multiple modaliases >> tomorrow after testing. >> > > So I had some time today to test this approach but unfortunately it seems > this workaround will not be possible because AFAICT kmod only takes into > account the last MODALIAS reported as a uevent. I still have to check the > kmod source code but that's my conclusion from trying to report both aliases. >
I dig a little more on this and is udev and not kmod that can't cope with more than one MODALIAS, kmod just use the alias that udev tells it to use. 1) OF modalias reported after SPI modalias ------------------------------------------ cat /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0/uevent DRIVER=cros-ec-spi OF_NAME=cros-ec OF_FULLNAME=/spi@12d40000/cros-ec@0 OF_COMPATIBLE_0=google,cros-ec-spi OF_COMPATIBLE_N=1 MODALIAS=spi:cros-ec-spi MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi udevadm test /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0 ACTION=add DEVPATH=/devices/platform/12d40000.spi/spi_master/spi2/spi2.0 DRIVER=cros-ec-spi MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi OF_COMPATIBLE_0=google,cros-ec-spi OF_COMPATIBLE_N=1 OF_FULLNAME=/spi@12d40000/cros-ec@0 OF_NAME=cros-ec SUBSYSTEM=spi USEC_INITIALIZED=52732787 run: 'kmod load of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi' 2) SPI modalias reported after OF modalias ------------------------------------------ cat /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0/uevent DRIVER=cros-ec-spi OF_NAME=cros-ec OF_FULLNAME=/spi@12d40000/cros-ec@0 OF_COMPATIBLE_0=google,cros-ec-spi OF_COMPATIBLE_N=1 MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi MODALIAS=spi:cros-ec-spi udevadm test /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0 ACTION=add DEVPATH=/devices/platform/12d40000.spi/spi_master/spi2/spi2.0 DRIVER=cros-ec-spi MODALIAS=spi:cros-ec-spi OF_COMPATIBLE_0=google,cros-ec-spi OF_COMPATIBLE_N=1 OF_FULLNAME=/spi@12d40000/cros-ec@0 OF_NAME=cros-ec SUBSYSTEM=spi USEC_INITIALIZED=288316488 run: 'kmod load spi:cros-ec-spi' So as you can see: 'kmod load $modalias' is only called for the last modalias. > > IOW, even when is possible to report more than one MODALIAS, user-space seems > to only use the last one reported so using both don't work. > > Of course kmod can be changed to check for more than one MODALIAS but since > the kernel should not break old user-space, the fact that a single MODALIAS > was reported seems to be an ABI now due how is used. > So I think our options are: a) Not change spi_uevent() to report an OF modalias and make a requirement to have a spi_device_id table even for OF-only to have autoload working. Regardless of the option, SPI ID tables should be present in drivers so these are supported in non-OF platforms as Mark said. b) Make sure that all OF drivers have a complete OF table with all entries in the SPI ID table before spi_uevent() is modified to report OF modaliases. My preference would be b) so the same table (OF or SPI ID) can be used for alias filling that match reported modalias and driver to device matching and will also be aligned with what other subsystems do. I'll check my script to see why the drivers/mtd/devices/m25p80.c was not found, likely because the entries in the spi_device_id table weren't in the DT binding doc before. Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/