[PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Amna Waseem]

The INA230 has alert polarity bit in Mask/Enable register which can be
configured to be active high or active low depending upon the requirements
of the hardware using this chip. The patches in this series adds the support
for passing alert polarity value from device tree to the driver. Alert polarity
property is added device tree bindings and the driver is modified to read
this property and set the Alert polarity (APOL) bit value in Mask/Enable register
of INA230.

Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>
---
Changes in v4:
- Remove unnecessary checks while setting alert polarity bit
- Link to v3: https://lore.kernel.org/r/20240603-apol-ina2xx-fix-v3-0-b9eff3158e4e@axis.com

Changes in v3:
- Convert the alert polarity property in dt bindings to be a flag
- Set alert polarity depending upon on the presence of flag in
  device tree. Otherwise, default value is set
- Make setting of alert polarity to be chip specific since only
  ina226,ina230 and ina231 supports alert polarity setting. 
- Link to v2: https://lore.kernel.org/r/20240529-apol-ina2xx-fix-v2-0-ee2d76142de2@axis.com

Changes in v2:
- Add vendor specific prefix to alert polarity property in binding.
- Minor improvement in description of alert polarity binding property
- Remove usage of mutex while setting alert polarity in Mask/Enable
  register
- Correct indentation
- Link to v1: https://lore.kernel.org/r/20240529-apol-ina2xx-fix-v1-0-77b4b382190f@axis.com

---
Amna Waseem (2):
      dt-bindings: hwmon: ti,ina2xx: Add ti,alert-polarity-active-high property
      hwmon: (ina2xx) Add device tree support to pass alert polarity

 .../devicetree/bindings/hwmon/ti,ina2xx.yaml       |  9 ++++++
 drivers/hwmon/ina2xx.c                             | 32 ++++++++++++++++++++++
 2 files changed, 41 insertions(+)
---
base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
change-id: 20240524-apol-ina2xx-fix-34e76346cb26

Best regards,
-- 
Amna Waseem <Amna.Waseem@axis.com>

[PATCH v4 1/2] dt-bindings: hwmon: ti,ina2xx: Add ti,alert-polarity-active-high property

[Amna Waseem]

Add a property to the binding to configure the Alert Polarity.
Alert pin is asserted based on the value of Alert Polarity bit of
Mask/Enable register. It is by default 0 which means Alert pin is
configured to be active low open collector. Value of 1 maps to
Inverted (active high open collector).

Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>
---
 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index df86c2c92037..9190ef0bda54 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -66,6 +66,14 @@ properties:
     description: phandle to the regulator that provides the VS supply typically
       in range from 2.7 V to 5.5 V.
 
+  ti,alert-polarity-active-high:
+    description: Alert pin is asserted based on the value of Alert polarity Bit
+      of Mask/Enable register. Default value is Normal (0 which maps to
+      active-low open collector). The other value is Inverted
+      (1 which maps to active-high open collector). Specify this property to set
+      the alert polarity to active-high. 
+    $ref: /schemas/types.yaml#/definitions/flag
+
 required:
   - compatible
   - reg
@@ -88,5 +96,6 @@ examples:
             label = "vdd_3v0";
             shunt-resistor = <1000>;
             vs-supply = <&vdd_3v0>;
+            ti,alert-polarity-active-high;
         };
     };

-- 
2.30.2

[PATCH v4 2/2] hwmon: (ina2xx) Add device tree support to pass alert polarity

[Amna Waseem]

The INA230 has an Alert pin which is asserted when the alert
function selected in the Mask/Enable register exceeds the
value programmed into the Alert Limit register. Assertion is based
on the Alert Polarity Bit (APOL, bit 1 of the Mask/Enable register).
It is default set to value 0 i.e Normal (active-low open collector).
However, hardware can be designed in such a way that expects Alert pin
to become active high if a user-defined threshold in Alert limit
register has been exceeded. This patch adds a way to pass alert polarity
value to the driver via device tree.

Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>
---
 drivers/hwmon/ina2xx.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index d8415d1f21fc..f26ba479bf5c 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -73,6 +73,11 @@
 #define INA226_READ_AVG(reg)		(((reg) & INA226_AVG_RD_MASK) >> 9)
 #define INA226_SHIFT_AVG(val)		((val) << 9)
 
+#define INA226_ALERT_POLARITY_MASK		0x0002
+#define INA226_SHIFT_ALERT_POLARITY(val)	((val) << 1)
+#define INA226_ALERT_POL_LOW			0
+#define INA226_ALERT_POL_HIGH			1
+
 /* bit number of alert functions in Mask/Enable Register */
 #define INA226_SHUNT_OVER_VOLTAGE_BIT	15
 #define INA226_SHUNT_UNDER_VOLTAGE_BIT	14
@@ -178,6 +183,14 @@ static u16 ina226_interval_to_reg(int interval)
 	return INA226_SHIFT_AVG(avg_bits);
 }
 
+static int ina2xx_set_alert_polarity(struct ina2xx_data *data,
+				     unsigned long val)
+{
+	return regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
+				 INA226_ALERT_POLARITY_MASK,
+				 INA226_SHIFT_ALERT_POLARITY(val));
+}
+
 /*
  * Calibration register is set to the best value, which eliminates
  * truncation errors on calculating current register in hardware.
@@ -659,6 +672,25 @@ static int ina2xx_probe(struct i2c_client *client)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to enable vs regulator\n");
 
+	if (chip == ina226) {
+		if (of_property_read_bool(dev->of_node, "ti,alert-polarity-active-high")) {
+			ret = ina2xx_set_alert_polarity(data,
+							INA226_ALERT_POL_HIGH);
+			if (ret < 0) {
+				return dev_err_probe(dev, ret,
+					"failed to set alert polarity active high\n");
+			}
+		} else {
+			/* Set default value i.e active low */
+			ret = ina2xx_set_alert_polarity(data,
+							INA226_ALERT_POL_LOW);
+			if (ret < 0) {
+				return dev_err_probe(dev, ret,
+					"failed to set alert polarity active low\n");
+			}
+		}
+	}
+
 	ret = ina2xx_init(data);
 	if (ret < 0) {
 		dev_err(dev, "error configuring the device: %d\n", ret);

-- 
2.30.2

Re: [PATCH v4 2/2] hwmon: (ina2xx) Add device tree support to pass alert polarity

[Guenter Roeck]

On Tue, Jun 11, 2024 at 11:36:26AM +0200, Amna Waseem wrote:
> The INA230 has an Alert pin which is asserted when the alert
> function selected in the Mask/Enable register exceeds the
> value programmed into the Alert Limit register. Assertion is based
> on the Alert Polarity Bit (APOL, bit 1 of the Mask/Enable register).
> It is default set to value 0 i.e Normal (active-low open collector).
> However, hardware can be designed in such a way that expects Alert pin
> to become active high if a user-defined threshold in Alert limit
> register has been exceeded. This patch adds a way to pass alert polarity
> value to the driver via device tree.
> 
> Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>

Applied.

Guenter

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Krzysztof Kozlowski]

On 11/06/2024 11:36, Amna Waseem wrote:
> The INA230 has alert polarity bit in Mask/Enable register which can be
> configured to be active high or active low depending upon the requirements
> of the hardware using this chip. The patches in this series adds the support
> for passing alert polarity value from device tree to the driver. Alert polarity
> property is added device tree bindings and the driver is modified to read
> this property and set the Alert polarity (APOL) bit value in Mask/Enable register
> of INA230.
> 
> Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>
> ---
> Changes in v4:
> - Remove unnecessary checks while setting alert polarity bit
> - Link to v3: https://lore.kernel.org/r/20240603-apol-ina2xx-fix-v3-0-b9eff3158e4e@axis.com

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

Missing this is really odd, considering you are using b4. Please read
the b4 guide.

Best regards,
Krzysztof

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Amna Waseem]

On 6/11/24 14:49, Krzysztof Kozlowski wrote:
> On 11/06/2024 11:36, Amna Waseem wrote:
>> The INA230 has alert polarity bit in Mask/Enable register which can be
>> configured to be active high or active low depending upon the requirements
>> of the hardware using this chip. The patches in this series adds the support
>> for passing alert polarity value from device tree to the driver. Alert polarity
>> property is added device tree bindings and the driver is modified to read
>> this property and set the Alert polarity (APOL) bit value in Mask/Enable register
>> of INA230.
>>
>> Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>
>> ---
>> Changes in v4:
>> - Remove unnecessary checks while setting alert polarity bit
>> - Link to v3: https://lore.kernel.org/r/20240603-apol-ina2xx-fix-v3-0-b9eff3158e4e@axis.com
> <form letter>
> This is a friendly reminder during the review process.
>
> It looks like you received a tag and forgot to add it.
>
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions, under or above your Signed-off-by tag. Tag is "received", when
> provided in a message replied to you on the mailing list. Tools like b4
> can help here. However, there's no need to repost patches *only* to add
> the tags. The upstream maintainer will do that for tags received on the
> version they apply.
>
> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
>
> If a tag was not added on purpose, please state why and what changed.
> </form letter>
>
> Missing this is really odd, considering you are using b4. Please read
> the b4 guide.
>
> Best regards,
> Krzysztof
>
Thanks Krzysztof for the information. I have read the documentation in 
the link you provided and I will keep it in mind next time if I submit 
patches.

Regards

Amna

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Krzysztof Kozlowski]

On 12/06/2024 08:18, Amna Waseem wrote:
> On 6/11/24 14:49, Krzysztof Kozlowski wrote:
>> On 11/06/2024 11:36, Amna Waseem wrote:
>>> The INA230 has alert polarity bit in Mask/Enable register which can be
>>> configured to be active high or active low depending upon the requirements
>>> of the hardware using this chip. The patches in this series adds the support
>>> for passing alert polarity value from device tree to the driver. Alert polarity
>>> property is added device tree bindings and the driver is modified to read
>>> this property and set the Alert polarity (APOL) bit value in Mask/Enable register
>>> of INA230.
>>>
>>> Signed-off-by: Amna Waseem <Amna.Waseem@axis.com>
>>> ---
>>> Changes in v4:
>>> - Remove unnecessary checks while setting alert polarity bit
>>> - Link to v3: https://lore.kernel.org/r/20240603-apol-ina2xx-fix-v3-0-b9eff3158e4e@axis.com
>> <form letter>
>> This is a friendly reminder during the review process.
>>
>> It looks like you received a tag and forgot to add it.
>>
>> If you do not know the process, here is a short explanation:
>> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
>> versions, under or above your Signed-off-by tag. Tag is "received", when
>> provided in a message replied to you on the mailing list. Tools like b4
>> can help here. However, there's no need to repost patches *only* to add
>> the tags. The upstream maintainer will do that for tags received on the
>> version they apply.
>>
>> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
>>
>> If a tag was not added on purpose, please state why and what changed.
>> </form letter>
>>
>> Missing this is really odd, considering you are using b4. Please read
>> the b4 guide.
>>
>> Best regards,
>> Krzysztof
>>
> Thanks Krzysztof for the information. I have read the documentation in 
> the link you provided and I will keep it in mind next time if I submit 
> patches.
> 

So you require someone to re-review?

Best regards,
Krzysztof

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Amna Waseem]

On 6/12/24 08:20, Krzysztof Kozlowski wrote:
> So you require someone to re-review?

Yes that will be great if someone re-review.

Regards

Amna

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Krzysztof Kozlowski]

On 12/06/2024 08:37, Amna Waseem wrote:
> On 6/12/24 08:20, Krzysztof Kozlowski wrote:
>> So you require someone to re-review?
> 
> Yes that will be great if someone re-review.

You are joking, right? We have hundreds of other patches to take care of
and you ignore review tag, so you get duplicated work from the same
reviewer? That's huge waste of time and actually quite disrespectful to
our time.

Best regards,
Krzysztof

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Amna Waseem]

On 6/12/24 08:44, Krzysztof Kozlowski wrote:
> On 12/06/2024 08:37, Amna Waseem wrote:
>> On 6/12/24 08:20, Krzysztof Kozlowski wrote:
>>> So you require someone to re-review?
>> Yes that will be great if someone re-review.
> You are joking, right? We have hundreds of other patches to take care of
> and you ignore review tag, so you get duplicated work from the same
> reviewer? That's huge waste of time and actually quite disrespectful to
> our time.
>
> Best regards,
> Krzysztof
>
I donot mean to be disrespectful to the reviewers. Please ignore my 
re-review request.  If the patches needs further changes in the code, I 
will add the tag in the next submission.

Regards

Amna

Re: [PATCH v4 0/2] hwmon: (ina2xx):Add Suppor for passing alert polarity from device tree to driver

[Guenter Roeck]

On 6/11/24 23:44, Krzysztof Kozlowski wrote:
> On 12/06/2024 08:37, Amna Waseem wrote:
>> On 6/12/24 08:20, Krzysztof Kozlowski wrote:
>>> So you require someone to re-review?
>>
>> Yes that will be great if someone re-review.
> 
> You are joking, right? We have hundreds of other patches to take care of
> and you ignore review tag, so you get duplicated work from the same
> reviewer? That's huge waste of time and actually quite disrespectful to
> our time.
> 

I applied v3 of the devicetree patch (after comparing the two to make sure
there are no changes).

Amna, I would suggest to run checkpatch on your patches in the future
to find problems such as the following.

ERROR: trailing whitespace
#178: FILE: Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml:74:
+      the alert polarity to active-high. $

Thanks,
Guenter