| Message ID | 20200612231918.8001-1-wcheng@codeaurora.org |
|---|---|
| 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.
Changes in v2: - Use devm_kzalloc() in qcom_pmic_typec_probe() - Add checks to make sure return value of typec_find_port_power_role() is valid - Added a VBUS output regulator driver, which will be used by the PMIC USB type c driver to enable/disable the source - Added logic to control vbus source from the PMIC type c driver when UFP/DFP is detected - Added dt-binding for this new regulator driver - Fixed Kconfig typec notation to match others - Leave type C block disabled until enabled by a platform DTS Add the required drivers for implementing type C orientation and role detection using the Qualcomm PMIC. Currently, PMICs such as the PM8150B have an integrated type C block, which can be utilized for this. This series adds the dt-binding, PMIC type C driver, and DTS nodes. The PMIC type C driver will register itself as a type C port w/ a registered type C switch for orientation, and will fetch a USB role switch handle for the role notifications. It will also have the ability to enable the VBUS output to any connected devices based on if the device is behaving as a UFP or DFP. Wesley Cheng (6): usb: typec: Add QCOM PMIC typec detection driver dt-bindings: usb: Add Qualcomm PMIC type C controller dt-binding arm64: boot: dts: qcom: pm8150b: Add node for USB type C block regulator: Add support for QCOM PMIC VBUS booster dt-bindings: regulator: Add dt-binding for QCOM PMIC VBUS output regulator arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster .../regulator/qcom,usb-vbus-regulator.yaml | 41 +++ .../bindings/usb/qcom,pmic-typec.yaml | 117 ++++++++ arch/arm64/boot/dts/qcom/pm8150b.dtsi | 13 + arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 7 + drivers/regulator/Kconfig | 10 + drivers/regulator/Makefile | 1 + drivers/regulator/qcom_usb_vbus-regulator.c | 147 ++++++++++ drivers/usb/typec/Kconfig | 12 + drivers/usb/typec/Makefile | 1 + drivers/usb/typec/qcom-pmic-typec.c | 275 ++++++++++++++++++ 10 files changed, 624 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.yaml create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml create mode 100644 drivers/regulator/qcom_usb_vbus-regulator.c create mode 100644 drivers/usb/typec/qcom-pmic-typec.c