Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 11 Mar 2021 06:53:22 -0600
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Shirish S <shirish.s@amd.com>
Cc:     Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Julian Schroeder <julian.schroeder@amd.com>,
        Daniel Drake <drake@endlessm.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH] PCI: Add AMD RV2 based APUs, such as 3015Ce, to D3hot to
 D3 quirk table.
Message-ID: <20210311125322.GA2122226@bjorn-Precision-5520>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210311044135.119942-1-shirish.s@amd.com>
Precedence: bulk

[+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@amd.com>
> 
> 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@amd.com>

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

Bjorn

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n356

> ---
>  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
>