Re: [PATCH v3 5/6] power: supply: add Huawei Matebook E Go psy driver

[Pengyu Luo]

On Sat, Feb 22, 2025 at 6:22 AM Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> wrote:
> On Fri, Feb 21, 2025 at 02:01:04PM +0800, Pengyu Luo wrote:
> > On Fri, Feb 21, 2025 at 9:33 AM Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> wrote:
> > > On Thu, Feb 20, 2025 at 02:43:20PM +0800, Pengyu Luo wrote:
> > > > On Thu, Feb 20, 2025 at 8:24 AM Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> wrote:
> > > > > On Tue, Jan 14, 2025 at 01:51:27AM +0800, Pengyu Luo wrote:
> > > > > > On the Huawei Matebook E Go tablet the EC provides access to the adapter
> > > > > > and battery status. Add the driver to read power supply status on the
> > > > > > tablet.
> > > > > >
> > > > > > Signed-off-by: Pengyu Luo <mitltlatltl@xxxxxxxxx>
> > > > > > ---
> > > > > >  .../ABI/testing/sysfs-class-power-gaokun      |  47 ++
> > > > > >  drivers/power/supply/Kconfig                  |  10 +
> > > > > >  drivers/power/supply/Makefile                 |   1 +
> > > > > >  drivers/power/supply/huawei-gaokun-battery.c  | 548 ++++++++++++++++++
> > > > > >  4 files changed, 606 insertions(+)
> > > > > >  create mode 100644 Documentation/ABI/testing/sysfs-class-power-gaokun
> > > > > >  create mode 100644 drivers/power/supply/huawei-gaokun-battery.c
> > > > > >
> > > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power-gaokun b/Documentation/ABI/testing/sysfs-class-power-gaokun
> > > > > > new file mode 100644
> > > > > > index 000000000..b1eb9e8d7
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/ABI/testing/sysfs-class-power-gaokun
> > > > > > @@ -0,0 +1,47 @@
> > > > > > +What:                /sys/class/power_supply/gaokun-ec-battery/smart_charge
> > > > > > +Date:                January 2025
> > > > > > +KernelVersion:       6.12
> > > > > > +Contact:     Pengyu Luo <mitltlatltl@xxxxxxxxx>
> > > > > > +Description:
> > > > > > +             This entry allows configuration of smart charging behavior with
> > > > > > +             four parameters. The format is: <mode> <delay> <start> <stop>.
> > > > > > +
> > > > > > +             - mode: Defines the charging mode (1 or 4). Mode 4 enables delay,
> > > > > > +                     while mode 1 does not.
> > > > > > +             - delay: Specifies the delay in hours (non-negative). This is
> > > > > > +                     only used when 'mode' is set to 4.
> > > > > > +             - start: The battery percentage at which charging starts (0-100).
> > > > > > +             - stop: The battery percentage at which charging stops (1-100).
> > > > > > +
> > > > > > +              When the laptop is connected to a power adapter, it starts
> > > > > > +              charging if the battery level is below the 'start' value. It
> > > > > > +              continues charging until the battery reaches the 'stop' level.
> > > > > > +              If the battery is already above the 'stop' level, charging is
> > > > > > +              paused.
> > > > > > +
> > > > > > +              When the power adapter is always connected, charging will
> > > > > > +              begin if the battery level falls below 'start', and charging
> > > > > > +              will stop once the battery reaches 'stop'.
> > > > > > +
> > > > > > +              If mode is set to 4, the above charging mode will only occur
> > > > > > +              after the specified delay in hours. If mode is 1, there is
> > > > > > +              no delay.
> > > > > > +
> > > > > > +             Access: Read, Write
> > > > > > +
> > > > > > +             Valid values:
> > > > > > +                     - mode: integer value (1 or 4)
> > > > > > +                     - delay: integer value, delay in hours (non-negative)
> > > > > > +                     - start: integer value, battery percentage (0-100)
> > > > > > +                     - stop: integer value, battery percentage (1-100)
> > > > >
> > > > > There are common properties for start and stop charging percentage,
> > > > > which should be used:
> > > > >
> > > > > * POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD
> > > > > * POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD
> > > > >
> > > >
> > > > Agree, but at least, we should pass delay, start, end. EC only
> > > > providedone interface to set mode and delay, that requires 4
> > > > arguments, we can handle it with 3 arguments, as you suggested
> > > > below. but if we treat start and end separated, then if we want
> > > > to set smart charge, we set start, set end, set delay(read start
> > > > read end, then set them again). It is a bit redundant.
> > >
> > > Yes, if these are separate properties you won't get atomic updates.
> > > But is that really a problem? Using the standard properties means
> > > that you get UI support in the future. I know at least the GNOME
> > > people are working on this.
> > >
> >
> > On my another x86_64 device with end threshold supported, KDE Plasma
> > supports showing this as
> >
> > > Battery is configured to charge up to aproximately <value>%
> >
> > it doesn't support setting things. So, can I keep passing delay, start,
> > end when setting, but also setting start and end as battery properties?
>
> No? Why should we create a custom sysfs ABI (which also breaks the
> one value per file rule), if we already have a standard ABI?
>

I got it, I will follow it. Since V4, I had dropped this driver to
focus on upstreaming the base EC driver, once the base driver is
upstreamed, I will send the new version of battery driver.

Best wishes,
Pengyu