Messages in this thread | | | Subject | Re: [PATCH] ARM: dts: db600c: add support to pcie refclk | From | Srinivas Kandagatla <> | Date | Wed, 21 Mar 2018 13:26:19 +0800 |
| |
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 >
| |