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

[Stefan Roese <sr@denx.de>]

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@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.

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@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@denx.de