| Message ID | 20240403004713.130365-1-dustin@howett.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops | expand |
On 2024-04-02 19:47:09-0500, Dustin L. Howett wrote: > This patch series adds support for the AMD models of the Framework > Laptop (both 13" and 16"). > > These models of Framework Laptop have switched to an NPCX embedded > controller, which uses the original Chrome EC linear memory-mapped I/O > model. > > However, these devices are not configured in a way that is compatible > with the cros_ec driver. Instead of mapping EC memory to I/O ports > [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF]. > > To address this difference without impacting cros_ec_lpcs' compatibility > with Chromebook/Chromebox devices or the existing Framework Laptop > platform, these patches add DMI match-specific driver data through which > we can detect per-machine "quirks". > > Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the > memory MMIO window. > > Changes in v2: > * Separated Framework Laptop (Intel) and Framework Laptop (AMD) > * dev_warn has been demoted to dev_info > * Style fixes > * Reworded the individual patch bodies for clarity and style > > Changes in v3: > * Addressed dev_warn feedback from Thomas Weissschuh > * Removed the host command region quirk; Framework Computer is slated to > release a firmware update that addresses the I/O region issue from > this series' v1 and v2. This leaves just one quirk, with the potential > for future quirks (e.g. changes required to reduce EC bus contention > on the Intel Framework Laptops) For me this works with todays 3.0.5 firmware. > Dustin L. Howett (4): > platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc > device > platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device > platform/chrome: cros_ec_lpc: add a "quirks" system > platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop > (AMD) > > drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++--- > 1 file changed, 74 insertions(+), 7 deletions(-) For the series: Reviewed-and-tested-by: Thomas Weißschuh <linux@weissschuh.net> Note: For new revisions it makes sense to Cc people who interacted with the previous ones. They are likely to have an interest and provide feedback. I only saw this by accident.
On Thu, Apr 4, 2024 at 1:53 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > On 2024-04-02 19:47:09-0500, Dustin L. Howett wrote: > > This patch series adds support for the AMD models of the Framework > > For me this works with todays 3.0.5 firmware. Thank you for testing! I've been seeing an issue where this module--even with these fixes--is not being loaded with the right DMI match table when it is automatically detected on boot. My running theory is that it is matching the less-specific entry for "Framework" + "Laptop" with no quirks. I've been looking into making the matches more specific for all Framework laptops, to future proof this change as well as make it work properly via automatic loading. > > For the series: > > Reviewed-and-tested-by: Thomas Weißschuh <linux@weissschuh.net> Thanks! I wouldn't land this changeset until after we can diagnose the load on boot issue. > > > Note: > For new revisions it makes sense to Cc people who interacted with > the previous ones. They are likely to have an interest and provide > feedback. > I only saw this by accident. I'm sorry about that. I'll keep it in mind for the future. I also think I messed up by replying to the v2 thread with the v3 series, as I realize I have not seen anybody else do this. :) d
On 2024-04-04 13:59:53-0500, Dustin Howett wrote: > On Thu, Apr 4, 2024 at 1:53 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > On 2024-04-02 19:47:09-0500, Dustin L. Howett wrote: > > > This patch series adds support for the AMD models of the Framework > > > > For me this works with todays 3.0.5 firmware. > > Thank you for testing! I've been seeing an issue where this > module--even with these fixes--is not being loaded with the right DMI > match table when it is automatically detected on boot. > > My running theory is that it is matching the less-specific entry for > "Framework" + "Laptop" with no quirks. > I've been looking into making the matches more specific for all > Framework laptops, to future proof this change as well as make it work > properly via automatic loading. This doesn't sound correct. The code is using dmi_first_match() which always iterates the dmi_id table in order. I'll try to reproduce this issue. FYI in the case when probing didn't work you can check the current DMI information in /sys/class/dmi/id/. > > For the series: > > > > Reviewed-and-tested-by: Thomas Weißschuh <linux@weissschuh.net> > > Thanks! > > I wouldn't land this changeset until after we can diagnose the load on > boot issue. Sounds reasonable. > > Note: > > For new revisions it makes sense to Cc people who interacted with > > the previous ones. They are likely to have an interest and provide > > feedback. > > I only saw this by accident. > > I'm sorry about that. I'll keep it in mind for the future. I also > think I messed up by replying to the v2 thread with the v3 series, as > I realize I have not seen anybody else do this. :) You could use b4 which can take care of this. It can also read additional recipients from its cover letter, so you don't have to manually keep track of them for each submission. Thomas
On Thu, Apr 4, 2024 at 2:57 PM Thomas Weißschuh <linux@weissschuh.net> wrote: > > This doesn't sound correct. The code is using dmi_first_match() which > always iterates the dmi_id table in order. > > I'll try to reproduce this issue. No need! It turns out that I was tricked by my own initramfs. Good tip not to let your distribution scripts pack modules you're actively working on. :) In that case: This series is ready to land! > You could use b4 which can take care of this. > It can also read additional recipients from its cover letter, so you > don't have to manually keep track of them for each submission. Excellent! Excited to use it for the next salvo. Thanks. d
On 4/2/24 19:47, Dustin L. Howett wrote: > This patch series adds support for the AMD models of the Framework > Laptop (both 13" and 16"). > > These models of Framework Laptop have switched to an NPCX embedded > controller, which uses the original Chrome EC linear memory-mapped I/O > model. > > However, these devices are not configured in a way that is compatible > with the cros_ec driver. Instead of mapping EC memory to I/O ports > [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF]. > > To address this difference without impacting cros_ec_lpcs' compatibility > with Chromebook/Chromebox devices or the existing Framework Laptop > platform, these patches add DMI match-specific driver data through which > we can detect per-machine "quirks". > > Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the > memory MMIO window. > > Changes in v2: > * Separated Framework Laptop (Intel) and Framework Laptop (AMD) > * dev_warn has been demoted to dev_info > * Style fixes > * Reworded the individual patch bodies for clarity and style > > Changes in v3: > * Addressed dev_warn feedback from Thomas Weissschuh > * Removed the host command region quirk; Framework Computer is slated to > release a firmware update that addresses the I/O region issue from > this series' v1 and v2. This leaves just one quirk, with the potential > for future quirks (e.g. changes required to reduce EC bus contention > on the Intel Framework Laptops) > > > Dustin L. Howett (4): > platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc > device > platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device > platform/chrome: cros_ec_lpc: add a "quirks" system > platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop > (AMD) > > drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++--- > 1 file changed, 74 insertions(+), 7 deletions(-) > I tested on top of a Framework 13 AMD running BIOS 03.05. I found that there are some errors shown related to charging abilities for the PD controller. Should cros-usbpd-charger support be quirked out when run on a FW laptop? [ 5.545352] cros-usbpd-charger cros-usbpd-charger.4.auto: No USB PD charging ports found [ 5.546722] cros-usbpd-charger cros-usbpd-charger.4.auto: Unexpected number of charge port count [ 5.546730] cros-usbpd-charger cros-usbpd-charger.4.auto: Failing probe (err:0xffffffb9) [ 5.546735] cros-usbpd-charger cros-usbpd-charger.4.auto: probe with driver cros-usbpd-charger failed with error -71
On 4/6/24 13:48, Mario Limonciello wrote: > > > On 4/2/24 19:47, Dustin L. Howett wrote: >> This patch series adds support for the AMD models of the Framework >> Laptop (both 13" and 16"). >> >> These models of Framework Laptop have switched to an NPCX embedded >> controller, which uses the original Chrome EC linear memory-mapped I/O >> model. >> >> However, these devices are not configured in a way that is compatible >> with the cros_ec driver. Instead of mapping EC memory to I/O ports >> [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF]. >> >> To address this difference without impacting cros_ec_lpcs' compatibility >> with Chromebook/Chromebox devices or the existing Framework Laptop >> platform, these patches add DMI match-specific driver data through which >> we can detect per-machine "quirks". >> >> Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the >> memory MMIO window. >> >> Changes in v2: >> * Separated Framework Laptop (Intel) and Framework Laptop (AMD) >> * dev_warn has been demoted to dev_info >> * Style fixes >> * Reworded the individual patch bodies for clarity and style >> >> Changes in v3: >> * Addressed dev_warn feedback from Thomas Weissschuh >> * Removed the host command region quirk; Framework Computer is slated to >> release a firmware update that addresses the I/O region issue from >> this series' v1 and v2. This leaves just one quirk, with the potential >> for future quirks (e.g. changes required to reduce EC bus contention >> on the Intel Framework Laptops) >> >> >> Dustin L. Howett (4): >> platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc >> device >> platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device >> platform/chrome: cros_ec_lpc: add a "quirks" system >> platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop >> (AMD) >> >> drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++--- >> 1 file changed, 74 insertions(+), 7 deletions(-) >> > > I tested on top of a Framework 13 AMD running BIOS 03.05. > > I found that there are some errors shown related to charging abilities > for the PD controller. Should cros-usbpd-charger support be quirked out > when run on a FW laptop? > > [ 5.545352] cros-usbpd-charger cros-usbpd-charger.4.auto: No USB PD > charging ports found > [ 5.546722] cros-usbpd-charger cros-usbpd-charger.4.auto: Unexpected > number of charge port count > [ 5.546730] cros-usbpd-charger cros-usbpd-charger.4.auto: Failing > probe (err:0xffffffb9) > [ 5.546735] cros-usbpd-charger cros-usbpd-charger.4.auto: probe with > driver cros-usbpd-charger failed with error -71 > They're all stemming from a missing return statement when no ports were found. It shouldn't block this series which otherwise works fine. Tested-by: Mario Limonciello <superm1@gmail.com> I'll send out another patch to add that return code which can be discussed separately.
Hello: This series was applied to chrome-platform/linux.git (for-kernelci) by Tzung-Bi Shih <tzungbi@kernel.org>: On Tue, 2 Apr 2024 19:47:09 -0500 you wrote: > This patch series adds support for the AMD models of the Framework > Laptop (both 13" and 16"). > > These models of Framework Laptop have switched to an NPCX embedded > controller, which uses the original Chrome EC linear memory-mapped I/O > model. > > [...] Here is the summary with links: - [v3,1/4] platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device https://git.kernel.org/chrome-platform/c/8ae8e4e694b1 - [v3,2/4] platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device https://git.kernel.org/chrome-platform/c/839720336e8c - [v3,3/4] platform/chrome: cros_ec_lpc: add a "quirks" system https://git.kernel.org/chrome-platform/c/6b0ab6de2d86 - [v3,4/4] platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD) https://git.kernel.org/chrome-platform/c/3e8d1b5350e0 You are awesome, thank you!
Hello: This series was applied to chrome-platform/linux.git (for-next) by Tzung-Bi Shih <tzungbi@kernel.org>: On Tue, 2 Apr 2024 19:47:09 -0500 you wrote: > This patch series adds support for the AMD models of the Framework > Laptop (both 13" and 16"). > > These models of Framework Laptop have switched to an NPCX embedded > controller, which uses the original Chrome EC linear memory-mapped I/O > model. > > [...] Here is the summary with links: - [v3,1/4] platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device https://git.kernel.org/chrome-platform/c/8ae8e4e694b1 - [v3,2/4] platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device https://git.kernel.org/chrome-platform/c/839720336e8c - [v3,3/4] platform/chrome: cros_ec_lpc: add a "quirks" system https://git.kernel.org/chrome-platform/c/6b0ab6de2d86 - [v3,4/4] platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD) https://git.kernel.org/chrome-platform/c/3e8d1b5350e0 You are awesome, thank you!
This patch series adds support for the AMD models of the Framework Laptop (both 13" and 16"). These models of Framework Laptop have switched to an NPCX embedded controller, which uses the original Chrome EC linear memory-mapped I/O model. However, these devices are not configured in a way that is compatible with the cros_ec driver. Instead of mapping EC memory to I/O ports [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF]. To address this difference without impacting cros_ec_lpcs' compatibility with Chromebook/Chromebox devices or the existing Framework Laptop platform, these patches add DMI match-specific driver data through which we can detect per-machine "quirks". Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the memory MMIO window. Changes in v2: * Separated Framework Laptop (Intel) and Framework Laptop (AMD) * dev_warn has been demoted to dev_info * Style fixes * Reworded the individual patch bodies for clarity and style Changes in v3: * Addressed dev_warn feedback from Thomas Weissschuh * Removed the host command region quirk; Framework Computer is slated to release a firmware update that addresses the I/O region issue from this series' v1 and v2. This leaves just one quirk, with the potential for future quirks (e.g. changes required to reduce EC bus contention on the Intel Framework Laptops) Dustin L. Howett (4): platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device platform/chrome: cros_ec_lpc: add a "quirks" system platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD) drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 7 deletions(-)