Re: [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The implementation of the alert polarity property in the driver is structurally broken and non-functional.
- [Medium] Vendor prefix violation and missing schema constraints for a generic hardware feature.
--

commit 92fb69393cd7ead8fd2be55819f1d1f0b77724f6
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property

This commit adds an optional boolean property ti,alert-polarity-active-high
to configure the alert pin polarity for the LM75 hardware monitor.

> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml

[ ... ]

> @@ -54,6 +54,13 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  ti,alert-polarity-active-high:
> +    description: Alert pin is asserted based on the value of alert polarity
> +      bit of configuration register. Default value is normal (0 which maps to
> +      active-low). The other value is inverted (1 which maps to active-high).
> +      Specify this property to set the alert polarity to active-high.
> +    $ref: /schemas/types.yaml#/definitions/flag

Is the OS polarity (bit 2) a generic feature of the LM75 specification? If so,
should this property be named without the vendor prefix (e.g.,
alert-polarity-active-high)?

If this is instead intended as a TI-specific extension, should it be
constrained via an allOf -> if block restricted to ti,* compatible strings?
As currently defined in the root properties block, it globally allows non-TI
compatible devices (such as nxp,pct2075 or maxim,max6625) to pass schema
validation with a TI-specific property.

Also, regarding the implementation in the subsequent driver patch ("hwmon:
(lm75) Support active-high alert polarity"), does the register update actually
take effect?

The driver code adds LM75_ALERT_POLARITY_HIGH_8_BIT (BIT(2)) to set_mask, but
does it also update clr_mask? The helper function lm75_write_config() passes
clr_mask | LM75_SHUTDOWN as the mask argument to regmap_update_bits(). 

Because BIT(2) does not appear to be included in the static clr_mask
definitions for devices in device_params, the bit would be excluded from the
regmap_update_bits() mask. This would cause the register update to ignore the
new polarity setting and preserve the existing hardware state.

> +
>  required:
>    - compatible
>    - reg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260502190408.3577731-1-markus.stockhausen@gmx.de?part=1