Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 25 Feb 2022 14:04:22 +0100
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali@kernel.org>
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
        Marek =?utf-8?B?QmVow7pu?= <kabel@kernel.org>,
        Russell King <rmk+kernel@armlinux.org.uk>,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 05/12] PCI: mvebu: Correctly configure x1/x4 mode
Message-ID: <20220225130422.2aordy6mb2nxcxht@pali>
References: <20220222155030.988-6-pali@kernel.org>
 <20220225000800.GA304526@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220225000800.GA304526@bhelgaas>
User-Agent: NeoMutt/20180716
Precedence: bulk

On Thursday 24 February 2022 18:08:00 Bjorn Helgaas wrote:
> On Tue, Feb 22, 2022 at 04:50:23PM +0100, Pali Rohár wrote:
> > If x1/x4 mode is not set correctly then link with endpoint card is not
> > established.
> > 
> > Use DTS property 'num-lanes' to deteriminate x1/x4 mode.
> 
> I know this is already merged, but if tweaking for any other reason,
> s/deteriminate/determine/
> 
> > +	 * Set Maximum Link Width to X1 or X4 in Root Port's PCIe Link
> > +	 * Capability register. This register is defined by PCIe specification
> > +	 * as read-only but this mvebu controller has it as read-write and must
> > +	 * be set to number of SerDes PCIe lanes (1 or 4). If this register is
> > +	 * not set correctly then link with endpoint card is not established.
> 
> True, everything in Link Capability is RO or HwInit, but that's for
> the architected access via config space.  I think a device-specific
> mechanism like this is fair game as long as you do it before anybody
> can read it via config space.

Maybe I was not clear and explicit in above comment, but this register
sets number of PCIe lanes which HW will use. Armada PCIe controllers
supports only x1 and x4. Sometimes default HW value is 4 for x1 HW and
sometimes default value for x4 HW is 1. First case cause that link never
comes up (HW is trying to setup 4 lanes but in reality there is only
one, so link training never finish) and second case cause degraded
performance (x4 link is established only in x1 mode as HW is via this
register instructed to ignores other 3 lanes).

So basically HW designers misused this Link Capability register for
configuring PCIe Link of PCIe Root Port.

> > +	 */
> > +	lnkcap = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP);
> > +	lnkcap &= ~PCI_EXP_LNKCAP_MLW;
> > +	lnkcap |= (port->is_x4 ? 4 : 1) << 4;
> > +	mvebu_writel(port, lnkcap, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP);