Re: [PATCH 1/2] usb: pci-quirks: Correct AMD PLL quirk detection

From: Alan Stern
Date: Fri Jul 05 2019 - 15:04:31 EST


On Thu, 4 Jul 2019, Ryan Kennedy wrote:

> The AMD PLL USB quirk is incorrectly enabled on newer Ryzen
> chipsets. The logic in usb_amd_find_chipset_info currently checks
> for unaffected chipsets rather than affected ones. This broke
> once a new chipset was added in e788787ef. It makes more sense
> to reverse the logic so it won't need to be updated as new
> chipsets are added. Note that the core of the workaround in
> usb_amd_quirk_pll does correctly check the chipset.
>
> Signed-off-by: Ryan Kennedy <ryan5544@xxxxxxxxx>
> ---
> drivers/usb/host/pci-quirks.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 3ce71cbfbb58..ad05c27b3a7b 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -205,7 +205,7 @@ int usb_amd_find_chipset_info(void)
> {
> unsigned long flags;
> struct amd_chipset_info info;
> - int ret;
> + int need_pll_quirk = 0;
>
> spin_lock_irqsave(&amd_lock, flags);
>
> @@ -219,21 +219,28 @@ int usb_amd_find_chipset_info(void)
> spin_unlock_irqrestore(&amd_lock, flags);
>
> if (!amd_chipset_sb_type_init(&info)) {
> - ret = 0;
> goto commit;
> }
>
> - /* Below chipset generations needn't enable AMD PLL quirk */
> - if (info.sb_type.gen == AMD_CHIPSET_UNKNOWN ||
> - info.sb_type.gen == AMD_CHIPSET_SB600 ||
> - info.sb_type.gen == AMD_CHIPSET_YANGTZE ||
> - (info.sb_type.gen == AMD_CHIPSET_SB700 &&
> - info.sb_type.rev > 0x3b)) {
> + switch (info.sb_type.gen) {
> + case AMD_CHIPSET_SB700:
> + need_pll_quirk = info.sb_type.rev <= 0x3B;
> + break;
> + case AMD_CHIPSET_SB800:
> + case AMD_CHIPSET_HUDSON2:
> + case AMD_CHIPSET_BOLTON:
> + need_pll_quirk = 1;
> + break;
> + default:
> + need_pll_quirk = 0;
> + break;
> + }
> +
> + if (!need_pll_quirk) {
> if (info.smbus_dev) {
> pci_dev_put(info.smbus_dev);
> info.smbus_dev = NULL;
> }
> - ret = 0;
> goto commit;
> }
>
> @@ -252,7 +259,7 @@ int usb_amd_find_chipset_info(void)
> }
> }
>
> - ret = info.probe_result = 1;
> + need_pll_quirk = info.probe_result = 1;
> printk(KERN_DEBUG "QUIRK: Enable AMD PLL fix\n");
>
> commit:
> @@ -263,7 +270,7 @@ int usb_amd_find_chipset_info(void)
>
> /* Mark that we where here */
> amd_chipset.probe_count++;
> - ret = amd_chipset.probe_result;
> + need_pll_quirk = amd_chipset.probe_result;
>
> spin_unlock_irqrestore(&amd_lock, flags);
>
> @@ -277,7 +284,7 @@ int usb_amd_find_chipset_info(void)
> spin_unlock_irqrestore(&amd_lock, flags);
> }
>
> - return ret;
> + return need_pll_quirk;
> }
> EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info);

Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>