[PATCH v4 23/29] pci: layerscape: add official ls1028a binding support

[Michael Walle]

Hi Zhiqiang,

thanks for looking at this patch.

Am 2021-10-13 03:46, schrieb Z.Q. Hou:
>> -----Original Message-----
>> From: Michael Walle <michael at walle.cc>
>> Sent: 2021年10月5日 16:38
>> To: u-boot at lists.denx.de
>> Cc: Jagan Teki <jagan at amarulasolutions.com>; Priyanka Jain
>> <priyanka.jain at nxp.com>; Vladimir Oltean <vladimir.oltean at nxp.com>;
>> Tom Rini <trini at konsulko.com>; Peter Griffin 
>> <peter.griffin at linaro.org>;
>> Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>; Michael
>> Walle <michael at walle.cc>; Z.Q. Hou <zhiqiang.hou at nxp.com>
>> Subject: [PATCH v4 23/29] pci: layerscape: add official ls1028a 
>> binding
>> support
>> 
>> The official bindind of the PCIe controller of the ls1028a has the 
>> following
>> compatible string:
>>   compatible = "fsl,ls1028a-pcie";
>> 
>> Additionally, the resource names and count are different. Update the 
>> driver
>> to support this binding and change the entry in the ls1028a device 
>> tree.
>> 
>> Cc: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
>> Signed-off-by: Michael Walle <michael at walle.cc>
>> ---
>>  arch/arm/dts/fsl-ls1028a.dtsi    | 20 +++++------
>>  drivers/pci/pcie_layerscape_rc.c | 61 
>> +++++++++++++++++++++++---------
>>  2 files changed, 53 insertions(+), 28 deletions(-)
>> 
>> diff --git a/arch/arm/dts/fsl-ls1028a.dtsi 
>> b/arch/arm/dts/fsl-ls1028a.dtsi
>> index cc055e65e5..435b965d00 100644
>> --- a/arch/arm/dts/fsl-ls1028a.dtsi
>> +++ b/arch/arm/dts/fsl-ls1028a.dtsi
>> @@ -344,12 +344,10 @@
>>  		};
>> 
>>  		pcie1: pcie at 3400000 {
>> -			compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";
>> -			reg = <0x00 0x03400000 0x0 0x80000
>> -			       0x00 0x03480000 0x0 0x40000   /* lut registers */
>> -			       0x00 0x034c0000 0x0 0x40000   /* pf controls
>> registers */
>> -			       0x80 0x00000000 0x0 0x20000>; /* configuration
>> space */
>> -			reg-names = "dbi", "lut", "ctrl", "config";
>> +			compatible = "fsl,ls1028a-pcie";
>> +			reg = <0x00 0x03400000 0x0 0x00100000>, /* controller
>> registers */
>> +			      <0x80 0x00000000 0x0 0x00002000>; /* configuration
>> space */
>> +			reg-names = "regs", "config";
>>  			#address-cells = <3>;
>>  			#size-cells = <2>;
>>  			device_type = "pci";
>> @@ -360,12 +358,10 @@
>>  		};
>> 
>>  		pcie2: pcie at 3500000 {
>> -			compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";
>> -			reg = <0x00 0x03500000 0x0 0x80000
>> -			       0x00 0x03580000 0x0 0x40000   /* lut registers */
>> -			       0x00 0x035c0000 0x0 0x40000   /* pf controls
>> registers */
>> -			       0x88 0x00000000 0x0 0x20000>; /* configuration
>> space */
>> -			reg-names = "dbi", "lut", "ctrl", "config";
>> +			compatible = "fsl,ls1028a-pcie";
>> +			reg = <0x00 0x03500000 0x0 0x00100000>, /* controller
>> registers */
>> +			      <0x88 0x00000000 0x0 0x00002000>; /* configuration
>> space */
>> +			reg-names = "regs", "config";
>>  			#address-cells = <3>;
>>  			#size-cells = <2>;
>>  			device_type = "pci";
>> diff --git a/drivers/pci/pcie_layerscape_rc.c
>> b/drivers/pci/pcie_layerscape_rc.c
>> index f50d6ef653..217b420076 100644
>> --- a/drivers/pci/pcie_layerscape_rc.c
>> +++ b/drivers/pci/pcie_layerscape_rc.c
>> @@ -21,6 +21,12 @@
>> 
>>  DECLARE_GLOBAL_DATA_PTR;
>> 
>> +struct ls_pcie_drvdata {
>> +	u32 lut_offset;
>> +	u32 ctrl_offset;
>> +	bool big_endian;
> 
> The endianness property is better only put in the DT nodes.

The whole point of this series is to align the u-boot binding with
the (official) found in linux. There is no endianness property in
the binding:
   
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml
   
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/layerscape-pci.txt

Also from a technical point of view, this property seems redundant
with the compatible string which names a specific CPU.

-michael

> The others looks good for me.
> 
> Thanks,
> Zhiqiang