RE: [PATCH] PCI: Add AMD RV2 based APUs, such as 3015Ce, to D3hot to D3 quirk table.

[Schroeder, Julian]

[AMD Official Use Only - Internal Distribution Only]

I observed an issue with D3hot to D0 transition on 3015e APUs.
Since the peripheral device IP of the APUs already covered by this quirk is almost identical. I added the 3015e. 
Further testing an a great many machines has not shown the issue occur again.

-----Original Message-----
From: Bjorn Helgaas <helgaas@xxxxxxxxxx> 
Sent: Thursday, March 11, 2021 6:53 AM
To: S, Shirish <Shirish.S@xxxxxxx>
Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Schroeder, Julian <Julian.Schroeder@xxxxxxx>; Daniel Drake <drake@xxxxxxxxxxxx>; Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH] PCI: Add AMD RV2 based APUs, such as 3015Ce, to D3hot to D3 quirk table.

[CAUTION: External Email]

[+cc Daniel, Mika (author, reviewer of 3030df209aa8]

On Thu, Mar 11, 2021 at 10:11:35AM +0530, Shirish S wrote:
> From: Julian Schroeder <julian.schroeder@xxxxxxx>
>
> This allows for an extra 10ms for the state transition.
> Currently only AMD PCO based APUs are covered by this table.

I'm really glad to see this coming straight from AMD.  Is this a documented erratum?  Please provide a reference to that.

The point is that quirks are for working around hardware defects.  If the device is not defective, and it is actually following the spec correctly, there should be a way to fix this in a generic way that doesn't require quirks.  That avoids the need to add more quirks for future devices.

> WIP. Working on commit to kernel.org.

I'm not sure what "WIP. Working on commit to kernel.org." means.  Does it mean I should ignore this and wait for the final posting?

I'm OCD enough that I like commits doing the same thing to have the same subject line.  This is an extension of 3030df209aa8 ("PCI:
Increase D3 delay for AMD Ryzen5/7 XHCI controllers"), so it should look like that.

> Signed-off-by: Julian Schroeder <julian.schroeder@xxxxxxx>

This appears to require an additional signoff from you, Shiresh; see [1].

Bjorn

[1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23n356&amp;data=04%7C01%7Cjulian.schroeder%40amd.com%7C7fc41008b90e486b882008d8e48ca91c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637510640444272647%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=nyJcTD5Vy%2BV1raz%2Fb7ZSiRdp7quMXcydjMdcD2FmQYs%3D&amp;reserved=0

> ---
>  drivers/pci/quirks.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 
> 653660e3ba9e..7d8f52524ada 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1904,6 +1904,7 @@ static void quirk_ryzen_xhci_d3hot(struct 
> pci_dev *dev)  }  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x15e0, 
> quirk_ryzen_xhci_d3hot);  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 
> 0x15e1, quirk_ryzen_xhci_d3hot);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x15e5, 
> +quirk_ryzen_xhci_d3hot);
>
>  #ifdef CONFIG_X86_IO_APIC
>  static int dmi_disable_ioapicreroute(const struct dmi_system_id *d)
> --
> 2.17.1
>