| Message ID | 20230413113438.1577658-1-bryan.odonoghue@linaro.org |
|---|---|
| Headers | show |
| Series | Add Qualcomm PMIC TPCM support | expand |
Hi Bryan, On Thu Apr 13, 2023 at 1:34 PM CEST, Bryan O'Donoghue wrote: > V5: > - Amagamates into once device, Heikki, Rob > > - Takes feedback on usage form Luka and Jianhua on VSafeV state transition detection > dev_err() -> dev_warn() > > - Orientation graph example and general expected bindings > I discussed offline with Bjorn the conclusions of the glink/sbu model. > The expected orientation-switch path is > connector/port@0 <-> phy/port@X <-> dp/port@0 > This can then be expanded to > connector/port@0 <-> redriver/port@0 <-> phy/port@X <-> dp/port@0 > > - Rob, Bjorn, Krzysztof > > - Data role > The data-role path is > connector/port@0 <-> dwc3/port@Y I believe I have adjusted my dts correctly for v5 compared to v4 but now the usb doesn't seem to work anymore in most cases. Only when having the phone already plugged in during boot in one orientation does USB come up, but also disappears once you replug the cable. I still see the same (or at least visually similar) messages when plugging in the USB cable or the USB stick but nothing more than that happens. Not that v4 worked perfectly on pm7250b+sm7225(/sm6350) but at least it worked in most cases as described in the emails there. Since the driver structure changed quite a bit, git diff isn't helpful here unfortunately. Don't think it matters but worth mentioning that sm6350 uses the new qmpphy bindings as described in qcom,sc8280xp-qmp-usb43dp-phy.yaml (this was also the case when testing v4 of this). Any idea? Regards Luca > > Previous set: > https://lore.kernel.org/linux-arm-msm/20230318121828.739424-1-bryan.odonoghue@linaro.org/ > > Bootable: > https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-04-12-pm8150b-tcpm-qcom-wrapper-typec-mux > > V4: > - Per Rob's input the pdphy and type-c appear as stadalone blocks > inside of the PMIC declaration which is a 1:1 mapping of PMIC hardware. > The TCPM virtual device is declared at the top-level. > https://lore.kernel.org/all/YY7p7jviA3ZG05gL@robh.at.kernel.org/ > > - Squashes the removal of the old driver with the addition of the new. - Heikki, Gunter > https://lore.kernel.org/all/YYVHcHC1Gm92VxEM@kuha.fi.intel.com/ > > - Reworked Dmitry's old patch for the QMP to account for file renames and > very minimal code-drift in the interregnum. > > - New yaml checks drive update of PMIC VBUS yaml > > - Some housekeeping on the sc7180 yaml side. sc7180 is not supported yet. > > - Expands and fixes the examples being added in the PMIC tcpm examples. > > Previous set: > https://lore.kernel.org/all/20211105033558.1573552-1-bryan.odonoghue@linaro.org/ > > Bootable: > https://git.codelinaro.org/bryan.odonoghue/kernel/-/commits/linux-next-23-03-18-pm8150b-tcpm-qcom-wrapper-typec-mux > > V3: > Rob Herrings review > > - Drops use of remote-endpoint and ports to bind > tcpm to pdphy and typec replacing with phandle > > - Drops pmic-pdphy-* and pmic-typec-* from interrupt names > as suggested > > - Passes make dt_binding_check DT_CHECKER_FLAGS=-m > > BOD > - Noticed qcom_pmic_tcpm_pdphy_enable() was missing a > regulator_disable in case of an error, added. > > - qcom_pmic_tcpm_pdphy_probe() > devm_regulator_get() should come before regmap_get() > as is the case in qcom_pmic_tcpm_typec_probe() > > - Fixes compatible name in qcom,pmic-typec.yaml should > have read qcom,pm8150b-typec not qcom,pm8150b-usb-typec > > - Makes sure compat for core is "qcom,pm8150b-tcpm" in > docs and driver > > - Drops redundant return in void qcom_pmic_tcpm_pdphy_reset_off() > > Kernel Robot > - Drops unused variable debounced in qcom_pmic_tcpm_typec_get_cc() > > - Drops unsused variable orientation in qcom_pmic_tcpm_typec_set_cc() > > Latest bootable series can be found here: > Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=usb-next-04-11-21-pm8150b-tcpm-v3 > > git diff usb-next-27-10-21-pm8150b-tcpm-v2 -- drivers/usb/typec/tcpm/qcom/ > git diff usb-next-27-10-21-pm8150b-tcpm-v2 -- Documentation/devicetree/bindings/usb/qcom,pmic* > > Previous set: > Link: https://lore.kernel.org/linux-usb/20211028164941.831918-1-bryan.odonoghue@linaro.org/T/#t > > V2 resend: > - Adding omitted devicetree mailing list > > V2: > > Guenter Roeck's review > - Converts suggested qcom_pmic_tcpm_core.c into one-liners > > - Adds comment on how polarity is set in set_polarity() > > - Removes optional set_current_limit() > > - regmap_read/regmap_write > Reviwing other pm8150b/spmi drivers I then added in checks for all > reamap_read()/regmap_write() calls. > > - Fixes (type == TCPC_TX_CABLE_RESET || TCPC_TX_HARD_RESET) > thanks I definitely had the blinkers on there and didn't see that at all > > - qcom_pmic_tcpm_pdphy_pd_transmit_payload() > Treats regmap_read and read value as separate error paths > > - qcom_pmic_tcpm_pdphy_set_pd_rx() > Replaces boolean if/else with !on as suggested > > - Returns -ENODEV not -EINVAL on dev_get_regmap() error > > - qcom_pmic_tcpm_pdphy_pd_receive() > Guenter asks: "No error return ?" > bod: No we are inside an ISR here if we read data we pass that off to TCPM > if somehow we don't read the data - it is "junk" there's no value IMO > in pushing an error upwards back to the handler. > > Heikki Krogerus' review > - Includes Makefile I missed adding to my git index > > - Removes old Kconfig entry for remove driver > > Randy Dunlap's review > - Rewords drivers/usb/typec/tcpm/Kconfig > > - Drops tautology "aggregates togther" > > - Corrects spelling typos > > BOD's own review > - Drops redundant include of regmap.h in qcom_pmic_tcpm_core.c > > - Propogates qcom_pmic_tcpm_pdphy_disable() error upwards > > - Propogates pmic_pdphy_reset() error upwards > > - Drops error prints in qcom_pmic_tcpm_pdphy_pd_transmit_payload() > I had these in-place during development and don't recall them being > triggered even once, they are redundant, remove. > > Differences between the two can be seen by > git diff usb-next-27-10-21-pm8150b-tcpm-v2..usb-next-25-10-21-pm8150b-tcpm -- drivers/usb/typec/tcpm > > Latest bootable series can be found here: > Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=usb-next-27-10-21-pm8150b-tcpm-v2 > > Previous set: > Link: https://lore.kernel.org/all/20211025150906.176686-1-bryan.odonoghue@linaro.org/T/#t > > V1: > This series adds a set of yaml and a driver to bind together the type-c and > pdphy silicon in qcom's pm8150b block as a Linux type-c port manager. > > As part of that we retire the existing qcom-pmic-typec driver and fully > replicate its functionality inside of the new block with the additional > pdphy stuff along with it. > > An additional series will follow this one for the SoC and RB5 dtsi and dts > respectively. > > A bootable series can be found here > > Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=usb-next-25-10-21-pm8150b-tcpm > > Bryan O'Donoghue (13): > dt-bindings: regulator: qcom,usb-vbus-regulator: Mark reg as required > dt-bindings: regulator: qcom,usb-vbus-regulator: Mark > regulator-*-microamp required > dt-bindings: phy: qcom,sc7180-qmp-usb3-dp-phy: Add orientation-switch > as optional > dt-bindings: phy: qcom,sc7180-qmp-usb3-dp-phy: Add ports as an > optional > dt-bindings: usb: Add Qualcomm PMIC Type-C YAML schema > dt-bindings: mfd: qcom,spmi-pmic: Add typec to SPMI device types > arm64: dts: qcom: sm8250: Define ports for qmpphy > orientation-switching > arm64: dts: qcom: pm8150b: Add a TCPM description > arm64: dts: qcom: qrb5165-rb5: Switch on Type-C VBUS boost > arm64: dts: qcom: qrb5165-rb5: Switch on basic TCPM > arm64: dts: qcom: qrb5165-rb5: Switch on TCPM usb-role-switching for > usb_1 > arm64: dts: qcom: qrb5165-rb5: Switch on TCPM orientation-switch for > usb_1_qmpphy > usb: typec: qcom: Add Qualcomm PMIC TCPM support > > Dmitry Baryshkov (1): > phy: qcom-qmp: Register as a typec switch for orientation detection > > .../bindings/mfd/qcom,spmi-pmic.yaml | 4 + > .../phy/qcom,sc7180-qmp-usb3-dp-phy.yaml | 40 ++ > .../regulator/qcom,usb-vbus-regulator.yaml | 10 +- > .../bindings/usb/qcom,pmic-typec.yaml | 169 ++++++ > MAINTAINERS | 10 + > arch/arm64/boot/dts/qcom/pm8150b.dtsi | 40 ++ > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 57 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 13 + > drivers/phy/qualcomm/Kconfig | 8 + > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 80 ++- > drivers/usb/typec/Kconfig | 13 - > drivers/usb/typec/Makefile | 1 - > drivers/usb/typec/qcom-pmic-typec.c | 261 -------- > drivers/usb/typec/tcpm/Kconfig | 11 + > drivers/usb/typec/tcpm/Makefile | 1 + > drivers/usb/typec/tcpm/qcom/Makefile | 6 + > drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 362 +++++++++++ > .../typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 528 +++++++++++++++++ > .../typec/tcpm/qcom/qcom_pmic_typec_pdphy.h | 115 ++++ > .../typec/tcpm/qcom/qcom_pmic_typec_port.c | 560 ++++++++++++++++++ > .../typec/tcpm/qcom/qcom_pmic_typec_port.h | 194 ++++++ > 21 files changed, 2202 insertions(+), 281 deletions(-) > create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > delete mode 100644 drivers/usb/typec/qcom-pmic-typec.c > create mode 100644 drivers/usb/typec/tcpm/qcom/Makefile > create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c > create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c > create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h > create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c > create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.h > > -- > 2.39.2
On 13/04/2023 15:19, Luca Weiss wrote: > Hi Bryan, > > On Thu Apr 13, 2023 at 1:34 PM CEST, Bryan O'Donoghue wrote: >> V5: >> - Amagamates into once device, Heikki, Rob >> >> - Takes feedback on usage form Luka and Jianhua on VSafeV state transition detection >> dev_err() -> dev_warn() >> >> - Orientation graph example and general expected bindings >> I discussed offline with Bjorn the conclusions of the glink/sbu model. >> The expected orientation-switch path is >> connector/port@0 <-> phy/port@X <-> dp/port@0 >> This can then be expanded to >> connector/port@0 <-> redriver/port@0 <-> phy/port@X <-> dp/port@0 >> >> - Rob, Bjorn, Krzysztof >> >> - Data role >> The data-role path is >> connector/port@0 <-> dwc3/port@Y > > I believe I have adjusted my dts correctly for v5 compared to v4 but now > the usb doesn't seem to work anymore in most cases. > > Only when having the phone already plugged in during boot in one > orientation does USB come up, but also disappears once you replug the > cable. I still see the same (or at least visually similar) messages when > plugging in the USB cable or the USB stick but nothing more than that > happens. > > Not that v4 worked perfectly on pm7250b+sm7225(/sm6350) but at least it > worked in most cases as described in the emails there. Since the driver > structure changed quite a bit, git diff isn't helpful here > unfortunately. > > Don't think it matters but worth mentioning that sm6350 uses the new > qmpphy bindings as described in qcom,sc8280xp-qmp-usb43dp-phy.yaml (this > was also the case when testing v4 of this). > > Any idea? Can you confirm the output of /sys/class/typec/port0/orientation in host mode with the USB key / peripheral in both orientations ? If that's still OK, then perhaps we can figure out the gap in the PHY code for v3 @caleb is working on this code for sdm845 which is a v3 PHY --- bod
On Thu Apr 13, 2023 at 5:08 PM CEST, Bryan O'Donoghue wrote: > On 13/04/2023 15:19, Luca Weiss wrote: > > Hi Bryan, > > > > On Thu Apr 13, 2023 at 1:34 PM CEST, Bryan O'Donoghue wrote: > >> V5: > >> - Amagamates into once device, Heikki, Rob > >> > >> - Takes feedback on usage form Luka and Jianhua on VSafeV state transition detection > >> dev_err() -> dev_warn() > >> > >> - Orientation graph example and general expected bindings > >> I discussed offline with Bjorn the conclusions of the glink/sbu model. > >> The expected orientation-switch path is > >> connector/port@0 <-> phy/port@X <-> dp/port@0 > >> This can then be expanded to > >> connector/port@0 <-> redriver/port@0 <-> phy/port@X <-> dp/port@0 > >> > >> - Rob, Bjorn, Krzysztof > >> > >> - Data role > >> The data-role path is > >> connector/port@0 <-> dwc3/port@Y > > > > I believe I have adjusted my dts correctly for v5 compared to v4 but now > > the usb doesn't seem to work anymore in most cases. > > > > Only when having the phone already plugged in during boot in one > > orientation does USB come up, but also disappears once you replug the > > cable. I still see the same (or at least visually similar) messages when > > plugging in the USB cable or the USB stick but nothing more than that > > happens. > > > > Not that v4 worked perfectly on pm7250b+sm7225(/sm6350) but at least it > > worked in most cases as described in the emails there. Since the driver > > structure changed quite a bit, git diff isn't helpful here > > unfortunately. > > > > Don't think it matters but worth mentioning that sm6350 uses the new > > qmpphy bindings as described in qcom,sc8280xp-qmp-usb43dp-phy.yaml (this > > was also the case when testing v4 of this). > > > > Any idea? > > Can you confirm the output of /sys/class/typec/port0/orientation in host > mode with the USB key / peripheral in both orientations ? I see "reverse" and "normal" depending on the direction the USB stick is plugged in. When unplugged but also when plugged into my PC it stays at "unknown". > > If that's still OK, then perhaps we can figure out the gap in the PHY > code for v3 > > @caleb is working on this code for sdm845 which is a v3 PHY Regards Luca > > --- > bod
On 2023-04-13 12:34:29, Bryan O'Donoghue wrote: > Add a YAML binding for the Type-C silicon interface inside Qualcomm's > pm8150b hardware block. > > The Type-C driver operates with a pdphy driver inside of a high level > single TCPM device. > > Based on original work by Wesley. > > Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../bindings/usb/qcom,pmic-typec.yaml | 169 ++++++++++++++++++ > 1 file changed, 169 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > > diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > new file mode 100644 > index 0000000000000..6d0f5d00305cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > @@ -0,0 +1,169 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/usb/qcom,pmic-typec.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Qualcomm PMIC based USB Type-C block > + > +maintainers: > + - Bryan O'Donoghue <bryan.odonoghue@linaro.org> > + > +description: | > + Qualcomm PMIC Type-C block > + > +properties: > + compatible: > + enum: > + - qcom,pm8150b-typec > + > + connector: > + type: object > + $ref: /schemas/connector/usb-connector.yaml# > + unevaluatedProperties: false > + > + reg: > + description: Type-C port and pdphy SPMI register base offsets > + minItems: 2 > + maxItems: 2 > + > + interrupts: > + items: > + - description: Bitmask of CC attach, VBUS error, tCCDebounce done and more > + - description: VCONN Powered Detection > + - description: CC state change > + - description: VCONN over-current condition > + - description: VBUS state change > + - description: Attach Deteach notification Deteach -> Detach > + - description: Legacy cable detect > + - description: Try.Src Try.Snk state change > + - description: Sig TX - transmitted reset signal > + - description: Sig RX - received reset signal > + - description: TX completion > + - description: RX completion > + - description: TX fail > + - description: TX discgard > + - description: RX discgard discgard (2x) -> discard > + - description: Fast Role Swap event None of these descriptions follow a similar pattern and are very hard to read and understand. Can you rewrite them? For starters, think about using the same wording and capitalization. > + > + interrupt-names: > + items: > + - const: or-rid-detect-change > + - const: vpd-detect > + - const: cc-state-change > + - const: vconn-oc > + - const: vbus-change > + - const: attach-detach > + - const: legacy-cable-detect > + - const: try-snk-src-detect > + - const: sig-tx > + - const: sig-rx > + - const: msg-tx > + - const: msg-rx > + - const: msg-tx-failed > + - const: msg-tx-discarded > + - const: msg-rx-discarded > + - const: fr-swap > + > + vdd-vbus-supply: > + description: VBUS power supply. > + > + vdd-pdphy-supply: > + description: VDD regulator supply to the PDPHY. > + > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Contains a port which produces data-role switching messages. > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-names > + - vdd-vbus-supply > + - vdd-pdphy-supply > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/usb/pd.h> > + > + pm8150b { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pm8150b_typec: typec@1500 { > + compatible = "qcom,pm8150b-typec"; > + reg = <0x1500>, > + <0x1700>; > + > + interrupts = <0x2 0x15 0x00 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x15 0x01 IRQ_TYPE_EDGE_BOTH>, > + <0x2 0x15 0x02 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x15 0x03 IRQ_TYPE_EDGE_BOTH>, > + <0x2 0x15 0x04 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x15 0x05 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x15 0x06 IRQ_TYPE_EDGE_BOTH>, > + <0x2 0x15 0x07 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x00 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x01 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x02 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x03 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x04 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x05 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x06 IRQ_TYPE_EDGE_RISING>, > + <0x2 0x17 0x07 IRQ_TYPE_EDGE_RISING>; > + > + interrupt-names = "or-rid-detect-change", > + "vpd-detect", > + "cc-state-change", > + "vconn-oc", > + "vbus-change", > + "attach-detach", > + "legacy-cable-detect", > + "try-snk-src-detect", > + "sig-tx", > + "sig-rx", > + "msg-tx", > + "msg-rx", > + "msg-tx-failed", > + "msg-tx-discarded", > + "msg-rx-discarded", > + "fr-swap"; > + > + vdd-vbus-supply = <&pm8150b_vbus>; > + vdd-pdphy-supply = <&vreg_l2a_3p1>; > + connector { > + compatible = "usb-c-connector"; > + > + power-role = "source"; > + data-role = "dual"; > + self-powered; > + > + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_DUAL_ROLE | > + PDO_FIXED_USB_COMM | PDO_FIXED_DATA_SWAP)>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + pm8150b_typec_mux_out: endpoint { > + remote-endpoint = <&qmpphy_typec_mux_in>; > + }; > + }; Also a newline? - Marijn > + port@1 { > + reg = <1>; > + pm8150b_typec_role_switch_out: endpoint { > + remote-endpoint = <&dwc3_role_switch_in>; > + }; > + }; > + }; > + }; > + }; > + }; > +... > -- > 2.39.2 >
On 13/04/2023 13:34, Bryan O'Donoghue wrote: > The regulator code needs to know the location of the register to write to > to switch on/off. Right now we have a driver that does this, a yaml that > partially describes it and no dts that uses it. > > Switching on the VBUS for sm8250 shows that we haven't documented reg as a > required property, do so now. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../devicetree/bindings/regulator/qcom,usb-vbus-regulator.yaml | 1 + > 1 file changed, 1 insertion(+) Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 13/04/2023 13:34, Bryan O'Donoghue wrote: > Add a YAML binding for the Type-C silicon interface inside Qualcomm's > pm8150b hardware block. > > The Type-C driver operates with a pdphy driver inside of a high level > single TCPM device. Subject: drop second/last, redundant "YAML schema". The "dt-bindings" prefix is already stating that these are bindings (and their format). > > Based on original work by Wesley. > > Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../bindings/usb/qcom,pmic-typec.yaml | 169 ++++++++++++++++++ > 1 file changed, 169 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > > diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > new file mode 100644 > index 0000000000000..6d0f5d00305cf > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml > @@ -0,0 +1,169 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/usb/qcom,pmic-typec.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" Drop quotes from both. > + > +title: Qualcomm PMIC based USB Type-C block > + > +maintainers: > + - Bryan O'Donoghue <bryan.odonoghue@linaro.org> > + > +description: | Do not need '|' unless you need to preserve formatting. > + Qualcomm PMIC Type-C block > + > +properties: > + compatible: > + enum: > + - qcom,pm8150b-typec > + > + connector: > + type: object > + $ref: /schemas/connector/usb-connector.yaml# > + unevaluatedProperties: false > + > + reg: > + description: Type-C port and pdphy SPMI register base offsets > + minItems: 2 Drop minItems. > + maxItems: 2 > + > + interrupts: > + items: > + - description: Bitmask of CC attach, VBUS error, tCCDebounce done and more > + - description: VCONN Powered Detection > + - description: CC state change > + - description: VCONN over-current condition > + - description: VBUS state change > + - description: Attach Deteach notification > + - description: Legacy cable detect > + - description: Try.Src Try.Snk state change > + - description: Sig TX - transmitted reset signal > + - description: Sig RX - received reset signal > + - description: TX completion > + - description: RX completion > + - description: TX fail > + - description: TX discgard > + - description: RX discgard > + - description: Fast Role Swap event > + > + interrupt-names: > + items: > + - const: or-rid-detect-change > + - const: vpd-detect > + - const: cc-state-change > + - const: vconn-oc > + - const: vbus-change > + - const: attach-detach > + - const: legacy-cable-detect > + - const: try-snk-src-detect > + - const: sig-tx > + - const: sig-rx > + - const: msg-tx > + - const: msg-rx > + - const: msg-tx-failed > + - const: msg-tx-discarded > + - const: msg-rx-discarded > + - const: fr-swap > + > + vdd-vbus-supply: > + description: VBUS power supply. > + > + vdd-pdphy-supply: > + description: VDD regulator supply to the PDPHY. > + > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Contains a port which produces data-role switching messages. I think Rob asked for example for this... > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-names > + - vdd-vbus-supply > + - vdd-pdphy-supply > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/usb/pd.h> > + > + pm8150b { pmic > + #address-cells = <1>; > + #size-cells = <0>; > + Best regards, Krzysztof
On 14/04/2023 07:51, Luca Weiss wrote: > I see "reverse" and "normal" depending on the direction the USB stick is > plugged in. When unplugged but also when plugged into my PC it stays at > "unknown". Right so, this is down to bad behavior on the PHY patch, which is resolved for me on sm8250 with the below. Basically when you unplug a device you would transition back to "TYPEC_ORIENTATION_NONE" but that would turn off the PHY, which is obs not very useful if you want to subsequently be a gadget. Anyway thanks for testing this - I'd missed the host->device->host->device ping-pong breakage. diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index b9a30c087423d..edb788a71edeb 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -3372,12 +3372,13 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw, qmp->orientation = orientation; - if (orientation == TYPEC_ORIENTATION_NONE) { - if (qmp->init_count) - ret = qmp_combo_dp_power_off(dp_phy); - } else { - if (!qmp->init_count) - ret = qmp_combo_dp_power_on(dp_phy); + if (orientation != TYPEC_ORIENTATION_NONE) { + ret = qmp_combo_dp_power_off(dp_phy); + if (ret) + return ret; + ret = qmp_combo_dp_power_on(dp_phy); + if (ret) + return ret; } --- bod
On Mon Apr 17, 2023 at 2:30 AM CEST, Bryan O'Donoghue wrote: > On 14/04/2023 07:51, Luca Weiss wrote: > > I see "reverse" and "normal" depending on the direction the USB stick is > > plugged in. When unplugged but also when plugged into my PC it stays at > > "unknown". > > Right so, this is down to bad behavior on the PHY patch, which is > resolved for me on sm8250 with the below. > > Basically when you unplug a device you would transition back to > "TYPEC_ORIENTATION_NONE" but that would turn off the PHY, which is obs > not very useful if you want to subsequently be a gadget. > > Anyway thanks for testing this - I'd missed the > host->device->host->device ping-pong breakage. Hm, unfortunately no improvement with this on my side.. No USB connection pops up on the host, or USB messages regarding the USB stick on the device. Do you have an idea in which part of the code to start debugging this? Since orientation detection is working is it maybe in the phy code and not in the tcpm driver? Or does that also touch crucial stuff for USB apart from telling phy which direction to use? Regards Luca > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index b9a30c087423d..edb788a71edeb 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3372,12 +3372,13 @@ static int qmp_combo_typec_switch_set(struct > typec_switch_dev *sw, > > qmp->orientation = orientation; > > - if (orientation == TYPEC_ORIENTATION_NONE) { > - if (qmp->init_count) > - ret = qmp_combo_dp_power_off(dp_phy); > - } else { > - if (!qmp->init_count) > - ret = qmp_combo_dp_power_on(dp_phy); > + if (orientation != TYPEC_ORIENTATION_NONE) { > + ret = qmp_combo_dp_power_off(dp_phy); > + if (ret) > + return ret; > + ret = qmp_combo_dp_power_on(dp_phy); > + if (ret) > + return ret; > } > > --- > bod
On 17/04/2023 08:35, Luca Weiss wrote: > Do you have an idea in which part of the code to start debugging this? > Since orientation detection is working is it maybe in the phy code and > not in the tcpm driver? Or does that also touch crucial stuff for USB > apart from telling phy which direction to use? PHY - I'd almost just do the following diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index edb788a71edeb..bbac82bd093f8 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -3369,7 +3369,7 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw, dev_dbg(qmp->dev, "Toggling orientation current %d requested %d\n", qmp->orientation, orientation); - +return 0; In that case the PHY should "just work" for host or device in one orientation. The other possibility is that the data role message is not hitting dwc3 drd on your platform. If you take the last commit on this branch - plus the updated PHY commit Commit: 171d7f507511 ("usb: dwc3: drd: Enable user-space triggered role-switching") Commit: eb0daa19f3ad ("phy: qcom-qmp: Register as a typec switch for orientation detection") https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-04-17-pm8150b-tcpm-qcom-wrapper-typec-mux cat /sys/class/usb_role/a600000.usb-role-switch/role On SM8250 it looks like this - Attach TypeC accessory with USB key plugged in [1] Mount USB key, read/write some data Unmount USB key cat /sys/class/usb_role/a600000.usb-role-switch/role host - Attach TypeC accessory in opposite orientation Run same test - Connect to PC via TypeC cable Run usb-ecm-up.sh [2] cat /sys/class/usb_role/a600000.usb-role-switch/role device PC : ifconfig enp49s0f3u2u4 192.168.8.1 SM8250 : ifconfig usb0 192.168.8.2 Then PC : iperf -s SM8250 : iperf -c 192.168.8.1 -t 10 [ 1] 0.0000-10.0706 sec 307 MBytes 256 Mbits/sec - Unplug from PC - replug TypeC accessory Rerun test in both orientations - Replug target to PC In this case we only have to reset the IP address PC : ifconfig enp49s0f3u2u4 192.168.8.1 SM8250 : ifconfig usb0 192.168.8.2 Yep its worth checking out that the data-role switch is working, we might be looking at the wrong thing for you on the PHY. [1] https://www.amazon.com/CableCreation-Multiport-Adapter-Gigabit-Ethernet/dp/B08FWMWGTD [2] usb-ecm-up.sh root@linaro-gnome:~# cat usb-ecm-up.sh #!/usr/bin/env bash # load libcomposite module modprobe libcomposite # ensure function is loaded modprobe usb_f_ecm modprobe usb_f_ncm mount -t configfs none /sys/kernel/config/ # create a gadget mkdir /sys/kernel/config/usb_gadget/g0 # cd to its configfs node cd /sys/kernel/config/usb_gadget/g0 # configure it (vid/pid can be anything if USB Class is used for driver compat) echo 0x0525 > idVendor echo 0xa4a4 > idProduct # configure its serial/mfg/product mkdir strings/0x409 echo 0xCAFEBABE > strings/0x409/serialnumber echo Linaro > strings/0x409/manufacturer echo qrb5165-rb5 > strings/0x409/product # create configs mkdir configs/c.1 mkdir configs/c.1/strings/0x409 # create the function (name must match a usb_f_<name> module such as 'acm') mkdir functions/ncm.0 echo "CDC ECM" > configs/c.1/strings/0x409/configuration # associate function with config ln -s functions/ncm.0 configs/c.1 # Set USB version 3.1 echo 0x0310 > bcdUSB echo "super-speed-plus" > max_speed # enable gadget by binding it to a UDC from /sys/class/udc #echo a600000.dwc3 > UDC echo a600000.usb > UDC # to unbind it: echo "" > UDC; sleep 1; rm -rf /sys/kernel/config/usb_gadget/g0 sleep 1 ifconfig usb0 192.168.8.2
On 17/04/2023 11:04, Bryan O'Donoghue wrote: > Unmount USB key > > cat /sys/class/usb_role/a600000.usb-role-switch/role > host Sorry obviously confirm the data role before detaching the accessory :)
On Thu, Apr 13, 2023 at 12:34:38PM +0100, Bryan O'Donoghue wrote: > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > The lane select switch for USB typec orientation is within the USB QMP PHY. > the current device. It could be connected through an endpoint, to an > independent device handling the typec detection, ie the QCOM SPMI typec > driver. > > bod: Fixed the logic qcom_qmp_phy_typec_switch_set() to disable phy > on disconnect if and only if we have initialized the PHY. > Retained CC orientation logic in qcom_qmp_phy_com_init() to simplify > patch. > > bod: Ported from earlier version of driver to phy-qcom-qmp-combo.c > > Co-developed-by: Wesley Cheng <wcheng@codeaurora.org> > Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > Co-developed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Your s-o-b should be last here... > --- > drivers/phy/qualcomm/Kconfig | 8 +++ > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 80 +++++++++++++++++++++-- > 2 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig > index 4850d48f31fa1..8240fffdbed4e 100644 > --- a/drivers/phy/qualcomm/Kconfig > +++ b/drivers/phy/qualcomm/Kconfig > @@ -101,6 +101,14 @@ config PHY_QCOM_QMP_USB > > endif # PHY_QCOM_QMP > > +config PHY_QCOM_QMP_TYPEC > + def_bool PHY_QCOM_QMP=y && TYPEC=y || PHY_QCOM_QMP=m && TYPEC > + help > + Register a type C switch from the QMP PHY driver for type C > + orientation support. This has dependencies with if the type C kernel > + configuration is enabled or not. This support will not be present if > + USB type C is disabled. > + > config PHY_QCOM_QUSB2 > tristate "Qualcomm QUSB2 PHY Driver" > depends on OF && (ARCH_QCOM || COMPILE_TEST) > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index 6850e04c329b8..b9a30c087423d 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -19,6 +19,7 @@ > #include <linux/regulator/consumer.h> > #include <linux/reset.h> > #include <linux/slab.h> > +#include <linux/usb/typec_mux.h> > > #include <dt-bindings/phy/phy-qcom-qmp.h> > > @@ -63,6 +64,10 @@ > /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */ > #define CLAMP_EN BIT(0) /* enables i/o clamp_n */ > > +/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */ > +#define SW_PORTSELECT_VAL BIT(0) > +#define SW_PORTSELECT_MUX BIT(1) > + > #define PHY_INIT_COMPLETE_TIMEOUT 10000 > > struct qmp_phy_init_tbl { > @@ -1323,6 +1328,9 @@ struct qmp_combo { > struct clk_fixed_rate pipe_clk_fixed; > struct clk_hw dp_link_hw; > struct clk_hw dp_pixel_hw; > + > + struct typec_switch_dev *sw; > + enum typec_orientation orientation; > }; > > static void qmp_v3_dp_aux_init(struct qmp_combo *qmp); > @@ -1955,7 +1963,8 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp) > static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp) > { > u32 val; > - bool reverse = false; > + bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE; > + const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts; > > val = DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN | > DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN; > @@ -1974,10 +1983,18 @@ static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp) > * if (orientation == ORIENTATION_CC2) > * writel(0x4c, qmp->dp_dp_phy + QSERDES_V3_DP_PHY_MODE); > */ > + if (dp_opts->lanes == 4 || reverse) > + val |= DP_PHY_PD_CTL_LANE_0_1_PWRDN; > + if (dp_opts->lanes == 4 || !reverse) > + val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN; > + > val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN; When "reverse" this implies 4 lanes, I think it's an accidental left over from introducing the conditionals. > writel(val, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL); > > - writel(0x5c, qmp->dp_dp_phy + QSERDES_DP_PHY_MODE); > + if (reverse) > + writel(0x4c, qmp->pcs + QSERDES_DP_PHY_MODE); > + else > + writel(0x5c, qmp->pcs + QSERDES_DP_PHY_MODE); > > return reverse; > } > @@ -2461,6 +2478,7 @@ static int qmp_combo_com_init(struct qmp_combo *qmp) > { > const struct qmp_phy_cfg *cfg = qmp->cfg; > void __iomem *com = qmp->com; > + u32 val; > int ret; > > mutex_lock(&qmp->phy_mutex); > @@ -2498,8 +2516,11 @@ static int qmp_combo_com_init(struct qmp_combo *qmp) > SW_DPPHY_RESET_MUX | SW_DPPHY_RESET | > SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET); > > - /* Default type-c orientation, i.e CC1 */ > - qphy_setbits(com, QPHY_V3_DP_COM_TYPEC_CTRL, 0x02); > + /* Latch CC orientation based on reported state by TCPM */ > + val = SW_PORTSELECT_MUX; > + if (qmp->orientation == TYPEC_ORIENTATION_REVERSE) > + val |= SW_PORTSELECT_VAL; > + qphy_setbits(com, QPHY_V3_DP_COM_TYPEC_CTRL, val); > > qphy_setbits(com, QPHY_V3_DP_COM_PHY_MODE_CTRL, USB3_MODE | DP_MODE); > > @@ -3338,6 +3359,53 @@ static struct phy *qmp_combo_phy_xlate(struct device *dev, struct of_phandle_arg > return ERR_PTR(-EINVAL); > } > > +#if IS_ENABLED(CONFIG_PHY_QCOM_QMP_TYPEC) > +static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw, > + enum typec_orientation orientation) > +{ > + struct qmp_combo *qmp = typec_switch_get_drvdata(sw); > + struct phy *dp_phy = qmp->dp_phy; > + int ret = 0; > + > + dev_dbg(qmp->dev, "Toggling orientation current %d requested %d\n", > + qmp->orientation, orientation); > + > + qmp->orientation = orientation; > + > + if (orientation == TYPEC_ORIENTATION_NONE) { > + if (qmp->init_count) > + ret = qmp_combo_dp_power_off(dp_phy); > + } else { > + if (!qmp->init_count) > + ret = qmp_combo_dp_power_on(dp_phy); > + } This sequence is crashing my laptop, need some more time to debug the actual cause. Regards, Bjorn > + > + return 0; > +} > + > +static int qmp_combo_typec_switch_register(struct qmp_combo *qmp) > +{ > + struct typec_switch_desc sw_desc; > + struct device *dev = qmp->dev; > + > + sw_desc.drvdata = qmp; > + sw_desc.fwnode = dev->fwnode; > + sw_desc.set = qmp_combo_typec_switch_set; > + qmp->sw = typec_switch_register(dev, &sw_desc); > + if (IS_ERR(qmp->sw)) { > + dev_err(dev, "Error registering typec switch: %ld\n", > + PTR_ERR(qmp->sw)); > + } > + > + return 0; > +} > +#else > +static int qmp_combo_typec_switch_register(struct qmp_combo *qmp) > +{ > + return 0; > +} > +#endif > + > static int qmp_combo_probe(struct platform_device *pdev) > { > struct qmp_combo *qmp; > @@ -3428,6 +3496,10 @@ static int qmp_combo_probe(struct platform_device *pdev) > else > phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > > + ret = qmp_combo_typec_switch_register(qmp); > + if (ret) > + goto err_node_put; > + > of_node_put(usb_np); > of_node_put(dp_np); > > -- > 2.39.2 >
On 18/04/2023 13:57, Bjorn Andersson wrote: >> + dev_dbg(qmp->dev, "Toggling orientation current %d requested %d\n", >> + qmp->orientation, orientation); >> + >> + qmp->orientation = orientation; >> + >> + if (orientation == TYPEC_ORIENTATION_NONE) { >> + if (qmp->init_count) >> + ret = qmp_combo_dp_power_off(dp_phy); >> + } else { >> + if (!qmp->init_count) >> + ret = qmp_combo_dp_power_on(dp_phy); >> + } > This sequence is crashing my laptop, need some more time to debug the > actual cause. > > Regards, > Bjorn > https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-04-17-pm8150b-tcpm-qcom-wrapper-typec-mux This works for me on sm8250 nicely - I can transition from device to host and back again in both orientations - I'm about to send out V6 with this contained, I haven't tried/enabled it on x13s yet though. https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/2c80c630636f1739bde4c1aac2b20940b84daf71 --- bod
Hi Bryan, On Mon Apr 17, 2023 at 12:04 PM CEST, Bryan O'Donoghue wrote: > On 17/04/2023 08:35, Luca Weiss wrote: > > Do you have an idea in which part of the code to start debugging this? > > Since orientation detection is working is it maybe in the phy code and > > not in the tcpm driver? Or does that also touch crucial stuff for USB > > apart from telling phy which direction to use? > > PHY - I'd almost just do the following > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index edb788a71edeb..bbac82bd093f8 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3369,7 +3369,7 @@ static int qmp_combo_typec_switch_set(struct > typec_switch_dev *sw, > > dev_dbg(qmp->dev, "Toggling orientation current %d requested %d\n", > qmp->orientation, orientation); > - > +return 0; > > In that case the PHY should "just work" for host or device in one > orientation. > > The other possibility is that the data role message is not hitting dwc3 > drd on your platform. > > If you take the last commit on this branch - plus the updated PHY commit > > Commit: 171d7f507511 ("usb: dwc3: drd: Enable user-space triggered > role-switching") > > Commit: eb0daa19f3ad ("phy: qcom-qmp: Register as a typec switch for > orientation detection") > > https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-04-17-pm8150b-tcpm-qcom-wrapper-typec-mux > > cat /sys/class/usb_role/a600000.usb-role-switch/role > > On SM8250 it looks like this > > - Attach TypeC accessory with USB key plugged in [1] > Mount USB key, read/write some data > Unmount USB key > > cat /sys/class/usb_role/a600000.usb-role-switch/role > host It feels like I spent way too much time now trying to understand the current behavior across the different patch versions, it's a bit messy, but in short: With the "user-space triggered role-switching" patch I can see that whatever scenario the USB-C port is in, the role is stuck on "device". Nothing = Role: device, Orientation: unknown USB(-A) cable to laptop (either direction) = Role: device, Orientation: unknown USB stick up = Role: device, Orientation: reverse USB stick down = Role: device, Orientation: normal Sometimes/mostly when the USB cable is attached during boot I get USB connection to the laptop until I unplug, then it won't reenable itself. Also the early return in qmp_combo_typec_switch_set doesn't seem to change much I believe? But for sure normally qmp_combo_dp_power_off/on does not get called so I wouldn't be suprised if this reinit breaks something in the phy. > <snip> > > Yep its worth checking out that the data-role switch is working, we > might be looking at the wrong thing for you on the PHY. > So this seems to be the case? If that's useful, I can also go back to the previous (v4?) TCPM revision where the switching mostly worked fine. (btw the subject has a typo, TPCM instead of TCPM :) ) Regards Luca
On 13.04.2023 13:34, 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> > --- This belongs to the SoC DTSI as it describes the capabilities of the USB controllers on the chip. Also please add a newline before each subnode. Konrad > arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 19 ++++++++++++++++++- > 1 file changed, 18 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 { > + 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>; > + }; > + }; > + }; > }; > };
On 21/04/2023 11:26, Luca Weiss wrote: > With the "user-space triggered role-switching" patch I can see that > whatever scenario the USB-C port is in, the role is stuck on "device". Hmm. Could you share a branch ? --- bod
V5: - Amagamates into once device, Heikki, Rob - Takes feedback on usage form Luka and Jianhua on VSafeV state transition detection dev_err() -> dev_warn() - Orientation graph example and general expected bindings I discussed offline with Bjorn the conclusions of the glink/sbu model. The expected orientation-switch path is connector/port@0 <-> phy/port@X <-> dp/port@0 This can then be expanded to connector/port@0 <-> redriver/port@0 <-> phy/port@X <-> dp/port@0 - Rob, Bjorn, Krzysztof - Data role The data-role path is connector/port@0 <-> dwc3/port@Y Previous set: https://lore.kernel.org/linux-arm-msm/20230318121828.739424-1-bryan.odonoghue@linaro.org/ Bootable: https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-04-12-pm8150b-tcpm-qcom-wrapper-typec-mux V4: - Per Rob's input the pdphy and type-c appear as stadalone blocks inside of the PMIC declaration which is a 1:1 mapping of PMIC hardware. The TCPM virtual device is declared at the top-level. https://lore.kernel.org/all/YY7p7jviA3ZG05gL@robh.at.kernel.org/ - Squashes the removal of the old driver with the addition of the new. - Heikki, Gunter https://lore.kernel.org/all/YYVHcHC1Gm92VxEM@kuha.fi.intel.com/ - Reworked Dmitry's old patch for the QMP to account for file renames and very minimal code-drift in the interregnum. - New yaml checks drive update of PMIC VBUS yaml - Some housekeeping on the sc7180 yaml side. sc7180 is not supported yet. - Expands and fixes the examples being added in the PMIC tcpm examples. Previous set: https://lore.kernel.org/all/20211105033558.1573552-1-bryan.odonoghue@linaro.org/ Bootable: https://git.codelinaro.org/bryan.odonoghue/kernel/-/commits/linux-next-23-03-18-pm8150b-tcpm-qcom-wrapper-typec-mux V3: Rob Herrings review - Drops use of remote-endpoint and ports to bind tcpm to pdphy and typec replacing with phandle - Drops pmic-pdphy-* and pmic-typec-* from interrupt names as suggested - Passes make dt_binding_check DT_CHECKER_FLAGS=-m BOD - Noticed qcom_pmic_tcpm_pdphy_enable() was missing a regulator_disable in case of an error, added. - qcom_pmic_tcpm_pdphy_probe() devm_regulator_get() should come before regmap_get() as is the case in qcom_pmic_tcpm_typec_probe() - Fixes compatible name in qcom,pmic-typec.yaml should have read qcom,pm8150b-typec not qcom,pm8150b-usb-typec - Makes sure compat for core is "qcom,pm8150b-tcpm" in docs and driver - Drops redundant return in void qcom_pmic_tcpm_pdphy_reset_off() Kernel Robot - Drops unused variable debounced in qcom_pmic_tcpm_typec_get_cc() - Drops unsused variable orientation in qcom_pmic_tcpm_typec_set_cc() Latest bootable series can be found here: Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=usb-next-04-11-21-pm8150b-tcpm-v3 git diff usb-next-27-10-21-pm8150b-tcpm-v2 -- drivers/usb/typec/tcpm/qcom/ git diff usb-next-27-10-21-pm8150b-tcpm-v2 -- Documentation/devicetree/bindings/usb/qcom,pmic* Previous set: Link: https://lore.kernel.org/linux-usb/20211028164941.831918-1-bryan.odonoghue@linaro.org/T/#t V2 resend: - Adding omitted devicetree mailing list V2: Guenter Roeck's review - Converts suggested qcom_pmic_tcpm_core.c into one-liners - Adds comment on how polarity is set in set_polarity() - Removes optional set_current_limit() - regmap_read/regmap_write Reviwing other pm8150b/spmi drivers I then added in checks for all reamap_read()/regmap_write() calls. - Fixes (type == TCPC_TX_CABLE_RESET || TCPC_TX_HARD_RESET) thanks I definitely had the blinkers on there and didn't see that at all - qcom_pmic_tcpm_pdphy_pd_transmit_payload() Treats regmap_read and read value as separate error paths - qcom_pmic_tcpm_pdphy_set_pd_rx() Replaces boolean if/else with !on as suggested - Returns -ENODEV not -EINVAL on dev_get_regmap() error - qcom_pmic_tcpm_pdphy_pd_receive() Guenter asks: "No error return ?" bod: No we are inside an ISR here if we read data we pass that off to TCPM if somehow we don't read the data - it is "junk" there's no value IMO in pushing an error upwards back to the handler. Heikki Krogerus' review - Includes Makefile I missed adding to my git index - Removes old Kconfig entry for remove driver Randy Dunlap's review - Rewords drivers/usb/typec/tcpm/Kconfig - Drops tautology "aggregates togther" - Corrects spelling typos BOD's own review - Drops redundant include of regmap.h in qcom_pmic_tcpm_core.c - Propogates qcom_pmic_tcpm_pdphy_disable() error upwards - Propogates pmic_pdphy_reset() error upwards - Drops error prints in qcom_pmic_tcpm_pdphy_pd_transmit_payload() I had these in-place during development and don't recall them being triggered even once, they are redundant, remove. Differences between the two can be seen by git diff usb-next-27-10-21-pm8150b-tcpm-v2..usb-next-25-10-21-pm8150b-tcpm -- drivers/usb/typec/tcpm Latest bootable series can be found here: Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=usb-next-27-10-21-pm8150b-tcpm-v2 Previous set: Link: https://lore.kernel.org/all/20211025150906.176686-1-bryan.odonoghue@linaro.org/T/#t V1: This series adds a set of yaml and a driver to bind together the type-c and pdphy silicon in qcom's pm8150b block as a Linux type-c port manager. As part of that we retire the existing qcom-pmic-typec driver and fully replicate its functionality inside of the new block with the additional pdphy stuff along with it. An additional series will follow this one for the SoC and RB5 dtsi and dts respectively. A bootable series can be found here Link: https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=usb-next-25-10-21-pm8150b-tcpm Bryan O'Donoghue (13): dt-bindings: regulator: qcom,usb-vbus-regulator: Mark reg as required dt-bindings: regulator: qcom,usb-vbus-regulator: Mark regulator-*-microamp required dt-bindings: phy: qcom,sc7180-qmp-usb3-dp-phy: Add orientation-switch as optional dt-bindings: phy: qcom,sc7180-qmp-usb3-dp-phy: Add ports as an optional dt-bindings: usb: Add Qualcomm PMIC Type-C YAML schema dt-bindings: mfd: qcom,spmi-pmic: Add typec to SPMI device types arm64: dts: qcom: sm8250: Define ports for qmpphy orientation-switching arm64: dts: qcom: pm8150b: Add a TCPM description arm64: dts: qcom: qrb5165-rb5: Switch on Type-C VBUS boost arm64: dts: qcom: qrb5165-rb5: Switch on basic TCPM arm64: dts: qcom: qrb5165-rb5: Switch on TCPM usb-role-switching for usb_1 arm64: dts: qcom: qrb5165-rb5: Switch on TCPM orientation-switch for usb_1_qmpphy usb: typec: qcom: Add Qualcomm PMIC TCPM support Dmitry Baryshkov (1): phy: qcom-qmp: Register as a typec switch for orientation detection .../bindings/mfd/qcom,spmi-pmic.yaml | 4 + .../phy/qcom,sc7180-qmp-usb3-dp-phy.yaml | 40 ++ .../regulator/qcom,usb-vbus-regulator.yaml | 10 +- .../bindings/usb/qcom,pmic-typec.yaml | 169 ++++++ MAINTAINERS | 10 + arch/arm64/boot/dts/qcom/pm8150b.dtsi | 40 ++ arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 57 +- arch/arm64/boot/dts/qcom/sm8250.dtsi | 13 + drivers/phy/qualcomm/Kconfig | 8 + drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 80 ++- drivers/usb/typec/Kconfig | 13 - drivers/usb/typec/Makefile | 1 - drivers/usb/typec/qcom-pmic-typec.c | 261 -------- drivers/usb/typec/tcpm/Kconfig | 11 + drivers/usb/typec/tcpm/Makefile | 1 + drivers/usb/typec/tcpm/qcom/Makefile | 6 + drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 362 +++++++++++ .../typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 528 +++++++++++++++++ .../typec/tcpm/qcom/qcom_pmic_typec_pdphy.h | 115 ++++ .../typec/tcpm/qcom/qcom_pmic_typec_port.c | 560 ++++++++++++++++++ .../typec/tcpm/qcom/qcom_pmic_typec_port.h | 194 ++++++ 21 files changed, 2202 insertions(+), 281 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml delete mode 100644 drivers/usb/typec/qcom-pmic-typec.c create mode 100644 drivers/usb/typec/tcpm/qcom/Makefile create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c create mode 100644 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.h