Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

2016-12-27 Thread Jaehoon Chung
On 12/28/2016 01:32 AM, Krzysztof Kozlowski wrote:
> On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
>> Add the Exxynos5433 pcie node for TM2.
>> This pcie device is used for supporting WiFi.
>>
>> And some gpios are already requested from pinctrl. so it doesn't need to
>> initialize.
>> GPJ2-0 is used for supplying to WiFi PCIe chip.
>>
>> Signed-off-by: Jaehoon Chung 
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++
>>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts  | 11 +--
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 
>> ++
>>  3 files changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi 
>> b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>> index ad71247..3e8b728 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
>> @@ -183,6 +183,13 @@
>>  interrupt-controller;
>>  #interrupt-cells = <2>;
>>  };
>> +
>> +pcie_wlanen: pcie-wlanen {
>> +samsung,pins = "gpj2-0";
>> +samsung,pin-function = <0>;
>> +samsung,pin-pud = <3>;
>> +samsung,pin-drv = <3>;
>> +};
>>  };
>>  
>>  &pinctrl_finger {
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
>> b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> index f21bdc2..c84a2ad 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> @@ -737,6 +737,15 @@
>>  bus-width = <4>;
>>  };
>>  
>> +&pcie {
>> +assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
>> +<&cmu_top CLK_MOUT_SCLK_PCIE_100>;
>> +assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
>> +<&cmu_top CLK_MOUT_BUS_PLL_USER>;
>> +assigned-clock-rates = <0>, <1>;
>> +status = "okay";
>> +};
>> +
>>  &pinctrl_alive {
>>  pinctrl-names = "default";
>>  pinctrl-0 = <&initial_alive>;
>> @@ -836,7 +845,6 @@
>>  pinctrl-0 = <&initial_ese>;
>>  
>>  initial_ese: initial-state {
>> -PIN(IN, gpj2-0, DOWN, LV1);
>>  PIN(IN, gpj2-1, DOWN, LV1);
>>  PIN(IN, gpj2-2, DOWN, LV1);
>>  };
>> @@ -851,7 +859,6 @@
>>  PIN(IN, gpr3-1, DOWN, LV1);
>>  PIN(IN, gpr3-2, DOWN, LV1);
>>  PIN(IN, gpr3-3, DOWN, LV1);
>> -PIN(IN, gpr3-7, NONE, LV1);
>>  };
>>  };
>>  
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
>> b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> index 2a15f18..da287f4 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
>> @@ -1457,6 +1457,29 @@
>>  samsung,fsys-sysreg = <&syscon_fsys>;
>>  status = "okay";
>>  };
>> +
>> +pcie: pcie@1570 {
>> +compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
>> +#address-cells = <3>;
>> +#size-cells = <2>;
>> +device_type = "pci";
>> +interrupts = ;
>> +interrupt-names = "intr";
>> +clocks = <&cmu_fsys CLK_PCIE>,
>> +   <&cmu_fsys CLK_PCLK_PCIE_PHY>;
> 
> Here and in the 'reg' property - indentation looks weird. Tabs+spaces
> but not aligned. Either you use spaces to align... or just don't care
> and use tabs. I prefer consistency and below the 'ranges' property is
> aligned.

Will fix.

> 
>> +clock-names = "pcie", "pcie_bus";
>> +num-lanes = <1>;
>> +pinctrl-names = "default";
>> +phys = <&pcie_phy>;
>> +phy-names = "pcie-phy";
>> +pinctrl-0 = <&pcie_bus &pcie_wlanen>;
>> +reg = <0x156b 0x1000>, <0x1570 0x1000>,
>> +<0x0c00 0x1000>;
>> +reg-names = "elbi", "dbi", "config";
> 
> This does not match the bindings documentation.

This is right. Bindings documentation is wrong. :)

And I will put the prefix as "dts" on subject, according to your other comment.

Best Regards,
Jaehoon Chung

> 
> Best regards,
> Krzysztof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 



Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

2016-12-27 Thread Krzysztof Kozlowski
On Tue, Dec 27, 2016 at 6:32 PM, Krzysztof Kozlowski  wrote:
> On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
>> Add the Exxynos5433 pcie node for TM2.
>> This pcie device is used for supporting WiFi.
>>
>> And some gpios are already requested from pinctrl. so it doesn't need to
>> initialize.
>> GPJ2-0 is used for supplying to WiFi PCIe chip.
>>
>> Signed-off-by: Jaehoon Chung 
>> ---


Ahhh, and one more comment - please add missing 'dts' prefix in the subject.

BR,
Krzysztof


Re: [RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

2016-12-27 Thread Krzysztof Kozlowski
On Mon, Dec 26, 2016 at 02:20:29PM +0900, Jaehoon Chung wrote:
> Add the Exxynos5433 pcie node for TM2.
> This pcie device is used for supporting WiFi.
> 
> And some gpios are already requested from pinctrl. so it doesn't need to
> initialize.
> GPJ2-0 is used for supplying to WiFi PCIe chip.
> 
> Signed-off-by: Jaehoon Chung 
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts  | 11 +--
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 
> ++
>  3 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi 
> b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> index ad71247..3e8b728 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
> @@ -183,6 +183,13 @@
>   interrupt-controller;
>   #interrupt-cells = <2>;
>   };
> +
> + pcie_wlanen: pcie-wlanen {
> + samsung,pins = "gpj2-0";
> + samsung,pin-function = <0>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <3>;
> + };
>  };
>  
>  &pinctrl_finger {
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
> b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index f21bdc2..c84a2ad 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -737,6 +737,15 @@
>   bus-width = <4>;
>  };
>  
> +&pcie {
> + assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
> + <&cmu_top CLK_MOUT_SCLK_PCIE_100>;
> + assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
> + <&cmu_top CLK_MOUT_BUS_PLL_USER>;
> + assigned-clock-rates = <0>, <1>;
> + status = "okay";
> +};
> +
>  &pinctrl_alive {
>   pinctrl-names = "default";
>   pinctrl-0 = <&initial_alive>;
> @@ -836,7 +845,6 @@
>   pinctrl-0 = <&initial_ese>;
>  
>   initial_ese: initial-state {
> - PIN(IN, gpj2-0, DOWN, LV1);
>   PIN(IN, gpj2-1, DOWN, LV1);
>   PIN(IN, gpj2-2, DOWN, LV1);
>   };
> @@ -851,7 +859,6 @@
>   PIN(IN, gpr3-1, DOWN, LV1);
>   PIN(IN, gpr3-2, DOWN, LV1);
>   PIN(IN, gpr3-3, DOWN, LV1);
> - PIN(IN, gpr3-7, NONE, LV1);
>   };
>  };
>  
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
> b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 2a15f18..da287f4 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -1457,6 +1457,29 @@
>   samsung,fsys-sysreg = <&syscon_fsys>;
>   status = "okay";
>   };
> +
> + pcie: pcie@1570 {
> + compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + interrupts = ;
> + interrupt-names = "intr";
> + clocks = <&cmu_fsys CLK_PCIE>,
> +<&cmu_fsys CLK_PCLK_PCIE_PHY>;

Here and in the 'reg' property - indentation looks weird. Tabs+spaces
but not aligned. Either you use spaces to align... or just don't care
and use tabs. I prefer consistency and below the 'ranges' property is
aligned.

> + clock-names = "pcie", "pcie_bus";
> + num-lanes = <1>;
> + pinctrl-names = "default";
> + phys = <&pcie_phy>;
> + phy-names = "pcie-phy";
> + pinctrl-0 = <&pcie_bus &pcie_wlanen>;
> + reg = <0x156b 0x1000>, <0x1570 0x1000>,
> + <0x0c00 0x1000>;
> + reg-names = "elbi", "dbi", "config";

This does not match the bindings documentation.

Best regards,
Krzysztof


[RFC PATCH 6/6] ARM64: exynos: add the pcie node for TM2

2016-12-25 Thread Jaehoon Chung
Add the Exxynos5433 pcie node for TM2.
This pcie device is used for supporting WiFi.

And some gpios are already requested from pinctrl. so it doesn't need to
initialize.
GPJ2-0 is used for supplying to WiFi PCIe chip.

Signed-off-by: Jaehoon Chung 
---
 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi |  7 +++
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts  | 11 +--
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 ++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi 
b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
index ad71247..3e8b728 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
@@ -183,6 +183,13 @@
interrupt-controller;
#interrupt-cells = <2>;
};
+
+   pcie_wlanen: pcie-wlanen {
+   samsung,pins = "gpj2-0";
+   samsung,pin-function = <0>;
+   samsung,pin-pud = <3>;
+   samsung,pin-drv = <3>;
+   };
 };
 
 &pinctrl_finger {
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts 
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index f21bdc2..c84a2ad 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -737,6 +737,15 @@
bus-width = <4>;
 };
 
+&pcie {
+   assigned-clocks = <&cmu_fsys CLK_MOUT_SCLK_PCIE_100_USER>,
+   <&cmu_top CLK_MOUT_SCLK_PCIE_100>;
+   assigned-clock-parents = <&cmu_top CLK_SCLK_PCIE_100_FSYS>,
+   <&cmu_top CLK_MOUT_BUS_PLL_USER>;
+   assigned-clock-rates = <0>, <1>;
+   status = "okay";
+};
+
 &pinctrl_alive {
pinctrl-names = "default";
pinctrl-0 = <&initial_alive>;
@@ -836,7 +845,6 @@
pinctrl-0 = <&initial_ese>;
 
initial_ese: initial-state {
-   PIN(IN, gpj2-0, DOWN, LV1);
PIN(IN, gpj2-1, DOWN, LV1);
PIN(IN, gpj2-2, DOWN, LV1);
};
@@ -851,7 +859,6 @@
PIN(IN, gpr3-1, DOWN, LV1);
PIN(IN, gpr3-2, DOWN, LV1);
PIN(IN, gpr3-3, DOWN, LV1);
-   PIN(IN, gpr3-7, NONE, LV1);
};
 };
 
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 2a15f18..da287f4 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1457,6 +1457,29 @@
samsung,fsys-sysreg = <&syscon_fsys>;
status = "okay";
};
+
+   pcie: pcie@1570 {
+   compatible = "samsung,exynos5433-pcie", "snps,dw-pcie";
+   #address-cells = <3>;
+   #size-cells = <2>;
+   device_type = "pci";
+   interrupts = ;
+   interrupt-names = "intr";
+   clocks = <&cmu_fsys CLK_PCIE>,
+  <&cmu_fsys CLK_PCLK_PCIE_PHY>;
+   clock-names = "pcie", "pcie_bus";
+   num-lanes = <1>;
+   pinctrl-names = "default";
+   phys = <&pcie_phy>;
+   phy-names = "pcie-phy";
+   pinctrl-0 = <&pcie_bus &pcie_wlanen>;
+   reg = <0x156b 0x1000>, <0x1570 0x1000>,
+   <0x0c00 0x1000>;
+   reg-names = "elbi", "dbi", "config";
+   ranges = <0x8100 0 0  0x0c001000 0 
0x0001
+ 0x8200 0 0x0c011000 0x0c011000 0 
0x3feefff>;
+   status = "disabled";
+   };
};
 
timer: timer {
-- 
2.10.2