[PATCH 0/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[PATCH 0/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Manaf Meethalavalappu Pallikunhi]

Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
monitor driver. The BCL peripheral is present in Qualcomm PMICs and
provides real-time monitoring battery overcurrent and under voltage
conditions.

Hardware Overview:
The BCL hardware monitors battery voltage and current through dedicated
channels with configurable thresholds. It supports different alarm
levels with independent interrupt lines:
- Level 0 (LVL0): Maximum/Minimum threshold - mapped to hwmon min/max
- Level 1 (LVL1): Critical threshold - mapped to hwmon lcrit/crit

There are 3 modes of BCL hardware
- BMX -  Both under voltage and over current monitor support
- CORE - Only under voltage monitor support
- WB -   Both under voltage and over current monitor support. It also
         supports current comparator for current monitor

The hardware uses different threshold representation schemes:
- Raw ADC values for Level 0 voltage thresholds
- Indexed values for Level 1 voltage thresholds
- Raw ADC values with variant-specific scaling for current thresholds
- Indexed value for current threshold for BCL WB modes.

Voltage and current monitoring can be independently enabled/disabled
by firmware, and the driver automatically detects the enabled features
at probe time.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
Manaf Meethalavalappu Pallikunhi (4):
      dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings
      hwmon: Add Qualcomm PMIC BCL hardware monitor driver
      arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device
      arm64: dts: qcom: pm8350c: Enable Qualcomm BCL device

 .../devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml  | 128 +++
 MAINTAINERS                                        |   9 +
 arch/arm64/boot/dts/qcom/pm7250b.dtsi              |  10 +
 arch/arm64/boot/dts/qcom/pm8350c.dtsi              |   9 +
 drivers/hwmon/Kconfig                              |   9 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/qcom-bcl-hwmon.c                     | 982 +++++++++++++++++++++
 drivers/hwmon/qcom-bcl-hwmon.h                     | 311 +++++++
 8 files changed, 1459 insertions(+)
---
base-commit: 0f8a890c4524d6e4013ff225e70de2aed7e6d726
change-id: 20260206-qcom-bcl-hwmon-149ce73d9440

Best regards,
-- 
Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>

[PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Manaf Meethalavalappu Pallikunhi]

Add devicetree binding documentation for Qualcomm PMIC Battery Current
Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
and alarm functionality for battery overcurrent and battery/system
under voltage conditions.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 .../devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml  | 128 +++++++++++++++++++++
 1 file changed, 128 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
new file mode 100644
index 000000000000..a0e8eaf13eec
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/qcom,bcl-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SPMI PMIC Battery Current Limiting (BCL) Hardware Monitor
+
+maintainers:
+  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
+
+description: |
+  SPMI PMIC Battery Current Limiting (BCL) hardware provides monitoring and
+  alarm functionality for battery overcurrent and battery or system under
+  voltage conditions. It monitors battery voltage and current, and
+  can trigger interrupts when configurable thresholds are exceeded.
+
+properties:
+  compatible:
+    oneOf:
+      - description: v1 based BCL
+        items:
+          - enum:
+              - qcom,pm7250b-bcl
+              - qcom,pm8250b-bcl
+          - const: qcom,bcl-v1
+
+      - description: v2 based BCL
+        items:
+          - enum:
+              - qcom,pm8350b-bcl
+              - qcom,pm8350c-bcl
+          - const: qcom,bcl-v2
+
+      - description: v3 bmx based BCL
+        items:
+          - enum:
+              - qcom,pm8550b-bcl
+              - qcom,pm7550ba-bcl
+          - const: qcom,bcl-v3-bmx
+
+      - description: v3 core based BCL
+        items:
+          - enum:
+              - qcom,pm8550-bc0l
+              - qcom,pm7550-bcl
+          - const: qcom,bcl-v3-core
+
+      - description: v3 wb based BCL
+        items:
+          - enum:
+              - qcom,pmw5100-bcl
+          - const: qcom,bcl-v3-wb
+
+      - description: v4 bmx based BCL
+        items:
+          - enum:
+              - qcom,pmih010-bcl
+          - const: qcom,bcl-v4-bmx
+
+      - description: v4 bmx with different scale based BCL
+        items:
+          - enum:
+              - qcom,pmv010-bcl
+          - const: qcom,bcl-v4-pmv010
+
+      - description: v4 core based BCL
+        items:
+          - enum:
+              - qcom,pmh010-bcl
+          - const: qcom,bcl-v4-core
+
+      - description: v4 wb based BCL
+        items:
+          - enum:
+              - qcom,pmw6100-bcl
+          - const: qcom,bcl-v4-wb
+
+  reg:
+    maxItems: 1
+    description: BCL base address in the SPMI PMIC register map
+
+  interrupts:
+    minItems: 2
+    maxItems: 2
+    description:
+      BCL alarm interrupts for different threshold levels
+
+  interrupt-names:
+    items:
+      - const: bcl-max-min
+      - const: bcl-critical
+
+  overcurrent-thresholds-milliamp:
+    description:
+      Current thresholds in milliamperes for the two configurable current
+      alarm levels (max and critical). These values are used to override
+      default thresholds if a platform has different battery ocp specification.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    maxItems: 2
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    pmic {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bcl@1d00 {
+            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
+            reg = <0x1d00>;
+            interrupts = <0x2 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
+                         <0x2 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "bcl-max-min",
+                              "bcl-critical";
+            overcurrent-thresholds-milliamp = <5500 6000>;
+        };
+    };

-- 
2.43.0

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Krzysztof Kozlowski]

On Fri, Feb 06, 2026 at 02:44:05AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> Add devicetree binding documentation for Qualcomm PMIC Battery Current

Subject - You almost hit bingo of what not to do. You miss the third one.
When you hit the bingo, get in touch for a beer.

And before, read the docs, because this was repeated SO MANY TIMES.


> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
> and alarm functionality for battery overcurrent and battery/system
> under voltage conditions.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml  | 128 +++++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
> new file mode 100644
> index 000000000000..a0e8eaf13eec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/qcom,bcl-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SPMI PMIC Battery Current Limiting (BCL) Hardware Monitor
> +
> +maintainers:
> +  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  SPMI PMIC Battery Current Limiting (BCL) hardware provides monitoring and
> +  alarm functionality for battery overcurrent and battery or system under
> +  voltage conditions. It monitors battery voltage and current, and
> +  can trigger interrupts when configurable thresholds are exceeded.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: v1 based BCL
> +        items:
> +          - enum:
> +              - qcom,pm7250b-bcl
> +              - qcom,pm8250b-bcl
> +          - const: qcom,bcl-v1

Drop all bcl fallbacks. Pointless, incorrect and misleading.

> +
> +      - description: v2 based BCL
> +        items:
> +          - enum:
> +              - qcom,pm8350b-bcl
> +              - qcom,pm8350c-bcl
> +          - const: qcom,bcl-v2
> +
> +      - description: v3 bmx based BCL
> +        items:
> +          - enum:
> +              - qcom,pm8550b-bcl
> +              - qcom,pm7550ba-bcl
> +          - const: qcom,bcl-v3-bmx
> +
> +      - description: v3 core based BCL
> +        items:
> +          - enum:
> +              - qcom,pm8550-bc0l
> +              - qcom,pm7550-bcl
> +          - const: qcom,bcl-v3-core
> +
> +      - description: v3 wb based BCL
> +        items:
> +          - enum:
> +              - qcom,pmw5100-bcl
> +          - const: qcom,bcl-v3-wb
> +
> +      - description: v4 bmx based BCL
> +        items:
> +          - enum:
> +              - qcom,pmih010-bcl
> +          - const: qcom,bcl-v4-bmx
> +
> +      - description: v4 bmx with different scale based BCL
> +        items:
> +          - enum:
> +              - qcom,pmv010-bcl
> +          - const: qcom,bcl-v4-pmv010
> +
> +      - description: v4 core based BCL
> +        items:
> +          - enum:
> +              - qcom,pmh010-bcl
> +          - const: qcom,bcl-v4-core
> +
> +      - description: v4 wb based BCL
> +        items:
> +          - enum:
> +              - qcom,pmw6100-bcl
> +          - const: qcom,bcl-v4-wb

I really do not get what you are expressing here but all this is wrong.

> +
> +  reg:
> +    maxItems: 1
> +    description: BCL base address in the SPMI PMIC register map

Bus defines it. Drop descrip=tion.

> +
> +  interrupts:
> +    minItems: 2

Drop.

> +    maxItems: 2
> +    description:
> +      BCL alarm interrupts for different threshold levels

Drop

> +
> +  interrupt-names:
> +    items:
> +      - const: bcl-max-min
> +      - const: bcl-critical

Drop bcl

> +
> +  overcurrent-thresholds-milliamp:
> +    description:
> +      Current thresholds in milliamperes for the two configurable current
> +      alarm levels (max and critical). These values are used to override
> +      default thresholds if a platform has different battery ocp specification.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

No, please read basic guides or presentations about DT bindings. The
suffix already defines this.

> +    minItems: 2
> +    maxItems: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +
> +unevaluatedProperties: false

You did not bother to read the docs prior to posting this.

You are supposed to do internal review first. Did it happen? I checked
and cannot find anything. It's not acceptable to use the community
instead of your internal review.

NAK

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    pmic {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        bcl@1d00 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Manaf Meethalavalappu Pallikunhi]

Hi Krzysztof,


On 2/6/2026 1:39 PM, Krzysztof Kozlowski wrote:
> On Fri, Feb 06, 2026 at 02:44:05AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> Add devicetree binding documentation for Qualcomm PMIC Battery Current
> Subject - You almost hit bingo of what not to do. You miss the third one.
> When you hit the bingo, get in touch for a beer.
>
> And before, read the docs, because this was repeated SO MANY TIMES.
Sure, I will update in next revision
>
>
>> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
>> and alarm functionality for battery overcurrent and battery/system
>> under voltage conditions.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   .../devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml  | 128 +++++++++++++++++++++
>>   1 file changed, 128 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
>> new file mode 100644
>> index 000000000000..a0e8eaf13eec
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
>> @@ -0,0 +1,128 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/qcom,bcl-hwmon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SPMI PMIC Battery Current Limiting (BCL) Hardware Monitor
>> +
>> +maintainers:
>> +  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> +
>> +description: |
> Do not need '|' unless you need to preserve formatting.
ACK
>
>> +  SPMI PMIC Battery Current Limiting (BCL) hardware provides monitoring and
>> +  alarm functionality for battery overcurrent and battery or system under
>> +  voltage conditions. It monitors battery voltage and current, and
>> +  can trigger interrupts when configurable thresholds are exceeded.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: v1 based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm7250b-bcl
>> +              - qcom,pm8250b-bcl
>> +          - const: qcom,bcl-v1
> Drop all bcl fallbacks. Pointless, incorrect and misleading.
ACK
>
>> +
>> +      - description: v2 based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm8350b-bcl
>> +              - qcom,pm8350c-bcl
>> +          - const: qcom,bcl-v2
>> +
>> +      - description: v3 bmx based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm8550b-bcl
>> +              - qcom,pm7550ba-bcl
>> +          - const: qcom,bcl-v3-bmx
>> +
>> +      - description: v3 core based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm8550-bc0l
>> +              - qcom,pm7550-bcl
>> +          - const: qcom,bcl-v3-core
>> +
>> +      - description: v3 wb based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmw5100-bcl
>> +          - const: qcom,bcl-v3-wb
>> +
>> +      - description: v4 bmx based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmih010-bcl
>> +          - const: qcom,bcl-v4-bmx
>> +
>> +      - description: v4 bmx with different scale based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmv010-bcl
>> +          - const: qcom,bcl-v4-pmv010
>> +
>> +      - description: v4 core based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmh010-bcl
>> +          - const: qcom,bcl-v4-core
>> +
>> +      - description: v4 wb based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmw6100-bcl
>> +          - const: qcom,bcl-v4-wb
> I really do not get what you are expressing here but all this is wrong.
>
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: BCL base address in the SPMI PMIC register map
> Bus defines it. Drop descrip=tion.
ACK
>
>> +
>> +  interrupts:
>> +    minItems: 2
> Drop.
ACK
>
>> +    maxItems: 2
>> +    description:
>> +      BCL alarm interrupts for different threshold levels
> Drop
ACK
>
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: bcl-max-min
>> +      - const: bcl-critical
> Drop bcl
ACK
>
>> +
>> +  overcurrent-thresholds-milliamp:
>> +    description:
>> +      Current thresholds in milliamperes for the two configurable current
>> +      alarm levels (max and critical). These values are used to override
>> +      default thresholds if a platform has different battery ocp specification.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> No, please read basic guides or presentations about DT bindings. The
> suffix already defines this.
ACK
>
>> +    minItems: 2
>> +    maxItems: 2
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - interrupt-names
>> +
>> +unevaluatedProperties: false
> You did not bother to read the docs prior to posting this.
>
> You are supposed to do internal review first. Did it happen? I checked
> and cannot find anything. It's not acceptable to use the community
> instead of your internal review.
>
> NAK
My understanding is, it is optional. I will send v2 for internal review.
>
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    pmic {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        bcl@1d00 {
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> If you cannot find a name matching your device, please check in kernel
> sources for similar cases or you can grow the spec (via pull request to
> DT spec repo).

ACK, Will use  node name as  sensor

Thanks,

Manaf

>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Dmitry Baryshkov]

On Fri, Feb 06, 2026 at 02:44:05AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> Add devicetree binding documentation for Qualcomm PMIC Battery Current
> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
> and alarm functionality for battery overcurrent and battery/system
> under voltage conditions.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml  | 128 +++++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
> new file mode 100644
> index 000000000000..a0e8eaf13eec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/qcom,bcl-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SPMI PMIC Battery Current Limiting (BCL) Hardware Monitor
> +
> +maintainers:
> +  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> +
> +description: |
> +  SPMI PMIC Battery Current Limiting (BCL) hardware provides monitoring and
> +  alarm functionality for battery overcurrent and battery or system under
> +  voltage conditions. It monitors battery voltage and current, and
> +  can trigger interrupts when configurable thresholds are exceeded.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: v1 based BCL
> +        items:
> +          - enum:
> +              - qcom,pm7250b-bcl
> +              - qcom,pm8250b-bcl
> +          - const: qcom,bcl-v1
> +
> +      - description: v2 based BCL
> +        items:
> +          - enum:
> +              - qcom,pm8350b-bcl
> +              - qcom,pm8350c-bcl
> +          - const: qcom,bcl-v2
> +
> +      - description: v3 bmx based BCL

You need to explain your abbreviations. What is BMX? What is WB (and
yep, they should be capital letters). What is the difference between all
three of them?

> +        items:
> +          - enum:
> +              - qcom,pm8550b-bcl
> +              - qcom,pm7550ba-bcl
> +          - const: qcom,bcl-v3-bmx
> +
> +      - description: v3 core based BCL
> +        items:
> +          - enum:
> +              - qcom,pm8550-bc0l
> +              - qcom,pm7550-bcl
> +          - const: qcom,bcl-v3-core
> +
> +      - description: v3 wb based BCL
> +        items:
> +          - enum:
> +              - qcom,pmw5100-bcl
> +          - const: qcom,bcl-v3-wb
> +
> +      - description: v4 bmx based BCL
> +        items:
> +          - enum:
> +              - qcom,pmih010-bcl
> +          - const: qcom,bcl-v4-bmx
> +
> +      - description: v4 bmx with different scale based BCL
> +        items:
> +          - enum:
> +              - qcom,pmv010-bcl
> +          - const: qcom,bcl-v4-pmv010
> +
> +      - description: v4 core based BCL
> +        items:
> +          - enum:
> +              - qcom,pmh010-bcl
> +          - const: qcom,bcl-v4-core
> +
> +      - description: v4 wb based BCL
> +        items:
> +          - enum:
> +              - qcom,pmw6100-bcl
> +          - const: qcom,bcl-v4-wb
> +
> +  reg:
> +    maxItems: 1
> +    description: BCL base address in the SPMI PMIC register map
> +
> +  interrupts:
> +    minItems: 2
> +    maxItems: 2
> +    description:
> +      BCL alarm interrupts for different threshold levels
> +
> +  interrupt-names:
> +    items:
> +      - const: bcl-max-min
> +      - const: bcl-critical
> +
> +  overcurrent-thresholds-milliamp:
> +    description:
> +      Current thresholds in milliamperes for the two configurable current
> +      alarm levels (max and critical). These values are used to override
> +      default thresholds if a platform has different battery ocp specification.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    maxItems: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    pmic {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        bcl@1d00 {
> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
> +            reg = <0x1d00>;
> +            interrupts = <0x2 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
> +                         <0x2 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
> +            interrupt-names = "bcl-max-min",
> +                              "bcl-critical";
> +            overcurrent-thresholds-milliamp = <5500 6000>;

How is going to be used by the power supply subsystem?

> +        };
> +    };
> 
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Manaf Meethalavalappu Pallikunhi]

Hi Dimitry,

On 2/6/2026 2:38 PM, Dmitry Baryshkov wrote:
> On Fri, Feb 06, 2026 at 02:44:05AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> Add devicetree binding documentation for Qualcomm PMIC Battery Current
>> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
>> and alarm functionality for battery overcurrent and battery/system
>> under voltage conditions.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   .../devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml  | 128 +++++++++++++++++++++
>>   1 file changed, 128 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
>> new file mode 100644
>> index 000000000000..a0e8eaf13eec
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
>> @@ -0,0 +1,128 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/qcom,bcl-hwmon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SPMI PMIC Battery Current Limiting (BCL) Hardware Monitor
>> +
>> +maintainers:
>> +  - Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> +
>> +description: |
>> +  SPMI PMIC Battery Current Limiting (BCL) hardware provides monitoring and
>> +  alarm functionality for battery overcurrent and battery or system under
>> +  voltage conditions. It monitors battery voltage and current, and
>> +  can trigger interrupts when configurable thresholds are exceeded.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: v1 based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm7250b-bcl
>> +              - qcom,pm8250b-bcl
>> +          - const: qcom,bcl-v1
>> +
>> +      - description: v2 based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm8350b-bcl
>> +              - qcom,pm8350c-bcl
>> +          - const: qcom,bcl-v2
>> +
>> +      - description: v3 bmx based BCL
> You need to explain your abbreviations. What is BMX? What is WB (and
> yep, they should be capital letters). What is the difference between all
> three of them?
ACK, Will update in next revision
>
>> +        items:
>> +          - enum:
>> +              - qcom,pm8550b-bcl
>> +              - qcom,pm7550ba-bcl
>> +          - const: qcom,bcl-v3-bmx
>> +
>> +      - description: v3 core based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm8550-bc0l
>> +              - qcom,pm7550-bcl
>> +          - const: qcom,bcl-v3-core
>> +
>> +      - description: v3 wb based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmw5100-bcl
>> +          - const: qcom,bcl-v3-wb
>> +
>> +      - description: v4 bmx based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmih010-bcl
>> +          - const: qcom,bcl-v4-bmx
>> +
>> +      - description: v4 bmx with different scale based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmv010-bcl
>> +          - const: qcom,bcl-v4-pmv010
>> +
>> +      - description: v4 core based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmh010-bcl
>> +          - const: qcom,bcl-v4-core
>> +
>> +      - description: v4 wb based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pmw6100-bcl
>> +          - const: qcom,bcl-v4-wb
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: BCL base address in the SPMI PMIC register map
>> +
>> +  interrupts:
>> +    minItems: 2
>> +    maxItems: 2
>> +    description:
>> +      BCL alarm interrupts for different threshold levels
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: bcl-max-min
>> +      - const: bcl-critical
>> +
>> +  overcurrent-thresholds-milliamp:
>> +    description:
>> +      Current thresholds in milliamperes for the two configurable current
>> +      alarm levels (max and critical). These values are used to override
>> +      default thresholds if a platform has different battery ocp specification.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 2
>> +    maxItems: 2
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - interrupt-names
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    pmic {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        bcl@1d00 {
>> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>> +            reg = <0x1d00>;
>> +            interrupts = <0x2 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>> +                         <0x2 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>> +            interrupt-names = "bcl-max-min",
>> +                              "bcl-critical";
>> +            overcurrent-thresholds-milliamp = <5500 6000>;
> How is going to be used by the power supply subsystem?

There is no any interaction with power supply subsystem for bcl alarms

Thanks,

Manaf

>
>> +        };
>> +    };
>>
>> -- 
>> 2.43.0
>>

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Konrad Dybcio]

On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Add devicetree binding documentation for Qualcomm PMIC Battery Current
> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
> and alarm functionality for battery overcurrent and battery/system
> under voltage conditions.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---

[...]

> +properties:
> +  compatible:
> +    oneOf:
> +      - description: v1 based BCL
> +        items:
> +          - enum:
> +              - qcom,pm7250b-bcl
> +              - qcom,pm8250b-bcl
> +          - const: qcom,bcl-v1

I see that e.g. PMI8998 has a BCL block, would that also be 'v1',
or something else?

Konrad

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,

On 2/6/2026 2:38 PM, Konrad Dybcio wrote:
> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Add devicetree binding documentation for Qualcomm PMIC Battery Current
>> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
>> and alarm functionality for battery overcurrent and battery/system
>> under voltage conditions.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
> [...]
>
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: v1 based BCL
>> +        items:
>> +          - enum:
>> +              - qcom,pm7250b-bcl
>> +              - qcom,pm8250b-bcl
>> +          - const: qcom,bcl-v1
> I see that e.g. PMI8998 has a BCL block, would that also be 'v1',
> or something else?

I added support for pmic 5 bcl design from v1 to v4 in this series. 
PMI8998 is older pmic design(pmic 4). As of now, we don't have any 
requirement to enable it

Thanks,

Manaf

>
> Konrad

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Konrad Dybcio]

On 2/13/26 7:04 AM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Konrad,
> 
> On 2/6/2026 2:38 PM, Konrad Dybcio wrote:
>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> Add devicetree binding documentation for Qualcomm PMIC Battery Current
>>> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
>>> and alarm functionality for battery overcurrent and battery/system
>>> under voltage conditions.
>>>
>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>> ---
>> [...]
>>
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - description: v1 based BCL
>>> +        items:
>>> +          - enum:
>>> +              - qcom,pm7250b-bcl
>>> +              - qcom,pm8250b-bcl
>>> +          - const: qcom,bcl-v1
>> I see that e.g. PMI8998 has a BCL block, would that also be 'v1',
>> or something else?
> 
> I added support for pmic 5 bcl design from v1 to v4 in this series. PMI8998 is older pmic design(pmic 4). As of now, we don't have any requirement to enable it

Right, but then you never mentioned "PMIC5" anywhere in the compatible
or the binding, so this wasn't obvious at all. With the request from others
to shift towards PMIC-specific compatibles that won't be an issue

Konrad

Re: [PATCH 1/4] dt-bindings: hwmon: Add qcom,bcl-hwmon yaml bindings

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,

On 2/13/2026 4:11 PM, Konrad Dybcio wrote:
> On 2/13/26 7:04 AM, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Konrad,
>>
>> On 2/6/2026 2:38 PM, Konrad Dybcio wrote:
>>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>> Add devicetree binding documentation for Qualcomm PMIC Battery Current
>>>> Limiting (BCL) hardware monitor. The BCL hardware provides monitoring
>>>> and alarm functionality for battery overcurrent and battery/system
>>>> under voltage conditions.
>>>>
>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>> ---
>>> [...]
>>>
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - description: v1 based BCL
>>>> +        items:
>>>> +          - enum:
>>>> +              - qcom,pm7250b-bcl
>>>> +              - qcom,pm8250b-bcl
>>>> +          - const: qcom,bcl-v1
>>> I see that e.g. PMI8998 has a BCL block, would that also be 'v1',
>>> or something else?
>> I added support for pmic 5 bcl design from v1 to v4 in this series. PMI8998 is older pmic design(pmic 4). As of now, we don't have any requirement to enable it
> Right, but then you never mentioned "PMIC5" anywhere in the compatible
> or the binding, so this wasn't obvious at all. With the request from others
> to shift towards PMIC-specific compatibles that won't be an issue
Sure, will move to pmic specific compatibles.
Thanks,
Manaf
>
> Konrad

[PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Manaf Meethalavalappu Pallikunhi]

Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
monitor driver. The BCL peripheral is present in Qualcomm PMICs and
provides real-time monitoring and protection against battery
overcurrent and under voltage conditions.

The driver monitors:
- Battery voltage with configurable low voltage thresholds
- Battery current with configurable high current thresholds
- Two limit alarm interrupts (max/min, critical)

The driver integrates with the Linux hwmon subsystem and provides
standard hwmon attributes for monitoring battery conditions.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 MAINTAINERS                    |   9 +
 drivers/hwmon/Kconfig          |   9 +
 drivers/hwmon/Makefile         |   1 +
 drivers/hwmon/qcom-bcl-hwmon.c | 982 +++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/qcom-bcl-hwmon.h | 311 +++++++++++++
 5 files changed, 1312 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b979cfc04c1e..16452a53ac46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21597,6 +21597,15 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/net/qcom,bam-dmux.yaml
 F:	drivers/net/wwan/qcom_bam_dmux.c
 
+QUALCOMM BCL HARDWARE MONITOR DRIVER
+M:	Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
+L:	linux-hwmon@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/qcom,bcl-hwmon.yaml
+F:	drivers/hwmon/qcom-bcl-hwmon.c
+F:	drivers/hwmon/qcom-bcl-hwmon.h
+
 QUALCOMM BLUETOOTH DRIVER
 M:	Bartosz Golaszewski <brgl@kernel.org>
 L:	linux-arm-msm@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 41c381764c2b..6dd7125559be 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1905,6 +1905,15 @@ config SENSORS_PWM_FAN
 	  This driver can also be built as a module. If so, the module
 	  will be called pwm-fan.
 
+config SENSORS_QCOM_BCL
+	tristate "Qualcomm BCL hardware monitoring"
+	help
+	  Say yes here to enable support for Qualcomm battery over current
+	  and under voltage alarms monitor.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called qcom-bcl-hwmon.
+
 config SENSORS_QNAP_MCU_HWMON
 	tristate "QNAP MCU hardware monitoring"
 	depends on MFD_QNAP_MCU
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index eade8e3b1bde..1b03eecd761d 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -197,6 +197,7 @@ obj-$(CONFIG_SENSORS_POWERZ)	+= powerz.o
 obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
 obj-$(CONFIG_SENSORS_PT5161L)	+= pt5161l.o
 obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
+obj-$(CONFIG_SENSORS_QCOM_BCL)	+= qcom-bcl-hwmon.o
 obj-$(CONFIG_SENSORS_QNAP_MCU_HWMON)	+= qnap-mcu-hwmon.o
 obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
 obj-$(CONFIG_SENSORS_SA67MCU)	+= sa67mcu-hwmon.o
diff --git a/drivers/hwmon/qcom-bcl-hwmon.c b/drivers/hwmon/qcom-bcl-hwmon.c
new file mode 100644
index 000000000000..a7e3b865de5c
--- /dev/null
+++ b/drivers/hwmon/qcom-bcl-hwmon.c
@@ -0,0 +1,982 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm pmic BCL hardware driver for battery overcurrent and
+ * battery or system under voltage monitor
+ *
+ * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/devm-helpers.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/workqueue.h>
+
+#include "qcom-bcl-hwmon.h"
+
+ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
+ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
+
+/* Interrupt names for each alarm level */
+static const char * const bcl_int_names[ALARM_MAX] = {
+	[LVL0] = "bcl-max-min",
+	[LVL1] = "bcl-critical",
+};
+
+static const char * const bcl_channel_label[CHANNEL_MAX] = {
+	"BCL Voltage",
+	"BCL Current",
+};
+
+/* Common Reg Fields */
+static const struct reg_field common_reg_fields[COMMON_FIELD_MAX] = {
+	[F_V_MAJOR]	= REG_FIELD(REVISION2, 0, 7),
+	[F_V_MINOR]	= REG_FIELD(REVISION1, 0, 7),
+	[F_CTL_EN]	= REG_FIELD(EN_CTL1, 7, 7),
+	[F_LVL0_ALARM]	= REG_FIELD(STATUS, 0, 0),
+	[F_LVL1_ALARM]	= REG_FIELD(STATUS, 1, 1),
+};
+
+/* BCL Version/Modes specific fields */
+static const struct reg_field bcl_v1_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(MODE_CTL1, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
+	[F_IN_INPUT_EN]	= REG_FIELD(VADC_CONV_REQ, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
+	[F_CURR_MON_EN]	= REG_FIELD(IADC_CONV_REQ, 0, 0),
+	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
+	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR, 0, 7),
+	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 7),
+};
+
+static const struct reg_field bcl_v2_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 1),
+	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
+	[F_IN_INPUT_EN]	= REG_FIELD(VADC_CONV_REQ, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
+	[F_CURR_MON_EN]	= REG_FIELD(IADC_CONV_REQ, 0, 0),
+	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
+	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR, 0, 7),
+	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 7),
+};
+
+static const struct reg_field bcl_v3_bmx_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
+	[F_IN_INPUT_EN]	= REG_FIELD(PARAM_1, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
+	[F_CURR_MON_EN]	= REG_FIELD(PARAM_1, 1, 1),
+	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
+	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR_GEN3, 0, 7),
+	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 7),
+};
+
+static const struct reg_field bcl_v3_wb_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
+	[F_IN_INPUT_EN]	= REG_FIELD(PARAM_1, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
+	[F_CURR_MON_EN]	= REG_FIELD(PARAM_1, 1, 1),
+	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
+	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR, 0, 3),
+	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 7),
+};
+
+static const struct reg_field bcl_v3_core_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VCMP_L0_THR, 0, 5),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
+	[F_IN_INPUT_EN]	= REG_FIELD(PARAM_1, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
+	[F_CURR_MON_EN]	= REG_FIELD(PARAM_1, 1, 1),
+};
+
+static const struct reg_field bcl_v4_bmx_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
+	[F_IN_INPUT_EN]	= REG_FIELD(PARAM_1, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 15),
+	[F_CURR_MON_EN]	= REG_FIELD(PARAM_1, 1, 1),
+	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
+	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR_GEN3, 0, 7),
+	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 15),
+};
+
+static const struct reg_field bcl_v4_wb_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 6),
+	[F_IN_INPUT_EN]	= REG_FIELD(PARAM_1, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 15),
+	[F_CURR_MON_EN]	= REG_FIELD(PARAM_1, 1, 1),
+	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
+	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR, 0, 4),
+	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 15),
+};
+
+static const struct reg_field bcl_v4_core_reg_fields[] = {
+	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 2),
+	[F_IN_L0_THR]	= REG_FIELD(VCMP_L0_THR, 0, 6),
+	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 6),
+	[F_IN_INPUT_EN]	= REG_FIELD(PARAM_1, 0, 0),
+	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 15),
+	[F_CURR_MON_EN]	= REG_FIELD(PARAM_1, 1, 1),
+};
+
+/* V1 BMX and core */
+static const struct bcl_desc pm7250b_data = {
+	.reg_fields = bcl_v1_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 8,
+	.thresh_field_bits_size = 7,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.max = 10000,
+		.default_scale_nu = 305180,
+		.thresh_type = {ADC, ADC},
+	},
+};
+
+/* V2 BMX and core */
+static const struct bcl_desc pm8350_data = {
+	.reg_fields = bcl_v2_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 8,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.max = 10000,
+		.default_scale_nu = 305180,
+		.thresh_type = {ADC, ADC},
+	},
+};
+
+/* V3 BMX  */
+static const struct bcl_desc pm8550b_data = {
+	.reg_fields = bcl_v3_bmx_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 8,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.max = 12000,
+		.default_scale_nu = 366220,
+		.thresh_type = {ADC, ADC},
+	},
+};
+
+/* V3 WB  */
+static const struct bcl_desc pmw5100_data = {
+	.reg_fields = bcl_v3_wb_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 8,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.base = 800,
+		.max = 2000,
+		.step = 100,
+		.default_scale_nu = 61035,
+		.thresh_type = {ADC, INDEX},
+	},
+};
+
+/* V3 CORE  */
+static const struct bcl_desc pm8550_data = {
+	.reg_fields = bcl_v3_core_reg_fields,
+	.num_reg_fields = F_CURR_MON_EN + 1,
+	.data_field_bits_size = 8,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.thresh_type = {INDEX, INDEX},
+	},
+};
+
+/* V4 BMX  */
+static const struct bcl_desc pmih010_data = {
+	.reg_fields = bcl_v4_bmx_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 16,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.max = 20000,
+		.default_scale_nu = 610370,
+		.thresh_type = {ADC, ADC},
+	},
+};
+
+/* V4 WB  */
+static const struct bcl_desc pmw6100_data = {
+	.reg_fields = bcl_v4_wb_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 16,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 1500,
+		.max = 4000,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.base = 900,
+		.max = 3300,
+		.step = 150,
+		.default_scale_nu = 152586,
+		.thresh_type = {ADC, INDEX},
+	},
+};
+
+/* V4 CORE */
+static const struct bcl_desc pmh010_data = {
+	.reg_fields = bcl_v4_core_reg_fields,
+	.num_reg_fields = F_CURR_MON_EN + 1,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 1500,
+		.max = 4000,
+		.step = 25,
+		.thresh_type = {INDEX, INDEX},
+	},
+};
+
+/* V4 BMX with different scale */
+static const struct bcl_desc pmv010_data = {
+	.reg_fields = bcl_v4_bmx_reg_fields,
+	.num_reg_fields = F_MAX_FIELDS,
+	.data_field_bits_size = 16,
+	.thresh_field_bits_size = 8,
+	.channel[IN] = {
+		.base = 2250,
+		.max = 3600,
+		.step = 25,
+		.default_scale_nu = 194637,
+		.thresh_type = {ADC, INDEX},
+	},
+	.channel[CURR] = {
+		.max = 12000,
+		.default_scale_nu = 366220,
+		.thresh_type = {ADC, ADC},
+	},
+};
+
+/**
+ * bcl_convert_raw_to_milliunit - Convert raw value to milli unit
+ * @desc: BCL device descriptor
+ * @raw_val: Raw ADC value from hardware
+ * @type: type of the channel, in or curr
+ * @field_width: bits size for data or threshold field
+ *
+ * Return: value in milli unit
+ */
+static unsigned int bcl_convert_raw_to_milliunit(const struct bcl_desc *desc, int raw_val,
+					enum bcl_channel_type type, u8 field_width)
+{
+	u32 def_scale = desc->channel[type].default_scale_nu;
+	u32 lsb_weight = field_width > 8 ? 1 : 1 << field_width;
+	u32 scaling_factor = def_scale * lsb_weight;
+
+	return div_s64((s64)raw_val * scaling_factor, 1000000);
+}
+
+/**
+ * bcl_convert_milliunit_to_raw - Convert milli unit to raw value
+ * @desc: BCL device descriptor
+ * @ma_val: threshold value in milli unit
+ * @type: type of the channel, in or curr
+ * @field_width: bits size for data or threshold field
+ *
+ * Return: Raw ADC value for hardware
+ */
+static unsigned int bcl_convert_milliunit_to_raw(const struct bcl_desc *desc, int mval,
+					enum bcl_channel_type type, u8 field_width)
+{
+	u32 def_scale = desc->channel[type].default_scale_nu;
+	u32 lsb_weight = field_width > 8 ? 1 : 1 << field_width;
+	u32 scaling_factor = def_scale * lsb_weight;
+
+	return div_s64((s64)mval * 1000000, scaling_factor);
+}
+
+/**
+ * bcl_convert_milliunit_to_index - Convert milliunit to in or curr index
+ * @desc: BCL device descriptor
+ * @val: in or curr value in milli unit
+ * @type: type of the channel, in or curr
+ *
+ * Converts a value in milli unit to an index for BCL that use indexed thresholds.
+ *
+ * Return: Voltage index value
+ */
+static unsigned int bcl_convert_milliunit_to_index(const struct bcl_desc *desc, int val,
+					  enum bcl_channel_type type)
+{
+	return div_s64((s64)val - desc->channel[type].base, desc->channel[type].step);
+}
+
+/**
+ * bcl_convert_index_to_milliunit - Convert in or curr index to milli unit
+ * @desc: BCL device descriptor
+ * @val: index value
+ * @type: type of the channel, in or curr
+ *
+ * Converts an index value to milli unit for BCL that use indexed thresholds.
+ *
+ * Return: Voltage value in millivolts
+ */
+static unsigned int bcl_convert_index_to_milliunit(const struct bcl_desc *desc, int val,
+					 enum bcl_channel_type type)
+{
+	return desc->channel[type].base + val * desc->channel[type].step;
+}
+
+static int bcl_in_thresh_write(struct bcl_device *bcl, long value, enum bcl_limit_alarm lvl)
+{
+	const struct bcl_desc *desc = bcl->desc;
+	u32 raw_val;
+
+	int thresh = clamp_val(value, desc->channel[IN].base, desc->channel[IN].max);
+
+	if (desc->channel[IN].thresh_type[lvl] == ADC)
+		raw_val = bcl_convert_milliunit_to_raw(desc, thresh, IN,
+						       desc->thresh_field_bits_size);
+	else
+		raw_val = bcl_convert_milliunit_to_index(desc, thresh, IN);
+
+	return regmap_field_write(bcl->fields[F_IN_L0_THR + lvl], raw_val);
+}
+
+static int bcl_curr_thresh_write(struct bcl_device *bcl, long value, enum bcl_limit_alarm lvl)
+{
+	const struct bcl_desc *desc = bcl->desc;
+	u32 raw_val;
+
+	/* Clamp only to curr max */
+	int thresh = clamp_val(value, value, desc->channel[CURR].max);
+
+	if (desc->channel[CURR].thresh_type[lvl] == ADC)
+		raw_val = bcl_convert_milliunit_to_raw(desc, thresh, CURR,
+						       desc->thresh_field_bits_size);
+	else
+		raw_val = bcl_convert_milliunit_to_index(desc, thresh, CURR);
+
+	return regmap_field_write(bcl->fields[F_CURR_H0_THR + lvl], raw_val);
+}
+
+static int bcl_in_thresh_read(struct bcl_device *bcl, enum bcl_limit_alarm lvl, long *out)
+{
+	int ret, thresh;
+	u32 raw_val = 0;
+	const struct bcl_desc *desc = bcl->desc;
+
+	ret = bcl_read_field_value(bcl, F_IN_L0_THR + lvl, &raw_val);
+	if (ret)
+		return ret;
+
+	if (desc->channel[IN].thresh_type[lvl] == ADC)
+		thresh = bcl_convert_raw_to_milliunit(desc, raw_val, IN,
+						      desc->thresh_field_bits_size);
+	else
+		thresh = bcl_convert_index_to_milliunit(desc, raw_val, IN);
+
+	*out = thresh;
+
+	return 0;
+}
+
+static int bcl_curr_thresh_read(struct bcl_device *bcl, enum bcl_limit_alarm lvl, long *out)
+{
+	int ret, thresh;
+	u32 raw_val = 0;
+	const struct bcl_desc *desc = bcl->desc;
+
+	ret = bcl_read_field_value(bcl, F_CURR_H0_THR + lvl, &raw_val);
+	if (ret)
+		return ret;
+
+	if (desc->channel[CURR].thresh_type[lvl] == ADC)
+		thresh = bcl_convert_raw_to_milliunit(desc, raw_val, CURR,
+						      desc->thresh_field_bits_size);
+	else
+		thresh = bcl_convert_index_to_milliunit(desc, raw_val, CURR);
+
+	*out = thresh;
+
+	return 0;
+}
+
+static int bcl_curr_input_read(struct bcl_device *bcl, long *out)
+{
+	int ret;
+	u32 raw_val = 0;
+	const struct bcl_desc *desc = bcl->desc;
+
+	ret = bcl_read_field_value(bcl, F_CURR_INPUT, &raw_val);
+	if (ret)
+		return ret;
+
+	/*
+	 * The sensor sometime can read a value 0 if there are
+	 * consecutive reads
+	 */
+	if (raw_val != 0)
+		bcl->last_curr_input =
+			bcl_convert_raw_to_milliunit(desc, raw_val, CURR,
+						     desc->data_field_bits_size);
+
+	*out = bcl->last_curr_input;
+
+	return 0;
+}
+
+static int bcl_in_input_read(struct bcl_device *bcl, long *out)
+{
+	int ret;
+	u32 raw_val = 0;
+	const struct bcl_desc *desc = bcl->desc;
+
+	ret = bcl_read_field_value(bcl, F_IN_INPUT, &raw_val);
+	if (ret)
+		return ret;
+
+	if (raw_val < GENMASK(desc->data_field_bits_size - 1, 0))
+		bcl->last_in_input =
+			bcl_convert_raw_to_milliunit(desc, raw_val, IN,
+						     desc->data_field_bits_size);
+
+	*out = bcl->last_in_input;
+
+	return 0;
+}
+
+static int bcl_read_alarm_status(struct bcl_device *bcl,
+				 enum bcl_limit_alarm lvl, long *status)
+{
+	int ret;
+	u32 raw_val = 0;
+
+	ret = bcl_read_field_value(bcl, F_LVL0_ALARM + lvl, &raw_val);
+	if (ret)
+		return ret;
+
+	*status = raw_val;
+
+	return 0;
+}
+
+static unsigned int bcl_get_version_major(const struct bcl_device *bcl)
+{
+	u32 raw_val = 0;
+
+	bcl_read_field_value(bcl, F_V_MAJOR, &raw_val);
+
+	return raw_val;
+}
+
+static unsigned int bcl_get_version_minor(const struct bcl_device *bcl)
+{
+	u32 raw_val = 0;
+
+	bcl_read_field_value(bcl, F_V_MINOR, &raw_val);
+
+	return raw_val;
+}
+
+static void bcl_hwmon_notify_event(struct bcl_device *bcl, enum bcl_limit_alarm alarm)
+{
+	if (bcl->in_mon_enabled)
+		hwmon_notify_event(bcl->hwmon_dev, hwmon_in,
+				in_lvl_to_attr_map[alarm], 0);
+	if (bcl->curr_mon_enabled)
+		hwmon_notify_event(bcl->hwmon_dev, hwmon_curr,
+				curr_lvl_to_attr_map[alarm], 0);
+}
+
+static void bcl_alarm_enable_poll(struct work_struct *work)
+{
+	struct bcl_alarm_data *alarm = container_of(work, struct bcl_alarm_data,
+							 alarm_poll_work.work);
+	struct bcl_device *bcl = alarm->device;
+	long status;
+
+	guard(mutex)(&bcl->lock);
+
+	if (bcl_read_alarm_status(bcl, alarm->type, &status))
+		goto re_schedule;
+
+	if (!status & !alarm->irq_enabled) {
+		bcl_enable_irq(alarm);
+		bcl_hwmon_notify_event(bcl, alarm->type);
+		return;
+	}
+
+re_schedule:
+	schedule_delayed_work(&alarm->alarm_poll_work,
+				msecs_to_jiffies(BCL_ALARM_POLLING_MS));
+}
+
+static irqreturn_t bcl_handle_alarm(int irq, void *data)
+{
+	struct bcl_alarm_data *alarm = data;
+	struct bcl_device *bcl = alarm->device;
+	long status;
+
+	guard(mutex)(&bcl->lock);
+
+	if (bcl_read_alarm_status(bcl, alarm->type, &status) || !status)
+		return IRQ_HANDLED;
+
+	if (!bcl->hwmon_dev)
+		return IRQ_HANDLED;
+
+	bcl_hwmon_notify_event(bcl, alarm->type);
+
+	bcl_disable_irq(alarm);
+	schedule_delayed_work(&alarm->alarm_poll_work,
+			msecs_to_jiffies(BCL_ALARM_POLLING_MS));
+
+	dev_dbg(bcl->dev, "Irq:%d triggered for bcl type:%d\n",
+			 irq, alarm->type);
+
+	return IRQ_HANDLED;
+}
+
+static umode_t bcl_hwmon_is_visible(const void *data,
+				    enum hwmon_sensor_types type,
+				    u32 attr, int channel)
+{
+	const struct bcl_device *bcl = data;
+
+	switch (type) {
+	case hwmon_in:
+		if (!bcl->in_mon_enabled)
+			return 0;
+		switch (attr) {
+		case hwmon_in_input:
+			return bcl->in_input_enabled ? 0444 : 0;
+		case hwmon_in_label:
+		case hwmon_in_min_alarm:
+		case hwmon_in_lcrit_alarm:
+			return 0444;
+		case hwmon_in_min:
+		case hwmon_in_lcrit:
+			return 0644;
+		default:
+			return 0;
+		}
+	case hwmon_curr:
+		if (!bcl->curr_mon_enabled)
+			return 0;
+		switch (attr) {
+		case hwmon_curr_input:
+		case hwmon_curr_label:
+		case hwmon_curr_max_alarm:
+		case hwmon_curr_crit_alarm:
+			return 0444;
+		case hwmon_curr_max:
+		case hwmon_curr_crit:
+			return 0644;
+		default:
+			return 0;
+		}
+	default:
+		return 0;
+	}
+}
+
+static int bcl_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, long val)
+{
+	struct bcl_device *bcl = dev_get_drvdata(dev);
+	int ret = -EOPNOTSUPP;
+
+	guard(mutex)(&bcl->lock);
+
+	switch (type) {
+	case hwmon_in:
+		switch (attr) {
+		case hwmon_in_min:
+		case hwmon_in_lcrit:
+			ret = bcl_in_thresh_write(bcl, val, in_attr_to_lvl_map[attr]);
+			break;
+		default:
+			ret = -EOPNOTSUPP;
+		}
+		break;
+	case hwmon_curr:
+		switch (attr) {
+		case hwmon_curr_max:
+		case hwmon_curr_crit:
+			ret = bcl_curr_thresh_write(bcl, val, curr_attr_to_lvl_map[attr]);
+			break;
+		default:
+			ret = -EOPNOTSUPP;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int bcl_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			  u32 attr, int channel, long *value)
+{
+	struct bcl_device *bcl = dev_get_drvdata(dev);
+	int ret;
+
+	guard(mutex)(&bcl->lock);
+
+	switch (type) {
+	case hwmon_in:
+		switch (attr) {
+		case hwmon_in_input:
+			ret = bcl_in_input_read(bcl, value);
+			break;
+		case hwmon_in_min:
+		case hwmon_in_lcrit:
+			ret = bcl_in_thresh_read(bcl, in_attr_to_lvl_map[attr], value);
+			break;
+		case hwmon_in_min_alarm:
+		case hwmon_in_lcrit_alarm:
+			ret = bcl_read_alarm_status(bcl, in_attr_to_lvl_map[attr], value);
+			break;
+		default:
+			ret = -EOPNOTSUPP;
+		}
+		break;
+	case hwmon_curr:
+		switch (attr) {
+		case hwmon_curr_input:
+			ret = bcl_curr_input_read(bcl, value);
+			break;
+		case hwmon_curr_max:
+		case hwmon_curr_crit:
+			ret = bcl_curr_thresh_read(bcl, curr_attr_to_lvl_map[attr], value);
+			break;
+		case hwmon_curr_max_alarm:
+		case hwmon_curr_crit_alarm:
+			ret = bcl_read_alarm_status(bcl, curr_attr_to_lvl_map[attr], value);
+			break;
+		default:
+			ret = -EOPNOTSUPP;
+		}
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	return ret;
+}
+
+static int bcl_hwmon_read_string(struct device *dev,
+				 enum hwmon_sensor_types type,
+				 u32 attr, int channel, const char **str)
+{
+	switch (type) {
+	case hwmon_in:
+		if (attr != hwmon_in_label)
+			break;
+		*str = bcl_channel_label[IN];
+		return 0;
+	case hwmon_curr:
+		if (attr != hwmon_curr_label)
+			break;
+		*str = bcl_channel_label[CURR];
+		return 0;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops bcl_hwmon_ops = {
+	.is_visible	= bcl_hwmon_is_visible,
+	.read		= bcl_hwmon_read,
+	.read_string	= bcl_hwmon_read_string,
+	.write		= bcl_hwmon_write,
+};
+
+static const struct hwmon_channel_info *bcl_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_MIN |
+			   HWMON_I_LCRIT | HWMON_I_MIN_ALARM |
+			   HWMON_I_LCRIT_ALARM),
+	HWMON_CHANNEL_INFO(curr,
+			   HWMON_C_INPUT | HWMON_C_LABEL | HWMON_C_MAX |
+			   HWMON_C_CRIT | HWMON_C_MAX_ALARM |
+			   HWMON_C_CRIT_ALARM),
+	NULL,
+};
+
+static const struct hwmon_chip_info bcl_hwmon_chip_info = {
+	.ops	= &bcl_hwmon_ops,
+	.info	= bcl_hwmon_info,
+};
+
+static int bcl_curr_thresh_update(struct bcl_device *bcl)
+{
+	int ret, i;
+
+	if (!bcl->curr_thresholds[0])
+		return 0;
+
+	for (i = 0; i < ALARM_MAX; i++) {
+		ret = bcl_curr_thresh_write(bcl, bcl->curr_thresholds[i], i);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void bcl_hw_channel_mon_init(struct bcl_device *bcl)
+{
+	bcl->in_mon_enabled = bcl_in_monitor_enabled(bcl);
+	bcl->in_input_enabled = bcl_in_input_enabled(bcl);
+	bcl->curr_mon_enabled = bcl_curr_monitor_enabled(bcl);
+}
+
+static int bcl_alarm_irq_init(struct platform_device *pdev,
+			      struct bcl_device *bcl)
+{
+	int ret = 0, irq_num = 0, i = 0;
+	struct bcl_alarm_data *alarm;
+
+	for (i = LVL0; i < ALARM_MAX; i++) {
+		alarm = &bcl->bcl_alarms[i];
+		alarm->type = i;
+		alarm->device = bcl;
+
+		ret = devm_delayed_work_autocancel(bcl->dev, &alarm->alarm_poll_work,
+					   bcl_alarm_enable_poll);
+		if (ret)
+			return ret;
+
+		irq_num = platform_get_irq_byname(pdev, bcl_int_names[i]);
+		if (irq_num <= 0)
+			continue;
+
+		ret = devm_request_threaded_irq(&pdev->dev, irq_num, NULL,
+						bcl_handle_alarm, IRQF_ONESHOT,
+						bcl_int_names[i], alarm);
+		if (ret) {
+			dev_err(&pdev->dev, "Error requesting irq(%s).err:%d\n",
+				bcl_int_names[i], ret);
+			return ret;
+		}
+		enable_irq_wake(alarm->irq);
+		alarm->irq_enabled = true;
+		alarm->irq = irq_num;
+	}
+
+	return 0;
+}
+
+static int bcl_regmap_field_init(struct device *dev, struct bcl_device *bcl,
+				 const struct bcl_desc *data)
+{
+	int i;
+	struct reg_field fields[F_MAX_FIELDS];
+
+	BUILD_BUG_ON(ARRAY_SIZE(common_reg_fields) != COMMON_FIELD_MAX);
+
+	for (i = 0; i < data->num_reg_fields; i++) {
+		if (i < COMMON_FIELD_MAX)
+			fields[i] = common_reg_fields[i];
+		else
+			fields[i] = data->reg_fields[i];
+
+		/* Need to adjust BCL base from regmap dynamically */
+		fields[i].reg += bcl->base;
+	}
+
+	return devm_regmap_field_bulk_alloc(dev, bcl->regmap, bcl->fields,
+					   fields, data->num_reg_fields);
+}
+
+static int bcl_get_device_property_data(struct platform_device *pdev,
+				   struct bcl_device *bcl)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+	u32 reg;
+
+	ret = device_property_read_u32(dev, "reg", &reg);
+	if (ret < 0)
+		return ret;
+
+	bcl->base = reg;
+
+	device_property_read_u32_array(dev, "overcurrent-thresholds-milliamp",
+				       bcl->curr_thresholds, 2);
+	return 0;
+}
+
+static int bcl_probe(struct platform_device *pdev)
+{
+	struct bcl_device *bcl;
+	int ret;
+
+	bcl = devm_kzalloc(&pdev->dev, sizeof(*bcl), GFP_KERNEL);
+	if (!bcl)
+		return -ENOMEM;
+
+	bcl->dev = &pdev->dev;
+	bcl->desc = device_get_match_data(&pdev->dev);
+	if (!bcl->desc)
+		return -EINVAL;
+
+	ret = devm_mutex_init(bcl->dev, &bcl->lock);
+	if (ret)
+		return ret;
+
+	bcl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!bcl->regmap) {
+		dev_err(&pdev->dev, "Couldn't get parent's regmap\n");
+		return -EINVAL;
+	}
+
+	ret = bcl_get_device_property_data(pdev, bcl);
+	if (ret < 0)
+		return ret;
+
+	ret = bcl_regmap_field_init(bcl->dev, bcl, bcl->desc);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Unable to allocate regmap fields, err:%d\n", ret);
+		return ret;
+	}
+
+	if (!bcl_hw_is_enabled(bcl))
+		return -ENODEV;
+
+	ret = bcl_curr_thresh_update(bcl);
+	if (ret < 0)
+		return ret;
+
+	ret = bcl_alarm_irq_init(pdev, bcl);
+	if (ret < 0)
+		return ret;
+
+	bcl_hw_channel_mon_init(bcl);
+
+	dev_set_drvdata(&pdev->dev, bcl);
+
+	bcl->hwmon_name = devm_hwmon_sanitize_name(&pdev->dev,
+						   dev_name(bcl->dev));
+	if (IS_ERR(bcl->hwmon_name)) {
+		dev_err(&pdev->dev, "Failed to sanitize hwmon name\n");
+		return PTR_ERR(bcl->hwmon_name);
+	}
+
+	bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
+							      bcl->hwmon_name,
+							      bcl,
+							      &bcl_hwmon_chip_info,
+							      NULL);
+	if (IS_ERR(bcl->hwmon_dev)) {
+		dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
+			PTR_ERR(bcl->hwmon_dev));
+		return PTR_ERR(bcl->hwmon_dev);
+	}
+
+	dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
+		bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
+
+	return 0;
+}
+
+static const struct of_device_id bcl_match[] = {
+	{
+		.compatible = "qcom,bcl-v1",
+		.data = &pm7250b_data,
+	}, {
+		.compatible = "qcom,bcl-v2",
+		.data = &pm8350_data,
+	}, {
+		.compatible = "qcom,bcl-v3-bmx",
+		.data = &pm8550b_data,
+	}, {
+		.compatible = "qcom,bcl-v3-wb",
+		.data = &pmw5100_data,
+	}, {
+		.compatible = "qcom,bcl-v3-core",
+		.data = &pm8550_data,
+	}, {
+		.compatible = "qcom,bcl-v4-bmx",
+		.data = &pmih010_data,
+	}, {
+		.compatible = "qcom,bcl-v4-wb",
+		.data = &pmw6100_data,
+	}, {
+		.compatible = "qcom,bcl-v4-core",
+		.data = &pmh010_data,
+	}, {
+		.compatible = "qcom,bcl-v4-pmv010",
+		.data = &pmv010_data,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bcl_match);
+
+static struct platform_driver bcl_driver = {
+	.probe	= bcl_probe,
+	.driver	= {
+		.name		= BCL_DRIVER_NAME,
+		.of_match_table	= bcl_match,
+	},
+};
+
+MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
+MODULE_DESCRIPTION("QCOM BCL HWMON driver");
+module_platform_driver(bcl_driver);
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/qcom-bcl-hwmon.h b/drivers/hwmon/qcom-bcl-hwmon.h
new file mode 100644
index 000000000000..28a7154d9486
--- /dev/null
+++ b/drivers/hwmon/qcom-bcl-hwmon.h
@@ -0,0 +1,311 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef __QCOM_BCL_HWMON_H__
+#define __QCOM_BCL_HWMON_H__
+
+#define BCL_DRIVER_NAME			"qcom-bcl-hwmon"
+
+/* BCL common regmap offset */
+#define REVISION1			0x0
+#define REVISION2			0x1
+#define STATUS				0x8
+#define INT_RT_STS			0x10
+#define EN_CTL1				0x46
+
+/* BCL GEN1 regmap offsets */
+#define MODE_CTL1			0x41
+#define VADC_L0_THR			0x48
+#define VCMP_L1_THR			0x49
+#define IADC_H0_THR			0x4b
+#define IADC_H1_THR			0x4c
+#define VADC_CONV_REQ			0x72
+#define IADC_CONV_REQ			0x82
+#define VADC_DATA1			0x76
+#define IADC_DATA1			0x86
+
+/* BCL GEN3 regmap offsets */
+#define VCMP_CTL			0x44
+#define VCMP_L0_THR			0x47
+#define PARAM_1				0x0e
+#define IADC_H1_THR_GEN3		0x4d
+
+#define BCL_IN_INC_MV			25
+#define BCL_ALARM_POLLING_MS		50
+
+/**
+ * enum bcl_limit_alarm - BCL alarm threshold levels
+ * @LVL0: Level 0 alarm threshold (mapped to in_min_alarm or curr_max_alarm)
+ * @LVL1: Level 1 alarm threshold (mapped to in_lcrit_alarm or curr_crit_alarm)
+ * @ALARM_MAX: sentinel value
+ *
+ * Defines the three threshold levels for BCL monitoring. Each level corresponds
+ * to different severity of in or curr conditions.
+ */
+enum bcl_limit_alarm {
+	LVL0,
+	LVL1,
+
+	ALARM_MAX,
+};
+
+/**
+ * enum bcl_channel_type - BCL supported sensor channel type
+ * @IN: in (voltage) channel
+ * @CURR: curr (current) channel
+ * @CHANNEL_MAX: sentinel value
+ *
+ * Defines the supported channel types for bcl.
+ */
+enum bcl_channel_type {
+	IN,
+	CURR,
+
+	CHANNEL_MAX,
+};
+
+/**
+ * enum bcl_thresh_type - voltage or current threshold representation type
+ * @ADC: Raw ADC value representation
+ * @INDEX: Index-based voltage or current representation
+ *
+ * Specifies how voltage or current thresholds are stored and interpreted in
+ * registers. Some PMICs use raw ADC values while others use indexed values.
+ */
+enum bcl_thresh_type {
+	ADC,
+	INDEX,
+};
+
+/**
+ * enum bcl_fields - BCL register field identifiers
+ * @F_V_MAJOR: Major revision info field
+ * @F_V_MINOR: Minor revision info field
+ * @F_CTL_EN: Monitor enable control field
+ * @F_LVL0_ALARM: Level 0 alarm status field
+ * @F_LVL1_ALARM: Level 1 alarm status field
+ * @COMMON_FIELD_MAX: sentinel value for common fields
+ * @F_IN_MON_EN: voltage monitor enable control field
+ * @F_IN_L0_THR: voltage level 0 threshold field
+ * @F_IN_L1_THR: voltage level 1 threshold field
+ * @F_IN_INPUT_EN: voltage input enable control field
+ * @F_IN_INPUT: voltage input data field
+ * @F_CURR_MON_EN: current monitor enable control field
+ * @F_CURR_H0_THR: current level 0 threshold field
+ * @F_CURR_H1_THR: current level 1 threshold field
+ * @F_CURR_INPUT: current input data field
+ * @F_MAX_FIELDS: sentinel value
+ *
+ * Enumeration of all register fields used by the BCL driver for accessing
+ * registers through regmap fields.
+ */
+enum bcl_fields {
+	F_V_MAJOR,
+	F_V_MINOR,
+
+	F_CTL_EN,
+
+	/* common alarm for in and curr channel */
+	F_LVL0_ALARM,
+	F_LVL1_ALARM,
+
+	COMMON_FIELD_MAX,
+
+	F_IN_MON_EN = COMMON_FIELD_MAX,
+	F_IN_L0_THR,
+	F_IN_L1_THR,
+
+	F_IN_INPUT_EN,
+	F_IN_INPUT,
+
+	F_CURR_MON_EN,
+	F_CURR_H0_THR,
+	F_CURR_H1_THR,
+
+	F_CURR_INPUT,
+
+	F_MAX_FIELDS
+};
+
+#define ADD_BCL_HWMON_ALARM_MAPS(_type, lvl0_attr, lvl1_attr)	\
+								\
+static const u8 _type##_attr_to_lvl_map[] = {			\
+	[hwmon_##_type##_##lvl0_attr] = LVL0,			\
+	[hwmon_##_type##_##lvl1_attr] = LVL1,			\
+	[hwmon_##_type##_##lvl0_attr##_alarm] = LVL0,		\
+	[hwmon_##_type##_##lvl1_attr##_alarm] = LVL1,		\
+};								\
+								\
+static const u8 _type##_lvl_to_attr_map[ALARM_MAX] = {		\
+	[LVL0] = hwmon_##_type##_##lvl0_attr##_alarm,		\
+	[LVL1] = hwmon_##_type##_##lvl1_attr##_alarm,		\
+}
+
+/**
+ * struct bcl_channel_cfg - BCL channel related configuration
+ * @default_scale_nu:  Default scaling factor in nano unit
+ * @base: Base threshold value in milli unit
+ * @max: Maximum threshold value in milli unit
+ * @step: step increment value between two indexed threshold value
+ * @thresh_type: Array specifying threshold representation type for each alarm level
+ *
+ * Contains hardware-specific configuration and scaling parameters for different
+ * channel(voltage and current)..
+ */
+
+struct bcl_channel_cfg {
+	u32 default_scale_nu;
+	u32 base;
+	u32 max;
+	u32 step;
+	u8 thresh_type[ALARM_MAX];
+};
+
+/**
+ * struct bcl_desc - BCL device descriptor
+ * @reg_fields: Array of register field definitions for this device variant
+ * @channel: Each channel specific(voltage or current) configuration
+ * @num_reg_fields: Number of register field definitions for this device variant
+ * @data_field_bits_size: data read register bit size
+ * @thresh_field_bits_size: lsb bit size those are not included in threshold register
+ *
+ * Contains hardware-specific configuration and scaling parameters for different
+ * BCL variants. Each PMIC model may have different register layouts and
+ * conversion factors.
+ */
+
+struct bcl_desc {
+	const struct reg_field *reg_fields;
+	struct bcl_channel_cfg channel[CHANNEL_MAX];
+	u8 num_reg_fields;
+	u8 data_field_bits_size;
+	u8 thresh_field_bits_size;
+};
+
+struct bcl_device;
+
+/**
+ * struct bcl_alarm_data - BCL alarm interrupt data
+ * @irq: IRQ number assigned to this alarm
+ * @irq_enabled: Flag indicating if IRQ is enabled
+ * @type: Alarm level type (LVL0, or LVL1)
+ * @device: Pointer to parent BCL device structure
+ * @alarm_poll_work: delayed_work to poll alarm status
+ *
+ * Stores interrupt-related information for each alarm threshold level.
+ * Used by the IRQ handler to identify which alarm triggered.
+ */
+struct bcl_alarm_data {
+	int			irq;
+	bool			irq_enabled;
+	enum bcl_limit_alarm	type;
+	struct bcl_device	*device;
+	struct delayed_work	alarm_poll_work;
+};
+
+/**
+ * struct bcl_device - Main BCL device structure
+ * @dev: Pointer to device structure
+ * @regmap: Regmap for accessing PMIC registers
+ * @fields: Array of regmap fields for register access
+ * @bcl_alarms: Array of alarm data structures for each threshold level
+ * @lock: Mutex for protecting concurrent hardware access
+ * @in_mon_enabled: Flag indicating if voltage monitoring is enabled
+ * @curr_mon_enabled: Flag indicating if current monitoring is enabled
+ * @curr_thresholds: Current threshold values in milliamps from dt-binding(LVL0 and LVL1)
+ * @base: the BCL regbase offset from regmap
+ * @in_input_enabled: Flag indicating if voltage input reading is enabled
+ * @last_in_input: Last valid voltage input reading in millivolts
+ * @last_curr_input: Last valid current input reading in milliamps
+ * @desc: Pointer to device descriptor with hardware-specific parameters
+ * @hwmon_dev: Pointer to registered hwmon device
+ * @hwmon_name: Sanitized name for hwmon device
+ *
+ * Main driver structure containing all state and configuration for a BCL
+ * monitoring instance. Manages voltage and current monitoring, thresholds,
+ * and alarm handling.
+ */
+struct bcl_device {
+	struct device		*dev;
+	struct regmap		*regmap;
+	u16			base;
+	struct regmap_field	*fields[F_MAX_FIELDS];
+	struct bcl_alarm_data	bcl_alarms[ALARM_MAX];
+	struct mutex		lock;
+	u32			curr_thresholds[ALARM_MAX];
+	u32			last_in_input;
+	u32			last_curr_input;
+	bool			in_mon_enabled;
+	bool			curr_mon_enabled;
+	bool			in_input_enabled;
+	const struct bcl_desc	*desc;
+	struct device		*hwmon_dev;
+	char			*hwmon_name;
+};
+
+/**
+ * bcl_read_field_value - Read alarm status for a given level
+ * @bcl: BCL device structure
+ * @id: Index in bcl->fields[]
+ * @val: Pointer to store val
+ *
+ * Return: 0 on success or regmap error code
+ */
+static inline int bcl_read_field_value(const struct bcl_device *bcl, enum bcl_fields id, u32 *val)
+{
+	return regmap_field_read(bcl->fields[id], val);
+}
+
+/**
+ * bcl_field_enabled - Generic helper to check if a regmap field is enabled
+ * @bcl: BCL device structure
+ * @field: Index in bcl->fields[]
+ *
+ * Return: true if field is non-zero, false otherwise
+ */
+static inline bool bcl_field_enabled(const struct bcl_device *bcl, enum bcl_fields id)
+{
+	int ret;
+	u32 val = 0;
+
+	ret = regmap_field_read(bcl->fields[id], &val);
+	if (ret)
+		return false;
+
+	return !!val;
+}
+
+#define bcl_in_input_enabled(bcl)	bcl_field_enabled(bcl, F_IN_INPUT_EN)
+#define bcl_curr_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_CURR_MON_EN)
+#define bcl_in_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_IN_MON_EN)
+#define bcl_hw_is_enabled(bcl)		bcl_field_enabled(bcl, F_CTL_EN)
+
+/**
+ * bcl_enable_irq - Generic helper to enable alarm irq
+ * @alarm: BCL level alarm data
+ */
+static inline void bcl_enable_irq(struct bcl_alarm_data *alarm)
+{
+	if (alarm->irq_enabled)
+		return;
+	alarm->irq_enabled = true;
+	enable_irq(alarm->irq);
+	enable_irq_wake(alarm->irq);
+}
+
+/**
+ * bcl_disable_irq - Generic helper to disable alarm irq
+ * @alarm: BCL level alarm data
+ */
+static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)
+{
+	if (!alarm->irq_enabled)
+		return;
+	alarm->irq_enabled = false;
+	disable_irq_nosync(alarm->irq);
+	disable_irq_wake(alarm->irq);
+}
+
+#endif /* __QCOM_BCL_HWMON_H__ */

-- 
2.43.0

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Krzysztof Kozlowski]

On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> +	bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> +							      bcl->hwmon_name,
> +							      bcl,
> +							      &bcl_hwmon_chip_info,
> +							      NULL);
> +	if (IS_ERR(bcl->hwmon_dev)) {
> +		dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
> +			PTR_ERR(bcl->hwmon_dev));
> +		return PTR_ERR(bcl->hwmon_dev);
> +	}
> +
> +	dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
> +		bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcl_match[] = {
> +	{
> +		.compatible = "qcom,bcl-v1",
> +		.data = &pm7250b_data,

And that's the proof that your binding is just repeating downstream
bollocks pattern. Something like "v1" does not exist if you even here
claim this is not "v1", but pm7250.

> +	}, {
> +		.compatible = "qcom,bcl-v2",
> +		.data = &pm8350_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-bmx",
> +		.data = &pm8550b_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-wb",
> +		.data = &pmw5100_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-core",
> +		.data = &pm8550_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-bmx",
> +		.data = &pmih010_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-wb",
> +		.data = &pmw6100_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-core",
> +		.data = &pmh010_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-pmv010",
> +		.data = &pmv010_data,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, bcl_match);
> +
> +static struct platform_driver bcl_driver = {
> +	.probe	= bcl_probe,
> +	.driver	= {
> +		.name		= BCL_DRIVER_NAME,

No, just use name here.

> +		.of_match_table	= bcl_match,
> +	},
> +};
> +
> +MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("QCOM BCL HWMON driver");
> +module_platform_driver(bcl_driver);
> +MODULE_LICENSE("GPL");

...

> +
> +/**
> + * bcl_disable_irq - Generic helper to disable alarm irq
> + * @alarm: BCL level alarm data
> + */
> +static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)

Why do you write actual functions/code in the headers?

Please follow standard C rules, which mean that the C unit contains code
intended for execution. Headers are only for declarations.

> +{
> +	if (!alarm->irq_enabled)
> +		return;
> +	alarm->irq_enabled = false;
> +	disable_irq_nosync(alarm->irq);
> +	disable_irq_wake(alarm->irq);
> +}
> +
> +#endif /* __QCOM_BCL_HWMON_H__ */
> 
> -- 
> 2.43.0
> 

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Manaf Meethalavalappu Pallikunhi]

Hi Krzysztof,

On 2/6/2026 1:42 PM, Krzysztof Kozlowski wrote:
> On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> +	bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
>> +							      bcl->hwmon_name,
>> +							      bcl,
>> +							      &bcl_hwmon_chip_info,
>> +							      NULL);
>> +	if (IS_ERR(bcl->hwmon_dev)) {
>> +		dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
>> +			PTR_ERR(bcl->hwmon_dev));
>> +		return PTR_ERR(bcl->hwmon_dev);
>> +	}
>> +
>> +	dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
>> +		bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id bcl_match[] = {
>> +	{
>> +		.compatible = "qcom,bcl-v1",
>> +		.data = &pm7250b_data,
> And that's the proof that your binding is just repeating downstream
> bollocks pattern. Something like "v1" does not exist if you even here
> claim this is not "v1", but pm7250.
ACK, I will update to pmic names
>
>> +	}, {
>> +		.compatible = "qcom,bcl-v2",
>> +		.data = &pm8350_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v3-bmx",
>> +		.data = &pm8550b_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v3-wb",
>> +		.data = &pmw5100_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v3-core",
>> +		.data = &pm8550_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-bmx",
>> +		.data = &pmih010_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-wb",
>> +		.data = &pmw6100_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-core",
>> +		.data = &pmh010_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-pmv010",
>> +		.data = &pmv010_data,
>> +	},
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, bcl_match);
>> +
>> +static struct platform_driver bcl_driver = {
>> +	.probe	= bcl_probe,
>> +	.driver	= {
>> +		.name		= BCL_DRIVER_NAME,
> No, just use name here.
ACK
>
>> +		.of_match_table	= bcl_match,
>> +	},
>> +};
>> +
>> +MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
>> +MODULE_DESCRIPTION("QCOM BCL HWMON driver");
>> +module_platform_driver(bcl_driver);
>> +MODULE_LICENSE("GPL");
> ...
>
>> +
>> +/**
>> + * bcl_disable_irq - Generic helper to disable alarm irq
>> + * @alarm: BCL level alarm data
>> + */
>> +static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)
> Why do you write actual functions/code in the headers?
>
> Please follow standard C rules, which mean that the C unit contains code
> intended for execution. Headers are only for declarations.

ACK, I will move all codes from header to driver and remove header in 
next revision based on this and other's comment.

Thanks,

Manaf

>
>> +{
>> +	if (!alarm->irq_enabled)
>> +		return;
>> +	alarm->irq_enabled = false;
>> +	disable_irq_nosync(alarm->irq);
>> +	disable_irq_wake(alarm->irq);
>> +}
>> +
>> +#endif /* __QCOM_BCL_HWMON_H__ */
>>
>> -- 
>> 2.43.0
>>

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Konrad Dybcio]

On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
> monitor driver. The BCL peripheral is present in Qualcomm PMICs and
> provides real-time monitoring and protection against battery
> overcurrent and under voltage conditions.
> 
> The driver monitors:
> - Battery voltage with configurable low voltage thresholds
> - Battery current with configurable high current thresholds
> - Two limit alarm interrupts (max/min, critical)
> 
> The driver integrates with the Linux hwmon subsystem and provides
> standard hwmon attributes for monitoring battery conditions.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---

[...]

> +/* Interrupt names for each alarm level */
> +static const char * const bcl_int_names[ALARM_MAX] = {
> +	[LVL0] = "bcl-max-min",
> +	[LVL1] = "bcl-critical",
> +};
> +
> +static const char * const bcl_channel_label[CHANNEL_MAX] = {
> +	"BCL Voltage",
> +	"BCL Current",
> +};

Let's strip the BCL prefix

[...]

> +/**
> + * bcl_convert_raw_to_milliunit - Convert raw value to milli unit
> + * @desc: BCL device descriptor
> + * @raw_val: Raw ADC value from hardware
> + * @type: type of the channel, in or curr
> + * @field_width: bits size for data or threshold field
> + *
> + * Return: value in milli unit
> + */
> +static unsigned int bcl_convert_raw_to_milliunit(const struct bcl_desc *desc, int raw_val,

raw_val is an int here, a u32 when you retrieve it and a s64 in the math..

> +					enum bcl_channel_type type, u8 field_width)
> +{
> +	u32 def_scale = desc->channel[type].default_scale_nu;
> +	u32 lsb_weight = field_width > 8 ? 1 : 1 << field_width;
> +	u32 scaling_factor = def_scale * lsb_weight;

Would this be equivalent?

if (field_width > 8)
	def_scale <<= field_width;

[...]

> +static unsigned int bcl_get_version_major(const struct bcl_device *bcl)
> +{
> +	u32 raw_val = 0;
> +
> +	bcl_read_field_value(bcl, F_V_MAJOR, &raw_val);
> +
> +	return raw_val;
> +}
> +
> +static unsigned int bcl_get_version_minor(const struct bcl_device *bcl)
> +{
> +	u32 raw_val = 0;
> +
> +	bcl_read_field_value(bcl, F_V_MINOR, &raw_val);
> +
> +	return raw_val;
> +}

Do we really need so many read-1-value functions?

> +static void bcl_hwmon_notify_event(struct bcl_device *bcl, enum bcl_limit_alarm alarm)
> +{
> +	if (bcl->in_mon_enabled)
> +		hwmon_notify_event(bcl->hwmon_dev, hwmon_in,
> +				in_lvl_to_attr_map[alarm], 0);
> +	if (bcl->curr_mon_enabled)
> +		hwmon_notify_event(bcl->hwmon_dev, hwmon_curr,
> +				curr_lvl_to_attr_map[alarm], 0);
> +}
> +
> +static void bcl_alarm_enable_poll(struct work_struct *work)
> +{
> +	struct bcl_alarm_data *alarm = container_of(work, struct bcl_alarm_data,
> +							 alarm_poll_work.work);
> +	struct bcl_device *bcl = alarm->device;
> +	long status;
> +
> +	guard(mutex)(&bcl->lock);
> +
> +	if (bcl_read_alarm_status(bcl, alarm->type, &status))
> +		goto re_schedule;

Do we ever expect regmap_read to *actually* fail?

[...]

> +static int bcl_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> +			   u32 attr, int channel, long val)
> +{
> +	struct bcl_device *bcl = dev_get_drvdata(dev);
> +	int ret = -EOPNOTSUPP;
> +
> +	guard(mutex)(&bcl->lock);
> +
> +	switch (type) {
> +	case hwmon_in:
> +		switch (attr) {
> +		case hwmon_in_min:
> +		case hwmon_in_lcrit:
> +			ret = bcl_in_thresh_write(bcl, val, in_attr_to_lvl_map[attr]);
> +			break;
> +		default:
> +			ret = -EOPNOTSUPP;

Please don't "ret = ...;break;, just return directly, also in the function
below

[...]

> +static int bcl_curr_thresh_update(struct bcl_device *bcl)
> +{
> +	int ret, i;
> +
> +	if (!bcl->curr_thresholds[0])
> +		return 0;
> +
> +	for (i = 0; i < ALARM_MAX; i++) {
> +		ret = bcl_curr_thresh_write(bcl, bcl->curr_thresholds[i], i);
> +		if (ret < 0)
> +			return ret;

This too, fails if a regmap_write() fails and leaves other registers
unconfigured if that happens for $reasons

[...]

> +static int bcl_get_device_property_data(struct platform_device *pdev,
> +				   struct bcl_device *bcl)
> +{
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +	u32 reg;
> +
> +	ret = device_property_read_u32(dev, "reg", &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	bcl->base = reg;
> +
> +	device_property_read_u32_array(dev, "overcurrent-thresholds-milliamp",
> +				       bcl->curr_thresholds, 2);
> +	return 0;

If you don't expect this to grow, just inline it in .probe

[...]

> +	if (!bcl_hw_is_enabled(bcl))
> +		return -ENODEV;

Please make this print a meaningful error - also, should we expect this to
ever happen, or would it mean that the bootloader (or something) hasn't
configured BCL prior to Linux booting?

[...]


> + * enum bcl_channel_type - BCL supported sensor channel type
> + * @IN: in (voltage) channel
> + * @CURR: curr (current) channel
> + * @CHANNEL_MAX: sentinel value
> + *
> + * Defines the supported channel types for bcl.
> + */
> +enum bcl_channel_type {
> +	IN,
> +	CURR,

The enum defines could use a prefix, say CHANNEL_CURR

> +
> +	CHANNEL_MAX,
> +};
> +
> +/**
> + * enum bcl_thresh_type - voltage or current threshold representation type
> + * @ADC: Raw ADC value representation
> + * @INDEX: Index-based voltage or current representation
> + *
> + * Specifies how voltage or current thresholds are stored and interpreted in
> + * registers. Some PMICs use raw ADC values while others use indexed values.
> + */
> +enum bcl_thresh_type {
> +	ADC,
> +	INDEX,

Same here, THRESH_TYPE_ADC

[...]

> +/**
> + * bcl_read_field_value - Read alarm status for a given level
> + * @bcl: BCL device structure
> + * @id: Index in bcl->fields[]
> + * @val: Pointer to store val
> + *
> + * Return: 0 on success or regmap error code
> + */
> +static inline int bcl_read_field_value(const struct bcl_device *bcl, enum bcl_fields id, u32 *val)
> +{
> +	return regmap_field_read(bcl->fields[id], val);
> +}

This produces more characters than it would to inline the function

Now, that doesn't mean it can't be like that, but it's certainly curious

Konrad

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Krzysztof Kozlowski]

On 06/02/2026 10:27, Konrad Dybcio wrote:
> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
> 
>> +static unsigned int bcl_get_version_major(const struct bcl_device *bcl)
>> +{
>> +	u32 raw_val = 0;
>> +
>> +	bcl_read_field_value(bcl, F_V_MAJOR, &raw_val);
>> +
>> +	return raw_val;
>> +}
>> +
>> +static unsigned int bcl_get_version_minor(const struct bcl_device *bcl)
>> +{
>> +	u32 raw_val = 0;
>> +
>> +	bcl_read_field_value(bcl, F_V_MINOR, &raw_val);
>> +
>> +	return raw_val;
>> +}
> 
> Do we really need so many read-1-value functions?

Yes, because we need multiple layers of indirections and multiple
abstract layers, so the code will be difficult to understand. This gives
us job security. Also, more lines of code justifies our spent time.
That's why this should also include kerneldoc of at least 10 lines.

Best regards,
Krzysztof

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,


On 2/6/2026 2:57 PM, Konrad Dybcio wrote:
> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
>> monitor driver. The BCL peripheral is present in Qualcomm PMICs and
>> provides real-time monitoring and protection against battery
>> overcurrent and under voltage conditions.
>>
>> The driver monitors:
>> - Battery voltage with configurable low voltage thresholds
>> - Battery current with configurable high current thresholds
>> - Two limit alarm interrupts (max/min, critical)
>>
>> The driver integrates with the Linux hwmon subsystem and provides
>> standard hwmon attributes for monitoring battery conditions.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
> [...]
>
>> +/* Interrupt names for each alarm level */
>> +static const char * const bcl_int_names[ALARM_MAX] = {
>> +	[LVL0] = "bcl-max-min",
>> +	[LVL1] = "bcl-critical",
>> +};
>> +
>> +static const char * const bcl_channel_label[CHANNEL_MAX] = {
>> +	"BCL Voltage",
>> +	"BCL Current",
>> +};
> Let's strip the BCL prefix
Ack
>
> [...]
>
>> +/**
>> + * bcl_convert_raw_to_milliunit - Convert raw value to milli unit
>> + * @desc: BCL device descriptor
>> + * @raw_val: Raw ADC value from hardware
>> + * @type: type of the channel, in or curr
>> + * @field_width: bits size for data or threshold field
>> + *
>> + * Return: value in milli unit
>> + */
>> +static unsigned int bcl_convert_raw_to_milliunit(const struct bcl_desc *desc, int raw_val,
> raw_val is an int here, a u32 when you retrieve it and a s64 in the math..
Will check and update in next revision
>
>> +					enum bcl_channel_type type, u8 field_width)
>> +{
>> +	u32 def_scale = desc->channel[type].default_scale_nu;
>> +	u32 lsb_weight = field_width > 8 ? 1 : 1 << field_width;
>> +	u32 scaling_factor = def_scale * lsb_weight;
> Would this be equivalent?
>
> if (field_width > 8)
> 	def_scale <<= field_width;
Ack
>
> [...]
>
>> +static unsigned int bcl_get_version_major(const struct bcl_device *bcl)
>> +{
>> +	u32 raw_val = 0;
>> +
>> +	bcl_read_field_value(bcl, F_V_MAJOR, &raw_val);
>> +
>> +	return raw_val;
>> +}
>> +
>> +static unsigned int bcl_get_version_minor(const struct bcl_device *bcl)
>> +{
>> +	u32 raw_val = 0;
>> +
>> +	bcl_read_field_value(bcl, F_V_MINOR, &raw_val);
>> +
>> +	return raw_val;
>> +}
> Do we really need so many read-1-value functions?
Ack, will remove those functions
>
>> +static void bcl_hwmon_notify_event(struct bcl_device *bcl, enum bcl_limit_alarm alarm)
>> +{
>> +	if (bcl->in_mon_enabled)
>> +		hwmon_notify_event(bcl->hwmon_dev, hwmon_in,
>> +				in_lvl_to_attr_map[alarm], 0);
>> +	if (bcl->curr_mon_enabled)
>> +		hwmon_notify_event(bcl->hwmon_dev, hwmon_curr,
>> +				curr_lvl_to_attr_map[alarm], 0);
>> +}
>> +
>> +static void bcl_alarm_enable_poll(struct work_struct *work)
>> +{
>> +	struct bcl_alarm_data *alarm = container_of(work, struct bcl_alarm_data,
>> +							 alarm_poll_work.work);
>> +	struct bcl_device *bcl = alarm->device;
>> +	long status;
>> +
>> +	guard(mutex)(&bcl->lock);
>> +
>> +	if (bcl_read_alarm_status(bcl, alarm->type, &status))
>> +		goto re_schedule;
> Do we ever expect regmap_read to *actually* fail?
Since regmap_read API itself is saying, it can fail, added check. Will 
remove it.
>
> [...]
>
>> +static int bcl_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>> +			   u32 attr, int channel, long val)
>> +{
>> +	struct bcl_device *bcl = dev_get_drvdata(dev);
>> +	int ret = -EOPNOTSUPP;
>> +
>> +	guard(mutex)(&bcl->lock);
>> +
>> +	switch (type) {
>> +	case hwmon_in:
>> +		switch (attr) {
>> +		case hwmon_in_min:
>> +		case hwmon_in_lcrit:
>> +			ret = bcl_in_thresh_write(bcl, val, in_attr_to_lvl_map[attr]);
>> +			break;
>> +		default:
>> +			ret = -EOPNOTSUPP;
> Please don't "ret = ...;break;, just return directly, also in the function
> below
Ack
>
> [...]
>
>> +static int bcl_curr_thresh_update(struct bcl_device *bcl)
>> +{
>> +	int ret, i;
>> +
>> +	if (!bcl->curr_thresholds[0])
>> +		return 0;
>> +
>> +	for (i = 0; i < ALARM_MAX; i++) {
>> +		ret = bcl_curr_thresh_write(bcl, bcl->curr_thresholds[i], i);
>> +		if (ret < 0)
>> +			return ret;
> This too, fails if a regmap_write() fails and leaves other registers
> unconfigured if that happens for $reasons
Ack
>
> [...]
>
>> +static int bcl_get_device_property_data(struct platform_device *pdev,
>> +				   struct bcl_device *bcl)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	int ret;
>> +	u32 reg;
>> +
>> +	ret = device_property_read_u32(dev, "reg", &reg);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	bcl->base = reg;
>> +
>> +	device_property_read_u32_array(dev, "overcurrent-thresholds-milliamp",
>> +				       bcl->curr_thresholds, 2);
>> +	return 0;
> If you don't expect this to grow, just inline it in .probe
For now, there is no any other requirement, will move it to probe
>
> [...]
>
>> +	if (!bcl_hw_is_enabled(bcl))
>> +		return -ENODEV;
> Please make this print a meaningful error - also, should we expect this to
Ack
> ever happen, or would it mean that the bootloader (or something) hasn't
> configured BCL prior to Linux booting?

Most of the cases, it will be enabled in firmware. But there is scenario 
for some variant, firmware will disable it at runtime

based on underlying battery sensing element due to hw issue. So We 
wanted to enable driver only if peripheral is enabled.

>
> [...]
>
>
>> + * enum bcl_channel_type - BCL supported sensor channel type
>> + * @IN: in (voltage) channel
>> + * @CURR: curr (current) channel
>> + * @CHANNEL_MAX: sentinel value
>> + *
>> + * Defines the supported channel types for bcl.
>> + */
>> +enum bcl_channel_type {
>> +	IN,
>> +	CURR,
> The enum defines could use a prefix, say CHANNEL_CURR
Ack
>
>> +
>> +	CHANNEL_MAX,
>> +};
>> +
>> +/**
>> + * enum bcl_thresh_type - voltage or current threshold representation type
>> + * @ADC: Raw ADC value representation
>> + * @INDEX: Index-based voltage or current representation
>> + *
>> + * Specifies how voltage or current thresholds are stored and interpreted in
>> + * registers. Some PMICs use raw ADC values while others use indexed values.
>> + */
>> +enum bcl_thresh_type {
>> +	ADC,
>> +	INDEX,
> Same here, THRESH_TYPE_ADC
Ack
>
> [...]
>
>> +/**
>> + * bcl_read_field_value - Read alarm status for a given level
>> + * @bcl: BCL device structure
>> + * @id: Index in bcl->fields[]
>> + * @val: Pointer to store val
>> + *
>> + * Return: 0 on success or regmap error code
>> + */
>> +static inline int bcl_read_field_value(const struct bcl_device *bcl, enum bcl_fields id, u32 *val)
>> +{
>> +	return regmap_field_read(bcl->fields[id], val);
>> +}
> This produces more characters than it would to inline the function
>
> Now, that doesn't mean it can't be like that, but it's certainly curious

Ack, will remove it in v2

Thanks,

Manaf

>
> Konrad

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Konrad Dybcio]

On 2/13/26 10:42 AM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Konrad,
> 
> 
> On 2/6/2026 2:57 PM, Konrad Dybcio wrote:
>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
>>> monitor driver. The BCL peripheral is present in Qualcomm PMICs and
>>> provides real-time monitoring and protection against battery
>>> overcurrent and under voltage conditions.
>>>
>>> The driver monitors:
>>> - Battery voltage with configurable low voltage thresholds
>>> - Battery current with configurable high current thresholds
>>> - Two limit alarm interrupts (max/min, critical)
>>>
>>> The driver integrates with the Linux hwmon subsystem and provides
>>> standard hwmon attributes for monitoring battery conditions.
>>>
>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>> ---

[...]

>>> +                    enum bcl_channel_type type, u8 field_width)
>>> +{
>>> +    u32 def_scale = desc->channel[type].default_scale_nu;
>>> +    u32 lsb_weight = field_width > 8 ? 1 : 1 << field_width;
>>> +    u32 scaling_factor = def_scale * lsb_weight;
>> Would this be equivalent?
>>
>> if (field_width > 8)
>>     def_scale <<= field_width;

Actually reading it again, that'd be "if (field_width *<=* 8)"

and the div_s64 could become a mult_frac() if you take care of the types

I would then read that as "some PMICs have a higher resolution ADC
but with the same range" - would that be right?

Konrad

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Bjorn Andersson]

On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> diff --git a/drivers/hwmon/qcom-bcl-hwmon.c b/drivers/hwmon/qcom-bcl-hwmon.c
> new file mode 100644
> index 000000000000..a7e3b865de5c
> --- /dev/null
> +++ b/drivers/hwmon/qcom-bcl-hwmon.c
> @@ -0,0 +1,982 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm pmic BCL hardware driver for battery overcurrent and
> + * battery or system under voltage monitor
> + *
> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.

That's the wrong statement.

> + */
> +
[..]
> +static void bcl_hw_channel_mon_init(struct bcl_device *bcl)
> +{
> +	bcl->in_mon_enabled = bcl_in_monitor_enabled(bcl);
> +	bcl->in_input_enabled = bcl_in_input_enabled(bcl);
> +	bcl->curr_mon_enabled = bcl_curr_monitor_enabled(bcl);
> +}
> +
> +static int bcl_alarm_irq_init(struct platform_device *pdev,
> +			      struct bcl_device *bcl)
> +{
> +	int ret = 0, irq_num = 0, i = 0;

First use of these three variables are assignments, no need to
zero-initialize them here.

> +	struct bcl_alarm_data *alarm;
> +
> +	for (i = LVL0; i < ALARM_MAX; i++) {

I would prefer ARRAY_SIZE(bcl->bcl_alarms) over ALARM_MAX.

> +		alarm = &bcl->bcl_alarms[i];
> +		alarm->type = i;
> +		alarm->device = bcl;
> +
> +		ret = devm_delayed_work_autocancel(bcl->dev, &alarm->alarm_poll_work,
> +					   bcl_alarm_enable_poll);
> +		if (ret)
> +			return ret;
> +
> +		irq_num = platform_get_irq_byname(pdev, bcl_int_names[i]);
> +		if (irq_num <= 0)
> +			continue;
> +
> +		ret = devm_request_threaded_irq(&pdev->dev, irq_num, NULL,
> +						bcl_handle_alarm, IRQF_ONESHOT,
> +						bcl_int_names[i], alarm);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Error requesting irq(%s).err:%d\n",
> +				bcl_int_names[i], ret);
> +			return ret;
> +		}
> +		enable_irq_wake(alarm->irq);
> +		alarm->irq_enabled = true;
> +		alarm->irq = irq_num;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcl_regmap_field_init(struct device *dev, struct bcl_device *bcl,
> +				 const struct bcl_desc *data)
> +{
> +	int i;
> +	struct reg_field fields[F_MAX_FIELDS];
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(common_reg_fields) != COMMON_FIELD_MAX);
> +
> +	for (i = 0; i < data->num_reg_fields; i++) {
> +		if (i < COMMON_FIELD_MAX)
> +			fields[i] = common_reg_fields[i];
> +		else
> +			fields[i] = data->reg_fields[i];
> +
> +		/* Need to adjust BCL base from regmap dynamically */
> +		fields[i].reg += bcl->base;
> +	}
> +
> +	return devm_regmap_field_bulk_alloc(dev, bcl->regmap, bcl->fields,
> +					   fields, data->num_reg_fields);
> +}
> +
> +static int bcl_get_device_property_data(struct platform_device *pdev,
> +				   struct bcl_device *bcl)
> +{
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +	u32 reg;
> +
> +	ret = device_property_read_u32(dev, "reg", &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	bcl->base = reg;
> +
> +	device_property_read_u32_array(dev, "overcurrent-thresholds-milliamp",
> +				       bcl->curr_thresholds, 2);
> +	return 0;
> +}
> +
> +static int bcl_probe(struct platform_device *pdev)
> +{
> +	struct bcl_device *bcl;
> +	int ret;
> +
> +	bcl = devm_kzalloc(&pdev->dev, sizeof(*bcl), GFP_KERNEL);
> +	if (!bcl)
> +		return -ENOMEM;
> +
> +	bcl->dev = &pdev->dev;
> +	bcl->desc = device_get_match_data(&pdev->dev);
> +	if (!bcl->desc)
> +		return -EINVAL;
> +
> +	ret = devm_mutex_init(bcl->dev, &bcl->lock);
> +	if (ret)
> +		return ret;
> +
> +	bcl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!bcl->regmap) {
> +		dev_err(&pdev->dev, "Couldn't get parent's regmap\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = bcl_get_device_property_data(pdev, bcl);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = bcl_regmap_field_init(bcl->dev, bcl, bcl->desc);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Unable to allocate regmap fields, err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	if (!bcl_hw_is_enabled(bcl))
> +		return -ENODEV;
> +
> +	ret = bcl_curr_thresh_update(bcl);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = bcl_alarm_irq_init(pdev, bcl);
> +	if (ret < 0)
> +		return ret;
> +
> +	bcl_hw_channel_mon_init(bcl);
> +
> +	dev_set_drvdata(&pdev->dev, bcl);
> +
> +	bcl->hwmon_name = devm_hwmon_sanitize_name(&pdev->dev,
> +						   dev_name(bcl->dev));
> +	if (IS_ERR(bcl->hwmon_name)) {
> +		dev_err(&pdev->dev, "Failed to sanitize hwmon name\n");

Afaict, devm_hwmon_sanitize_name() can only return -ENOMEM, which
already printed an error.

> +		return PTR_ERR(bcl->hwmon_name);
> +	}
> +
> +	bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> +							      bcl->hwmon_name,
> +							      bcl,
> +							      &bcl_hwmon_chip_info,
> +							      NULL);
> +	if (IS_ERR(bcl->hwmon_dev)) {
> +		dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
> +			PTR_ERR(bcl->hwmon_dev));
> +		return PTR_ERR(bcl->hwmon_dev);
> +	}
> +
> +	dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
> +		bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcl_match[] = {
> +	{
> +		.compatible = "qcom,bcl-v1",
> +		.data = &pm7250b_data,

Why generic compatibles but pmic-specific data structures? If anything
I'd expect tthe other way around...

> +	}, {
> +		.compatible = "qcom,bcl-v2",
> +		.data = &pm8350_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-bmx",
> +		.data = &pm8550b_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-wb",
> +		.data = &pmw5100_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-core",
> +		.data = &pm8550_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-bmx",
> +		.data = &pmih010_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-wb",
> +		.data = &pmw6100_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-core",
> +		.data = &pmh010_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-pmv010",
> +		.data = &pmv010_data,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, bcl_match);
> +
> +static struct platform_driver bcl_driver = {
> +	.probe	= bcl_probe,
> +	.driver	= {
> +		.name		= BCL_DRIVER_NAME,
> +		.of_match_table	= bcl_match,
> +	},
> +};
> +
> +MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("QCOM BCL HWMON driver");
> +module_platform_driver(bcl_driver);

This relates to the bcl_driver declaration, not module properties. So
move it there.

> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/qcom-bcl-hwmon.h b/drivers/hwmon/qcom-bcl-hwmon.h

Why is there a header file, is this going to be accessed by some other
driver? It mostly contain driver-internal thing, and some helpers that
won't be useful outside of the driver.

> new file mode 100644
> index 000000000000..28a7154d9486
> --- /dev/null
> +++ b/drivers/hwmon/qcom-bcl-hwmon.h
> @@ -0,0 +1,311 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.

Please fix this one as well. (Or...drop the file)

> + */
> +
> +#ifndef __QCOM_BCL_HWMON_H__
> +#define __QCOM_BCL_HWMON_H__
> +
> +#define BCL_DRIVER_NAME			"qcom-bcl-hwmon"

This belong in the driver...but frankly, you can just put the string
directly in bcl_driver.driver.name, no need to have a define for it...

[..]
> +/**
> + * bcl_field_enabled - Generic helper to check if a regmap field is enabled
> + * @bcl: BCL device structure
> + * @field: Index in bcl->fields[]
> + *
> + * Return: true if field is non-zero, false otherwise
> + */
> +static inline bool bcl_field_enabled(const struct bcl_device *bcl, enum bcl_fields id)
> +{
> +	int ret;
> +	u32 val = 0;
> +
> +	ret = regmap_field_read(bcl->fields[id], &val);
> +	if (ret)
> +		return false;
> +
> +	return !!val;
> +}
> +
> +#define bcl_in_input_enabled(bcl)	bcl_field_enabled(bcl, F_IN_INPUT_EN)
> +#define bcl_curr_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_CURR_MON_EN)
> +#define bcl_in_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_IN_MON_EN)
> +#define bcl_hw_is_enabled(bcl)		bcl_field_enabled(bcl, F_CTL_EN)

This whole thing is only used in bcl_hw_channel_mon_init(), just put the
code in bcl_hw_channel_mon_init().


You have a few other regmap_field_*() calls in the driver, I would
suggest that you just call that directly for these cases as well.

> +
> +/**
> + * bcl_enable_irq - Generic helper to enable alarm irq
> + * @alarm: BCL level alarm data
> + */
> +static inline void bcl_enable_irq(struct bcl_alarm_data *alarm)
> +{
> +	if (alarm->irq_enabled)
> +		return;

This can't happen, but because you separated this to a helper function
it's not obvious

I'd suggest that you inline the remaining 3 lines in the one place where
this function is called.

> +	alarm->irq_enabled = true;
> +	enable_irq(alarm->irq);
> +	enable_irq_wake(alarm->irq);
> +}
> +
> +/**
> + * bcl_disable_irq - Generic helper to disable alarm irq
> + * @alarm: BCL level alarm data
> + */
> +static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)
> +{
> +	if (!alarm->irq_enabled)
> +		return;

This is tricker, because there's a window between
devm_request_threaded_irq() and the assignment of irq_enabled, where the
interrupt function might execute and the attempt to bcl_disable_irq()
will face irq_enabled == 0.

But I don't think that's intentional...

I think this too would be better to just inline in the one place its'
being called.

Regards,
Bjorn

> +	alarm->irq_enabled = false;
> +	disable_irq_nosync(alarm->irq);
> +	disable_irq_wake(alarm->irq);
> +}
> +
> +#endif /* __QCOM_BCL_HWMON_H__ */
> 
> -- 
> 2.43.0
> 

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Manaf Meethalavalappu Pallikunhi]

Hi Bjorn,

On 2/6/2026 6:54 PM, Bjorn Andersson wrote:
> On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> diff --git a/drivers/hwmon/qcom-bcl-hwmon.c b/drivers/hwmon/qcom-bcl-hwmon.c
>> new file mode 100644
>> index 000000000000..a7e3b865de5c
>> --- /dev/null
>> +++ b/drivers/hwmon/qcom-bcl-hwmon.c
>> @@ -0,0 +1,982 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Qualcomm pmic BCL hardware driver for battery overcurrent and
>> + * battery or system under voltage monitor
>> + *
>> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
> That's the wrong statement.
My bad, I copied old copyrights, will update
>
>> + */
>> +
> [..]
>> +static void bcl_hw_channel_mon_init(struct bcl_device *bcl)
>> +{
>> +	bcl->in_mon_enabled = bcl_in_monitor_enabled(bcl);
>> +	bcl->in_input_enabled = bcl_in_input_enabled(bcl);
>> +	bcl->curr_mon_enabled = bcl_curr_monitor_enabled(bcl);
>> +}
>> +
>> +static int bcl_alarm_irq_init(struct platform_device *pdev,
>> +			      struct bcl_device *bcl)
>> +{
>> +	int ret = 0, irq_num = 0, i = 0;
> First use of these three variables are assignments, no need to
> zero-initialize them here.
>
>> +	struct bcl_alarm_data *alarm;
>> +
>> +	for (i = LVL0; i < ALARM_MAX; i++) {
> I would prefer ARRAY_SIZE(bcl->bcl_alarms) over ALARM_MAX.
Ack
>
>> +		alarm = &bcl->bcl_alarms[i];
>> +		alarm->type = i;
>> +		alarm->device = bcl;
>> +
>> +		ret = devm_delayed_work_autocancel(bcl->dev, &alarm->alarm_poll_work,
>> +					   bcl_alarm_enable_poll);
>> +		if (ret)
>> +			return ret;
>> +
>> +		irq_num = platform_get_irq_byname(pdev, bcl_int_names[i]);
>> +		if (irq_num <= 0)
>> +			continue;
>> +
>> +		ret = devm_request_threaded_irq(&pdev->dev, irq_num, NULL,
>> +						bcl_handle_alarm, IRQF_ONESHOT,
>> +						bcl_int_names[i], alarm);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "Error requesting irq(%s).err:%d\n",
>> +				bcl_int_names[i], ret);
>> +			return ret;
>> +		}
>> +		enable_irq_wake(alarm->irq);
>> +		alarm->irq_enabled = true;
>> +		alarm->irq = irq_num;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int bcl_regmap_field_init(struct device *dev, struct bcl_device *bcl,
>> +				 const struct bcl_desc *data)
>> +{
>> +	int i;
>> +	struct reg_field fields[F_MAX_FIELDS];
>> +
>> +	BUILD_BUG_ON(ARRAY_SIZE(common_reg_fields) != COMMON_FIELD_MAX);
>> +
>> +	for (i = 0; i < data->num_reg_fields; i++) {
>> +		if (i < COMMON_FIELD_MAX)
>> +			fields[i] = common_reg_fields[i];
>> +		else
>> +			fields[i] = data->reg_fields[i];
>> +
>> +		/* Need to adjust BCL base from regmap dynamically */
>> +		fields[i].reg += bcl->base;
>> +	}
>> +
>> +	return devm_regmap_field_bulk_alloc(dev, bcl->regmap, bcl->fields,
>> +					   fields, data->num_reg_fields);
>> +}
>> +
>> +static int bcl_get_device_property_data(struct platform_device *pdev,
>> +				   struct bcl_device *bcl)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	int ret;
>> +	u32 reg;
>> +
>> +	ret = device_property_read_u32(dev, "reg", &reg);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	bcl->base = reg;
>> +
>> +	device_property_read_u32_array(dev, "overcurrent-thresholds-milliamp",
>> +				       bcl->curr_thresholds, 2);
>> +	return 0;
>> +}
>> +
>> +static int bcl_probe(struct platform_device *pdev)
>> +{
>> +	struct bcl_device *bcl;
>> +	int ret;
>> +
>> +	bcl = devm_kzalloc(&pdev->dev, sizeof(*bcl), GFP_KERNEL);
>> +	if (!bcl)
>> +		return -ENOMEM;
>> +
>> +	bcl->dev = &pdev->dev;
>> +	bcl->desc = device_get_match_data(&pdev->dev);
>> +	if (!bcl->desc)
>> +		return -EINVAL;
>> +
>> +	ret = devm_mutex_init(bcl->dev, &bcl->lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	bcl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> +	if (!bcl->regmap) {
>> +		dev_err(&pdev->dev, "Couldn't get parent's regmap\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = bcl_get_device_property_data(pdev, bcl);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = bcl_regmap_field_init(bcl->dev, bcl, bcl->desc);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "Unable to allocate regmap fields, err:%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	if (!bcl_hw_is_enabled(bcl))
>> +		return -ENODEV;
>> +
>> +	ret = bcl_curr_thresh_update(bcl);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = bcl_alarm_irq_init(pdev, bcl);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	bcl_hw_channel_mon_init(bcl);
>> +
>> +	dev_set_drvdata(&pdev->dev, bcl);
>> +
>> +	bcl->hwmon_name = devm_hwmon_sanitize_name(&pdev->dev,
>> +						   dev_name(bcl->dev));
>> +	if (IS_ERR(bcl->hwmon_name)) {
>> +		dev_err(&pdev->dev, "Failed to sanitize hwmon name\n");
> Afaict, devm_hwmon_sanitize_name() can only return -ENOMEM, which
> already printed an error.
Ack
>
>> +		return PTR_ERR(bcl->hwmon_name);
>> +	}
>> +
>> +	bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
>> +							      bcl->hwmon_name,
>> +							      bcl,
>> +							      &bcl_hwmon_chip_info,
>> +							      NULL);
>> +	if (IS_ERR(bcl->hwmon_dev)) {
>> +		dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
>> +			PTR_ERR(bcl->hwmon_dev));
>> +		return PTR_ERR(bcl->hwmon_dev);
>> +	}
>> +
>> +	dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
>> +		bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id bcl_match[] = {
>> +	{
>> +		.compatible = "qcom,bcl-v1",
>> +		.data = &pm7250b_data,
> Why generic compatibles but pmic-specific data structures? If anything
> I'd expect tthe other way around...
Ack, what I thought is, there can be multiple pmics which share same 
data structure in same generation. In that case no need to add extra binding
config,  Just use generic compatibles. By reading  all these comments,  
it shouldn't be like this. I will remove generic compatibles.
>
>> +	}, {
>> +		.compatible = "qcom,bcl-v2",
>> +		.data = &pm8350_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v3-bmx",
>> +		.data = &pm8550b_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v3-wb",
>> +		.data = &pmw5100_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v3-core",
>> +		.data = &pm8550_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-bmx",
>> +		.data = &pmih010_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-wb",
>> +		.data = &pmw6100_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-core",
>> +		.data = &pmh010_data,
>> +	}, {
>> +		.compatible = "qcom,bcl-v4-pmv010",
>> +		.data = &pmv010_data,
>> +	},
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, bcl_match);
>> +
>> +static struct platform_driver bcl_driver = {
>> +	.probe	= bcl_probe,
>> +	.driver	= {
>> +		.name		= BCL_DRIVER_NAME,
>> +		.of_match_table	= bcl_match,
>> +	},
>> +};
>> +
>> +MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
>> +MODULE_DESCRIPTION("QCOM BCL HWMON driver");
>> +module_platform_driver(bcl_driver);
> This relates to the bcl_driver declaration, not module properties. So
> move it there.
Ack
>
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/hwmon/qcom-bcl-hwmon.h b/drivers/hwmon/qcom-bcl-hwmon.h
> Why is there a header file, is this going to be accessed by some other
> driver? It mostly contain driver-internal thing, and some helpers that
> won't be useful outside of the driver.
Ack, will move all changes in to driver file
>
>> new file mode 100644
>> index 000000000000..28a7154d9486
>> --- /dev/null
>> +++ b/drivers/hwmon/qcom-bcl-hwmon.h
>> @@ -0,0 +1,311 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
> Please fix this one as well. (Or...drop the file)
>
>> + */
>> +
>> +#ifndef __QCOM_BCL_HWMON_H__
>> +#define __QCOM_BCL_HWMON_H__
>> +
>> +#define BCL_DRIVER_NAME			"qcom-bcl-hwmon"
> This belong in the driver...but frankly, you can just put the string
> directly in bcl_driver.driver.name, no need to have a define for it...
Ack
>
> [..]
>> +/**
>> + * bcl_field_enabled - Generic helper to check if a regmap field is enabled
>> + * @bcl: BCL device structure
>> + * @field: Index in bcl->fields[]
>> + *
>> + * Return: true if field is non-zero, false otherwise
>> + */
>> +static inline bool bcl_field_enabled(const struct bcl_device *bcl, enum bcl_fields id)
>> +{
>> +	int ret;
>> +	u32 val = 0;
>> +
>> +	ret = regmap_field_read(bcl->fields[id], &val);
>> +	if (ret)
>> +		return false;
>> +
>> +	return !!val;
>> +}
>> +
>> +#define bcl_in_input_enabled(bcl)	bcl_field_enabled(bcl, F_IN_INPUT_EN)
>> +#define bcl_curr_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_CURR_MON_EN)
>> +#define bcl_in_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_IN_MON_EN)
>> +#define bcl_hw_is_enabled(bcl)		bcl_field_enabled(bcl, F_CTL_EN)
> This whole thing is only used in bcl_hw_channel_mon_init(), just put the
> code in bcl_hw_channel_mon_init().
>
>
> You have a few other regmap_field_*() calls in the driver, I would
> suggest that you just call that directly for these cases as well.
Ack
>
>> +
>> +/**
>> + * bcl_enable_irq - Generic helper to enable alarm irq
>> + * @alarm: BCL level alarm data
>> + */
>> +static inline void bcl_enable_irq(struct bcl_alarm_data *alarm)
>> +{
>> +	if (alarm->irq_enabled)
>> +		return;
> This can't happen, but because you separated this to a helper function
> it's not obvious
Agree, will update in v2
>
> I'd suggest that you inline the remaining 3 lines in the one place where
> this function is called.
Ack
>
>> +	alarm->irq_enabled = true;
>> +	enable_irq(alarm->irq);
>> +	enable_irq_wake(alarm->irq);
>> +}
>> +
>> +/**
>> + * bcl_disable_irq - Generic helper to disable alarm irq
>> + * @alarm: BCL level alarm data
>> + */
>> +static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)
>> +{
>> +	if (!alarm->irq_enabled)
>> +		return;
> This is tricker, because there's a window between
> devm_request_threaded_irq() and the assignment of irq_enabled, where the
> interrupt function might execute and the attempt to bcl_disable_irq()
> will face irq_enabled == 0.
>
> But I don't think that's intentional...
got it, I will synchronize this with per alarm lock in next patch
>
> I think this too would be better to just inline in the one place its'
> being called.

Ack

Thanks,
Manaf
>
> Regards,
> Bjorn
>
>> +	alarm->irq_enabled = false;
>> +	disable_irq_nosync(alarm->irq);
>> +	disable_irq_wake(alarm->irq);
>> +}
>> +
>> +#endif /* __QCOM_BCL_HWMON_H__ */
>>
>> -- 
>> 2.43.0
>>

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[kernel test robot]

Hi Manaf,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 0f8a890c4524d6e4013ff225e70de2aed7e6d726]

url:    https://github.com/intel-lab-lkp/linux/commits/Manaf-Meethalavalappu-Pallikunhi/dt-bindings-hwmon-Add-qcom-bcl-hwmon-yaml-bindings/20260206-051819
base:   0f8a890c4524d6e4013ff225e70de2aed7e6d726
patch link:    https://lore.kernel.org/r/20260206-qcom-bcl-hwmon-v1-2-7b426f0b77a1%40oss.qualcomm.com
patch subject: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260208/202602080939.NxUyWMbv-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260208/202602080939.NxUyWMbv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602080939.NxUyWMbv-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/hwmon/qcom-bcl-hwmon.c:338 function parameter 'mval' not described in 'bcl_convert_milliunit_to_raw'
>> Warning: drivers/hwmon/qcom-bcl-hwmon.c:338 function parameter 'mval' not described in 'bcl_convert_milliunit_to_raw'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Daniel Lezcano]

Hi Manaf,

On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
> monitor driver. The BCL peripheral is present in Qualcomm PMICs and
> provides real-time monitoring and protection against battery
> overcurrent and under voltage conditions.
> 
> The driver monitors:
> - Battery voltage with configurable low voltage thresholds
> - Battery current with configurable high current thresholds
> - Two limit alarm interrupts (max/min, critical)

Can you describe the behavior of the alarm ?

I assume the alarm is raised when a threshold is crossed from normal
to anormal condition leading to a hwmon event.

 * Does the BCL trigger an interrupt when going back to the normal condition ?

 * When is the alarm flag reset ?

 * Can we have a flood of events if the current / voltage is wavering
   around the thresholds ?

Overall, the driver is too big, so hard to review. It is better to
provide a simplified version with one version supported.

> The driver integrates with the Linux hwmon subsystem and provides
> standard hwmon attributes for monitoring battery conditions.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  MAINTAINERS                    |   9 +
>  drivers/hwmon/Kconfig          |   9 +
>  drivers/hwmon/Makefile         |   1 +
>  drivers/hwmon/qcom-bcl-hwmon.c | 982 +++++++++++++++++++++++++++++++++++++++++
>  drivers/hwmon/qcom-bcl-hwmon.h | 311 +++++++++++++
>  5 files changed, 1312 insertions(+)

[ ... ]

> diff --git a/drivers/hwmon/qcom-bcl-hwmon.c b/drivers/hwmon/qcom-bcl-hwmon.c
> new file mode 100644
> index 000000000000..a7e3b865de5c
> --- /dev/null
> +++ b/drivers/hwmon/qcom-bcl-hwmon.c
> @@ -0,0 +1,982 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm pmic BCL hardware driver for battery overcurrent and
> + * battery or system under voltage monitor
> + *
> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
> + */

Old copyright format

> +#include <linux/devm-helpers.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/workqueue.h>
> +
> +#include "qcom-bcl-hwmon.h"
> +
> +ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
> +ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
> +
> +/* Interrupt names for each alarm level */
> +static const char * const bcl_int_names[ALARM_MAX] = {
> +	[LVL0] = "bcl-max-min",
> +	[LVL1] = "bcl-critical",
> +};

IIUC there are three levels of alarms but the hwmon only has max/min
and critical. Would it make sense to do adaptative min / max ? So when
LVL0 is reached, update min / max with LVL1 value, LVL2 is critical
and does not change ?

> +static const char * const bcl_channel_label[CHANNEL_MAX] = {
> +	"BCL Voltage",
> +	"BCL Current",
> +};

s/[CHANNEL_MAX]/[]/

> +/* Common Reg Fields */
> +static const struct reg_field common_reg_fields[COMMON_FIELD_MAX] = {
> +	[F_V_MAJOR]	= REG_FIELD(REVISION2, 0, 7),
> +	[F_V_MINOR]	= REG_FIELD(REVISION1, 0, 7),
> +	[F_CTL_EN]	= REG_FIELD(EN_CTL1, 7, 7),
> +	[F_LVL0_ALARM]	= REG_FIELD(STATUS, 0, 0),
> +	[F_LVL1_ALARM]	= REG_FIELD(STATUS, 1, 1),
> +};
> +
> +/* BCL Version/Modes specific fields */
> +static const struct reg_field bcl_v1_reg_fields[] = {
> +	[F_IN_MON_EN]	= REG_FIELD(MODE_CTL1, 0, 2),
> +	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
> +	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
> +	[F_IN_INPUT_EN]	= REG_FIELD(VADC_CONV_REQ, 0, 0),
> +	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
> +	[F_CURR_MON_EN]	= REG_FIELD(IADC_CONV_REQ, 0, 0),
> +	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
> +	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR, 0, 7),
> +	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 7),
> +};
> +
> +static const struct reg_field bcl_v2_reg_fields[] = {
> +	[F_IN_MON_EN]	= REG_FIELD(VCMP_CTL, 0, 1),
> +	[F_IN_L0_THR]	= REG_FIELD(VADC_L0_THR, 0, 7),
> +	[F_IN_L1_THR]	= REG_FIELD(VCMP_L1_THR, 0, 5),
> +	[F_IN_INPUT_EN]	= REG_FIELD(VADC_CONV_REQ, 0, 0),
> +	[F_IN_INPUT]	= REG_FIELD(VADC_DATA1, 0, 7),
> +	[F_CURR_MON_EN]	= REG_FIELD(IADC_CONV_REQ, 0, 0),
> +	[F_CURR_H0_THR]	= REG_FIELD(IADC_H0_THR, 0, 7),
> +	[F_CURR_H1_THR]	= REG_FIELD(IADC_H1_THR, 0, 7),
> +	[F_CURR_INPUT]	= REG_FIELD(IADC_DATA1, 0, 7),
> +};

Please begin with a simplified driver supporting one version and then
add more versions. That will help for the review process.

[ ... ]

> +/* V1 BMX and core */
> +static const struct bcl_desc pm7250b_data = {
> +	.reg_fields = bcl_v1_reg_fields,
> +	.num_reg_fields = F_MAX_FIELDS,
> +	.data_field_bits_size = 8,
> +	.thresh_field_bits_size = 7,
> +	.channel[IN] = {
> +		.base = 2250,
> +		.max = 3600,
> +		.step = 25,
> +		.default_scale_nu = 194637,
> +		.thresh_type = {ADC, INDEX},
> +	},
> +	.channel[CURR] = {
> +		.max = 10000,
> +		.default_scale_nu = 305180,
> +		.thresh_type = {ADC, ADC},
> +	},
> +};
>
> +/* V2 BMX and core */

Ditto

[ ... ]

> +/**
> + * bcl_convert_raw_to_milliunit - Convert raw value to milli unit
> + * @desc: BCL device descriptor
> + * @raw_val: Raw ADC value from hardware
> + * @type: type of the channel, in or curr
> + * @field_width: bits size for data or threshold field
> + *
> + * Return: value in milli unit
> + */
> +static unsigned int bcl_convert_raw_to_milliunit(const struct bcl_desc *desc, int raw_val,
> +					enum bcl_channel_type type, u8 field_width)
> +{
> +	u32 def_scale = desc->channel[type].default_scale_nu;
> +	u32 lsb_weight = field_width > 8 ? 1 : 1 << field_width;
> +	u32 scaling_factor = def_scale * lsb_weight;

This is confusing, can you explain ?

if 'field_width' == 7, then we do def_scale * 1^7 ?

Why ?

> +	return div_s64((s64)raw_val * scaling_factor, 1000000);

If it is milliunit why dividing by 10^6 ?

> +/**
> + * bcl_convert_milliunit_to_index - Convert milliunit to in or curr index
> + * @desc: BCL device descriptor
> + * @val: in or curr value in milli unit
> + * @type: type of the channel, in or curr
> + *
> + * Converts a value in milli unit to an index for BCL that use indexed thresholds.
> + *
> + * Return: Voltage index value
> + */
> +static unsigned int bcl_convert_milliunit_to_index(const struct bcl_desc *desc, int val,
> +					  enum bcl_channel_type type)
> +{
> +	return div_s64((s64)val - desc->channel[type].base, desc->channel[type].step);
> +}
> +
> +/**
> + * bcl_convert_index_to_milliunit - Convert in or curr index to milli unit
> + * @desc: BCL device descriptor
> + * @val: index value
> + * @type: type of the channel, in or curr
> + *
> + * Converts an index value to milli unit for BCL that use indexed thresholds.
> + *
> + * Return: Voltage value in millivolts
> + */
> +static unsigned int bcl_convert_index_to_milliunit(const struct bcl_desc *desc, int val,
> +					 enum bcl_channel_type type)
> +{
> +	return desc->channel[type].base + val * desc->channel[type].step;

To be sure, is it (A + val) * B, or, A + (val * B) ?

> +}
> +
> +static int bcl_in_thresh_write(struct bcl_device *bcl, long value, enum bcl_limit_alarm lvl)
> +{
> +	const struct bcl_desc *desc = bcl->desc;
> +	u32 raw_val;
> +
> +	int thresh = clamp_val(value, desc->channel[IN].base, desc->channel[IN].max);
> +
> +	if (desc->channel[IN].thresh_type[lvl] == ADC)
> +		raw_val = bcl_convert_milliunit_to_raw(desc, thresh, IN,
> +						       desc->thresh_field_bits_size);
> +	else
> +		raw_val = bcl_convert_milliunit_to_index(desc, thresh, IN);
> +
> +	return regmap_field_write(bcl->fields[F_IN_L0_THR + lvl], raw_val);
> +}
> +
> +static int bcl_curr_thresh_write(struct bcl_device *bcl, long value, enum bcl_limit_alarm lvl)
> +{
> +	const struct bcl_desc *desc = bcl->desc;
> +	u32 raw_val;
> +
> +	/* Clamp only to curr max */
> +	int thresh = clamp_val(value, value, desc->channel[CURR].max);
> +
> +	if (desc->channel[CURR].thresh_type[lvl] == ADC)
> +		raw_val = bcl_convert_milliunit_to_raw(desc, thresh, CURR,
> +						       desc->thresh_field_bits_size);
> +	else
> +		raw_val = bcl_convert_milliunit_to_index(desc, thresh, CURR);
> +
> +	return regmap_field_write(bcl->fields[F_CURR_H0_THR + lvl], raw_val);
> +}
> +
> +static int bcl_in_thresh_read(struct bcl_device *bcl, enum bcl_limit_alarm lvl, long *out)
> +{
> +	int ret, thresh;
> +	u32 raw_val = 0;
> +	const struct bcl_desc *desc = bcl->desc;
> +
> +	ret = bcl_read_field_value(bcl, F_IN_L0_THR + lvl, &raw_val);
> +	if (ret)
> +		return ret;
> +
> +	if (desc->channel[IN].thresh_type[lvl] == ADC)
> +		thresh = bcl_convert_raw_to_milliunit(desc, raw_val, IN,
> +						      desc->thresh_field_bits_size);
> +	else
> +		thresh = bcl_convert_index_to_milliunit(desc, raw_val, IN);
> +
> +	*out = thresh;
> +
> +	return 0;
> +}
> +
> +static int bcl_curr_thresh_read(struct bcl_device *bcl, enum bcl_limit_alarm lvl, long *out)
> +{
> +	int ret, thresh;
> +	u32 raw_val = 0;
> +	const struct bcl_desc *desc = bcl->desc;
> +
> +	ret = bcl_read_field_value(bcl, F_CURR_H0_THR + lvl, &raw_val);
> +	if (ret)
> +		return ret;
> +
> +	if (desc->channel[CURR].thresh_type[lvl] == ADC)
> +		thresh = bcl_convert_raw_to_milliunit(desc, raw_val, CURR,
> +						      desc->thresh_field_bits_size);
> +	else
> +		thresh = bcl_convert_index_to_milliunit(desc, raw_val, CURR);
> +
> +	*out = thresh;
> +
> +	return 0;
> +}
> +
> +static int bcl_curr_input_read(struct bcl_device *bcl, long *out)
> +{
> +	int ret;
> +	u32 raw_val = 0;
> +	const struct bcl_desc *desc = bcl->desc;
> +
> +	ret = bcl_read_field_value(bcl, F_CURR_INPUT, &raw_val);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The sensor sometime can read a value 0 if there are
> +	 * consecutive reads
> +	 */

In order to prevent the userspace to read in a too high rate the
values of a hwmon, where I guess it is the reason why the value can be
0, the cached value is returned if two reads are under a minimal
jiffies based interval.

Something like:

	  if (time_before(jiffies, last_updated + HZ))
	     return bcl->last_curr_input;

If it is correct, then that could be put at the beginning of the
function instead of checking the zero value.

> +	if (raw_val != 0)
> +		bcl->last_curr_input =
> +			bcl_convert_raw_to_milliunit(desc, raw_val, CURR,
> +						     desc->data_field_bits_size);
> +
> +	*out = bcl->last_curr_input;
> +
> +	return 0;
> +}
> +
> +static int bcl_in_input_read(struct bcl_device *bcl, long *out)
> +{
> +	int ret;
> +	u32 raw_val = 0;
> +	const struct bcl_desc *desc = bcl->desc;
> +
> +	ret = bcl_read_field_value(bcl, F_IN_INPUT, &raw_val);
> +	if (ret)
> +		return ret;
> +
> +	if (raw_val < GENMASK(desc->data_field_bits_size - 1, 0))
> +		bcl->last_in_input =
> +			bcl_convert_raw_to_milliunit(desc, raw_val, IN,
> +						     desc->data_field_bits_size);
> +
> +	*out = bcl->last_in_input;
> +
> +	return 0;
> +}
> +
> +static int bcl_read_alarm_status(struct bcl_device *bcl,
> +				 enum bcl_limit_alarm lvl, long *status)
> +{
> +	int ret;
> +	u32 raw_val = 0;
> +
> +	ret = bcl_read_field_value(bcl, F_LVL0_ALARM + lvl, &raw_val);
> +	if (ret)
> +		return ret;
> +
> +	*status = raw_val;
> +
> +	return 0;
> +}
> +
> +static unsigned int bcl_get_version_major(const struct bcl_device *bcl)
> +{
> +	u32 raw_val = 0;
> +
> +	bcl_read_field_value(bcl, F_V_MAJOR, &raw_val);
> +
> +	return raw_val;
> +}
> +
> +static unsigned int bcl_get_version_minor(const struct bcl_device *bcl)
> +{
> +	u32 raw_val = 0;
> +
> +	bcl_read_field_value(bcl, F_V_MINOR, &raw_val);
> +
> +	return raw_val;
> +}
> +
> +static void bcl_hwmon_notify_event(struct bcl_device *bcl, enum bcl_limit_alarm alarm)
> +{
> +	if (bcl->in_mon_enabled)
> +		hwmon_notify_event(bcl->hwmon_dev, hwmon_in,
> +				in_lvl_to_attr_map[alarm], 0);
> +	if (bcl->curr_mon_enabled)
> +		hwmon_notify_event(bcl->hwmon_dev, hwmon_curr,
> +				curr_lvl_to_attr_map[alarm], 0);
> +}

What is the rate of the events we can face if they are mitigations
happening under the hood from the HW ?

> +static void bcl_alarm_enable_poll(struct work_struct *work)
> +{
> +	struct bcl_alarm_data *alarm = container_of(work, struct bcl_alarm_data,
> +							 alarm_poll_work.work);
> +	struct bcl_device *bcl = alarm->device;
> +	long status;
> +
> +	guard(mutex)(&bcl->lock);
> +
> +	if (bcl_read_alarm_status(bcl, alarm->type, &status))
> +		goto re_schedule;
> +
> +	if (!status & !alarm->irq_enabled) {
> +		bcl_enable_irq(alarm);
> +		bcl_hwmon_notify_event(bcl, alarm->type);
> +		return;
> +	}
> +
> +re_schedule:
> +	schedule_delayed_work(&alarm->alarm_poll_work,
> +				msecs_to_jiffies(BCL_ALARM_POLLING_MS));
> +}
> +
> +static irqreturn_t bcl_handle_alarm(int irq, void *data)
> +{
> +	struct bcl_alarm_data *alarm = data;
> +	struct bcl_device *bcl = alarm->device;
> +	long status;
> +
> +	guard(mutex)(&bcl->lock);
> +
> +	if (bcl_read_alarm_status(bcl, alarm->type, &status) || !status)
> +		return IRQ_HANDLED;

So it is possible to have an interrupt but not an alarm (!status) ?

> +	if (!bcl->hwmon_dev)
> +		return IRQ_HANDLED;

This should not be needed, revisit the init routine

> +
> +	bcl_hwmon_notify_event(bcl, alarm->type);
> +
> +	bcl_disable_irq(alarm);
> +	schedule_delayed_work(&alarm->alarm_poll_work,
> +			msecs_to_jiffies(BCL_ALARM_POLLING_MS));

Why ?

> +	dev_dbg(bcl->dev, "Irq:%d triggered for bcl type:%d\n",
> +			 irq, alarm->type);

Please remove, that is debug code

> +	return IRQ_HANDLED;
> +}
> +
> +static umode_t bcl_hwmon_is_visible(const void *data,
> +				    enum hwmon_sensor_types type,
> +				    u32 attr, int channel)
> +{
> +	const struct bcl_device *bcl = data;
> +
> +	switch (type) {
> +	case hwmon_in:
> +		if (!bcl->in_mon_enabled)
> +			return 0;
> +		switch (attr) {
> +		case hwmon_in_input:
> +			return bcl->in_input_enabled ? 0444 : 0;
> +		case hwmon_in_label:
> +		case hwmon_in_min_alarm:
> +		case hwmon_in_lcrit_alarm:
> +			return 0444;
> +		case hwmon_in_min:
> +		case hwmon_in_lcrit:
> +			return 0644;
> +		default:
> +			return 0;
> +		}
> +	case hwmon_curr:
> +		if (!bcl->curr_mon_enabled)
> +			return 0;
> +		switch (attr) {
> +		case hwmon_curr_input:
> +		case hwmon_curr_label:
> +		case hwmon_curr_max_alarm:
> +		case hwmon_curr_crit_alarm:
> +			return 0444;
> +		case hwmon_curr_max:
> +		case hwmon_curr_crit:
> +			return 0644;
> +		default:
> +			return 0;
> +		}
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int bcl_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> +			   u32 attr, int channel, long val)

What is the benefit of the userspace to set the thresholds ? Should
they be a platform property ?

Should the thresholds be check to be in ascending order and separated
with 100mV (for IN) ?

> +{
> +	struct bcl_device *bcl = dev_get_drvdata(dev);
> +	int ret = -EOPNOTSUPP;
> +
> +	guard(mutex)(&bcl->lock);
> +
> +	switch (type) {
> +	case hwmon_in:
> +		switch (attr) {
> +		case hwmon_in_min:
> +		case hwmon_in_lcrit:
> +			ret = bcl_in_thresh_write(bcl, val, in_attr_to_lvl_map[attr]);
> +			break;
> +		default:
> +			ret = -EOPNOTSUPP;
> +		}
> +		break;
> +	case hwmon_curr:
> +		switch (attr) {
> +		case hwmon_curr_max:
> +		case hwmon_curr_crit:
> +			ret = bcl_curr_thresh_write(bcl, val, curr_attr_to_lvl_map[attr]);
> +			break;
> +		default:
> +			ret = -EOPNOTSUPP;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int bcl_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +			  u32 attr, int channel, long *value)
> +{
> +	struct bcl_device *bcl = dev_get_drvdata(dev);
> +	int ret;
> +
> +	guard(mutex)(&bcl->lock);
> +
> +	switch (type) {
> +	case hwmon_in:
> +		switch (attr) {
> +		case hwmon_in_input:
> +			ret = bcl_in_input_read(bcl, value);
> +			break;
> +		case hwmon_in_min:
> +		case hwmon_in_lcrit:
> +			ret = bcl_in_thresh_read(bcl, in_attr_to_lvl_map[attr], value);
> +			break;
> +		case hwmon_in_min_alarm:
> +		case hwmon_in_lcrit_alarm:
> +			ret = bcl_read_alarm_status(bcl, in_attr_to_lvl_map[attr], value);
> +			break;
> +		default:
> +			ret = -EOPNOTSUPP;
> +		}
> +		break;

Please split this function into:

       case hwmon_in:
       	    return bcl_in_read();
       case hwmon_curr:
       	    return bcl_curr_read();
       default:
             return -EOPNOTSUPP;

> +	case hwmon_curr:
> +		switch (attr) {
> +		case hwmon_curr_input:
> +			ret = bcl_curr_input_read(bcl, value);
> +			break;
> +		case hwmon_curr_max:
> +		case hwmon_curr_crit:
> +			ret = bcl_curr_thresh_read(bcl, curr_attr_to_lvl_map[attr], value);
> +			break;
> +		case hwmon_curr_max_alarm:
> +		case hwmon_curr_crit_alarm:
> +			ret = bcl_read_alarm_status(bcl, curr_attr_to_lvl_map[attr], value);
> +			break;
> +		default:
> +			ret = -EOPNOTSUPP;
> +		}
> +		break;
> +	default:
> +		ret = -EOPNOTSUPP;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int bcl_hwmon_read_string(struct device *dev,
> +				 enum hwmon_sensor_types type,
> +				 u32 attr, int channel, const char **str)
> +{
> +	switch (type) {
> +	case hwmon_in:
> +		if (attr != hwmon_in_label)
> +			break;
> +		*str = bcl_channel_label[IN];

Why not use 'channel' passed as parameter ?

> +		return 0;
> +	case hwmon_curr:
> +		if (attr != hwmon_curr_label)
> +			break;
> +		*str = bcl_channel_label[CURR];
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	return -EOPNOTSUPP;

Given there are few channels I suggest to simplify to:

      if (type == hwmon_in && attr == hwmon_in_label) {
      	 *str = "BCL Voltage";
      } else if (type == hwmon_curr && attr == hwmon_curr_label) {
         *str = "BCL Current";
      } else {
      	 *str = NULL;
      }

      return *str ? 0 : -EOPNOTSUPP;

Up to you.

> +}
> +
> +static const struct hwmon_ops bcl_hwmon_ops = {
> +	.is_visible	= bcl_hwmon_is_visible,
> +	.read		= bcl_hwmon_read,
> +	.read_string	= bcl_hwmon_read_string,
> +	.write		= bcl_hwmon_write,
> +};
> +
> +static const struct hwmon_channel_info *bcl_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(in,
> +			   HWMON_I_INPUT | HWMON_I_LABEL | HWMON_I_MIN |
> +			   HWMON_I_LCRIT | HWMON_I_MIN_ALARM |
> +			   HWMON_I_LCRIT_ALARM),
> +	HWMON_CHANNEL_INFO(curr,
> +			   HWMON_C_INPUT | HWMON_C_LABEL | HWMON_C_MAX |
> +			   HWMON_C_CRIT | HWMON_C_MAX_ALARM |
> +			   HWMON_C_CRIT_ALARM),
> +	NULL,
> +};
> +
> +static const struct hwmon_chip_info bcl_hwmon_chip_info = {
> +	.ops	= &bcl_hwmon_ops,
> +	.info	= bcl_hwmon_info,
> +};
> +
> +static int bcl_curr_thresh_update(struct bcl_device *bcl)
> +{
> +	int ret, i;
> +
> +	if (!bcl->curr_thresholds[0])
> +		return 0;
> +
> +	for (i = 0; i < ALARM_MAX; i++) {
> +		ret = bcl_curr_thresh_write(bcl, bcl->curr_thresholds[i], i);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void bcl_hw_channel_mon_init(struct bcl_device *bcl)
> +{
> +	bcl->in_mon_enabled = bcl_in_monitor_enabled(bcl);
> +	bcl->in_input_enabled = bcl_in_input_enabled(bcl);
> +	bcl->curr_mon_enabled = bcl_curr_monitor_enabled(bcl);

Can you describe why we can have this optionnal ?

Why not set bcl_hwmon_info[] regarding what is enabled instead of
having boolean checks all over the place ?

> +}
> +
> +static int bcl_alarm_irq_init(struct platform_device *pdev,
> +			      struct bcl_device *bcl)
> +{
> +	int ret = 0, irq_num = 0, i = 0;
> +	struct bcl_alarm_data *alarm;
> +
> +	for (i = LVL0; i < ALARM_MAX; i++) {
> +		alarm = &bcl->bcl_alarms[i];
> +		alarm->type = i;
> +		alarm->device = bcl;
> +
> +		ret = devm_delayed_work_autocancel(bcl->dev, &alarm->alarm_poll_work,
> +					   bcl_alarm_enable_poll);
> +		if (ret)
> +			return ret;

Why ?

> +		irq_num = platform_get_irq_byname(pdev, bcl_int_names[i]);
> +		if (irq_num <= 0)
> +			continue;
> +
> +		ret = devm_request_threaded_irq(&pdev->dev, irq_num, NULL,
> +						bcl_handle_alarm, IRQF_ONESHOT,
> +						bcl_int_names[i], alarm);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Error requesting irq(%s).err:%d\n",
> +				bcl_int_names[i], ret);
> +			return ret;
> +		}
> +		enable_irq_wake(alarm->irq);
> +		alarm->irq_enabled = true;
> +		alarm->irq = irq_num;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcl_regmap_field_init(struct device *dev, struct bcl_device *bcl,
> +				 const struct bcl_desc *data)
> +{
> +	int i;
> +	struct reg_field fields[F_MAX_FIELDS];
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(common_reg_fields) != COMMON_FIELD_MAX);

No, this error implies the way the fields are declared is fragile, may
be revisit the declaration.

> +
> +	for (i = 0; i < data->num_reg_fields; i++) {
> +		if (i < COMMON_FIELD_MAX)
> +			fields[i] = common_reg_fields[i];
> +		else
> +			fields[i] = data->reg_fields[i];
> +
> +		/* Need to adjust BCL base from regmap dynamically */
> +		fields[i].reg += bcl->base;
> +	}
> +
> +	return devm_regmap_field_bulk_alloc(dev, bcl->regmap, bcl->fields,
> +					   fields, data->num_reg_fields);
> +}
> +
> +static int bcl_get_device_property_data(struct platform_device *pdev,
> +				   struct bcl_device *bcl)

*dev could be passed as parameter instead of *pdev

> +{
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +	u32 reg;
> +
> +	ret = device_property_read_u32(dev, "reg", &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	bcl->base = reg;
> +
> +	device_property_read_u32_array(dev, "overcurrent-thresholds-milliamp",
> +				       bcl->curr_thresholds, 2);

Why don't you want this as a required property ?

> +	return 0;
> +}
> +
> +static int bcl_probe(struct platform_device *pdev)
> +{
> +	struct bcl_device *bcl;
> +	int ret;
> +
> +	bcl = devm_kzalloc(&pdev->dev, sizeof(*bcl), GFP_KERNEL);
> +	if (!bcl)
> +		return -ENOMEM;
> +
> +	bcl->dev = &pdev->dev;
> +	bcl->desc = device_get_match_data(&pdev->dev);
> +	if (!bcl->desc)
> +		return -EINVAL;
> +
> +	ret = devm_mutex_init(bcl->dev, &bcl->lock);
> +	if (ret)
> +		return ret;
> +
> +	bcl->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!bcl->regmap) {
> +		dev_err(&pdev->dev, "Couldn't get parent's regmap\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = bcl_get_device_property_data(pdev, bcl);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = bcl_regmap_field_init(bcl->dev, bcl, bcl->desc);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Unable to allocate regmap fields, err:%d\n", ret);
> +		return ret;
> +	}
> +
> +	if (!bcl_hw_is_enabled(bcl))
> +		return -ENODEV;
> +
> +	ret = bcl_curr_thresh_update(bcl);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = bcl_alarm_irq_init(pdev, bcl);
> +	if (ret < 0)
> +		return ret;
> +
> +	bcl_hw_channel_mon_init(bcl);
> +
> +	dev_set_drvdata(&pdev->dev, bcl);
> +
> +	bcl->hwmon_name = devm_hwmon_sanitize_name(&pdev->dev,
> +						   dev_name(bcl->dev));
> +	if (IS_ERR(bcl->hwmon_name)) {
> +		dev_err(&pdev->dev, "Failed to sanitize hwmon name\n");
> +		return PTR_ERR(bcl->hwmon_name);
> +	}
> +
> +	bcl->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> +							      bcl->hwmon_name,
> +							      bcl,
> +							      &bcl_hwmon_chip_info,
> +							      NULL);
> +	if (IS_ERR(bcl->hwmon_dev)) {
> +		dev_err(&pdev->dev, "Failed to register hwmon device: %ld\n",
> +			PTR_ERR(bcl->hwmon_dev));
> +		return PTR_ERR(bcl->hwmon_dev);
> +	}
> +
> +	dev_dbg(&pdev->dev, "BCL hwmon device with version: %u.%u registered\n",
> +		bcl_get_version_major(bcl), bcl_get_version_minor(bcl));
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bcl_match[] = {
> +	{
> +		.compatible = "qcom,bcl-v1",
> +		.data = &pm7250b_data,
> +	}, {
> +		.compatible = "qcom,bcl-v2",
> +		.data = &pm8350_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-bmx",
> +		.data = &pm8550b_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-wb",
> +		.data = &pmw5100_data,
> +	}, {
> +		.compatible = "qcom,bcl-v3-core",
> +		.data = &pm8550_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-bmx",
> +		.data = &pmih010_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-wb",
> +		.data = &pmw6100_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-core",
> +		.data = &pmh010_data,
> +	}, {
> +		.compatible = "qcom,bcl-v4-pmv010",
> +		.data = &pmv010_data,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, bcl_match);
> +
> +static struct platform_driver bcl_driver = {
> +	.probe	= bcl_probe,
> +	.driver	= {
> +		.name		= BCL_DRIVER_NAME,
> +		.of_match_table	= bcl_match,
> +	},
> +};
> +
> +MODULE_AUTHOR("Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("QCOM BCL HWMON driver");
> +module_platform_driver(bcl_driver);
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/qcom-bcl-hwmon.h b/drivers/hwmon/qcom-bcl-hwmon.h
> new file mode 100644
> index 000000000000..28a7154d9486
> --- /dev/null
> +++ b/drivers/hwmon/qcom-bcl-hwmon.h

As there is nothing shared with other files, no header is needed.

> @@ -0,0 +1,311 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef __QCOM_BCL_HWMON_H__
> +#define __QCOM_BCL_HWMON_H__
> +
> +#define BCL_DRIVER_NAME			"qcom-bcl-hwmon"
> +
> +/* BCL common regmap offset */
> +#define REVISION1			0x0
> +#define REVISION2			0x1
> +#define STATUS				0x8
> +#define INT_RT_STS			0x10
> +#define EN_CTL1				0x46
> +
> +/* BCL GEN1 regmap offsets */
> +#define MODE_CTL1			0x41
> +#define VADC_L0_THR			0x48
> +#define VCMP_L1_THR			0x49
> +#define IADC_H0_THR			0x4b
> +#define IADC_H1_THR			0x4c
> +#define VADC_CONV_REQ			0x72
> +#define IADC_CONV_REQ			0x82
> +#define VADC_DATA1			0x76
> +#define IADC_DATA1			0x86
> +
> +/* BCL GEN3 regmap offsets */
> +#define VCMP_CTL			0x44
> +#define VCMP_L0_THR			0x47
> +#define PARAM_1				0x0e
> +#define IADC_H1_THR_GEN3		0x4d
> +
> +#define BCL_IN_INC_MV			25
> +#define BCL_ALARM_POLLING_MS		50
> +
> +/**
> + * enum bcl_limit_alarm - BCL alarm threshold levels
> + * @LVL0: Level 0 alarm threshold (mapped to in_min_alarm or curr_max_alarm)
> + * @LVL1: Level 1 alarm threshold (mapped to in_lcrit_alarm or curr_crit_alarm)
> + * @ALARM_MAX: sentinel value
> + *
> + * Defines the three threshold levels for BCL monitoring. Each level corresponds
> + * to different severity of in or curr conditions.
> + */
> +enum bcl_limit_alarm {
> +	LVL0,
> +	LVL1,
> +	ALARM_MAX,
> +};

These are too generic names and they can collide with other
subsystems. BCL_LIMIT_ALRM_LVL0/1, BLC_LIMIT_ALRM_MAX should be fine.

> +/**
> + * enum bcl_channel_type - BCL supported sensor channel type
> + * @IN: in (voltage) channel
> + * @CURR: curr (current) channel
> + * @CHANNEL_MAX: sentinel value
> + *
> + * Defines the supported channel types for bcl.
> + */
> +enum bcl_channel_type {

s/bcl_channel_type/bcl_channel/ ?

> +	IN,
> +	CURR,
> +
> +	CHANNEL_MAX,
> +};

Same comment: BCL_CHANNEL_IN, BCL_CHANNEL_CURR, BCL_CHANNEL_MAX

> +/**
> + * enum bcl_thresh_type - voltage or current threshold representation type
> + * @ADC: Raw ADC value representation
> + * @INDEX: Index-based voltage or current representation
> + *
> + * Specifies how voltage or current thresholds are stored and interpreted in
> + * registers. Some PMICs use raw ADC values while others use indexed values.
> + */
> +enum bcl_thresh_type {
> +	ADC,
> +	INDEX,
> +};

Ditto

> +/**
> + * enum bcl_fields - BCL register field identifiers
> + * @F_V_MAJOR: Major revision info field
> + * @F_V_MINOR: Minor revision info field
> + * @F_CTL_EN: Monitor enable control field
> + * @F_LVL0_ALARM: Level 0 alarm status field
> + * @F_LVL1_ALARM: Level 1 alarm status field
> + * @COMMON_FIELD_MAX: sentinel value for common fields
> + * @F_IN_MON_EN: voltage monitor enable control field
> + * @F_IN_L0_THR: voltage level 0 threshold field
> + * @F_IN_L1_THR: voltage level 1 threshold field
> + * @F_IN_INPUT_EN: voltage input enable control field
> + * @F_IN_INPUT: voltage input data field
> + * @F_CURR_MON_EN: current monitor enable control field
> + * @F_CURR_H0_THR: current level 0 threshold field
> + * @F_CURR_H1_THR: current level 1 threshold field
> + * @F_CURR_INPUT: current input data field
> + * @F_MAX_FIELDS: sentinel value
> + *
> + * Enumeration of all register fields used by the BCL driver for accessing
> + * registers through regmap fields.
> + */
> +enum bcl_fields {
> +	F_V_MAJOR,
> +	F_V_MINOR,
> +
> +	F_CTL_EN,
> +
> +	/* common alarm for in and curr channel */
> +	F_LVL0_ALARM,
> +	F_LVL1_ALARM,
> +
> +	COMMON_FIELD_MAX,
> +
> +	F_IN_MON_EN = COMMON_FIELD_MAX,
> +	F_IN_L0_THR,
> +	F_IN_L1_THR,
> +
> +	F_IN_INPUT_EN,
> +	F_IN_INPUT,
> +
> +	F_CURR_MON_EN,
> +	F_CURR_H0_THR,
> +	F_CURR_H1_THR,
> +
> +	F_CURR_INPUT,
> +
> +	F_MAX_FIELDS
> +};

Usually this is described with macro, not enum.

> +#define ADD_BCL_HWMON_ALARM_MAPS(_type, lvl0_attr, lvl1_attr)	\
> +								\
> +static const u8 _type##_attr_to_lvl_map[] = {			\
> +	[hwmon_##_type##_##lvl0_attr] = LVL0,			\
> +	[hwmon_##_type##_##lvl1_attr] = LVL1,			\
> +	[hwmon_##_type##_##lvl0_attr##_alarm] = LVL0,		\
> +	[hwmon_##_type##_##lvl1_attr##_alarm] = LVL1,		\
> +};								\
> +								\
> +static const u8 _type##_lvl_to_attr_map[ALARM_MAX] = {		\
> +	[LVL0] = hwmon_##_type##_##lvl0_attr##_alarm,		\
> +	[LVL1] = hwmon_##_type##_##lvl1_attr##_alarm,		\
> +}

Please avoid these macros, they don't help to the readability.

> +/**
> + * struct bcl_channel_cfg - BCL channel related configuration
> + * @default_scale_nu:  Default scaling factor in nano unit
> + * @base: Base threshold value in milli unit
> + * @max: Maximum threshold value in milli unit
> + * @step: step increment value between two indexed threshold value
> + * @thresh_type: Array specifying threshold representation type for each alarm level
> + *
> + * Contains hardware-specific configuration and scaling parameters for different
> + * channel(voltage and current)..
> + */
> +
> +struct bcl_channel_cfg {
> +	u32 default_scale_nu;
> +	u32 base;
> +	u32 max;
> +	u32 step;
> +	u8 thresh_type[ALARM_MAX];
> +};
> +
> +/**
> + * struct bcl_desc - BCL device descriptor
> + * @reg_fields: Array of register field definitions for this device variant
> + * @channel: Each channel specific(voltage or current) configuration
> + * @num_reg_fields: Number of register field definitions for this device variant
> + * @data_field_bits_size: data read register bit size
> + * @thresh_field_bits_size: lsb bit size those are not included in threshold register
> + *
> + * Contains hardware-specific configuration and scaling parameters for different
> + * BCL variants. Each PMIC model may have different register layouts and
> + * conversion factors.
> + */
> +
> +struct bcl_desc {
> +	const struct reg_field *reg_fields;
> +	struct bcl_channel_cfg channel[CHANNEL_MAX];
> +	u8 num_reg_fields;
> +	u8 data_field_bits_size;
> +	u8 thresh_field_bits_size;
> +};
> +
> +struct bcl_device;

No forward declaration, reorg the structure definitions below

> +/**
> + * struct bcl_alarm_data - BCL alarm interrupt data
> + * @irq: IRQ number assigned to this alarm
> + * @irq_enabled: Flag indicating if IRQ is enabled
> + * @type: Alarm level type (LVL0, or LVL1)
> + * @device: Pointer to parent BCL device structure
> + * @alarm_poll_work: delayed_work to poll alarm status

Why do you want to poll the alarm status if there is an interrupt ?

> + *
> + * Stores interrupt-related information for each alarm threshold level.
> + * Used by the IRQ handler to identify which alarm triggered.
> + */
> +struct bcl_alarm_data {
> +	int			irq;
> +	bool			irq_enabled;
> +	enum bcl_limit_alarm	type;
> +	struct bcl_device	*device;
> +	struct delayed_work	alarm_poll_work;
> +};
> +
> +/**
> + * struct bcl_device - Main BCL device structure
> + * @dev: Pointer to device structure
> + * @regmap: Regmap for accessing PMIC registers
> + * @fields: Array of regmap fields for register access
> + * @bcl_alarms: Array of alarm data structures for each threshold level
> + * @lock: Mutex for protecting concurrent hardware access
> + * @in_mon_enabled: Flag indicating if voltage monitoring is enabled
> + * @curr_mon_enabled: Flag indicating if current monitoring is enabled
> + * @curr_thresholds: Current threshold values in milliamps from dt-binding(LVL0 and LVL1)
> + * @base: the BCL regbase offset from regmap
> + * @in_input_enabled: Flag indicating if voltage input reading is enabled
> + * @last_in_input: Last valid voltage input reading in millivolts
> + * @last_curr_input: Last valid current input reading in milliamps
> + * @desc: Pointer to device descriptor with hardware-specific parameters
> + * @hwmon_dev: Pointer to registered hwmon device
> + * @hwmon_name: Sanitized name for hwmon device
> + *
> + * Main driver structure containing all state and configuration for a BCL
> + * monitoring instance. Manages voltage and current monitoring, thresholds,
> + * and alarm handling.
> + */
> +struct bcl_device {
> +	struct device		*dev;

This field is unnecessary if the debug message is removed from the
interrupt handler.

> +	struct regmap		*regmap;
> +	u16			base;
> +	struct regmap_field	*fields[F_MAX_FIELDS];
> +	struct bcl_alarm_data	bcl_alarms[ALARM_MAX];
> +	struct mutex		lock;

What is this lock for ?

> +	u32			curr_thresholds[ALARM_MAX];
> +	u32			last_in_input;
> +	u32			last_curr_input;
> +	bool			in_mon_enabled;
> +	bool			curr_mon_enabled;
> +	bool			in_input_enabled;
> +	const struct bcl_desc	*desc;
> +	struct device		*hwmon_dev;
> +	char			*hwmon_name;
> +};
> +
> +/**
> + * bcl_read_field_value - Read alarm status for a given level
> + * @bcl: BCL device structure
> + * @id: Index in bcl->fields[]
> + * @val: Pointer to store val
> + *
> + * Return: 0 on success or regmap error code
> + */
> +static inline int bcl_read_field_value(const struct bcl_device *bcl, enum bcl_fields id, u32 *val)
> +{
> +	return regmap_field_read(bcl->fields[id], val);
> +}
> +
> +/**
> + * bcl_field_enabled - Generic helper to check if a regmap field is enabled
> + * @bcl: BCL device structure
> + * @field: Index in bcl->fields[]
> + *
> + * Return: true if field is non-zero, false otherwise
> + */
> +static inline bool bcl_field_enabled(const struct bcl_device *bcl, enum bcl_fields id)
> +{
> +	int ret;
> +	u32 val = 0;
> +
> +	ret = regmap_field_read(bcl->fields[id], &val);
> +	if (ret)
> +		return false;
> +
> +	return !!val;
> +}
> +
> +#define bcl_in_input_enabled(bcl)	bcl_field_enabled(bcl, F_IN_INPUT_EN)
> +#define bcl_curr_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_CURR_MON_EN)
> +#define bcl_in_monitor_enabled(bcl)	bcl_field_enabled(bcl, F_IN_MON_EN)
> +#define bcl_hw_is_enabled(bcl)		bcl_field_enabled(bcl, F_CTL_EN)
> +
> +/**
> + * bcl_enable_irq - Generic helper to enable alarm irq
> + * @alarm: BCL level alarm data
> + */
> +static inline void bcl_enable_irq(struct bcl_alarm_data *alarm)
> +{
> +	if (alarm->irq_enabled)
> +		return;
> +	alarm->irq_enabled = true;
> +	enable_irq(alarm->irq);
> +	enable_irq_wake(alarm->irq);
> +}
> +
> +/**
> + * bcl_disable_irq - Generic helper to disable alarm irq
> + * @alarm: BCL level alarm data
> + */
> +static inline void bcl_disable_irq(struct bcl_alarm_data *alarm)
> +{
> +	if (!alarm->irq_enabled)
> +		return;
> +	alarm->irq_enabled = false;
> +	disable_irq_nosync(alarm->irq);
> +	disable_irq_wake(alarm->irq);
> +}
> +#endif /* __QCOM_BCL_HWMON_H__ */
> 
> -- 
> 2.43.0
> 

-- 

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Guenter Roeck]

On 3/20/26 07:52, Daniel Lezcano wrote:
> Hi Manaf,
> 
> On Fri, Feb 06, 2026 at 02:44:06AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> Add support for Qualcomm PMIC Battery Current Limiting (BCL) hardware
>> monitor driver. The BCL peripheral is present in Qualcomm PMICs and
>> provides real-time monitoring and protection against battery
>> overcurrent and under voltage conditions.
>>
>> The driver monitors:
>> - Battery voltage with configurable low voltage thresholds
>> - Battery current with configurable high current thresholds
>> - Two limit alarm interrupts (max/min, critical)
> 
> Can you describe the behavior of the alarm ?
> 
> I assume the alarm is raised when a threshold is crossed from normal
> to anormal condition leading to a hwmon event.
> 
>   * Does the BCL trigger an interrupt when going back to the normal condition ?
> 
>   * When is the alarm flag reset ?
> 
>   * Can we have a flood of events if the current / voltage is wavering
>     around the thresholds ?
> 
> Overall, the driver is too big, so hard to review. It is better to
> provide a simplified version with one version supported.
> 
>> The driver integrates with the Linux hwmon subsystem and provides
>> standard hwmon attributes for monitoring battery conditions.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   MAINTAINERS                    |   9 +
>>   drivers/hwmon/Kconfig          |   9 +
>>   drivers/hwmon/Makefile         |   1 +
>>   drivers/hwmon/qcom-bcl-hwmon.c | 982 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/hwmon/qcom-bcl-hwmon.h | 311 +++++++++++++
>>   5 files changed, 1312 insertions(+)
> 
> [ ... ]
> 
>> diff --git a/drivers/hwmon/qcom-bcl-hwmon.c b/drivers/hwmon/qcom-bcl-hwmon.c
>> new file mode 100644
>> index 000000000000..a7e3b865de5c
>> --- /dev/null
>> +++ b/drivers/hwmon/qcom-bcl-hwmon.c
>> @@ -0,0 +1,982 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Qualcomm pmic BCL hardware driver for battery overcurrent and
>> + * battery or system under voltage monitor
>> + *
>> + * Copyright (c) 2026, Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
> 
> Old copyright format
> 
>> +#include <linux/devm-helpers.h>
>> +#include <linux/err.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/property.h>
>> +#include <linux/regmap.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include "qcom-bcl-hwmon.h"
>> +
>> +ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
>> +ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
>> +
>> +/* Interrupt names for each alarm level */
>> +static const char * const bcl_int_names[ALARM_MAX] = {
>> +	[LVL0] = "bcl-max-min",
>> +	[LVL1] = "bcl-critical",
>> +};
> 
> IIUC there are three levels of alarms but the hwmon only has max/min
> and critical. Would it make sense to do adaptative min / max ? So when

hwmon has lcrit, min, max, and crit alarms for all sensor types, plus
an additional _cap_alarm for power attributes and _emergency_alarm
for temperature attributes. There is also a generic _alarm attribute
for each sensor, which is supposed to be used if the specific alarm
type is not known.

What exactly are the three levels of alarms ?

Thanks,
Guenter

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Daniel Lezcano]

Hi Guenter,

On 3/20/26 16:22, Guenter Roeck wrote:
> On 3/20/26 07:52, Daniel Lezcano wrote:

[ ... ]

>>> +
>>> +ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
>>> +ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
>>> +
>>> +/* Interrupt names for each alarm level */
>>> +static const char * const bcl_int_names[ALARM_MAX] = {
>>> +    [LVL0] = "bcl-max-min",
>>> +    [LVL1] = "bcl-critical",
>>> +};
>>
>> IIUC there are three levels of alarms but the hwmon only has max/min
>> and critical. Would it make sense to do adaptative min / max ? So when
> 
> hwmon has lcrit, min, max, and crit alarms for all sensor types, plus
> an additional _cap_alarm for power attributes and _emergency_alarm
> for temperature attributes. There is also a generic _alarm attribute
> for each sensor, which is supposed to be used if the specific alarm
> type is not known.
> 
> What exactly are the three levels of alarms ?

Manaf can give more clarifications, but it is like we have yellow, 
orange and red alarms. So there is an additional alarm comparing to what 
is available in hwmon. The proposed driver maps orange and red alarms, 
respectively to bcl-max and bcl-critical.

I'm just asking if it is important to have this 'yellow' alarm ? And as 
there is a missing alarm to describe it in hwmon, how can we use it ?

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Guenter Roeck]

On 3/20/26 09:08, Daniel Lezcano wrote:
> 
> Hi Guenter,
> 
> On 3/20/26 16:22, Guenter Roeck wrote:
>> On 3/20/26 07:52, Daniel Lezcano wrote:
> 
> [ ... ]
> 
>>>> +
>>>> +ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
>>>> +ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
>>>> +
>>>> +/* Interrupt names for each alarm level */
>>>> +static const char * const bcl_int_names[ALARM_MAX] = {
>>>> +    [LVL0] = "bcl-max-min",
>>>> +    [LVL1] = "bcl-critical",
>>>> +};
>>>
>>> IIUC there are three levels of alarms but the hwmon only has max/min
>>> and critical. Would it make sense to do adaptative min / max ? So when
>>
>> hwmon has lcrit, min, max, and crit alarms for all sensor types, plus
>> an additional _cap_alarm for power attributes and _emergency_alarm
>> for temperature attributes. There is also a generic _alarm attribute
>> for each sensor, which is supposed to be used if the specific alarm
>> type is not known.
>>
>> What exactly are the three levels of alarms ?
> 
> Manaf can give more clarifications, but it is like we have yellow, orange and red alarms. So there is an additional alarm comparing to what is available in hwmon. The proposed driver maps orange and red alarms, respectively to bcl-max and bcl-critical.
> 
> I'm just asking if it is important to have this 'yellow' alarm ? And as there is a missing alarm to describe it in hwmon, how can we use it ?
> 

Is this for high alarms only or also for low alarms ?

I would not mind adding _emergency attributes for non-temperature
alarms if needed. We could also add another low alarm if needed,
though I don't have a good idea for a good name for that.

Guenter

Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Daniel Lezcano]

On 3/20/26 17:59, Guenter Roeck wrote:
> On 3/20/26 09:08, Daniel Lezcano wrote:
>>
>> Hi Guenter,
>>
>> On 3/20/26 16:22, Guenter Roeck wrote:
>>> On 3/20/26 07:52, Daniel Lezcano wrote:
>>
>> [ ... ]
>>
>>>>> +
>>>>> +ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
>>>>> +ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
>>>>> +
>>>>> +/* Interrupt names for each alarm level */
>>>>> +static const char * const bcl_int_names[ALARM_MAX] = {
>>>>> +    [LVL0] = "bcl-max-min",
>>>>> +    [LVL1] = "bcl-critical",
>>>>> +};
>>>>
>>>> IIUC there are three levels of alarms but the hwmon only has max/min
>>>> and critical. Would it make sense to do adaptative min / max ? So when
>>>
>>> hwmon has lcrit, min, max, and crit alarms for all sensor types, plus
>>> an additional _cap_alarm for power attributes and _emergency_alarm
>>> for temperature attributes. There is also a generic _alarm attribute
>>> for each sensor, which is supposed to be used if the specific alarm
>>> type is not known.
>>>
>>> What exactly are the three levels of alarms ?
>>
>> Manaf can give more clarifications, but it is like we have yellow, 
>> orange and red alarms. So there is an additional alarm comparing to 
>> what is available in hwmon. The proposed driver maps orange and red 
>> alarms, respectively to bcl-max and bcl-critical.
>>
>> I'm just asking if it is important to have this 'yellow' alarm ? And 
>> as there is a missing alarm to describe it in hwmon, how can we use it ?
>>
> 
> Is this for high alarms only or also for low alarms ?

It is high for over current and low for under voltage.

> I would not mind adding _emergency attributes for non-temperature
> alarms if needed. We could also add another low alarm if needed,
> though I don't have a good idea for a good name for that.

May be "warning" ?

Alternatively, could it be interesting to have an alarm with a value 
passed in the event which represent the value in the unit of the 
monitored attribute ? So there is no need to add more alarm files

[PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Enable Qualcomm BCL hardware devicetree binding configuration
for pm7250b.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
index 0761e6b5fd8d..69ad76831cde 100644
--- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
@@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
 			interrupt-controller;
 			#interrupt-cells = <2>;
 		};
+
+		bcl@1d00 {
+			compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
+			reg = <0x1d00>;
+			interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
+				     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "bcl-max-min",
+					  "bcl-critical";
+			overcurrent-thresholds-milliamp = <5500 6000>;
+		};
 	};
 
 	pmic@PM7250B_SID1 {

-- 
2.43.0

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Krzysztof Kozlowski]

On Fri, Feb 06, 2026 at 02:44:07AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
> Enable Qualcomm BCL hardware devicetree binding configuration
> for pm7250b.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> index 0761e6b5fd8d..69ad76831cde 100644
> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  		};
> +
> +		bcl@1d00 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

Plus, I doubt this was ever tested. Considering lack of internal review
I do not think this should be posted.

Best regards,
Krzysztof

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Hi Krzysztof,

On 2/6/2026 1:43 PM, Krzysztof Kozlowski wrote:
> On Fri, Feb 06, 2026 at 02:44:07AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>> Enable Qualcomm BCL hardware devicetree binding configuration
>> for pm7250b.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> index 0761e6b5fd8d..69ad76831cde 100644
>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>   			interrupt-controller;
>>   			#interrupt-cells = <2>;
>>   		};
>> +
>> +		bcl@1d00 {
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> If you cannot find a name matching your device, please check in kernel
> sources for similar cases or you can grow the spec (via pull request to
> DT spec repo).
Ack, I will use node as "sensor" as we are enabling hwmon sensors here.
>
> Plus, I doubt this was ever tested. Considering lack of internal review
> I do not think this should be posted.
It is tested on qcs6490-rb3gen2 for pm7250b BCL and pm8350c bcl
>
> Best regards,
> Krzysztof
>

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Konrad Dybcio]

On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Enable Qualcomm BCL hardware devicetree binding configuration
> for pm7250b.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> index 0761e6b5fd8d..69ad76831cde 100644
> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  		};
> +
> +		bcl@1d00 {
> +			compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
> +			reg = <0x1d00>;
> +			interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
> +				     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "bcl-max-min",
> +					  "bcl-critical";

We should strip the "bcl-" prefix, since these interrupts happen
to be under the bcl device

> +			overcurrent-thresholds-milliamp = <5500 6000>;

Is that something that we expect to change between boards, or is
that an electrical characteristic of the PM7250B?

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,

On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Enable Qualcomm BCL hardware devicetree binding configuration
>> for pm7250b.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> index 0761e6b5fd8d..69ad76831cde 100644
>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>   			interrupt-controller;
>>   			#interrupt-cells = <2>;
>>   		};
>> +
>> +		bcl@1d00 {
>> +			compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>> +			reg = <0x1d00>;
>> +			interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>> +				     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>> +			interrupt-names = "bcl-max-min",
>> +					  "bcl-critical";
> We should strip the "bcl-" prefix, since these interrupts happen
> to be under the bcl device
Ack
>
>> +			overcurrent-thresholds-milliamp = <5500 6000>;
> Is that something that we expect to change between boards, or is
> that an electrical characteristic of the PM7250B?
Yes,  It can change based on battery used for that board as these 
thresholds will be close below battery OCP spec.
It is not based on pmic spec. Max current threshold support for specific 
pmic bcl is taken care in driver pmic data.


Thanks,
Manaf
>
> Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Konrad Dybcio]

On 2/13/26 12:55 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Konrad,
> 
> On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> Enable Qualcomm BCL hardware devicetree binding configuration
>>> for pm7250b.
>>>
>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>> ---
>>>   arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>> index 0761e6b5fd8d..69ad76831cde 100644
>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>>               interrupt-controller;
>>>               #interrupt-cells = <2>;
>>>           };
>>> +
>>> +        bcl@1d00 {
>>> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>>> +            reg = <0x1d00>;
>>> +            interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>>> +                     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>>> +            interrupt-names = "bcl-max-min",
>>> +                      "bcl-critical";
>> We should strip the "bcl-" prefix, since these interrupts happen
>> to be under the bcl device
> Ack
>>
>>> +            overcurrent-thresholds-milliamp = <5500 6000>;
>> Is that something that we expect to change between boards, or is
>> that an electrical characteristic of the PM7250B?
> Yes,  It can change based on battery used for that board as these thresholds will be close below battery OCP spec.
> It is not based on pmic spec. Max current threshold support for specific pmic bcl is taken care in driver pmic data.

Okay, so this property must not live in the common PMIC DTSI then..

I think ideally this could be communicated over battmgr, since it already
has a lot of information about the battery that's currently connected to
the device. Do you think that would be reasonable? Would you know who we
could talk to internally?

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,

On 2/16/2026 5:18 PM, Konrad Dybcio wrote:
> On 2/13/26 12:55 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Konrad,
>>
>> On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
>>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>> Enable Qualcomm BCL hardware devicetree binding configuration
>>>> for pm7250b.
>>>>
>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>>>    1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> index 0761e6b5fd8d..69ad76831cde 100644
>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>>>                interrupt-controller;
>>>>                #interrupt-cells = <2>;
>>>>            };
>>>> +
>>>> +        bcl@1d00 {
>>>> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>>>> +            reg = <0x1d00>;
>>>> +            interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>>>> +                     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>>>> +            interrupt-names = "bcl-max-min",
>>>> +                      "bcl-critical";
>>> We should strip the "bcl-" prefix, since these interrupts happen
>>> to be under the bcl device
>> Ack
>>>
>>>> +            overcurrent-thresholds-milliamp = <5500 6000>;
>>> Is that something that we expect to change between boards, or is
>>> that an electrical characteristic of the PM7250B?
>> Yes,  It can change based on battery used for that board as these thresholds will be close below battery OCP spec.
>> It is not based on pmic spec. Max current threshold support for specific pmic bcl is taken care in driver pmic data.
> 
> Okay, so this property must not live in the common PMIC DTSI then..

Ack, I will move it into board file wherever it is required in next revision

> 
> I think ideally this could be communicated over battmgr, since it already
> has a lot of information about the battery that's currently connected to
> the device. Do you think that would be reasonable? Would you know who we
> could talk to internally?

We are not adding any battery information here. This configuration is 
specifically for the BCL peripheral to monitor current and trigger an 
over‑current alarm. While the BCL settings are derived from battery 
specifications, they are not the same as the battery’s own limits,the 
BCL thresholds will always be set below the battery’s OCP specification.
The intent of the BCL is to provide early detection of over‑current or 
under‑voltage conditions, notify the SoC/peripherals, and allow 
corrective action before the system ever reaches the battery’s actual 
protection limits.

Thanks,
Manaf
> 
> Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Konrad Dybcio]

On 2/19/26 12:34 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Konrad,
> 
> On 2/16/2026 5:18 PM, Konrad Dybcio wrote:
>> On 2/13/26 12:55 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> Hi Konrad,
>>>
>>> On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
>>>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>>> Enable Qualcomm BCL hardware devicetree binding configuration
>>>>> for pm7250b.
>>>>>
>>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>>> ---
>>>>>    arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>>>>    1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>> index 0761e6b5fd8d..69ad76831cde 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>>>>                interrupt-controller;
>>>>>                #interrupt-cells = <2>;
>>>>>            };
>>>>> +
>>>>> +        bcl@1d00 {
>>>>> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>>>>> +            reg = <0x1d00>;
>>>>> +            interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>>>>> +                     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>>>>> +            interrupt-names = "bcl-max-min",
>>>>> +                      "bcl-critical";
>>>> We should strip the "bcl-" prefix, since these interrupts happen
>>>> to be under the bcl device
>>> Ack
>>>>
>>>>> +            overcurrent-thresholds-milliamp = <5500 6000>;
>>>> Is that something that we expect to change between boards, or is
>>>> that an electrical characteristic of the PM7250B?
>>> Yes,  It can change based on battery used for that board as these thresholds will be close below battery OCP spec.
>>> It is not based on pmic spec. Max current threshold support for specific pmic bcl is taken care in driver pmic data.
>>
>> Okay, so this property must not live in the common PMIC DTSI then..
> 
> Ack, I will move it into board file wherever it is required in next revision
> 
>>
>> I think ideally this could be communicated over battmgr, since it already
>> has a lot of information about the battery that's currently connected to
>> the device. Do you think that would be reasonable? Would you know who we
>> could talk to internally?
> 
> We are not adding any battery information here. This configuration is specifically for the BCL peripheral to monitor current and trigger an over‑current alarm. While the BCL settings are derived from battery specifications, they are not the same as the battery’s own limits,the BCL thresholds will always be set below the battery’s OCP specification.
> The intent of the BCL is to provide early detection of over‑current or under‑voltage conditions, notify the SoC/peripherals, and allow corrective action before the system ever reaches the battery’s actual protection limits.

Right, but as you say they are derived from the battery spec, I would
guesstimate it's something like "90% max current", so swapping out different
batteries for the same device could potentially affect this value. Since we
already have a place where OEMs are required to add the battery specs, I
would imagine this could be beneficial to move over to battmgr in the future

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,

On 2/19/2026 6:34 PM, Konrad Dybcio wrote:
> On 2/19/26 12:34 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Konrad,
>>
>> On 2/16/2026 5:18 PM, Konrad Dybcio wrote:
>>> On 2/13/26 12:55 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>> Hi Konrad,
>>>>
>>>> On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
>>>>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>>>> Enable Qualcomm BCL hardware devicetree binding configuration
>>>>>> for pm7250b.
>>>>>>
>>>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>>>>>     1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>> index 0761e6b5fd8d..69ad76831cde 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>>>>>                 interrupt-controller;
>>>>>>                 #interrupt-cells = <2>;
>>>>>>             };
>>>>>> +
>>>>>> +        bcl@1d00 {
>>>>>> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>>>>>> +            reg = <0x1d00>;
>>>>>> +            interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>>>>>> +                     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>>>>>> +            interrupt-names = "bcl-max-min",
>>>>>> +                      "bcl-critical";
>>>>> We should strip the "bcl-" prefix, since these interrupts happen
>>>>> to be under the bcl device
>>>> Ack
>>>>>
>>>>>> +            overcurrent-thresholds-milliamp = <5500 6000>;
>>>>> Is that something that we expect to change between boards, or is
>>>>> that an electrical characteristic of the PM7250B?
>>>> Yes,  It can change based on battery used for that board as these thresholds will be close below battery OCP spec.
>>>> It is not based on pmic spec. Max current threshold support for specific pmic bcl is taken care in driver pmic data.
>>>
>>> Okay, so this property must not live in the common PMIC DTSI then..
>>
>> Ack, I will move it into board file wherever it is required in next revision
>>
>>>
>>> I think ideally this could be communicated over battmgr, since it already
>>> has a lot of information about the battery that's currently connected to
>>> the device. Do you think that would be reasonable? Would you know who we
>>> could talk to internally?
>>
>> We are not adding any battery information here. This configuration is specifically for the BCL peripheral to monitor current and trigger an over‑current alarm. While the BCL settings are derived from battery specifications, they are not the same as the battery’s own limits,the BCL thresholds will always be set below the battery’s OCP specification.
>> The intent of the BCL is to provide early detection of over‑current or under‑voltage conditions, notify the SoC/peripherals, and allow corrective action before the system ever reaches the battery’s actual protection limits.
> 
> Right, but as you say they are derived from the battery spec, I would
> guesstimate it's something like "90% max current", so swapping out different
> batteries for the same device could potentially affect this value. Since we

No, there is no predefined equation to derive these values directly from 
the battery specifications. The actual limits depend on multiple 
factors, including the underlying mitigation support available on the 
board for different components (fast path/slow path), the peak‑current 
use cases defined for that board, the battery characteristics etc. 
Simply swapping the battery will not automatically make it work. These 
parameters must be carefully tuned and configured.

I discussed this with the internal battery manager driver team, and they 
mentioned that the power‑supply framework does not expose any 
discharging current spec details. Typically, only 
charging‑current–related information is provided.

For some modern battery use cases, these configurations must also be 
adjusted dynamically based on factors such as ambient temperature and 
battery state of charge. The driver already supports this through the 
hwmon sysfs interface, allowing these values to be overridden at runtime.

Thanks,
Manaf

> already have a place where OEMs are required to add the battery specs, I
> would imagine this could be beneficial to move over to battmgr in the future
> 
> Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Konrad Dybcio]

On 2/24/26 7:35 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Konrad,
> 
> On 2/19/2026 6:34 PM, Konrad Dybcio wrote:
>> On 2/19/26 12:34 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>> Hi Konrad,
>>>
>>> On 2/16/2026 5:18 PM, Konrad Dybcio wrote:
>>>> On 2/13/26 12:55 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>>> Hi Konrad,
>>>>>
>>>>> On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
>>>>>> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>>>>>>> Enable Qualcomm BCL hardware devicetree binding configuration
>>>>>>> for pm7250b.
>>>>>>>
>>>>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>>>>> ---
>>>>>>>     arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>>>>>>     1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>> index 0761e6b5fd8d..69ad76831cde 100644
>>>>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>>>>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>>>>>>                 interrupt-controller;
>>>>>>>                 #interrupt-cells = <2>;
>>>>>>>             };
>>>>>>> +
>>>>>>> +        bcl@1d00 {
>>>>>>> +            compatible = "qcom,pm7250b-bcl", "qcom,bcl-v1";
>>>>>>> +            reg = <0x1d00>;
>>>>>>> +            interrupts = <PM7250B_SID 0x1d 0x0 IRQ_TYPE_EDGE_RISING>,
>>>>>>> +                     <PM7250B_SID 0x1d 0x1 IRQ_TYPE_EDGE_RISING>;
>>>>>>> +            interrupt-names = "bcl-max-min",
>>>>>>> +                      "bcl-critical";
>>>>>> We should strip the "bcl-" prefix, since these interrupts happen
>>>>>> to be under the bcl device
>>>>> Ack
>>>>>>
>>>>>>> +            overcurrent-thresholds-milliamp = <5500 6000>;
>>>>>> Is that something that we expect to change between boards, or is
>>>>>> that an electrical characteristic of the PM7250B?
>>>>> Yes,  It can change based on battery used for that board as these thresholds will be close below battery OCP spec.
>>>>> It is not based on pmic spec. Max current threshold support for specific pmic bcl is taken care in driver pmic data.
>>>>
>>>> Okay, so this property must not live in the common PMIC DTSI then..
>>>
>>> Ack, I will move it into board file wherever it is required in next revision
>>>
>>>>
>>>> I think ideally this could be communicated over battmgr, since it already
>>>> has a lot of information about the battery that's currently connected to
>>>> the device. Do you think that would be reasonable? Would you know who we
>>>> could talk to internally?
>>>
>>> We are not adding any battery information here. This configuration is specifically for the BCL peripheral to monitor current and trigger an over‑current alarm. While the BCL settings are derived from battery specifications, they are not the same as the battery’s own limits,the BCL thresholds will always be set below the battery’s OCP specification.
>>> The intent of the BCL is to provide early detection of over‑current or under‑voltage conditions, notify the SoC/peripherals, and allow corrective action before the system ever reaches the battery’s actual protection limits.
>>
>> Right, but as you say they are derived from the battery spec, I would
>> guesstimate it's something like "90% max current", so swapping out different
>> batteries for the same device could potentially affect this value. Since we
> 
> No, there is no predefined equation to derive these values directly from the battery specifications. The actual limits depend on multiple factors, including the underlying mitigation support available on the board for different components (fast path/slow path), the peak‑current use cases defined for that board, the battery characteristics etc. Simply swapping the battery will not automatically make it work. These parameters must be carefully tuned and configured.

What I had in mind is that if a battery is dual- (or more-) sourced for a
given device by a given vendor, they could perform all that tuning and then
store the data alongside other battery configuration that they presumably
already are required to include in battmgr for all of the specific battery
types they intend to support

> I discussed this with the internal battery manager driver team, and they mentioned that the power‑supply framework does not expose any discharging current spec details. Typically, only charging‑current–related information is provided.

Right, but the power-supply framework happens to be open-source, so if that
would be worth implementing, I think the maintainers would certainly be open
to having that discussion (+Sebastian)


> For some modern battery use cases, these configurations must also be adjusted dynamically based on factors such as ambient temperature and battery state of charge. The driver already supports this through the hwmon sysfs interface, allowing these values to be overridden at runtime.

One could assume that depends on (presumably) proprietary userspace and I
would imagine it'd be generally preferred to keep as much charging infra as
possible inside the kernel (or even better, in some battle-tested FW).

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Konrad Dybcio]

On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
> Enable Qualcomm BCL hardware devicetree binding configuration
> for pm7250b.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> index 0761e6b5fd8d..69ad76831cde 100644
> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  		};
> +
> +		bcl@1d00 {

This should be higher up (the node above is 0xc000, this one is 0x1d00)

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: pm7250b: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Hi Konrad,


On 2/6/2026 2:41 PM, Konrad Dybcio wrote:
> On 2/5/26 10:14 PM, Manaf Meethalavalappu Pallikunhi wrote:
>> Enable Qualcomm BCL hardware devicetree binding configuration
>> for pm7250b.
>>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/pm7250b.dtsi | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> index 0761e6b5fd8d..69ad76831cde 100644
>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>> @@ -202,6 +202,16 @@ pm7250b_gpios: gpio@c000 {
>>   			interrupt-controller;
>>   			#interrupt-cells = <2>;
>>   		};
>> +
>> +		bcl@1d00 {
> This should be higher up (the node above is 0xc000, this one is 0x1d00)
Ack
>
> Konrad

[PATCH 4/4] arm64: dts: qcom: pm8350c: Enable Qualcomm BCL device

[Manaf Meethalavalappu Pallikunhi]

Enable Qualcomm BCL hardware devicetree binding configuration
for pm8350c.

Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/pm8350c.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index 1a24e6439e36..151a02d325c1 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -41,6 +41,15 @@ pm8350c_pwm: pwm {
 			#pwm-cells = <2>;
 			status = "disabled";
 		};
+
+		bcl@4700 {
+			compatible = "qcom,pm8350c-bcl", "qcom,bcl-v2";
+			reg = <0x4700>;
+			interrupts = <0x2 0x47 0x0 IRQ_TYPE_EDGE_RISING>,
+				     <0x2 0x47 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "bcl-max-min",
+					  "bcl-critical";
+		};
 	};
 };
 

-- 
2.43.0