Hi Alex, On Fri, Jun 7, 2019 at 10:41 PM Alex Marginean <alexm.ossl...@gmail.com> wrote: > > On 6/5/2019 4:09 AM, Bin Meng wrote: > > Hi Alex, > > > > On Tue, Jun 4, 2019 at 9:59 PM Alex Marginean <alexm.ossl...@gmail.com> > > wrote: > >> > >> Hi Bin, > >> > >> On 6/2/2019 5:22 PM, Bin Meng wrote: > >>> Hi Alex, > >>> > >>> On Fri, May 31, 2019 at 12:27 AM Alex Marginean <alexm.ossl...@gmail.com> > >>> wrote: > >>>> > >>>> LS1028A includes an integrated PCI bus with 8M of ECAM space plus > >>>> register > >>>> space for the integrated devices. This integrated PCI bus is driven > >>>> using > >>>> the generic ECAM driver. > >>>> > >>>> Signed-off-by: Alex Marginean <alexm.ossl...@gmail.com> > >>>> --- > >>>> arch/arm/dts/fsl-ls1028a.dtsi | 10 ++++++++++ > >>>> arch/arm/include/asm/arch-fsl-layerscape/cpu.h | 2 ++ > >>>> arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h | 2 ++ > >>>> configs/ls1028aqds_tfa_defconfig | 1 + > >>>> configs/ls1028ardb_tfa_defconfig | 1 + > >>>> 5 files changed, 16 insertions(+) > >>>> > >>>> diff --git a/arch/arm/dts/fsl-ls1028a.dtsi > >>>> b/arch/arm/dts/fsl-ls1028a.dtsi > >>>> index e6a443aa77..263c29af23 100644 > >>>> --- a/arch/arm/dts/fsl-ls1028a.dtsi > >>>> +++ b/arch/arm/dts/fsl-ls1028a.dtsi > >>>> @@ -108,6 +108,16 @@ > >>>> 0x82000000 0x0 0x40000000 0x88 0x40000000 0x0 > >>>> 0x40000000>; /* non-prefetchable memory */ > >>>> }; > >>>> > >>>> + pcie@1f0000000 { > >>>> + compatible = "pci-host-ecam-generic"; > >>>> + reg = <0x01 0xf0000000 0x0 0x100000>; > >>>> + #address-cells = <3>; > >>>> + #size-cells = <2>; > >>>> + device_type = "pci"; > >>>> + bus-range = <0x0 0x0>; > >>> > >>> I think this should be <0x0 0x7> since you mentioned in the commit > >>> message that only an 8M ECAM space is allocated. > >> > >> I think I was looking at the wrong spec, it seems the platform actually > >> has 256MB of address space reserved for ECAM. It's not all implemented > >> of course, in fact all the devices we care about in u-boot are on bus 0. > >> Anyway, I can just remove bus-range, the default is 0-255 and that does > >> match the HW. > >> > >>> > >>>> + ranges= <0x82000000 0x0 0x00000000 0x1 0xf8000000 0x0 > >>>> 0x160000>; > >>>> + }; > >>>> + > >>>> i2c0: i2c@2000000 { > >>>> compatible = "fsl,vf610-i2c"; > >>>> #address-cells = <1>; > >>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/cpu.h > >>>> b/arch/arm/include/asm/arch-fsl-layerscape/cpu.h > >>>> index bdeb62576c..7759acdb8f 100644 > >>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/cpu.h > >>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/cpu.h > >>>> @@ -42,7 +42,9 @@ > >>>> #else > >>>> #define CONFIG_SYS_PCIE1_PHYS_SIZE 0x800000000 > >>>> #define CONFIG_SYS_PCIE2_PHYS_SIZE 0x800000000 > >>>> +#ifndef CONFIG_SYS_PCIE3_PHYS_SIZE > >>>> #define CONFIG_SYS_PCIE3_PHYS_SIZE 0x800000000 > >>>> +#endif > >>>> #define CONFIG_SYS_PCIE4_PHYS_SIZE 0x800000000 > >>>> #define SYS_PCIE5_PHYS_SIZE 0x800000000 > >>>> #define SYS_PCIE6_PHYS_SIZE 0x800000000 > >>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >>>> index 24c1b0e482..273157230f 100644 > >>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h > >>>> @@ -186,6 +186,8 @@ > >>>> #elif CONFIG_ARCH_LS1028A > >>>> #define CONFIG_SYS_PCIE1_PHYS_ADDR 0x8000000000ULL > >>>> #define CONFIG_SYS_PCIE2_PHYS_ADDR 0x8800000000ULL > >>>> +#define CONFIG_SYS_PCIE3_PHYS_ADDR 0x01f0000000ULL > >>>> +#define CONFIG_SYS_PCIE3_PHYS_SIZE 0x0010000000ULL > >>> > >>> DT says the size is 0x100000. This does not match. > >> > >> I'll extend reg to 256MB along with removing bus-range. > >> > >>> These macros really look to me this platform is still using lots of > >>> non-DM approaches when it comes to driver support. These hard coded > >>> values should really be dropped and retrieved from DT instead via > >>> proper DM drivers. > >> > >> ECAM is driven by the ecam generic host driver and it doesn't care about > >> these macros. > >> These are used int arch/arm/cpu/armv8/fsl-layerscape/cpu.c to set up > >> the MMU at boot though, accessing ECAM doesn't work without them. > > > > Yes, but what I was trying to say that with driver model things like > > register base and size should be retrieved from DT. Can we consider > > doing an ARMv8 DM MMU driver that reads DT and set up the page table > > for everything? > > I've dug a bit more into the address ranges used here, there is indeed a > 256MB space but that's not just for ECAM. In fact 2nd half contains > device registers for the devices presented as PCI through ECAM. > Given than only bus 0 is populated I kept bus-range = <0x0 0x0>. > > On the other hand both ECAM and device registers have to be mapped in > MMU, so I didn't change the 256MB size macro. > > I agree the existing solution for mapping in MMU isn't great, since it > duplicates information between dts and code. I'm not sure what would > be a good alternative. > > Mapping based on dts could work, but the code would have to deal with > various corner cases. Some nodes don't use reg for addresses, for > integrated PCI with EA mapping ECAM alone is not sufficient, we also > need the BAR equivalents mapped, those kind of things. That means more > knowledge added one way or another to the arm MMU code, I'm not sure > that would end up being so much cleaner than what is there now.
Yep, I believe this is not an NXP specific problem. It applies to other ARM platforms, so I was hoping Tom or Simon could give some generic solution. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot