[PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

[Pali Rohár]

On Monday 13 December 2021 08:36:00 Stefan Roese wrote:
> Hi Pali,
> 
> On 12/10/21 15:23, Pali Rohár wrote:
> 
> <snip>
> 
> > > > > 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 at 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.
> > 
> > I'm looking at this right now and it is even more complicated. For
> > example Armada XP has for some ports dedicated enable bits and for some
> > ports has just one shared enable bit.
> > 
> > And this HW design does not fit into current U-Boot PCI DM model where
> > for each PCIe port there is dedicated mvebu_pcie_probe() call which
> > should setup PCIe port, enable PCIe port and returns once PCIe port is
> > ready. In U-Boot PCI DM model is every PCIe port as separate DM device.
> > 
> > Any idea how to solve this issue?
> 
> Sorry, but I don't have clear "solution" for this in my mind right now.

Ok, I will try to invent something...

> Thanks,
> Stefan
> 
> > > > 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 at 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
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de