| Message ID | 20230501121111.1058190-13-bryan.odonoghue@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Qualcomm PMIC TPCM support | expand |
On 1.05.2023 14:11, Bryan O'Donoghue wrote: > Switch on usb-role-switching for usb_1 via TCPM. We need to declare > usb-role-switch in &usb_1 and associate with the remote-endpoint in TCPM > which provides the necessary signal. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 19 ++++++++++++++++++- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 4 ++++ > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts > index 1e0b6fd59abc9..b5cc45358a474 100644 > --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts > +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts > @@ -1273,7 +1273,13 @@ &usb_1 { > }; > > &usb_1_dwc3 { > - dr_mode = "peripheral"; > + dr_mode = "otg"; > + usb-role-switch; > + port { > + dwc3_role_switch_in: endpoint { Any reason you're redefining it? You can just do &dwc3_role_switch_in { remote-endpoint = <&pm8150b_role_switch_out>; }; > + remote-endpoint = <&pm8150b_role_switch_out>; > + }; > + }; > }; > > &usb_1_hsphy { > @@ -1359,5 +1365,16 @@ connector { > PDO_FIXED_DUAL_ROLE | > PDO_FIXED_USB_COMM | > PDO_FIXED_DATA_SWAP)>; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + pm8150b_role_switch_out: endpoint { Similarly to the QMPPHY, the port definition can be moved to the common node in the SoC DTSI Konrad > + remote-endpoint = <&dwc3_role_switch_in>; > + }; > + }; > + }; > }; > }; > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi > index af16d3ba76b8e..af988328db6b9 100644 > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi > @@ -3740,6 +3740,10 @@ usb_1_dwc3: usb@a600000 { > snps,dis_enblslpm_quirk; > phys = <&usb_1_hsphy>, <&usb_1_ssphy>; > phy-names = "usb2-phy", "usb3-phy"; > + > + port { > + dwc3_role_switch_in: endpoint {}; > + }; > }; > }; >
On 02/05/2023 12:00, Konrad Dybcio wrote: >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + pm8150b_role_switch_out: endpoint { > Similarly to the QMPPHY, the port definition can be moved to > the common node in the SoC DTSI But then the connector would have to be defined in the SoC dtsi and not all derivatives of SoC can be assumed to have a usb-c-connector. grep "usb-c-connector" arch/arm64/boot/dts/qcom/* --- bod
On 2.05.2023 13:03, Bryan O'Donoghue wrote: > On 02/05/2023 12:00, Konrad Dybcio wrote: >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + port@0 { >>> + reg = <0>; >>> + pm8150b_role_switch_out: endpoint { >> Similarly to the QMPPHY, the port definition can be moved to >> the common node in the SoC DTSI > > But then the connector would have to be defined in the SoC dtsi and not all derivatives of SoC can be assumed to have a usb-c-connector. Not quite! You can define an empty endpoint (like we do with DSI<->panel ones) and fill it in on the device side. Konrad > > grep "usb-c-connector" arch/arm64/boot/dts/qcom/* > > --- > bod
On 02/05/2023 12:13, Konrad Dybcio wrote: > > > On 2.05.2023 13:03, Bryan O'Donoghue wrote: >> On 02/05/2023 12:00, Konrad Dybcio wrote: >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + port@0 { >>>> + reg = <0>; >>>> + pm8150b_role_switch_out: endpoint { >>> Similarly to the QMPPHY, the port definition can be moved to >>> the common node in the SoC DTSI >> >> But then the connector would have to be defined in the SoC dtsi and not all derivatives of SoC can be assumed to have a usb-c-connector. > Not quite! > > You can define an empty endpoint (like we do with DSI<->panel ones) and > fill it in on the device side. Sorry you're saying to define as an example the port here in the dtsi &usb_1_dwc3 { dr_mode = "otg"; usb-role-switch; port { dwc3_role_switch_in: endpoint { remote-endpoint = <&pm8150b_role_switch_out>; }; }; }; and to leave the reciprocal definition in the connector to the dts ? &pm8150b_typec { connector { compatible = "usb-c-connector"; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; pm8150b_role_switch_out: endpoint { remote-endpoint = <&dwc3_role_switch_in>; }; };
On 2.05.2023 13:16, Bryan O'Donoghue wrote: > On 02/05/2023 12:13, Konrad Dybcio wrote: >> >> >> On 2.05.2023 13:03, Bryan O'Donoghue wrote: >>> On 02/05/2023 12:00, Konrad Dybcio wrote: >>>>> + #address-cells = <1>; >>>>> + #size-cells = <0>; >>>>> + >>>>> + port@0 { >>>>> + reg = <0>; >>>>> + pm8150b_role_switch_out: endpoint { >>>> Similarly to the QMPPHY, the port definition can be moved to >>>> the common node in the SoC DTSI >>> >>> But then the connector would have to be defined in the SoC dtsi and not all derivatives of SoC can be assumed to have a usb-c-connector. >> Not quite! >> >> You can define an empty endpoint (like we do with DSI<->panel ones) and >> fill it in on the device side. > > Sorry you're saying to define as an example the port here in the dtsi > > &usb_1_dwc3 { > dr_mode = "otg"; > usb-role-switch; === > port { > dwc3_role_switch_in: endpoint { > remote-endpoint = <&pm8150b_role_switch_out>; > }; > }; === this part (minus remote-endpoint) would go to SoC dtsi remote-endpoint would be assigned on a per-device basis > }; > > and to leave the reciprocal definition in the connector to the dts ? > > &pm8150b_typec { > ==== > connector { > compatible = "usb-c-connector"; > ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > pm8150b_role_switch_out: endpoint { > remote-endpoint = <&dwc3_role_switch_in>; > }; > }; ==== this part (minus remote-endpoint) would go to pm8150b.dtsi remote-endpoint would be assigned (or left empty) on a per-device basis Konrad >
On 02/05/2023 12:47, Konrad Dybcio wrote: > > this part (minus remote-endpoint) would go to pm8150b.dtsi > > remote-endpoint would be assigned (or left empty) on a per-device basis Hmm. Yes, actually I think I can locate the connector {} defintion in the pm8150b.dtsi.. you would need to have one tcpm {typec, pdphy} block per type-c port so.. yes done --- bod
On 02/05/2023 12:47, Konrad Dybcio wrote: >> connector { >> compatible = "usb-c-connector"; >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> port@0 { >> reg = <0>; >> pm8150b_role_switch_out: endpoint { >> remote-endpoint = <&dwc3_role_switch_in>; >> }; >> }; > ==== Hmm, I'm actually recoiling from doing this now that I look at it. - Every other implementation defines the connector in the platfrom dts not the core dtsi - The connector would have to be named something like pm8150b_type_connector which is not consistent with the rest of the Type-C connector namespace Yes it can be done that the connector with the ports can be defined in the pm8150b dtsi but TBH I think it is more confusing that way and not consistent with other implementations. I'm citing prior art here I just think this is nicer/easier to follow - arch/arm64/boot/dts/qcom/sc8280xp.dtsi - arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts So for now https://imgflip.com/i/7kw6ck --- bod
diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts index 1e0b6fd59abc9..b5cc45358a474 100644 --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts @@ -1273,7 +1273,13 @@ &usb_1 { }; &usb_1_dwc3 { - dr_mode = "peripheral"; + dr_mode = "otg"; + usb-role-switch; + port { + dwc3_role_switch_in: endpoint { + remote-endpoint = <&pm8150b_role_switch_out>; + }; + }; }; &usb_1_hsphy { @@ -1359,5 +1365,16 @@ connector { PDO_FIXED_DUAL_ROLE | PDO_FIXED_USB_COMM | PDO_FIXED_DATA_SWAP)>; + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + pm8150b_role_switch_out: endpoint { + remote-endpoint = <&dwc3_role_switch_in>; + }; + }; + }; }; }; diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi index af16d3ba76b8e..af988328db6b9 100644 --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi @@ -3740,6 +3740,10 @@ usb_1_dwc3: usb@a600000 { snps,dis_enblslpm_quirk; phys = <&usb_1_hsphy>, <&usb_1_ssphy>; phy-names = "usb2-phy", "usb3-phy"; + + port { + dwc3_role_switch_in: endpoint {}; + }; }; };
Switch on usb-role-switching for usb_1 via TCPM. We need to declare usb-role-switch in &usb_1 and associate with the remote-endpoint in TCPM which provides the necessary signal. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 19 ++++++++++++++++++- arch/arm64/boot/dts/qcom/sm8250.dtsi | 4 ++++ 2 files changed, 22 insertions(+), 1 deletion(-)