Re: [PATCH v3] usb: typec: ucsi: Use GET_CAPABILITY attributes data to set power supply scope

[Mario Limonciello]

On 10/10/2023 03:42, Heikki Krogerus wrote:
> On Mon, Oct 09, 2023 at 01:46:43PM -0500, Mario Limonciello wrote:
> > On some OEM systems, adding a W7900 dGPU triggers RAS errors and hangs
> > at a black screen on startup.  This issue occurs only if `ucsi_acpi` has
> > loaded before `amdgpu` has loaded.  The reason for this failure is that
> > `amdgpu` uses power_supply_is_system_supplied() to determine if running
> > on AC or DC power at startup. If this value is reported incorrectly the
> > dGPU will also be programmed incorrectly and trigger errors.
> >
> > power_supply_is_system_supplied() reports the wrong value because UCSI
> > power supplies provided as part of the system don't properly report the
> > scope as "DEVICE" scope (not powering the system).
> >
> > In order to fix this issue check the capabilities reported from the UCSI
> > power supply to ensure that it supports charging a battery and that it can
> > be powered by AC.  Mark the scope accordingly.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: a7fbfd44c020 ("usb: typec: ucsi: Mark dGPUs as DEVICE scope")
> > Link: https://www.intel.com/content/www/us/en/products/docs/io/universal-serial-bus/usb-type-c-ucsi-spec.html p28
> > Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

Thanks!

Greg, this behavior is pretty awful with the hang at bootup on these OEM 
systems, do you think this can come into one of the 6.6-rcX or should it 
wait until 6.7?

> > ---
> > Cc: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> > Cc: Alex Deucher <Alexander.Deucher@xxxxxxx>>
> > Cc: Richard Gong <Richard.Gong@xxxxxxx>
> >
> > v2->v3:
> >   * Pick up tag
> >   * Add missing stable tag
> >   * Drop patch 2, we'll just avoid upstreaming it.
> > ---
> >   drivers/usb/typec/ucsi/psy.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c
> > index 384b42267f1f..b35c6e07911e 100644
> > --- a/drivers/usb/typec/ucsi/psy.c
> > +++ b/drivers/usb/typec/ucsi/psy.c
> > @@ -37,6 +37,15 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con,
> >   	struct device *dev = con->ucsi->dev;
> >   
> >   	device_property_read_u8(dev, "scope", &scope);
> > +	if (scope == POWER_SUPPLY_SCOPE_UNKNOWN) {
> > +		u32 mask = UCSI_CAP_ATTR_POWER_AC_SUPPLY |
> > +			   UCSI_CAP_ATTR_BATTERY_CHARGING;
> > +
> > +		if (con->ucsi->cap.attributes & mask)
> > +			scope = POWER_SUPPLY_SCOPE_SYSTEM;
> > +		else
> > +			scope = POWER_SUPPLY_SCOPE_DEVICE;
> > +	}
> >   	val->intval = scope;
> >   	return 0;
> >   }
> > --
> > 2.34.1