On 11/29/21 15:28, Pali Rohár wrote:
On Monday 29 November 2021 14:27:48 Pali Rohár wrote:
On Monday 29 November 2021 13:30:45 Stefan Roese wrote:
Hi Pali,
On 11/29/21 12:47, Pali Rohár wrote:
Hello!
On Monday 29 November 2021 10:22:58 Stefan Roese wrote:
On 11/29/21 10:06, Pali Rohár wrote:
<snip>
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 the PCIe ports are now trying to establish the links, even when the
correct configuration is not yet done. This might work but sound far
from perfect to me IMHO.
Yes, it looks like (based on behavior of the first port). And it is not
perfect, just another mess :-(
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.
One idea would be, to use a "reset-controller" driver on the Armada
platforms, that is capable to at least reset and release the PCIe ports.
Via the SoC Control 1 reg on A38x and via the SoC Control Register
on AXP.
In that specification is also written:
Enable the PCI Express interface by setting the <PCIe0En>, <PCIe1En>,
<PCIe2En>, or <PCIe3En> field in the SoC Control 1 Register (Table
1888 p. 1395). This allows programming of link parameters before the
start of link initialization. The highest common link width is
established according to the following order: x4 to x1.
So I think the correct behavior should be:
1. pci-mvebu.c configures all controller registers to correct values
2. PCIe port is enabled via SoC-specific register
3. pci-mvebu.c waits for link up
I guess that reset-controller does not help, as core release this reset
prior starting driver initialization.
Ok, it looks like that reset controller API allows to do this. It would
mean to define that "system-controller@18200" as reset controller,
exports from it for each PCIe port reset functionality and implements
assert and deassert functions which disable and enable port.
And because DTS for pci-mvebu.c driver is defined as multi-root-port,
"resets" property would need to be defined for each port separately.
Okay. Sounds like a plan to me.
Just I'm not sure if this "enable port functionality" should be
implemented via Reset Controller API...
How else should / could this be done then? Do you have alterative ideas?
Thanks,
Stefan
Anyway, this A385 SoC Control 1 Register is at address 0x18204 which is
part of following device defined in kernel DTS file:
systemc: system-controller@18200 {
compatible =
"marvell,armada-380-system-controller",
"marvell,armada-370-xp-system-controller";
reg = <0x18200 0x100>;
};
Linux kernel has driver for this DTS device is file:
arch/arm/mach-mvebu/system-controller.c
U-Boot does not have any driver for this compatible string.
So PCIe port enable/disable should be in this driver. I can write simple
driver also for U-Boot which can control this register. But I really do
not know which interface should it use.
Has somebody else any idea?
I just looked into some Linux PCIe DT bindings and found e.g. this in
the mediatek spec:
Documentation/devicetree/bindings/pci/mediatek-pcie.txt
...
Required properties for MT7623/MT2701:
...
- resets: Must contain an entry for each entry in reset-names.
See ../reset/reset.txt for details.
- reset-names: Must be "pcie-rst0", "pcie-rst1", "pcie-rstN".. based on the
number of root ports.
...
resets = <&hifsys MT2701_HIFSYS_PCIE0_RST>,
<&hifsys MT2701_HIFSYS_PCIE1_RST>,
<&hifsys MT2701_HIFSYS_PCIE2_RST>;
reset-names = "pcie-rst0", "pcie-rst1", "pcie-rst2";
phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>,
<&pcie2_phy PHY_TYPE_PCIE>;
phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
And make sure in the serdes code keeps (or actively sets?) these PCIe
ports into the reset state. The PCIe driver would then release the ports
out of reset after their configuration.
Or is some other serdes code missing in between "get PCIe port out of
reset" and the MVEBU PCIe driver taking over the control?
What do you think? I might be missing something here.
Thanks,
Stefan
