Re: [PATCH 0/2] platform/chrome: cros_ec_framework_laptop: new driver

[Dustin Howett]

On Mon, May 6, 2024 at 12:43 PM Thomas Weißschuh <linux@xxxxxxxxxxxxxx> wrote:
>
> On 2024-05-06 08:09:07+0000, Limonciello, Mario wrote:
> >
> >
> > On 5/6/2024 1:09 AM, Thomas Weißschuh wrote:
> > > On 2024-05-05 22:56:33+0000, Thomas Weißschuh wrote:
> > > > Framework Laptops are using embedded controller firmware based on the
> > > > ChromeOS EC project.
> > > > In addition to the standard upstream commands some vendor-specific
> > > > commands are implemented.
> > > >
> > > > Add a driver that implements battery charge thresholds using these
> > > > custom commands.
> > >
> > > It turns out that standard ChromesOS EC defines EC_CMD_CHARGE_CONTROL.
> > > The kernel headers however only define v1 of the protocol, which is very
> > > limited.
> > >
> > > But in the upstream firmware repo there is a v3 which is much better.
> > >
> > > The Framework laptop only implements v2 which is also fine.
> > > Given that v3 was only introduced late last year, it seems better to
> > > stick to v2 anyways for now.
> > >
> > > So please disregard Patch 2, I'll see on how to use this via a normal
> > > cros_ec driver.
> > >
> > > There are some other Framework-only features that will use Patch 1,
> > > so feedback for that would still be good.
> >
> > What other kinds of features do you have in mind?
>

Definitely privacy switch reporting belongs in a driver like this.

Overall, I'm not sure about making it a subjugate driver under the
cros_ec_mfd virtual "bus"... even though a lot of the features take a
dependency on cros_ec.
Doing so centralizes the work in the platform-chrome tree and may
serve as a guidepost for any future laptop OEMs that derive their
embedded controller firmware from ChromeOS's.
If the owners of this tree sign off on that, that's awesome! I'd be
concerned about making it all their responsibility.

I may be a bit biased, as I have been working on a driver of my own[1]
for this purpose. It currently supports battery charge limiting[3],
reporting fan speed via hwmon, the keyboard backlight[2], and has an
open pull request that exposes the status of the privacy switches.

It is destined--once I find the time to clean it up--for
drivers\platforms\x86 instead of ...\chrome.

This may be a good place for us to combine our efforts!
d

[1] https://github.com/DHowett/framework-laptop-kmod
[2] I found that the Azalea did not report its keyboard backlight
values through the standard cros ec KBLIGHT interface like hx20/30
did, so the driver as it stands implements a fallback that uses the
raw PWM state. I'm sure that you'd've noticed this if it was still
true... so I am always happy to drop an unnecessary workaround. :)
[3] Which I believe still requires a special host command and is not
integrated into the charge manager, at least as of Azalea/Lotus and
_definitely_ not as of hx20/30!

> [...]
>
> > [..]
>
> Thomas