Re: [PATCH v3 2/5] dt-bindings: power: supply: Max17040: Add low level SOC alert threshold

[Krzysztof Kozlowski]

On Sun, 2 Jun 2019 at 23:42, Matheus Castello <matheus@xxxxxxxxxxxxxxx> wrote:
>
> > On Mon, 27 May 2019 at 04:45, Matheus Castello <matheus@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> >> ---
> >>   .../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