[U-Boot] [PATCH] arm: ls1028a: define the integrated PCI bus (ECAM)

[Alex Marginean]

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.osslist at 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.osslist at 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 at 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 at 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.

> 
>>   #else
>>   #define CONFIG_SYS_PCIE1_PHYS_ADDR             0x1000000000ULL
>>   #define CONFIG_SYS_PCIE2_PHYS_ADDR             0x1200000000ULL
>> diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
>> index 717b810523..7982ce4157 100644
>> --- a/configs/ls1028aqds_tfa_defconfig
>> +++ b/configs/ls1028aqds_tfa_defconfig
>> @@ -48,6 +48,7 @@ CONFIG_E1000=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>>   CONFIG_DM_PCI_COMPAT=y
>> +CONFIG_PCIE_ECAM_GENERIC=y
>>   CONFIG_PCIE_LAYERSCAPE=y
>>   CONFIG_SCSI=y
>>   CONFIG_DM_SCSI=y
>> diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
>> index a8e4ddb7a8..c65e37df79 100644
>> --- a/configs/ls1028ardb_tfa_defconfig
>> +++ b/configs/ls1028ardb_tfa_defconfig
>> @@ -48,6 +48,7 @@ CONFIG_E1000=y
>>   CONFIG_PCI=y
>>   CONFIG_DM_PCI=y
>>   CONFIG_DM_PCI_COMPAT=y
>> +CONFIG_PCIE_ECAM_GENERIC=y
>>   CONFIG_PCIE_LAYERSCAPE=y
>>   CONFIG_SCSI=y
>>   CONFIG_DM_SCSI=y
>> --
> 
> Regards,
> Bin
> 

Thank you!
Alex