On 19:36-20200901, Kishon Vijay Abraham I wrote:
> Add PCIe device tree node (both RC and EP) for the four
> PCIe instances here.
> 
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
>  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 218 ++++++++++++++++++++++
>  arch/arm64/boot/dts/ti/k3-j721e.dtsi      |   5 +-
>  2 files changed, 222 insertions(+), 1 deletion(-)


Did you look at the diff of the dtbs_check before and after this
series? I see: https://pastebin.ubuntu.com/p/9fyfrTjx9M/

> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi 
> b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> index 00a36a14efe7..a36909d8b8c3 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> @@ -28,6 +28,26 @@
>               #size-cells = <1>;
>               ranges = <0x0 0x0 0x00100000 0x1c000>;
>  
> +             pcie0_ctrl: pcie-ctrl@4070 {
https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf
Section 2.2.2: why not use syscon@4070 and so on?

> +                     compatible = "syscon";
> +                     reg = <0x00004070 0x4>;
> +             };
> +
> +             pcie1_ctrl: pcie-ctrl@4074 {
> +                     compatible = "syscon";
> +                     reg = <0x00004074 0x4>;
> +             };
> +
> +             pcie2_ctrl: pcie-ctrl@4078 {
> +                     compatible = "syscon";
> +                     reg = <0x00004078 0x4>;
> +             };
> +
> +             pcie3_ctrl: pcie-ctrl@407c {
> +                     compatible = "syscon";
> +                     reg = <0x0000407c 0x4>;
> +             };
> +
>               serdes_ln_ctrl: serdes-ln-ctrl@4080 {
>                       compatible = "mmio-mux";
>                       reg = <0x00004080 0x50>;
> @@ -576,6 +596,204 @@
>               };
>       };
>  
> +     pcie0_rc: pcie@2900000 {
> +             compatible = "ti,j721e-pcie-host";
> +             reg = <0x00 0x02900000 0x00 0x1000>,
> +                   <0x00 0x02907000 0x00 0x400>,
> +                   <0x00 0x0d000000 0x00 0x00800000>,
> +                   <0x00 0x10000000 0x00 0x00001000>;
> +             reg-names = "intd_cfg", "user_cfg", "reg", "cfg";
> +             interrupt-names = "link_state";
> +             interrupts = <GIC_SPI 318 IRQ_TYPE_EDGE_RISING>;
> +             device_type = "pci";
> +             ti,syscon-pcie-ctrl = <&pcie0_ctrl>;
> +             max-link-speed = <3>;
> +             num-lanes = <2>;
> +             power-domains = <&k3_pds 239 TI_SCI_PD_EXCLUSIVE>;
> +             clocks = <&k3_clks 239 1>;
> +             clock-names = "fck";
> +             #address-cells = <3>;
> +             #size-cells = <2>;
> +             bus-range = <0x0 0xf>;
> +             vendor-id = <0x104c>;
> +             device-id = <0xb00d>;
> +             msi-map = <0x0 &gic_its 0x0 0x10000>;
> +             dma-coherent;
> +             ranges = <0x01000000 0x0 0x10001000 0x0 0x10001000 0x0 
> 0x0010000>,
> +                      <0x02000000 0x0 0x10011000 0x0 0x10011000 0x0 
> 0x7fef000>;
> +             dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>;
> +     };
> +
> +     pcie0_ep: pcie-ep@2900000 {
Not related to this patch, but just a suggestion: pcie-ep -> do we
need to add that to the Generic names in DT spec?

[...]
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi 
> b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> index f787aa73aaae..eeb02115b966 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> @@ -132,9 +132,12 @@
>                        <0x00 0x06400000 0x00 0x06400000 0x00 0x00400000>, /* 
> USBSS1 */
>                        <0x00 0x01000000 0x00 0x01000000 0x00 0x0af02400>, /* 
> Most peripherals */
>                        <0x00 0x30000000 0x00 0x30000000 0x00 0x0c400000>, /* 
> MAIN NAVSS */
> -                      <0x00 0x0d000000 0x00 0x0d000000 0x00 0x01000000>, /* 
> PCIe Core*/
> +                      <0x00 0x0d000000 0x00 0x0d000000 0x00 0x01800000>, /* 
> PCIe Core*/
> +                      <0x00 0x0e000000 0x00 0x0e000000 0x00 0x01800000>, /* 
> PCIe Core*/
>                        <0x00 0x10000000 0x00 0x10000000 0x00 0x10000000>, /* 
> PCIe DAT */
                                                                                
^^
                                                                        should 
be PCIe1?
Just because you are introducing PCIe2,3 in this patch, the net result
does'nt look consistent? Also might want to cover this change in the
$commit_message.
[...]
>                        <0x00 0x64800000 0x00 0x64800000 0x00 0x00800000>, /* 
> C71 */
> +                      <0x44 0x00000000 0x44 0x00000000 0x00 0x08000000>, /* 
> PCIe2 DAT */
> +                      <0x44 0x10000000 0x44 0x10000000 0x00 0x08000000>, /* 
> PCIe3 DAT */
>                        <0x4d 0x80800000 0x4d 0x80800000 0x00 0x00800000>, /* 
> C66_0 */
>                        <0x4d 0x81800000 0x4d 0x81800000 0x00 0x00800000>, /* 
> C66_1 */
>                        <0x4e 0x20000000 0x4e 0x20000000 0x00 0x00080000>, /* 
> GPU */
> -- 
> 2.17.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Reply via email to