diff mbox series

[v4,1/4] hwmon: lm75: Add lm75 to of_match list

Message ID 20210206095121.20625-2-matwey@sai.msu.ru (mailing list archive)
State New
Headers show
Series hwmon: lm75: Handle broken device nodes gracefully | expand

Commit Message

Matwey V. Kornilov Feb. 6, 2021, 9:51 a.m. UTC
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(+)

Comments

Guenter Roeck Feb. 6, 2021, 4:46 p.m. UTC | #1
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
Rob Herring Feb. 10, 2021, 7:47 p.m. UTC | #2
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
diff mbox series

Patch

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