| Message ID | 20230413113438.1577658-1-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| 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 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 :)
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 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
On Sun Apr 23, 2023 at 12:16 AM CEST, Bryan O'Donoghue wrote: > 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 ? Sure, at https://github.com/z3ntu/linux I pushed the following branches. All are a bit messy, sorry for that, but the end result should be okay. * fp4-6.2.y-wip-tcpm2: TCPM patches v4, worked for the most part apart from the host mode not working in one direction as mentioned in the emails there * fp4-6.2.y-wip-tcpm3: TCPM patches v5 * fp4-6.2.y-wip-tcpm4: TCPM patches from the branch you posted, probably not much difference to the previous branch Regards Luca > > --- > bod
On Thu, 13 Apr 2023 12:34:24 +0100, 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 > > [...] Applied, thanks! [01/14] dt-bindings: regulator: qcom,usb-vbus-regulator: Mark reg as required (no commit info) [02/14] dt-bindings: regulator: qcom,usb-vbus-regulator: Mark regulator-*-microamp required (no commit info) [03/14] dt-bindings: phy: qcom,sc7180-qmp-usb3-dp-phy: Add orientation-switch as optional (no commit info) [04/14] dt-bindings: phy: qcom,sc7180-qmp-usb3-dp-phy: Add ports as an optional (no commit info) [05/14] dt-bindings: usb: Add Qualcomm PMIC Type-C YAML schema (no commit info) [06/14] dt-bindings: mfd: qcom,spmi-pmic: Add typec to SPMI device types (no commit info) [07/14] arm64: dts: qcom: sm8250: Define ports for qmpphy orientation-switching commit: ea96b90a58cf5d2e91ac177f081118ff26b85c1d [08/14] arm64: dts: qcom: pm8150b: Add a TCPM description commit: 5a0539515cbfad30b3e08a00004ed0c86136add5 [09/14] arm64: dts: qcom: qrb5165-rb5: Switch on Type-C VBUS boost commit: c627d7337aae4d83b4db621fdb9e8f638056dcee [10/14] arm64: dts: qcom: qrb5165-rb5: Switch on basic TCPM commit: 5b1b6da9d39d515395d85dc678ddac7ff1689438 [11/14] arm64: dts: qcom: qrb5165-rb5: Switch on TCPM usb-role-switching for usb_1 commit: 25defdca4d902b338c05bc01a1de1064a6d3b7f3 [12/14] arm64: dts: qcom: qrb5165-rb5: Switch on TCPM orientation-switch for usb_1_qmpphy commit: 45219a6b9497cb7713dd2bc221248ee1a7e9bb3d [13/14] usb: typec: qcom: Add Qualcomm PMIC TCPM support (no commit info) [14/14] phy: qcom-qmp: Register as a typec switch for orientation detection (no commit info) Best regards,
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