Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Support active-high alert polarity for LM75
@ 2026-05-02 19:04 Markus Stockhausen
  2026-05-02 19:04 ` [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property Markus Stockhausen
  2026-05-02 19:04 ` [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity Markus Stockhausen
  0 siblings, 2 replies; 7+ messages in thread
From: Markus Stockhausen @ 2026-05-02 19:04 UTC (permalink / raw)
  To: linux, robh, krzk+dt, conor+dt, jdelvare, linux-hwmon, devicetree
  Cc: Markus Stockhausen

The LM75 configuration register allows to switch the alert polarity.
In default mode the alert output pin is active-low. There are hardware
designs that use this alert output for an hardware assisted automatic
fan speed control. E.g. the D-Link DGS-1250 implements

- temperature below Tmax threshold -> alert pin low -> fan slow speed
- temperature above Tmax threshold -> alert pin high -> fan high speed

Provide a devicetree configuration option and a driver enhancement to
support these hardware designs.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

---

Changes in v2
- Carve out AS6200 polarity fix into separate series
- Rename devicetree prefix from lm75 to ti


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

* [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property
  2026-05-02 19:04 [PATCH v2 0/2] Support active-high alert polarity for LM75 Markus Stockhausen
@ 2026-05-02 19:04 ` Markus Stockhausen
  2026-05-02 19:19   ` sashiko-bot
  2026-05-03 18:13   ` Conor Dooley
  2026-05-02 19:04 ` [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity Markus Stockhausen
  1 sibling, 2 replies; 7+ messages in thread
From: Markus Stockhausen @ 2026-05-02 19:04 UTC (permalink / raw)
  To: linux, robh, krzk+dt, conor+dt, jdelvare, linux-hwmon, devicetree
  Cc: Markus Stockhausen

The LM75 alert pin is asserted based on the value of alert polarity bit of
the configuration register. The device/driver default is 0 which means alert
pin is configured to be active-low. A value of 1 maps to inverted (active-high).

Add an optional boolean property "ti,alert-polarity-active-high" to
override the alert pin polarity. When absent, the default active-low
polarity is kept.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 Documentation/devicetree/bindings/hwmon/lm75.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index 0b9fda81e3ec..b48bf3fd721f 100644
--- 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
+
 required:
   - compatible
   - reg
-- 
2.54.0


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

* [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
  2026-05-02 19:04 [PATCH v2 0/2] Support active-high alert polarity for LM75 Markus Stockhausen
  2026-05-02 19:04 ` [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property Markus Stockhausen
@ 2026-05-02 19:04 ` Markus Stockhausen
  2026-05-02 19:36   ` sashiko-bot
  1 sibling, 1 reply; 7+ messages in thread
From: Markus Stockhausen @ 2026-05-02 19:04 UTC (permalink / raw)
  To: linux, robh, krzk+dt, conor+dt, jdelvare, linux-hwmon, devicetree
  Cc: Markus Stockhausen

LM75 devices supported by this driver support configurable active-high
alert polarity. This is already documented in the devicetree description.
Add support for it to the driver.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/hwmon/lm75.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 8b74cc314196..9542f04e215e 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -123,7 +123,9 @@ struct lm75_data {
 
 static const u8 lm75_sample_set_masks[] = { 0 << 5, 1 << 5, 2 << 5, 3 << 5 };
 
-#define LM75_SAMPLE_CLEAR_MASK	(3 << 5)
+#define LM75_ALERT_POLARITY_HIGH_8_BIT	(BIT(2))
+#define LM75_ALERT_POLARITY_HIGH_16_BIT	(BIT(2) << 8)
+#define LM75_SAMPLE_CLEAR_MASK		(3 << 5)
 
 /* The structure below stores the configuration values of the supported devices.
  * In case of being supported multiple configurations, the default one must
@@ -728,6 +730,7 @@ static int lm75_generic_probe(struct device *dev, const char *name,
 	struct device *hwmon_dev;
 	struct lm75_data *data;
 	int status, err;
+	u16 set_mask;
 
 	data = devm_kzalloc(dev, sizeof(struct lm75_data), GFP_KERNEL);
 	if (!data)
@@ -762,8 +765,15 @@ static int lm75_generic_probe(struct device *dev, const char *name,
 		return err;
 	data->orig_conf = status;
 
-	err = lm75_write_config(data, data->params->set_mask,
-				data->params->clr_mask);
+	set_mask = data->params->set_mask;
+	if (of_property_read_bool(dev->of_node, "ti,alert-polarity-active-high")) {
+		if (!data->params->config_reg_16bits)
+			set_mask |= LM75_ALERT_POLARITY_HIGH_8_BIT;
+		else
+			set_mask |= LM75_ALERT_POLARITY_HIGH_16_BIT;
+	}
+
+	err = lm75_write_config(data, set_mask, data->params->clr_mask);
 	if (err)
 		return err;
 
-- 
2.54.0


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

* Re: [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property
  2026-05-02 19:04 ` [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property Markus Stockhausen
@ 2026-05-02 19:19   ` sashiko-bot
  2026-05-03 18:13   ` Conor Dooley
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-05-02 19:19 UTC (permalink / raw)
  To: Markus Stockhausen; +Cc: linux-hwmon

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

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

* Re: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
  2026-05-02 19:04 ` [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity Markus Stockhausen
@ 2026-05-02 19:36   ` sashiko-bot
  2026-05-03 19:06     ` Guenter Roeck
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-05-02 19:36 UTC (permalink / raw)
  To: Markus Stockhausen; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The alert polarity configuration is silently ignored by the hardware because the new polarity bit is omitted from the register update mask.
- [High] For devices with 16-bit configuration registers, the polarity bit is incorrectly shifted into the wrong byte, modifying a reserved hardware bit instead of the polarity bit.
- [Low] The patch directly uses the Device Tree specific `of_property_read_bool()` API instead of the firmware-agnostic `device_property_read_bool()`.
--

commit 80ab4767c4e330cd441a8b4dcd804fcd1dec493b
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

hwmon: (lm75) Support active-high alert polarity

LM75 devices supported by this driver support configurable active-high
alert polarity. This is already documented in the devicetree description.
Add support for it to the driver.

> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index f1a1e5b888f6..d5a14a3b2820 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -123,7 +123,9 @@ struct lm75_data {
>  
>  static const u8 lm75_sample_set_masks[] = { 0 << 5, 1 << 5, 2 << 5, 3 << 5 };
>  
> -#define LM75_SAMPLE_CLEAR_MASK	(3 << 5)
> +#define LM75_ALERT_POLARITY_HIGH_8_BIT	(BIT(2))
> +#define LM75_ALERT_POLARITY_HIGH_16_BIT	(BIT(2) << 8)

Does this shift target the correct byte for 16-bit registers?

Because the hardware typically expects the first transmitted byte to be the
most significant byte of its configuration register (which contains the POL bit
at bit 2), and i2c_smbus_write_word_data() transmits the least significant
byte first, the POL bit might need to remain in the LSB of the logical value
(BIT(2)).

By shifting it by 8 bits, does this target the MSB of the value, which maps
to the LSB of the hardware register, potentially modifying a reserved bit
instead of the polarity?

> +#define LM75_SAMPLE_CLEAR_MASK		(3 << 5)
>  

[ ... ]

> @@ -762,8 +765,15 @@ static int lm75_generic_probe(struct device *dev, const char *name,
>  		return err;
>  	data->orig_conf = status;
>  
> -	err = lm75_write_config(data, data->params->set_mask,
> -				data->params->clr_mask);
> +	set_mask = data->params->set_mask;
> +	if (of_property_read_bool(dev->of_node, "ti,alert-polarity-active-high")) {

Could we use device_property_read_bool() here instead?

The probe function already uses device_property_read_string() to read the label
property earlier. Hardcoding the Device Tree specific API here breaks
consistency and prevents the active-high polarity feature from working on
systems using ACPI or software nodes.

> +		if (!data->params->config_reg_16bits)
> +			set_mask |= LM75_ALERT_POLARITY_HIGH_8_BIT;
> +		else
> +			set_mask |= LM75_ALERT_POLARITY_HIGH_16_BIT;
> +	}
> +
> +	err = lm75_write_config(data, set_mask, data->params->clr_mask);

Are we missing an update to the clear mask?

When we pass data->params->clr_mask into lm75_write_config():

drivers/hwmon/lm75.c:lm75_write_config() {
	return regmap_update_bits(data->regmap, LM75_REG_CONF,
				  clr_mask | LM75_SHUTDOWN, set_mask);
}

The regmap_update_bits() function uses clr_mask | LM75_SHUTDOWN as the mask
of bits to modify. Since the newly added polarity bit isn't included in the
mask, is the alert polarity configuration silently ignored by the hardware?

If the device tree configures the interrupt controller for an active-high
signal but the sensor remains in its default active-low state, could this
lead to an unhandled interrupt storm?

>  	if (err)
>  		return err;
>

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

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

* Re: [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property
  2026-05-02 19:04 ` [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property Markus Stockhausen
  2026-05-02 19:19   ` sashiko-bot
@ 2026-05-03 18:13   ` Conor Dooley
  1 sibling, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2026-05-03 18:13 UTC (permalink / raw)
  To: Markus Stockhausen
  Cc: linux, robh, krzk+dt, conor+dt, jdelvare, linux-hwmon, devicetree

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

On Sat, May 02, 2026 at 09:04:07PM +0200, Markus Stockhausen wrote:
> The LM75 alert pin is asserted based on the value of alert polarity bit of
> the configuration register. The device/driver default is 0 which means alert
> pin is configured to be active-low. A value of 1 maps to inverted (active-high).
> 
> Add an optional boolean property "ti,alert-polarity-active-high" to
> override the alert pin polarity. When absent, the default active-low
> polarity is kept.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

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

* Re: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
  2026-05-02 19:36   ` sashiko-bot
@ 2026-05-03 19:06     ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-05-03 19:06 UTC (permalink / raw)
  To: sashiko, Markus Stockhausen; +Cc: linux-hwmon

On 5/2/26 12:36, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] The alert polarity configuration is silently ignored by the hardware because the new polarity bit is omitted from the register update mask.
> - [High] For devices with 16-bit configuration registers, the polarity bit is incorrectly shifted into the wrong byte, modifying a reserved hardware bit instead of the polarity bit.
> - [Low] The patch directly uses the Device Tree specific `of_property_read_bool()` API instead of the firmware-agnostic `device_property_read_bool()`.
> --
> 
> commit 80ab4767c4e330cd441a8b4dcd804fcd1dec493b
> Author: Markus Stockhausen <markus.stockhausen@gmx.de>
> 
> hwmon: (lm75) Support active-high alert polarity
> 
> LM75 devices supported by this driver support configurable active-high
> alert polarity. This is already documented in the devicetree description.
> Add support for it to the driver.
> 
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index f1a1e5b888f6..d5a14a3b2820 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -123,7 +123,9 @@ struct lm75_data {
>>   
>>   static const u8 lm75_sample_set_masks[] = { 0 << 5, 1 << 5, 2 << 5, 3 << 5 };
>>   
>> -#define LM75_SAMPLE_CLEAR_MASK	(3 << 5)
>> +#define LM75_ALERT_POLARITY_HIGH_8_BIT	(BIT(2))
>> +#define LM75_ALERT_POLARITY_HIGH_16_BIT	(BIT(2) << 8)
> 
> Does this shift target the correct byte for 16-bit registers?
> 
> Because the hardware typically expects the first transmitted byte to be the
> most significant byte of its configuration register (which contains the POL bit
> at bit 2), and i2c_smbus_write_word_data() transmits the least significant
> byte first, the POL bit might need to remain in the LSB of the logical value
> (BIT(2)).
> 
> By shifting it by 8 bits, does this target the MSB of the value, which maps
> to the LSB of the hardware register, potentially modifying a reserved bit
> instead of the polarity?
> 
Ignore.

>> +#define LM75_SAMPLE_CLEAR_MASK		(3 << 5)
>>   
> 
> [ ... ]
> 
>> @@ -762,8 +765,15 @@ static int lm75_generic_probe(struct device *dev, const char *name,
>>   		return err;
>>   	data->orig_conf = status;
>>   
>> -	err = lm75_write_config(data, data->params->set_mask,
>> -				data->params->clr_mask);
>> +	set_mask = data->params->set_mask;
>> +	if (of_property_read_bool(dev->of_node, "ti,alert-polarity-active-high")) {
> 
> Could we use device_property_read_bool() here instead?
> 
> The probe function already uses device_property_read_string() to read the label
> property earlier. Hardcoding the Device Tree specific API here breaks
> consistency and prevents the active-high polarity feature from working on
> systems using ACPI or software nodes.
> 
Please use device_property_read_bool().

>> +		if (!data->params->config_reg_16bits)
>> +			set_mask |= LM75_ALERT_POLARITY_HIGH_8_BIT;
>> +		else
>> +			set_mask |= LM75_ALERT_POLARITY_HIGH_16_BIT;
>> +	}
>> +
>> +	err = lm75_write_config(data, set_mask, data->params->clr_mask);
> 
> Are we missing an update to the clear mask?
> 
> When we pass data->params->clr_mask into lm75_write_config():
> 
> drivers/hwmon/lm75.c:lm75_write_config() {
> 	return regmap_update_bits(data->regmap, LM75_REG_CONF,
> 				  clr_mask | LM75_SHUTDOWN, set_mask);
> }
> 
> The regmap_update_bits() function uses clr_mask | LM75_SHUTDOWN as the mask
> of bits to modify. Since the newly added polarity bit isn't included in the
> mask, is the alert polarity configuration silently ignored by the hardware?
> 
> If the device tree configures the interrupt controller for an active-high
> signal but the sensor remains in its default active-low state, could this
> lead to an unhandled interrupt storm?
> 
You'll have to pass the bit in clr_mask if the property is not set
for consistency.

Thanks,
Guenter


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

end of thread, other threads:[~2026-05-03 19:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-02 19:04 [PATCH v2 0/2] Support active-high alert polarity for LM75 Markus Stockhausen
2026-05-02 19:04 ` [PATCH v2 1/2] dt-bindings: hwmon: lm75: Add ti,alert-polarity-active-high property Markus Stockhausen
2026-05-02 19:19   ` sashiko-bot
2026-05-03 18:13   ` Conor Dooley
2026-05-02 19:04 ` [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity Markus Stockhausen
2026-05-02 19:36   ` sashiko-bot
2026-05-03 19:06     ` Guenter Roeck

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