lkml.org 
[lkml]   [2018]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] ARM: dts: db600c: add support to pcie refclk
From
Date
Thanks Bjorn for the review comments,

On 21/03/18 11:35, Bjorn Andersson wrote:
> On Tue 20 Mar 17:28 HKT 2018, Srinivas Kandagatla wrote:
>
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patch adds support to external pcie refclk.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> arch/arm/boot/dts/qcom-apq8064-db600c.dts | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064-db600c.dts b/arch/arm/boot/dts/qcom-apq8064-db600c.dts
>> index 50151ef6e912..7587088c122f 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064-db600c.dts
>> +++ b/arch/arm/boot/dts/qcom-apq8064-db600c.dts
>> @@ -45,6 +45,18 @@
>>
>> };
>>
>> + clocks {
>> + compatible = "simple-bus";
>> + pcie_refclk: pcie-refclk {
>
> As the si53154 is used to acquire 3 different clocks (2 pcie and 1 sata)
> it would be nice to accurately represent this in the DT. That said,
> exposing this as a clock allow us to change the representation here
> later. (Both pcie_core_clk and pcie_eth_clk use the same gpio pin as
> enable pin, so I don't know how to represent this...)
>
This is a refclk, which should be always same for both PCIe Controllers
and connected PCIe endpoints.

For sata we should have one more refclk entry of its own.

> So for now, please name this "pcie_core_clk", per the schematics.

This is pcie refclk, if you follow the line in the schematics should see
that it terminates to pcie refclk.

>
>> + pinctrl-0 = <&pcie_pins>;
>> + pinctrl-names = "default";
>> + compatible = "gpio-gate-clock";
>> + clocks = <&rpmcc 27>;
>> + #clock-cells = <0>;
>> + enable-gpios = <&pm8921_gpio 22 GPIO_ACTIVE_HIGH>;
>> + };
>> + };
>> +
>> hdmi-out {
>> compatible = "hdmi-connector";
>> type = "a";
>> @@ -140,6 +152,16 @@
>>
>> qcom,ssbi@500000 {
>> pmic@0 {
>> + gpio@150 {
>> + pcie_pins: pcie_pins {
>
> Please label this "pcie_clk_en_pin" and don't use '_' in the name of the
> node.
>
I agree, Will fix it.
>> + pios {
>
> You can omit this level and just put the properties in directly in the
> pcie-clk-en node.
Okay.

thanks,
srini
>
>> + pins = "gpio22";
>> + function = "normal";
>> + power-source = <PM8921_GPIO_VPH>;
>> + };
>> + };
>> + };
>> +
>
> Regards,
> Bjorn
>

\
 
 \ /
  Last update: 2018-03-21 06:27    [W:0.043 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site