diff mbox series

dt-bindings: power: supply: sc27xx-fg: add low voltage alarm IRQ

Message ID Zr3SAHlq5A78QvrW@standask-GA-A55M-S2HP (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series dt-bindings: power: supply: sc27xx-fg: add low voltage alarm IRQ | expand

Commit Message

Stanislav Jakubek Aug. 15, 2024, 10:01 a.m. UTC
The SC27XX fuel gauge supports a low voltage alarm IRQ, which is used
for more accurate battery capacity measurements with lower voltages.

This was unfortunately never documented in bindings, do so now.

Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
---
Initial Linux driver submission adding this feature:
https://lore.kernel.org/lkml/ee1dd39f126bd03fb88381de9663d32df994d341.1542185618.git.baolin.wang@linaro.org/

The only in-tree user (sc2731.dtsi) has had interrupts specified since its
initial fuel-gauge submission:
https://lore.kernel.org/lkml/4f66af3b47ba241380f8092e08879aca6d7c35b3.1548052878.git.baolin.wang@linaro.org/

 .../devicetree/bindings/power/supply/sc27xx-fg.yaml         | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Conor Dooley Aug. 15, 2024, 2:46 p.m. UTC | #1
On Thu, Aug 15, 2024 at 12:01:36PM +0200, Stanislav Jakubek wrote:
> The SC27XX fuel gauge supports a low voltage alarm IRQ, which is used
> for more accurate battery capacity measurements with lower voltages.
> 
> This was unfortunately never documented in bindings, do so now.
> 
> Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
> ---
> Initial Linux driver submission adding this feature:
> https://lore.kernel.org/lkml/ee1dd39f126bd03fb88381de9663d32df994d341.1542185618.git.baolin.wang@linaro.org/
> 
> The only in-tree user (sc2731.dtsi) has had interrupts specified since its
> initial fuel-gauge submission:
> https://lore.kernel.org/lkml/4f66af3b47ba241380f8092e08879aca6d7c35b3.1548052878.git.baolin.wang@linaro.org/

This context could go into the commit message I think, as justification
for making the interrupt required.

Also, this binding is odd in that it has several compatibles in an enum,
but the driver (added at the same time) only has one compatible in it.
Are you using the sc2731 in your device?

> 
>  .../devicetree/bindings/power/supply/sc27xx-fg.yaml         | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml b/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml
> index de43e45a43b7..9108a2841caf 100644
> --- a/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml
> @@ -27,6 +27,9 @@ properties:
>    battery-detect-gpios:
>      maxItems: 1
>  
> +  interrupts:
> +    maxItems: 1
> +
>    io-channels:
>      items:
>        - description: Battery Temperature ADC
> @@ -53,6 +56,7 @@ required:
>    - compatible
>    - reg
>    - battery-detect-gpios
> +  - interrupts
>    - io-channels
>    - io-channel-names
>    - nvmem-cells
> @@ -88,6 +92,8 @@ examples:
>          compatible = "sprd,sc2731-fgu";
>          reg = <0xa00>;
>          battery-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
> +        interrupt-parent = <&sc2731_pmic>;
> +        interrupts = <4>;
>          io-channels = <&pmic_adc 5>, <&pmic_adc 14>;
>          io-channel-names = "bat-temp", "charge-vol";
>          nvmem-cells = <&fgu_calib>;
> -- 
> 2.34.1
>
Stanislav Jakubek Aug. 16, 2024, 6:58 a.m. UTC | #2
Hi Conor,

On Thu, Aug 15, 2024 at 03:46:18PM +0100, Conor Dooley wrote:
> On Thu, Aug 15, 2024 at 12:01:36PM +0200, Stanislav Jakubek wrote:
> > The SC27XX fuel gauge supports a low voltage alarm IRQ, which is used
> > for more accurate battery capacity measurements with lower voltages.
> > 
> > This was unfortunately never documented in bindings, do so now.
> > 
> > Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
> > ---
> > Initial Linux driver submission adding this feature:
> > https://lore.kernel.org/lkml/ee1dd39f126bd03fb88381de9663d32df994d341.1542185618.git.baolin.wang@linaro.org/
> > 
> > The only in-tree user (sc2731.dtsi) has had interrupts specified since its
> > initial fuel-gauge submission:
> > https://lore.kernel.org/lkml/4f66af3b47ba241380f8092e08879aca6d7c35b3.1548052878.git.baolin.wang@linaro.org/
> 
> This context could go into the commit message I think, as justification
> for making the interrupt required.

TBH I'm not 100% sure that the interrupt is required, I just looked at
the Linux driver, and that it returns from probe if it doesn't get the IRQ.

> 
> Also, this binding is odd in that it has several compatibles in an enum,
> but the driver (added at the same time) only has one compatible in it.

I think the intent was to document the entire sc27xx series of PMICs, as
they're supposedly very similar (this is just my guess), while initially
adding support only for sc2731.

> Are you using the sc2731 in your device?

No, I do not have any such device.

Regards,
Stanislav
Conor Dooley Aug. 16, 2024, 4:04 p.m. UTC | #3
On Fri, Aug 16, 2024 at 08:58:43AM +0200, Stanislav Jakubek wrote:
> Hi Conor,
> 
> On Thu, Aug 15, 2024 at 03:46:18PM +0100, Conor Dooley wrote:
> > On Thu, Aug 15, 2024 at 12:01:36PM +0200, Stanislav Jakubek wrote:
> > > The SC27XX fuel gauge supports a low voltage alarm IRQ, which is used
> > > for more accurate battery capacity measurements with lower voltages.
> > > 
> > > This was unfortunately never documented in bindings, do so now.
> > > 
> > > Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
> > > ---
> > > Initial Linux driver submission adding this feature:
> > > https://lore.kernel.org/lkml/ee1dd39f126bd03fb88381de9663d32df994d341.1542185618.git.baolin.wang@linaro.org/
> > > 
> > > The only in-tree user (sc2731.dtsi) has had interrupts specified since its
> > > initial fuel-gauge submission:
> > > https://lore.kernel.org/lkml/4f66af3b47ba241380f8092e08879aca6d7c35b3.1548052878.git.baolin.wang@linaro.org/
> > 
> > This context could go into the commit message I think, as justification
> > for making the interrupt required.
> 
> TBH I'm not 100% sure that the interrupt is required, I just looked at
> the Linux driver, and that it returns from probe if it doesn't get the IRQ.
> 
> > 
> > Also, this binding is odd in that it has several compatibles in an enum,
> > but the driver (added at the same time) only has one compatible in it.
> 
> I think the intent was to document the entire sc27xx series of PMICs, as
> they're supposedly very similar (this is just my guess), while initially
> adding support only for sc2731.
> 
> > Are you using the sc2731 in your device?
> 
> No, I do not have any such device.

No worries.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Sebastian Reichel Aug. 27, 2024, 4:25 p.m. UTC | #4
On Thu, 15 Aug 2024 12:01:36 +0200, Stanislav Jakubek wrote:
> The SC27XX fuel gauge supports a low voltage alarm IRQ, which is used
> for more accurate battery capacity measurements with lower voltages.
> 
> This was unfortunately never documented in bindings, do so now.
> 
> 

Applied, thanks!

[1/1] dt-bindings: power: supply: sc27xx-fg: add low voltage alarm IRQ
      commit: 17656d2215c3978bbe2811a5e249cd07fe3de77f

Best regards,
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml b/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml
index de43e45a43b7..9108a2841caf 100644
--- a/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml
+++ b/Documentation/devicetree/bindings/power/supply/sc27xx-fg.yaml
@@ -27,6 +27,9 @@  properties:
   battery-detect-gpios:
     maxItems: 1
 
+  interrupts:
+    maxItems: 1
+
   io-channels:
     items:
       - description: Battery Temperature ADC
@@ -53,6 +56,7 @@  required:
   - compatible
   - reg
   - battery-detect-gpios
+  - interrupts
   - io-channels
   - io-channel-names
   - nvmem-cells
@@ -88,6 +92,8 @@  examples:
         compatible = "sprd,sc2731-fgu";
         reg = <0xa00>;
         battery-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
+        interrupt-parent = <&sc2731_pmic>;
+        interrupts = <4>;
         io-channels = <&pmic_adc 5>, <&pmic_adc 14>;
         io-channel-names = "bat-temp", "charge-vol";
         nvmem-cells = <&fgu_calib>;