| Message ID | 20230130-topic-sm8450-upstream-pmic-glink-v2-8-71fea256474f@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: qcom: add UCSI function to PMIC GLINK | expand |
On 10.02.2023 16:02, Neil Armstrong wrote: > Add the pmic glink node linked with the DWC3 USB controller > switched to OTG mode and tagged with usb-role-switch. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 77 ++++++++++++++++++++++++++++----- > 1 file changed, 65 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index 54654eb75c28..28fc9a835c5d 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -31,6 +31,40 @@ hdmi_con: endpoint { > }; > }; > > + pmic-glink { > + compatible = "qcom,sm8350-pmic-glink", "qcom,pmic-glink"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + power-role = "dual"; > + data-role = "dual"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + pmic_glink_hs_in: endpoint { > + remote-endpoint = <&usb_1_dwc3_hs>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + pmic_glink_ss_in: endpoint { > + remote-endpoint = <&usb_1_dwc3_ss>; > + }; > + }; > + }; > + }; > + }; > + > vph_pwr: vph-pwr-regulator { > compatible = "regulator-fixed"; > regulator-name = "vph_pwr"; > @@ -666,23 +700,42 @@ &usb_1 { > }; > > &usb_1_dwc3 { > - /* TODO: Define USB-C connector properly */ > - dr_mode = "peripheral"; > -}; > + dr_mode = "otg"; > + usb-role-switch; > > -&usb_1_hsphy { Are you removing the hsphy completely? Konrad > - status = "okay"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > > - vdda-pll-supply = <&vreg_l5b_0p88>; > - vdda18-supply = <&vreg_l1c_1p8>; > - vdda33-supply = <&vreg_l2b_3p07>; > + port@0 { > + reg = <0>; > + > + usb_1_dwc3_hs: endpoint { > + remote-endpoint = <&pmic_glink_hs_in>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + usb_1_dwc3_ss: endpoint { > + remote-endpoint = <&pmic_glink_ss_in>; > + }; > + }; > + }; > }; > > -&usb_1_qmpphy { > - status = "okay"; > +&usb_1_dwc3 { > + dr_mode = "otg"; > + usb-role-switch; > +}; > > - vdda-phy-supply = <&vreg_l6b_1p2>; > - vdda-pll-supply = <&vreg_l1b_0p88>; > +&usb_1_dwc3_hs { > + remote-endpoint = <&pmic_glink_hs_in>; > +}; > + > +&usb_1_dwc3_ss { > + remote-endpoint = <&pmic_glink_ss_in>; > }; > > &usb_2 { >
On 10/02/2023 16:21, Konrad Dybcio wrote: > > > On 10.02.2023 16:02, Neil Armstrong wrote: >> Add the pmic glink node linked with the DWC3 USB controller >> switched to OTG mode and tagged with usb-role-switch. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 77 ++++++++++++++++++++++++++++----- >> 1 file changed, 65 insertions(+), 12 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts >> index 54654eb75c28..28fc9a835c5d 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts >> +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts >> @@ -31,6 +31,40 @@ hdmi_con: endpoint { >> }; >> }; >> >> + pmic-glink { >> + compatible = "qcom,sm8350-pmic-glink", "qcom,pmic-glink"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + connector@0 { >> + compatible = "usb-c-connector"; >> + reg = <0>; >> + power-role = "dual"; >> + data-role = "dual"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + >> + pmic_glink_hs_in: endpoint { >> + remote-endpoint = <&usb_1_dwc3_hs>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + >> + pmic_glink_ss_in: endpoint { >> + remote-endpoint = <&usb_1_dwc3_ss>; >> + }; >> + }; >> + }; >> + }; >> + }; >> + >> vph_pwr: vph-pwr-regulator { >> compatible = "regulator-fixed"; >> regulator-name = "vph_pwr"; >> @@ -666,23 +700,42 @@ &usb_1 { >> }; >> >> &usb_1_dwc3 { >> - /* TODO: Define USB-C connector properly */ >> - dr_mode = "peripheral"; >> -}; >> + dr_mode = "otg"; >> + usb-role-switch; >> >> -&usb_1_hsphy { > Are you removing the hsphy completely? Oops seems I did remove too much stuff... thx for noticing > > Konrad >> - status = "okay"; >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> >> - vdda-pll-supply = <&vreg_l5b_0p88>; >> - vdda18-supply = <&vreg_l1c_1p8>; >> - vdda33-supply = <&vreg_l2b_3p07>; >> + port@0 { >> + reg = <0>; >> + >> + usb_1_dwc3_hs: endpoint { >> + remote-endpoint = <&pmic_glink_hs_in>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + >> + usb_1_dwc3_ss: endpoint { >> + remote-endpoint = <&pmic_glink_ss_in>; >> + }; >> + }; >> + }; >> }; >> >> -&usb_1_qmpphy { >> - status = "okay"; >> +&usb_1_dwc3 { >> + dr_mode = "otg"; >> + usb-role-switch; >> +}; >> >> - vdda-phy-supply = <&vreg_l6b_1p2>; >> - vdda-pll-supply = <&vreg_l1b_0p88>; >> +&usb_1_dwc3_hs { >> + remote-endpoint = <&pmic_glink_hs_in>; >> +}; >> + >> +&usb_1_dwc3_ss { >> + remote-endpoint = <&pmic_glink_ss_in>; >> }; >> >> &usb_2 { >>
On 10/02/2023 17:02, Neil Armstrong wrote: > Add the pmic glink node linked with the DWC3 USB controller > switched to OTG mode and tagged with usb-role-switch. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 77 ++++++++++++++++++++++++++++----- > 1 file changed, 65 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index 54654eb75c28..28fc9a835c5d 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -31,6 +31,40 @@ hdmi_con: endpoint { > }; > }; > > + pmic-glink { > + compatible = "qcom,sm8350-pmic-glink", "qcom,pmic-glink"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + power-role = "dual"; > + data-role = "dual"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + pmic_glink_hs_in: endpoint { > + remote-endpoint = <&usb_1_dwc3_hs>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + pmic_glink_ss_in: endpoint { > + remote-endpoint = <&usb_1_dwc3_ss>; > + }; > + }; > + }; > + }; > + }; > + > vph_pwr: vph-pwr-regulator { > compatible = "regulator-fixed"; > regulator-name = "vph_pwr"; > @@ -666,23 +700,42 @@ &usb_1 { > }; > > &usb_1_dwc3 { > - /* TODO: Define USB-C connector properly */ > - dr_mode = "peripheral"; > -}; > + dr_mode = "otg"; I could not verify UCSI / OTG on sm8350-HDK, see the GET_PDOS issue. Unless it is fixed, I suggest fixing this to "host" to get DP to work. > + usb-role-switch; > > -&usb_1_hsphy { > - status = "okay"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > > - vdda-pll-supply = <&vreg_l5b_0p88>; > - vdda18-supply = <&vreg_l1c_1p8>; > - vdda33-supply = <&vreg_l2b_3p07>; > + port@0 { > + reg = <0>; > + > + usb_1_dwc3_hs: endpoint { > + remote-endpoint = <&pmic_glink_hs_in>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + usb_1_dwc3_ss: endpoint { > + remote-endpoint = <&pmic_glink_ss_in>; > + }; > + }; > + }; > }; > > -&usb_1_qmpphy { > - status = "okay"; > +&usb_1_dwc3 { > + dr_mode = "otg"; > + usb-role-switch; > +}; Duplicate > > - vdda-phy-supply = <&vreg_l6b_1p2>; > - vdda-pll-supply = <&vreg_l1b_0p88>; > +&usb_1_dwc3_hs { > + remote-endpoint = <&pmic_glink_hs_in>; > +}; > + > +&usb_1_dwc3_ss { > + remote-endpoint = <&pmic_glink_ss_in>; > }; > > &usb_2 { >
On Fri, Feb 10, 2023 at 04:02:11PM +0100, Neil Armstrong wrote: > Add the pmic glink node linked with the DWC3 USB controller > switched to OTG mode and tagged with usb-role-switch. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 77 ++++++++++++++++++++++++++++----- > 1 file changed, 65 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > index 54654eb75c28..28fc9a835c5d 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts > @@ -31,6 +31,40 @@ hdmi_con: endpoint { > }; > }; > > + pmic-glink { > + compatible = "qcom,sm8350-pmic-glink", "qcom,pmic-glink"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + power-role = "dual"; > + data-role = "dual"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + pmic_glink_hs_in: endpoint { > + remote-endpoint = <&usb_1_dwc3_hs>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + pmic_glink_ss_in: endpoint { > + remote-endpoint = <&usb_1_dwc3_ss>; > + }; > + }; > + }; > + }; > + }; > + > vph_pwr: vph-pwr-regulator { > compatible = "regulator-fixed"; > regulator-name = "vph_pwr"; > @@ -666,23 +700,42 @@ &usb_1 { > }; > > &usb_1_dwc3 { > - /* TODO: Define USB-C connector properly */ > - dr_mode = "peripheral"; > -}; > + dr_mode = "otg"; > + usb-role-switch; > > -&usb_1_hsphy { > - status = "okay"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > > - vdda-pll-supply = <&vreg_l5b_0p88>; > - vdda18-supply = <&vreg_l1c_1p8>; > - vdda33-supply = <&vreg_l2b_3p07>; > + port@0 { > + reg = <0>; > + > + usb_1_dwc3_hs: endpoint { > + remote-endpoint = <&pmic_glink_hs_in>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + usb_1_dwc3_ss: endpoint { > + remote-endpoint = <&pmic_glink_ss_in>; The connector is indeed the next active component on the SuperSpeed lanes for USB. But as you're targeting to introduce QMP on that path, connector@0/port@1 would be pointing to QMP/ports/port@N. Do you plan to express the datapath between USB and QMP using this port at that time? (It's the correct thing to do...) Or will we not describe the SS lanes in this scenario? Regards, Bjorn > + }; > + }; > + }; > }; > > -&usb_1_qmpphy { > - status = "okay"; > +&usb_1_dwc3 { > + dr_mode = "otg"; > + usb-role-switch; > +}; > > - vdda-phy-supply = <&vreg_l6b_1p2>; > - vdda-pll-supply = <&vreg_l1b_0p88>; > +&usb_1_dwc3_hs { > + remote-endpoint = <&pmic_glink_hs_in>; > +}; > + > +&usb_1_dwc3_ss { > + remote-endpoint = <&pmic_glink_ss_in>; > }; > > &usb_2 { > > -- > 2.34.1 >
On 10/02/2023 21:13, Bjorn Andersson wrote: > On Fri, Feb 10, 2023 at 04:02:11PM +0100, Neil Armstrong wrote: >> Add the pmic glink node linked with the DWC3 USB controller >> switched to OTG mode and tagged with usb-role-switch. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 77 ++++++++++++++++++++++++++++----- >> 1 file changed, 65 insertions(+), 12 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts >> index 54654eb75c28..28fc9a835c5d 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts >> +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts >> @@ -31,6 +31,40 @@ hdmi_con: endpoint { >> }; >> }; >> >> + pmic-glink { >> + compatible = "qcom,sm8350-pmic-glink", "qcom,pmic-glink"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + connector@0 { >> + compatible = "usb-c-connector"; >> + reg = <0>; >> + power-role = "dual"; >> + data-role = "dual"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + >> + pmic_glink_hs_in: endpoint { >> + remote-endpoint = <&usb_1_dwc3_hs>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + >> + pmic_glink_ss_in: endpoint { >> + remote-endpoint = <&usb_1_dwc3_ss>; >> + }; >> + }; >> + }; >> + }; >> + }; >> + >> vph_pwr: vph-pwr-regulator { >> compatible = "regulator-fixed"; >> regulator-name = "vph_pwr"; >> @@ -666,23 +700,42 @@ &usb_1 { >> }; >> >> &usb_1_dwc3 { >> - /* TODO: Define USB-C connector properly */ >> - dr_mode = "peripheral"; >> -}; >> + dr_mode = "otg"; >> + usb-role-switch; >> >> -&usb_1_hsphy { >> - status = "okay"; >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> >> - vdda-pll-supply = <&vreg_l5b_0p88>; >> - vdda18-supply = <&vreg_l1c_1p8>; >> - vdda33-supply = <&vreg_l2b_3p07>; >> + port@0 { >> + reg = <0>; >> + >> + usb_1_dwc3_hs: endpoint { >> + remote-endpoint = <&pmic_glink_hs_in>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + >> + usb_1_dwc3_ss: endpoint { >> + remote-endpoint = <&pmic_glink_ss_in>; > > The connector is indeed the next active component on the SuperSpeed > lanes for USB. But as you're targeting to introduce QMP on that path, > connector@0/port@1 would be pointing to QMP/ports/port@N. > > Do you plan to express the datapath between USB and QMP using this port > at that time? (It's the correct thing to do...) Yeah we need to figure out, because ultimately the datapath should be: connector: port0 --> usb_1_dwc3_hs port1 --> qmpphy --> usb_1_dwc3_ss port2 --> fsa4480 --> qmpphy -> dp_controller And with a retimer, it gets even more complex: connector: port0 --> usb_1_dwc3_hs port1 --> retimer --> qmpphy --> usb_1_dwc3_ss port2 --> retimer --> qmpphy -> dp_controller The solution I was using is instead of having chained port/endpoints, I use multiple endpoints like : port0 --> usb_1_dwc3_hs port1 --> ep0: usb_1_dwc3_ss ep1: qmpphy port2 --> ep0: fsa4480 ep1: dp_controller But I'm sure this is valid... but it is much simpler. Neil > > Or will we not describe the SS lanes in this scenario? > > Regards, > Bjorn > >> + }; >> + }; >> + }; >> }; >> >> -&usb_1_qmpphy { >> - status = "okay"; >> +&usb_1_dwc3 { >> + dr_mode = "otg"; >> + usb-role-switch; >> +}; >> >> - vdda-phy-supply = <&vreg_l6b_1p2>; >> - vdda-pll-supply = <&vreg_l1b_0p88>; >> +&usb_1_dwc3_hs { >> + remote-endpoint = <&pmic_glink_hs_in>; >> +}; >> + >> +&usb_1_dwc3_ss { >> + remote-endpoint = <&pmic_glink_ss_in>; >> }; >> >> &usb_2 { >> >> -- >> 2.34.1 >>
diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts index 54654eb75c28..28fc9a835c5d 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts @@ -31,6 +31,40 @@ hdmi_con: endpoint { }; }; + pmic-glink { + compatible = "qcom,sm8350-pmic-glink", "qcom,pmic-glink"; + #address-cells = <1>; + #size-cells = <0>; + + connector@0 { + compatible = "usb-c-connector"; + reg = <0>; + power-role = "dual"; + data-role = "dual"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + pmic_glink_hs_in: endpoint { + remote-endpoint = <&usb_1_dwc3_hs>; + }; + }; + + port@1 { + reg = <1>; + + pmic_glink_ss_in: endpoint { + remote-endpoint = <&usb_1_dwc3_ss>; + }; + }; + }; + }; + }; + vph_pwr: vph-pwr-regulator { compatible = "regulator-fixed"; regulator-name = "vph_pwr"; @@ -666,23 +700,42 @@ &usb_1 { }; &usb_1_dwc3 { - /* TODO: Define USB-C connector properly */ - dr_mode = "peripheral"; -}; + dr_mode = "otg"; + usb-role-switch; -&usb_1_hsphy { - status = "okay"; + ports { + #address-cells = <1>; + #size-cells = <0>; - vdda-pll-supply = <&vreg_l5b_0p88>; - vdda18-supply = <&vreg_l1c_1p8>; - vdda33-supply = <&vreg_l2b_3p07>; + port@0 { + reg = <0>; + + usb_1_dwc3_hs: endpoint { + remote-endpoint = <&pmic_glink_hs_in>; + }; + }; + + port@1 { + reg = <1>; + + usb_1_dwc3_ss: endpoint { + remote-endpoint = <&pmic_glink_ss_in>; + }; + }; + }; }; -&usb_1_qmpphy { - status = "okay"; +&usb_1_dwc3 { + dr_mode = "otg"; + usb-role-switch; +}; - vdda-phy-supply = <&vreg_l6b_1p2>; - vdda-pll-supply = <&vreg_l1b_0p88>; +&usb_1_dwc3_hs { + remote-endpoint = <&pmic_glink_hs_in>; +}; + +&usb_1_dwc3_ss { + remote-endpoint = <&pmic_glink_ss_in>; }; &usb_2 {
Add the pmic glink node linked with the DWC3 USB controller switched to OTG mode and tagged with usb-role-switch. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 77 ++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 12 deletions(-)