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

Groups > linux.kernel > #1592001 > unrolled thread

[PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 15:40 +0100
    [PATCH 08/13] rtc: ds1374: Set .of_match_table to OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 15:40 +0100
    [PATCH 02/13] rtc: rv3029: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 15:40 +0100
    [PATCH 05/13] rtc: rx8010: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 15:40 +0100
    [PATCH 07/13] rtc: rtc-ds1672: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 15:40 +0100
    [PATCH 01/13] rtc: rv8803: Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 15:40 +0100
    Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are  missing it Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-03-03 16:20 +0100
      Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are  missing it Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 16:50 +0100
        Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are  missing it Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-03-03 17:30 +0100
          Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are  missing it Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-03 18:20 +0100
    Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are  missing it Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-03-09 04:40 +0100

#1592001 — [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

Hello,

This series add OF device ID tables to RTC 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.

So these patches will make sure that RTC 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 (13):
  rtc: rv8803: Add OF device ID table
  rtc: rv3029: Add OF device ID table
  rtc: bq32k: Add OF device ID table
  rtc: ds1307: Add OF device ID table
  rtc: rx8010: Add OF device ID table
  rtc: ds3232: Add OF device ID table
  rtc: rtc-ds1672: Add OF device ID table
  rtc: ds1374: Set .of_match_table to OF device ID table
  rtc: isl1208: Add OF device ID table
  rtc: s35390a: Add OF device ID table
  rtc: rx8581: Add OF device ID table
  rtc: m41t80: Add OF device ID table
  rtc: rs5c372: Add OF device ID table

 drivers/rtc/rtc-bq32k.c    |  7 +++++
 drivers/rtc/rtc-ds1307.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++-
 drivers/rtc/rtc-ds1374.c   |  1 +
 drivers/rtc/rtc-ds1672.c   |  9 +++++-
 drivers/rtc/rtc-ds3232.c   |  7 +++++
 drivers/rtc/rtc-isl1208.c  | 12 ++++++--
 drivers/rtc/rtc-m41t80.c   | 63 ++++++++++++++++++++++++++++++++++++++++--
 drivers/rtc/rtc-rs5c372.c  | 37 ++++++++++++++++++++++++-
 drivers/rtc/rtc-rv3029c2.c |  9 ++++++
 drivers/rtc/rtc-rv8803.c   | 21 +++++++++++++-
 drivers/rtc/rtc-rx8010.c   |  7 +++++
 drivers/rtc/rtc-rx8581.c   |  7 +++++
 drivers/rtc/rtc-s35390a.c  |  8 ++++++
 13 files changed, 248 insertions(+), 8 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1592002 — [PATCH 08/13] rtc: ds1374: Set .of_match_table to OF device ID table

The driver has a OF device ID table but the struct i2c_driver
.of_match_table field is not set.

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

 drivers/rtc/rtc-ds1374.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 52429f0a57cc..4cd115e93223 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -704,6 +704,7 @@ static SIMPLE_DEV_PM_OPS(ds1374_pm, ds1374_suspend, ds1374_resume);
 static struct i2c_driver ds1374_driver = {
 	.driver = {
 		.name = "rtc-ds1374",
+		.of_match_table = of_match_ptr(ds1374_of_match),
 		.pm = &ds1374_pm,
 	},
 	.probe = ds1374_probe,
-- 
2.9.3

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

#1592003 — [PATCH 02/13] rtc: rv3029: 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/rtc/rtc-rv3029c2.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/rtc/rtc-rv3029c2.c b/drivers/rtc/rtc-rv3029c2.c
index 1f9f7b4bf3fb..85fa1da03762 100644
--- a/drivers/rtc/rtc-rv3029c2.c
+++ b/drivers/rtc/rtc-rv3029c2.c
@@ -875,9 +875,18 @@ static struct i2c_device_id rv3029_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rv3029_id);
 
+static const struct of_device_id rv3029_of_match[] = {
+	{ .compatible = "rv3029" },
+	{ .compatible = "rv3029c2" },
+	{ .compatible = "mc,rv3029c2" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rv3029_of_match);
+
 static struct i2c_driver rv3029_driver = {
 	.driver = {
 		.name = "rtc-rv3029c2",
+		.of_match_table = of_match_ptr(rv3029_of_match),
 	},
 	.probe		= rv3029_i2c_probe,
 	.id_table	= rv3029_id,
-- 
2.9.3

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

#1592004 — [PATCH 05/13] rtc: rx8010: 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/rtc/rtc-rx8010.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index d08da371912c..1ed3403ff8ac 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -59,6 +59,12 @@ static const struct i2c_device_id rx8010_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rx8010_id);
 
+static const struct of_device_id rx8010_of_match[] = {
+	{ .compatible = "epson,rx8010" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rx8010_of_match);
+
 struct rx8010_data {
 	struct i2c_client *client;
 	struct rtc_device *rtc;
@@ -487,6 +493,7 @@ static int rx8010_probe(struct i2c_client *client,
 static struct i2c_driver rx8010_driver = {
 	.driver = {
 		.name = "rtc-rx8010",
+		.of_match_table = of_match_ptr(rx8010_of_match),
 	},
 	.probe		= rx8010_probe,
 	.id_table	= rx8010_id,
-- 
2.9.3

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

#1592006 — [PATCH 07/13] rtc: rtc-ds1672: 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/rtc/rtc-ds1672.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
index 5c18ac7394c4..7bf46bfe11a4 100644
--- a/drivers/rtc/rtc-ds1672.c
+++ b/drivers/rtc/rtc-ds1672.c
@@ -196,10 +196,17 @@ static struct i2c_device_id ds1672_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ds1672_id);
 
+static const struct of_device_id ds1672_of_match[] = {
+	{ .compatible = "dallas,ds1672" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ds1672_of_match);
+
 static struct i2c_driver ds1672_driver = {
 	.driver = {
 		   .name = "rtc-ds1672",
-		   },
+		   .of_match_table = of_match_ptr(ds1672_of_match),
+	},
 	.probe = &ds1672_probe,
 	.id_table = ds1672_id,
 };
-- 
2.9.3

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

#1592008 — [PATCH 01/13] rtc: rv8803: 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/rtc/rtc-rv8803.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index f9277e536f7e..9ad97ab29866 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/rtc.h>
 
 #define RV8803_I2C_TRY_COUNT		4
@@ -556,7 +557,11 @@ static int rv8803_probe(struct i2c_client *client,
 
 	mutex_init(&rv8803->flags_lock);
 	rv8803->client = client;
-	rv8803->type = id->driver_data;
+	if (client->dev.of_node)
+		rv8803->type = (enum rv8803_type)
+			of_device_get_match_data(&client->dev);
+	else
+		rv8803->type = id->driver_data;
 	i2c_set_clientdata(client, rv8803);
 
 	flags = rv8803_read_reg(client, RV8803_FLAG);
@@ -627,9 +632,23 @@ static const struct i2c_device_id rv8803_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rv8803_id);
 
+static const struct of_device_id rv8803_of_match[] = {
+	{
+		.compatible = "microcrystal,rv8803",
+		.data = (void *)rx_8900
+	},
+	{
+		.compatible = "epson,rx8900",
+		.data = (void *)rx_8900
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rv8803_of_match);
+
 static struct i2c_driver rv8803_driver = {
 	.driver = {
 		.name = "rtc-rv8803",
+		.of_match_table = of_match_ptr(rv8803_of_match),
 	},
 	.probe		= rv8803_probe,
 	.remove		= rv8803_remove,
-- 
2.9.3

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

#1592037 — Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

Hi,

On 03/03/2017 at 11:29:11 -0300, Javier Martinez Canillas wrote:
> This series add OF device ID tables to RTC 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.
> 

Doesn't that break the DT ABI for all i2c devices? A lot of people are
getting the vendor wrong in the compatible string and because the i2c
core doesn't care, the driver is still probed. Dropping that will break
all those DTBs.
Or will that only affect module autoload?


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

#1592079 — Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

Hello Alexandre,

On 03/03/2017 12:01 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 03/03/2017 at 11:29:11 -0300, Javier Martinez Canillas wrote:
>> This series add OF device ID tables to RTC 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.
>>
> 
> Doesn't that break the DT ABI for all i2c devices? A lot of people are
> getting the vendor wrong in the compatible string and because the i2c
> core doesn't care, the driver is still probed. Dropping that will break
> all those DTBs.
> Or will that only affect module autoload?
> 

The change will only be for module autoload. The I2C core will still attempt
to match the I2C device ID table as a fallback if fails to match the OF one.
So there's no change in the match logic for drivers that do the wrong thing.

If someone is using a wrong vendor prefix and the DT can't be changed, then
an entry in the OF device ID table has to be added for "wrong_vendor,device"
so the MODALIAS uevent can be "of:N*T*wrong_vendor,device".

This is the RFC patch for the I2C core so you get a better idea:

https://patchwork.kernel.org/patch/6903981/

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

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

#1592109 — Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

On 03/03/2017 at 12:27:23 -0300, Javier Martinez Canillas wrote:
> On 03/03/2017 12:01 PM, Alexandre Belloni wrote:
> > On 03/03/2017 at 11:29:11 -0300, Javier Martinez Canillas wrote:
> >> This series add OF device ID tables to RTC 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.
> >>
> > 
> > Doesn't that break the DT ABI for all i2c devices? A lot of people are
> > getting the vendor wrong in the compatible string and because the i2c
> > core doesn't care, the driver is still probed. Dropping that will break
> > all those DTBs.
> > Or will that only affect module autoload?
> > 
> 
> The change will only be for module autoload. The I2C core will still attempt
> to match the I2C device ID table as a fallback if fails to match the OF one.
> So there's no change in the match logic for drivers that do the wrong thing.
> 

I'm probably not seeing the big picture here but then I don't understand
why you are reworking the id->driver_data handling and using
of_device_get_match_data instead.

If this is related to b8a1a4cd5a98a2adf8dfd6902cd98e57d910ee12, that is
a completely different issue (and that is probably the kind of changes
that should be notified to maintainers).

> If someone is using a wrong vendor prefix and the DT can't be changed, then
> an entry in the OF device ID table has to be added for "wrong_vendor,device"
> so the MODALIAS uevent can be "of:N*T*wrong_vendor,device".
> 

And then, we end up with a crap load of useless strings that are matched
against at boottime...
I'm saying that because all the rv3029 compatible strings are wrong.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

#1592148 — Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

Hello Alexandre,

Sorry for the long email...

On 03/03/2017 01:04 PM, Alexandre Belloni wrote:
> On 03/03/2017 at 12:27:23 -0300, Javier Martinez Canillas wrote:
>> On 03/03/2017 12:01 PM, Alexandre Belloni wrote:
>>> On 03/03/2017 at 11:29:11 -0300, Javier Martinez Canillas wrote:
>>>> This series add OF device ID tables to RTC 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.
>>>>
>>>
>>> Doesn't that break the DT ABI for all i2c devices? A lot of people are
>>> getting the vendor wrong in the compatible string and because the i2c
>>> core doesn't care, the driver is still probed. Dropping that will break
>>> all those DTBs.
>>> Or will that only affect module autoload?
>>>
>>
>> The change will only be for module autoload. The I2C core will still attempt
>> to match the I2C device ID table as a fallback if fails to match the OF one.
>> So there's no change in the match logic for drivers that do the wrong thing.
>>
> 
> I'm probably not seeing the big picture here but then I don't understand

The big picture is to make the I2C subsystem to be consistent with other
subsystems.

Take the platform bus as an example, platform drivers define an OF ID table
if the platform devices are going to be registered via DT and a platform ID
table if the driver supports legacy platform device registration.

But that's not the case for I2C, either only a I2C device ID table is defined
(and the vendor prefix not used which lead to the current situation of wrong
DT's) or a OF device ID table is used to match but still a I2C device table
with duplicated entries is needed only to make the driver module to autoload.

> why you are reworking the id->driver_data handling and using
> of_device_get_match_data instead.
>

I meant that the proposed change to the I2C core is only for module autoload.
The driver changes are for both match and autoload (once I2C core is fixed).

So the change to use the of_device_id .data instead of the i2c_device_id
.driver_data is to use the correct entry in case the I2C core matches using
the OF table instead of the I2C table. Another option is to do the opposite
of what's needed currently, that is to have an I2C table to match and only
use the OF table to populate the OF aliases in the module (i.e: not setting
the .of_match_table field).

But I think that if the device is registered via OF, then the correct thing
to do is to match using the OF table and the OF aliases for module autoload.
 
> If this is related to b8a1a4cd5a98a2adf8dfd6902cd98e57d910ee12, that is
> a completely different issue (and that is probably the kind of changes
> that should be notified to maintainers).
>

It's related in some way. There were two reasons why a I2C driver required a
I2C table:

1) Because their probe function received a struct i2c_device_id as argument.

2) Because the reported MODALIAS is always i2c:<foo> regardless of how the
   device was registered (legacy platform dev or DT).

Lee's commit that you mention solves (1) and I'm trying to solve (2).

>> If someone is using a wrong vendor prefix and the DT can't be changed, then
>> an entry in the OF device ID table has to be added for "wrong_vendor,device"
>> so the MODALIAS uevent can be "of:N*T*wrong_vendor,device".
>>
> 
> And then, we end up with a crap load of useless strings that are matched
> against at boottime...
> I'm saying that because all the rv3029 compatible strings are wrong.
> 

Yeah... that's the joy of the DT backward compatibility. The reason why added
those (wrong) compatibles for rv3029 is because there are mainline users that
are either not using a vendor or using a wrong one (mc instead of microchip).

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

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

#1595659 — Re: [PATCH 00/13] rtc: Add OF device table to I2C drivers that are missing it

On 03/03/2017 at 11:29:11 -0300, Javier Martinez Canillas wrote:
> Hello,
> 
> This series add OF device ID tables to RTC 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.
> 
> So these patches will make sure that RTC 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 (13):
>   rtc: rv8803: Add OF device ID table
>   rtc: rv3029: Add OF device ID table
>   rtc: bq32k: Add OF device ID table
>   rtc: ds1307: Add OF device ID table
>   rtc: rx8010: Add OF device ID table
>   rtc: ds3232: Add OF device ID table
>   rtc: rtc-ds1672: Add OF device ID table
>   rtc: ds1374: Set .of_match_table to OF device ID table
>   rtc: isl1208: Add OF device ID table
>   rtc: s35390a: Add OF device ID table
>   rtc: rx8581: Add OF device ID table
>   rtc: m41t80: Add OF device ID table
>   rtc: rs5c372: Add OF device ID table
> 

All applied, thanks!

>  drivers/rtc/rtc-bq32k.c    |  7 +++++
>  drivers/rtc/rtc-ds1307.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/rtc/rtc-ds1374.c   |  1 +
>  drivers/rtc/rtc-ds1672.c   |  9 +++++-
>  drivers/rtc/rtc-ds3232.c   |  7 +++++
>  drivers/rtc/rtc-isl1208.c  | 12 ++++++--
>  drivers/rtc/rtc-m41t80.c   | 63 ++++++++++++++++++++++++++++++++++++++++--
>  drivers/rtc/rtc-rs5c372.c  | 37 ++++++++++++++++++++++++-
>  drivers/rtc/rtc-rv3029c2.c |  9 ++++++
>  drivers/rtc/rtc-rv8803.c   | 21 +++++++++++++-
>  drivers/rtc/rtc-rx8010.c   |  7 +++++
>  drivers/rtc/rtc-rx8581.c   |  7 +++++
>  drivers/rtc/rtc-s35390a.c  |  8 ++++++
>  13 files changed, 248 insertions(+), 8 deletions(-)
> 
> -- 
> 2.9.3
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web