Message ID | 20231117101817.4401-8-quic_tengfan@quicinc.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | arm64: qcom: add sm8550-aim300 board support | expand |
On 17/11/2023 12:18, Tengfei Fan wrote: > Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, > thus skip pcie_1_phy_aux_clk input clock to GCC. > > Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sm8550-aim300.dts | 32 ++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts > index 202b979da8ca..3aca0a433a00 100644 > --- a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts > +++ b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts > @@ -393,6 +393,38 @@ > }; > }; > > +&gcc { > + clocks = <&bi_tcxo_div2>, <&sleep_clk>, > + <&pcie0_phy>, > + <&pcie1_phy>, > + <0>, > + <&ufs_mem_phy 0>, > + <&ufs_mem_phy 1>, > + <&ufs_mem_phy 2>, > + <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>; > +}; NAK, this should go to sm8550.dtsi unless there is a good reason. > + > +&pcie_1_phy_aux_clk { > + status = "disabled"; > +}; > + > +&pcie0 { > + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; > + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; > + > + pinctrl-0 = <&pcie0_default_state>; > + pinctrl-names = "default"; > + > + status = "okay"; > +}; > + > +&pcie0_phy { > + vdda-phy-supply = <&vreg_l1e_0p88>; > + vdda-pll-supply = <&vreg_l3e_1p2>; > + > + status = "okay"; > +}; > + > &pm8550b_eusb2_repeater { > vdd18-supply = <&vreg_l15b_1p8>; > vdd3-supply = <&vreg_l5b_3p1>;
On 17/11/2023 11:18, Tengfei Fan wrote: > Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, > thus skip pcie_1_phy_aux_clk input clock to GCC. > > Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> You just added this board. Does it mean you added incomplete and wrong DTSU? Best regards, Krzysztof
On 17/11/2023 11:29, Dmitry Baryshkov wrote: > On 17/11/2023 12:18, Tengfei Fan wrote: >> Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, >> thus skip pcie_1_phy_aux_clk input clock to GCC. >> >> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sm8550-aim300.dts | 32 ++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >> index 202b979da8ca..3aca0a433a00 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >> +++ b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >> @@ -393,6 +393,38 @@ >> }; >> }; >> +&gcc { >> + clocks = <&bi_tcxo_div2>, <&sleep_clk>, >> + <&pcie0_phy>, >> + <&pcie1_phy>, >> + <0>, >> + <&ufs_mem_phy 0>, >> + <&ufs_mem_phy 1>, >> + <&ufs_mem_phy 2>, >> + <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>; >> +}; > > NAK, this should go to sm8550.dtsi unless there is a good reason. Actually this is how QRD8550 was designed, so it's fine to mimic. Neil > >> + >> +&pcie_1_phy_aux_clk { >> + status = "disabled"; >> +}; >> + >> +&pcie0 { >> + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; >> + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; >> + >> + pinctrl-0 = <&pcie0_default_state>; >> + pinctrl-names = "default"; >> + >> + status = "okay"; >> +}; >> + >> +&pcie0_phy { >> + vdda-phy-supply = <&vreg_l1e_0p88>; >> + vdda-pll-supply = <&vreg_l3e_1p2>; >> + >> + status = "okay"; >> +}; >> + >> &pm8550b_eusb2_repeater { >> vdd18-supply = <&vreg_l15b_1p8>; >> vdd3-supply = <&vreg_l5b_3p1>; >
On 17.11.2023 11:41, neil.armstrong@linaro.org wrote: > On 17/11/2023 11:29, Dmitry Baryshkov wrote: >> On 17/11/2023 12:18, Tengfei Fan wrote: >>> Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, >>> thus skip pcie_1_phy_aux_clk input clock to GCC. >>> >>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> >>> --- >>> arch/arm64/boot/dts/qcom/sm8550-aim300.dts | 32 ++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>> index 202b979da8ca..3aca0a433a00 100644 >>> --- a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>> +++ b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>> @@ -393,6 +393,38 @@ >>> }; >>> }; >>> +&gcc { >>> + clocks = <&bi_tcxo_div2>, <&sleep_clk>, >>> + <&pcie0_phy>, >>> + <&pcie1_phy>, >>> + <0>, >>> + <&ufs_mem_phy 0>, >>> + <&ufs_mem_phy 1>, >>> + <&ufs_mem_phy 2>, >>> + <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>; >>> +}; >> >> NAK, this should go to sm8550.dtsi unless there is a good reason. > > Actually this is how QRD8550 was designed, so it's fine to mimic. Does CCF not handle this gracefully? Konrad
Le 18/11/2023 à 01:08, Konrad Dybcio a écrit : > On 17.11.2023 11:41, neil.armstrong@linaro.org wrote: >> On 17/11/2023 11:29, Dmitry Baryshkov wrote: >>> On 17/11/2023 12:18, Tengfei Fan wrote: >>>> Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, >>>> thus skip pcie_1_phy_aux_clk input clock to GCC. >>>> >>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> >>>> --- >>>> arch/arm64/boot/dts/qcom/sm8550-aim300.dts | 32 ++++++++++++++++++++++ >>>> 1 file changed, 32 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>> index 202b979da8ca..3aca0a433a00 100644 >>>> --- a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>> +++ b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>> @@ -393,6 +393,38 @@ >>>> }; >>>> }; >>>> +&gcc { >>>> + clocks = <&bi_tcxo_div2>, <&sleep_clk>, >>>> + <&pcie0_phy>, >>>> + <&pcie1_phy>, >>>> + <0>, >>>> + <&ufs_mem_phy 0>, >>>> + <&ufs_mem_phy 1>, >>>> + <&ufs_mem_phy 2>, >>>> + <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>; >>>> +}; >>> >>> NAK, this should go to sm8550.dtsi unless there is a good reason. >> >> Actually this is how QRD8550 was designed, so it's fine to mimic. > Does CCF not handle this gracefully? CCF handles this very gracefully and it's a perfectly valid DT in regard to the bindings... neil > > Konrad
在 11/20/2023 1:59 AM, Neil Armstrong 写道: > Le 18/11/2023 à 01:08, Konrad Dybcio a écrit : >> On 17.11.2023 11:41, neil.armstrong@linaro.org wrote: >>> On 17/11/2023 11:29, Dmitry Baryshkov wrote: >>>> On 17/11/2023 12:18, Tengfei Fan wrote: >>>>> Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, >>>>> thus skip pcie_1_phy_aux_clk input clock to GCC. >>>>> >>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> >>>>> --- >>>>> arch/arm64/boot/dts/qcom/sm8550-aim300.dts | 32 >>>>> ++++++++++++++++++++++ >>>>> 1 file changed, 32 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>>> b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>>> index 202b979da8ca..3aca0a433a00 100644 >>>>> --- a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>>> +++ b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts >>>>> @@ -393,6 +393,38 @@ >>>>> }; >>>>> }; >>>>> +&gcc { >>>>> + clocks = <&bi_tcxo_div2>, <&sleep_clk>, >>>>> + <&pcie0_phy>, >>>>> + <&pcie1_phy>, >>>>> + <0>, >>>>> + <&ufs_mem_phy 0>, >>>>> + <&ufs_mem_phy 1>, >>>>> + <&ufs_mem_phy 2>, >>>>> + <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>; >>>>> +}; >>>> >>>> NAK, this should go to sm8550.dtsi unless there is a good reason. >>> >>> Actually this is how QRD8550 was designed, so it's fine to mimic. >> Does CCF not handle this gracefully? > > CCF handles this very gracefully and it's a perfectly valid DT in regard > to the bindings... > > neil > >> >> Konrad > Thanks Konrad and Neil comments and disscusion this patch, I also will confirm this with internal team.
在 11/17/2023 6:30 PM, Krzysztof Kozlowski 写道: > On 17/11/2023 11:18, Tengfei Fan wrote: >> Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, >> thus skip pcie_1_phy_aux_clk input clock to GCC. >> >> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> > > You just added this board. Does it mean you added incomplete and wrong DTSU? > > Best regards, > Krzysztof > Hi Krzysztof, I will drop PCIe1 setting in dts file because of PCIe1 still have not enable in dts file. Another I understand what your comments means is I should combine all the functions which should be implemented together and submit as a complete patch, right? I will combine all the functions patch to a total patch when I do next version patch series, because there is another your comments also want to me do as so.
diff --git a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts index 202b979da8ca..3aca0a433a00 100644 --- a/arch/arm64/boot/dts/qcom/sm8550-aim300.dts +++ b/arch/arm64/boot/dts/qcom/sm8550-aim300.dts @@ -393,6 +393,38 @@ }; }; +&gcc { + clocks = <&bi_tcxo_div2>, <&sleep_clk>, + <&pcie0_phy>, + <&pcie1_phy>, + <0>, + <&ufs_mem_phy 0>, + <&ufs_mem_phy 1>, + <&ufs_mem_phy 2>, + <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>; +}; + +&pcie_1_phy_aux_clk { + status = "disabled"; +}; + +&pcie0 { + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; + + pinctrl-0 = <&pcie0_default_state>; + pinctrl-names = "default"; + + status = "okay"; +}; + +&pcie0_phy { + vdda-phy-supply = <&vreg_l1e_0p88>; + vdda-pll-supply = <&vreg_l3e_1p2>; + + status = "okay"; +}; + &pm8550b_eusb2_repeater { vdd18-supply = <&vreg_l15b_1p8>; vdd3-supply = <&vreg_l5b_3p1>;
Add PCIe0 nodes used with WCN7851 device. The PCIe1 is not connected, thus skip pcie_1_phy_aux_clk input clock to GCC. Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com> --- arch/arm64/boot/dts/qcom/sm8550-aim300.dts | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+)