Received-SPF: pass (google.com: domain of linux-kernel+bounces-180653-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 16 May 2024 11:07:27 +0530
From: Siddharth Vadapalli <s-vadapalli@ti.com>
To: Bjorn Helgaas <helgaas@kernel.org>
CC: Siddharth Vadapalli <s-vadapalli@ti.com>, <lpieralisi@kernel.org>,
        <kw@linux.com>, <robh@kernel.org>, <bhelgaas@google.com>,
        <manivannan.sadhasivam@linaro.org>, <fancer.lancer@gmail.com>,
        <u.kleine-koenig@pengutronix.de>, <cassel@kernel.org>,
        <dlemoal@kernel.org>, <yoshihiro.shimoda.uh@renesas.com>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <srk@ti.com>
Subject: Re: [PATCH v7 2/2] PCI: keystone: Fix pci_ops for AM654x SoC
Message-ID: <ee5f54a8-c19f-4a56-9a9b-f8aeebf475d0@ti.com>
References: <20240514211452.GA2082543@bhelgaas>
 <20240515192614.GA2133406@bhelgaas>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240515192614.GA2133406@bhelgaas>

On Wed, May 15, 2024 at 02:26:14PM -0500, Bjorn Helgaas wrote:
> On Tue, May 14, 2024 at 04:14:54PM -0500, Bjorn Helgaas wrote:
> > On Tue, May 14, 2024 at 05:41:48PM +0530, Siddharth Vadapalli wrote:
> > > On Mon, May 13, 2024 at 04:53:50PM -0500, Bjorn Helgaas wrote:
> > ...
> 
> > > > I'm not quite clear on the mechanism, but it would be helpful to at
> > > > least know what's wrong and on what platform.  E.g., currently v4.90
> > > > suffers Completion Timeouts and 45 second boot delays?  And this patch
> > > > fixes that?
> > > 
> > > Yes, the Completion Timeouts cause the 45 second boot delays and this
> > > patch fixes that.
> > 
> > And this problem happens on AM654x/v4.90a, right?  I really want the
> > commit log to say what platform is affected!
> > 
> > Maybe something like this?
> > 
> >   PCI: keystone: Enable BAR 0 only for v3.65a
> > 
> >   The BAR 0 initialization done by ks_pcie_v3_65_add_bus() should
> >   happen for v3.65a devices only.  On other devices, BAR 0 should be
> >   left disabled, as it is by dw_pcie_setup_rc().
> > 
> >   After 6ab15b5e7057 ("PCI: dwc: keystone: Convert .scan_bus()
> >   callback to use add_bus"), ks_pcie_v3_65_add_bus() enabled BAR 0 for
> >   both v3.65a and v4.90a devices.  On the AM654x SoC, which uses
> >   v4.90a, enabling BAR 0 causes Completion Timeouts when setting up
> >   MSI-X.  These timeouts delay boot of the AM654x by about 45 seconds.
> > 
> >   Move the BAR 0 initialization to ks_pcie_msi_host_init(), which is
> >   only used for v3.65a devices, and remove ks_pcie_v3_65_add_bus().
> 
> I haven't heard anything so I amended it to the above.  But please
> correct me if it's wrong.

I would suggest specifying the failing combination since I do not know
if there is another device that is using v4.90a but doesn't see this
issue. What is certain is that this issue is seen with the v4.90a
controller on AM654x platform. Despite the PCIe Controller version
remaining the same across different platforms, it might be possible
that not all features supported by the PCIe Controller are enabled on
all platforms. For that reason, it appears to me that the subject could
be:

  PCI: keystone: Don't enable BAR 0 for AM654x

which implicitly indicates the combination as well (v4.90a on AM654x).

The commit message's contents could be reduced to:

  After 6ab15b5e7057 ("PCI: dwc: keystone: Convert .scan_bus()
  callback to use add_bus"), ks_pcie_v3_65_add_bus() enabled BAR 0 for
  both v3.65a and v4.90a devices.  On the AM654x SoC, which uses
  v4.90a, enabling BAR 0 causes Completion Timeouts when setting up
  MSI-X.  These timeouts delay boot of the AM654x by about 45 seconds.

  Move the BAR 0 initialization to ks_pcie_msi_host_init(), which is
  only used for v3.65a devices, and remove ks_pcie_v3_65_add_bus().

by dropping:

  The BAR 0 initialization done by ks_pcie_v3_65_add_bus() should
  happen for v3.65a devices only.  On other devices, BAR 0 should be
  left disabled, as it is by dw_pcie_setup_rc().

The reason behind dropping the above paragraph is that BAR 0 could
probably be enabled on other controller versions as well, but not on the
v4.90a controller on the AM654x SoC.

Thank you Bjorn, for enhancing the commit message.

Regards,
Siddharth.