Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1600592 > unrolled thread

[PATCH 0/4] misc: Add OF device table to I2C drivers that are missing it

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] misc: Add OF device table to I2C drivers that are missing it Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-14 16:30 +0100
    [PATCH 1/4] misc: tsl2550: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-14 16:30 +0100
      Re: [PATCH 1/4] misc: tsl2550: Add OF device ID table Arnd Bergmann <arnd@arndb.de> - 2017-03-14 22:00 +0100
    [PATCH 3/4] eeprom: idt_89hpesx: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-14 16:30 +0100
    [PATCH 2/4] misc: ds1682: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-14 16:30 +0100
      Re: [PATCH 2/4] misc: ds1682: Add OF device ID table Arnd Bergmann <arnd@arndb.de> - 2017-03-14 21:50 +0100
        Re: [PATCH 2/4] misc: ds1682: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-15 01:40 +0100
          Re: [PATCH 2/4] misc: ds1682: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-15 04:30 +0100
    [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-14 16:30 +0100
      Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-15 00:10 +0100
        Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-15 01:20 +0100
          Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Wolfram Sang <wsa@the-dreams.de> - 2017-03-15 09:00 +0100
            Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-15 12:10 +0100
              Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-15 12:30 +0100
                Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-15 12:50 +0100
                  Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-15 23:50 +0100
                    Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-16 13:30 +0100
              Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Wolfram Sang <wsa@the-dreams.de> - 2017-03-16 14:10 +0100
                Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-16 14:20 +0100
                  Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Wolfram Sang <wsa@the-dreams.de> - 2017-03-16 14:40 +0100
                    Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-16 15:10 +0100
                      Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Wolfram Sang <wsa@the-dreams.de> - 2017-03-16 16:10 +0100
                        Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-16 16:40 +0100
                          Re: [PATCH 4/4] eeprom: at24: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-20 17:50 +0100

Page 1 of 2  [1] 2  Next page →

#1600592 — [PATCH 0/4] misc: Add OF device table to I2C drivers that are missing it

Hello,

This series add OF device ID tables to misc I2C drivers whose devices are
either used in Device Tree source files or are listed in binding docs as
a compatible string.

That's done because the plan is to change the I2C core to report proper OF
modaliases instead of always reporting a MODALIAS=i2c:<foo> regardless if
a device was registered via DT or using the legacy platform data mechanism.

So these patches will make sure that misc I2C drivers modules will continue
to be autoloaded once the I2C core is changed to report proper OF modalias.

Best regards,
Javier


Javier Martinez Canillas (4):
  misc: tsl2550: Add OF device ID table
  misc: ds1682: Add OF device ID table
  eeprom: idt_89hpesx: Add OF device ID table
  eeprom: at24: Add OF device ID table

 drivers/misc/ds1682.c             |   7 ++
 drivers/misc/eeprom/at24.c        | 189 +++++++++++++++++++++++++++++++++++++-
 drivers/misc/eeprom/idt_89hpesx.c |  57 ++++++++++++
 drivers/misc/tsl2550.c            |   7 ++
 4 files changed, 259 insertions(+), 1 deletion(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1600595 — [PATCH 1/4] misc: tsl2550: Add OF device ID table

The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.

But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.

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

 drivers/misc/tsl2550.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/misc/tsl2550.c b/drivers/misc/tsl2550.c
index 87a13374fdc0..9e7d01c68c68 100644
--- a/drivers/misc/tsl2550.c
+++ b/drivers/misc/tsl2550.c
@@ -443,9 +443,16 @@ static const struct i2c_device_id tsl2550_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, tsl2550_id);
 
+static const struct of_device_id tsl2550_of_match[] = {
+	{ .compatible = "taos,tsl2550" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tsl2550_of_match);
+
 static struct i2c_driver tsl2550_driver = {
 	.driver = {
 		.name	= TSL2550_DRV_NAME,
+		.of_match_table = of_match_ptr(tsl2550_of_match),
 		.pm	= TSL2550_PM_OPS,
 	},
 	.probe	= tsl2550_probe,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1600901 — Re: [PATCH 1/4] misc: tsl2550: Add OF device ID table

\
>  static struct i2c_driver tsl2550_driver = {
>         .driver = {
>                 .name   = TSL2550_DRV_NAME,
> +               .of_match_table = of_match_ptr(tsl2550_of_match),
>                 .pm     = TSL2550_PM_OPS,
>         },

Please drop the incorrect of_match_ptr().

    Arnd

[toc] | [prev] | [next] | [standalone]

#1600599 — [PATCH 3/4] eeprom: idt_89hpesx: Add OF device ID table

The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.

But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.

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

 drivers/misc/eeprom/idt_89hpesx.c | 57 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/misc/eeprom/idt_89hpesx.c b/drivers/misc/eeprom/idt_89hpesx.c
index 4a22a1d99395..ec99f823a4ef 100644
--- a/drivers/misc/eeprom/idt_89hpesx.c
+++ b/drivers/misc/eeprom/idt_89hpesx.c
@@ -1541,12 +1541,69 @@ static const struct i2c_device_id idt_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, idt_ids);
 
+static const struct of_device_id idt_of_match[] = {
+	{ .compatible = "idt,89hpes8nt2", },
+	{ .compatible = "idt,89hpes12nt3", },
+
+	{ .compatible = "idt,89hpes24nt6ag2", },
+	{ .compatible = "idt,89hpes32nt8ag2", },
+	{ .compatible = "idt,89hpes32nt8bg2", },
+	{ .compatible = "idt,89hpes12nt12g2", },
+	{ .compatible = "idt,89hpes16nt16g2", },
+	{ .compatible = "idt,89hpes24nt24g2", },
+	{ .compatible = "idt,89hpes32nt24ag2", },
+	{ .compatible = "idt,89hpes32nt24bg2", },
+
+	{ .compatible = "idt,89hpes12n3", },
+	{ .compatible = "idt,89hpes12n3a", },
+	{ .compatible = "idt,89hpes24n3", },
+	{ .compatible = "idt,89hpes24n3a", },
+
+	{ .compatible = "idt,89hpes32h8", },
+	{ .compatible = "idt,89hpes32h8g2", },
+	{ .compatible = "idt,89hpes48h12", },
+	{ .compatible = "idt,89hpes48h12g2", },
+	{ .compatible = "idt,89hpes48h12ag2", },
+	{ .compatible = "idt,89hpes16h16", },
+	{ .compatible = "idt,89hpes22h16", },
+	{ .compatible = "idt,89hpes22h16g2", },
+	{ .compatible = "idt,89hpes34h16", },
+	{ .compatible = "idt,89hpes34h16g2", },
+	{ .compatible = "idt,89hpes64h16", },
+	{ .compatible = "idt,89hpes64h16g2", },
+	{ .compatible = "idt,89hpes64h16ag2", },
+
+	{ .compatible = "idt,89hpes12t3g2", },
+	{ .compatible = "idt,89hpes24t3g2", },
+
+	{ .compatible = "idt,89hpes16t4", },
+	{ .compatible = "idt,89hpes4t4g2", },
+	{ .compatible = "idt,89hpes10t4g2", },
+	{ .compatible = "idt,89hpes16t4g2", },
+	{ .compatible = "idt,89hpes16t4ag2", },
+	{ .compatible = "idt,89hpes5t5", },
+	{ .compatible = "idt,89hpes6t5", },
+	{ .compatible = "idt,89hpes8t5", },
+	{ .compatible = "idt,89hpes8t5a", },
+	{ .compatible = "idt,89hpes24t6", },
+	{ .compatible = "idt,89hpes6t6g2", },
+	{ .compatible = "idt,89hpes24t6g2", },
+	{ .compatible = "idt,89hpes16t7", },
+	{ .compatible = "idt,89hpes32t8", },
+	{ .compatible = "idt,89hpes32t8g2", },
+	{ .compatible = "idt,89hpes48t12", },
+	{ .compatible = "idt,89hpes48t12g2", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, idt_of_match);
+
 /*
  * idt_driver - IDT 89HPESx driver structure
  */
 static struct i2c_driver idt_driver = {
 	.driver = {
 		.name = IDT_NAME,
+		.of_match_table = of_match_ptr(idt_of_match),
 	},
 	.probe = idt_probe,
 	.remove = idt_remove,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1600605 — [PATCH 2/4] misc: ds1682: Add OF device ID table

The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.

But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.

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

 drivers/misc/ds1682.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/misc/ds1682.c b/drivers/misc/ds1682.c
index c7112276a039..a9ad06646a9b 100644
--- a/drivers/misc/ds1682.c
+++ b/drivers/misc/ds1682.c
@@ -227,9 +227,16 @@ static const struct i2c_device_id ds1682_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ds1682_id);
 
+static const struct of_device_id ds1682_of_match[] = {
+	{ .compatible = "dallas,ds1682", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ds1682_of_match);
+
 static struct i2c_driver ds1682_driver = {
 	.driver = {
 		.name = "ds1682",
+		.of_match_table = of_match_ptr(ds1682_of_match),
 	},
 	.probe = ds1682_probe,
 	.remove = ds1682_remove,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1600889 — Re: [PATCH 2/4] misc: ds1682: Add OF device ID table

On Tue, Mar 14, 2017 at 4:16 PM, Javier Martinez Canillas
<javier@osg.samsung.com> wrote:

> +static const struct of_device_id ds1682_of_match[] = {
> +       { .compatible = "dallas,ds1682", },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, ds1682_of_match);
> +
>  static struct i2c_driver ds1682_driver = {
>         .driver = {
>                 .name = "ds1682",
> +               .of_match_table = of_match_ptr(ds1682_of_match),
>         },
>         .probe = ds1682_probe,
>         .remove = ds1682_remove,

This will cause a warning if CONFIG_OF is disabled, since ds1682_of_match
becomes unused in this case. Please remove the of_match_ptr() around
the reference to ds1682_of_match.

    Arnd

[toc] | [prev] | [next] | [standalone]

#1600965 — Re: [PATCH 2/4] misc: ds1682: Add OF device ID table

Hello Arnd,

Thanks a lot for your feedback.

On 03/14/2017 05:46 PM, Arnd Bergmann wrote:
> On Tue, Mar 14, 2017 at 4:16 PM, Javier Martinez Canillas
> <javier@osg.samsung.com> wrote:
> 
>> +static const struct of_device_id ds1682_of_match[] = {
>> +       { .compatible = "dallas,ds1682", },
>> +       {}
>> +};
>> +MODULE_DEVICE_TABLE(of, ds1682_of_match);
>> +
>>  static struct i2c_driver ds1682_driver = {
>>         .driver = {
>>                 .name = "ds1682",
>> +               .of_match_table = of_match_ptr(ds1682_of_match),
>>         },
>>         .probe = ds1682_probe,
>>         .remove = ds1682_remove,
> 
> This will cause a warning if CONFIG_OF is disabled, since ds1682_of_match
> becomes unused in this case. Please remove the of_match_ptr() around
> the reference to ds1682_of_match.
>

Right, I tested it when CONFIG_OF is disabled with gcc (GCC) 6.3.1 20161221
and I didn't see any warning. But you are right and I'll re-spin the series
without using the macro.

>     Arnd
> 

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

[toc] | [prev] | [next] | [standalone]

#1601018 — Re: [PATCH 2/4] misc: ds1682: Add OF device ID table

On 03/14/2017 09:38 PM, Javier Martinez Canillas wrote:
> Hello Arnd,
> 
> Thanks a lot for your feedback.
> 
> On 03/14/2017 05:46 PM, Arnd Bergmann wrote:
>> On Tue, Mar 14, 2017 at 4:16 PM, Javier Martinez Canillas
>> <javier@osg.samsung.com> wrote:
>>
>>> +static const struct of_device_id ds1682_of_match[] = {
>>> +       { .compatible = "dallas,ds1682", },
>>> +       {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, ds1682_of_match);
>>> +
>>>  static struct i2c_driver ds1682_driver = {
>>>         .driver = {
>>>                 .name = "ds1682",
>>> +               .of_match_table = of_match_ptr(ds1682_of_match),
>>>         },
>>>         .probe = ds1682_probe,
>>>         .remove = ds1682_remove,
>>
>> This will cause a warning if CONFIG_OF is disabled, since ds1682_of_match
>> becomes unused in this case. Please remove the of_match_ptr() around
>> the reference to ds1682_of_match.
>>
> 
> Right, I tested it when CONFIG_OF is disabled with gcc (GCC) 6.3.1 20161221
> and I didn't see any warning. But you are right and I'll re-spin the series

I didn't notice the build warning because I forgot to build with W=1 before
posting, sorry about that.

> without using the macro.
> 

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

[toc] | [prev] | [next] | [standalone]

#1600609 — [PATCH 4/4] eeprom: at24: Add OF device ID table

The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.

But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.

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

---
Hello,

I added in the OF device ID table all the manufacturer,model combinations
that I found on either a DT bindings doc or Device Tree source files, but
the Documentation/devicetree/bindings/eeprom/eeprom.txt docs says that:

- compatible : should be "<manufacturer>,<type>", like these:

And give some examples. I wonder if such a lax definition for compatibles
is valid or if all possible combinations should be documented in the doc.

I would had expected the latter and keep both the OF device ID table and
bindings document updated as compatible EEPROM from a new vendor is used.

Best regards,
Javier

 drivers/misc/eeprom/at24.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 188 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 764ff5df0dbc..b67b804855f9 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -12,6 +12,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>
@@ -175,6 +176,188 @@ static const struct i2c_device_id at24_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, at24_ids);
 
+static const struct of_device_id at24_of_match[] = {
+	{
+		.compatible = "atmel,24c00",
+		.data = (void *)AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR)
+	},
+	{
+		.compatible = "atmel,24c01",
+		.data = (void *)AT24_DEVICE_MAGIC(1024 / 8, 0)
+	},
+	{
+		.compatible = "at24,24c02",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_SERIAL | AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "atmel,24c02",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_SERIAL | AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "microchip,24c02",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_SERIAL | AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "nxp,24c02",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_SERIAL | AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "renesas,24c02",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_SERIAL | AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "renesas,r1ex24002",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_SERIAL | AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "at24,spd",
+		.data = (void *)AT24_DEVICE_MAGIC(2048 / 8,
+				AT24_FLAG_READONLY | AT24_FLAG_IRUGO)
+	},
+	{
+		.compatible = "at,24c04",
+		.data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0)
+	},
+	{
+		.compatible = "at24,24c04",
+		.data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0)
+	},
+	{
+		.compatible = "atmel,24c04",
+		.data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0)
+	},
+	{
+		.compatible = "at,24c08",
+		.data = (void *)AT24_DEVICE_MAGIC(8192 / 8, 0)
+	},
+	{
+		.compatible = "atmel,24c08",
+		.data = (void *)AT24_DEVICE_MAGIC(8192 / 8, 0)
+	},
+	{
+		.compatible = "atmel,24c16",
+		.data = (void *)AT24_DEVICE_MAGIC(16384 / 8, 0)
+	},
+	{
+		.compatible = "at,24c32",
+		.data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "at24,24c32",
+		.data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "atmel,24c32",
+		.data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "catalyst,24c32",
+		.data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "microchip,24c32",
+		.data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "at,24c64",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_ADDR16 |
+				AT24_FLAG_SERIAL |
+				AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "at24,24c64",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_ADDR16 |
+				AT24_FLAG_SERIAL |
+				AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "atmel,24c64",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_ADDR16 |
+				AT24_FLAG_SERIAL |
+				AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "microchip,24c64",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_ADDR16 |
+				AT24_FLAG_SERIAL |
+				AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "ramtron,24c64",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_ADDR16 |
+				AT24_FLAG_SERIAL |
+				AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "st,24c64",
+		.data = (void *)AT24_DEVICE_MAGIC(16,
+				AT24_FLAG_ADDR16 |
+				AT24_FLAG_SERIAL |
+				AT24_FLAG_READONLY)
+	},
+	{
+		.compatible = "atmel,24c128",
+		.data = (void *)AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "renesas,24c128",
+		.data = (void *)AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "at,24c256",
+		.data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "at24,24c256",
+		.data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "atmel,24c256",
+		.data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "st,24c256",
+		.data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "at24,24c512",
+		.data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "atmel,24c512",
+		.data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "microchip,24c512",
+		.data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "at,24c1024",
+		.data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "atmel,24c1024",
+		.data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16)
+	},
+	{
+		.compatible = "st,24c1024",
+		.data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16)
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, at24_of_match);
+
 static const struct acpi_device_id at24_acpi_ids[] = {
 	{ "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) },
 	{ }
@@ -598,7 +781,10 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (client->dev.platform_data) {
 		chip = *(struct at24_platform_data *)client->dev.platform_data;
 	} else {
-		if (id) {
+		if (client->dev.of_node) {
+			magic = (kernel_ulong_t)
+				of_device_get_match_data(&client->dev);
+		} else if (id) {
 			magic = id->driver_data;
 		} else {
 			const struct acpi_device_id *aid;
@@ -814,6 +1000,7 @@ static int at24_remove(struct i2c_client *client)
 static struct i2c_driver at24_driver = {
 	.driver = {
 		.name = "at24",
+		.of_match_table = of_match_ptr(at24_of_match),
 		.acpi_match_table = ACPI_PTR(at24_acpi_ids),
 	},
 	.probe = at24_probe,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1600947 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

On Tue, Mar 14, 2017 at 5:16 PM, Javier Martinez Canillas
<javier@osg.samsung.com> wrote:
> The driver doesn't have a struct of_device_id table but supported devices
> are registered via Device Trees. This is working on the assumption that a
> I2C device registered via OF will always match a legacy I2C device ID and
> that the MODALIAS reported will always be of the form i2c:<device>.
>
> But this could change in the future so the correct approach is to have an
> OF device ID table if the devices are registered via OF.

I'm bot sure this patch does something useful right now. Can we
survive without it? I think we may.

>  drivers/misc/eeprom/at24.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 188 insertions(+), 1 deletion(-)

It's a huge! It will increase not only driver code base but memory
footprint for almost no benefit.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1600959 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

Hello Andy,

On 03/14/2017 07:59 PM, Andy Shevchenko wrote:
> On Tue, Mar 14, 2017 at 5:16 PM, Javier Martinez Canillas
> <javier@osg.samsung.com> wrote:
>> The driver doesn't have a struct of_device_id table but supported devices
>> are registered via Device Trees. This is working on the assumption that a
>> I2C device registered via OF will always match a legacy I2C device ID and
>> that the MODALIAS reported will always be of the form i2c:<device>.
>>
>> But this could change in the future so the correct approach is to have an
>> OF device ID table if the devices are registered via OF.
> 
> I'm bot sure this patch does something useful right now. Can we
> survive without it? I think we may.
> 

Yes, we can survive without it for now. But the problem is that with current
I2C core, DT-only I2C drivers must have an I2C device table in order to have
module auto-load working. That's because the core always reports as modalias
i2c:<foo> regardless if the device was registered via DT or legacy mechanism.

And some maintainers don't accept patches doing this duplication and instead
ask for the core to be fixed, i.e [0]. But to make sure that fixing the core
won't add regressions in drivers that are relying in the current behavior,
patches like $SUBJECT are needed.

So there isn't an agreement if is better to just rely in the current behavior
(and have a superfluous I2C device ID table) or fix the I2C core (and need a
OF device ID table). I personally prefer the latter since that means that an
DT-only driver will only need a OF table, and only drivers that supports both
will need both tables.

>>  drivers/misc/eeprom/at24.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 188 insertions(+), 1 deletion(-)
> 
> It's a huge! It will increase not only driver code base but memory
> footprint for almost no benefit.
>

Indeed, but these all are compatible strings used by DTS in mainline and so
should be in the OF device ID table in order to be matched and the proper
modalias reported (once the I2C core is fixed).

One option is to add #ifdef CONFIG_OF guards for the OF device table definition
but again there's no agreement on that one since some maintainers say the it is
better to always build the OF ID table than having #ifdefery in C code...

[0]: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1321026.html

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

[toc] | [prev] | [next] | [standalone]

#1601121 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

[Multipart message — attachments visible in raw view] — view raw

> So there isn't an agreement if is better to just rely in the current behavior
> (and have a superfluous I2C device ID table) or fix the I2C core (and need a
> OF device ID table).

For at24, the i2c_device_id table is not superfluous! It is used outside
the DT world as well.

> Indeed, but these all are compatible strings used by DTS in mainline and so
> should be in the OF device ID table in order to be matched and the proper
> modalias reported (once the I2C core is fixed).

I'd think we should fix the DTS files instead to contain a fallback we
agree on. Say, we agree on "atmel,at24c01" as a the generic fallback,
the DTS should contain:

	compatible = "<your_vendor>,<your_type>", "atmel,at24c01"

And we shall only keep compatible values in the source file which differ
in behaviour fromt the generic case.

> One option is to add #ifdef CONFIG_OF guards for the OF device table definition
> but again there's no agreement on that one since some maintainers say the it is
> better to always build the OF ID table than having #ifdefery in C code...

I don't like the #ifdeffery as well.

[toc] | [prev] | [next] | [standalone]

#1601274 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

Hello Wolfram,

On 03/15/2017 04:58 AM, Wolfram Sang wrote:
> 
>> So there isn't an agreement if is better to just rely in the current behavior
>> (and have a superfluous I2C device ID table) or fix the I2C core (and need a
>> OF device ID table).
> 
> For at24, the i2c_device_id table is not superfluous! It is used outside
> the DT world as well.
>

Yes, I know. I was trying to explain to Andy what's the problem that I want to
solve in general, not talking about this particular driver. Sorry if that was
confusing.
 
>> Indeed, but these all are compatible strings used by DTS in mainline and so
>> should be in the OF device ID table in order to be matched and the proper
>> modalias reported (once the I2C core is fixed).
> 
> I'd think we should fix the DTS files instead to contain a fallback we
> agree on. Say, we agree on "atmel,at24c01" as a the generic fallback,
> the DTS should contain:
> 
> 	compatible = "<your_vendor>,<your_type>", "atmel,at24c01"
> 
> And we shall only keep compatible values in the source file which differ
> in behaviour fromt the generic case.
>

Do you know who that's familiar with this device and driver can do this? I've
been trying to fix all the drivers that are relying on having an I2C ID table
but whose devices are registered via DT and I've now posted patches for all.

I wanted to do that so this patch can finally land [0] but to be honest I'm
about to give up on this... it seems is causing a lot of churn to maintainers
and many don't see the benefit on having the I2C core to report a proper OF
modalias, and don't think the current behavior is particularly bad.

Unfortunately some maintainers do and don't accept patches adding I2C tables
only to have module autoloading working so I still think it should be fixed.

[0]: https://patchwork.kernel.org/patch/6903991/

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

[toc] | [prev] | [next] | [standalone]

#1601308 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

On Wed, Mar 15, 2017 at 12:58 PM, Javier Martinez Canillas
<javier@osg.samsung.com> wrote:
> On 03/15/2017 04:58 AM, Wolfram Sang wrote:

> Unfortunately some maintainers do and don't accept patches adding I2C tables
> only to have module autoloading working so I still think it should be fixed.

Wait, how does it work for now?!

Sounds for me you are trying to solve non-existing issue.

> [0]: https://patchwork.kernel.org/patch/6903991/

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1601336 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

Hello Andy,

On 03/15/2017 08:21 AM, Andy Shevchenko wrote:
> On Wed, Mar 15, 2017 at 12:58 PM, Javier Martinez Canillas
> <javier@osg.samsung.com> wrote:
>> On 03/15/2017 04:58 AM, Wolfram Sang wrote:
> 
>> Unfortunately some maintainers do and don't accept patches adding I2C tables
>> only to have module autoloading working so I still think it should be fixed.
> 
> Wait, how does it work for now?!
>

It only works if you have an I2C device ID table, but that may not be the case
for DT-only drivers that could only have an OF device ID table. In the latter
case module autoload won't work.

> Sounds for me you are trying to solve non-existing issue.
>

It's an existing issue. You _must_ have an I2C device ID table if you want to
autload a device driver which is superfluous for DT-only drivers.

In other words, if you register an I2C device using OF the modalias will be:

$ cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias
i2c:maxtouch

While the correct thing to report should be:

$ cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias
of:NtrackpadT<NULL>Catmel,maxtouch

>> [0]: https://patchwork.kernel.org/patch/6903991/
> 

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

[toc] | [prev] | [next] | [standalone]

#1601792 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

On Wed, Mar 15, 2017 at 1:39 PM, Javier Martinez Canillas
<javier@osg.samsung.com> wrote:
> Hello Andy,
>
> On 03/15/2017 08:21 AM, Andy Shevchenko wrote:
>> On Wed, Mar 15, 2017 at 12:58 PM, Javier Martinez Canillas
>> <javier@osg.samsung.com> wrote:
>>> On 03/15/2017 04:58 AM, Wolfram Sang wrote:
>>
>>> Unfortunately some maintainers do and don't accept patches adding I2C tables
>>> only to have module autoloading working so I still think it should be fixed.
>>
>> Wait, how does it work for now?!

> It only works if you have an I2C device ID table, but that may not be the case
> for DT-only drivers that could only have an OF device ID table. In the latter
> case module autoload won't work.

OK.

>> Sounds for me you are trying to solve non-existing issue.

> It's an existing issue. You _must_ have an I2C device ID table if you want to
> autload a device driver which is superfluous for DT-only drivers.

Okay, can you scope only affected drivers then? Looking to spread
patches from you over all drivers I dunno they are all affected right
now.

P.S. Personally I agree with maintainers who do *not* apply this. Sorry.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1602224 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

Hello Andy,

On 03/15/2017 07:43 PM, Andy Shevchenko wrote:
> On Wed, Mar 15, 2017 at 1:39 PM, Javier Martinez Canillas
> <javier@osg.samsung.com> wrote:
>> Hello Andy,
>>
>> On 03/15/2017 08:21 AM, Andy Shevchenko wrote:
>>> On Wed, Mar 15, 2017 at 12:58 PM, Javier Martinez Canillas
>>> <javier@osg.samsung.com> wrote:
>>>> On 03/15/2017 04:58 AM, Wolfram Sang wrote:
>>>
>>>> Unfortunately some maintainers do and don't accept patches adding I2C tables
>>>> only to have module autoloading working so I still think it should be fixed.
>>>
>>> Wait, how does it work for now?!
> 
>> It only works if you have an I2C device ID table, but that may not be the case
>> for DT-only drivers that could only have an OF device ID table. In the latter
>> case module autoload won't work.
> 
> OK.
> 
>>> Sounds for me you are trying to solve non-existing issue.
> 
>> It's an existing issue. You _must_ have an I2C device ID table if you want to
>> autload a device driver which is superfluous for DT-only drivers.
> 
> Okay, can you scope only affected drivers then? Looking to spread
> patches from you over all drivers I dunno they are all affected right
> now.
>

That's what I did. I've only posted patches for drivers that have DT support
but don't have an OF device ID table since module autoload will be broken for
those if the I2C core is fixed to report a proper OF modalias.

And drivers/misc/eeprom/at24.c is one of those drivers.
 
> P.S. Personally I agree with maintainers who do *not* apply this. Sorry.
> 

So what's your suggestion to solve the issue then? When I said that some
maintainers don't want a superfluous device table to be added I was talking
about I2C device ID table for DT-only drivers, but $SUBJECT is the opposite.

I've the impression that you are nacking $SUBJECT without fully understanding
what the problem is and how this patch + the patch for I2C core are fixing it.

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

[toc] | [prev] | [next] | [standalone]

#1602259 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

[Multipart message — attachments visible in raw view] — view raw

> > I'd think we should fix the DTS files instead to contain a fallback we
> > agree on. Say, we agree on "atmel,at24c01" as a the generic fallback,
> > the DTS should contain:
> > 
> > 	compatible = "<your_vendor>,<your_type>", "atmel,at24c01"
> > 
> > And we shall only keep compatible values in the source file which differ
> > in behaviour fromt the generic case.
> >
> 
> Do you know who that's familiar with this device and driver can do this? I've

Sorry, I don't understand your question. What do you mean?

[toc] | [prev] | [next] | [standalone]

#1602263 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

Hello Wolfram,

On 03/16/2017 10:07 AM, Wolfram Sang wrote:
> 
>>> I'd think we should fix the DTS files instead to contain a fallback we
>>> agree on. Say, we agree on "atmel,at24c01" as a the generic fallback,
>>> the DTS should contain:
>>>
>>> 	compatible = "<your_vendor>,<your_type>", "atmel,at24c01"
>>>
>>> And we shall only keep compatible values in the source file which differ
>>> in behaviour fromt the generic case.
>>>
>>
>> Do you know who that's familiar with this device and driver can do this? I've
> 
> Sorry, I don't understand your question. What do you mean?
> 

Sorry, for not explaining myself correctly. I meant to ask who can do what you
suggested before. I'm certainly not familiar with this driver to identify what
is the minimum set of compatible strings that can be used as generic fallback.

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

[toc] | [prev] | [next] | [standalone]

#1602277 — Re: [PATCH 4/4] eeprom: at24: Add OF device ID table

[Multipart message — attachments visible in raw view] — view raw

> Sorry, for not explaining myself correctly. I meant to ask who can do what you
> suggested before. I'm certainly not familiar with this driver to identify what
> is the minimum set of compatible strings that can be used as generic fallback.

Well, I am the maintainer of this driver :) But we should definately get
Rob into the boat if he is OK with updating all DTS files having such an
EEPROM.

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web