Re: [PATCH] usb: ehci-pci: Set has_hostpc for TDI EHCI on Intel CE5300/CE2600

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: ehci: tdi_in_host_mode() fails to check USBMODE_EX
--

commit 493a630291d0b5961a888e40fa36840fea16fbf5
Author: Joseph Telaak <joseph@telaak.dev>
    usb: ehci-pci: Set has_hostpc for TDI EHCI on Intel CE5300/CE2600

The TransDimension EHCI core in the Intel CE5300 and CE2600 SoCs
(192e:0101, revision >= 6) has the Moorestown-style register extensions:
host mode is selected in USBMODE_EX rather than USBMODE. This commit sets
has_hostpc so that USBMODE_EX reads back 0x23 and devices enumerate.

> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index bd3a635555942..8c71c3de3082e 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -145,8 +145,16 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
>  			hcd->has_tt = 1;
>  		break;
>  	case PCI_VENDOR_ID_TDI:
> -		if (pdev->device == PCI_DEVICE_ID_TDI_EHCI)
> +		if (pdev->device == PCI_DEVICE_ID_TDI_EHCI) {
>  			hcd->has_tt = 1;
> +			/*
> +			 * The TDI core in the Intel CE5300/CE2600 (rev >= 6)
> +			 * selects host mode in USBMODE_EX and reports port
> +			 * speed in HOSTPC, as on Moorestown.
> +			 */
> +			if (pdev->revision >= 0x6)
> +				ehci->has_hostpc = 1;
> +		}

[Severity: High]
Since this hardware selects host mode via USBMODE_EX (offset 0xc8), the
standard USBMODE register (offset 0x68) may read as 0. If it does, will
tdi_in_host_mode() falsely return 0?

drivers/usb/host/ehci-hcd.c:tdi_in_host_mode() {
    ...
    tmp = ehci_readl(ehci, &ehci->regs->usbmode);
    return (tmp & 3) == USBMODE_CM_HC;
}

During driver unbind or system suspend, ehci_halt() relies on
tdi_in_host_mode(). A false 0 return causes ehci_halt() to exit
immediately and completely bypass the teardown sequence (clearing
CMD_RUN and waiting for STS_HALT):

drivers/usb/host/ehci-hcd.c:ehci_halt() {
    ...
    if (ehci_is_TDI(ehci) && !ehci->no_tdi_mode &&
        !tdi_in_host_mode(ehci)) {
        spin_unlock_irq(&ehci->lock);
        return 0;
    }
    ...
}

Can this bypassed teardown sequence leave DMA and IRQs active on the
newly enabled core, potentially leading to memory corruption or a system
panic? Does tdi_in_host_mode() need to check USBMODE_EX if has_hostpc
is set?

>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-ehci-ce5300-hostpc-v1-1-3cae67c11946@telaak.dev?part=1