[PATCH v6 0/4] Introduce PMIC based USB type C detection

[PATCH v6 0/4] Introduce PMIC based USB type C detection

[Wesley Cheng]

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 seperate 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         | 131 +++++++++
 arch/arm64/boot/dts/qcom/pm8150b.dtsi         |  15 +-
 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           | 275 ++++++++++++++++++
 6 files changed, 437 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
 create mode 100644 drivers/usb/typec/qcom-pmic-typec.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v6 1/4] usb: typec: Add QCOM PMIC typec detection driver

[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 <wcheng@codeaurora.org>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/Kconfig           |  12 ++
 drivers/usb/typec/Makefile          |   1 +
 drivers/usb/typec/qcom-pmic-typec.c | 275 ++++++++++++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 drivers/usb/typec/qcom-pmic-typec.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 559dd06117e7..3e375f82849d 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -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
+	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"
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7753a5c3cd46..cceffd987643 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 000000000000..5ae3af03c638
--- /dev/null
+++ b/drivers/usb/typec/qcom-pmic-typec.c
@@ -0,0 +1,275 @@
+// 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/regulator/consumer.h>
+
+#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;
+};
+
+static void qcom_pmic_typec_enable_vbus_regulator(struct qcom_pmic_typec
+							*qcom_usb, bool enable)
+{
+	int ret = 0;
+
+	if (enable == qcom_usb->vbus_enabled)
+		return;
+
+	if (!qcom_usb->vbus_reg) {
+		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) >> 1) ?
+				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;
+}
+
+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) ?
+					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 = of_property_read_u32(dev->of_node, "reg", &reg);
+	if (ret < 0) {
+		dev_err(dev, "missing base address");
+		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");
+		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_threaded_irq(qcom_usb->dev, irq, NULL,
+					qcom_pmic_typec_interrupt, IRQF_ONESHOT,
+					"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 = 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);
+		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);
+	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;
+
+	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");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/4] usb: typec: Add QCOM PMIC typec detection driver

[Stephen Boyd]

Quoting Wesley Cheng (2020-07-29 00:13:37)
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 559dd06117e7..3e375f82849d 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -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

Can you add || COMPILE_TEST here?

> +       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"
> diff --git a/drivers/usb/typec/qcom-pmic-typec.c b/drivers/usb/typec/qcom-pmic-typec.c
> new file mode 100644
> index 000000000000..5ae3af03c638
> --- /dev/null
> +++ b/drivers/usb/typec/qcom-pmic-typec.c
> @@ -0,0 +1,275 @@
> +// 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>

Is this include used?

> +#include <linux/platform_device.h>
> +#include <linux/of_device.h>

Is this include used?

> +#include <linux/usb/role.h>
> +#include <linux/usb/typec_mux.h>
> +#include <linux/regulator/consumer.h>
> +
> +#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;
> +};
> +
> +static void qcom_pmic_typec_enable_vbus_regulator(struct qcom_pmic_typec
> +                                                       *qcom_usb, bool enable)
> +{
> +       int ret = 0;

Please don't assign and then reassign before testing this variable.

> +
> +       if (enable == qcom_usb->vbus_enabled)
> +               return;
> +
> +       if (!qcom_usb->vbus_reg) {
> +               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) >> 1) ?

Do we really need to shift >> by 1? Seems useless for a test.

> +                               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;
> +}
> +
> +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) ?
> +                                       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 = of_property_read_u32(dev->of_node, "reg", &reg);
> +       if (ret < 0) {
> +               dev_err(dev, "missing base address");

Please add newlines at the end of printk messages.

> +               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");
> +               return -EINVAL;
> +       }
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0) {
> +               dev_err(dev, "Failed to get CC irq\n");

platform_get_irq() already prints an error message. Please remove this.

> +               return -EINVAL;
> +       }
> +
> +       ret = devm_request_threaded_irq(qcom_usb->dev, irq, NULL,
> +                                       qcom_pmic_typec_interrupt, IRQF_ONESHOT,
> +                                       "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 = 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);
> +               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);
> +       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;
> +
> +       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" },
> +       { },

Nitpick: Drop the comma here so nothing can come after without causing a
compile error.

> +};
> +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,
> +};
> +

Another nitpick: Drop the newline and make module_platform_driver()
follow directly after the driver.

> +module_platform_driver(qcom_pmic_typec);
> +
> +MODULE_DESCRIPTION("QCOM PMIC USB type C driver");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH v6 1/4] usb: typec: Add QCOM PMIC typec detection driver

[Wesley Cheng]

On 7/29/2020 1:33 AM, Stephen Boyd wrote:
> Quoting Wesley Cheng (2020-07-29 00:13:37)
>> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
>> index 559dd06117e7..3e375f82849d 100644
>> --- a/drivers/usb/typec/Kconfig
>> +++ b/drivers/usb/typec/Kconfig
>> @@ -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
> 
> Can you add || COMPILE_TEST here?
> 

Sure, will do.

>> +#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>
> 
> Is this include used?
> 
>> +#include <linux/platform_device.h>
>> +#include <linux/of_device.h>
> 
> Is this include used?
> 

Reviewed which includes I used, and removed the ones that were not needed.

>> +static void qcom_pmic_typec_enable_vbus_regulator(struct qcom_pmic_typec
>> +                                                       *qcom_usb, bool enable)
>> +{
>> +       int ret = 0;
> 
> Please don't assign and then reassign before testing this variable.
> 

I will just remove the assignment here.

>> +       if (stat & CC_ATTACHED) {
>> +               orientation = ((stat & CC_ORIENTATION) >> 1) ?
> 
> Do we really need to shift >> by 1? Seems useless for a test.
> 

Agreed, we can remove the shift.

>> +       ret = of_property_read_u32(dev->of_node, "reg", &reg);
>> +       if (ret < 0) {
>> +               dev_err(dev, "missing base address");
> 
> Please add newlines at the end of printk messages.
> 

Done.

>> +       irq = platform_get_irq(pdev, 0);
>> +       if (irq < 0) {
>> +               dev_err(dev, "Failed to get CC irq\n");
> 
> platform_get_irq() already prints an error message. Please remove this.
> 

Got it.

>> +static const struct of_device_id qcom_pmic_typec_table[] = {
>> +       { .compatible = "qcom,pm8150b-usb-typec" },
>> +       { },
> 
> Nitpick: Drop the comma here so nothing can come after without causing a
> compile error.
> 

Sure.

>> +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,
>> +};
>> +
> 
> Another nitpick: Drop the newline and make module_platform_driver()
> follow directly after the driver.
> 

Ok, will do.

Thanks for the review/feedback, Stephen.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v6 2/4] dt-bindings: usb: Add Qualcomm PMIC type C controller dt-binding

[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 <wcheng@codeaurora.org>
---
 .../bindings/usb/qcom,pmic-typec.yaml         | 131 ++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml

diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
new file mode 100644
index 000000000000..877e979f413f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
@@ -0,0 +1,131 @@
+# 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 <wcheng@codeaurora.org>
+
+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: /schemas/connector/usb-connector.yaml#/properties/compatible
+        enum:
+          - usb-c-connector
+
+      power-role:
+        $ref: /schemas/connector/usb-connector.yaml#/properties/power-role
+        enum:
+         - dual
+         - source
+         - sink
+
+      data-role:
+        $ref: /schemas/connector/usb-connector.yaml#/properties/data-role
+        enum:
+          - dual
+          - host
+          - device
+
+      ports:
+        description: Remote endpoint connections
+        type: object
+        $ref: /schemas/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
+
+                properties:
+                  remote-endpoint:
+                    description: Node reference to the type C mux
+
+              endpoint@1:
+                description: Connection to role switch node
+                type: object
+
+                properties:
+                  remote-endpoint:
+                    description: Node reference to the role switch node
+
+    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>;
+                        };
+                    };
+                };
+            };
+        };
+    };
+...
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v6 3/4] arm64: boot: dts: qcom: pm8150b: Add node for USB type C block

[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.

Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/pm8150b.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm8150b.dtsi b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
index e112e8876db6..053c659734a7 100644
--- a/arch/arm64/boot/dts/qcom/pm8150b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
@@ -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>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v6 4/4] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster

[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 <wcheng@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/pm8150b.dtsi   | 6 ++++++
 arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pm8150b.dtsi b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
index 053c659734a7..9e560c1ca30d 100644
--- a/arch/arm64/boot/dts/qcom/pm8150b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
@@ -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";
diff --git a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
index 6c6325c3af59..ba3b5b802954 100644
--- a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
@@ -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>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project