[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 01/18] iio: Export SPI module alias information in missing drivers

[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.

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

 drivers/iio/amplifiers/ad8366.c | 1 +
 drivers/iio/frequency/adf4350.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index c0d364ebaea8..32b82a2dc894 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -195,6 +195,7 @@ static const struct spi_device_id ad8366_id[] = {
 	{"ad8366", 0},
 	{}
 };
+MODULE_DEVICE_TABLE(spi, ad8366_id);
 
 static struct spi_driver ad8366_driver = {
 	.driver = {
diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 9890c81c027d..8bd873fc3787 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -621,6 +621,7 @@ static const struct spi_device_id adf4350_id[] = {
 	{"adf4351", 4351},
 	{}
 };
+MODULE_DEVICE_TABLE(spi, adf4350_id);
 
 static struct spi_driver adf4350_driver = {
 	.driver = {
-- 
2.4.3

[PATCH 02/18] staging: iio: hmc5843: Export missing SPI 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.

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

 drivers/staging/iio/magnetometer/hmc5843_spi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/iio/magnetometer/hmc5843_spi.c b/drivers/staging/iio/magnetometer/hmc5843_spi.c
index 8e658f736e1f..4dfb372d2fec 100644
--- a/drivers/staging/iio/magnetometer/hmc5843_spi.c
+++ b/drivers/staging/iio/magnetometer/hmc5843_spi.c
@@ -81,6 +81,7 @@ static const struct spi_device_id hmc5843_id[] = {
 	{ "hmc5983", HMC5983_ID },
 	{ }
 };
+MODULE_DEVICE_TABLE(spi, hmc5843_id);
 
 static struct spi_driver hmc5843_driver = {
 		.driver = {
-- 
2.4.3

[PATCH 10/18] iio: dac: ad7303: Add OF match table

[Javier Martinez Canillas]

The Documentation/devicetree/bindings/iio/dac/ad7303.txt DT binding doc
lists "adi,ad7303" as a compatible string but the corresponding driver
does not have an OF match table. Add the table to the driver so the SPI
core can do an OF style match.

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

 drivers/iio/dac/ad7303.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/dac/ad7303.c b/drivers/iio/dac/ad7303.c
index fa2810032968..18a4ad5ff8c5 100644
--- a/drivers/iio/dac/ad7303.c
+++ b/drivers/iio/dac/ad7303.c
@@ -281,6 +281,12 @@ static int ad7303_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ad7303_spi_of_match[] = {
+	{ .compatible = "adi,ad7303", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ad7303_spi_of_match);
+
 static const struct spi_device_id ad7303_spi_ids[] = {
 	{ "ad7303", 0 },
 	{}
@@ -290,6 +296,7 @@ MODULE_DEVICE_TABLE(spi, ad7303_spi_ids);
 static struct spi_driver ad7303_driver = {
 	.driver = {
 		.name = "ad7303",
+		.of_match_table = of_match_ptr(ad7303_spi_of_match),
 		.owner = THIS_MODULE,
 	},
 	.probe = ad7303_probe,
-- 
2.4.3

[PATCH 11/18] iio: adc: max1027: Set struct spi_driver .of_match_table

[Javier Martinez Canillas]

The driver has an OF id table but the .of_match_table is not set so
the SPI core can't do an OF style match and the table was unused.

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

 drivers/iio/adc/max1027.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
index 44bf815adb6c..54a8302aaace 100644
--- a/drivers/iio/adc/max1027.c
+++ b/drivers/iio/adc/max1027.c
@@ -508,6 +508,7 @@ static int max1027_remove(struct spi_device *spi)
 static struct spi_driver max1027_driver = {
 	.driver = {
 		.name	= "max1027",
+		.of_match_table = of_match_ptr(max1027_adc_dt_ids),
 		.owner	= THIS_MODULE,
 	},
 	.probe		= max1027_probe,
-- 
2.4.3

[PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Javier Martinez Canillas]

The driver has an OF id table but the .of_match_table is not set so
the SPI core can't do an OF style match and the table was unused.

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

 drivers/iio/adc/mcp320x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index b19e4f9d16e0..41a21e986c1a 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -404,6 +404,7 @@ MODULE_DEVICE_TABLE(spi, mcp320x_id);
 static struct spi_driver mcp320x_driver = {
 	.driver = {
 		.name = "mcp320x",
+		.of_match_table = of_match_ptr(mcp320x_dt_ids),
 		.owner = THIS_MODULE,
 	},
 	.probe = mcp320x_probe,
-- 
2.4.3

[PATCH 14/18] iio: as3935: Add OF match table

[Javier Martinez Canillas]

The Documentation/devicetree/bindings/iio/proximity/as3935.txt DT binding
doc lists "ams,as3935" as a compatible string but the corresponding driver
does not have an OF match table. Add the table to the driver so the SPI
core can do an OF style match.

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

 drivers/iio/proximity/as3935.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
index bc0d68efd455..e95035136889 100644
--- a/drivers/iio/proximity/as3935.c
+++ b/drivers/iio/proximity/as3935.c
@@ -434,6 +434,12 @@ static int as3935_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id as3935_of_match[] = {
+	{ .compatible = "ams,as3935", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, as3935_of_match);
+
 static const struct spi_device_id as3935_id[] = {
 	{"as3935", 0},
 	{},
@@ -443,6 +449,7 @@ MODULE_DEVICE_TABLE(spi, as3935_id);
 static struct spi_driver as3935_driver = {
 	.driver = {
 		.name	= "as3935",
+		.of_match_table = of_match_ptr(as3935_of_match),
 		.owner	= THIS_MODULE,
 		.pm	= AS3935_PM_OPS,
 	},
-- 
2.4.3

[PATCH 15/18] iio: adc128s052: Add OF match table

[Javier Martinez Canillas]

The Documentation/devicetree/bindings/iio/adc/ti-adc128s052.txt DT binding
doc lists "ti,adc128s052" or "ti,adc122s021" as compatible strings but the
corresponding driver does not have an OF match table. Add the table to the
driver so the SPI core can do an OF style match.

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

 drivers/iio/adc/ti-adc128s052.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 915be6b60097..98c0d2b444bf 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -174,6 +174,13 @@ static int adc128_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id adc128_of_match[] = {
+	{ .compatible = "ti,adc128s052", },
+	{ .compatible = "ti,adc122s021", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, adc128_of_match);
+
 static const struct spi_device_id adc128_id[] = {
 	{ "adc128s052", 0},	/* index into adc128_config */
 	{ "adc122s021",	1},
@@ -184,6 +191,7 @@ MODULE_DEVICE_TABLE(spi, adc128_id);
 static struct spi_driver adc128_driver = {
 	.driver = {
 		.name = "adc128s052",
+		.of_match_table = of_match_ptr(adc128_of_match),
 		.owner = THIS_MODULE,
 	},
 	.probe = adc128_probe,
-- 
2.4.3

[PATCH 16/18] iio: frequency: adf4350: Add OF match table

[Javier Martinez Canillas]

The Documentation/devicetree/bindings/iio/frequency/adf4350.txt DT binding
doc lists "adi,adf4350" or "adi,adf4351" as compatible strings but the
corresponding driver does not have an OF match table. Add the table to the
driver so the SPI core can do an OF style match.

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

 drivers/iio/frequency/adf4350.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 8bd873fc3787..73f27e0a08dd 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -616,6 +616,13 @@ static int adf4350_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id adf4350_of_match[] = {
+	{ .compatible = "adi,adf4350", },
+	{ .compatible = "adi,adf4351", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, adf4350_of_match);
+
 static const struct spi_device_id adf4350_id[] = {
 	{"adf4350", 4350},
 	{"adf4351", 4351},
@@ -626,6 +633,7 @@ MODULE_DEVICE_TABLE(spi, adf4350_id);
 static struct spi_driver adf4350_driver = {
 	.driver = {
 		.name	= "adf4350",
+		.of_match_table = of_match_ptr(adf4350_of_match),
 		.owner	= THIS_MODULE,
 	},
 	.probe		= adf4350_probe,
-- 
2.4.3

Re: [PATCH 16/18] iio: frequency: adf4350: Add OF match table

[Lars-Peter Clausen]

On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/frequency/adf4350.txt DT binding
> doc lists "adi,adf4350" or "adi,adf4351" as compatible strings but the
> corresponding driver does not have an OF match table. Add the table to the
> driver so the SPI core can do an OF style match.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

Re: [PATCH 10/18] iio: dac: ad7303: Add OF match table

[Lars-Peter Clausen]

On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/dac/ad7303.txt DT binding doc
> lists "adi,ad7303" as a compatible string but the corresponding driver
> does not have an OF match table. Add the table to the driver so the SPI
> core can do an OF style match.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>


Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

Re: [PATCH 01/18] iio: Export SPI module alias information in missing drivers

[Lars-Peter Clausen]

On 08/20/2015 09:07 AM, 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.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Michael Welling]

On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> The driver has an OF id table but the .of_match_table is not set so
> the SPI core can't do an OF style match and the table was unused.
> 

Is an OF style match necessary?

I have been using devicetree and it matches based on the .id_table.

Couldn't we just remove the mcp320x_dt_ids table instead?

> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/iio/adc/mcp320x.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index b19e4f9d16e0..41a21e986c1a 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -404,6 +404,7 @@ MODULE_DEVICE_TABLE(spi, mcp320x_id);
>  static struct spi_driver mcp320x_driver = {
>  	.driver = {
>  		.name = "mcp320x",
> +		.of_match_table = of_match_ptr(mcp320x_dt_ids),
>  		.owner = THIS_MODULE,
>  	},
>  	.probe = mcp320x_probe,
> -- 
> 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 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Javier Martinez Canillas]

Hello Michael,

On 08/20/2015 10:09 PM, Michael Welling wrote:
> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>> The driver has an OF id table but the .of_match_table is not set so
>> the SPI core can't do an OF style match and the table was unused.
>>
> 
> Is an OF style match necessary?
>

Did you read the cover letter [0] on which I explain why is needed to
avoid breaking module autoloading in the future? Once the SPI core is
changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).

> I have been using devicetree and it matches based on the .id_table.
>

Yes it fallbacks to the .id_table or the driver name but the correct
thing to do for devices registered by OF, is to match using the
compatible string.

> Couldn't we just remove the mcp320x_dt_ids table instead?
>

No, that is the wrong thing to do IMHO since the compatible string
contains both vendor and device name whle the .id_table only contains
a device name.

So it makes sense to match using the compatible string and also report
the OF modalias information to user-space.

Otherwise what's the point of the vendor in the compatible string for
SPI devices? You can just use "bar" instead of "foo,bar" as a string.

[0]: https://lkml.org/lkml/2015/8/20/109

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

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Michael Welling]

On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
> Hello Michael,
> 
> On 08/20/2015 10:09 PM, Michael Welling wrote:
> > On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> >> The driver has an OF id table but the .of_match_table is not set so
> >> the SPI core can't do an OF style match and the table was unused.
> >>
> > 
> > Is an OF style match necessary?
> >
> 
> Did you read the cover letter [0] on which I explain why is needed to
> avoid breaking module autoloading in the future? Once the SPI core is
> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).

Well I have read it now. :)

> 
> > I have been using devicetree and it matches based on the .id_table.
> >
> 
> Yes it fallbacks to the .id_table or the driver name but the correct
> thing to do for devices registered by OF, is to match using the
> compatible string.
> 
> > Couldn't we just remove the mcp320x_dt_ids table instead?
> >
> 
> No, that is the wrong thing to do IMHO since the compatible string
> contains both vendor and device name whle the .id_table only contains
> a device name.
> 
> So it makes sense to match using the compatible string and also report
> the OF modalias information to user-space.
> 
> Otherwise what's the point of the vendor in the compatible string for
> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
> 

Well then shouldn't the patch include adding the vendor to the compatible
string?


> [0]: https://lkml.org/lkml/2015/8/20/109
> 
> Best regards,
> -- 
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Javier Martinez Canillas]

Hello Michael,

On 08/21/2015 12:29 AM, Michael Welling wrote:
> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>> The driver has an OF id table but the .of_match_table is not set so
>>>> the SPI core can't do an OF style match and the table was unused.
>>>>
>>>
>>> Is an OF style match necessary?
>>>
>>
>> Did you read the cover letter [0] on which I explain why is needed to
>> avoid breaking module autoloading in the future? Once the SPI core is
>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
> 
> Well I have read it now. :)
>

Great :)
 
>>
>>> I have been using devicetree and it matches based on the .id_table.
>>>
>>
>> Yes it fallbacks to the .id_table or the driver name but the correct
>> thing to do for devices registered by OF, is to match using the
>> compatible string.
>>
>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>
>>
>> No, that is the wrong thing to do IMHO since the compatible string
>> contains both vendor and device name whle the .id_table only contains
>> a device name.
>>
>> So it makes sense to match using the compatible string and also report
>> the OF modalias information to user-space.
>>
>> Otherwise what's the point of the vendor in the compatible string for
>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>
> 
> Well then shouldn't the patch include adding the vendor to the compatible
> string?
>

Well, I was talking in general. You are right that this specific driver does
not have a vendor prefix for the compatible strings. This is incorrect
according to the ePAPR document [0].

However, these compatible strings are already documented as a DT binding doc
in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
what is the correct thing to do in this situation.

Changing the compatible string will break old DTB with newer kernels and that
is a no go. But that doesn't invalidate what I said since once the SPI core
is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
regardless if the compatible string has a vendor prefix or not.

[0]: https://www.power.org/documentation/epapr-version-1-1/

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

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Michael Welling]

On Fri, Aug 21, 2015 at 12:48:23AM +0200, Javier Martinez Canillas wrote:
> Hello Michael,
> 
> On 08/21/2015 12:29 AM, Michael Welling wrote:
> > On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
> >> Hello Michael,
> >>
> >> On 08/20/2015 10:09 PM, Michael Welling wrote:
> >>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> >>>> The driver has an OF id table but the .of_match_table is not set so
> >>>> the SPI core can't do an OF style match and the table was unused.
> >>>>
> >>>
> >>> Is an OF style match necessary?
> >>>
> >>
> >> Did you read the cover letter [0] on which I explain why is needed to
> >> avoid breaking module autoloading in the future? Once the SPI core is
> >> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
> > 
> > Well I have read it now. :)
> >
> 
> Great :)
>  
> >>
> >>> I have been using devicetree and it matches based on the .id_table.
> >>>
> >>
> >> Yes it fallbacks to the .id_table or the driver name but the correct
> >> thing to do for devices registered by OF, is to match using the
> >> compatible string.
> >>
> >>> Couldn't we just remove the mcp320x_dt_ids table instead?
> >>>
> >>
> >> No, that is the wrong thing to do IMHO since the compatible string
> >> contains both vendor and device name whle the .id_table only contains
> >> a device name.
> >>
> >> So it makes sense to match using the compatible string and also report
> >> the OF modalias information to user-space.
> >>
> >> Otherwise what's the point of the vendor in the compatible string for
> >> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
> >>
> > 
> > Well then shouldn't the patch include adding the vendor to the compatible
> > string?
> >
> 
> Well, I was talking in general. You are right that this specific driver does
> not have a vendor prefix for the compatible strings. This is incorrect
> according to the ePAPR document [0].
> 
> However, these compatible strings are already documented as a DT binding doc
> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
> what is the correct thing to do in this situation.
> 
> Changing the compatible string will break old DTB with newer kernels and that
> is a no go. But that doesn't invalidate what I said since once the SPI core
> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
> regardless if the compatible string has a vendor prefix or not.
>

Okay.

Acked-by: Michael Welling <mwelling@ieee.org>
 
> [0]: https://www.power.org/documentation/epapr-version-1-1/
> 
> Best regards,
> -- 
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Jonathan Cameron]

On 20/08/15 23:48, Javier Martinez Canillas wrote:
> Hello Michael,
> 
> On 08/21/2015 12:29 AM, Michael Welling wrote:
>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>> Hello Michael,
>>>
>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>
>>>>
>>>> Is an OF style match necessary?
>>>>
>>>
>>> Did you read the cover letter [0] on which I explain why is needed to
>>> avoid breaking module autoloading in the future? Once the SPI core is
>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>
>> Well I have read it now. :)
>>
> 
> Great :)
>  
>>>
>>>> I have been using devicetree and it matches based on the .id_table.
>>>>
>>>
>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>> thing to do for devices registered by OF, is to match using the
>>> compatible string.
>>>
>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>
>>>
>>> No, that is the wrong thing to do IMHO since the compatible string
>>> contains both vendor and device name whle the .id_table only contains
>>> a device name.
>>>
>>> So it makes sense to match using the compatible string and also report
>>> the OF modalias information to user-space.
>>>
>>> Otherwise what's the point of the vendor in the compatible string for
>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>
>>
>> Well then shouldn't the patch include adding the vendor to the compatible
>> string?
>>
> 
> Well, I was talking in general. You are right that this specific driver does
> not have a vendor prefix for the compatible strings. This is incorrect
> according to the ePAPR document [0].
> 
> However, these compatible strings are already documented as a DT binding doc
> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
> what is the correct thing to do in this situation.
Take the view the old version is wrong but needs to be supported and add also
the corrected strings + document them.

cc'd The device tree list for any more comments on this.
> 
> Changing the compatible string will break old DTB with newer kernels and that
> is a no go. But that doesn't invalidate what I said since once the SPI core
> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
> regardless if the compatible string has a vendor prefix or not.
> 
> [0]: https://www.power.org/documentation/epapr-version-1-1/
> 
> Best regards,
> 

Re: [PATCH 02/18] staging: iio: hmc5843: Export missing SPI module alias information

[Jonathan Cameron]

On 20/08/15 08:07, 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.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Applied to the togreg branch of iio.git.

This is too late for the upcoming merge window so it will be queued up for
the next one.

Thanks,

Jonathan
> ---
> 
>  drivers/staging/iio/magnetometer/hmc5843_spi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843_spi.c b/drivers/staging/iio/magnetometer/hmc5843_spi.c
> index 8e658f736e1f..4dfb372d2fec 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843_spi.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843_spi.c
> @@ -81,6 +81,7 @@ static const struct spi_device_id hmc5843_id[] = {
>  	{ "hmc5983", HMC5983_ID },
>  	{ }
>  };
> +MODULE_DEVICE_TABLE(spi, hmc5843_id);
>  
>  static struct spi_driver hmc5843_driver = {
>  		.driver = {
> 

Re: [PATCH 11/18] iio: adc: max1027: Set struct spi_driver .of_match_table

[Jonathan Cameron]

On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The driver has an OF id table but the .of_match_table is not set so
> the SPI core can't do an OF style match and the table was unused.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Applied to the togreg branch of iio.git

Thanks,

Joanthan
> ---
> 
>  drivers/iio/adc/max1027.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
> index 44bf815adb6c..54a8302aaace 100644
> --- a/drivers/iio/adc/max1027.c
> +++ b/drivers/iio/adc/max1027.c
> @@ -508,6 +508,7 @@ static int max1027_remove(struct spi_device *spi)
>  static struct spi_driver max1027_driver = {
>  	.driver = {
>  		.name	= "max1027",
> +		.of_match_table = of_match_ptr(max1027_adc_dt_ids),
>  		.owner	= THIS_MODULE,
>  	},
>  	.probe		= max1027_probe,
> 

Re: [PATCH 14/18] iio: as3935: Add OF match table

[Jonathan Cameron]

On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/proximity/as3935.txt DT binding
> doc lists "ams,as3935" as a compatible string but the corresponding driver
> does not have an OF match table. Add the table to the driver so the SPI
> core can do an OF style match.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play with it.
> ---
> 
>  drivers/iio/proximity/as3935.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
> index bc0d68efd455..e95035136889 100644
> --- a/drivers/iio/proximity/as3935.c
> +++ b/drivers/iio/proximity/as3935.c
> @@ -434,6 +434,12 @@ static int as3935_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static const struct of_device_id as3935_of_match[] = {
> +	{ .compatible = "ams,as3935", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, as3935_of_match);
> +
>  static const struct spi_device_id as3935_id[] = {
>  	{"as3935", 0},
>  	{},
> @@ -443,6 +449,7 @@ MODULE_DEVICE_TABLE(spi, as3935_id);
>  static struct spi_driver as3935_driver = {
>  	.driver = {
>  		.name	= "as3935",
> +		.of_match_table = of_match_ptr(as3935_of_match),
>  		.owner	= THIS_MODULE,
>  		.pm	= AS3935_PM_OPS,
>  	},
> 

Re: [PATCH 15/18] iio: adc128s052: Add OF match table

[Jonathan Cameron]

On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/adc/ti-adc128s052.txt DT binding
> doc lists "ti,adc128s052" or "ti,adc122s021" as compatible strings but the
> corresponding driver does not have an OF match table. Add the table to the
> driver so the SPI core can do an OF style match.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Applied to the togreg branch of iio.git - pushed out as testing for the 
autobuilders to play with it.

Thanks,

Jonathan
> ---
> 
>  drivers/iio/adc/ti-adc128s052.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 915be6b60097..98c0d2b444bf 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -174,6 +174,13 @@ static int adc128_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static const struct of_device_id adc128_of_match[] = {
> +	{ .compatible = "ti,adc128s052", },
> +	{ .compatible = "ti,adc122s021", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, adc128_of_match);
> +
>  static const struct spi_device_id adc128_id[] = {
>  	{ "adc128s052", 0},	/* index into adc128_config */
>  	{ "adc122s021",	1},
> @@ -184,6 +191,7 @@ MODULE_DEVICE_TABLE(spi, adc128_id);
>  static struct spi_driver adc128_driver = {
>  	.driver = {
>  		.name = "adc128s052",
> +		.of_match_table = of_match_ptr(adc128_of_match),
>  		.owner = THIS_MODULE,
>  	},
>  	.probe = adc128_probe,
> 

Re: [PATCH 16/18] iio: frequency: adf4350: Add OF match table

[Jonathan Cameron]

On 20/08/15 16:49, Lars-Peter Clausen wrote:
> On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
>> The Documentation/devicetree/bindings/iio/frequency/adf4350.txt DT binding
>> doc lists "adi,adf4350" or "adi,adf4351" as compatible strings but the
>> corresponding driver does not have an OF match table. Add the table to the
>> driver so the SPI core can do an OF style match.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git

Thanks,
> 
> Thanks.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 10/18] iio: dac: ad7303: Add OF match table

[Jonathan Cameron]

On 20/08/15 16:49, Lars-Peter Clausen wrote:
> On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
>> The Documentation/devicetree/bindings/iio/dac/ad7303.txt DT binding doc
>> lists "adi,ad7303" as a compatible string but the corresponding driver
>> does not have an OF match table. Add the table to the driver so the SPI
>> core can do an OF style match.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git - pushed out as testing.

Thanks,

Jonathan
> 
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 01/18] iio: Export SPI module alias information in missing drivers

[Jonathan Cameron]

On 20/08/15 16:49, Lars-Peter Clausen wrote:
> On 08/20/2015 09:07 AM, 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.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git - initially pushed out as testing.

Thanks,

Jonathan
> 
> Thanks.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Jonathan Cameron]

On 21/08/15 00:53, Michael Welling wrote:
> On Fri, Aug 21, 2015 at 12:48:23AM +0200, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>> Hello Michael,
>>>>
>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>
>>>>>
>>>>> Is an OF style match necessary?
>>>>>
>>>>
>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>
>>> Well I have read it now. :)
>>>
>>
>> Great :)
>>  
>>>>
>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>
>>>>
>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>> thing to do for devices registered by OF, is to match using the
>>>> compatible string.
>>>>
>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>
>>>>
>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>> contains both vendor and device name whle the .id_table only contains
>>>> a device name.
>>>>
>>>> So it makes sense to match using the compatible string and also report
>>>> the OF modalias information to user-space.
>>>>
>>>> Otherwise what's the point of the vendor in the compatible string for
>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>
>>>
>>> Well then shouldn't the patch include adding the vendor to the compatible
>>> string?
>>>
>>
>> Well, I was talking in general. You are right that this specific driver does
>> not have a vendor prefix for the compatible strings. This is incorrect
>> according to the ePAPR document [0].
>>
>> However, these compatible strings are already documented as a DT binding doc
>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>> what is the correct thing to do in this situation.
>>
>> Changing the compatible string will break old DTB with newer kernels and that
>> is a no go. But that doesn't invalidate what I said since once the SPI core
>> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
>> regardless if the compatible string has a vendor prefix or not.
>>
> 
> Okay.
> 
> Acked-by: Michael Welling <mwelling@ieee.org>
Applied to the togreg branch of iio.git

Thanks,

Jonathan
>  
>> [0]: https://www.power.org/documentation/epapr-version-1-1/
>>
>> Best regards,
>> -- 
>> Javier Martinez Canillas
>> Open Source Group
>> Samsung Research America
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Rob Herring]

On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 20/08/15 23:48, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>> Hello Michael,
>>>>
>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>
>>>>>
>>>>> Is an OF style match necessary?
>>>>>
>>>>
>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>
>>> Well I have read it now. :)
>>>
>>
>> Great :)
>>
>>>>
>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>
>>>>
>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>> thing to do for devices registered by OF, is to match using the
>>>> compatible string.
>>>>
>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>
>>>>
>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>> contains both vendor and device name whle the .id_table only contains
>>>> a device name.
>>>>
>>>> So it makes sense to match using the compatible string and also report
>>>> the OF modalias information to user-space.
>>>>
>>>> Otherwise what's the point of the vendor in the compatible string for
>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>
>>>
>>> Well then shouldn't the patch include adding the vendor to the compatible
>>> string?
>>>
>>
>> Well, I was talking in general. You are right that this specific driver does
>> not have a vendor prefix for the compatible strings. This is incorrect
>> according to the ePAPR document [0].
>>
>> However, these compatible strings are already documented as a DT binding doc
>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>> what is the correct thing to do in this situation.
> Take the view the old version is wrong but needs to be supported and add also
> the corrected strings + document them.
>
> cc'd The device tree list for any more comments on this.

Agreed. Document both and mark the old one deprecated.

Rob

Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

[Javier Martinez Canillas]

Hello Rob and Jonathan,

On 08/24/2015 12:10 AM, Rob Herring wrote:
> On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 20/08/15 23:48, Javier Martinez Canillas wrote:
>>> Hello Michael,
>>>
>>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>>> Hello Michael,
>>>>>
>>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>>
>>>>>>
>>>>>> Is an OF style match necessary?
>>>>>>
>>>>>
>>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>>
>>>> Well I have read it now. :)
>>>>
>>>
>>> Great :)
>>>
>>>>>
>>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>>
>>>>>
>>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>>> thing to do for devices registered by OF, is to match using the
>>>>> compatible string.
>>>>>
>>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>>
>>>>>
>>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>>> contains both vendor and device name whle the .id_table only contains
>>>>> a device name.
>>>>>
>>>>> So it makes sense to match using the compatible string and also report
>>>>> the OF modalias information to user-space.
>>>>>
>>>>> Otherwise what's the point of the vendor in the compatible string for
>>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>>
>>>>
>>>> Well then shouldn't the patch include adding the vendor to the compatible
>>>> string?
>>>>
>>>
>>> Well, I was talking in general. You are right that this specific driver does
>>> not have a vendor prefix for the compatible strings. This is incorrect
>>> according to the ePAPR document [0].
>>>
>>> However, these compatible strings are already documented as a DT binding doc
>>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>>> what is the correct thing to do in this situation.
>> Take the view the old version is wrong but needs to be supported and add also
>> the corrected strings + document them.
>>
>> cc'd The device tree list for any more comments on this.
> 
> Agreed. Document both and mark the old one deprecated.
>

Thanks a lot for your comments. I'll do that as a follow up then.
 
> Rob
> 

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

Re: [PATCH 02/18] staging: iio: hmc5843: Export missing SPI module alias information

[Javier Martinez Canillas]

Hello Jonathan,

On 08/22/2015 07:59 PM, Jonathan Cameron wrote:
> On 20/08/15 08:07, 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.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Applied to the togreg branch of iio.git.
> 
> This is too late for the upcoming merge window so it will be queued up for
> the next one.
>

IMHO this patch and "[PATCH 01/18] iio: Export SPI module alias
information in missing drivers" [0] are fixing broken module
autoloading which are bugs so are material for the 4.3 -rc cycle.
 
> Thanks,
> 
> Jonathan

[0]: https://lkml.org/lkml/2015/8/20/111

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

Re: [PATCH 02/18] staging: iio: hmc5843: Export missing SPI module alias information

[Jonathan Cameron]

On 01/09/15 00:09, Javier Martinez Canillas wrote:
> Hello Jonathan,
> 
> On 08/22/2015 07:59 PM, Jonathan Cameron wrote:
>> On 20/08/15 08:07, 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.
>>>
>>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> Applied to the togreg branch of iio.git.
>>
>> This is too late for the upcoming merge window so it will be queued up for
>> the next one.
>>
> 
> IMHO this patch and "[PATCH 01/18] iio: Export SPI module alias
> information in missing drivers" [0] are fixing broken module
> autoloading which are bugs so are material for the 4.3 -rc cycle.

It's a corner case.  Could also be argued that this isn't
a bug but rather a case of a feature (autoprobing) being added
that wasn't supported before.  It's not obligatory to support
autoloading (even if we would normally aim to do so).

I'm happy enough for a request to apply these to stable occurs
after they hit Linus' tree (as a trivial backport) but I don't
think they really deserve being sent on as fixes.

Of course, I might be missing something that means something is
actually broken, as opposed to not present.

Jonathan

>  
>> Thanks,
>>
>> Jonathan
> 
> [0]: https://lkml.org/lkml/2015/8/20/111
> 
> Best regards,
> 

Re: [PATCH 02/18] staging: iio: hmc5843: Export missing SPI module alias information

[Javier Martinez Canillas]

Hello Jonathan,

On 09/05/2015 06:31 PM, Jonathan Cameron wrote:
> On 01/09/15 00:09, Javier Martinez Canillas wrote:
>> Hello Jonathan,
>>
>> On 08/22/2015 07:59 PM, Jonathan Cameron wrote:
>>> On 20/08/15 08:07, 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.
>>>>
>>>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>> Applied to the togreg branch of iio.git.
>>>
>>> This is too late for the upcoming merge window so it will be queued up for
>>> the next one.
>>>
>>
>> IMHO this patch and "[PATCH 01/18] iio: Export SPI module alias
>> information in missing drivers" [0] are fixing broken module
>> autoloading which are bugs so are material for the 4.3 -rc cycle.
> 
> It's a corner case.  Could also be argued that this isn't
> a bug but rather a case of a feature (autoprobing) being added
> that wasn't supported before.  It's not obligatory to support
> autoloading (even if we would normally aim to do so).
> 
> I'm happy enough for a request to apply these to stable occurs
> after they hit Linus' tree (as a trivial backport) but I don't
> think they really deserve being sent on as fixes.
> 
> Of course, I might be missing something that means something is
> actually broken, as opposed to not present.
>

Well, as a user I would expect that if I have a driver built-in
and it works, building it as a module will also work so I think
module autoload it's a bugfix and not a new feature.

And the patch is trivial and won't cause any issues so I don't
see why it can't be -rc material.

But of course is up to you, git log shows me that these drivers
have been since v3.10 so it seems that nobody cared anyways.
 
> Jonathan
> 

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