Introduce PMIC based USB type C detection
Subsystems (6)
v8 cover Introduce PMIC based USB type C detection
Changes in v8:
- Simplified some property definitions, and corrected the connector reference in the dt binding.
Changes in v7:
- Fixups in qcom-pmic-typec.c to remove uncesscary includes, printk formatting, and revising some logic operations.
Changes in v6:
- Removed qcom_usb_vbus-regulator.c and qcom,usb-vbus-regulator.yaml from the series as they have been merged on regulator.git
- Added separate references to the usb-connector.yaml in qcom,pmic-typec.yaml instead of referencing the entire schema.
Changes in v5:
- Fix dt_binding_check warning/error in qcom,pmic-typec.yaml
Changes in v4:
- Modified qcom,pmic-typec binding to include the SS mux and the DRD remote endpoint nodes underneath port@1, which is assigned to the SSUSB path according to usb-connector
- Added usb-connector reference to the typec dt-binding
- Added tags to the usb type c and vbus nodes
- Removed "qcom" tags from type c and vbus nodes
- Modified Kconfig module name, and removed module alias from the typec driver
Changes in v3:
- Fix driver reference to match driver name in Kconfig for qcom_usb_vbus-regulator.c
- Utilize regulator bitmap helpers for enable, disable and is enabled calls in qcom_usb_vbus-regulator.c
- Use of_get_regulator_init_data() to initialize regulator init data, and to set constraints in qcom_usb_vbus-regulator.c
- Remove the need for a local device structure in the vbus regulator driver
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 (4):
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
arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
.../bindings/usb/qcom,pmic-typec.yaml | 112 ++++++++ arch/arm64/boot/dts/qcom/pm8150b.dtsi | 13 + arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 4 + drivers/usb/typec/Kconfig | 12 + drivers/usb/typec/Makefile | 1 + drivers/usb/typec/qcom-pmic-typec.c | 271 ++++++++++++++++++ 6 files changed, 413 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml create mode 100644 drivers/usb/typec/qcom-pmic-typec.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v8 1/4 usb: typec: Add QCOM PMIC typec detection driver
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 attains a handle to the VBUS output regulator, so it can enable/disable the VBUS source when acting as a host/device.
--- drivers/usb/typec/Kconfig | 12 ++ drivers/usb/typec/Makefile | 1 + drivers/usb/typec/qcom-pmic-typec.c | 271 ++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 drivers/usb/typec/qcom-pmic-typec.c
drivers/usb/typec/Kconfig +12 -0
@@ -73,6 +73,18 @@ 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 Type-C driver" + depends on ARCH_QCOM || COMPILE_TEST + help + Driver for supporting role switch over the Qualcomm PMIC. This will + handle the USB Type-C role and orientation detection reported by the + QCOM PMIC if the PMIC has the capability to handle USB 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"
drivers/usb/typec/Makefile +1 -0
@@ -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/
drivers/usb/typec/qcom-pmic-typec.c new file +271 -0
271 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v8 1/4] usb: typec: Add QCOM PMIC typec detection driver
Quoted 61 lines · Wesley Cheng
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 attains a handle to the VBUS output regulator, so it can enable/disable the VBUS source when acting as a host/device.Signed-off-by Wesley Cheng <[email protected]>Acked-by Heikki Krogerus <[email protected]>Reviewed-by Stephen Boyd <[email protected]>--- drivers/usb/typec/Kconfig | 12 ++ drivers/usb/typec/Makefile | 1 + drivers/usb/typec/qcom-pmic-typec.c | 271 ++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 drivers/usb/typec/qcom-pmic-typec.cdrivers/usb/typec/Kconfig +12
-0@@ -73,6 +73,18 @@ 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 Type-C driver" + depends on ARCH_QCOM || COMPILE_TEST + help + Driver for supporting role switch over the Qualcomm PMIC. This will + handle the USB Type-C role and orientation detection reported by the + QCOM PMIC if the PMIC has the capability to handle USB 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"drivers/usb/typec/Makefile +1
-0@@ -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/drivers/usb/typec/qcom-pmic-typec.c new file +6
-0@@ -0,0 +1,271 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2020, The Linux Foundation. All rights reserved. +*/ + +#include <linux/err.h>
drivers/usb/typec/qcom-pmic-typec.c line 16
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+#include <linux/regmap.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/interrupt.h> +#include <linux/platform_device.h> +#include <linux/mod_devicetable.h> +#include <linux/usb/role.h> +#include <linux/usb/typec_mux.h> +#include <linux/regulator/consumer.h>
Please sort these alphabetically.
Quoted 30 lines
drivers/usb/typec/qcom-pmic-typec.c new file +30
-0+ +#define TYPEC_MISC_STATUS 0xb +#define CC_ATTACHED BIT(0) +#define CC_ORIENTATION BIT(1) +#define SNK_SRC_MODE BIT(6) +#define TYPEC_MODE_CFG 0x44 +#define TYPEC_DISABLE_CMD BIT(0) +#define EN_SNK_ONLY BIT(1) +#define EN_SRC_ONLY BIT(2) +#define TYPEC_VCONN_CONTROL 0x46 +#define VCONN_EN_SRC BIT(0) +#define VCONN_EN_VAL BIT(1) +#define TYPEC_EXIT_STATE_CFG 0x50 +#define SEL_SRC_UPPER_REF BIT(2) +#define TYPEC_INTR_EN_CFG_1 0x5e +#define TYPEC_INTR_EN_CFG_1_MASK GENMASK(7, 0) + +struct qcom_pmic_typec { +struct device *dev; +struct fwnode_handle *fwnode; +struct regmap *regmap; +u32 base; + +struct typec_capability *cap; +struct typec_port *port; +struct usb_role_switch *role_sw; + +struct regulator *vbus_reg; +bool vbus_enabled; +};
drivers/usb/typec/qcom-pmic-typec.c line 56
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+ +static void qcom_pmic_typec_enable_vbus_regulator (struct qcom_pmic_typec + *qcom_usb,bool enable) +{ +int ret; + +if (enable == qcom_usb->vbus_enabled) +return ; + +if (!qcom_usb->vbus_reg) {
I don't think this does what you intend it to.
You call this function during probe, which will attempt to look up the dcdc. In the subsequent patches you don't specify usb_vbus-supply, so this will successfully return a valid (dummy) regulator and we won't enter here again.
If on the other hand you specify usb_vbus-supply in the DT and this driver has not yet probed you will set vbus_reg to NULL and hence retry the next time, but you will continue to report the new role regardless.
So afaict, it's possible to be in host role with vbus disabled (or vice versa depending on boot state) until some event would cause this logic to trigger again. Which I assume would mean that the user would have to disconnect/reconnect the cable.
So, rather than acquiring usb_vbus lazily here I think you want to do this directly in probe and propagate any errors (such as -EPROBE_DEFER) to ensure that the DCDC is registered before evaluating check_connection().
Quoted 58 lines
drivers/usb/typec/qcom-pmic-typec.c new file +58
-0+ qcom_usb->vbus_reg =devm_regulator_get (qcom_usb->dev, +"usb_vbus" ); +if (IS_ERR (qcom_usb->vbus_reg)) { + qcom_usb->vbus_reg =NULL ; +return ; + } + } + +if (enable) { + ret =regulator_enable (qcom_usb->vbus_reg); +if (ret) +return ; + }else { + ret =regulator_disable (qcom_usb->vbus_reg); +if (ret) +return ; + } + qcom_usb->vbus_enabled = enable; +} + +static void qcom_pmic_typec_check_connection (struct qcom_pmic_typec *qcom_usb) +{ +enum typec_orientation orientation; +enum usb_role role; +unsigned int stat; +bool enable_vbus; + +regmap_read (qcom_usb->regmap, qcom_usb->base +TYPEC_MISC_STATUS , + &stat); + +if (stat &CC_ATTACHED ) { + orientation = (stat &CC_ORIENTATION ) ? +TYPEC_ORIENTATION_REVERSE : +TYPEC_ORIENTATION_NORMAL ; +typec_set_orientation (qcom_usb->port, orientation); + + role = (stat &SNK_SRC_MODE ) ?USB_ROLE_HOST :USB_ROLE_DEVICE ; +if (role ==USB_ROLE_HOST ) + enable_vbus = true; +else + enable_vbus = false; + }else { + role =USB_ROLE_NONE ; + enable_vbus = false; + } + +qcom_pmic_typec_enable_vbus_regulator (qcom_usb, enable_vbus); +usb_role_switch_set_role (qcom_usb->role_sw, role); +} + +static irqreturn_t qcom_pmic_typec_interrupt (int irq,void *_qcom_usb) +{ +struct qcom_pmic_typec *qcom_usb = _qcom_usb; + +qcom_pmic_typec_check_connection (qcom_usb); +return IRQ_HANDLED ; +} +
drivers/usb/typec/qcom-pmic-typec.c line 124
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+static void qcom_pmic_typec_typec_hw_init (struct qcom_pmic_typec *qcom_usb) +{ +u8 mode =0 ; + +regmap_update_bits (qcom_usb->regmap, + qcom_usb->base +TYPEC_INTR_EN_CFG_1 , +TYPEC_INTR_EN_CFG_1_MASK ,0 ); + +if (qcom_usb->cap->type !=TYPEC_PORT_DRP ) + mode = (qcom_usb->cap->type == TYPEC_PORT_SNK) ?
Afaict cap->type can have the values DRP, SNK and SRC. So I think it would be cleaner just to do:
if (type == SRC) mode = EN_SRC_ONLY; else if (type == SNK) mode = EN_SNK_ONLY;
I also think you should pass "type" as a parameter to this function, rather than digging it out of qcom_usb->cap (which serves no other purpose).
Quoted 37 lines
drivers/usb/typec/qcom-pmic-typec.c new file +37
-0+ EN_SNK_ONLY :EN_SRC_ONLY ; +regmap_update_bits (qcom_usb->regmap, qcom_usb->base +TYPEC_MODE_CFG , +EN_SNK_ONLY |EN_SRC_ONLY , mode); + +regmap_update_bits (qcom_usb->regmap, + qcom_usb->base +TYPEC_VCONN_CONTROL , +VCONN_EN_SRC |VCONN_EN_VAL ,VCONN_EN_SRC ); +regmap_update_bits (qcom_usb->regmap, + qcom_usb->base +TYPEC_EXIT_STATE_CFG , +SEL_SRC_UPPER_REF ,SEL_SRC_UPPER_REF ); +} + +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; +u32 reg; + + ret =device_property_read_u32 (dev,"reg" , ®); +if (ret <0 ) { +dev_err (dev,"missing base address\n" ); +return ret; + } + + qcom_usb =devm_kzalloc (dev,sizeof (*qcom_usb),GFP_KERNEL ); +if (!qcom_usb) +return -ENOMEM ; + + qcom_usb->dev = dev; + qcom_usb->base = reg; + + qcom_usb->regmap =dev_get_regmap (dev->parent,NULL ); +if (!qcom_usb->regmap) { +dev_err (dev,"Failed to get regmap\n" );
drivers/usb/typec/qcom-pmic-typec.c line 171
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+return -EINVAL ; + } + + irq =platform_get_irq (pdev,0 ); +if (irq <0 ) +return -EINVAL ; + + ret =devm_request_threaded_irq (qcom_usb->dev, irq,NULL , + qcom_pmic_typec_interrupt,IRQF_ONESHOT , +"qcom-pmic-typec" , qcom_usb);
qcom_usb->port and qcom_usb->role_sw are both dereferenced in the irq handler, but not created until later in this function. You need to reorder these to make sure the qcom_usb is fully initialized before you register the isr.
drivers/usb/typec/qcom-pmic-typec.c line 177
Quoted 6 lines
drivers/usb/typec/qcom-pmic-typec.c new file +6
-0+if (ret) { +dev_err (&pdev->dev,"Could not request IRQ\n" ); +return ret; + } + + qcom_usb->fwnode =device_get_named_child_node (dev,"connector" );
I don't see that you need to carry fwnode (and a reference to it) past probe. So just keep it on the stack and release it after acquiring the two string and the role switcher.
Quoted 22 lines
drivers/usb/typec/qcom-pmic-typec.c new file +22
-0+if (!qcom_usb->fwnode) +return -EINVAL ; + + cap =devm_kzalloc (dev,sizeof (*cap),GFP_KERNEL ); +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 ) + role =TYPEC_PORT_SNK ; + }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);
drivers/usb/typec/qcom-pmic-typec.c line 209
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+if (role <0 ) + role =TYPEC_PORT_UFP ; + }else { + role =TYPEC_PORT_UFP ; + } + cap->data = role; + + cap->prefer_role =TYPEC_NO_PREFERRED_ROLE ; + cap->fwnode = qcom_usb->fwnode; + qcom_usb->port =typec_register_port (dev, cap);
cap is cloned in typec_register_port(), which implies that it doesn't have to live beyond this function call. So rather than allocating cap on the heap I think you should just carry it on the stack.
drivers/usb/typec/qcom-pmic-typec.c line 216
Quoted 7 lines
drivers/usb/typec/qcom-pmic-typec.c new file +7
-0+if (IS_ERR (qcom_usb->port)) { + ret =PTR_ERR (qcom_usb->port); +dev_err (dev,"Failed to register type c port %d\n" , ret); + goto err_put_node; + } + + qcom_usb->cap = cap;
As stated above this seems to only serve the purpose of passing cap->type into hw_init() called below, so I don't think you should stash it in qcom_usb.
Regards,
Bjorn
Quoted 59 lines
drivers/usb/typec/qcom-pmic-typec.c new file +55
-0+ + 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; + } + +platform_set_drvdata (pdev, qcom_usb); +qcom_pmic_typec_typec_hw_init (qcom_usb); +qcom_pmic_typec_check_connection (qcom_usb); + +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); + +usb_role_switch_set_role (qcom_usb->role_sw,USB_ROLE_NONE ); +qcom_pmic_typec_enable_vbus_regulator (qcom_usb,0 ); + +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" );Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v8 1/4] usb: typec: Add QCOM PMIC typec detection driver
Quoted 67 lines · Bjorn Andersson
Quoted 65 lines · Wesley Cheng
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 attains a handle to the VBUS output regulator, so it can enable/disable the VBUS source when acting as a host/device.Signed-off-by Wesley Cheng <[email protected]>Acked-by Heikki Krogerus <[email protected]>Reviewed-by Stephen Boyd <[email protected]>--- drivers/usb/typec/Kconfig | 12 ++ drivers/usb/typec/Makefile | 1 + drivers/usb/typec/qcom-pmic-typec.c | 271 ++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 drivers/usb/typec/qcom-pmic-typec.cdrivers/usb/typec/Kconfig +12
-0@@ -73,6 +73,18 @@ 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 Type-C driver" + depends on ARCH_QCOM || COMPILE_TEST + help + Driver for supporting role switch over the Qualcomm PMIC. This will + handle the USB Type-C role and orientation detection reported by the + QCOM PMIC if the PMIC has the capability to handle USB 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"drivers/usb/typec/Makefile +1
-0@@ -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/drivers/usb/typec/qcom-pmic-typec.c new file +10
-0@@ -0,0 +1,271 @@ +// 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>
Quoted 9 lines
drivers/usb/typec/qcom-pmic-typec.c line 16
Quoted 6 lines
drivers/usb/typec/qcom-pmic-typec.c new file +6
-0+#include <linux/interrupt.h> +#include <linux/platform_device.h> +#include <linux/mod_devicetable.h> +#include <linux/usb/role.h> +#include <linux/usb/typec_mux.h> +#include <linux/regulator/consumer.h> Please sort these alphabetically.
Hi Bjorn,
Thanks for all the inputs. Will make all the suggested changes.
Thanks
Wesley
Quoted 317 lines
Quoted 30 lines
drivers/usb/typec/qcom-pmic-typec.c new file +30
-0+ +#define TYPEC_MISC_STATUS 0xb +#define CC_ATTACHED BIT(0) +#define CC_ORIENTATION BIT(1) +#define SNK_SRC_MODE BIT(6) +#define TYPEC_MODE_CFG 0x44 +#define TYPEC_DISABLE_CMD BIT(0) +#define EN_SNK_ONLY BIT(1) +#define EN_SRC_ONLY BIT(2) +#define TYPEC_VCONN_CONTROL 0x46 +#define VCONN_EN_SRC BIT(0) +#define VCONN_EN_VAL BIT(1) +#define TYPEC_EXIT_STATE_CFG 0x50 +#define SEL_SRC_UPPER_REF BIT(2) +#define TYPEC_INTR_EN_CFG_1 0x5e +#define TYPEC_INTR_EN_CFG_1_MASK GENMASK(7, 0) + +struct qcom_pmic_typec { +struct device *dev; +struct fwnode_handle *fwnode; +struct regmap *regmap; +u32 base; + +struct typec_capability *cap; +struct typec_port *port; +struct usb_role_switch *role_sw; + +struct regulator *vbus_reg; +bool vbus_enabled; +};drivers/usb/typec/qcom-pmic-typec.c line 56
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+ +static void qcom_pmic_typec_enable_vbus_regulator (struct qcom_pmic_typec + *qcom_usb,bool enable) +{ +int ret; + +if (enable == qcom_usb->vbus_enabled) +return ; + +if (!qcom_usb->vbus_reg) {I don't think this does what you intend it to.
You call this function during probe, which will attempt to look up the dcdc. In the subsequent patches you don't specify usb_vbus-supply, so this will successfully return a valid (dummy) regulator and we won't enter here again.
If on the other hand you specify usb_vbus-supply in the DT and this driver has not yet probed you will set
vbus_regto NULL and hence retry the next time, but you will continue to report the new role regardless.So afaict, it's possible to be in host role with vbus disabled (or vice versa depending on boot state) until some event would cause this logic to trigger again. Which I assume would mean that the user would have to disconnect/reconnect the cable.
So, rather than acquiring
usb_vbuslazily here I think you want to do this directly in probe and propagate any errors (such as -EPROBE_DEFER) to ensure that the DCDC is registered before evaluatingcheck_connection().Quoted 58 lines
drivers/usb/typec/qcom-pmic-typec.c new file +58
-0+ qcom_usb->vbus_reg =devm_regulator_get (qcom_usb->dev, +"usb_vbus" ); +if (IS_ERR (qcom_usb->vbus_reg)) { + qcom_usb->vbus_reg =NULL ; +return ; + } + } + +if (enable) { + ret =regulator_enable (qcom_usb->vbus_reg); +if (ret) +return ; + }else { + ret =regulator_disable (qcom_usb->vbus_reg); +if (ret) +return ; + } + qcom_usb->vbus_enabled = enable; +} + +static void qcom_pmic_typec_check_connection (struct qcom_pmic_typec *qcom_usb) +{ +enum typec_orientation orientation; +enum usb_role role; +unsigned int stat; +bool enable_vbus; + +regmap_read (qcom_usb->regmap, qcom_usb->base +TYPEC_MISC_STATUS , + &stat); + +if (stat &CC_ATTACHED ) { + orientation = (stat &CC_ORIENTATION ) ? +TYPEC_ORIENTATION_REVERSE : +TYPEC_ORIENTATION_NORMAL ; +typec_set_orientation (qcom_usb->port, orientation); + + role = (stat &SNK_SRC_MODE ) ?USB_ROLE_HOST :USB_ROLE_DEVICE ; +if (role ==USB_ROLE_HOST ) + enable_vbus = true; +else + enable_vbus = false; + }else { + role =USB_ROLE_NONE ; + enable_vbus = false; + } + +qcom_pmic_typec_enable_vbus_regulator (qcom_usb, enable_vbus); +usb_role_switch_set_role (qcom_usb->role_sw, role); +} + +static irqreturn_t qcom_pmic_typec_interrupt (int irq,void *_qcom_usb) +{ +struct qcom_pmic_typec *qcom_usb = _qcom_usb; + +qcom_pmic_typec_check_connection (qcom_usb); +return IRQ_HANDLED ; +} +drivers/usb/typec/qcom-pmic-typec.c line 124
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+static void qcom_pmic_typec_typec_hw_init (struct qcom_pmic_typec *qcom_usb) +{ +u8 mode =0 ; + +regmap_update_bits (qcom_usb->regmap, + qcom_usb->base +TYPEC_INTR_EN_CFG_1 , +TYPEC_INTR_EN_CFG_1_MASK ,0 ); + +if (qcom_usb->cap->type !=TYPEC_PORT_DRP ) + mode = (qcom_usb->cap->type == TYPEC_PORT_SNK) ?Afaict
cap->typecan have the values DRP, SNK and SRC. So I think it would be cleaner just to do:if (type == SRC) mode = EN_SRC_ONLY; else if (type == SNK) mode = EN_SNK_ONLY;I also think you should pass "type" as a parameter to this function, rather than digging it out of
qcom_usb->cap(which serves no other purpose).Quoted 37 lines
drivers/usb/typec/qcom-pmic-typec.c new file +37
-0+ EN_SNK_ONLY :EN_SRC_ONLY ; +regmap_update_bits (qcom_usb->regmap, qcom_usb->base +TYPEC_MODE_CFG , +EN_SNK_ONLY |EN_SRC_ONLY , mode); + +regmap_update_bits (qcom_usb->regmap, + qcom_usb->base +TYPEC_VCONN_CONTROL , +VCONN_EN_SRC |VCONN_EN_VAL ,VCONN_EN_SRC ); +regmap_update_bits (qcom_usb->regmap, + qcom_usb->base +TYPEC_EXIT_STATE_CFG , +SEL_SRC_UPPER_REF ,SEL_SRC_UPPER_REF ); +} + +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; +u32 reg; + + ret =device_property_read_u32 (dev,"reg" , ®); +if (ret <0 ) { +dev_err (dev,"missing base address\n" ); +return ret; + } + + qcom_usb =devm_kzalloc (dev,sizeof (*qcom_usb),GFP_KERNEL ); +if (!qcom_usb) +return -ENOMEM ; + + qcom_usb->dev = dev; + qcom_usb->base = reg; + + qcom_usb->regmap =dev_get_regmap (dev->parent,NULL ); +if (!qcom_usb->regmap) { +dev_err (dev,"Failed to get regmap\n" );drivers/usb/typec/qcom-pmic-typec.c line 171
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+return -EINVAL ; + } + + irq =platform_get_irq (pdev,0 ); +if (irq <0 ) +return -EINVAL ; + + ret =devm_request_threaded_irq (qcom_usb->dev, irq,NULL , + qcom_pmic_typec_interrupt,IRQF_ONESHOT , +"qcom-pmic-typec" , qcom_usb);
qcom_usb->portandqcom_usb->role_sware both dereferenced in the irq handler, but not created until later in this function. You need to reorder these to make sure theqcom_usbis fully initialized before you register the isr.drivers/usb/typec/qcom-pmic-typec.c line 177
Quoted 6 lines
drivers/usb/typec/qcom-pmic-typec.c new file +6
-0+if (ret) { +dev_err (&pdev->dev,"Could not request IRQ\n" ); +return ret; + } + + qcom_usb->fwnode =device_get_named_child_node (dev,"connector" );I don't see that you need to carry fwnode (and a reference to it) past probe. So just keep it on the stack and release it after acquiring the two string and the role switcher.
Quoted 22 lines
drivers/usb/typec/qcom-pmic-typec.c new file +22
-0+if (!qcom_usb->fwnode) +return -EINVAL ; + + cap =devm_kzalloc (dev,sizeof (*cap),GFP_KERNEL ); +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 ) + role =TYPEC_PORT_SNK ; + }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);drivers/usb/typec/qcom-pmic-typec.c line 209
Quoted 10 lines
drivers/usb/typec/qcom-pmic-typec.c new file +10
-0+if (role <0 ) + role =TYPEC_PORT_UFP ; + }else { + role =TYPEC_PORT_UFP ; + } + cap->data = role; + + cap->prefer_role =TYPEC_NO_PREFERRED_ROLE ; + cap->fwnode = qcom_usb->fwnode; + qcom_usb->port =typec_register_port (dev, cap);cap is cloned in
typec_register_port(), which implies that it doesn't have to live beyond this function call. So rather than allocating cap on the heap I think you should just carry it on the stack.drivers/usb/typec/qcom-pmic-typec.c line 216
Quoted 7 lines
drivers/usb/typec/qcom-pmic-typec.c new file +7
-0+if (IS_ERR (qcom_usb->port)) { + ret =PTR_ERR (qcom_usb->port); +dev_err (dev,"Failed to register type c port %d\n" , ret); + goto err_put_node; + } + + qcom_usb->cap = cap;As stated above this seems to only serve the purpose of passing
cap->typeintohw_init()called below, so I don't think you should stash it inqcom_usb.Regards,
BjornQuoted 59 lines
drivers/usb/typec/qcom-pmic-typec.c new file +55
-0+ + 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; + } + +platform_set_drvdata (pdev, qcom_usb); +qcom_pmic_typec_typec_hw_init (qcom_usb); +qcom_pmic_typec_check_connection (qcom_usb); + +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); + +usb_role_switch_set_role (qcom_usb->role_sw,USB_ROLE_NONE ); +qcom_pmic_typec_enable_vbus_regulator (qcom_usb,0 ); + +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" );Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v8 2/4 dt-bindings: usb: Add Qualcomm PMIC type C controller dt-binding
In reply to Wesley ChengIntroduce the dt-binding for enabling USB type C orientation and role detection using the PM8150B. The driver will be responsible for receiving the interrupt at a state change on the CC lines, reading the orientation/role, and communicating this information to the remote clients, which can include a role switch node and a type C switch.
--- .../bindings/usb/qcom,pmic-typec.yaml | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml new file +112 -0
@@ -0,0 +1,112 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/qcom,pmic-typec.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm PMIC based USB type C Detection Driver + +maintainers: + - Wesley Cheng <[email protected]> + +description: | + Qualcomm PMIC Type C Detect + +properties: + compatible: + enum: + - qcom,pm8150b-usb-typec + + reg: + maxItems: 1 + description: Type C base address + + interrupts: + maxItems: 1 + description: CC change interrupt from PMIC + + connector: + description: Connector type for remote endpoints + type: object + + properties: + compatible: + $ref: /connector/usb-connector.yaml#/properties/compatible + enum: + - usb-c-connector + + power-role: true + data-role: true + + ports: + description: Remote endpoint connections + type: object + $ref: /connector/usb-connector.yaml#/properties/ports + + properties: + port@0: + description: Remote endpoints for the High Speed path + type: object + + port@1: + description: Remote endpoints for the Super Speed path + type: object + + properties: + endpoint@0: + description: Connection to USB type C mux node + type: object + + endpoint@1: + description: Connection to role switch node + type: object + + required: + - compatible + +required: + - compatible + - interrupts + - connector + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + pm8150b { + #address-cells = <1>; + #size-cells = <0>; + pm8150b_typec: typec@1500 { + compatible = "qcom,pm8150b-usb-typec"; + reg = <0x1500>; + interrupts = <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>; + + connector { + compatible = "usb-c-connector"; + power-role = "dual"; + data-role = "dual"; + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + }; + port@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + usb3_data_ss: endpoint@0 { + reg = <0>; + remote-endpoint = <&qmp_ss_mux>; + }; + usb3_role: endpoint@1 { + reg = <1>; + remote-endpoint = <&dwc3_drd_switch>; + }; + }; + }; + }; + }; + }; +...
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Rob Herring
maintainerRe: [PATCH v8 2/4] dt-bindings: usb: Add Qualcomm PMIC type C controller dt-binding
Quoted 42 lines · Wesley Cheng
Introduce the dt-binding for enabling USB type C orientation and role detection using the PM8150B. The driver will be responsible for receiving the interrupt at a state change on the CC lines, reading the orientation/role, and communicating this information to the remote clients, which can include a role switch node and a type C switch.
Signed-off-by Wesley Cheng <[email protected]>--- .../bindings/usb/qcom,pmic-typec.yaml | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yamlDocumentation/devicetree/bindings/usb/qcom,pmic-typec.yaml new file +24
-0@@ -0,0 +1,112 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/qcom,pmic-typec.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm PMIC based USB type C Detection Driver + +maintainers: + - Wesley Cheng <[email protected]> + +description: | + Qualcomm PMIC Type C Detect + +properties: + compatible: + enum: + - qcom,pm8150b-usb-typec + + reg: + maxItems: 1 + description: Type C base address + + interrupts:
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml line 34
Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml new file +10
-0+ maxItems: 1 + description: CC change interrupt from PMIC + + connector: + description: Connector type for remote endpoints + type: object + + properties: + compatible: + $ref: /connector/usb-connector.yaml#/properties/compatible
Still wrong. As I said in v7, under 'connector':
$ref: /connector/usb-connector.yaml#
And drop all these references.
Quoted 26 lines
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml new file +26
-0+ enum: + - usb-c-connector + + power-role: true + data-role: true + + ports: + description: Remote endpoint connections + type: object + $ref: /connector/usb-connector.yaml#/properties/ports + + properties: + port@0: + description: Remote endpoints for the High Speed path + type: object + + port@1: + description: Remote endpoints for the Super Speed path + type: object + + properties: + endpoint@0: + description: Connection to USB type C mux node + type: object + + endpoint@1:
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml line 70
Quoted 10 lines
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml new file +10
-0+ description: Connection to role switch node + type: object + + required: + - compatible + +required: + - compatible + - interrupts + - connector
'reg' not required?
Quoted 46 lines
Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml new file +42
-0+ +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + pm8150b { + #address-cells = <1>; + #size-cells = <0>; + pm8150b_typec: typec@1500 { + compatible = "qcom,pm8150b-usb-typec"; + reg = <0x1500>; + interrupts = <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>; + + connector { + compatible = "usb-c-connector"; + power-role = "dual"; + data-role = "dual"; + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + }; + port@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + usb3_data_ss: endpoint@0 { + reg = <0>; + remote-endpoint = <&qmp_ss_mux>; + }; + usb3_role: endpoint@1 { + reg = <1>; + remote-endpoint = <&dwc3_drd_switch>; + }; + }; + }; + }; + }; + }; +...Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v8 3/4 arm64: boot: dts: qcom: pm8150b: Add node for USB type C block
In reply to Wesley ChengThe PM8150B has a dedicated USB type C block, which can be used for type C orientation and role detection. Create the reference node to this type C block for further use.
--- arch/arm64/boot/dts/qcom/pm8150b.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm64/boot/dts/qcom/pm8150b.dtsi +7 -0
@@ -53,6 +53,13 @@ power-on@800 { status = "disabled"; }; + pm8150b_typec: typec@1500 { + compatible = "qcom,pm8150b-usb-typec"; + status = "disabled"; + reg = <0x1500>; + interrupts = <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>; + }; + pm8150b_temp: temp-alarm@2400 { compatible = "qcom,spmi-temp-alarm"; reg = <0x2400>;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v8 3/4] arm64: boot: dts: qcom: pm8150b: Add node for USB type C block
Quoted 4 lines · Wesley Cheng
The PM8150B has a dedicated USB type C block, which can be used for type C orientation and role detection. Create the reference node to this type C block for further use.
Regards,
Bjorn
Quoted 27 lines
Signed-off-by Wesley Cheng <[email protected]>--- arch/arm64/boot/dts/qcom/pm8150b.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)arch/arm64/boot/dts/qcom/pm8150b.dtsi +7
-0@@ -53,6 +53,13 @@ power-on@800 { status = "disabled"; }; + pm8150b_typec: typec@1500 { + compatible = "qcom,pm8150b-usb-typec"; + status = "disabled"; + reg = <0x1500>; + interrupts = <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>; + }; + pm8150b_temp: temp-alarm@2400 { compatible = "qcom,spmi-temp-alarm"; reg = <0x2400>;Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v8 4/4 arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
In reply to Wesley ChengAdd the required DTS node for the USB VBUS output regulator, which is available on PM8150B. This will provide the VBUS source to connected peripherals.
--- arch/arm64/boot/dts/qcom/pm8150b.dtsi | 6 ++++++ arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 4 ++++ 2 files changed, 10 insertions(+)
arch/arm64/boot/dts/qcom/pm8150b.dtsi +6 -0
@@ -53,6 +53,12 @@ power-on@800 { status = "disabled"; }; + pm8150b_vbus: dcdc@1100 { + compatible = "qcom,pm8150b-vbus-reg"; + status = "disabled"; + reg = <0x1100>; + }; + pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-usb-typec"; status = "disabled";
arch/arm64/boot/dts/qcom/sm8150-mtp.dts +4 -0
@@ -409,6 +409,10 @@ &ufs_mem_phy { vdda-pll-max-microamp = <19000>; }; +&pm8150b_vbus { + status = "okay"; +}; + &usb_1_hsphy { status = "okay"; vdda-pll-supply = <&vdd_usb_hs_core>;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v8 4/4] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
Hello!
Quoted 15 lines · Wesley Cheng
Add the required DTS node for the USB VBUS output regulator, which is available on PM8150B. This will provide the VBUS source to connected peripherals.
Signed-off-by Wesley Cheng <[email protected]>--- arch/arm64/boot/dts/qcom/pm8150b.dtsi | 6 ++++++ arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 4 ++++ 2 files changed, 10 insertions(+)arch/arm64/boot/dts/qcom/pm8150b.dtsi +0
-0
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 56
Quoted 4 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +1
-0@@ -53,6 +53,12 @@ power-on@800 { status = "disabled"; }; + pm8150b_vbus: dcdc@1100 {
s/dcdc/regulator/? What is "dcdc", anyway?
The device nodes must have the generic names, according to the DT spec.
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 64
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +5
-0+ compatible = "qcom,pm8150b-vbus-reg"; + status = "disabled"; + reg = <0x1100>; + }; + pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-usb-typec"; status = "disabled";
[...]
MBR, Sergei
Re: [PATCH v8 4/4] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
Quoted 14 lines · Sergei Shtylyov
Hello!
Quoted 10 lines · Wesley Cheng
Add the required DTS node for the USB VBUS output regulator, which is available on PM8150B. This will provide the VBUS source to connected peripherals.
Signed-off-by Wesley Cheng <[email protected]>---
arch/arm64/boot/dts/qcom/pm8150b.dtsi | 6 ++++++arch/arm64/boot/dts/qcom/sm8150-mtp.dts| 4 ++++2 files changed, 10 insertions(+)
Quoted 13 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 55
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +0
-0@@ -53,6 +53,12 @@ power-on@800 { status = "disabled"; }; + pm8150b_vbus: dcdc@1100 {s/dcdc/regulator/? What is "dcdc", anyway?
The device nodes must have the generic names, according to the DT spec.
Hi Sergei,
Thanks for the comment!
DCDC is the label that we use for the DC to DC converter block, since the VBUS booster will output 5V to the connected devices. Would it make more sense to have "dc-dc?"
Thanks
Wesley
Quoted 11 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 64
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +5
-0+ compatible = "qcom,pm8150b-vbus-reg"; + status = "disabled"; + reg = <0x1100>; + }; + pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-usb-typec"; status = "disabled";[...]
MBR, Sergei
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v8 4/4] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
On 20.08.2020 10:47, Wesley Cheng wrote:
[...]
Quoted 24 lines
Quoted 10 lines
Quoted 10 lines
Add the required DTS node for the USB VBUS output regulator, which is available on PM8150B. This will provide the VBUS source to connected peripherals.
Signed-off-by Wesley Cheng <[email protected]>--- arch/arm64/boot/dts/qcom/pm8150b.dtsi | 6 ++++++ arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 4 ++++ 2 files changed, 10 insertions(+)Quoted 13 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 55
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +0
-0@@ -53,6 +53,12 @@ power-on@800 { status = "disabled"; }; + pm8150b_vbus: dcdc@1100 {s/dcdc/regulator/? What is "dcdc", anyway?
The device nodes must have the generic names, according to the DT spec.
Quoted 3 lines
Hi Sergei,
Thanks for the comment!
You're welcome.
Quoted 3 lines
DCDC is the label that we use for the DC to DC converter block, since the VBUS booster will output 5V to the connected devices. Would it make more sense to have "dc-dc?"
Better use s/th like "regulator-dcdc". As I said, the names should be
generic, reflecting the device class.
Quoted 12 lines
Thanks
WesleyQuoted 9 lines
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +5
-0+ compatible = "qcom,pm8150b-vbus-reg"; + status = "disabled"; + reg = <0x1100>; + }; + pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-usb-typec"; status = "disabled";[...]
MBR, Sergei
Re: [PATCH v8 4/4] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
In reply to Wesley ChengQuoted 31 lines · Wesley Cheng
Quoted 14 lines · Sergei Shtylyov
Hello!
Quoted 10 lines · Wesley Cheng
Add the required DTS node for the USB VBUS output regulator, which is available on PM8150B. This will provide the VBUS source to connected peripherals.
Signed-off-by Wesley Cheng <[email protected]>---
arch/arm64/boot/dts/qcom/pm8150b.dtsi | 6 ++++++arch/arm64/boot/dts/qcom/sm8150-mtp.dts| 4 ++++2 files changed, 10 insertions(+)Quoted 13 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 55
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +0
-0@@ -53,6 +53,12 @@ power-on@800 { status = "disabled"; }; + pm8150b_vbus: dcdc@1100 {s/dcdc/regulator/? What is "dcdc", anyway?
The device nodes must have the generic names, according to the DT spec.
Quoted 8 lines
Hi Sergei,
Thanks for the comment!
DCDC is the label that we use for the DC to DC converter block, since the VBUS booster will output 5V to the connected devices. Would it make more sense to have "dc-dc?"
At this level it's just a regulator at 0x1100, so it should be "regulator@1100". If you would like a more useful name in the running system you should be able to use the "regulator-name" property.
Regards,
Bjorn
Quoted 18 lines
Thanks
WesleyQuoted 11 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 64
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +5
-0+ compatible = "qcom,pm8150b-vbus-reg"; + status = "disabled"; + reg = <0x1100>; + }; + pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-usb-typec"; status = "disabled";[...]
MBR, Sergei
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v8 4/4] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster
Quoted 42 lines · Bjorn Andersson
Quoted 31 lines · Wesley Cheng
Quoted 14 lines · Sergei Shtylyov
Hello!
Quoted 10 lines · Wesley Cheng
Add the required DTS node for the USB VBUS output regulator, which is available on PM8150B. This will provide the VBUS source to connected peripherals.
Signed-off-by Wesley Cheng <[email protected]>---
arch/arm64/boot/dts/qcom/pm8150b.dtsi | 6 ++++++arch/arm64/boot/dts/qcom/sm8150-mtp.dts| 4 ++++2 files changed, 10 insertions(+)Quoted 13 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 55
Quoted 9 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +0
-0@@ -53,6 +53,12 @@ power-on@800 { status = "disabled"; }; + pm8150b_vbus: dcdc@1100 {s/dcdc/regulator/? What is "dcdc", anyway?
The device nodes must have the generic names, according to the DT spec.Quoted 8 lines
Hi Sergei,
Thanks for the comment!
DCDC is the label that we use for the DC to DC converter block, since the VBUS booster will output 5V to the connected devices. Would it make more sense to have "dc-dc?"
Quoted 7 lines
At this level it's just a regulator at
0x1100, so it should be "regulator@1100". If you would like a more useful name in the running system you should be able to use the "regulator-name" property.Regards,
Bjorn
Hi Bjorn,
Thanks for the suggestion. Sounds good, I will just use the "regulator" name for now.
Thanks
Wesley
Quoted 18 lines
Quoted 18 lines
Thanks
WesleyQuoted 11 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi power-on@800 { · line 64
Quoted 8 lines
arch/arm64/boot/dts/qcom/pm8150b.dtsi +5
-0+ compatible = "qcom,pm8150b-vbus-reg"; + status = "disabled"; + reg = <0x1100>; + }; + pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-usb-typec"; status = "disabled";[...]
MBR, Sergei
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project