| Message ID | 20240505-cros_ec-framework-v1-0-402662d6276b@weissschuh.net |
|---|---|
| Headers | show |
| Series | platform/chrome: cros_ec_framework_laptop: new driver | expand |
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? There are at least fan and privacy switch reporting in which im interested. But fan reporting also has a standard command which I'll try to use first. And then there are many others I'm personally not interested in: Fingerprint LED configuration, Intrusion Detection, "Standalone mode", some diagnostics, etc. Especially as they can also be handled from userspace. > Considering your above finding I think it's better to put patch 1 into the > series of "other framework only features" that will use it so it's clearer > if it's the best way or not. Agreed, that is what I tried to express. > [..] Thomas
On Mon, May 6, 2024 at 12:43 PM Thomas Weißschuh <linux@weissschuh.net> 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
Hi Dustin, On 2024-05-06 13:29:32+0000, Dustin Howett wrote: > On Mon, May 6, 2024 at 12:43 PM Thomas Weißschuh <linux@weissschuh.net> 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. If it can't be done via one of the upstream CrOS EC commands, surely. > 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. Yes, some guidance from the maintainers will be great. > 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. I have taken a look at that driver but wasn't fond of the fact that it is not using cros_ec mfd. Taking a reference on a completely different device looks iffy to me and in violation of the device hierarchy. FYI I have completely non-Framework-specific implementations for keyboard backlight [0], charge limiting [1] and hwmon [2]. (I didn't look at the privacy switches yet, maybe there is a generic solution) (I'm currently polishing [1] and [2], any feedback already would also be much appreciated) All of them work correctly on my Framework 13 AMD, Firmware 3.05. These standard APIs are more powerful than the Framework-only ones. Charge control can do start_threshold, stop_threshold and charge_behaviour. Hwmon can do fans and temperature sensors. Keyboard backlight just reuses the existing mainline driver. > 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! Surely! Personally I only have the AMD 13 device (Azalea), so I can't test anything else. And I'd like to focus on the mainline-compatible APIs (first). Feel free to contact me (privately?) if you have any suggestions. > [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. :) For me the posted driver under [0] works as expected. > [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! This also works for me correctly with [1]. Do you know if there are plans by Framework to move the older devices to a newer firmware? This would also make their own maintenance work easier in the future, especially considering their commitment to software longevity[3]. > [..] Thomas [0] https://lore.kernel.org/lkml/20240505-cros_ec-kbd-led-framework-v1-1-bfcca69013d2@weissschuh.net/ [1] https://git.sr.ht/~t-8ch/linux/tree/b4/cros_ec-charge-control [2] https://git.sr.ht/~t-8ch/linux/tree/b4/cros_ec-hwmon [3] https://frame.work/de/en/blog/enabling-software-longevity
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. Patch 1 adds the general scaffolding and device binding. Patch 2 implements the battery charge thresholds. This series is based on https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- Thomas Weißschuh (2): platform/chrome: cros_ec_framework_laptop: introduce driver platform/chrome: cros_ec_framework_laptop: implement battery charge thresholds MAINTAINERS | 5 + drivers/mfd/cros_ec_dev.c | 13 ++ drivers/platform/chrome/Kconfig | 11 ++ drivers/platform/chrome/Makefile | 1 + drivers/platform/chrome/cros_ec_framework_laptop.c | 173 +++++++++++++++++++++ 5 files changed, 203 insertions(+) --- base-commit: 2fbe479c0024e1c6b992184a799055e19932aa48 change-id: 20240505-cros_ec-framework-10e627c46a0a Best regards,