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

Hi Zhiqiang,

thanks for looking at this patch.

Am 2021-10-13 03:46, schrieb Z.Q. Hou:

...

-----Original Message----- From: Michael Walle michael@walle.cc Sent: 2021年10月5日 16:38 To: u-boot@lists.denx.de Cc: Jagan Teki jagan@amarulasolutions.com; Priyanka Jain priyanka.jain@nxp.com; Vladimir Oltean vladimir.oltean@nxp.com; Tom Rini trini@konsulko.com; Peter Griffin peter.griffin@linaro.org; Manivannan Sadhasivam manivannan.sadhasivam@linaro.org; Michael Walle michael@walle.cc; Z.Q. Hou zhiqiang.hou@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@nxp.com Signed-off-by: Michael Walle michael@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@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@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/bindi...

https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindi...

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