[PATCH 00/18] Export SPI and OF module aliases in missing drivers

[PATCH 00/18] Export SPI and OF module aliases in missing drivers

[Javier Martinez Canillas]

Hello,

Short version:

This patch series is the SPI equivalent of the I2C one posted before [0].

This series add the missing MODULE_DEVICE_TABLE() for OF and SPI tables
to export that information so modules have the correct aliases built-in
and autoloading works correctly.

Longer version:

The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device (i.e:
OF or board code) and the table that is used later to match the driver
with the device (i.e: SPI id table or OF match table).

But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the device node's compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.

Also, there are many SPI drivers whose module auto-loading is not working
because of this fact that the SPI core always reports the MODALIAS as
spi:<modalias> and many developers didn't expect this since is not how
other subsystems behave.

I've identified SPI drivers with 3 types of different issues:

a) Those that have an spi_table but are not exported. The match works
   if the driver is built-in but since the ID table is not exported,
   module auto-load won't work.

b) Those that have a of_table but are not exported. This is currently
   not an issue since even when the of_table is used to match the dev
   with the driver, an OF modalias is not reported by the SPI core.
   But if the SPI core is changed to report the MODALIAS of the form
   of:N*T*C as it's made by other subsystems, then module auto-load
   will break for these drivers.

c) Those that don't have an of_table but should since are OF drivers
   with DT bindings doc for them. Since the SPI core does not report
   a OF modalias and since spi_match_device() fallbacks to match the
   device part of the compatible string with the SPI device ID table,
   many OF drivers don't have an of_table to match. After all having
   a SPI device ID table is mandatory so it works without a of_table.

So, in order to not make mandatory to have a SPI device ID table, all
these three kind of issues have to be addressed. This series does that.

I split the changes so the patches in this series are independent and
can be picked individually by subsystem maintainers.

Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches

Patch #18 changes the logic of spi_uevent() to report an OF modalias if
the device was registered using OF. But this patch is included in the
series only as an RFC for illustration purposes since changing that
without first applying all the other patches in this series, will break
module autoloading for the drivers of devices registered using OF but
that lacks an of_match_table. I'll repost patch #18 once all the patches
in this series have landed.

[0]: https://lkml.org/lkml/2015/7/30/519

Best regards,
Javier


Javier Martinez Canillas (18):
  iio: Export SPI module alias information in missing drivers
  staging: iio: hmc5843: Export missing SPI module alias information
  mtd: dataflash: Export OF module alias information
  OMAPDSS: panel-sony-acx565akm: Export OF module alias information
  mmc: mmc_spi: Export OF module alias information
  staging: mt29f_spinand: Export OF module alias information
  net: ks8851: Export OF module alias information
  [media] s5c73m3: Export OF module alias information
  mfd: cros_ec: spi: Add OF match table
  iio: dac: ad7303: Add OF match table
  iio: adc: max1027: Set struct spi_driver .of_match_table
  mfd: stmpe: Add OF match table
  iio: adc: mcp320x: Set struct spi_driver .of_match_table
  iio: as3935: Add OF match table
  iio: adc128s052: Add OF match table
  iio: frequency: adf4350: Add OF match table
  NFC: trf7970a: Add OF match table
  spi: (RFC, don't apply) report OF style modalias when probing using DT

 drivers/iio/adc/max1027.c                                   |  1 +
 drivers/iio/adc/mcp320x.c                                   |  1 +
 drivers/iio/adc/ti-adc128s052.c                             |  8 ++++++++
 drivers/iio/amplifiers/ad8366.c                             |  1 +
 drivers/iio/dac/ad7303.c                                    |  7 +++++++
 drivers/iio/frequency/adf4350.c                             |  9 +++++++++
 drivers/iio/proximity/as3935.c                              |  7 +++++++
 drivers/media/i2c/s5c73m3/s5c73m3-spi.c                     |  1 +
 drivers/mfd/cros_ec_spi.c                                   |  7 +++++++
 drivers/mfd/stmpe-spi.c                                     | 13 +++++++++++++
 drivers/mmc/host/mmc_spi.c                                  |  1 +
 drivers/mtd/devices/mtd_dataflash.c                         |  1 +
 drivers/net/ethernet/micrel/ks8851.c                        |  1 +
 drivers/nfc/trf7970a.c                                      |  7 +++++++
 drivers/spi/spi.c                                           |  8 ++++++++
 drivers/staging/iio/magnetometer/hmc5843_spi.c              |  1 +
 drivers/staging/mt29f_spinand/mt29f_spinand.c               |  1 +
 .../video/fbdev/omap2/displays-new/panel-sony-acx565akm.c   |  1 +
 18 files changed, 76 insertions(+)

-- 
2.4.3

[PATCH 03/18] mtd: dataflash: Export OF module alias information

[Javier Martinez Canillas]

The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).

So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.

But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.

To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/mtd/devices/mtd_dataflash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index 0099aba72a8b..df6f61137376 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -102,6 +102,7 @@ static const struct of_device_id dataflash_dt_ids[] = {
 	{ .compatible = "atmel,dataflash", },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, dataflash_dt_ids);
 #endif
 
 /* ......................................................................... */
-- 
2.4.3

Re: [PATCH 00/18] Export SPI and OF module aliases in missing drivers

[Brian Norris]

On Thu, Aug 20, 2015 at 09:07:13AM +0200, Javier Martinez Canillas wrote:
> Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches

^^^ I'm dying to know how this sentence ends :)

> Patch #18 changes the logic of spi_uevent() to report an OF modalias if
> the device was registered using OF. But this patch is included in the
> series only as an RFC for illustration purposes since changing that
> without first applying all the other patches in this series, will break
> module autoloading for the drivers of devices registered using OF but
> that lacks an of_match_table. I'll repost patch #18 once all the patches
> in this series have landed.

On a more productive note, the patches I've looked at look good to me.
The missing aliases are a problem enough that should be fixed (i.e.,
part (b)). I'll leave the SPI framework changes to others to comment on.

Brian

Re: [PATCH 00/18] Export SPI and OF module aliases in missing drivers

[Javier Martinez Canillas]

Hello Brian,

On 08/20/2015 11:11 PM, Brian Norris wrote:
> On Thu, Aug 20, 2015 at 09:07:13AM +0200, Javier Martinez Canillas wrote:
>> Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches
> 
> ^^^ I'm dying to know how this sentence ends :)
>

Sigh, I did some last minute restructuring of the cover letter and
seems I missed a sentence. I meant to said:

"and patches #9 to #17 solves c)."
 
>> Patch #18 changes the logic of spi_uevent() to report an OF modalias if
>> the device was registered using OF. But this patch is included in the
>> series only as an RFC for illustration purposes since changing that
>> without first applying all the other patches in this series, will break
>> module autoloading for the drivers of devices registered using OF but
>> that lacks an of_match_table. I'll repost patch #18 once all the patches
>> in this series have landed.
> 
> On a more productive note, the patches I've looked at look good to me.
> The missing aliases are a problem enough that should be fixed (i.e.,
> part (b)). I'll leave the SPI framework changes to others to comment on.
>

Great, thanks a lot for your feedback.
 
> Brian
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Brian Norris]

On Thu, Aug 20, 2015 at 09:07:16AM +0200, Javier Martinez Canillas wrote:
> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
> regardless of the mechanism that was used to register the device
> (i.e: OF or board code) and the table that is used later to match
> the driver with the device (i.e: SPI id table or OF match table).
> 
> So drivers needs to export the SPI id table and this be built into
> the module or udev won't have the necessary information to autoload
> the needed driver module when the device is added.
> 
> But this means that OF-only drivers needs to have both OF and SPI id
> tables that have to be kept in sync and also the dev node compatible
> manufacturer prefix is stripped when reporting the MODALIAS. Which can
> lead to issues if two vendors use the same SPI device name for example.
> 
> To avoid the above, the SPI core behavior may be changed in the future
> to not require an SPI device table for OF-only drivers and report the
> OF module alias. So, it's better to also export the OF table even when
> is unused now to prevent breaking module loading when the core changes.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

I appreciate the context for the whole problem here, but this commit
subject mostly doesn't apply to this driver, and it deson't seem to
match the problem this is solving. As I see it, the patch description
for this patch should be more like:

  "The OF ID table is used for auto-probing the device, but it is not
  exported to the module device table. That means this driver won't
  autoload when built as a module.

  Export the OF ID table to fix that.

  (insert some reference to the larger issue here)"

Do you want to rewrite the message, shall I just prepend my
modifications, or am I off-base?

> ---
> 
>  drivers/mtd/devices/mtd_dataflash.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> index 0099aba72a8b..df6f61137376 100644
> --- a/drivers/mtd/devices/mtd_dataflash.c
> +++ b/drivers/mtd/devices/mtd_dataflash.c
> @@ -102,6 +102,7 @@ static const struct of_device_id dataflash_dt_ids[] = {
>  	{ .compatible = "atmel,dataflash", },
>  	{ /* sentinel */ }
>  };
> +MODULE_DEVICE_TABLE(of, dataflash_dt_ids);
>  #endif
>  
>  /* ......................................................................... */

For the patch:

Reviewed-by: Brian Norris <computersforpeace@gmail.com>

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Javier Martinez Canillas]

Hello Brian,

On 08/20/2015 11:54 PM, Brian Norris wrote:
> On Thu, Aug 20, 2015 at 09:07:16AM +0200, Javier Martinez Canillas wrote:
>> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
>> regardless of the mechanism that was used to register the device
>> (i.e: OF or board code) and the table that is used later to match
>> the driver with the device (i.e: SPI id table or OF match table).
>>
>> So drivers needs to export the SPI id table and this be built into
>> the module or udev won't have the necessary information to autoload
>> the needed driver module when the device is added.
>>
>> But this means that OF-only drivers needs to have both OF and SPI id
>> tables that have to be kept in sync and also the dev node compatible
>> manufacturer prefix is stripped when reporting the MODALIAS. Which can
>> lead to issues if two vendors use the same SPI device name for example.
>>
>> To avoid the above, the SPI core behavior may be changed in the future
>> to not require an SPI device table for OF-only drivers and report the
>> OF module alias. So, it's better to also export the OF table even when
>> is unused now to prevent breaking module loading when the core changes.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> I appreciate the context for the whole problem here, but this commit
> subject mostly doesn't apply to this driver, and it deson't seem to
> match the problem this is solving. As I see it, the patch description
> for this patch should be more like:
> 
>   "The OF ID table is used for auto-probing the device, but it is not
>   exported to the module device table. That means this driver won't
>   autoload when built as a module.
> 
>   Export the OF ID table to fix that.
> 
>   (insert some reference to the larger issue here)"
> 
> Do you want to rewrite the message, shall I just prepend my
> modifications, or am I off-base?
>

Well, I believe my commit message is more accurate than yours :-)

I mean, right now the OF modalias information is not really used neither
by the SPI core since always MODALIAS=spi:foo is reported nor by user
space since the MODALIAS uevent will never match the OF exported modalias.

So this patch really is a no-op right now and is not fixing anything.
As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
work without a MODULE_DEVICE_TABLE(of,...).

However, this patch is needed to avoid breaking module autoloading in
the future once RFC patch 18/18 is applied and the SPI core is changed
to report a MODALIAS=of:N*T*Cfoo,bar for SPI devices registered by OF.

Having said that, I will of course re-spin the patch and write a
commit message that you agree on. So I'll be happy to trim it down if
is providing too much content or is not correct.

I just don't think your suggestion better matches reality than mine.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Brian Norris]

On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
> So this patch really is a no-op right now and is not fixing anything.
> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will

This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
use spi_device_id. So, it is currently broken for the module use case.

> work without a MODULE_DEVICE_TABLE(of,...).

Brian

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Javier Martinez Canillas]

Hello Brian,

On 08/21/2015 12:34 AM, Brian Norris wrote:
> On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
>> So this patch really is a no-op right now and is not fixing anything.
>> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
> 
> This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
> use spi_device_id. So, it is currently broken for the module use case.
>

That is correct but also that does not mean that this patch will fix
module autoloading right now. It will though once the SPI core is
changed to report a OF type module alias.

So what about something along this lines?

mtd: dataflash: Export OF module alias information

The SPI core currently reports the MODALIAS uevent as "spi:<modalias>"
even for SPI devices that were registered by OF.

That means the OF module alias exported by MODULE_OF_TABLE(of,...) is
currently not used and user-space has no way to autoload this module.

But is still a good practice to add the OF module alias information
into the kernel module even when currently is unused so once the SPI
core is changed to report a correct OF modalias uevent, module
autoloading will be working for this driver.
 
>> work without a MODULE_DEVICE_TABLE(of,...).
> 
> Brian
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Brian Norris]

Hi Javier,

On Fri, Aug 21, 2015 at 12:57:54AM +0200, Javier Martinez Canillas wrote:
> On 08/21/2015 12:34 AM, Brian Norris wrote:
> > On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
> >> So this patch really is a no-op right now and is not fixing anything.
> >> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
> > 
> > This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
> > use spi_device_id. So, it is currently broken for the module use case.
> >
> 
> That is correct but also that does not mean that this patch will fix
> module autoloading right now. It will though once the SPI core is
> changed to report a OF type module alias.

Ah, thanks for the patience. I missed the point that it will still be
broken.

> So what about something along this lines?
> 
> mtd: dataflash: Export OF module alias information
> 
> The SPI core currently reports the MODALIAS uevent as "spi:<modalias>"
> even for SPI devices that were registered by OF.
> 
> That means the OF module alias exported by MODULE_OF_TABLE(of,...) is
> currently not used and user-space has no way to autoload this module.
> 
> But is still a good practice to add the OF module alias information

nit: s/is/it is/

> into the kernel module even when currently is unused so once the SPI

ditto

> core is changed to report a correct OF modalias uevent, module
> autoloading will be working for this driver.

Otherwise, looks good. I'll either patch in this commit message, or
await v2. Your call.

Regards,
Brian

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Javier Martinez Canillas]

Hello Brian,

On 08/22/2015 12:47 AM, Brian Norris wrote:
> Hi Javier,
> 
> On Fri, Aug 21, 2015 at 12:57:54AM +0200, Javier Martinez Canillas wrote:
>> On 08/21/2015 12:34 AM, Brian Norris wrote:
>>> On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
>>>> So this patch really is a no-op right now and is not fixing anything.
>>>> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
>>>
>>> This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
>>> use spi_device_id. So, it is currently broken for the module use case.
>>>
>>
>> That is correct but also that does not mean that this patch will fix
>> module autoloading right now. It will though once the SPI core is
>> changed to report a OF type module alias.
> 
> Ah, thanks for the patience. I missed the point that it will still be
> broken.
>

No worries, I'm glad that we are on the same page now.
 
>> So what about something along this lines?
>>
>> mtd: dataflash: Export OF module alias information
>>
>> The SPI core currently reports the MODALIAS uevent as "spi:<modalias>"
>> even for SPI devices that were registered by OF.
>>
>> That means the OF module alias exported by MODULE_OF_TABLE(of,...) is
>> currently not used and user-space has no way to autoload this module.
>>
>> But is still a good practice to add the OF module alias information
> 
> nit: s/is/it is/
> 
>> into the kernel module even when currently is unused so once the SPI
> 
> ditto
> 
>> core is changed to report a correct OF modalias uevent, module
>> autoloading will be working for this driver.
> 
> Otherwise, looks good. I'll either patch in this commit message, or
> await v2. Your call.
>

If you don't mind fixing it up yourself when applying, then that
would be really helpful. Otherwise I can post a v2 on Monday.

> Regards,
> Brian
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Brian Norris]

On Sat, Aug 22, 2015 at 02:26:14AM +0200, Javier Martinez Canillas wrote:
> On 08/22/2015 12:47 AM, Brian Norris wrote:
> > Otherwise, looks good. I'll either patch in this commit message, or
> > await v2. Your call.
> 
> If you don't mind fixing it up yourself when applying, then that
> would be really helpful. Otherwise I can post a v2 on Monday.

Queued to l2-mtd.git/next (for 4.4), with the edited commit message.

Re: [PATCH 03/18] mtd: dataflash: Export OF module alias information

[Javier Martinez Canillas]

Hello Brian,

On 08/22/2015 03:05 AM, Brian Norris wrote:
> On Sat, Aug 22, 2015 at 02:26:14AM +0200, Javier Martinez Canillas wrote:
>> On 08/22/2015 12:47 AM, Brian Norris wrote:
>>> Otherwise, looks good. I'll either patch in this commit message, or
>>> await v2. Your call.
>>
>> If you don't mind fixing it up yourself when applying, then that
>> would be really helpful. Otherwise I can post a v2 on Monday.
> 
> Queued to l2-mtd.git/next (for 4.4), with the edited commit message.
> 

Great, thanks a lot for your help!

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America