linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list
@ 2021-01-30 10:38 Matwey V. Kornilov
  2021-01-30 10:38 ` [PATCH 2/2] hwmon: lm75: Add another name for NXP LM75B " Matwey V. Kornilov
  2021-01-30 15:41 ` [PATCH 1/2] hwmon: lm75: Add NXP LM75A " Guenter Roeck
  0 siblings, 2 replies; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-01-30 10:38 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, open list:HARDWARE MONITORING, open list
  Cc: matwey.kornilov, Matwey V. Kornilov,
	open list:HARDWARE MONITORING, open list

NXP LM75A is compatible with original LM75A while it has improved
11-bit precision.

https://www.nxp.com/docs/en/data-sheet/LM75A.pdf

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 drivers/hwmon/lm75.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 3aa7f9454f57..37dc903ebf54 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -699,6 +699,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
 		.compatible = "national,lm75b",
 		.data = (void *)lm75b
 	},
+	{
+		.compatible = "nxp,lm75a",
+		.data = (void *)lm75b
+	},
 	{
 		.compatible = "maxim,max6625",
 		.data = (void *)max6625
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] hwmon: lm75: Add another name for NXP LM75B to of_match list
  2021-01-30 10:38 [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list Matwey V. Kornilov
@ 2021-01-30 10:38 ` Matwey V. Kornilov
  2021-01-30 15:41 ` [PATCH 1/2] hwmon: lm75: Add NXP LM75A " Guenter Roeck
  1 sibling, 0 replies; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-01-30 10:38 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, open list:HARDWARE MONITORING, open list
  Cc: matwey.kornilov, Matwey V. Kornilov,
	open list:HARDWARE MONITORING, open list

NXP LM75B is compatible with original LM75A while it has improved
11-bit precision.

https://www.nxp.com/docs/en/data-sheet/LM75B.pdf

NXP LM75B support was added in

    799fc6021430 ("hwmon: (lm75) Add support for the NXP LM75B")

OF compatible string "national,lm75b" for LM75B was created in

    e97a45f1b460 ("hwmon: (lm75) Add OF device ID table")

Since the previous commit introduces "nxp,lm75a" compatible string
for LM75A, there is a reason to add another alias for LM75B.

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 drivers/hwmon/lm75.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 37dc903ebf54..6cd951e062f0 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -703,6 +703,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
 		.compatible = "nxp,lm75a",
 		.data = (void *)lm75b
 	},
+	{
+		.compatible = "nxp,lm75b",
+		.data = (void *)lm75b
+	},
 	{
 		.compatible = "maxim,max6625",
 		.data = (void *)max6625
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list
  2021-01-30 10:38 [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list Matwey V. Kornilov
  2021-01-30 10:38 ` [PATCH 2/2] hwmon: lm75: Add another name for NXP LM75B " Matwey V. Kornilov
@ 2021-01-30 15:41 ` Guenter Roeck
       [not found]   ` <CAJs94EbBAK1LAZt4pfbzbUCxJtRo8kTAxdaN=y_gRbuvX0rz8Q@mail.gmail.com>
  1 sibling, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2021-01-30 15:41 UTC (permalink / raw)
  To: Matwey V. Kornilov, Jean Delvare, open list:HARDWARE MONITORING,
	open list
  Cc: matwey.kornilov

On 1/30/21 2:38 AM, Matwey V. Kornilov wrote:
> NXP LM75A is compatible with original LM75A while it has improved
> 11-bit precision.
> 
> https://www.nxp.com/docs/en/data-sheet/LM75A.pdf
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  drivers/hwmon/lm75.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 3aa7f9454f57..37dc903ebf54 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -699,6 +699,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
>  		.compatible = "national,lm75b",
>  		.data = (void *)lm75b
>  	},
> +	{
> +		.compatible = "nxp,lm75a",
> +		.data = (void *)lm75b

This should get a different identifier (such as lm75a_nxp or whatever)
because otherwise the results would be different on non-devicetree
systems which would only match "lm75a".

> +	},
>  	{
>  		.compatible = "maxim,max6625",
>  		.data = (void *)max6625
> 

Both "nxp,lm75a" and "nxp,lm75b" need to be added to
Documentation/devicetree/bindings/hwmon/lm75.yaml (in a separate
patch with copy to dt maintainers for review).

Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list
       [not found]   ` <CAJs94EbBAK1LAZt4pfbzbUCxJtRo8kTAxdaN=y_gRbuvX0rz8Q@mail.gmail.com>
@ 2021-02-03 19:57     ` Guenter Roeck
  2021-02-06  9:51       ` [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2021-02-03 19:57 UTC (permalink / raw)
  To: Matwey V. Kornilov; +Cc: Jean Delvare, open list:HARDWARE MONITORING, open list

On 2/3/21 9:13 AM, Matwey V. Kornilov wrote:
> 
> 
> сб, 30 янв. 2021 г. в 18:41, Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>>:
>>
>> On 1/30/21 2:38 AM, Matwey V. Kornilov wrote:
>> > NXP LM75A is compatible with original LM75A while it has improved
>> > 11-bit precision.
>> >
>> > https://www.nxp.com/docs/en/data-sheet/LM75A.pdf
>> >
>> > Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru <mailto:matwey@sai.msu.ru>>
>> > ---
>> >  drivers/hwmon/lm75.c | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> > index 3aa7f9454f57..37dc903ebf54 100644
>> > --- a/drivers/hwmon/lm75.c
>> > +++ b/drivers/hwmon/lm75.c
>> > @@ -699,6 +699,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
>> >               .compatible = "national,lm75b",
>> >               .data = (void *)lm75b
>> >       },
>> > +     {
>> > +             .compatible = "nxp,lm75a",
>> > +             .data = (void *)lm75b
>>
>> This should get a different identifier (such as lm75a_nxp or whatever)
>> because otherwise the results would be different on non-devicetree
>> systems which would only match "lm75a".
>>
> 
> Sorry, I don't understand it. Non-devicetree systems won't use this table at all.
>  
... and instantiate the driver with "lm75a", ie differently than on a devicetree
system. Some purist could then complain that they have to instantiate the driver
as lm75b even though it is a lm75a to get the higher resolution.

Besides, I just noticed that the resolution for the limit registers for nxp's
version of LM75A is different than the resolution of the temperature register.
That means that you'll need a separate entry for this chip anyway,
one that specifies a .default_resolution of 11 and .resolution_limits of 9.

Making things worse, that also applies to NXP's version of LM75B. And
the resolution of TI's version of LM75B/C is 9 bit for both temperature
and limit registers, which does make me wonder where the 11 bit in the
driver comes from ... oh, yes, that was commit 799fc602143024 ("hwmon:
(lm75) Add support for the NXP LM75B"). The devicetree table was added later,
and with that the entry was wrongly attributed to National and not to NXP.

So in reality we'd really need a number of additional entries to
match sensor and limit resolutions correctly.

National/TI: Resolution is always 9 bit for LM95 and LM95A/B/C.
NXP: Resolution is 11 bit for temperature, 9 bit for limits for LM75A/B.
Oh, as it turns out the limit register resolution for NXP's PCT2075 is
also 9 bit. Sigh.

Guenter

>>
>> > +     },
>> >       {
>> >               .compatible = "maxim,max6625",
>> >               .data = (void *)max6625
>> >
>>
>> Both "nxp,lm75a" and "nxp,lm75b" need to be added to
>> Documentation/devicetree/bindings/hwmon/lm75.yaml (in a separate
>> patch with copy to dt maintainers for review).
>>
>> Guenter
> 
> 
> 
> --
> With best regards,
> Matwey V. Kornilov


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully
  2021-02-03 19:57     ` Guenter Roeck
@ 2021-02-06  9:51       ` Matwey V. Kornilov
  2021-02-06  9:51         ` [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list Matwey V. Kornilov
                           ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-02-06  9:51 UTC (permalink / raw)
  Cc: linux-hwmon, linux-kernel, Matwey V. Kornilov

This series is to fix a logical issue in lm75 driver.
The actual fix is in the last patch. 
Other patches are present in order not to break existing users.

Matwey V. Kornilov (4):
  hwmon: lm75: Add lm75 to of_match list
  hwmon: lm75: Add nxp,lm75a to of_match list
  hwmon: lm75: Add ti,lm75 to of_match list
  hwmon: lm75: Handle broken device nodes gracefully

 .../devicetree/bindings/hwmon/lm75.yaml       |  3 ++
 drivers/hwmon/lm75.c                          | 32 +++++++++++++++++--
 2 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list
  2021-02-06  9:51       ` [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
@ 2021-02-06  9:51         ` Matwey V. Kornilov
  2021-02-06 16:46           ` Guenter Roeck
  2021-02-06  9:51         ` [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a " Matwey V. Kornilov
                           ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-02-06  9:51 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
  Cc: linux-hwmon, linux-kernel, Matwey V. Kornilov,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Currently, many boards use just 'lm75' as a compatible string.

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 Documentation/devicetree/bindings/hwmon/lm75.yaml | 1 +
 drivers/hwmon/lm75.c                              | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index 96eed5cc7841..aec8edd1e0c6 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -13,6 +13,7 @@ maintainers:
 properties:
   compatible:
     enum:
+      - lm75
       - adi,adt75
       - dallas,ds1775
       - dallas,ds75
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index e447febd121a..08cde1c446db 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -667,6 +667,10 @@ static const struct i2c_device_id lm75_ids[] = {
 MODULE_DEVICE_TABLE(i2c, lm75_ids);
 
 static const struct of_device_id __maybe_unused lm75_of_match[] = {
+	{
+		.compatible = "lm75",
+		.data = (void *)lm75
+	},
 	{
 		.compatible = "adi,adt75",
 		.data = (void *)adt75
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a to of_match list
  2021-02-06  9:51       ` [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
  2021-02-06  9:51         ` [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list Matwey V. Kornilov
@ 2021-02-06  9:51         ` Matwey V. Kornilov
  2021-02-06 16:48           ` Guenter Roeck
  2021-02-06  9:51         ` [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 " Matwey V. Kornilov
  2021-02-06  9:51         ` [PATCH v4 4/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
  3 siblings, 1 reply; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-02-06  9:51 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
  Cc: linux-hwmon, linux-kernel, Matwey V. Kornilov,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

NXP LM75A is compatible with original LM75A while it has improved
11-bit precision.

https://www.nxp.com/docs/en/data-sheet/LM75A.pdf

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 Documentation/devicetree/bindings/hwmon/lm75.yaml |  1 +
 drivers/hwmon/lm75.c                              | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index aec8edd1e0c6..8c3848f4c277 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -22,6 +22,7 @@ properties:
       - national,lm75
       - national,lm75a
       - national,lm75b
+      - nxp,lm75a
       - maxim,max6625
       - maxim,max6626
       - maxim,max31725
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 08cde1c446db..9c54c7d86771 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -33,6 +33,7 @@ enum lm75_type {		/* keep sorted in alphabetical order */
 	lm75,
 	lm75a,
 	lm75b,
+	nxp_lm75,
 	max6625,
 	max6626,
 	max31725,
@@ -182,6 +183,11 @@ static const struct lm75_params device_params[] = {
 		.default_resolution = 11,
 		.default_sample_time = MSEC_PER_SEC / 10,
 	},
+	[nxp_lm75] = {
+		.default_resolution = 11,
+		.default_sample_time = MSEC_PER_SEC / 10,
+		.resolution_limits = 9,
+	},
 	[max6625] = {
 		.default_resolution = 9,
 		.default_sample_time = MSEC_PER_SEC / 7,
@@ -644,6 +650,7 @@ static const struct i2c_device_id lm75_ids[] = {
 	{ "lm75", lm75, },
 	{ "lm75a", lm75a, },
 	{ "lm75b", lm75b, },
+	{ "nxp_lm75a", nxp_lm75, },
 	{ "max6625", max6625, },
 	{ "max6626", max6626, },
 	{ "max31725", max31725, },
@@ -703,6 +710,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
 		.compatible = "national,lm75b",
 		.data = (void *)lm75b
 	},
+	{
+		.compatible = "nxp,lm75a",
+		.data = (void *)nxp_lm75
+	},
 	{
 		.compatible = "maxim,max6625",
 		.data = (void *)max6625
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 to of_match list
  2021-02-06  9:51       ` [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
  2021-02-06  9:51         ` [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list Matwey V. Kornilov
  2021-02-06  9:51         ` [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a " Matwey V. Kornilov
@ 2021-02-06  9:51         ` Matwey V. Kornilov
  2021-02-06 16:54           ` Guenter Roeck
  2021-02-06  9:51         ` [PATCH v4 4/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
  3 siblings, 1 reply; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-02-06  9:51 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
  Cc: linux-hwmon, linux-kernel, Matwey V. Kornilov,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Currently, armada-388-helios4.dts and nuvoton-npcm730-kudo.dts use
"ti,lm75" compatible string.

TI LM75A/B are compatible with original LM75A

https://www.ti.com/lit/ds/symlink/lm75a.pdf
https://www.ti.com/lit/ds/symlink/lm75b.pdf

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 Documentation/devicetree/bindings/hwmon/lm75.yaml | 1 +
 drivers/hwmon/lm75.c                              | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index 8c3848f4c277..721e77ce4390 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -32,6 +32,7 @@ properties:
       - st,stds75
       - st,stlm75
       - microchip,tcn75
+      - ti,lm75
       - ti,tmp100
       - ti,tmp101
       - ti,tmp105
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 9c54c7d86771..3e4374aa2f99 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -750,6 +750,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
 		.compatible = "microchip,tcn75",
 		.data = (void *)tcn75
 	},
+	{
+		.compatible = "ti,lm75",
+		.data = (void *)lm75a
+	},
 	{
 		.compatible = "ti,tmp100",
 		.data = (void *)tmp100
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/4] hwmon: lm75: Handle broken device nodes gracefully
  2021-02-06  9:51       ` [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
                           ` (2 preceding siblings ...)
  2021-02-06  9:51         ` [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 " Matwey V. Kornilov
@ 2021-02-06  9:51         ` Matwey V. Kornilov
  3 siblings, 0 replies; 14+ messages in thread
From: Matwey V. Kornilov @ 2021-02-06  9:51 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Javier Martinez Canillas,
	open list:HARDWARE MONITORING, open list
  Cc: linux-hwmon, linux-kernel, Matwey V. Kornilov

There is a logical flaw in lm75_probe() function introduced in

    e97a45f1b460 ("hwmon: (lm75) Add OF device ID table")

Note, that of_device_get_match_data() returns NULL when no match
is found. This is the case when OF node exists but has unknown
compatible line, while the module is still loaded via i2c
detection.

arch/powerpc/boot/dts/fsl/p2041rdb.dts:

    lm75b@48 {
    	compatible = "nxp,lm75a";
    	reg = <0x48>;
    };

In this case, the sensor is mistakenly considered as ADT75 variant.

Fixes: e97a45f1b460 ("hwmon: (lm75) Add OF device ID table")
Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 drivers/hwmon/lm75.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 3e4374aa2f99..cd2cda4f557a 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -567,10 +567,17 @@ static int lm75_probe(struct i2c_client *client)
 	int status, err;
 	enum lm75_type kind;
 
-	if (client->dev.of_node)
-		kind = (enum lm75_type)of_device_get_match_data(&client->dev);
-	else
+	if (dev->of_node) {
+		const struct of_device_id *match =
+			of_match_device(dev->driver->of_match_table, dev);
+
+		if (!match)
+			return -ENODEV;
+
+		kind = (enum lm75_type)(match->data);
+	} else {
 		kind = i2c_match_id(lm75_ids, client)->driver_data;
+	}
 
 	if (!i2c_check_functionality(client->adapter,
 			I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list
  2021-02-06  9:51         ` [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list Matwey V. Kornilov
@ 2021-02-06 16:46           ` Guenter Roeck
  2021-02-10 19:47             ` Rob Herring
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2021-02-06 16:46 UTC (permalink / raw)
  To: Matwey V. Kornilov, Jean Delvare, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2/6/21 1:51 AM, Matwey V. Kornilov wrote:
> Currently, many boards use just 'lm75' as a compatible string.
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  Documentation/devicetree/bindings/hwmon/lm75.yaml | 1 +
>  drivers/hwmon/lm75.c                              | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> index 96eed5cc7841..aec8edd1e0c6 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> @@ -13,6 +13,7 @@ maintainers:
>  properties:
>    compatible:
>      enum:
> +      - lm75

Please split the .yaml file changes into a single patch, separate
from the code changes. Also please make sure that the subject indicates
that this is a bindings change.

For this change, we'll definitely need feedback from Rob. I am not sure
if such a generic compatible string is permitted or if we need to change
the dts files instead.

On a higher level, while lm75 is an extreme case, I see a few other
violators as well.

drivers/macintosh/windfarm_ad7417_sensor.c:     { .compatible = "ad7417", },
drivers/macintosh/windfarm_max6690_sensor.c:    { .compatible = "max6690", },
arch/arm/boot/dts/socfpga_arria10_socdk.dtsi:           compatible = "ltc2977";
arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts:                           compatible = "tmp421";
arch/arm/boot/dts/nuvoton-npcm750-evb.dts:              compatible = "tmp100";
arch/arm/boot/dts/nuvoton-npcm750-evb.dts:              compatible = "tmp100";

so it would be good to know how to handle those in general.

Note that there is also:

Documentation/devicetree/bindings/display/repaper.txt:          compatible = "lm75b";

but maybe that doesn't matter as much since it is not actually
used in dts files.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a to of_match list
  2021-02-06  9:51         ` [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a " Matwey V. Kornilov
@ 2021-02-06 16:48           ` Guenter Roeck
  2021-02-06 16:54             ` Guenter Roeck
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2021-02-06 16:48 UTC (permalink / raw)
  To: Matwey V. Kornilov, Jean Delvare, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2/6/21 1:51 AM, Matwey V. Kornilov wrote:
> NXP LM75A is compatible with original LM75A while it has improved
> 11-bit precision.
> 
> https://www.nxp.com/docs/en/data-sheet/LM75A.pdf
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  Documentation/devicetree/bindings/hwmon/lm75.yaml |  1 +
>  drivers/hwmon/lm75.c                              | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> index aec8edd1e0c6..8c3848f4c277 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> @@ -22,6 +22,7 @@ properties:
>        - national,lm75
>        - national,lm75a
>        - national,lm75b
> +      - nxp,lm75a
>        - maxim,max6625
>        - maxim,max6626
>        - maxim,max31725
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 08cde1c446db..9c54c7d86771 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -33,6 +33,7 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>  	lm75,
>  	lm75a,
>  	lm75b,
> +	nxp_lm75,

Please make this lm75a_nxp for improved alphabetical alignment
and to reflect that it is LM75A.

>  	max6625,
>  	max6626,
>  	max31725,
> @@ -182,6 +183,11 @@ static const struct lm75_params device_params[] = {
>  		.default_resolution = 11,
>  		.default_sample_time = MSEC_PER_SEC / 10,
>  	},
> +	[nxp_lm75] = {
> +		.default_resolution = 11,
> +		.default_sample_time = MSEC_PER_SEC / 10,
> +		.resolution_limits = 9,
> +	},
>  	[max6625] = {
>  		.default_resolution = 9,
>  		.default_sample_time = MSEC_PER_SEC / 7,
> @@ -644,6 +650,7 @@ static const struct i2c_device_id lm75_ids[] = {
>  	{ "lm75", lm75, },
>  	{ "lm75a", lm75a, },
>  	{ "lm75b", lm75b, },
> +	{ "nxp_lm75a", nxp_lm75, },
>  	{ "max6625", max6625, },
>  	{ "max6626", max6626, },
>  	{ "max31725", max31725, },
> @@ -703,6 +710,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
>  		.compatible = "national,lm75b",
>  		.data = (void *)lm75b
>  	},
> +	{
> +		.compatible = "nxp,lm75a",
> +		.data = (void *)nxp_lm75
> +	},
>  	{
>  		.compatible = "maxim,max6625",
>  		.data = (void *)max6625
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 to of_match list
  2021-02-06  9:51         ` [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 " Matwey V. Kornilov
@ 2021-02-06 16:54           ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2021-02-06 16:54 UTC (permalink / raw)
  To: Matwey V. Kornilov, Jean Delvare, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2/6/21 1:51 AM, Matwey V. Kornilov wrote:
> Currently, armada-388-helios4.dts and nuvoton-npcm730-kudo.dts use
> "ti,lm75" compatible string.
> 
> TI LM75A/B are compatible with original LM75A
> 
> https://www.ti.com/lit/ds/symlink/lm75a.pdf
> https://www.ti.com/lit/ds/symlink/lm75b.pdf
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  Documentation/devicetree/bindings/hwmon/lm75.yaml | 1 +
>  drivers/hwmon/lm75.c                              | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> index 8c3848f4c277..721e77ce4390 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> @@ -32,6 +32,7 @@ properties:
>        - st,stds75
>        - st,stlm75
>        - microchip,tcn75
> +      - ti,lm75
>        - ti,tmp100
>        - ti,tmp101
>        - ti,tmp105
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 9c54c7d86771..3e4374aa2f99 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -750,6 +750,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
>  		.compatible = "microchip,tcn75",
>  		.data = (void *)tcn75
>  	},
> +	{
> +		.compatible = "ti,lm75",
> +		.data = (void *)lm75a
> +	},

I think that would be better aligned with lm75, not with lm75a.

Thanks,
Guenter

>  	{
>  		.compatible = "ti,tmp100",
>  		.data = (void *)tmp100
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a to of_match list
  2021-02-06 16:48           ` Guenter Roeck
@ 2021-02-06 16:54             ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2021-02-06 16:54 UTC (permalink / raw)
  To: Matwey V. Kornilov, Jean Delvare, Rob Herring,
	open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2/6/21 8:48 AM, Guenter Roeck wrote:
> On 2/6/21 1:51 AM, Matwey V. Kornilov wrote:
>> NXP LM75A is compatible with original LM75A while it has improved
>> 11-bit precision.
>>
>> https://www.nxp.com/docs/en/data-sheet/LM75A.pdf
>>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  Documentation/devicetree/bindings/hwmon/lm75.yaml |  1 +
>>  drivers/hwmon/lm75.c                              | 11 +++++++++++
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
>> index aec8edd1e0c6..8c3848f4c277 100644
>> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
>> @@ -22,6 +22,7 @@ properties:
>>        - national,lm75
>>        - national,lm75a
>>        - national,lm75b
>> +      - nxp,lm75a

We'll also need nxp,lm75b because that is distinctly different to
national,lm75b / ti,lm75b. Also, we'll need to fix the entry for
those to reflect that the sensor resolution is only 9 bit, not
11 bit as currently claimed.

Thanks,
Guenter

>>        - maxim,max6625
>>        - maxim,max6626
>>        - maxim,max31725
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index 08cde1c446db..9c54c7d86771 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -33,6 +33,7 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>>  	lm75,
>>  	lm75a,
>>  	lm75b,
>> +	nxp_lm75,
> 
> Please make this lm75a_nxp for improved alphabetical alignment
> and to reflect that it is LM75A.
> 
>>  	max6625,
>>  	max6626,
>>  	max31725,
>> @@ -182,6 +183,11 @@ static const struct lm75_params device_params[] = {
>>  		.default_resolution = 11,
>>  		.default_sample_time = MSEC_PER_SEC / 10,
>>  	},
>> +	[nxp_lm75] = {
>> +		.default_resolution = 11,
>> +		.default_sample_time = MSEC_PER_SEC / 10,
>> +		.resolution_limits = 9,
>> +	},
>>  	[max6625] = {
>>  		.default_resolution = 9,
>>  		.default_sample_time = MSEC_PER_SEC / 7,
>> @@ -644,6 +650,7 @@ static const struct i2c_device_id lm75_ids[] = {
>>  	{ "lm75", lm75, },
>>  	{ "lm75a", lm75a, },
>>  	{ "lm75b", lm75b, },
>> +	{ "nxp_lm75a", nxp_lm75, },
>>  	{ "max6625", max6625, },
>>  	{ "max6626", max6626, },
>>  	{ "max31725", max31725, },
>> @@ -703,6 +710,10 @@ static const struct of_device_id __maybe_unused lm75_of_match[] = {
>>  		.compatible = "national,lm75b",
>>  		.data = (void *)lm75b
>>  	},
>> +	{
>> +		.compatible = "nxp,lm75a",
>> +		.data = (void *)nxp_lm75
>> +	},
>>  	{
>>  		.compatible = "maxim,max6625",
>>  		.data = (void *)max6625
>>
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list
  2021-02-06 16:46           ` Guenter Roeck
@ 2021-02-10 19:47             ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-02-10 19:47 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Matwey V. Kornilov, Jean Delvare, open list:HARDWARE MONITORING,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On Sat, Feb 06, 2021 at 08:46:16AM -0800, Guenter Roeck wrote:
> On 2/6/21 1:51 AM, Matwey V. Kornilov wrote:
> > Currently, many boards use just 'lm75' as a compatible string.
> > 
> > Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> > ---
> >  Documentation/devicetree/bindings/hwmon/lm75.yaml | 1 +
> >  drivers/hwmon/lm75.c                              | 4 ++++
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> > index 96eed5cc7841..aec8edd1e0c6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> > @@ -13,6 +13,7 @@ maintainers:
> >  properties:
> >    compatible:
> >      enum:
> > +      - lm75
> 
> Please split the .yaml file changes into a single patch, separate
> from the code changes. Also please make sure that the subject indicates
> that this is a bindings change.
> 
> For this change, we'll definitely need feedback from Rob. I am not sure
> if such a generic compatible string is permitted or if we need to change
> the dts files instead.
> 
> On a higher level, while lm75 is an extreme case, I see a few other
> violators as well.
> 
> drivers/macintosh/windfarm_ad7417_sensor.c:     { .compatible = "ad7417", },
> drivers/macintosh/windfarm_max6690_sensor.c:    { .compatible = "max6690", },

Old as dirt PowerMac stuff...

> arch/arm/boot/dts/socfpga_arria10_socdk.dtsi:           compatible = "ltc2977";
> arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts:                           compatible = "tmp421";

Pretty much a dead platform...

> arch/arm/boot/dts/nuvoton-npcm750-evb.dts:              compatible = "tmp100";
> arch/arm/boot/dts/nuvoton-npcm750-evb.dts:              compatible = "tmp100";
> 
> so it would be good to know how to handle those in general.

The dts files can be fixed without a compatibility issue (at least for 
Linux), so we should update them and leave the documentation as-is. We 
just can't add a new vendor compatible to the driver and then change the 
dts files like these as old kernels wouldn't recognized the new 
compatibles (though we should backport compatibles like PCI IDs).

> 
> Note that there is also:
> 
> Documentation/devicetree/bindings/display/repaper.txt:          compatible = "lm75b";
> 
> but maybe that doesn't matter as much since it is not actually
> used in dts files.

Right.

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-02-10 19:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-30 10:38 [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list Matwey V. Kornilov
2021-01-30 10:38 ` [PATCH 2/2] hwmon: lm75: Add another name for NXP LM75B " Matwey V. Kornilov
2021-01-30 15:41 ` [PATCH 1/2] hwmon: lm75: Add NXP LM75A " Guenter Roeck
     [not found]   ` <CAJs94EbBAK1LAZt4pfbzbUCxJtRo8kTAxdaN=y_gRbuvX0rz8Q@mail.gmail.com>
2021-02-03 19:57     ` Guenter Roeck
2021-02-06  9:51       ` [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov
2021-02-06  9:51         ` [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list Matwey V. Kornilov
2021-02-06 16:46           ` Guenter Roeck
2021-02-10 19:47             ` Rob Herring
2021-02-06  9:51         ` [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a " Matwey V. Kornilov
2021-02-06 16:48           ` Guenter Roeck
2021-02-06 16:54             ` Guenter Roeck
2021-02-06  9:51         ` [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 " Matwey V. Kornilov
2021-02-06 16:54           ` Guenter Roeck
2021-02-06  9:51         ` [PATCH v4 4/4] hwmon: lm75: Handle broken device nodes gracefully Matwey V. Kornilov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).