2021-01-30 10:43:27

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match 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 <[email protected]>
---
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


2021-01-30 10:44:19

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH 2/2] hwmon: lm75: Add another name for NXP LM75B to of_match 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 <[email protected]>
---
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

2021-01-30 16:25:11

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list

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 <[email protected]>
> ---
> 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

2021-02-04 01:35:47

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 1/2] hwmon: lm75: Add NXP LM75A to of_match list

On 2/3/21 9:13 AM, Matwey V. Kornilov wrote:
>
>
> сб, 30 янв. 2021 г. в 18:41, Guenter Roeck <[email protected] <mailto:[email protected]>>:
>>
>> 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 <[email protected] <mailto:[email protected]>>
>> > ---
>> >  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

2021-02-06 09:54:57

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH v4 0/4] hwmon: lm75: Handle broken device nodes gracefully

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

2021-02-06 09:55:35

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a to of_match 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 <[email protected]>
---
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

2021-02-06 09:55:53

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 to of_match list

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 <[email protected]>
---
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

2021-02-06 09:57:13

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match list

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

Signed-off-by: Matwey V. Kornilov <[email protected]>
---
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

2021-02-06 09:57:56

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH v4 4/4] hwmon: lm75: Handle broken device nodes gracefully

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 <[email protected]>
---
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

2021-02-06 16:50:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match 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 <[email protected]>
> ---
> 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

2021-02-06 16:56:36

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a to of_match 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 <[email protected]>
> ---
> 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
>

2021-02-06 16:57:24

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] hwmon: lm75: Add ti,lm75 to of_match 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 <[email protected]>
> ---
> 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
>

2021-02-06 16:58:14

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] hwmon: lm75: Add nxp,lm75a to of_match 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 <[email protected]>
>> ---
>> 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
>>
>

2021-02-10 19:49:22

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] hwmon: lm75: Add lm75 to of_match 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 <[email protected]>
> > ---
> > 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