| Message ID | 20240113-pmi632-typec-v1-6-de7dfd459353@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| 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 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
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); 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; + } 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; +}
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(-)