2022-02-03 01:07:46

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 0/4] usb: typec: WUSB3801 devicetree bindings and driver

This series adds bindings and a driver for the Willsemi WUSB3801. This
chip's bindings use the standard USB Type-C connector bindings, but the
driver does not use the TCPM library, so a refactoring patch is included
to avoid duplicating some fwnode parsing code.

Changes in v2:
- Always put the return values from typec_find_* in a signed variable
for error checking.
- License the driver as GPL 2 only; probably best anyway as I used a
lot of other drivers/usb/typec code as inspiration
- Don't try to be clever; use `default` instead of `unreachable`
- Free the IRQ before unregistering the partner/port

Samuel Holland (4):
dt-bindings: vendor-prefixes: Add willsemi
dt-bindings: usb: Add WUSB3801 Type-C Port Controller
usb: typec: Factor out non-PD fwnode properties
usb: typec: Support the WUSB3801 port controller

.../bindings/usb/willsemi,wusb3801.yaml | 75 +++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/usb/typec/Kconfig | 10 +
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/class.c | 52 ++
drivers/usb/typec/tcpm/tcpm.c | 32 +-
drivers/usb/typec/wusb3801.c | 445 ++++++++++++++++++
include/linux/usb/typec.h | 3 +
8 files changed, 589 insertions(+), 31 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/willsemi,wusb3801.yaml
create mode 100644 drivers/usb/typec/wusb3801.c

--
2.33.1


2022-02-03 10:17:34

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 4/4] usb: typec: Support the WUSB3801 port controller

WUSB3801 features a configurable port type, accessory detection, and
plug orientation detection. It provides a hardware "ID" pin output for
compatibility with USB 2.0 OTG PHYs. Add a typec class driver for it.

Signed-off-by: Samuel Holland <[email protected]>
---

Changes in v2:
- License the driver as GPL 2 only; probably best anyway as I used a
lot of other drivers/usb/typec code as inspiration
- Don't try to be clever; use `default` instead of `unreachable`
- Free the IRQ before unregistering the partner/port

drivers/usb/typec/Kconfig | 10 +
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/wusb3801.c | 445 +++++++++++++++++++++++++++++++++++
3 files changed, 456 insertions(+)
create mode 100644 drivers/usb/typec/wusb3801.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index ab480f38523a..74acde9fc09f 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -88,6 +88,16 @@ config TYPEC_QCOM_PMIC
It will also enable the VBUS output to connected devices when a
DFP connection is made.

+config TYPEC_WUSB3801
+ tristate "Willsemi WUSB3801 Type-C port controller driver"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y or M here if your system has a WUSB3801 Type-C port controller.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called wusb3801.ko.
+
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 57870a2bd787..3b722ec28d99 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_TYPEC_TPS6598X) += tipd/
obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
obj-$(CONFIG_TYPEC_QCOM_PMIC) += qcom-pmic-typec.o
obj-$(CONFIG_TYPEC_STUSB160X) += stusb160x.o
+obj-$(CONFIG_TYPEC_WUSB3801) += wusb3801.o
obj-$(CONFIG_TYPEC) += mux/
diff --git a/drivers/usb/typec/wusb3801.c b/drivers/usb/typec/wusb3801.c
new file mode 100644
index 000000000000..77c2805ba803
--- /dev/null
+++ b/drivers/usb/typec/wusb3801.c
@@ -0,0 +1,445 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Willsemi WUSB3801 Type-C port controller driver
+ *
+ * Copyright (C) 2022 Samuel Holland <[email protected]>
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/typec.h>
+
+#define WUSB3801_REG_DEVICE_ID 0x01
+#define WUSB3801_REG_CTRL0 0x02
+#define WUSB3801_REG_INT 0x03
+#define WUSB3801_REG_STAT 0x04
+#define WUSB3801_REG_CTRL1 0x05
+#define WUSB3801_REG_TEST00 0x06
+#define WUSB3801_REG_TEST01 0x07
+#define WUSB3801_REG_TEST02 0x08
+#define WUSB3801_REG_TEST03 0x09
+#define WUSB3801_REG_TEST04 0x0a
+#define WUSB3801_REG_TEST05 0x0b
+#define WUSB3801_REG_TEST06 0x0c
+#define WUSB3801_REG_TEST07 0x0d
+#define WUSB3801_REG_TEST08 0x0e
+#define WUSB3801_REG_TEST09 0x0f
+#define WUSB3801_REG_TEST0A 0x10
+#define WUSB3801_REG_TEST0B 0x11
+#define WUSB3801_REG_TEST0C 0x12
+#define WUSB3801_REG_TEST0D 0x13
+#define WUSB3801_REG_TEST0E 0x14
+#define WUSB3801_REG_TEST0F 0x15
+#define WUSB3801_REG_TEST10 0x16
+#define WUSB3801_REG_TEST11 0x17
+#define WUSB3801_REG_TEST12 0x18
+
+#define WUSB3801_DEVICE_ID_VERSION_ID GENMASK(7, 3)
+#define WUSB3801_DEVICE_ID_VENDOR_ID GENMASK(2, 0)
+
+#define WUSB3801_CTRL0_DIS_ACC_SUPPORT BIT(7)
+#define WUSB3801_CTRL0_TRY GENMASK(6, 5)
+#define WUSB3801_CTRL0_TRY_NONE (0x0 << 5)
+#define WUSB3801_CTRL0_TRY_SNK (0x1 << 5)
+#define WUSB3801_CTRL0_TRY_SRC (0x2 << 5)
+#define WUSB3801_CTRL0_CURRENT GENMASK(4, 3) /* SRC */
+#define WUSB3801_CTRL0_CURRENT_DEFAULT (0x0 << 3)
+#define WUSB3801_CTRL0_CURRENT_1_5A (0x1 << 3)
+#define WUSB3801_CTRL0_CURRENT_3_0A (0x2 << 3)
+#define WUSB3801_CTRL0_ROLE GENMASK(2, 1)
+#define WUSB3801_CTRL0_ROLE_SNK (0x0 << 1)
+#define WUSB3801_CTRL0_ROLE_SRC (0x1 << 1)
+#define WUSB3801_CTRL0_ROLE_DRP (0x2 << 1)
+#define WUSB3801_CTRL0_INT_MASK BIT(0)
+
+#define WUSB3801_INT_ATTACHED BIT(0)
+#define WUSB3801_INT_DETACHED BIT(1)
+
+#define WUSB3801_STAT_VBUS_DETECTED BIT(7)
+#define WUSB3801_STAT_CURRENT GENMASK(6, 5) /* SNK */
+#define WUSB3801_STAT_CURRENT_STANDBY (0x0 << 5)
+#define WUSB3801_STAT_CURRENT_DEFAULT (0x1 << 5)
+#define WUSB3801_STAT_CURRENT_1_5A (0x2 << 5)
+#define WUSB3801_STAT_CURRENT_3_0A (0x3 << 5)
+#define WUSB3801_STAT_PARTNER GENMASK(4, 2)
+#define WUSB3801_STAT_PARTNER_STANDBY (0x0 << 2)
+#define WUSB3801_STAT_PARTNER_SNK (0x1 << 2)
+#define WUSB3801_STAT_PARTNER_SRC (0x2 << 2)
+#define WUSB3801_STAT_PARTNER_AUDIO (0x3 << 2)
+#define WUSB3801_STAT_PARTNER_DEBUG (0x4 << 2)
+#define WUSB3801_STAT_ORIENTATION GENMASK(1, 0)
+#define WUSB3801_STAT_ORIENTATION_NONE (0x0 << 0)
+#define WUSB3801_STAT_ORIENTATION_CC1 (0x1 << 0)
+#define WUSB3801_STAT_ORIENTATION_CC2 (0x2 << 0)
+#define WUSB3801_STAT_ORIENTATION_BOTH (0x3 << 0)
+
+#define WUSB3801_CTRL1_SM_RESET BIT(0)
+
+#define WUSB3801_TEST01_VENDOR_SUB_ID (BIT(8) | BIT(6))
+
+#define WUSB3801_TEST02_FORCE_ERR_RCY BIT(8)
+
+#define WUSB3801_TEST0A_WAIT_VBUS BIT(5)
+
+struct wusb3801 {
+ struct typec_capability cap;
+ struct device *dev;
+ struct typec_partner *partner;
+ struct typec_port *port;
+ struct regmap *regmap;
+ struct regulator *vbus_supply;
+ unsigned int partner_type;
+ enum typec_port_type port_type;
+ enum typec_pwr_opmode pwr_opmode;
+ bool vbus_on;
+};
+
+static enum typec_role wusb3801_get_default_role(struct wusb3801 *wusb3801)
+{
+ switch (wusb3801->port_type) {
+ case TYPEC_PORT_SRC:
+ return TYPEC_SOURCE;
+ case TYPEC_PORT_SNK:
+ return TYPEC_SINK;
+ case TYPEC_PORT_DRP:
+ default:
+ if (wusb3801->cap.prefer_role == TYPEC_SOURCE)
+ return TYPEC_SOURCE;
+ return TYPEC_SINK;
+ }
+}
+
+static int wusb3801_map_port_type(enum typec_port_type type)
+{
+ switch (type) {
+ case TYPEC_PORT_SRC:
+ return WUSB3801_CTRL0_ROLE_SRC;
+ case TYPEC_PORT_SNK:
+ return WUSB3801_CTRL0_ROLE_SNK;
+ case TYPEC_PORT_DRP:
+ default:
+ return WUSB3801_CTRL0_ROLE_DRP;
+ }
+}
+
+static int wusb3801_map_pwr_opmode(enum typec_pwr_opmode mode)
+{
+ switch (mode) {
+ case TYPEC_PWR_MODE_USB:
+ default:
+ return WUSB3801_CTRL0_CURRENT_DEFAULT;
+ case TYPEC_PWR_MODE_1_5A:
+ return WUSB3801_CTRL0_CURRENT_1_5A;
+ case TYPEC_PWR_MODE_3_0A:
+ return WUSB3801_CTRL0_CURRENT_3_0A;
+ }
+}
+
+static unsigned int wusb3801_map_try_role(int role)
+{
+ switch (role) {
+ case TYPEC_NO_PREFERRED_ROLE:
+ default:
+ return WUSB3801_CTRL0_TRY_NONE;
+ case TYPEC_SINK:
+ return WUSB3801_CTRL0_TRY_SNK;
+ case TYPEC_SOURCE:
+ return WUSB3801_CTRL0_TRY_SRC;
+ }
+}
+
+static enum typec_orientation wusb3801_unmap_orientation(unsigned int status)
+{
+ switch (status & WUSB3801_STAT_ORIENTATION) {
+ case WUSB3801_STAT_ORIENTATION_NONE:
+ case WUSB3801_STAT_ORIENTATION_BOTH:
+ default:
+ return TYPEC_ORIENTATION_NONE;
+ case WUSB3801_STAT_ORIENTATION_CC1:
+ return TYPEC_ORIENTATION_NORMAL;
+ case WUSB3801_STAT_ORIENTATION_CC2:
+ return TYPEC_ORIENTATION_REVERSE;
+ }
+}
+
+static enum typec_pwr_opmode wusb3801_unmap_pwr_opmode(unsigned int status)
+{
+ switch (status & WUSB3801_STAT_CURRENT) {
+ case WUSB3801_STAT_CURRENT_STANDBY:
+ case WUSB3801_STAT_CURRENT_DEFAULT:
+ default:
+ return TYPEC_PWR_MODE_USB;
+ case WUSB3801_STAT_CURRENT_1_5A:
+ return TYPEC_PWR_MODE_1_5A;
+ case WUSB3801_STAT_CURRENT_3_0A:
+ return TYPEC_PWR_MODE_3_0A;
+ }
+}
+
+static int wusb3801_try_role(struct typec_port *port, int role)
+{
+ struct wusb3801 *wusb3801 = typec_get_drvdata(port);
+
+ return regmap_update_bits(wusb3801->regmap, WUSB3801_REG_CTRL0,
+ WUSB3801_CTRL0_TRY,
+ wusb3801_map_try_role(role));
+}
+
+static int wusb3801_port_type_set(struct typec_port *port,
+ enum typec_port_type type)
+{
+ struct wusb3801 *wusb3801 = typec_get_drvdata(port);
+ int ret;
+
+ ret = regmap_update_bits(wusb3801->regmap, WUSB3801_REG_CTRL0,
+ WUSB3801_CTRL0_ROLE,
+ wusb3801_map_port_type(type));
+ if (ret)
+ return ret;
+
+ wusb3801->port_type = type;
+
+ return 0;
+}
+
+static const struct typec_operations wusb3801_typec_ops = {
+ .try_role = wusb3801_try_role,
+ .port_type_set = wusb3801_port_type_set,
+};
+
+static int wusb3801_hw_init(struct wusb3801 *wusb3801)
+{
+ return regmap_write(wusb3801->regmap, WUSB3801_REG_CTRL0,
+ wusb3801_map_try_role(wusb3801->cap.prefer_role) |
+ wusb3801_map_pwr_opmode(wusb3801->pwr_opmode) |
+ wusb3801_map_port_type(wusb3801->port_type));
+}
+
+static void wusb3801_hw_update(struct wusb3801 *wusb3801)
+{
+ struct typec_port *port = wusb3801->port;
+ struct device *dev = wusb3801->dev;
+ unsigned int partner_type, status;
+ int ret;
+
+ ret = regmap_read(wusb3801->regmap, WUSB3801_REG_STAT, &status);
+ if (ret) {
+ dev_warn(dev, "Failed to read port status: %d\n", ret);
+ status = 0;
+ }
+ dev_dbg(dev, "status = 0x%02x\n", status);
+
+ partner_type = status & WUSB3801_STAT_PARTNER;
+
+ if (partner_type == WUSB3801_STAT_PARTNER_SNK) {
+ if (!wusb3801->vbus_on) {
+ ret = regulator_enable(wusb3801->vbus_supply);
+ if (ret)
+ dev_warn(dev, "Failed to enable VBUS: %d\n", ret);
+ wusb3801->vbus_on = true;
+ }
+ } else {
+ if (wusb3801->vbus_on) {
+ regulator_disable(wusb3801->vbus_supply);
+ wusb3801->vbus_on = false;
+ }
+ }
+
+ if (partner_type != wusb3801->partner_type) {
+ struct typec_partner_desc desc = {};
+ enum typec_data_role data_role;
+ enum typec_role pwr_role = wusb3801_get_default_role(wusb3801);
+
+ switch (partner_type) {
+ case WUSB3801_STAT_PARTNER_STANDBY:
+ break;
+ case WUSB3801_STAT_PARTNER_SNK:
+ pwr_role = TYPEC_SOURCE;
+ break;
+ case WUSB3801_STAT_PARTNER_SRC:
+ pwr_role = TYPEC_SINK;
+ break;
+ case WUSB3801_STAT_PARTNER_AUDIO:
+ desc.accessory = TYPEC_ACCESSORY_AUDIO;
+ break;
+ case WUSB3801_STAT_PARTNER_DEBUG:
+ desc.accessory = TYPEC_ACCESSORY_DEBUG;
+ break;
+ }
+
+ if (wusb3801->partner) {
+ typec_unregister_partner(wusb3801->partner);
+ wusb3801->partner = NULL;
+ }
+
+ if (partner_type != WUSB3801_STAT_PARTNER_STANDBY) {
+ wusb3801->partner = typec_register_partner(port, &desc);
+ if (IS_ERR(wusb3801->partner))
+ dev_err(dev, "Failed to register partner: %ld\n",
+ PTR_ERR(wusb3801->partner));
+ }
+
+ data_role = pwr_role == TYPEC_SOURCE ? TYPEC_HOST : TYPEC_DEVICE;
+ typec_set_data_role(port, data_role);
+ typec_set_pwr_role(port, pwr_role);
+ typec_set_vconn_role(port, pwr_role);
+ }
+
+ typec_set_pwr_opmode(wusb3801->port,
+ partner_type == WUSB3801_STAT_PARTNER_SRC
+ ? wusb3801_unmap_pwr_opmode(status)
+ : wusb3801->pwr_opmode);
+ typec_set_orientation(wusb3801->port,
+ wusb3801_unmap_orientation(status));
+
+ wusb3801->partner_type = partner_type;
+}
+
+static irqreturn_t wusb3801_irq(int irq, void *data)
+{
+ struct wusb3801 *wusb3801 = data;
+ unsigned int dummy;
+
+ /*
+ * The interrupt register must be read in order to clear the IRQ,
+ * but all of the useful information is in the status register.
+ */
+ regmap_read(wusb3801->regmap, WUSB3801_REG_INT, &dummy);
+
+ wusb3801_hw_update(wusb3801);
+
+ return IRQ_HANDLED;
+}
+
+static const struct regmap_config config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = WUSB3801_REG_TEST12,
+};
+
+static int wusb3801_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct fwnode_handle *connector;
+ unsigned int device_id, test01;
+ struct wusb3801 *wusb3801;
+ const char *cap_str;
+ int ret;
+
+ wusb3801 = devm_kzalloc(dev, sizeof(*wusb3801), GFP_KERNEL);
+ if (!wusb3801)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, wusb3801);
+
+ wusb3801->dev = dev;
+
+ wusb3801->regmap = devm_regmap_init_i2c(client, &config);
+ if (IS_ERR(wusb3801->regmap))
+ return PTR_ERR(wusb3801->regmap);
+
+ regmap_read(wusb3801->regmap, WUSB3801_REG_DEVICE_ID, &device_id);
+ regmap_read(wusb3801->regmap, WUSB3801_REG_TEST01, &test01);
+ dev_info(dev, "Vendor ID: %ld, Version ID: %ld, Vendor SubID: 0x%02lx\n",
+ device_id & WUSB3801_DEVICE_ID_VENDOR_ID,
+ (device_id & WUSB3801_DEVICE_ID_VERSION_ID) >> 3,
+ test01 & WUSB3801_TEST01_VENDOR_SUB_ID);
+
+ wusb3801->vbus_supply = devm_regulator_get(dev, "vbus");
+ if (IS_ERR(wusb3801->vbus_supply))
+ return PTR_ERR(wusb3801->vbus_supply);
+
+ connector = device_get_named_child_node(dev, "connector");
+ if (!connector)
+ return -ENODEV;
+
+ ret = typec_get_fw_cap(&wusb3801->cap, connector);
+ if (ret)
+ goto err_put_connector;
+ wusb3801->port_type = wusb3801->cap.type;
+
+ ret = fwnode_property_read_string(connector, "typec-power-opmode", &cap_str);
+ if (ret)
+ goto err_put_connector;
+
+ ret = typec_find_pwr_opmode(cap_str);
+ if (ret < 0 || ret == TYPEC_PWR_MODE_PD)
+ goto err_put_connector;
+ wusb3801->pwr_opmode = ret;
+
+ /* Initialize the hardware with the devicetree settings. */
+ ret = wusb3801_hw_init(wusb3801);
+ if (ret)
+ return ret;
+
+ wusb3801->cap.revision = USB_TYPEC_REV_1_2;
+ wusb3801->cap.accessory[0] = TYPEC_ACCESSORY_AUDIO;
+ wusb3801->cap.accessory[1] = TYPEC_ACCESSORY_DEBUG;
+ wusb3801->cap.orientation_aware = true;
+ wusb3801->cap.driver_data = wusb3801;
+ wusb3801->cap.ops = &wusb3801_typec_ops;
+
+ wusb3801->port = typec_register_port(dev, &wusb3801->cap);
+ if (IS_ERR(wusb3801->port)) {
+ ret = PTR_ERR(wusb3801->port);
+ goto err_put_connector;
+ }
+
+ /* Initialize the port attributes from the hardware state. */
+ wusb3801_hw_update(wusb3801);
+
+ ret = request_threaded_irq(client->irq, NULL, wusb3801_irq,
+ IRQF_ONESHOT, dev_name(dev), wusb3801);
+ if (ret)
+ goto err_unregister_port;
+
+ fwnode_handle_put(connector);
+
+ return 0;
+
+err_unregister_port:
+ typec_unregister_port(wusb3801->port);
+err_put_connector:
+ fwnode_handle_put(connector);
+
+ return ret;
+}
+
+static int wusb3801_remove(struct i2c_client *client)
+{
+ struct wusb3801 *wusb3801 = i2c_get_clientdata(client);
+
+ free_irq(client->irq, wusb3801);
+
+ if (wusb3801->partner)
+ typec_unregister_partner(wusb3801->partner);
+ typec_unregister_port(wusb3801->port);
+
+ if (wusb3801->vbus_on)
+ regulator_disable(wusb3801->vbus_supply);
+
+ return 0;
+}
+
+static const struct of_device_id wusb3801_of_match[] = {
+ { .compatible = "willsemi,wusb3801" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, wusb3801_of_match);
+
+static struct i2c_driver wusb3801_driver = {
+ .probe_new = wusb3801_probe,
+ .remove = wusb3801_remove,
+ .driver = {
+ .name = "wusb3801",
+ .of_match_table = wusb3801_of_match,
+ },
+};
+
+module_i2c_driver(wusb3801_driver);
+
+MODULE_AUTHOR("Samuel Holland <[email protected]>");
+MODULE_DESCRIPTION("Willsemi WUSB3801 Type-C port controller driver");
+MODULE_LICENSE("GPL");
--
2.33.1

2022-02-03 11:05:16

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 3/4] usb: typec: Factor out non-PD fwnode properties

Basic programmable non-PD Type-C port controllers do not need the full
TCPM library, but they share the same devicetree binding and the same
typec_capability structure. Factor out a helper for parsing those
properties which map to fields in struct typec_capability, so the code
can be shared between TCPM and basic non-TCPM drivers.

Signed-off-by: Samuel Holland <[email protected]>
---

Changes in v2:
- Always put the return values from typec_find_* in a signed variable
for error checking.

drivers/usb/typec/class.c | 52 +++++++++++++++++++++++++++++++++++
drivers/usb/typec/tcpm/tcpm.c | 32 +--------------------
include/linux/usb/typec.h | 3 ++
3 files changed, 56 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 45a6f0c807cb..b67ba9478c82 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1894,6 +1894,58 @@ void *typec_get_drvdata(struct typec_port *port)
}
EXPORT_SYMBOL_GPL(typec_get_drvdata);

+int typec_get_fw_cap(struct typec_capability *cap,
+ struct fwnode_handle *fwnode)
+{
+ const char *cap_str;
+ int ret;
+
+ /*
+ * This fwnode has a "compatible" property, but is never populated as a
+ * struct device. Instead we simply parse it to read the properties.
+ * This it breaks fw_devlink=on. To maintain backward compatibility
+ * with existing DT files, we work around this by deleting any
+ * fwnode_links to/from this fwnode.
+ */
+ fw_devlink_purge_absent_suppliers(fwnode);
+
+ cap->fwnode = fwnode;
+
+ ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
+ if (ret < 0)
+ return ret;
+
+ ret = typec_find_port_power_role(cap_str);
+ if (ret < 0)
+ return ret;
+ cap->type = ret;
+
+ /* USB data support is optional */
+ ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
+ if (ret == 0) {
+ ret = typec_find_port_data_role(cap_str);
+ if (ret < 0)
+ return ret;
+ cap->data = ret;
+ }
+
+ /* Get the preferred power role for a DRP */
+ if (cap->type == TYPEC_PORT_DRP) {
+ cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
+
+ ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
+ if (ret == 0) {
+ ret = typec_find_power_role(cap_str);
+ if (ret < 0)
+ return ret;
+ cap->prefer_role = ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(typec_get_fw_cap);
+
/**
* typec_port_register_altmode - Register USB Type-C Port Alternate Mode
* @port: USB Type-C Port that supports the alternate mode
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 5fce795b69c7..8b58aa6e3509 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -5935,32 +5935,10 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
if (!fwnode)
return -EINVAL;

- /*
- * This fwnode has a "compatible" property, but is never populated as a
- * struct device. Instead we simply parse it to read the properties.
- * This it breaks fw_devlink=on. To maintain backward compatibility
- * with existing DT files, we work around this by deleting any
- * fwnode_links to/from this fwnode.
- */
- fw_devlink_purge_absent_suppliers(fwnode);
-
- /* USB data support is optional */
- ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
- if (ret == 0) {
- ret = typec_find_port_data_role(cap_str);
- if (ret < 0)
- return ret;
- port->typec_caps.data = ret;
- }
-
- ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
+ ret = typec_get_fw_cap(&port->typec_caps, fwnode);
if (ret < 0)
return ret;

- ret = typec_find_port_power_role(cap_str);
- if (ret < 0)
- return ret;
- port->typec_caps.type = ret;
port->port_type = port->typec_caps.type;
port->pd_supported = !fwnode_property_read_bool(fwnode, "pd-disable");

@@ -5997,14 +5975,6 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
if (port->port_type == TYPEC_PORT_SRC)
return 0;

- /* Get the preferred power role for DRP */
- ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
- if (ret < 0)
- return ret;
-
- port->typec_caps.prefer_role = typec_find_power_role(cap_str);
- if (port->typec_caps.prefer_role < 0)
- return -EINVAL;
sink:
port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");

diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 7ba45a97eeae..fdf737d48b3b 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -295,6 +295,9 @@ int typec_set_mode(struct typec_port *port, int mode);

void *typec_get_drvdata(struct typec_port *port);

+int typec_get_fw_cap(struct typec_capability *cap,
+ struct fwnode_handle *fwnode);
+
int typec_find_pwr_opmode(const char *name);
int typec_find_orientation(const char *name);
int typec_find_port_power_role(const char *name);
--
2.33.1

2022-02-03 19:37:07

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add willsemi

Add prefix for Will Semiconductor Co. Ltd. (http://www.willsemi.com/)

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 294093d45a23..a8ab97717a46 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1340,6 +1340,8 @@ patternProperties:
description: Wi2Wi, Inc.
"^wiligear,.*":
description: Wiligear, Ltd.
+ "^willsemi,.*":
+ description: Will Semiconductor Ltd.
"^winbond,.*":
description: Winbond Electronics corp.
"^wingtech,.*":
--
2.33.1

2022-02-07 15:01:47

by Samuel Holland

[permalink] [raw]
Subject: [PATCH v2 2/4] dt-bindings: usb: Add WUSB3801 Type-C Port Controller

Add devicetree support for the Will Semiconductor WUSB3801. This is a
basic non-PD Type-C port controller.

Signed-off-by: Samuel Holland <[email protected]>
---

(no changes since v1)

.../bindings/usb/willsemi,wusb3801.yaml | 75 +++++++++++++++++++
1 file changed, 75 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/willsemi,wusb3801.yaml

diff --git a/Documentation/devicetree/bindings/usb/willsemi,wusb3801.yaml b/Documentation/devicetree/bindings/usb/willsemi,wusb3801.yaml
new file mode 100644
index 000000000000..c2b2243c7892
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/willsemi,wusb3801.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/willsemi,wusb3801.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: WUSB3801 Type-C port controller DT bindings
+
+description:
+ The Will Semiconductor WUSB3801 is a USB Type-C port controller which
+ supports role and plug orientation detection using the CC pins. It is
+ compatible with the USB Type-C Cable and Connector Specification v1.2.
+
+maintainers:
+ - Samuel Holland <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - willsemi,wusb3801
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ connector:
+ type: object
+ $ref: ../connector/usb-connector.yaml#
+ description:
+ The managed USB Type-C connector. Since WUSB3801 does not support
+ Power Delivery, the node should have the "pd-disable" property.
+
+ properties:
+ compatible:
+ const: usb-c-connector
+
+ required:
+ - pd-disable
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - connector
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tcpc@60 {
+ compatible = "willsemi,wusb3801";
+ reg = <0x60>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
+
+ connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ vbus-supply = <&otg_switch>;
+ power-role = "dual";
+ try-power-role = "sink";
+ data-role = "dual";
+ typec-power-opmode = "default";
+ pd-disable;
+ };
+ };
+ };
--
2.33.1


2022-02-09 12:30:14

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] usb: typec: Factor out non-PD fwnode properties

Hi,

On Wed, Feb 02, 2022 at 04:19:46PM -0600, Samuel Holland wrote:
> Basic programmable non-PD Type-C port controllers do not need the full
> TCPM library, but they share the same devicetree binding and the same
> typec_capability structure. Factor out a helper for parsing those
> properties which map to fields in struct typec_capability, so the code
> can be shared between TCPM and basic non-TCPM drivers.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v2:
> - Always put the return values from typec_find_* in a signed variable
> for error checking.
>
> drivers/usb/typec/class.c | 52 +++++++++++++++++++++++++++++++++++
> drivers/usb/typec/tcpm/tcpm.c | 32 +--------------------
> include/linux/usb/typec.h | 3 ++
> 3 files changed, 56 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 45a6f0c807cb..b67ba9478c82 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1894,6 +1894,58 @@ void *typec_get_drvdata(struct typec_port *port)
> }
> EXPORT_SYMBOL_GPL(typec_get_drvdata);
>
> +int typec_get_fw_cap(struct typec_capability *cap,
> + struct fwnode_handle *fwnode)
> +{
> + const char *cap_str;
> + int ret;
> +
> + /*
> + * This fwnode has a "compatible" property, but is never populated as a
> + * struct device. Instead we simply parse it to read the properties.
> + * This it breaks fw_devlink=on. To maintain backward compatibility
> + * with existing DT files, we work around this by deleting any
> + * fwnode_links to/from this fwnode.
> + */
> + fw_devlink_purge_absent_suppliers(fwnode);

Let's not put that call here. That function is broken. I think it in
practice assumes that there can only be one device linked to fwnode,
but that's not true. The fwnodes can be, and are, shared. So by using
it we may end up doing things to some completely wrong devices.

So let's keep that call in the drivers that really have to have it for
now. I think that function - fw_devlink_purge_absent_suppliers() -
needs some serious rethinking.

There is some deeper problem. I have a feeling that all the functions
that rely on the fwnode->dev member are broken. We need a proper
reverce search mechanism that can be used to find the devices linked
to fwnodes. But I have no idea how to that could be done.

> + cap->fwnode = fwnode;
> +
> + ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
> + if (ret < 0)
> + return ret;
> +
> + ret = typec_find_port_power_role(cap_str);
> + if (ret < 0)
> + return ret;
> + cap->type = ret;
> +
> + /* USB data support is optional */
> + ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
> + if (ret == 0) {
> + ret = typec_find_port_data_role(cap_str);
> + if (ret < 0)
> + return ret;
> + cap->data = ret;
> + }
> +
> + /* Get the preferred power role for a DRP */
> + if (cap->type == TYPEC_PORT_DRP) {
> + cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
> +
> + ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
> + if (ret == 0) {
> + ret = typec_find_power_role(cap_str);
> + if (ret < 0)
> + return ret;
> + cap->prefer_role = ret;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(typec_get_fw_cap);

thanks,

--
heikki

2022-02-09 12:46:53

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] usb: typec: Factor out non-PD fwnode properties

On Wed, Feb 02, 2022 at 04:19:46PM -0600, Samuel Holland wrote:
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 5fce795b69c7..8b58aa6e3509 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -5935,32 +5935,10 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> if (!fwnode)
> return -EINVAL;
>
> - /*
> - * This fwnode has a "compatible" property, but is never populated as a
> - * struct device. Instead we simply parse it to read the properties.
> - * This it breaks fw_devlink=on. To maintain backward compatibility
> - * with existing DT files, we work around this by deleting any
> - * fwnode_links to/from this fwnode.
> - */
> - fw_devlink_purge_absent_suppliers(fwnode);
> -
> - /* USB data support is optional */
> - ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
> - if (ret == 0) {
> - ret = typec_find_port_data_role(cap_str);
> - if (ret < 0)
> - return ret;
> - port->typec_caps.data = ret;
> - }
> -
> - ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
> + ret = typec_get_fw_cap(&port->typec_caps, fwnode);
> if (ret < 0)
> return ret;
>
> - ret = typec_find_port_power_role(cap_str);
> - if (ret < 0)
> - return ret;
> - port->typec_caps.type = ret;
> port->port_type = port->typec_caps.type;
> port->pd_supported = !fwnode_property_read_bool(fwnode, "pd-disable");
>
> @@ -5997,14 +5975,6 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> if (port->port_type == TYPEC_PORT_SRC)
> return 0;
>
> - /* Get the preferred power role for DRP */
> - ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
> - if (ret < 0)
> - return ret;
> -
> - port->typec_caps.prefer_role = typec_find_power_role(cap_str);
> - if (port->typec_caps.prefer_role < 0)
> - return -EINVAL;
> sink:
> port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");

It looks like after this there are no more users for that cap_str
variable. You need to remove that too.

thanks,

--
heikki

2022-02-09 13:08:59

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] usb: typec: Support the WUSB3801 port controller

On Wed, Feb 02, 2022 at 04:19:47PM -0600, Samuel Holland wrote:
> WUSB3801 features a configurable port type, accessory detection, and
> plug orientation detection. It provides a hardware "ID" pin output for
> compatibility with USB 2.0 OTG PHYs. Add a typec class driver for it.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Changes in v2:
> - License the driver as GPL 2 only; probably best anyway as I used a
> lot of other drivers/usb/typec code as inspiration
> - Don't try to be clever; use `default` instead of `unreachable`
> - Free the IRQ before unregistering the partner/port
>
> drivers/usb/typec/Kconfig | 10 +
> drivers/usb/typec/Makefile | 1 +
> drivers/usb/typec/wusb3801.c | 445 +++++++++++++++++++++++++++++++++++
> 3 files changed, 456 insertions(+)
> create mode 100644 drivers/usb/typec/wusb3801.c

This looked mostly OK to me. One nitpick below.

> +static int wusb3801_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct fwnode_handle *connector;
> + unsigned int device_id, test01;
> + struct wusb3801 *wusb3801;
> + const char *cap_str;
> + int ret;
> +
> + wusb3801 = devm_kzalloc(dev, sizeof(*wusb3801), GFP_KERNEL);
> + if (!wusb3801)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, wusb3801);
> +
> + wusb3801->dev = dev;
> +
> + wusb3801->regmap = devm_regmap_init_i2c(client, &config);
> + if (IS_ERR(wusb3801->regmap))
> + return PTR_ERR(wusb3801->regmap);
> +
> + regmap_read(wusb3801->regmap, WUSB3801_REG_DEVICE_ID, &device_id);
> + regmap_read(wusb3801->regmap, WUSB3801_REG_TEST01, &test01);
> + dev_info(dev, "Vendor ID: %ld, Version ID: %ld, Vendor SubID: 0x%02lx\n",
> + device_id & WUSB3801_DEVICE_ID_VENDOR_ID,
> + (device_id & WUSB3801_DEVICE_ID_VERSION_ID) >> 3,
> + test01 & WUSB3801_TEST01_VENDOR_SUB_ID);

That is just noise.

> + wusb3801->vbus_supply = devm_regulator_get(dev, "vbus");
> + if (IS_ERR(wusb3801->vbus_supply))
> + return PTR_ERR(wusb3801->vbus_supply);
> +
> + connector = device_get_named_child_node(dev, "connector");
> + if (!connector)
> + return -ENODEV;
> +
> + ret = typec_get_fw_cap(&wusb3801->cap, connector);

One note here: Don't use fw_devlink_purge_absent_suppliers() here
either unless you really see some problem yourself!

That function is broken like I said. What ever it's fixing, it's doing
it wrong. That function seems to be just a broken hack that most
likely covered some individual case that was reported at the time.
Instead of hacks like that, we need to figure out a solution for the
core problem, what ever that might be.

> + if (ret)
> + goto err_put_connector;
> + wusb3801->port_type = wusb3801->cap.type;
> +
> + ret = fwnode_property_read_string(connector, "typec-power-opmode", &cap_str);
> + if (ret)
> + goto err_put_connector;
> +
> + ret = typec_find_pwr_opmode(cap_str);
> + if (ret < 0 || ret == TYPEC_PWR_MODE_PD)
> + goto err_put_connector;
> + wusb3801->pwr_opmode = ret;
> +
> + /* Initialize the hardware with the devicetree settings. */
> + ret = wusb3801_hw_init(wusb3801);
> + if (ret)
> + return ret;
> +
> + wusb3801->cap.revision = USB_TYPEC_REV_1_2;
> + wusb3801->cap.accessory[0] = TYPEC_ACCESSORY_AUDIO;
> + wusb3801->cap.accessory[1] = TYPEC_ACCESSORY_DEBUG;
> + wusb3801->cap.orientation_aware = true;
> + wusb3801->cap.driver_data = wusb3801;
> + wusb3801->cap.ops = &wusb3801_typec_ops;
> +
> + wusb3801->port = typec_register_port(dev, &wusb3801->cap);
> + if (IS_ERR(wusb3801->port)) {
> + ret = PTR_ERR(wusb3801->port);
> + goto err_put_connector;
> + }
> +
> + /* Initialize the port attributes from the hardware state. */
> + wusb3801_hw_update(wusb3801);
> +
> + ret = request_threaded_irq(client->irq, NULL, wusb3801_irq,
> + IRQF_ONESHOT, dev_name(dev), wusb3801);
> + if (ret)
> + goto err_unregister_port;
> +
> + fwnode_handle_put(connector);
> +
> + return 0;
> +
> +err_unregister_port:
> + typec_unregister_port(wusb3801->port);
> +err_put_connector:
> + fwnode_handle_put(connector);
> +
> + return ret;
> +}

thanks,

--
heikki

2022-02-09 23:33:12

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add willsemi

On Wed, 02 Feb 2022 16:19:44 -0600, Samuel Holland wrote:
> Add prefix for Will Semiconductor Co. Ltd. (http://www.willsemi.com/)
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2022-02-09 23:34:04

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: usb: Add WUSB3801 Type-C Port Controller

On Wed, 02 Feb 2022 16:19:45 -0600, Samuel Holland wrote:
> Add devicetree support for the Will Semiconductor WUSB3801. This is a
> basic non-PD Type-C port controller.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> (no changes since v1)
>
> .../bindings/usb/willsemi,wusb3801.yaml | 75 +++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/willsemi,wusb3801.yaml
>

Reviewed-by: Rob Herring <[email protected]>