| Message ID | 20180320092840.4600-1-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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...) So for now, please name this "pcie_core_clk", per the schematics. > + 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. > + pios { You can omit this level and just put the properties in directly in the pcie-clk-en node. > + pins = "gpio22"; > + function = "normal"; > + power-source = <PM8921_GPIO_VPH>; > + }; > + }; > + }; > + Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 { + 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 { + pios { + pins = "gpio22"; + function = "normal"; + power-source = <PM8921_GPIO_VPH>; + }; + }; + }; + mpps@50 { pcie_perst: pcie-perst { pinconf { @@ -439,6 +461,11 @@ pinctrl-0 = <&pcie_perst>; pinctrl-names = "default"; perst-gpio = <&pm8921_mpps 1 GPIO_ACTIVE_LOW>; + clocks = <&gcc PCIE_A_CLK>, + <&gcc PCIE_H_CLK>, + <&gcc PCIE_PHY_REF_CLK>, + <&pcie_refclk>; + clock-names = "core", "iface", "phy", "ref"; }; /* OTG */