Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Mario Limonciello <superm1@gmail.com>]

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.