On Thu, Apr 08, 2021 at 05:11:12PM +0800, Longfang Liu wrote:
> Some types of EHCI controllers do not have SBRN registers.
> By comparing the white list, the operation of reading the SBRN
> registers is skipped.
>
> Subsequent EHCI controller types without SBRN registers can be
> directly added to the white list.
>
> The current patch does not affect the drive function.
>
> Signed-off-by: Longfang Liu <liulongfang@xxxxxxxxxx>
> ---
> drivers/usb/host/ehci-pci.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 3c3820a..6a30afa 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -47,6 +47,28 @@ static inline bool is_bypassed_id(struct pci_dev *pdev)
> return !!pci_match_id(bypass_pci_id_table, pdev);
> }
>
> +static const struct usb_nosbrn_whitelist_entry {
> + unsigned short vendor;
> + unsigned short device;
u16 here please.
> +} usb_nosbrn_whitelist[] = {
> + /* STMICRO ConneXT has no sbrn register */
> + {PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_USB_HOST},
> + {}
trailing , please.
> +};
> +
> +static bool usb_nosbrn_whitelist_check(struct pci_dev *pdev)
> +{
> + const struct usb_nosbrn_whitelist_entry *entry;
> +
> + for (entry = usb_nosbrn_whitelist; entry->vendor; entry++) {
> + if (pdev->vendor == entry->vendor &&
> + pdev->device == entry->device)
> + return true;
> + }
> +
> + return false;
> +}
> +
> /*
> * 0x84 is the offset of in/out threshold register,
> * and it is the same offset as the register of 'hostpc'.
> @@ -288,10 +310,7 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
> }
>
> /* Serial Bus Release Number is at PCI 0x60 offset */
> - if (pdev->vendor == PCI_VENDOR_ID_STMICRO
> - && pdev->device == PCI_DEVICE_ID_STMICRO_USB_HOST)
> - ; /* ConneXT has no sbrn register */
> - else
> + if (!usb_nosbrn_whitelist_check(pdev))
Doing this as a "negative" is hard to understand. Should this just be:
forbid_sbrn_read()
or something like that?
The term "whitelist" is not a good thing to use as it does not really
explain anything here.
thanks,
greg k-h
