| Message ID | 20190527022258.32748-3-matheus@castello.eng.br (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | None | expand |
On Mon, 27 May 2019 at 04:45, Matheus Castello <matheus@castello.eng.br> wrote: > > For configure low level state of charge threshold alert signaled from > max17040 we add "maxim,alert-low-soc-level" property. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > --- > .../power/supply/max17040_battery.txt | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt > > diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt > new file mode 100644 > index 000000000000..a13e8d50ff7b > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt > @@ -0,0 +1,28 @@ > +max17040_battery > +~~~~~~~~~~~~~~~~ > + > +Required properties : > + - compatible : "maxim,max17040" or "maxim,max77836-battery" > + > +Optional properties : > +- maxim,alert-low-soc-level : The alert threshold that sets the state of > + charge level (%) where an interrupt is > + generated. Can be configured from 1 up to 32 > + (%). If skipped the power up default value of > + 4 (%) will be used. > +- interrupts : Interrupt line see Documentation/devicetree/ > + bindings/interrupt-controller/interrupts.txt Based on driver's behavior, I understand that these two properties come in pair so maxim,alert-low-soc-level (or its default value) will be used if and only if interrupts property is present. Maybe mention this? In general looks fine to me: Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof > +- wakeup-source : This device has wakeup capabilities. Use this > + property to use alert low SOC level interrupt > + as wake up source. > + > +Example: > + > + battery-fuel-gauge@36 { > + compatible = "maxim,max17040"; > + reg = <0x36>; > + maxim,alert-low-soc-level = <10>; > + interrupt-parent = <&gpio7>; > + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; > + wakeup-source; > + }; > -- > 2.20.1 >
On Mon, 27 May 2019 at 04:45, Matheus Castello <matheus@castello.eng.br> wrote: > > For configure low level state of charge threshold alert signaled from > max17040 we add "maxim,alert-low-soc-level" property. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > --- > .../power/supply/max17040_battery.txt | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt > > diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt > new file mode 100644 > index 000000000000..a13e8d50ff7b > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt > @@ -0,0 +1,28 @@ > +max17040_battery > +~~~~~~~~~~~~~~~~ > + > +Required properties : > + - compatible : "maxim,max17040" or "maxim,max77836-battery" One more comment. The datasheet for max17040 says that there is on ALERT pin and ALERT bits in RCOMP register. Which device are you using? If it turns out that max17040 does not support it, then the driver and bindings should reflect this - interrupts should not be set on max17040. Best regards, Krzysztof
> On Mon, 27 May 2019 at 04:45, Matheus Castello <matheus@castello.eng.br> wrote: >> >> For configure low level state of charge threshold alert signaled from >> max17040 we add "maxim,alert-low-soc-level" property. >> >> Signed-off-by: Matheus Castello <matheus@castello.eng.br> >> --- >> .../power/supply/max17040_battery.txt | 28 +++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt >> >> diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt >> new file mode 100644 >> index 000000000000..a13e8d50ff7b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt >> @@ -0,0 +1,28 @@ >> +max17040_battery >> +~~~~~~~~~~~~~~~~ >> + >> +Required properties : >> + - compatible : "maxim,max17040" or "maxim,max77836-battery" > > One more comment. The datasheet for max17040 says that there is on > ALERT pin and ALERT bits in RCOMP register. Which device are you > using? If it turns out that max17040 does not support it, then the > driver and bindings should reflect this - interrupts should not be set > on max17040. > Yes you are right, max17040 have no ALERT pin. I am using max17043. Let me know what you think would be best, put a note about it in the description, add a compatibles like "maxim,max17043" and "maxim,max17044"? What do you think? Best Regards, Matheus Castello > Best regards, > Krzysztof >
On Sun, 2 Jun 2019 at 23:42, Matheus Castello <matheus@castello.eng.br> wrote: > > > On Mon, 27 May 2019 at 04:45, Matheus Castello <matheus@castello.eng.br> wrote: > >> > >> For configure low level state of charge threshold alert signaled from > >> max17040 we add "maxim,alert-low-soc-level" property. > >> > >> Signed-off-by: Matheus Castello <matheus@castello.eng.br> > >> --- > >> .../power/supply/max17040_battery.txt | 28 +++++++++++++++++++ > >> 1 file changed, 28 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt > >> > >> diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt > >> new file mode 100644 > >> index 000000000000..a13e8d50ff7b > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt > >> @@ -0,0 +1,28 @@ > >> +max17040_battery > >> +~~~~~~~~~~~~~~~~ > >> + > >> +Required properties : > >> + - compatible : "maxim,max17040" or "maxim,max77836-battery" > > > > One more comment. The datasheet for max17040 says that there is on > > ALERT pin and ALERT bits in RCOMP register. Which device are you > > using? If it turns out that max17040 does not support it, then the > > driver and bindings should reflect this - interrupts should not be set > > on max17040. > > > > Yes you are right, max17040 have no ALERT pin. I am using max17043. Let > me know what you think would be best, put a note about it in the > description, add a compatibles like "maxim,max17043" and > "maxim,max17044"? What do you think? Usually in such case driver should behave differently for different devices. This difference is chosen by compatible. Since there is already max77836 compatible - which has the ALERT pin (probably it just includes 17043 fuel gauge) - you can customize it. No need for new compatible, unless it stops working on max77836... Anyway, configuring interrupts on max17040 would be probably harmless but still it is not really correct. The registers should not be touched in such case. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt new file mode 100644 index 000000000000..a13e8d50ff7b --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt @@ -0,0 +1,28 @@ +max17040_battery +~~~~~~~~~~~~~~~~ + +Required properties : + - compatible : "maxim,max17040" or "maxim,max77836-battery" + +Optional properties : +- maxim,alert-low-soc-level : The alert threshold that sets the state of + charge level (%) where an interrupt is + generated. Can be configured from 1 up to 32 + (%). If skipped the power up default value of + 4 (%) will be used. +- interrupts : Interrupt line see Documentation/devicetree/ + bindings/interrupt-controller/interrupts.txt +- wakeup-source : This device has wakeup capabilities. Use this + property to use alert low SOC level interrupt + as wake up source. + +Example: + + battery-fuel-gauge@36 { + compatible = "maxim,max17040"; + reg = <0x36>; + maxim,alert-low-soc-level = <10>; + interrupt-parent = <&gpio7>; + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; + wakeup-source; + };
For configure low level state of charge threshold alert signaled from max17040 we add "maxim,alert-low-soc-level" property. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- .../power/supply/max17040_battery.txt | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/max17040_battery.txt -- 2.20.1