| Message ID | 20240113-pmi632-typec-v1-0-de7dfd459353@linaro.org |
|---|---|
| Headers | show |
| Series | usb: typec: qcom-pmic-typec: enable support for PMI632 PMIC | expand |
On 13.01.2024 06:42, Dmitry Baryshkov wrote: > The PMI632 PMIC support Type-C port handling, but lacks USB > PowerDelivery support. The TCPM requires all callbacks to be provided > by the implementation. Implement a special, 'stub' Qcom PD PHY > implementation to enable the PMI632 support. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- [...] > #endif /* __QCOM_PMIC_TYPEC_PDPHY_H__ */ > diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c > new file mode 100644 > index 000000000000..5d3b0e78d4d8 > --- /dev/null > +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c Not a fan. Maybe add some TCPM_FLAG_NO_PD and solve it in a generic manner? Konrad
On 13.01.2024 06:42, Dmitry Baryshkov wrote: > Now as all dual-lane PHYs have been migrated to a new driver, drop > support for dual lanes configuration. If the PHY uses two lanes for USB, > it is symthom that it should use either a combo USB+DP or a USB-C PHY > driver. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- [...] > - if (cfg->lanes >= 2) { > - qmp->tx2 = devm_of_iomap(dev, np, 3, NULL); > - if (IS_ERR(qmp->tx2)) > - return PTR_ERR(qmp->tx2); > - > - qmp->rx2 = devm_of_iomap(dev, np, 4, NULL); > - if (IS_ERR(qmp->rx2)) > - return PTR_ERR(qmp->rx2); > - > - qmp->pcs_misc = devm_of_iomap(dev, np, 5, NULL); > - } else { > - qmp->pcs_misc = devm_of_iomap(dev, np, 3, NULL); > - } > + qmp->pcs_misc = devm_of_iomap(dev, np, 3, NULL); > > if (IS_ERR(qmp->pcs_misc)) { Since you may need a new revision, you can remove the neline above to get a nice ret = ... if (ret) ... equivalent while at it Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On 13/01/2024 05:42, Dmitry Baryshkov wrote: > Define VBUS regulator and the Type-C handling block as present on the > Quacomm PMI632 PMIC. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm64/boot/dts/qcom/pmi632.dtsi | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/pmi632.dtsi b/arch/arm64/boot/dts/qcom/pmi632.dtsi > index 4eb79e0ce40a..ccf288ddc987 100644 > --- a/arch/arm64/boot/dts/qcom/pmi632.dtsi > +++ b/arch/arm64/boot/dts/qcom/pmi632.dtsi > @@ -45,6 +45,35 @@ pmic@2 { > #address-cells = <1>; > #size-cells = <0>; > > + pmi632_vbus: usb-vbus-regulator@1100 { > + compatible = "qcom,pmi632-vbus-reg", "qcom,pm8150b-vbus-reg"; > + status = "disabled"; > + reg = <0x1100>; > + }; > + > + pmi632_typec: typec@1500 { > + compatible = "qcom,pmi632-typec"; > + status = "disabled"; > + reg = <0x1500>; > + 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>; > + interrupt-names = "or-rid-detect-change", > + "vpd-detect", > + "cc-state-change", > + "vconn-oc", > + "vbus-change", > + "attach-detach", > + "legacy-cable-detect", > + "try-snk-src-detect"; > + vdd-vbus-supply = <&pmi632_vbus>; > + }; > + > pmi632_temp: temp-alarm@2400 { > compatible = "qcom,spmi-temp-alarm"; > reg = <0x2400>; > LTGM Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 13/01/2024 05:42, Dmitry Baryshkov wrote: > The PMI632 PMIC support Type-C port handling, but lacks USB > PowerDelivery support. The TCPM requires all callbacks to be provided > by the implementation. Implement a special, 'stub' Qcom PD PHY > implementation to enable the PMI632 support. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/usb/typec/tcpm/qcom/Makefile | 3 +- > drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 30 +++++++--- > .../usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h | 2 + > .../typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c | 67 ++++++++++++++++++++++ > 4 files changed, 94 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/qcom/Makefile b/drivers/usb/typec/tcpm/qcom/Makefile > index dc1e8832e197..cc23042b9487 100644 > --- a/drivers/usb/typec/tcpm/qcom/Makefile > +++ b/drivers/usb/typec/tcpm/qcom/Makefile > @@ -3,4 +3,5 @@ > obj-$(CONFIG_TYPEC_QCOM_PMIC) += qcom_pmic_tcpm.o > qcom_pmic_tcpm-y += qcom_pmic_typec.o \ > qcom_pmic_typec_port.o \ > - qcom_pmic_typec_pdphy.o > + qcom_pmic_typec_pdphy.o \ > + qcom_pmic_typec_pdphy_stub.o \ > diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c > index 4f2dbf20da12..e2513549c58a 100644 > --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c > +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c > @@ -118,7 +118,7 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev) > const struct pmic_typec_resources *res; > struct regmap *regmap; > struct device *bridge_dev; > - u32 base[2]; > + u32 base; > int ret; > > res = of_device_get_match_data(dev); > @@ -145,7 +145,7 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev) > return -ENODEV; > } > > - ret = of_property_read_u32_array(np, "reg", base, 2); > + ret = of_property_read_u32_index(np, "reg", 0, &base); So I had to do a double-take here but, this seems fine to me. > if (ret) > return ret; > > @@ -154,14 +154,24 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev) > return PTR_ERR(tcpm->pmic_typec_port); > > ret = qcom_pmic_typec_port_probe(pdev, tcpm->pmic_typec_port, > - res->port_res, regmap, base[0]); > + res->port_res, regmap, base); > if (ret) > return ret; > > - ret = res->pdphy_probe(pdev, tcpm, > - res->pdphy_res, regmap, base[1]); > - if (ret) > - return ret; > + if (res->pdphy_res) { > + ret = of_property_read_u32_index(np, "reg", 1, &base); > + if (ret) > + return ret; > + > + ret = qcom_pmic_typec_pdphy_probe(pdev, tcpm, > + res->pdphy_res, regmap, base); > + if (ret) > + return ret; > + } else { > + ret = qcom_pmic_typec_pdphy_stub_probe(pdev, tcpm); > + if (ret) > + return ret; > + } Looks fine. > > mutex_init(&tcpm->lock); > platform_set_drvdata(pdev, tcpm); > @@ -253,8 +263,14 @@ static struct pmic_typec_resources pm8150b_typec_res = { > .port_res = &pm8150b_port_res, > }; > > +static struct pmic_typec_resources pmi632_typec_res = { > + /* PD PHY not present */ > + .port_res = &pm8150b_port_res, > +}; > + > static const struct of_device_id qcom_pmic_typec_table[] = { > { .compatible = "qcom,pm8150b-typec", .data = &pm8150b_typec_res }, > + { .compatible = "qcom,pmi632-typec", .data = &pmi632_typec_res }, > { } > }; > MODULE_DEVICE_TABLE(of, qcom_pmic_typec_table); > diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h > index b94eccadb042..2a7dedeb3009 100644 > --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h > +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h > @@ -31,5 +31,7 @@ int qcom_pmic_typec_pdphy_probe(struct platform_device *pdev, > const struct pmic_typec_pdphy_resources *res, > struct regmap *regmap, > u32 base); > +int qcom_pmic_typec_pdphy_stub_probe(struct platform_device *pdev, > + struct pmic_typec *tcpm); > > #endif /* __QCOM_PMIC_TYPEC_PDPHY_H__ */ > diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c > new file mode 100644 > index 000000000000..5d3b0e78d4d8 > --- /dev/null > +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c > @@ -0,0 +1,67 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2024, Linaro Ltd. All rights reserved. > + */ > + > +#include <linux/err.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/slab.h> > +#include <linux/usb/pd.h> > +#include <linux/usb/tcpm.h> > +#include "qcom_pmic_typec.h" > +#include "qcom_pmic_typec_pdphy.h" > + > +static int qcom_pmic_typec_pdphy_stub_pd_transmit(struct tcpc_dev *tcpc, > + enum tcpm_transmit_type type, > + const struct pd_message *msg, > + unsigned int negotiated_rev) > +{ > + struct pmic_typec *tcpm = tcpc_to_tcpm(tcpc); > + struct device *dev = tcpm->dev; > + > + dev_dbg(dev, "pdphy_transmit: type=%d\n", type); > + > + tcpm_pd_transmit_complete(tcpm->tcpm_port, > + TCPC_TX_SUCCESS); > + > + return 0; > +} > + > +static int qcom_pmic_typec_pdphy_stub_set_pd_rx(struct tcpc_dev *tcpc, bool on) > +{ > + struct pmic_typec *tcpm = tcpc_to_tcpm(tcpc); > + struct device *dev = tcpm->dev; > + > + dev_dbg(dev, "set_pd_rx: %s\n", on ? "on" : "off"); > + > + return 0; > +} > + > +static int qcom_pmic_typec_pdphy_stub_set_roles(struct tcpc_dev *tcpc, bool attached, > + enum typec_role power_role, > + enum typec_data_role data_role) > +{ > + struct pmic_typec *tcpm = tcpc_to_tcpm(tcpc); > + struct device *dev = tcpm->dev; > + > + dev_dbg(dev, "pdphy_set_roles: data_role_host=%d power_role_src=%d\n", > + data_role, power_role); > + > + return 0; > +} > + > +int qcom_pmic_typec_pdphy_stub_probe(struct platform_device *pdev, > + struct pmic_typec *tcpm) > +{ > + tcpm->tcpc.set_pd_rx = qcom_pmic_typec_pdphy_stub_set_pd_rx; > + tcpm->tcpc.set_roles = qcom_pmic_typec_pdphy_stub_set_roles; > + tcpm->tcpc.pd_transmit = qcom_pmic_typec_pdphy_stub_pd_transmit; > + > + return 0; > +} > So this answers the question I had on IRC whether or not the Linux TCPM layer could tolerate a stubbed PD layers. I think this _should_ be fine, I certainly have no problem with the approach overall and the intevention in the code seems small. Good work. Acked-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 13/01/2024 10:33, Konrad Dybcio wrote: > On 13.01.2024 06:42, Dmitry Baryshkov wrote: >> The PMI632 PMIC support Type-C port handling, but lacks USB >> PowerDelivery support. The TCPM requires all callbacks to be provided >> by the implementation. Implement a special, 'stub' Qcom PD PHY >> implementation to enable the PMI632 support. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- > > [...] > >> #endif /* __QCOM_PMIC_TYPEC_PDPHY_H__ */ >> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c >> new file mode 100644 >> index 000000000000..5d3b0e78d4d8 >> --- /dev/null >> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c > > Not a fan. > > Maybe add some TCPM_FLAG_NO_PD and solve it in a generic manner? > > Konrad This is a good suggestion, provided its the direction Heikki and Guenter want to go. Otherwise @Dmitry you can retain my Acked-by for the stubification. --- bod
On Sat, 13 Jan 2024 at 12:52, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 13.01.2024 06:42, Dmitry Baryshkov wrote: > > Plug in USB-C related bits and pieces to enable USB role switching and > > USB-C orientation handling for the Qualcomm RB2 board. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 62 ++++++++++++++++++++++++++++++++ > > arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++ > > 2 files changed, 100 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > index 52f31f3166c2..a96e3afb65bc 100644 > > --- a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > +++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > @@ -6,8 +6,10 @@ > > /dts-v1/; > > > > #include <dt-bindings/leds/common.h> > > +#include <dt-bindings/usb/pd.h> > > #include "sm4250.dtsi" > > #include "pm6125.dtsi" > > +#include "pmi632.dtsi" > > > > / { > > model = "Qualcomm Technologies, Inc. QRB4210 RB2"; > > @@ -256,6 +258,53 @@ kypd_vol_up_n: kypd-vol-up-n-state { > > }; > > }; > > > > +&pmi632_typec { > > + status = "okay"; > > + > > + connector { > > + compatible = "usb-c-connector"; > > + > > + power-role = "dual"; > > + data-role = "dual"; > > + self-powered; > > + > > + source-pdos = <PDO_FIXED(5000, 3000, > > + PDO_FIXED_DUAL_ROLE | > > + PDO_FIXED_USB_COMM | > > + PDO_FIXED_DATA_SWAP)>; > > + sink-pdos = <PDO_FIXED(5000, 500, > > + PDO_FIXED_DUAL_ROLE | > > + PDO_FIXED_USB_COMM | > > + PDO_FIXED_DATA_SWAP)>; > > + op-sink-microwatt = <10000000>; > So RB2 can provide 15 watts over the USB-C port, consume 2.5 but > requires 10? That doesn't make a whole lot of sense.. > > Unless I'm reading this wrong.. Let me double-check the properties and the docs before answering. > > > +&usb_dwc3 { > > + usb-role-switch; > > Since this is a dual-role controller, I think this could live in the SoC Sure. Should we update other boards too? > DT > > > +}; > > + > > +&usb_dwc3_hs { > > + remote-endpoint = <&pmi632_hs_in>; > > +}; > > + > > &usb_hsphy { > > vdd-supply = <&vreg_l4a_0p9>; > > vdda-pll-supply = <&vreg_l12a_1p8>; > > @@ -618,10 +675,15 @@ &usb_hsphy { > > &usb_qmpphy { > > vdda-phy-supply = <&vreg_l4a_0p9>; > > vdda-pll-supply = <&vreg_l12a_1p8>; > > + orientation-switch; > > Similarly, if this doesn't kaboom w/ our implementation too much, the > PHY itself has orientation detection capabilities > > Konrad
On 13.01.2024 15:15, Dmitry Baryshkov wrote: > On Sat, 13 Jan 2024 at 12:42, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> On 13.01.2024 06:42, Dmitry Baryshkov wrote: >>> In preparation to adding Type-C handling for MSM8998, QCM2290 and SM6115 >>> platforms, create new QMP USB-C PHY driver by splitting mentioned >>> platforms to a separate file. In future it will also be extended with >>> support for the DisplayPort handling. It will also be reused later for >>> such platforms as SDM660, SM6125, SM6150. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >> >> [...] >> >> >>> +#include "phy-qcom-qmp.h" >>> +#include "phy-qcom-qmp-pcs-misc-v3.h" >>> + >>> +/* QPHY_SW_RESET bit */ >>> +#define SW_RESET BIT(0) >>> +/* QPHY_POWER_DOWN_CONTROL */ >>> +#define SW_PWRDN BIT(0) >> >> Most / all of these defines could probably live in a header file. > > For this (and several other comments), see > https://lore.kernel.org/linux-arm-msm/20240109-phy-qmp-merge-common-v1-0-572899a14318@linaro.org/ So, I'd assume the plan is to land these two series in parallel and then submit a cleanup to this one? Sounds ok then! Konrad
The Qualcomm PMI632 PMIC (found on Qualcomm Robotics RB2 platform) doesn't support USB Power Delivery. However this PMIC still supports handling of the Type-C port (orientation detection, etc). Reuse exiting qcom-pmic-typec driver to support Type-C related functionality of this PMIC. Use this to enable USB-C connector support on the RB2 platform. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Dmitry Baryshkov (12): dt-bindings: regulator: qcom,usb-vbus-regulator: add support for PMI632 dt-bindings: usb: qcom,pmic-typec: add support for the PMI632 block dt-bindings: phy: qcom,msm8998-qmp-usb3-phy: split from sc8280xp PHY schema dt-bindings: phy: qcom,msm8998-qmp-usb3-phy: support USB-C data usb: typec: qcom-pmic-typec: allow different implementations for the PD PHY usb: typec: qcom-pmic-typec: add support for PMI632 PMIC phy: qcom: qmp-usb: split USB-C PHY driver phy: qcom: qmp-usb: drop dual-lane handling phy: qcom: qmp-usbc: drop single lane handling phy: qcom: qmp-usbc: add support for the Type-C handling arm64: dts: qcom: pmi632: define USB-C related blocks arm64: dts: qcom: qrb4210-rb2: enable USB-C port handling Vladimir Zapolskiy (1): arm64: dts: qcom: sm6115: drop pipe clock selection .../bindings/phy/qcom,msm8998-qmp-usb3-phy.yaml | 171 +++ .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 22 - .../regulator/qcom,usb-vbus-regulator.yaml | 9 +- .../devicetree/bindings/usb/qcom,pmic-typec.yaml | 28 +- arch/arm64/boot/dts/qcom/pmi632.dtsi | 29 + arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 60 +- arch/arm64/boot/dts/qcom/sm6115.dtsi | 39 +- drivers/phy/qualcomm/Makefile | 2 +- drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 323 +----- drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1202 ++++++++++++++++++++ drivers/usb/typec/tcpm/qcom/Makefile | 3 +- drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 126 +- drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.h | 25 + .../usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 155 ++- .../usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.h | 92 +- .../typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c | 67 ++ drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.h | 4 +- 17 files changed, 1805 insertions(+), 552 deletions(-) --- base-commit: 9e21984d62c56a0f6d1fc6f76b646212cfd7fe88 change-id: 20240112-pmi632-typec-4c7533092387 Best regards,