| Message ID | 20200612231918.8001-5-wcheng@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce PMIC based USB type C detection | expand |
On 6/12/20 4:19 PM, Wesley Cheng wrote: > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > index 074a2ef55943..f9165f9f9051 100644 > --- a/drivers/regulator/Kconfig > +++ b/drivers/regulator/Kconfig > @@ -797,6 +797,16 @@ config REGULATOR_QCOM_SPMI > Qualcomm SPMI PMICs as a module. The module will be named > "qcom_spmi-regulator". > > +config REGULATOR_QCOM_USB_VBUS > + tristate "Qualcomm USB Vbus regulator driver" > + depends on SPMI || COMPILE_TEST > + help > + If you say yes to this option, support will be included for the > + regulator used to enable the VBUS output. > + > + Say M here if you want to include support for enabling the VBUS output > + as a module. The module will be named "qcom_usb-regulator". Hi, Shouldn't that module name match what is in the Makefile? > + > config REGULATOR_RC5T583 > tristate "RICOH RC5T583 Power regulators" > depends on MFD_RC5T583 > diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile > index c0d6b96ebd78..cbab28aa7b56 100644 > --- a/drivers/regulator/Makefile > +++ b/drivers/regulator/Makefile > @@ -89,6 +89,7 @@ obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o > obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o > obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o > obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o > +obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o > obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o > obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o > obj-$(CONFIG_REGULATOR_PV88060) += pv88060-regulator.o thanks.
On Fri, Jun 12, 2020 at 04:19:16PM -0700, Wesley Cheng wrote: > +++ b/drivers/regulator/qcom_usb_vbus-regulator.c > @@ -0,0 +1,147 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > + */ Please make the entire comment a C++ one so things look more intentional. > +static int qcom_usb_vbus_enable(struct regulator_dev *rdev) > +{ > +static int qcom_usb_vbus_disable(struct regulator_dev *rdev) > +{ > +static int qcom_usb_vbus_is_enabled(struct regulator_dev *rdev) > +{ These operations can all be replaced by regulator_is_enabled_regmap() and friends. > + init_data.constraints.valid_ops_mask |= REGULATOR_CHANGE_STATUS; No, this is broken - regulators should not override the constraints the machine sets.
On 6/12/2020 8:28 PM, Randy Dunlap wrote: > On 6/12/20 4:19 PM, Wesley Cheng wrote: >> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig >> index 074a2ef55943..f9165f9f9051 100644 >> --- a/drivers/regulator/Kconfig >> +++ b/drivers/regulator/Kconfig >> @@ -797,6 +797,16 @@ config REGULATOR_QCOM_SPMI >> Qualcomm SPMI PMICs as a module. The module will be named >> "qcom_spmi-regulator". >> >> +config REGULATOR_QCOM_USB_VBUS >> + tristate "Qualcomm USB Vbus regulator driver" >> + depends on SPMI || COMPILE_TEST >> + help >> + If you say yes to this option, support will be included for the >> + regulator used to enable the VBUS output. >> + >> + Say M here if you want to include support for enabling the VBUS output >> + as a module. The module will be named "qcom_usb-regulator". > > Hi, > Shouldn't that module name match what is in the Makefile? > > Thanks, Randy. Missed this as I was going back and forth on the file name. Thanks for the catch. >> + >> config REGULATOR_RC5T583 >> tristate "RICOH RC5T583 Power regulators" >> depends on MFD_RC5T583 >> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile >> index c0d6b96ebd78..cbab28aa7b56 100644 >> --- a/drivers/regulator/Makefile >> +++ b/drivers/regulator/Makefile >> @@ -89,6 +89,7 @@ obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o >> obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o >> obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o >> obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o >> +obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o >> obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o >> obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o >> obj-$(CONFIG_REGULATOR_PV88060) += pv88060-regulator.o > > > thanks. >
On 6/15/2020 5:00 AM, Mark Brown wrote: > On Fri, Jun 12, 2020 at 04:19:16PM -0700, Wesley Cheng wrote: > >> +++ b/drivers/regulator/qcom_usb_vbus-regulator.c >> @@ -0,0 +1,147 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. >> + */ > > Please make the entire comment a C++ one so things look more > intentional. > Hi Mark, Sure, will do. >> +static int qcom_usb_vbus_enable(struct regulator_dev *rdev) >> +{ > >> +static int qcom_usb_vbus_disable(struct regulator_dev *rdev) >> +{ > >> +static int qcom_usb_vbus_is_enabled(struct regulator_dev *rdev) >> +{ > > These operations can all be replaced by regulator_is_enabled_regmap() > and friends. > Got it. This simplifies the driver a lot. Thanks for the tip. >> + init_data.constraints.valid_ops_mask |= REGULATOR_CHANGE_STATUS; > > No, this is broken - regulators should not override the constraints the > machine sets. > Understood. I decided to go with of_get_regulator_init_data() to initialize the init_data parameter. This should take care of the constraint settings.
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 074a2ef55943..f9165f9f9051 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -797,6 +797,16 @@ config REGULATOR_QCOM_SPMI Qualcomm SPMI PMICs as a module. The module will be named "qcom_spmi-regulator". +config REGULATOR_QCOM_USB_VBUS + tristate "Qualcomm USB Vbus regulator driver" + depends on SPMI || COMPILE_TEST + help + If you say yes to this option, support will be included for the + regulator used to enable the VBUS output. + + Say M here if you want to include support for enabling the VBUS output + as a module. The module will be named "qcom_usb-regulator". + config REGULATOR_RC5T583 tristate "RICOH RC5T583 Power regulators" depends on MFD_RC5T583 diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index c0d6b96ebd78..cbab28aa7b56 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -89,6 +89,7 @@ obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o +obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o obj-$(CONFIG_REGULATOR_PV88060) += pv88060-regulator.o diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c new file mode 100644 index 000000000000..2b8129a04684 --- /dev/null +++ b/drivers/regulator/qcom_usb_vbus-regulator.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020, The Linux Foundation. All rights reserved. + */ + +#include <linux/module.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/interrupt.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regmap.h> + +#define CMD_OTG 0x40 +#define OTG_EN BIT(0) +#define OTG_CFG 0x53 +#define OTG_EN_SRC_CFG BIT(1) + +struct qcom_vbus { + struct device *dev; + u32 base; + struct regmap *regmap; + struct regulator_dev *usb_vbus_reg; +}; + +static int qcom_usb_vbus_enable(struct regulator_dev *rdev) +{ + struct qcom_vbus *vbus_out = rdev_get_drvdata(rdev); + int rc; + + rc = regmap_update_bits(vbus_out->regmap, vbus_out->base + CMD_OTG, + OTG_EN, OTG_EN); + if (rc) + dev_err(vbus_out->dev, "failed to enable usb_vbus\n"); + + return rc; +} + +static int qcom_usb_vbus_disable(struct regulator_dev *rdev) +{ + struct qcom_vbus *vbus_out = rdev_get_drvdata(rdev); + int rc; + + rc = regmap_update_bits(vbus_out->regmap, vbus_out->base + CMD_OTG, + OTG_EN, 0); + if (rc) + dev_err(vbus_out->dev, "failed to disable usb_vbus\n"); + + return rc; +} + +static int qcom_usb_vbus_is_enabled(struct regulator_dev *rdev) +{ + struct qcom_vbus *vbus_out = rdev_get_drvdata(rdev); + unsigned int value = 0; + int rc; + + rc = regmap_read(vbus_out->regmap, vbus_out->base + CMD_OTG, &value); + if (rc) + dev_err(vbus_out->dev, "failed to read OTG_CTL\n"); + + return !!(value & OTG_EN); +} + +static const struct regulator_ops qcom_usb_vbus_reg_ops = { + .enable = qcom_usb_vbus_enable, + .disable = qcom_usb_vbus_disable, + .is_enabled = qcom_usb_vbus_is_enabled, +}; + +static const struct regulator_desc qcom_usb_vbus_rdesc = { + .name = "usb_vbus", + .ops = &qcom_usb_vbus_reg_ops, + .owner = THIS_MODULE, + .type = REGULATOR_VOLTAGE, +}; + +static const struct of_device_id qcom_usb_vbus_regulator_match[] = { + { .compatible = "qcom,pm8150b-vbus-reg" }, + { } +}; +MODULE_DEVICE_TABLE(of, qcom_usb_vbus_regulator_match); + +static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev) +{ + struct qcom_vbus *vbus_out; + struct device *dev = &pdev->dev; + struct regulator_config config = { }; + struct regulator_init_data init_data = { }; + int ret; + u32 reg; + + ret = of_property_read_u32(dev->of_node, "reg", ®); + if (ret < 0) { + dev_err(dev, "no base address found\n"); + return ret; + } + + vbus_out = devm_kzalloc(dev, sizeof(vbus_out), GFP_KERNEL); + if (!vbus_out) + return -ENOMEM; + vbus_out->base = reg; + vbus_out->dev = dev; + + vbus_out->regmap = dev_get_regmap(dev->parent, NULL); + if (!vbus_out->regmap) { + dev_err(dev, "Failed to get regmap\n"); + return -ENOENT; + } + + init_data.constraints.valid_ops_mask |= REGULATOR_CHANGE_STATUS; + config.dev = dev; + config.driver_data = vbus_out; + config.init_data = &init_data; + + vbus_out->usb_vbus_reg = devm_regulator_register(dev, + &qcom_usb_vbus_rdesc, + &config); + if (IS_ERR(vbus_out->usb_vbus_reg)) { + ret = PTR_ERR(vbus_out->usb_vbus_reg); + dev_err(dev, "not able to register vbus reg %d\n", ret); + return ret; + } + + /* Disable HW logic for VBUS enable */ + regmap_update_bits(vbus_out->regmap, vbus_out->base + OTG_CFG, + OTG_EN_SRC_CFG, 0); + + return 0; +} + +static struct platform_driver qcom_usb_vbus_regulator_driver = { + .driver = { + .name = "qcom-usb-vbus-regulator", + .of_match_table = qcom_usb_vbus_regulator_match, + }, + .probe = qcom_usb_vbus_regulator_probe, +}; +module_platform_driver(qcom_usb_vbus_regulator_driver); + +MODULE_DESCRIPTION("Qualcomm USB vbus regulator driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:qcom-usb-vbus-regulator");
Some Qualcomm PMICs have the capability to source the VBUS output to connected peripherals. This driver will register a regulator to the regulator list to enable or disable this source by an external driver. Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> --- drivers/regulator/Kconfig | 10 ++ drivers/regulator/Makefile | 1 + drivers/regulator/qcom_usb_vbus-regulator.c | 147 ++++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 drivers/regulator/qcom_usb_vbus-regulator.c