Re: [PATCH v2] usb: typec: ucsi: do not register UCSI on Lenovo Legion Pro 7 16IAX10H

[Greg KH]

On Mon, Aug 31, 2026 at 12:15:21PM +0200, Heikki Krogerus wrote:
> On Wed, Aug 26, 2026 at 09:31:18PM +0800, Huang Wei wrote:
> > The EC firmware of the Lenovo Legion Pro 7 16IAX10H (DMI product name
> > 83F5) has a broken UCSI implementation: GET_PDOS returns no source
> > capabilities, GET_CONNECTOR_STATUS reports an incomplete RDO, and any
> > UCSI traffic disturbs the EC's autonomous USB-C Power Delivery
> > negotiation. With ucsi_acpi bound, chargers connect only briefly and
> > then drop, and voltage_now reads 0; with the driver blacklisted, the
> > EC negotiates full PD contracts on its own and charging works normally.
> > 
> > The problem is unaffected by BIOS updates up to Q7CN78WW (2026-04).
> > The AMD sibling Legion Pro 7 16AFR10H (product name 83RU) reportedly
> > uses the same EC firmware, but it is left out of the quirk until the
> > failure is confirmed on that model.
> > 
> > Skip probe on this machine, which is equivalent to the blacklist
> > workaround the reporter verified to restore charging.
> > 
> > Reported-by: Alex Bartz <alex@xxxxxxxxxxxxx>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221142
> > Tested-by: Alex Bartz <alex@xxxxxxxxxxxxx>
> > Signed-off-by: Huang Wei <huangwei@xxxxxxxxxx>
> 
> I'm not sure about the dev_info message. Otherwise this is good:

Should be dev_err(), right?

> 
> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> 
> > ---
> > Changes in v2:
> > - Add Tested-by tag from Alex Bartz, who verified the quirk on his
> >   machine (out-of-tree build, runtime module swap, PD charging held
> >   across the reload).
> > ---
> >  drivers/usb/typec/ucsi/ucsi_acpi.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> > index 18286d3e9cc5..9f36f8cfce4a 100644
> > --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> > +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> > @@ -173,6 +173,21 @@ static const struct dmi_system_id ucsi_acpi_quirks[] = {
> >  	{ }
> >  };
> >  
> > +/*
> > + * Platforms with a completely broken UCSI implementation in firmware.
> > + * Do not register UCSI at all on these machines.
> > + */
> > +static const struct dmi_system_id ucsi_acpi_ignore[] = {
> > +	{
> > +		/* Lenovo Legion Pro 7 16IAX10H: broken EC UCSI firmware */
> > +		.matches = {
> > +			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > +			DMI_MATCH(DMI_PRODUCT_NAME, "83F5"),
> > +		},
> > +	},
> > +	{ }
> > +};
> > +
> >  static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
> >  {
> >  	struct ucsi_acpi *ua = data;
> > @@ -199,6 +214,12 @@ static int ucsi_acpi_probe(struct platform_device *pdev)
> >  	if (adev->dep_unmet)
> >  		return -EPROBE_DEFER;
> >  
> > +	if (dmi_check_system(ucsi_acpi_ignore)) {
> > +		dev_info(&pdev->dev,
> > +			 "UCSI implementation in firmware is broken, ignoring\n");

This shows the hardware is broken, make it dev_err() so that someone
notices it and can pester the hardware company to fix it.

BUT, if the firwmare ever does get changed, this will still fail, so
shouldn't this all be triggered on a version/range of broken firmware,
and NOT just on the whole platform itself?  Otherwise if it is fixed,
this code will be wrong.

thanks,

greg k-h