| Message ID | 20200609205851.30113-1-wcheng@codeaurora.org |
|---|---|
| Headers | show |
| Series | Introduce PMIC based USB type C detection | expand |
On Tue 09 Jun 18:20 PDT 2020, Jack Pham wrote: > Hi Wesley, > > On Tue, Jun 09, 2020 at 01:58:49PM -0700, Wesley Cheng wrote: > > The QCOM SPMI typec driver handles the role and orientation detection, and > > notifies client drivers using the USB role switch framework. It registers > > as a typec port, so orientation can be communicated using the typec switch > > APIs. The driver also registers the VBUS output regulator, so client > > Doesn't look like it.. As we discussed in earlier revisions we decided > to drop the regulator. > > > drivers can enable the VBUS source when acting as a source/host. > > > > Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > > --- > > drivers/usb/typec/Kconfig | 11 ++ > > drivers/usb/typec/Makefile | 1 + > > drivers/usb/typec/qcom-pmic-typec.c | 278 ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 290 insertions(+) > > create mode 100644 drivers/usb/typec/qcom-pmic-typec.c > > > > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig > > index 559dd06..8de2520 100644 > > --- a/drivers/usb/typec/Kconfig > > +++ b/drivers/usb/typec/Kconfig > > @@ -73,6 +73,17 @@ config TYPEC_TPS6598X > > If you choose to build this driver as a dynamically linked module, the > > module will be called tps6598x.ko. > > > > +config TYPEC_QCOM_PMIC > > + tristate "Qualcomm PMIC USB typec driver" > > + depends on ARCH_QCOM > > + help > > + Driver for supporting role switch over the Qualcomm PMIC. This will > > + handle the type C role and orientation detection reported by the QCOM > > + PMIC if the PMIC has the capability to handle type C detection. > > + > > + It will also enable the VBUS output to connected devices when a > > + DFP connection is made. > > + > > source "drivers/usb/typec/mux/Kconfig" > > > > source "drivers/usb/typec/altmodes/Kconfig" > > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile > > index 7753a5c3..cceffd9 100644 > > --- a/drivers/usb/typec/Makefile > > +++ b/drivers/usb/typec/Makefile > > @@ -6,4 +6,5 @@ obj-$(CONFIG_TYPEC_TCPM) += tcpm/ > > obj-$(CONFIG_TYPEC_UCSI) += ucsi/ > > obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o > > obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o > > +obj-$(CONFIG_TYPEC_QCOM_PMIC) += qcom-pmic-typec.o > > obj-$(CONFIG_TYPEC) += mux/ > > diff --git a/drivers/usb/typec/qcom-pmic-typec.c b/drivers/usb/typec/qcom-pmic-typec.c > > new file mode 100644 > > index 0000000..ce6319c > > --- /dev/null > > +++ b/drivers/usb/typec/qcom-pmic-typec.c > > @@ -0,0 +1,278 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > > + */ > > + > > +#include <linux/err.h> > > +#include <linux/regmap.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/slab.h> > > +#include <linux/interrupt.h> > > +#include <linux/of_irq.h> > > +#include <linux/platform_device.h> > > +#include <linux/of_device.h> > > +#include <linux/usb/role.h> > > +#include <linux/usb/typec_mux.h> > > +#include <linux/workqueue.h> > > +#include <linux/regulator/consumer.h> > > +#include <linux/regulator/driver.h> > > +#include <linux/regulator/machine.h> > > + > > +#define DCDC_BASE 0x1100 > > along with USB_BASE @ 0x1300, is it ok to allow this driver to access > registers outside of its 'reg' base (0x1500 according to the DT > bindings)? > Depending on how entangled a future driver for the charger blocks would be one could either just upstream a dcdc regulator driver to control vbus today, or a "lite version" of a charging driver exposing just the vbus regulator. Either way I would prefer this over poking the register directly from this driver, as it will make it tricky to migrate to a proper charger driver later. Regards, Bjorn
On 6/9/2020 2:20 PM, Randy Dunlap wrote: > On 6/9/20 1:58 PM, Wesley Cheng wrote: >> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig >> index 559dd06..8de2520 100644 >> --- a/drivers/usb/typec/Kconfig >> +++ b/drivers/usb/typec/Kconfig >> @@ -73,6 +73,17 @@ config TYPEC_TPS6598X >> If you choose to build this driver as a dynamically linked module, the >> module will be called tps6598x.ko. >> > > Hi, > Please spell "Type-C" like all of the other drivers do. > >> +config TYPEC_QCOM_PMIC >> + tristate "Qualcomm PMIC USB typec driver" >> + depends on ARCH_QCOM >> + help >> + Driver for supporting role switch over the Qualcomm PMIC. This will >> + handle the type C role and orientation detection reported by the QCOM >> + PMIC if the PMIC has the capability to handle type C detection. >> + >> + It will also enable the VBUS output to connected devices when a >> + DFP connection is made. >> + >> source "drivers/usb/typec/mux/Kconfig" >> >> source "drivers/usb/typec/altmodes/Kconfig" Hi Randy, Will do. Thanks > >
On 6/9/2020 7:27 PM, Jun Li wrote: >> +static int qcom_pmic_typec_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct qcom_pmic_typec *qcom_usb; >> + struct typec_capability *cap; >> + const char *buf; >> + int ret, irq, role; >> + >> + qcom_usb = kzalloc(sizeof(*qcom_usb), GFP_KERNEL); > > devm_kzalloc(). > Hi Jun, Thanks for the input. I have replaced with devm_kzalloc() as you recommended. >> + if (!qcom_usb) >> + return -ENOMEM; >> + >> + qcom_usb->dev = dev; >> + >> + qcom_usb->regmap = dev_get_regmap(dev->parent, NULL); >> + if (!qcom_usb->regmap) { >> + dev_err(dev, "Failed to get regmap\n"); >> + return -EINVAL; >> + } >> + >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) { >> + dev_err(dev, "Failed to get CC irq\n"); >> + return -EINVAL; >> + } >> + >> + ret = devm_request_irq(qcom_usb->dev, irq, qcom_pmic_typec_interrupt, >> + IRQF_SHARED, "qcom-pmic-typec", qcom_usb); >> + if (ret) { >> + dev_err(&pdev->dev, "Could not request IRQ\n"); >> + return ret; >> + } >> + >> + qcom_usb->fwnode = device_get_named_child_node(dev, "connector"); >> + if (!qcom_usb->fwnode) >> + return -EINVAL; >> + >> + cap = kzalloc(sizeof(*cap), GFP_KERNEL); > > devm_kzalloc(). > >> + if (!cap) { >> + ret = -ENOMEM; >> + goto err_put_node; >> + } >> + >> + ret = fwnode_property_read_string(qcom_usb->fwnode, "power-role", &buf); >> + if (!ret) >> + role = typec_find_port_power_role(buf); > > if (role < 0) > Added a check for role <0, if so, set to SNK as well >> + else >> + role = TYPEC_PORT_SNK; >> + cap->type = role; >> + >> + ret = fwnode_property_read_string(qcom_usb->fwnode, "data-role", &buf); >> + if (!ret) >> + role = typec_find_port_data_role(buf); > > if (role < 0) > Added a check for role <0, if so, set to UFP as well >> + else >> + role = TYPEC_PORT_UFP; >> + cap->data = role; >> + >> + cap->prefer_role = -1; > > TYPEC_NO_PREFERRED_ROLE > Done. >> + cap->fwnode = qcom_usb->fwnode; >> + qcom_usb->port = typec_register_port(dev, cap); >> + if (IS_ERR(qcom_usb->port)) { >> + dev_err(dev, "Failed to register type c port %d\n", >> + IS_ERR(qcom_usb->port)); >> + ret = PTR_ERR(qcom_usb->port); > > dev_err(dev, , ret)? > > Li Jun Agreed. Thanks! >> + goto err_put_node; >> + } >> + >> + qcom_usb->cap = cap; >> + >> + qcom_usb->role_sw = fwnode_usb_role_switch_get(qcom_usb->fwnode); >> + if (IS_ERR(qcom_usb->role_sw)) { >> + if (PTR_ERR(qcom_usb->role_sw) != -EPROBE_DEFER) >> + dev_err(dev, "failed to get role switch\n"); >> + ret = PTR_ERR(qcom_usb->role_sw); >> + goto err_typec_port; >> + } >> + >> + INIT_WORK(&qcom_usb->bh_work, qcom_pmic_typec_bh_work); >> + platform_set_drvdata(pdev, qcom_usb); >> + qcom_pmic_typec_typec_hw_init(qcom_usb); >> + >> + queue_work(system_power_efficient_wq, &qcom_usb->bh_work); >> + >> + return 0; >> + >> +err_typec_port: >> + typec_unregister_port(qcom_usb->port); >> +err_put_node: >> + fwnode_handle_put(qcom_usb->fwnode); >> + >> + return ret; >> +} >> + >> +static int qcom_pmic_typec_remove(struct platform_device *pdev) >> +{ >> + struct qcom_pmic_typec *qcom_usb = platform_get_drvdata(pdev); >> + >> + cancel_work_sync(&qcom_usb->bh_work); >> + usb_role_switch_set_role(qcom_usb->role_sw, USB_ROLE_NONE); >> + qcom_pmic_typec_vbus_disable(qcom_usb); >> + >> + typec_unregister_port(qcom_usb->port); >> + usb_role_switch_put(qcom_usb->role_sw); >> + fwnode_handle_put(qcom_usb->fwnode); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id qcom_pmic_typec_table[] = { >> + { .compatible = "qcom,pm8150b-usb-typec" }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, qcom_pmic_typec_table); >> + >> +static struct platform_driver qcom_pmic_typec = { >> + .driver = { >> + .name = "qcom,pmic-typec", >> + .of_match_table = qcom_pmic_typec_table, >> + }, >> + .probe = qcom_pmic_typec_probe, >> + .remove = qcom_pmic_typec_remove, >> +}; >> + >> +module_platform_driver(qcom_pmic_typec); >> + >> +MODULE_DESCRIPTION("QCOM PMIC USB type C driver"); >> +MODULE_LICENSE("GPL v2"); >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, >> a Linux Foundation Collaborative Project >>