Re: [PATCH v1 1/3] dt-bindings: hwmon: lm90: Document SMSC EMC1402/1403/1404/1428

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema artificially restricts the number of allowed channels, rejecting valid hardware configurations for the newly added SMSC EMC1404 and EMC1428 devices.
- [Medium] The dt-binding incorrectly applies maximum temperature threshold limits to the 'temperature-offset-millicelsius' property.
--

commit 7ac2109c4a63c7879d955565cef8a4c30bd05436
Author: Svyatoslav Ryhel <clamor95@gmail.com>

dt-bindings: hwmon: lm90: Document SMSC EMC1402/1403/1404/1428

Document SMSC (now Microchip) EMC1402/1403/1404/1428 thermal sensors.

> diff --git a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> index 164068ba069d7..a1de96e0b87f3 100644
> --- a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml
> @@ -12,38 +12,67 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
[ ... ]
> +    oneOf:
> +      - enum:
[ ... ]
> +          - smsc,emc1402
> +          - smsc,emc1403
> +          - smsc,emc1404
> +          - smsc,emc1428

[Severity: Medium]
Does adding smsc,emc1404 and smsc,emc1428 here without expanding the global
channel limits cause schema validation failures for valid hardware?

The emc1403 driver supports up to 4 channels for the EMC1404 and up to 8
channels for the EMC1428. However, the base schema in
Documentation/devicetree/bindings/hwmon/national,lm90.yaml globally restricts
channels:

  patternProperties:
    "^channel@([0-2])$":
      type: object
      ...
      properties:
        reg:
          items:
            minimum: 0
            maximum: 2

With additionalProperties: false at the root, defining channel@3 for EMC1404
or channel@7 for EMC1428 will fail schema validation.

Can we expand the global channel regex and reg bounds, and use if-then blocks
to restrict legacy devices to fewer channels?

[ ... ]
> @@ -169,6 +198,7 @@ allOf:
>                - national,lm99
>                - nuvoton,nct7718
>                - nxp,sa56004
> +              - smsc,emc1428

[Severity: Medium]
Are we confusing the maximum temperature threshold limits with the maximum
offset limit here?

By adding smsc,emc1428 to this block, it inherits a maximum value of 127875
for temperature-offset-millicelsius. In the emc1403.c driver's
emc1403_set_temp() function, temperature thresholds are clamped using
val = clamp_val(val, -128000, 127875) for the emc1428. However, these are
bounds for thresholds (tempX_max, tempX_min, tempX_crit), not temperature
offsets.

The emc1403 driver doesn't seem to support an offset register. Should these
compatibles be added to the block that explicitly sets
temperature-offset-millicelsius: false instead?

>                - winbond,w83l771
>      then:
>        patternProperties:
[ ... ]
> @@ -191,6 +221,21 @@ allOf:
>              temperature-offset-millicelsius:
>                maximum: 127937
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - smsc,emc1402
> +              - smsc,emc1403
> +              - smsc,emc1404
> +    then:
> +      patternProperties:
> +        "^channel@([0-2])$":
> +          properties:
> +            temperature-offset-millicelsius:
> +              maximum: 255875

[Severity: Medium]
Does this block also incorrectly apply temperature threshold maximums to the
offset property?

In the emc1403.c driver's emc1403_set_temp() function, temperature thresholds
for these devices are clamped using val = clamp_val(val, 0, 255875). This
limit applies to threshold registers rather than the offset register.

If the hardware lacks offset support, should
temperature-offset-millicelsius be disabled for these compatibles as well?

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