diff mbox series

[5/8] PCI: rockchip: Added dtsi entry for PCIe endpoint controller

Message ID 20230126135049.708524-6-rick.wertenbroek@gmail.com
State New
Headers show
Series PCI: rockchip: Fix PCIe endpoint controller driver | expand

Commit Message

Rick Wertenbroek Jan. 26, 2023, 1:50 p.m. UTC
Added missing PCIe endpoint controller entry in the device tree. This
entry is documented in :
Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
The status is disabled by default, so it will not be loaded unless
explicitly chosen to.

Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Krzysztof Kozlowski Jan. 26, 2023, 3:23 p.m. UTC | #1
On 26/01/2023 14:50, Rick Wertenbroek wrote:
> Added missing PCIe endpoint controller entry in the device tree. This
> entry is documented in :
> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt

There is no such file

> The status is disabled by default, so it will not be loaded unless
> explicitly chosen to.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

> 
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 9d5b0e8c9..5f7251118 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
>  		};
>  	};
>  
> +	pcie0_ep: pcie-ep@f8000000 {
> +		compatible = "rockchip,rk3399-pcie-ep";

reg is usually second property...
> +		#address-cells = <3>;
> +		#size-cells = <2>;
Best regards,
Krzysztof
Rick Wertenbroek Jan. 26, 2023, 3:30 p.m. UTC | #2
On Thu, Jan 26, 2023 at 4:23 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/01/2023 14:50, Rick Wertenbroek wrote:
> > Added missing PCIe endpoint controller entry in the device tree. This
> > entry is documented in :
> > Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
>
> There is no such file

Sorry but the file exists see :
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=v6.0.19
It also exists in 6.1 :
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=linux-6.1.y

>
> > The status is disabled by default, so it will not be loaded unless
> > explicitly chosen to.
>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>

Sorry about this, you are right, I'll fix it for the next iteration.
Thank you

> >
> > Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > index 9d5b0e8c9..5f7251118 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
> >               };
> >       };
> >
> > +     pcie0_ep: pcie-ep@f8000000 {
> > +             compatible = "rockchip,rk3399-pcie-ep";
>
> reg is usually second property...
> > +             #address-cells = <3>;
> > +             #size-cells = <2>;
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Jan. 26, 2023, 3:43 p.m. UTC | #3
On 26/01/2023 16:30, Rick Wertenbroek wrote:
> On Thu, Jan 26, 2023 at 4:23 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 26/01/2023 14:50, Rick Wertenbroek wrote:
>>> Added missing PCIe endpoint controller entry in the device tree. This
>>> entry is documented in :
>>> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
>>
>> There is no such file
> 
> Sorry but the file exists see :
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=v6.0.19
> It also exists in 6.1 :
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt?h=linux-6.1.y

It's some old tree. It could have existed in the past. But it does not
exist. Don't refer to non-existing paths.

Your code needs then rebasing if you refer to some old trees. Please
always work on latest maintainer's tree. Optionally on linux-next.

Best regards,
Krzysztof
ALOK TIWARI Jan. 27, 2023, 8:42 a.m. UTC | #4
DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dtb
../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning 
(pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not 
configuration space
   DTC arch/arm64/boot/dts/amlogic/meson-gxm-s912-libretech-pc.dtb
../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning 
(pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not 
configuration space
   HDRINST usr/include/linux/aio_abi.h
   HDRINST usr/include/linux/am437x-vpfe.h
../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning 
(pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not 
configuration space
   DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm017-dc3.dtb


Thanks,

Alok

On 1/26/2023 7:20 PM, Rick Wertenbroek wrote:
> Added missing PCIe endpoint controller entry in the device tree. This
> entry is documented in :
> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
> The status is disabled by default, so it will not be loaded unless
> explicitly chosen to.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 9d5b0e8c9..5f7251118 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
>   		};
>   	};
>   
> +	pcie0_ep: pcie-ep@f8000000 {
> +		compatible = "rockchip,rk3399-pcie-ep";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		rockchip,max-outbound-regions = <32>;
> +		clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
> +			<&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
> +		clock-names = "aclk", "aclk-perf",
> +				"hclk", "pm";
> +		max-functions = /bits/ 8 <8>;
> +		num-lanes = <4>;
> +		reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
> +		reg-names = "apb-base", "mem-base";
> +		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> +			<&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
> +			<&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
> +		reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +				"pm", "pclk", "aclk";
> +		phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
> +		phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_clkreqnb_cpm>;
> +		status = "disabled";
> +	};
> +
>   	gmac: ethernet@fe300000 {
>   		compatible = "rockchip,rk3399-gmac";
>   		reg = <0x0 0xfe300000 0x0 0x10000>;
Rick Wertenbroek Jan. 30, 2023, 1:52 p.m. UTC | #5
On Fri, Jan 27, 2023 at 9:43 AM ALOK TIWARI <alok.a.tiwari@oracle.com> wrote:
>
>    DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dtb
> ../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning
> (pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not
> configuration space
>    DTC arch/arm64/boot/dts/amlogic/meson-gxm-s912-libretech-pc.dtb
> ../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning
> (pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not
> configuration space
>    HDRINST usr/include/linux/aio_abi.h
>    HDRINST usr/include/linux/am437x-vpfe.h
> ../arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi:460.3-52: Warning
> (pci_device_reg): /pcie@f8000000/pcie@0,0:reg: PCI reg address is not
> configuration space
>    DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm017-dc3.dtb
>
>
> Thanks,
>
> Alok

These warnings are for the pcie (host controller), I did not touch that entry.
Plus they are for another file (arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi)
They are related to "pci_device_reg" warnings (can be suppressed with
"-Wno-pci_device_reg").

Sorry, but this does not seem to be related to my change and I don't know
how to fix this.

Rick
Rob Herring Jan. 30, 2023, 3:04 p.m. UTC | #6
On Thu, Jan 26, 2023 at 7:52 AM Rick Wertenbroek
<rick.wertenbroek@gmail.com> wrote:
>
> Added missing PCIe endpoint controller entry in the device tree. This
> entry is documented in :
> Documentation/devicetree/bindings/pci/rockchip-pcie-ep.txt
> The status is disabled by default, so it will not be loaded unless
> explicitly chosen to.
>
> Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 9d5b0e8c9..5f7251118 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -265,6 +265,31 @@ pcie0_intc: interrupt-controller {
>                 };
>         };
>
> +       pcie0_ep: pcie-ep@f8000000 {
> +               compatible = "rockchip,rk3399-pcie-ep";
> +               #address-cells = <3>;
> +               #size-cells = <2>;

These are only needed when you have child nodes. Additionally, it
would not be a PCI bus which is the only case that has 3 address
cells.

There's a schema for this in linux-next now. Please test this change
with that. It should point out the above issue and maybe others.

> +               rockchip,max-outbound-regions = <32>;
> +               clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
> +                       <&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
> +               clock-names = "aclk", "aclk-perf",
> +                               "hclk", "pm";
> +               max-functions = /bits/ 8 <8>;
> +               num-lanes = <4>;
> +               reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
> +               reg-names = "apb-base", "mem-base";
> +               resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> +                       <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
> +                       <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
> +               reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +                               "pm", "pclk", "aclk";
> +               phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
> +               phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pcie_clkreqnb_cpm>;
> +               status = "disabled";
> +       };
> +
>         gmac: ethernet@fe300000 {
>                 compatible = "rockchip,rk3399-gmac";
>                 reg = <0x0 0xfe300000 0x0 0x10000>;
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 9d5b0e8c9..5f7251118 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -265,6 +265,31 @@  pcie0_intc: interrupt-controller {
 		};
 	};
 
+	pcie0_ep: pcie-ep@f8000000 {
+		compatible = "rockchip,rk3399-pcie-ep";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		rockchip,max-outbound-regions = <32>;
+		clocks = <&cru ACLK_PCIE>, <&cru ACLK_PERF_PCIE>,
+			<&cru PCLK_PCIE>, <&cru SCLK_PCIE_PM>;
+		clock-names = "aclk", "aclk-perf",
+				"hclk", "pm";
+		max-functions = /bits/ 8 <8>;
+		num-lanes = <4>;
+		reg = <0x0 0xfd000000 0x0 0x1000000>, <0x0 0xfa000000 0x0 0x2000000>;
+		reg-names = "apb-base", "mem-base";
+		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
+			<&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
+			<&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
+		reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
+				"pm", "pclk", "aclk";
+		phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
+		phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pcie_clkreqnb_cpm>;
+		status = "disabled";
+	};
+
 	gmac: ethernet@fe300000 {
 		compatible = "rockchip,rk3399-gmac";
 		reg = <0x0 0xfe300000 0x0 0x10000>;