Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH 0/2] hwmon: lm90: Fix DT channel constraints and reject unsupported channel 2
@ 2026-08-24 18:38 Flaviu Nistor
  2026-08-24 18:38 ` [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset Flaviu Nistor
  2026-08-24 18:39 ` [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor Flaviu Nistor
  0 siblings, 2 replies; 6+ messages in thread
From: Flaviu Nistor @ 2026-08-24 18:38 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree

The intention of this patch series is to fix DT channel constrains in the
dt-bindings and inprove the driver to reject unsupported channel 2.

dt-bindings:
Forbid temperature-offset-millicelsius on channel@0 (local sensor).
Rejecting of assigning temperature-offset-millicelsius for channel@0 is
already implemented in the driver.
Reject channel@2 for compatibles that have only one remote sensor.

driver:
If the sensor have only one remote channel, reject a possible unsupported
channel 2 and return an error, similar to miss-assigment of temperature
-offset-millicelsius property to channel@0.

Flaviu Nistor (2):
  dt-bindings: hwmon: national,lm90: Fix channel constraints for
    temperature offset
  hwmon: (lm90) Reject channel 2 on chips with only one remote sensor

 .../bindings/hwmon/national,lm90.yaml         | 29 +++++++++++++++++--
 drivers/hwmon/lm90.c                          |  5 ++++
 2 files changed, 31 insertions(+), 3 deletions(-)

-- 
2.34.1


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

* [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset
  2026-08-24 18:38 [PATCH 0/2] hwmon: lm90: Fix DT channel constraints and reject unsupported channel 2 Flaviu Nistor
@ 2026-08-24 18:38 ` Flaviu Nistor
  2026-08-24 18:49   ` sashiko-bot
  2026-08-25 16:58   ` Conor Dooley
  2026-08-24 18:39 ` [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor Flaviu Nistor
  1 sibling, 2 replies; 6+ messages in thread
From: Flaviu Nistor @ 2026-08-24 18:38 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree

Limit temperature-offset-millicelsius to remote channels only, since
channel 0 is local and this property does not apply to it.
Channel 2 is only valid on devices with two remote sensors, so reject
channel 2 for compatibles that do not support a second remote channel.

Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
 .../bindings/hwmon/national,lm90.yaml         | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
index 164068ba069d..a7f82b202b86 100644
--- a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
+++ b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
@@ -113,6 +113,29 @@ allOf:
       properties:
         ti,extended-range-enable: false
 
+  - patternProperties:
+      "channel@0":
+        type: object
+        properties:
+          temperature-offset-millicelsius: false
+
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - adi,adt7481
+                - dallas,max6695
+                - dallas,max6696
+    then:
+      patternProperties:
+        "^channel@[0-1]$":
+          properties:
+            reg:
+              enum: [0, 1]
+        "channel@2": false
+
   - if:
       properties:
         compatible:
@@ -149,7 +172,7 @@ allOf:
               - onnn,nct1008
     then:
       patternProperties:
-        "^channel@([0-2])$":
+        "^channel@([1-2])$":
           properties:
             temperature-offset-millicelsius:
               maximum: 127750
@@ -172,7 +195,7 @@ allOf:
               - winbond,w83l771
     then:
       patternProperties:
-        "^channel@([0-2])$":
+        "^channel@([1-2])$":
           properties:
             temperature-offset-millicelsius:
               maximum: 127875
@@ -186,7 +209,7 @@ allOf:
               - ti,tmp461
     then:
       patternProperties:
-        "^channel@([0-2])$":
+        "^channel@([1-2])$":
           properties:
             temperature-offset-millicelsius:
               maximum: 127937
-- 
2.34.1


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

* [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor
  2026-08-24 18:38 [PATCH 0/2] hwmon: lm90: Fix DT channel constraints and reject unsupported channel 2 Flaviu Nistor
  2026-08-24 18:38 ` [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset Flaviu Nistor
@ 2026-08-24 18:39 ` Flaviu Nistor
  2026-08-24 18:50   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Flaviu Nistor @ 2026-08-24 18:39 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree

Validate firmware channel definitions against chip capabilities and return
-EINVAL when channel 2 is configured on devices with 1 remote channel.

Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
 drivers/hwmon/lm90.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 1c603272538a..3faeb2c6ab01 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -2712,6 +2712,11 @@ static int lm90_probe_channel(struct i2c_client *client,
 		return -EINVAL;
 	}
 
+	if (id == 2 && !(data->flags & LM90_HAVE_TEMP3)) {
+		dev_err(dev, "channel %d is not supported for this chip in %pfw\n", id, child);
+		return -EINVAL;
+	}
+
 	err = fwnode_property_read_string(child, "label", &data->channel_label[id]);
 	if (err == -ENODATA || err == -EILSEQ) {
 		dev_err(dev, "invalid label property in %pfw\n", child);
-- 
2.34.1


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

* Re: [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset
  2026-08-24 18:38 ` [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset Flaviu Nistor
@ 2026-08-24 18:49   ` sashiko-bot
  2026-08-25 16:58   ` Conor Dooley
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-24 18:49 UTC (permalink / raw)
  To: Flaviu Nistor; +Cc: robh, conor+dt, linux-hwmon, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unanchored regular expressions in `patternProperties` violate dt-bindings conventions by matching substrings instead of exact node names.
--

--- Patch [1]: [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset ---
commit 9782538dd0bd62fc7f4323e17bb521a9d2fa819b
Author: Flaviu Nistor <flaviu.nistor@gmail.com>

dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset

This commit limits the temperature-offset-millicelsius property to remote
channels only, as it doesn't apply to the local channel 0. It also rejects
channel 2 for devices that do not support a second remote channel.

> diff --git a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> index 164068ba069d7..a7f82b202b861 100644
> --- a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
[ ... ]
> @@ -113,6 +113,29 @@ allOf:
>        properties:
>          ti,extended-range-enable: false
>  
> +  - patternProperties:
> +      "channel@0":

[Severity: Low]
Should this regular expression be anchored as "^channel@0$"?

Without anchors, this matches any child node containing "channel@0" as a
substring (such as "mychannel@0"), causing incorrect schema validation rules
to be applied during dt_binding_check execution.

> +        type: object
> +        properties:
> +          temperature-offset-millicelsius: false
> +
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - adi,adt7481
> +                - dallas,max6695
> +                - dallas,max6696
> +    then:
> +      patternProperties:
> +        "^channel@[0-1]$":
> +          properties:
> +            reg:
> +              enum: [0, 1]
> +        "channel@2": false

[Severity: Low]
Does this pattern also need to be anchored as "^channel@2$"?

As written, it behaves as a substring match and might inadvertently reject
node names containing it as a substring.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824183900.8983-1-flaviu.nistor@gmail.com?part=1

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

* Re: [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor
  2026-08-24 18:39 ` [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor Flaviu Nistor
@ 2026-08-24 18:50   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-24 18:50 UTC (permalink / raw)
  To: Flaviu Nistor; +Cc: robh, conor+dt, linux-hwmon, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Setting the temperature offset for channel 2 during probe clobbers the device's hardware configuration register because `data->config` is used before it is initialized.
--

--- Patch [2]: [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor ---
commit dbe7fc3f39616fac014beb08a22b533d36d4c54b
Author: Flaviu Nistor <flaviu.nistor@gmail.com>
Subject: hwmon: (lm90) Reject channel 2 on chips with only one remote sensor

This patch validates firmware channel definitions against chip capabilities
and returns -EINVAL when channel 2 is configured on devices with 1 remote
channel.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 1c603272538aa..3faeb2c6ab01b 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -2712,6 +2712,11 @@ static int lm90_probe_channel(struct i2c_client *client,
>  		return -EINVAL;
>  	}
>  
> +	if (id == 2 && !(data->flags & LM90_HAVE_TEMP3)) {
> +		dev_err(dev, "channel %d is not supported for this chip in %pfw\n", id, child);
> +		return -EINVAL;
> +	}
> +
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem when channel 2
is valid and has a temperature offset configured.

In lm90_probe(), lm90_parse_channel_info() is called before
lm90_init_client(). If the device tree specifies the property
temperature-offset-millicelsius for channel 2, it invokes
lm90_set_temp_offset() further down in lm90_probe_channel().

For channel 2, lm90_set_temp_offset() calls lm90_select_remote_channel():

drivers/hwmon/lm90.c:lm90_set_temp_offset() {
        ...
        /* For ADT7481 we can use the same registers for remote channel 1 and 2 */
        if (channel > 1)
                lm90_select_remote_channel(data, true);
        ...
}

Because lm90_init_client() hasn't executed yet, data->config is still 0 from
its initial allocation. This causes the driver to write 0 directly to the
hardware configuration register, which might clobber the original
BIOS/bootloader configuration before it is ever read and saved.

Could this silently overwrite the chip's thermal alert masks or run states
during device probe on systems providing the offset property?

>  	err = fwnode_property_read_string(child, "label", &data->channel_label[id]);
>  	if (err == -ENODATA || err == -EILSEQ) {
>  		dev_err(dev, "invalid label property in %pfw\n", child);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824183900.8983-1-flaviu.nistor@gmail.com?part=2

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

* Re: [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset
  2026-08-24 18:38 ` [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset Flaviu Nistor
  2026-08-24 18:49   ` sashiko-bot
@ 2026-08-25 16:58   ` Conor Dooley
  1 sibling, 0 replies; 6+ messages in thread
From: Conor Dooley @ 2026-08-25 16:58 UTC (permalink / raw)
  To: Flaviu Nistor
  Cc: Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-hwmon, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 2614 bytes --]

On Mon, Aug 24, 2026 at 09:38:59PM +0300, Flaviu Nistor wrote:
> Limit temperature-offset-millicelsius to remote channels only, since
> channel 0 is local and this property does not apply to it.
> Channel 2 is only valid on devices with two remote sensors, so reject
> channel 2 for compatibles that do not support a second remote channel.
> 
> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
> ---
>  .../bindings/hwmon/national,lm90.yaml         | 29 +++++++++++++++++--
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> index 164068ba069d..a7f82b202b86 100644
> --- a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> @@ -113,6 +113,29 @@ allOf:
>        properties:
>          ti,extended-range-enable: false
>  
> +  - patternProperties:
> +      "channel@0":
> +        type: object
> +        properties:
> +          temperature-offset-millicelsius: false

Instead of this, could the two if/then blocks below grow an else with
this instead? What you've got here is a bit abnormal.

Cheers,
Conor.

> +
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - adi,adt7481
> +                - dallas,max6695
> +                - dallas,max6696
> +    then:
> +      patternProperties:
> +        "^channel@[0-1]$":
> +          properties:
> +            reg:
> +              enum: [0, 1]
> +        "channel@2": false
> +
>    - if:
>        properties:
>          compatible:
> @@ -149,7 +172,7 @@ allOf:
>                - onnn,nct1008
>      then:
>        patternProperties:
> -        "^channel@([0-2])$":
> +        "^channel@([1-2])$":
>            properties:
>              temperature-offset-millicelsius:
>                maximum: 127750
> @@ -172,7 +195,7 @@ allOf:
>                - winbond,w83l771
>      then:
>        patternProperties:
> -        "^channel@([0-2])$":
> +        "^channel@([1-2])$":
>            properties:
>              temperature-offset-millicelsius:
>                maximum: 127875
> @@ -186,7 +209,7 @@ allOf:
>                - ti,tmp461
>      then:
>        patternProperties:
> -        "^channel@([0-2])$":
> +        "^channel@([1-2])$":
>            properties:
>              temperature-offset-millicelsius:
>                maximum: 127937
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

end of thread, other threads:[~2026-08-25 16:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 18:38 [PATCH 0/2] hwmon: lm90: Fix DT channel constraints and reject unsupported channel 2 Flaviu Nistor
2026-08-24 18:38 ` [PATCH 1/2] dt-bindings: hwmon: national,lm90: Fix channel constraints for temperature offset Flaviu Nistor
2026-08-24 18:49   ` sashiko-bot
2026-08-25 16:58   ` Conor Dooley
2026-08-24 18:39 ` [PATCH 2/2] hwmon: (lm90) Reject channel 2 on chips with only one remote sensor Flaviu Nistor
2026-08-24 18:50   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox