| Message ID | 20240528-cros_ec-charge-control-v2-0-81fb27e1cff4@weissschuh.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | ChromeOS Embedded Controller charge control driver | expand |
On Tue, May 28, 2024 at 3:05 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > Add a power supply driver that supports charge thresholds and behaviour > configuration. > > This is a complete rework of > "platform/chrome: cros_ec_framework_laptop: new driver" [0], which used > Framework specific EC commands. > > The driver propsed in this series only uses upstream CrOS functionality. > > Tested on a Framework 13 AMD, Firmware 3.05. > I've tested this out on the Framework Laptop 13, 11th gen intel core and AMD Ryzen 7040 editions. The problem is that the AMD framework laptop *reports* support for the CrOS charge controller, but it does not truly support it. As with the 11th Gen Intel Core (and by proxy the 12th, 13th) it still does require the OEM-specific command. This is evinced by a mismatch between the firmware-configured value and the value reported by the charge control subsystem through this driver. $ cat /sys/class/power_supply/BAT1/charge_control_end_threshold 100 $ ectool raw 0x3E03 b8 # OEM command 0x3E03 with BIT(3) in the payload is Framework's charge limit query host command Read 2 bytes 50 00 |P. | (in my case, 80 in decimal) The charge limit is managed at [1], and it does not appear to integrate with the standard charge control machinery. I'll pursue getting this board not to report support for CrOS charge control. This driver is still entirely fit for purpose, just not for this board. Cheers, d
On 2024-06-02 18:40:18+0000, Dustin Howett wrote: > On Tue, May 28, 2024 at 3:05 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > > > Add a power supply driver that supports charge thresholds and behaviour > > configuration. > > > > This is a complete rework of > > "platform/chrome: cros_ec_framework_laptop: new driver" [0], which used > > Framework specific EC commands. > > > > The driver propsed in this series only uses upstream CrOS functionality. > > > > Tested on a Framework 13 AMD, Firmware 3.05. > > > > I've tested this out on the Framework Laptop 13, 11th gen intel core > and AMD Ryzen 7040 editions. Thanks! > The problem is that the AMD framework laptop *reports* support for the > CrOS charge controller, but it does not truly support it. > As with the 11th Gen Intel Core (and by proxy the 12th, 13th) it still > does require the OEM-specific command. This is surpising, it works on my machine, which is also a AMD 7040. > This is evinced by a mismatch between the firmware-configured value > and the value reported by the charge control subsystem through this > driver. > > $ cat /sys/class/power_supply/BAT1/charge_control_end_threshold > 100 > > $ ectool raw 0x3E03 b8 # OEM command 0x3E03 with BIT(3) in the payload > is Framework's charge limit query host command > Read 2 bytes > 50 00 |P. | > (in my case, 80 in decimal) > > The charge limit is managed at [1], and it does not appear to > integrate with the standard charge control machinery. > > I'll pursue getting this board not to report support for CrOS charge > control. This driver is still entirely fit for purpose, just not for > this board. Can you try disabling all of the Framework-specific charge control settings and test again? Probably the different, disparate logics in the Framework ECs are conflicting with each other. Thomas
On Mon, Jun 3, 2024 at 3:59 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > Can you try disabling all of the Framework-specific charge control > settings and test again? > Probably the different, disparate logics in the Framework ECs are > conflicting with each other. Fascinating! This board does indeed support charge limiting through both interfaces. It looks like the most recently set one wins for a time. The UEFI setup utility only sets the framework-specific charge limit value. We should probably find some way to converge them, for all of the supported Framework Laptop programs. > Thomas
On 2024-06-04 20:27:57+0000, Dustin Howett wrote: > On Mon, Jun 3, 2024 at 3:59 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > > > Can you try disabling all of the Framework-specific charge control > > settings and test again? > > Probably the different, disparate logics in the Framework ECs are > > conflicting with each other. > > Fascinating! This board does indeed support charge limiting through > both interfaces. It looks like the most recently set one wins for a > time. If it is the most recent one, shouldn't the driver have worked? What does "for a time" mean? I'm using only the upstream EC command and that seems to work fine. > The UEFI setup utility only sets the framework-specific charge limit value. > > We should probably find some way to converge them, for all of the > supported Framework Laptop programs. In the long term, Framework should align their implementation with upstream CrOS EC and either drop their custom command or make it a thin wrapper around the normal the upstream command. (As you are familiar with EC programming maybe you want to tackle this?) Until then I think we can detect at probe-time if the Framework APIs are available and use them to disable the Framework-specific mechanism. Then the CrOS EC commands should be usable. The drawback is, that userspace using the Framework APIs will break the driver. That userspace would need to migrate to the standard UAPI. Also the settings set in the firmware would be ignored at that point. I don't want to use the functionality of the Framework command because it's less featureful and I really hope it will go away at some point.
On 6/5/2024 04:33, Thomas Weißschuh wrote: > On 2024-06-04 20:27:57+0000, Dustin Howett wrote: >> On Mon, Jun 3, 2024 at 3:59 PM Thomas Weißschuh <linux@weissschuh.net> wrote: >>> >>> Can you try disabling all of the Framework-specific charge control >>> settings and test again? >>> Probably the different, disparate logics in the Framework ECs are >>> conflicting with each other. >> >> Fascinating! This board does indeed support charge limiting through >> both interfaces. It looks like the most recently set one wins for a >> time. > > If it is the most recent one, shouldn't the driver have worked? > What does "for a time" mean? > I'm using only the upstream EC command and that seems to work fine. > >> The UEFI setup utility only sets the framework-specific charge limit value. >> >> We should probably find some way to converge them, for all of the >> supported Framework Laptop programs. > > In the long term, Framework should align their implementation with > upstream CrOS EC and either drop their custom command or make it a thin > wrapper around the normal the upstream command. > > (As you are familiar with EC programming maybe you want to tackle this?) > > Until then I think we can detect at probe-time if the Framework APIs are > available and use them to disable the Framework-specific mechanism. > Then the CrOS EC commands should be usable. > > The drawback is, that userspace using the Framework APIs will break > the driver. That userspace would need to migrate to the standard UAPI. How does userspace access the Framework APIs? Surely it needs to go through the kernel? Could you "filter" the userspace calls to block them? For example this is something that currently happens in the dell-pc driver to block userspace from doing thermal calls and instead guide people to the proper API that the driver exports. > > Also the settings set in the firmware would be ignored at that point. > > I don't want to use the functionality of the Framework command because > it's less featureful and I really hope it will go away at some point.
+Matt, the Linux support lead for Framework. Hi Matt, below we are discussing on how to implement charge controls for ChromeOS EC devices including Framework laptops in mainline Linux. Some feedback would be great. On 2024-06-05 15:32:33+0000, Mario Limonciello wrote: > On 6/5/2024 04:33, Thomas Weißschuh wrote: > > On 2024-06-04 20:27:57+0000, Dustin Howett wrote: > > > On Mon, Jun 3, 2024 at 3:59 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > > > > > > > Can you try disabling all of the Framework-specific charge control > > > > settings and test again? > > > > Probably the different, disparate logics in the Framework ECs are > > > > conflicting with each other. > > > > > > Fascinating! This board does indeed support charge limiting through > > > both interfaces. It looks like the most recently set one wins for a > > > time. > > > > If it is the most recent one, shouldn't the driver have worked? > > What does "for a time" mean? > > I'm using only the upstream EC command and that seems to work fine. > > > > > The UEFI setup utility only sets the framework-specific charge limit value. > > > > > > We should probably find some way to converge them, for all of the > > > supported Framework Laptop programs. > > > > In the long term, Framework should align their implementation with > > upstream CrOS EC and either drop their custom command or make it a thin > > wrapper around the normal the upstream command. > > > > (As you are familiar with EC programming maybe you want to tackle this?) > > > > Until then I think we can detect at probe-time if the Framework APIs are > > available and use them to disable the Framework-specific mechanism. > > Then the CrOS EC commands should be usable. > > > > The drawback is, that userspace using the Framework APIs will break > > the driver. That userspace would need to migrate to the standard UAPI. > > How does userspace access the Framework APIs? Surely it needs to go through > the kernel? Could you "filter" the userspace calls to block them? > > For example this is something that currently happens in the dell-pc driver > to block userspace from doing thermal calls and instead guide people to the > proper API that the driver exports. This would work when userspace uses /dev/cros_ec. But the EC can also used via raw port IO which wouldn't be covered. Given that /dev/cros_ec wasn't usable on Framework AMD until v6.9 it's not unlikely users are using that. And technically both aproaches would break userspace. Another aproach would be to not load the module on Framework devices which implement their custom command (overwritable by module parameter). Framework unifies the implementation of their command with the core CrOS EC logic so both commands work on the same data. The custom command is adapted to also implement a new command version. This is completely transparent as the old version will continue to work. We update the Linux driver to recognize that new command version, know that they are now compatible and probe the driver. Newer devices could also drop the custom command and the driver would start working. This scheme requires some cooperation from Framework, though. > > > > Also the settings set in the firmware would be ignored at that point. > > > > I don't want to use the functionality of the Framework command because > > it's less featureful and I really hope it will go away at some point. >
Add a power supply driver that supports charge thresholds and behaviour configuration. This is a complete rework of "platform/chrome: cros_ec_framework_laptop: new driver" [0], which used Framework specific EC commands. The driver propsed in this series only uses upstream CrOS functionality. Tested on a Framework 13 AMD, Firmware 3.05. [0] https://lore.kernel.org/lkml/20240505-cros_ec-framework-v1-0-402662d6276b@weissschuh.net/ Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- Changes in v2: - Accept "0" as charge_start_threshold - Don't include linux/kernel.h - Only bind to the first found battery - Import EC_CMD_CHARGE_CONTROL v3 headers - Add support for v1 and v3 commands - Sort mfd cell entry alphabetically - Link to v1: https://lore.kernel.org/r/20240519-cros_ec-charge-control-v1-0-baf305dc79b8@weissschuh.net --- Thomas Weißschuh (3): platform/chrome: Update binary interface for EC-based charge control power: supply: add ChromeOS EC based charge control driver mfd: cros_ec: Register charge control subdevice MAINTAINERS | 6 + drivers/mfd/cros_ec_dev.c | 1 + drivers/power/supply/Kconfig | 12 + drivers/power/supply/Makefile | 1 + drivers/power/supply/cros_charge-control.c | 353 +++++++++++++++++++++++++ include/linux/platform_data/cros_ec_commands.h | 49 +++- 6 files changed, 420 insertions(+), 2 deletions(-) --- base-commit: e0cce98fe279b64f4a7d81b7f5c3a23d80b92fbc change-id: 20240506-cros_ec-charge-control-685617e8ed87 Best regards,