Hallo Krzysztof, > > On Tue, Dec 10, 2024 at 09:23:43AM +0000, Stefan Raufhake wrote: > > From: Stefan Raufhake <s.raufhake@xxxxxxxxxxx> > > > > Some GPIO-controlled power supplies can be turned off (charging disabled). > > Support changing the charging state by setting charge_type to > > POWER_SUPPLY_CHARGE_TYPE_STANDARD and disabling charging by setting > > charge_type to POWER_SUPPLY_CHARGE_TYPE_NONE. One potential use case for > > this is disabling battery backup on a UPS. > > > > Signed-off-by: Stefan Raufhake <s.raufhake@xxxxxxxxxxx> > > --- > > .../bindings/power/supply/gpio-charger.yaml | 6 +++ > > drivers/power/supply/gpio-charger.c | 43 +++++++++++++++++++ > > 2 files changed, 49 insertions(+) > > > > <form letter> > This is a friendly reminder during the review process. > > It seems my or other reviewer's previous comments were not fully > addressed. Maybe the feedback got lost between the quotes, maybe you > just forgot to apply it. Please go back to the previous discussion and > either implement all requested changes or keep discussing them. > > Thank you. > </form letter> Sorry, it seems I made a mistake during the patch review process. Should I reply to your email about version 1 of the patch or only about version 2? I don't want to make another mistake and open two discussions at the same time. I hope to do better in the future. > > > diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml > > index 89f8e2bcb2d7..084520bfc040 100644 > > --- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml > > +++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml > > @@ -44,6 +44,10 @@ properties: > > maxItems: 32 > > description: GPIOs used for current limiting > > > > + enable-gpios: > > + maxItems: 1 > > + description: GPIO is used to enable/disable the charger > > + > > You did not respond to my comments, nothing improved. Without > explanation based on hardware - which I asked - this is still a no. > > Implement and respond fully to previous feedback. > > Best regards, > Krzysztof > Sorry, I'm new to this and don't really know what exactly you want for the hardware description and how best to represent our hardware in dts. For the gpio power supply, it can basically be any circuit that implements a "fully charged" GPIO and a "disable ups" GPIO. We're using a circuit built around the LTC3350 (super capacitor ups chip): We use this pin to indicate that our UPS is fully charged (once the input is gone, it's not fully charged anymore): PFO (pin 38): Power-Fail Status Output. This open-drain output is pulled low when a power failure has occurred. For the "disable ups" GPIO, we have some external circuitry around the LTC3350. I can't share the schematic, but it boils down to "disable usage of ups" so that the device shuts down immediately when power is lost. We've implemented this in many of our devices, but first we're looking at [1] and [2], which we also want to upstream the device trees for. [1] https://www.beckhoff.com/en-en/products/ipc/embedded-pcs/cx9240-arm-r-cortex-r-a53/cx9240.html [2] https://www.beckhoff.com/en-en/products/ipc/embedded-pcs/cx8200-arm-r-cortex-r-a53/cx8200.html For the LTC3350, there is a separate driver posted to the Linux kernel mail list [3] by another devolper that we would like to use in the future, but without this gpio, our circuit won't work. [3] https://lore.kernel.org/all/?q=power%3A+supply%3A+ltc3350-charger Best regards, Stefan
