From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id F3A4EC433EF for ; Mon, 29 Nov 2021 11:47:19 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5B8BE82A66; Mon, 29 Nov 2021 12:47:17 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="rkvGjz8C"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C0BF282A66; Mon, 29 Nov 2021 12:47:14 +0100 (CET) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9EC8082A53 for ; Mon, 29 Nov 2021 12:47:07 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=pali@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id A6B2361280; Mon, 29 Nov 2021 11:47:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B4552C004E1; Mon, 29 Nov 2021 11:47:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1638186425; bh=0Uhb4CBZG6y6bucVXly8A3ehXft5fpDrjLipbYADa5Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rkvGjz8CSD+XXCOhAVWl/39jmFfn/7XgpvoYXQwJZGL4Ag99AWBpLhYdkInLMJIYo yXpGGxaUfbVFT0snrPFd6hdAKHFHpkVKx7SxXeQUeTs0aBlghqHDKk1uK28ltU7UU1 OhNq+j4UMZJn0yTrt4R+vCuafEvuATSorIlfbYJpGZzdtDJDU+Lzz9x72ANC1S180Z Aw+5VVpCdHEPCKlnd37faWrf2KVZS7GZozl760u3WeANOTJEO/gCsq72+ZjIl1tCMI vLfxpxGDYEief5BZfk/dQ3y1mn15wsDAIQgVeE5iWq7/OyBoqNvJdXjNgEoGV+rOo9 /DRQZxdkknhuQ== Received: by pali.im (Postfix) id 56FAFEAA; Mon, 29 Nov 2021 12:47:01 +0100 (CET) Date: Mon, 29 Nov 2021 12:47:01 +0100 From: Pali =?utf-8?B?Um9ow6Fy?= To: Stefan Roese Cc: Marek =?utf-8?B?QmVow7pu?= , u-boot@lists.denx.de, Marek =?utf-8?B?QmVow7pu?= Subject: Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c Message-ID: <20211129114701.evkol44t6l3rvdpf@pali> References: <20211111153549.29111-1-kabel@kernel.org> <20211111153549.29111-3-kabel@kernel.org> <20211118180103.wewgff3pqqrwqjxr@pali> <5ea0641f-cde6-e553-dfb8-993ab6daff67@denx.de> <20211123155953.4cuju6mtwgmrzumq@pali> <20211129090612.q3pdg64bhhk4gnvz@pali> <83bd5ea0-46ce-39be-d566-46c397db2037@denx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <83bd5ea0-46ce-39be-d566-46c397db2037@denx.de> User-Agent: NeoMutt/20180716 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.37 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hello! On Monday 29 November 2021 10:22:58 Stefan Roese wrote: > On 11/29/21 10:06, Pali Rohár wrote: > > > > > > > After this DTS change, pci-mvebu.c will just replace value of current > > > > number of lanes (which is set to 4 by serdes code) to value from DTS, > > > > which is 4. Therefore there should be no change. > > > > > > > > Could you test whole patch series with above DTS change if it works > > > > properly on Theadorable board? > > > > > > Yes, I don't see any issues with this patchset applied plus this DT > > > patch on theadorable. The PCIe links are up and with the correct width. > > > > > > What I'm wondering is, when exactly does the PCIe RP start the link > > > establishment. In my case with AXP this is still in the AXP serdes code > > > of course. But in the A38x code, it should be in the PCIe controller > > > driver now AFAIU. I see that you configure the link width in the > > > controller and do some other configuration (address windows etc), but > > > at the end you "simply" wait for the link to come up via > > > mvebu_pcie_wait_for_link(). I would have expected here some special > > > command (config bit?) to the PCIe controller to start the link > > > establishment. So when exactly does the A38x start this action? > > > > That is interesting question... While I'm reading it again, I really do > > not know. Because you are right that mvebu_pcie_wait_for_link() is just > > waiting for a link and it "magically" comes up. I have tested it on A385 > > and it is stable with different Compex Atheros cards which caused issues > > in past also on A3720. > > I would prefer, to fully understand when exactly the link establishment > is started. Since this is crucial for the setup of the controller that > needs to be done *before* the link starts to come up. I try to dig as more information as possible and finally I find out that important information is available also in now removed, but originally public A38x documentation. Thankfully web archive has copy of it: https://web.archive.org/web/20200420191927/https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-embedded-processors-armada-38x-functional-specifications-2015-11.pdf 17.3 Link Initialization In case the initialization fails and no link is established, the PHY will keep on trying to initiate a link forever unless the port is disabled. As long as the port is enabled, the PHY will continue trying to establish a link; once the PHY identifies that a device is connected to it, a link will be established. PCIe port is enabled by bits in SoC Control 1 Register, which is done in U-Boot SerDes initialization code. This is IIRC SoC specific, and reason why every Armada SoC has own SerDes init code. And looks like that due to "the PHY will keep on trying to initiate a link forever", the PCIe link comes up when pci-mvebu.c sets all required registers to correct values. E.g. set correct mode (RC vs endpoint), link width (x1 vs x4), etc... > Could you perhaps try to remove some of the register configurations in > the MVEBU PCIe driver to see, if the link establishment relies on this > register to be written to (e.g. PCI_EXP_LNKCAP)? First port on A385 is by default set to X4 width, other ports to X1 width. Without updating LNKCAP to correct width, card in first PCIe port never initialize. And cards in other ports are initialized even before pci-mvebu.c starts configuration. So seems that this matches above behavior. SerDes init code enabled all PCIe ports. Ports which are using default configuration (second, third) are immediately initialized and link is established. Port which requires additional configuration (first port, for switching from X4 to X1) just stay in that "keep on trying to initiate a link forever" state until pci-mvebu starts and set PCI_EXP_LNKCAP register, after which PHY try X1 width and success. And seems that this is the reason why 100ms timeout is needed... As at this stage when pci-mvebu.c switches X4 to X1, init timeout as defined in PCIe spec (that 100ms) starts ticking. For other ports it starts ticking when serdes init code enables ports. I have looked into all PCIe registers which are present in functional spec, but it looks like that there is no pci-mvebu register which can turn of LTSSM and link training, like it is in other PCIe controllers. It looks like that only SoC-specific port enable bits are there. It is starting to be bigger mess as before... Any suggestion how to continue with it? We cannot (easily) move that code which flips PCIe bits in SoC Control 1 Register from SerDes init code to pci-mvebu.c as this is outside of pci-mvebu.c address space and also it is different on every SoC. pci-mvebu.c registers are same on all Marvell SoCs, starting from Orion up to the A39x. > Thanks, > Stefan