On Wed, Sep 01, 2021 at 11:34:50PM +0200, Michael Walle wrote:
Am 2021-09-01 12:29, schrieb Vladimir Oltean:
On Wed, Sep 01, 2021 at 10:55:21AM +0200, Michael Walle wrote:
pcie1: pcie@3400000 {compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";reg = <0x00 0x03400000 0x0 0x800000x00 0x03480000 0x0 0x40000 /* lut registers */0x00 0x034c0000 0x0 0x40000 /* pf controls registers */0x80 0x00000000 0x0 0x20000>; /* configuration space */reg-names = "dbi", "lut", "ctrl", "config";#address-cells = <3>;#size-cells = <2>;device_type = "pci";num-lanes = <4>;bus-range = <0x0 0xff>;ranges = <0x81000000 0x0 0x00000000 0x80 0x00020000 0x00x00010000 /* downstream I/O */
0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>;/* non-prefetchable memory */
};
pcie1: pcie@3400000 {compatible = "fsl,ls1028a-pcie";reg = <0x00 0x03400000 0x0 0x00100000>, /* controller registers */<0x80 0x00000000 0x0 0x00002000>; /* configuration space */This doesn't look good, the conversion is lossy. The Linux driver (drivers/pci/controller/dwc/pci-layerscape.c) knows the lut_offset by compatible string, but the U-Boot driver (drivers/pci/pcie_layerscape_rc.c) calls fdt_get_named_resource("lut"). I.... don't think that the driver works with a NULL pcie->lut pointer? The StreamID writes in fdt_fixup_pcie_device_ls probably didn't explode because I don't have any PCIe card plugged in.
I guess it didn't probe anyway, because there is no driver for that compatible string ;)
omg, "ls1028" vs "ls1028a" :-/
Similar thing for the PEX PF control memory region. In the LS1028A RM, technically this is part of the larger PEX_LUT memory map, just starting from the 4_0014h offset.
U-Boot has this piece of logic to deduce pcie->ctrl based on pcie->lut, but it seems that it needs to be extended to cover LS1028A:
Only that one probably don't want to overwrite cfg_res.start and cfg_res.end.
My internal parser is broken by this phrase's syntax, but yes.
/* * Fix the pcie memory map address and PF control registers address * for LS2088A series SoCs */ svr = get_svr(); svr = (svr >> SVR_VAR_PER_SHIFT) & 0xFFFFFE; if (svr == SVR_LS2088A || svr == SVR_LS2084A || svr == SVR_LS2048A || svr == SVR_LS2044A || svr == SVR_LS2081A || svr == SVR_LS2041A) { pcie_rc->cfg_res.start = LS2088A_PCIE1_PHYS_ADDR + LS2088A_PCIE_PHYS_SIZE * pcie->idx; pcie_rc->cfg_res.end = pcie_rc->cfg_res.start + cfg_size; pcie->ctrl = pcie->lut + 0x40000; }
Oh the horror! Why didn't one just change the config register value in the (u-boot) device tree instead of overwriting the value for some specific SoCs.
Code added in commit 3d8553f0a3ee ("pci: layerscape: add LS2088A series SoC pcie support"). As to why the differentiation between LS2080A and LS2088A/LS2084A and the 4-core variants is done through code rather than through DT, Zhiqiang said:
| There isn't LS2088A DT file, actually it reuse LS2080A DT file | under u-boot, while under kernel there are DT files for LS2080A | and LS2088A respectively with different PCIe node compatible | string.
https://u-boot.denx.narkive.com/XVDnIKG9/patchv3-1-2-pci-layerscape-add-ls20...
Again, as to why that is, no clue whatsoever.
As for pcie->lut itself, simplest would be to just default to what is now the "dbi" reg value, plus a .lut_offset determined by compatible string, in the case of "fsl,ls1028a-pcie" 0x80000, just like Linux.
Ah, and not to mention that the reg-names are not even the same between U-Boot and Linux. What is "dbi" in U-Boot is "regs" in Linux :)
Well and there is a big-endian property, which of course is set for any Layerscape but the LS1028A. linux has instead just one endianess, but use a shift offset.
You lost me here. How are registers accessed with the right endianness in Linux and in U-Boot?
Mh, I'm for sure not going to fix all these hacks in there. I'm thinking about bascially branching to a different probe for the ls1028a based on the compatible string (or rather if .data is set or not).
Makes sense to introduce a driver data structure for LS1028A.