2019-05-29 07:45:35

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 00/10] add USB Type-B GPIO connector driver

Because the USB Connector is introduced and the requirement of
usb-connector.txt binding, the old way using extcon to support
USB Dual-Role switch is now deprecated, meanwhile there is no
available common driver when use Type-B connector, typically
using an input GPIO to detect USB ID pin.
This patch series introduce a Type-B GPIO connector driver and try
to replace the function provided by extcon-usb-gpio driver.

v6 changes:
1. merge [1] and [2] into this series
2. don't use graph anymore to find usb-role-switch
3. abandon [3] and introduce three patches (6, 7, 8 in this series)
to rebuild APIs getting usb-role-switch

[1]: [v3] dt-binding: usb: add usb-role-switch property
https://patchwork.kernel.org/patch/10934835/
[2]: [v6,08/13] usb: roles: Introduce stubs for the exiting functions in role.h
https://patchwork.kernel.org/patch/10909971/

[3]: [PATCH v5 4/6] usb: roles: add API to get usb_role_switch by node

v5 changes:
1. remove linux/of.h and put usb_role_switch when error happens,
suggested by Biju
2. treat Type-B connector as USB controller's child, but not as
a virtual device, suggested by Rob
3. provide and use generic property "usb-role-switch", see [1],
suggested by Rob

Note: this series still depends on [2]

[1]: [v3] dt-binding: usb: add usb-role-switch property
https://patchwork.kernel.org/patch/10934835/
[2]: [v6,08/13] usb: roles: Introduce stubs for the exiting functions in role.h
https://patchwork.kernel.org/patch/10909971/

v4 changes:
1. use switch_fwnode_match() to find fwnode suggested by Heikki
2. assign fwnode member of usb_role_switch struct suggested by Heikki
3. make [4/6] depend on [2]
3. remove linux/gpio.h suggested by Linus
4. put node when error happens

[4/6] usb: roles: add API to get usb_role_switch by node
[2] [v6,08/13] usb: roles: Introduce stubs for the exiting functions in role.h
https://patchwork.kernel.org/patch/10909971/

v3 changes:
1. add GPIO direction, and use fixed-regulator for GPIO controlled
VBUS regulator suggested by Rob;
2. rebuild fwnode_usb_role_switch_get() suggested by Andy and Heikki
3. treat the type-B connector as a virtual device;
4. change file name of driver again
5. select USB_ROLE_SWITCH in mtu3/Kconfig suggested by Heikki
6. rename ssusb_mode_manual_switch() to ssusb_mode_switch()

v2 changes:
1. make binding clear, and add a extra compatible suggested by Hans

Chunfeng Yun (7):
dt-binding: usb: add usb-role-switch property
dt-bindings: connector: add optional properties for Type-B
dt-bindings: usb: add binding for Type-B GPIO connector driver
dt-bindings: usb: mtu3: add properties about USB Role Switch
usb: roles: get usb-role-switch from parent
usb: roles: add USB Type-B GPIO connector driver
usb: mtu3: register a USB Role Switch for dual role mode

Heikki Krogerus (2):
device connection: Add fwnode_connection_find_match()
usb: roles: Add fwnode_usb_role_switch_get() function

Yu Chen (1):
usb: roles: Introduce stubs for the exiting functions in role.h.

.../bindings/connector/usb-connector.txt | 14 +
.../devicetree/bindings/usb/generic.txt | 4 +
.../devicetree/bindings/usb/mediatek,mtu3.txt | 10 +
.../bindings/usb/typeb-conn-gpio.txt | 30 ++
drivers/base/devcon.c | 33 +-
drivers/usb/mtu3/Kconfig | 1 +
drivers/usb/mtu3/mtu3.h | 5 +
drivers/usb/mtu3/mtu3_debugfs.c | 4 +-
drivers/usb/mtu3/mtu3_dr.c | 48 ++-
drivers/usb/mtu3/mtu3_dr.h | 6 +-
drivers/usb/mtu3/mtu3_plat.c | 3 +-
drivers/usb/roles/Kconfig | 11 +
drivers/usb/roles/Makefile | 1 +
drivers/usb/roles/class.c | 41 ++-
drivers/usb/roles/typeb-conn-gpio.c | 286 ++++++++++++++++++
include/linux/device.h | 10 +-
include/linux/usb/role.h | 37 +++
17 files changed, 524 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/typeb-conn-gpio.txt
create mode 100644 drivers/usb/roles/typeb-conn-gpio.c

--
2.21.0


2019-05-29 07:46:19

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

Due to the requirement of usb-connector.txt binding, the old way
using extcon to support USB Dual-Role switch is now deprecated
when use Type-B connector.
This patch introduces a driver of Type-B connector which typically
uses an input GPIO to detect USB ID pin, and try to replace the
function provided by extcon-usb-gpio driver

Signed-off-by: Chunfeng Yun <[email protected]>
---
v6 changes:
1. get usb-role-swtich by usb_role_switch_get()

v5 changes:
1. put usb_role_switch when error happens suggested by Biju
2. don't treat bype-B connector as a virtual device suggested by Rob

v4 changes:
1. remove linux/gpio.h suggested by Linus
2. put node when error happens

v3 changes:
1. treat bype-B connector as a virtual device;
2. change file name again

v2 changes:
1. file name is changed
2. use new compatible
---
drivers/usb/roles/Kconfig | 11 ++
drivers/usb/roles/Makefile | 1 +
drivers/usb/roles/typeb-conn-gpio.c | 286 ++++++++++++++++++++++++++++
3 files changed, 298 insertions(+)
create mode 100644 drivers/usb/roles/typeb-conn-gpio.c

diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig
index f8b31aa67526..d1156e18a81a 100644
--- a/drivers/usb/roles/Kconfig
+++ b/drivers/usb/roles/Kconfig
@@ -26,4 +26,15 @@ config USB_ROLES_INTEL_XHCI
To compile the driver as a module, choose M here: the module will
be called intel-xhci-usb-role-switch.

+config TYPEB_CONN_GPIO
+ tristate "USB Type-B GPIO Connector"
+ depends on GPIOLIB
+ help
+ The driver supports USB role switch between host and device via GPIO
+ based USB cable detection, used typically if an input GPIO is used
+ to detect USB ID pin.
+
+ To compile the driver as a module, choose M here: the module will
+ be called typeb-conn-gpio.ko
+
endif # USB_ROLE_SWITCH
diff --git a/drivers/usb/roles/Makefile b/drivers/usb/roles/Makefile
index 757a7d2797eb..5d5620d9d113 100644
--- a/drivers/usb/roles/Makefile
+++ b/drivers/usb/roles/Makefile
@@ -3,3 +3,4 @@
obj-$(CONFIG_USB_ROLE_SWITCH) += roles.o
roles-y := class.o
obj-$(CONFIG_USB_ROLES_INTEL_XHCI) += intel-xhci-usb-role-switch.o
+obj-$(CONFIG_TYPEB_CONN_GPIO) += typeb-conn-gpio.o
diff --git a/drivers/usb/roles/typeb-conn-gpio.c b/drivers/usb/roles/typeb-conn-gpio.c
new file mode 100644
index 000000000000..c66f852365ef
--- /dev/null
+++ b/drivers/usb/roles/typeb-conn-gpio.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Type-B GPIO Connector Driver
+ *
+ * Copyright (C) 2019 MediaTek Inc.
+ *
+ * Author: Chunfeng Yun <[email protected]>
+ *
+ * Some code borrowed from drivers/extcon/extcon-usb-gpio.c
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/role.h>
+
+#define USB_GPIO_DEB_MS 20 /* ms */
+#define USB_GPIO_DEB_US ((USB_GPIO_DEB_MS) * 1000) /* us */
+
+#define USB_CONN_IRQF \
+ (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT)
+
+struct usb_conn_info {
+ struct device *dev;
+ struct usb_role_switch *role_sw;
+ enum usb_role last_role;
+ struct regulator *vbus;
+ struct delayed_work dw_det;
+ unsigned long debounce_jiffies;
+
+ struct gpio_desc *id_gpiod;
+ struct gpio_desc *vbus_gpiod;
+ int id_irq;
+ int vbus_irq;
+};
+
+/**
+ * "DEVICE" = VBUS and "HOST" = !ID, so we have:
+ * Both "DEVICE" and "HOST" can't be set as active at the same time
+ * so if "HOST" is active (i.e. ID is 0) we keep "DEVICE" inactive
+ * even if VBUS is on.
+ *
+ * Role | ID | VBUS
+ * ------------------------------------
+ * [1] DEVICE | H | H
+ * [2] NONE | H | L
+ * [3] HOST | L | H
+ * [4] HOST | L | L
+ *
+ * In case we have only one of these signals:
+ * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1
+ * - ID only - we want to distinguish between [1] and [4], so VBUS = ID
+ */
+static void usb_conn_detect_cable(struct work_struct *work)
+{
+ struct usb_conn_info *info;
+ enum usb_role role;
+ int id, vbus, ret;
+
+ info = container_of(to_delayed_work(work),
+ struct usb_conn_info, dw_det);
+
+ /* check ID and VBUS */
+ id = info->id_gpiod ?
+ gpiod_get_value_cansleep(info->id_gpiod) : 1;
+ vbus = info->vbus_gpiod ?
+ gpiod_get_value_cansleep(info->vbus_gpiod) : id;
+
+ if (!id)
+ role = USB_ROLE_HOST;
+ else if (vbus)
+ role = USB_ROLE_DEVICE;
+ else
+ role = USB_ROLE_NONE;
+
+ dev_dbg(info->dev, "role %d/%d, gpios: id %d, vbus %d\n",
+ info->last_role, role, id, vbus);
+
+ if (info->last_role == role) {
+ dev_warn(info->dev, "repeated role: %d\n", role);
+ return;
+ }
+
+ if (info->last_role == USB_ROLE_HOST)
+ regulator_disable(info->vbus);
+
+ ret = usb_role_switch_set_role(info->role_sw, role);
+ if (ret)
+ dev_err(info->dev, "failed to set role: %d\n", ret);
+
+ if (role == USB_ROLE_HOST) {
+ ret = regulator_enable(info->vbus);
+ if (ret)
+ dev_err(info->dev, "enable vbus regulator failed\n");
+ }
+
+ info->last_role = role;
+
+ dev_dbg(info->dev, "vbus regulator is %s\n",
+ regulator_is_enabled(info->vbus) ? "enabled" : "disabled");
+}
+
+static void usb_conn_queue_dwork(struct usb_conn_info *info,
+ unsigned long delay)
+{
+ queue_delayed_work(system_power_efficient_wq, &info->dw_det, delay);
+}
+
+static irqreturn_t usb_conn_isr(int irq, void *dev_id)
+{
+ struct usb_conn_info *info = dev_id;
+
+ usb_conn_queue_dwork(info, info->debounce_jiffies);
+
+ return IRQ_HANDLED;
+}
+
+static int usb_conn_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct usb_conn_info *info;
+ int ret = 0;
+
+ info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info->dev = dev;
+ info->id_gpiod = devm_gpiod_get_optional(dev, "id", GPIOD_IN);
+ if (IS_ERR(info->id_gpiod))
+ return PTR_ERR(info->id_gpiod);
+
+ info->vbus_gpiod = devm_gpiod_get_optional(dev, "vbus", GPIOD_IN);
+ if (IS_ERR(info->vbus_gpiod))
+ return PTR_ERR(info->vbus_gpiod);
+
+ if (!info->id_gpiod && !info->vbus_gpiod) {
+ dev_err(dev, "failed to get gpios\n");
+ return -ENODEV;
+ }
+
+ if (info->id_gpiod)
+ ret = gpiod_set_debounce(info->id_gpiod, USB_GPIO_DEB_US);
+ if (!ret && info->vbus_gpiod)
+ ret = gpiod_set_debounce(info->vbus_gpiod, USB_GPIO_DEB_US);
+ if (ret < 0)
+ info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEB_MS);
+
+ INIT_DELAYED_WORK(&info->dw_det, usb_conn_detect_cable);
+
+ info->vbus = devm_regulator_get(dev, "vbus");
+ if (IS_ERR(info->vbus)) {
+ dev_err(dev, "failed to get vbus\n");
+ return PTR_ERR(info->vbus);
+ }
+
+ info->role_sw = usb_role_switch_get(dev);
+ if (IS_ERR(info->role_sw)) {
+ if (PTR_ERR(info->role_sw) != -EPROBE_DEFER)
+ dev_err(dev, "failed to get role switch\n");
+
+ return PTR_ERR(info->role_sw);
+ }
+
+ if (info->id_gpiod) {
+ info->id_irq = gpiod_to_irq(info->id_gpiod);
+ if (info->id_irq < 0) {
+ dev_err(dev, "failed to get ID IRQ\n");
+ ret = info->id_irq;
+ goto put_role_sw;
+ }
+
+ ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
+ usb_conn_isr, USB_CONN_IRQF,
+ pdev->name, info);
+ if (ret < 0) {
+ dev_err(dev, "failed to request ID IRQ\n");
+ goto put_role_sw;
+ }
+ }
+
+ if (info->vbus_gpiod) {
+ info->vbus_irq = gpiod_to_irq(info->vbus_gpiod);
+ if (info->vbus_irq < 0) {
+ dev_err(dev, "failed to get VBUS IRQ\n");
+ ret = info->vbus_irq;
+ goto put_role_sw;
+ }
+
+ ret = devm_request_threaded_irq(dev, info->vbus_irq, NULL,
+ usb_conn_isr, USB_CONN_IRQF,
+ pdev->name, info);
+ if (ret < 0) {
+ dev_err(dev, "failed to request VBUS IRQ\n");
+ goto put_role_sw;
+ }
+ }
+
+ platform_set_drvdata(pdev, info);
+
+ /* Perform initial detection */
+ usb_conn_queue_dwork(info, 0);
+
+ return 0;
+
+put_role_sw:
+ usb_role_switch_put(info->role_sw);
+ return ret;
+}
+
+static int usb_conn_remove(struct platform_device *pdev)
+{
+ struct usb_conn_info *info = platform_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&info->dw_det);
+
+ if (info->last_role == USB_ROLE_HOST)
+ regulator_disable(info->vbus);
+
+ usb_role_switch_put(info->role_sw);
+
+ return 0;
+}
+
+static int __maybe_unused usb_conn_suspend(struct device *dev)
+{
+ struct usb_conn_info *info = dev_get_drvdata(dev);
+
+ if (info->id_gpiod)
+ disable_irq(info->id_irq);
+ if (info->vbus_gpiod)
+ disable_irq(info->vbus_irq);
+
+ pinctrl_pm_select_sleep_state(dev);
+
+ return 0;
+}
+
+static int __maybe_unused usb_conn_resume(struct device *dev)
+{
+ struct usb_conn_info *info = dev_get_drvdata(dev);
+
+ pinctrl_pm_select_default_state(dev);
+
+ if (info->id_gpiod)
+ enable_irq(info->id_irq);
+ if (info->vbus_gpiod)
+ enable_irq(info->vbus_irq);
+
+ usb_conn_queue_dwork(info, 0);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
+ usb_conn_suspend, usb_conn_resume);
+
+#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)
+
+static const struct of_device_id usb_conn_dt_match[] = {
+ { .compatible = "linux,typeb-conn-gpio", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, usb_conn_dt_match);
+
+static struct platform_driver usb_conn_driver = {
+ .probe = usb_conn_probe,
+ .remove = usb_conn_remove,
+ .driver = {
+ .name = "typeb-conn-gpio",
+ .pm = DEV_PMS_OPS,
+ .of_match_table = usb_conn_dt_match,
+ },
+};
+
+module_platform_driver(usb_conn_driver);
+
+MODULE_AUTHOR("Chunfeng Yun <[email protected]>");
+MODULE_DESCRIPTION("USB Type-B GPIO connector driver");
+MODULE_LICENSE("GPL v2");
--
2.21.0

2019-05-29 07:46:23

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 06/10] device connection: Add fwnode_connection_find_match()

From: Heikki Krogerus <[email protected]>

The fwnode_connection_find_match() function is exactly the
same as device_connection_find_match(), except it takes
struct fwnode_handle as parameter instead of struct device.
That allows locating device connections before the device
entries have been created.

Signed-off-by: Heikki Krogerus <[email protected]>
---
v6:
new patch
---
drivers/base/devcon.c | 33 +++++++++++++++++++++++++--------
include/linux/device.h | 10 +++++++---
2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 04db9ae235e4..8311b70bbca2 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -12,9 +12,6 @@
static DEFINE_MUTEX(devcon_lock);
static LIST_HEAD(devcon_list);

-typedef void *(*devcon_match_fn_t)(struct device_connection *con, int ep,
- void *data);
-
static void *
fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
void *data, devcon_match_fn_t match)
@@ -38,6 +35,28 @@ fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
return NULL;
}

+/**
+ * fwnode_connection_find_match - Find connection from a device node
+ * @fwnode: Device node with the connection
+ * @con_id: Identifier for the connection
+ * @data: Data for the match function
+ * @match: Function to check and convert the connection description
+ *
+ * Find a connection with unique identifier @con_id between @fwnode and another
+ * device node. @match will be used to convert the connection description to
+ * data the caller is expecting to be returned.
+ */
+void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
+ const char *con_id, void *data,
+ devcon_match_fn_t match)
+{
+ if (!fwnode || !match)
+ return NULL;
+
+ return fwnode_graph_devcon_match(fwnode, con_id, data, match);
+}
+EXPORT_SYMBOL_GPL(fwnode_connection_find_match);
+
/**
* device_connection_find_match - Find physical connection to a device
* @dev: Device with the connection
@@ -61,11 +80,9 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
if (!match)
return NULL;

- if (fwnode) {
- ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
- if (ret)
- return ret;
- }
+ ret = fwnode_connection_find_match(fwnode, con_id, data, match);
+ if (ret)
+ return ret;

mutex_lock(&devcon_lock);

diff --git a/include/linux/device.h b/include/linux/device.h
index e85264fb6616..9445f068602f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -772,10 +772,14 @@ struct device_connection {
struct list_head list;
};

+typedef void *(*devcon_match_fn_t)(struct device_connection *con, int ep,
+ void *data);
+
+void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
+ const char *con_id, void *data,
+ devcon_match_fn_t match);
void *device_connection_find_match(struct device *dev, const char *con_id,
- void *data,
- void *(*match)(struct device_connection *con,
- int ep, void *data));
+ void *data, devcon_match_fn_t match);

struct device *device_connection_find(struct device *dev, const char *con_id);

--
2.21.0

2019-05-29 07:46:23

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 10/10] usb: mtu3: register a USB Role Switch for dual role mode

Because extcon is not allowed for new bindings, and the
dual role switch is supported by USB Role Switch,
especially for Type-C drivers, so register a USB Role
Switch to support the new way

Signed-off-by: Chunfeng Yun <[email protected]>
---
v6 no changes

v5 no changes

v4 changes:
1. assign fwnode member of usb_role_switch struct suggested by Heikki

v3 changes:
1. select USB_ROLE_SWITCH in Kconfig suggested by Heikki
2. rename ssusb_mode_manual_switch() to ssusb_mode_switch()

v2 no changes
---
drivers/usb/mtu3/Kconfig | 1 +
drivers/usb/mtu3/mtu3.h | 5 ++++
drivers/usb/mtu3/mtu3_debugfs.c | 4 +--
drivers/usb/mtu3/mtu3_dr.c | 48 ++++++++++++++++++++++++++++++++-
drivers/usb/mtu3/mtu3_dr.h | 6 ++---
drivers/usb/mtu3/mtu3_plat.c | 3 ++-
6 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
index 928c2cd6fc00..bf98fd36341d 100644
--- a/drivers/usb/mtu3/Kconfig
+++ b/drivers/usb/mtu3/Kconfig
@@ -44,6 +44,7 @@ config USB_MTU3_DUAL_ROLE
bool "Dual Role mode"
depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || USB_GADGET=USB_MTU3))
depends on (EXTCON=y || EXTCON=USB_MTU3)
+ select USB_ROLE_SWITCH
help
This is the default mode of working of MTU3 controller where
both host and gadget features are enabled.
diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index 76ecf12fdf62..6087be236a35 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -199,6 +199,9 @@ struct mtu3_gpd_ring {
* @id_nb : notifier for iddig(idpin) detection
* @id_work : work of iddig detection notifier
* @id_event : event of iddig detecion notifier
+* @role_sw : use USB Role Switch to support dual-role switch, can't use
+* extcon at the same time, and extcon is deprecated.
+* @role_sw_used : true when the USB Role Switch is used.
* @is_u3_drd: whether port0 supports usb3.0 dual-role device or not
* @manual_drd_enabled: it's true when supports dual-role device by debugfs
* to switch host/device modes depending on user input.
@@ -212,6 +215,8 @@ struct otg_switch_mtk {
struct notifier_block id_nb;
struct work_struct id_work;
unsigned long id_event;
+ struct usb_role_switch *role_sw;
+ bool role_sw_used;
bool is_u3_drd;
bool manual_drd_enabled;
};
diff --git a/drivers/usb/mtu3/mtu3_debugfs.c b/drivers/usb/mtu3/mtu3_debugfs.c
index 62c57ddc554e..c96e5dab0a48 100644
--- a/drivers/usb/mtu3/mtu3_debugfs.c
+++ b/drivers/usb/mtu3/mtu3_debugfs.c
@@ -453,9 +453,9 @@ static ssize_t ssusb_mode_write(struct file *file, const char __user *ubuf,
return -EFAULT;

if (!strncmp(buf, "host", 4) && !ssusb->is_host) {
- ssusb_mode_manual_switch(ssusb, 1);
+ ssusb_mode_switch(ssusb, 1);
} else if (!strncmp(buf, "device", 6) && ssusb->is_host) {
- ssusb_mode_manual_switch(ssusb, 0);
+ ssusb_mode_switch(ssusb, 0);
} else {
dev_err(ssusb->dev, "wrong or duplicated setting\n");
return -EINVAL;
diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
index 5fcb71af875a..08e18448e8b8 100644
--- a/drivers/usb/mtu3/mtu3_dr.c
+++ b/drivers/usb/mtu3/mtu3_dr.c
@@ -7,6 +7,8 @@
* Author: Chunfeng Yun <[email protected]>
*/

+#include <linux/usb/role.h>
+
#include "mtu3.h"
#include "mtu3_dr.h"
#include "mtu3_debug.h"
@@ -280,7 +282,7 @@ static int ssusb_extcon_register(struct otg_switch_mtk *otg_sx)
* This is useful in special cases, such as uses TYPE-A receptacle but also
* wants to support dual-role mode.
*/
-void ssusb_mode_manual_switch(struct ssusb_mtk *ssusb, int to_host)
+void ssusb_mode_switch(struct ssusb_mtk *ssusb, int to_host)
{
struct otg_switch_mtk *otg_sx = &ssusb->otg_switch;

@@ -318,6 +320,47 @@ void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
mtu3_writel(ssusb->ippc_base, SSUSB_U2_CTRL(0), value);
}

+static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
+{
+ struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
+ bool to_host = false;
+
+ if (role == USB_ROLE_HOST)
+ to_host = true;
+
+ if (to_host ^ ssusb->is_host)
+ ssusb_mode_switch(ssusb, to_host);
+
+ return 0;
+}
+
+static enum usb_role ssusb_role_sw_get(struct device *dev)
+{
+ struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
+ enum usb_role role;
+
+ role = ssusb->is_host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+
+ return role;
+}
+
+static int ssusb_role_sw_register(struct otg_switch_mtk *otg_sx)
+{
+ struct usb_role_switch_desc role_sx_desc = { 0 };
+ struct ssusb_mtk *ssusb =
+ container_of(otg_sx, struct ssusb_mtk, otg_switch);
+
+ if (!otg_sx->role_sw_used)
+ return 0;
+
+ role_sx_desc.set = ssusb_role_sw_set;
+ role_sx_desc.get = ssusb_role_sw_get;
+ role_sx_desc.fwnode = dev_fwnode(ssusb->dev);
+ otg_sx->role_sw = usb_role_switch_register(ssusb->dev, &role_sx_desc);
+
+ return PTR_ERR_OR_ZERO(otg_sx->role_sw);
+}
+
int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)
{
struct otg_switch_mtk *otg_sx = &ssusb->otg_switch;
@@ -328,6 +371,8 @@ int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)

if (otg_sx->manual_drd_enabled)
ssusb_dr_debugfs_init(ssusb);
+ else if (otg_sx->role_sw_used)
+ ret = ssusb_role_sw_register(otg_sx);
else
ret = ssusb_extcon_register(otg_sx);

@@ -340,4 +385,5 @@ void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb)

cancel_work_sync(&otg_sx->id_work);
cancel_work_sync(&otg_sx->vbus_work);
+ usb_role_switch_unregister(otg_sx->role_sw);
}
diff --git a/drivers/usb/mtu3/mtu3_dr.h b/drivers/usb/mtu3/mtu3_dr.h
index ba6fe357ce29..5e58c4dbd54a 100644
--- a/drivers/usb/mtu3/mtu3_dr.h
+++ b/drivers/usb/mtu3/mtu3_dr.h
@@ -71,7 +71,7 @@ static inline void ssusb_gadget_exit(struct ssusb_mtk *ssusb)
#if IS_ENABLED(CONFIG_USB_MTU3_DUAL_ROLE)
int ssusb_otg_switch_init(struct ssusb_mtk *ssusb);
void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb);
-void ssusb_mode_manual_switch(struct ssusb_mtk *ssusb, int to_host);
+void ssusb_mode_switch(struct ssusb_mtk *ssusb, int to_host);
int ssusb_set_vbus(struct otg_switch_mtk *otg_sx, int is_on);
void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
enum mtu3_dr_force_mode mode);
@@ -86,8 +86,8 @@ static inline int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)
static inline void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb)
{}

-static inline void
-ssusb_mode_manual_switch(struct ssusb_mtk *ssusb, int to_host) {}
+static inline void ssusb_mode_switch(struct ssusb_mtk *ssusb, int to_host)
+{}

static inline int ssusb_set_vbus(struct otg_switch_mtk *otg_sx, int is_on)
{
diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
index fd0f6c5dfbc1..9c256ea3cdf5 100644
--- a/drivers/usb/mtu3/mtu3_plat.c
+++ b/drivers/usb/mtu3/mtu3_plat.c
@@ -299,8 +299,9 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
otg_sx->is_u3_drd = of_property_read_bool(node, "mediatek,usb3-drd");
otg_sx->manual_drd_enabled =
of_property_read_bool(node, "enable-manual-drd");
+ otg_sx->role_sw_used = of_property_read_bool(node, "usb-role-switch");

- if (of_property_read_bool(node, "extcon")) {
+ if (!otg_sx->role_sw_used && of_property_read_bool(node, "extcon")) {
otg_sx->edev = extcon_get_edev_by_phandle(ssusb->dev, 0);
if (IS_ERR(otg_sx->edev)) {
dev_err(ssusb->dev, "couldn't get extcon device\n");
--
2.21.0

2019-05-29 07:46:30

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 04/10] dt-bindings: usb: mtu3: add properties about USB Role Switch

Now the USB Role Switch is supported, so add properties about it,
and modify some description related.

Signed-off-by: Chunfeng Yun <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v4 no changes

v5 changes:
1. modify decription about extcon and vbus-supply properties
2. make this patch depend on [1]

[1]: [v3] dt-binding: usb: add usb-role-switch property
https://patchwork.kernel.org/patch/10934835/

v4 no changes
v3 no changes

v2 changes:
1. fix typo
2. refer new binding about connector property
---
.../devicetree/bindings/usb/mediatek,mtu3.txt | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt b/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
index 3382b5cb471d..3a8300205cdb 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
@@ -28,8 +28,13 @@ Optional properties:
parent's address space
- extcon : external connector for vbus and idpin changes detection, needed
when supports dual-role mode.
+ it's considered valid for compatibility reasons, not allowed for
+ new bindings, and use "usb-role-switch" property instead.
- vbus-supply : reference to the VBUS regulator, needed when supports
dual-role mode.
+ it's considered valid for compatibility reasons, not allowed for
+ new bindings, and put into a usb-connector node.
+ see connector/usb-connector.txt.
- pinctrl-names : a pinctrl state named "default" is optional, and need be
defined if auto drd switch is enabled, that means the property dr_mode
is set as "otg", and meanwhile the property "mediatek,enable-manual-drd"
@@ -39,6 +44,8 @@ Optional properties:

- maximum-speed : valid arguments are "super-speed", "high-speed" and
"full-speed"; refer to usb/generic.txt
+ - usb-role-switch : use USB Role Switch to support dual-role switch, but
+ not extcon; see usb/generic.txt.
- enable-manual-drd : supports manual dual-role switch via debugfs; usually
used when receptacle is TYPE-A and also wants to support dual-role
mode.
@@ -61,6 +68,9 @@ The xhci should be added as subnode to mtu3 as shown in the following example
if host mode is enabled. The DT binding details of xhci can be found in:
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt

+The port would be added as subnode if use "usb-role-switch" property.
+ see graph.txt
+
Example:
ssusb: usb@11271000 {
compatible = "mediatek,mt8173-mtu3";
--
2.21.0

2019-05-29 07:46:55

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 02/10] dt-bindings: connector: add optional properties for Type-B

Add id-gpios, vbus-gpios, vbus-supply and pinctrl properties for
usb-b-connector

Signed-off-by: Chunfeng Yun <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v6 no changes

v5 changes:
1. add reviewed by Rob

v4 no changes

v3 changes:
1. add GPIO direction, and use fixed-regulator for GPIO controlled
VBUS regulator suggested by Rob;

v2 changes:
1. describe more clear for vbus-gpios and vbus-supply suggested by Hans
---
.../bindings/connector/usb-connector.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
index cef556d4e5ee..d357987181ee 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.txt
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -17,6 +17,20 @@ Optional properties:
- self-powered: Set this property if the usb device that has its own power
source.

+Optional properties for usb-b-connector:
+- id-gpios: an input gpio for USB ID pin.
+- vbus-gpios: an input gpio for USB VBUS pin, used to detect presence of
+ VBUS 5V.
+ see gpio/gpio.txt.
+- vbus-supply: a phandle to the regulator for USB VBUS if needed when host
+ mode or dual role mode is supported.
+ Particularly, if use an output GPIO to control a VBUS regulator, should
+ model it as a regulator.
+ see regulator/fixed-regulator.yaml
+- pinctrl-names : a pinctrl state named "default" is optional
+- pinctrl-0 : pin control group
+ see pinctrl/pinctrl-bindings.txt
+
Optional properties for usb-c-connector:
- power-role: should be one of "source", "sink" or "dual"(DRP) if typec
connector has power support.
--
2.21.0

2019-05-29 07:47:06

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 01/10] dt-binding: usb: add usb-role-switch property

Add a property usb-role-switch to tell the driver that use
USB Role Switch framework to handle the role switch,
it's useful when the driver has already supported other ways,
such as extcon framework etc.

Cc: Biju Das <[email protected]>
Cc: Yu Chen <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
---
v6:
1. merge into this series patch
2. add Reviewed-by

(no v4, v5)

v3:
add property type, modify description suggested by Heikki

v2:
describe it in terms of h/w functionality suggested by Rob

v1:
the property is discussed in:
[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property
https://patchwork.kernel.org/patch/10852497/

Mediatek and Hisilicon also try to use it:
[v4,3/6] dt-bindings: usb: mtu3: add properties about USB Role Switch
https://patchwork.kernel.org/patch/10918385/
[v4,6/6] usb: mtu3: register a USB Role Switch for dual role mode
https://patchwork.kernel.org/patch/10918367/

[v6,10/13] usb: dwc3: Registering a role switch in the DRD code
https://patchwork.kernel.org/patch/10909981/
---
Documentation/devicetree/bindings/usb/generic.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
index 0a74ab8dfdc2..cf5a1ad456e6 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -30,6 +30,10 @@ Optional properties:
optional for OTG device.
- adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
optional for OTG device.
+ - usb-role-switch: boolean, indicates that the device is capable of assigning
+ the USB data role (USB host or USB device) for a given
+ USB connector, such as Type-C, Type-B(micro).
+ see connector/usb-connector.txt.

This is an attribute to a USB controller such as:

--
2.21.0

2019-05-29 07:47:18

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 05/10] usb: roles: Introduce stubs for the exiting functions in role.h.

From: Yu Chen <[email protected]>

This patch adds stubs for the exiting functions while
CONFIG_USB_ROLE_SWITCH does not enabled.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: John Stultz <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
v6:
merge this patch [1] into this series to add new API

[1] https://patchwork.kernel.org/patch/10909971/
---
include/linux/usb/role.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index c05ffa6abda9..da2b9641b877 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -42,6 +42,8 @@ struct usb_role_switch_desc {
bool allow_userspace_control;
};

+
+#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
struct usb_role_switch *usb_role_switch_get(struct device *dev);
@@ -51,5 +53,33 @@ struct usb_role_switch *
usb_role_switch_register(struct device *parent,
const struct usb_role_switch_desc *desc);
void usb_role_switch_unregister(struct usb_role_switch *sw);
+#else
+static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
+ enum usb_role role)
+{
+ return 0;
+}
+
+static inline enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
+{
+ return USB_ROLE_NONE;
+}
+
+static inline struct usb_role_switch *usb_role_switch_get(struct device *dev)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
+
+static inline struct usb_role_switch *
+usb_role_switch_register(struct device *parent,
+ const struct usb_role_switch_desc *desc)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
+#endif

#endif /* __LINUX_USB_ROLE_H */
--
2.21.0

2019-05-29 07:47:21

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 07/10] usb: roles: Add fwnode_usb_role_switch_get() function

From: Heikki Krogerus <[email protected]>

The fwnode_usb_role_switch_get() function is exactly the
same as usb_role_switch_get(), except that it takes struct
fwnode_handle as parameter instead of struct device.

Signed-off-by: Heikki Krogerus <[email protected]>
---
v6:
new patch
---
drivers/usb/roles/class.c | 20 ++++++++++++++++++++
include/linux/usb/role.h | 7 +++++++
2 files changed, 27 insertions(+)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index f45d8df5cfb8..aab795b54c7f 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -135,6 +135,26 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
}
EXPORT_SYMBOL_GPL(usb_role_switch_get);

+/**
+ * fwnode_usb_role_switch_get - Find USB role switch linked with the caller
+ * @fwnode: The caller device node
+ *
+ * This is similar to the usb_role_switch_get() function above, but it searches
+ * the switch using fwnode instead of device entry.
+ */
+struct usb_role_switch *fwnode_usb_role_switch_get(struct fwnode_handle *fwnode)
+{
+ struct usb_role_switch *sw;
+
+ sw = fwnode_connection_find_match(fwnode, "usb-role-switch", NULL,
+ usb_role_switch_match);
+ if (!IS_ERR_OR_NULL(sw))
+ WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+ return sw;
+}
+EXPORT_SYMBOL_GPL(fwnode_usb_role_switch_get);
+
/**
* usb_role_switch_put - Release handle to a switch
* @sw: USB Role Switch
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index da2b9641b877..2d77f97df72d 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -47,6 +47,7 @@ struct usb_role_switch_desc {
int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
struct usb_role_switch *usb_role_switch_get(struct device *dev);
+struct usb_role_switch *fwnode_usb_role_switch_get(struct fwnode_handle *node);
void usb_role_switch_put(struct usb_role_switch *sw);

struct usb_role_switch *
@@ -70,6 +71,12 @@ static inline struct usb_role_switch *usb_role_switch_get(struct device *dev)
return ERR_PTR(-ENODEV);
}

+static inline struct usb_role_switch *
+fwnode_usb_role_switch_get(struct fwnode_handle *node)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline void usb_role_switch_put(struct usb_role_switch *sw) { }

static inline struct usb_role_switch *
--
2.21.0

2019-05-29 07:47:32

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 03/10] dt-bindings: usb: add binding for Type-B GPIO connector driver

It's used to support dual role switch via GPIO when use Type-B
receptacle, typically the USB ID pin is connected to an input
GPIO pin

Signed-off-by: Chunfeng Yun <[email protected]>
---
v6 changes:
1. remove status and port nodes in example
2. make vbus-supply as optional property

v5 changes:
1. treat type-B connector as child device of USB controller's, but not
as a separate virtual device, suggested by Rob
2. put connector's port node under connector node, suggested by Rob

v4 no changes

v3 changes:
1. treat type-B connector as a virtual device, but not child device of
USB controller's

v2 changes:
1. new patch to make binding clear suggested by Hans
---
.../bindings/usb/typeb-conn-gpio.txt | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/typeb-conn-gpio.txt

diff --git a/Documentation/devicetree/bindings/usb/typeb-conn-gpio.txt b/Documentation/devicetree/bindings/usb/typeb-conn-gpio.txt
new file mode 100644
index 000000000000..5be443a16c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typeb-conn-gpio.txt
@@ -0,0 +1,30 @@
+USB Type-B GPIO Connector
+
+This is typically used to switch dual role mode from the USB ID pin connected
+to an input GPIO pin.
+
+Required properties:
+- compatible : should include "linux,typeb-conn-gpio" and "usb-b-connector".
+- id-gpios, vbus-gpios : input gpios, either one of them must be present,
+ and both can be present as well.
+ see connector/usb-connector.txt
+
+Optional properties:
+- vbus-supply : can be present if needed when supports dual role mode.
+ see connector/usb-connector.txt
+
+- Sub-nodes:
+ - port : can be present.
+ see graph.txt
+
+Example:
+
+&mtu3 {
+ connector {
+ compatible = "linux,typeb-conn-gpio", "usb-b-connector";
+ label = "micro-USB";
+ type = "micro";
+ id-gpios = <&pio 12 GPIO_ACTIVE_HIGH>;
+ vbus-supply = <&usb_p0_vbus>;
+ };
+};
--
2.21.0

2019-05-29 07:47:57

by Chunfeng Yun

[permalink] [raw]
Subject: [PATCH v6 08/10] usb: roles: get usb-role-switch from parent

when the USB host controller is the parent of the connector,
usually type-B, sometimes don't need the graph, so we should
check whether it's parent registers usb-role-switch or not
firstly, and get it if exists.

Signed-off-by: Heikki Krogerus <[email protected]>
Signed-off-by: Chunfeng Yun <[email protected]>
---
v6:
new patch
---
drivers/usb/roles/class.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index aab795b54c7f..1f6af0ef3954 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -114,6 +114,19 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}

+static struct usb_role_switch *
+usb_role_switch_is_parent(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *parent = fwnode_get_parent(fwnode);
+ struct device *dev;
+
+ if (!parent || !fwnode_property_present(parent, "usb-role-switch"))
+ return NULL;
+
+ dev = class_find_device(role_class, NULL, parent, switch_fwnode_match);
+ return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
+}
+
/**
* usb_role_switch_get - Find USB role switch linked with the caller
* @dev: The caller device
@@ -125,8 +138,10 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
{
struct usb_role_switch *sw;

- sw = device_connection_find_match(dev, "usb-role-switch", NULL,
- usb_role_switch_match);
+ sw = usb_role_switch_is_parent(dev_fwnode(dev));
+ if (!sw)
+ sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);

if (!IS_ERR_OR_NULL(sw))
WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
@@ -146,8 +161,10 @@ struct usb_role_switch *fwnode_usb_role_switch_get(struct fwnode_handle *fwnode)
{
struct usb_role_switch *sw;

- sw = fwnode_connection_find_match(fwnode, "usb-role-switch", NULL,
- usb_role_switch_match);
+ sw = usb_role_switch_is_parent(fwnode);
+ if (!sw)
+ sw = fwnode_connection_find_match(fwnode, "usb-role-switch",
+ NULL, usb_role_switch_match);
if (!IS_ERR_OR_NULL(sw))
WARN_ON(!try_module_get(sw->dev.parent->driver->owner));

--
2.21.0

2019-05-31 07:43:37

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH v6 06/10] device connection: Add fwnode_connection_find_match()

Hi,

Thanks for the patch

> Subject: [PATCH v6 06/10] device connection: Add
> fwnode_connection_find_match()
>
> From: Heikki Krogerus <[email protected]>
>
> The fwnode_connection_find_match() function is exactly the same as
> device_connection_find_match(), except it takes struct fwnode_handle as
> parameter instead of struct device.
> That allows locating device connections before the device entries have been
> created.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
Tested-by: Biju Das <[email protected]>
> ---
> v6:
> new patch
> ---
> drivers/base/devcon.c | 33 +++++++++++++++++++++++++--------
> include/linux/device.h | 10 +++++++---
> 2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c index
> 04db9ae235e4..8311b70bbca2 100644
> --- a/drivers/base/devcon.c
> +++ b/drivers/base/devcon.c
> @@ -12,9 +12,6 @@
> static DEFINE_MUTEX(devcon_lock);
> static LIST_HEAD(devcon_list);
>
> -typedef void *(*devcon_match_fn_t)(struct device_connection *con, int
> ep,
> - void *data);
> -
> static void *
> fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char
> *con_id,
> void *data, devcon_match_fn_t match) @@ -38,6
> +35,28 @@ fwnode_graph_devcon_match(struct fwnode_handle *fwnode,
> const char *con_id,
> return NULL;
> }
>
> +/**
> + * fwnode_connection_find_match - Find connection from a device node
> + * @fwnode: Device node with the connection
> + * @con_id: Identifier for the connection
> + * @data: Data for the match function
> + * @match: Function to check and convert the connection description
> + *
> + * Find a connection with unique identifier @con_id between @fwnode and
> +another
> + * device node. @match will be used to convert the connection
> +description to
> + * data the caller is expecting to be returned.
> + */
> +void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
> + const char *con_id, void *data,
> + devcon_match_fn_t match)
> +{
> + if (!fwnode || !match)
> + return NULL;
> +
> + return fwnode_graph_devcon_match(fwnode, con_id, data, match);
> }
> +EXPORT_SYMBOL_GPL(fwnode_connection_find_match);
> +
> /**
> * device_connection_find_match - Find physical connection to a device
> * @dev: Device with the connection
> @@ -61,11 +80,9 @@ void *device_connection_find_match(struct device
> *dev, const char *con_id,
> if (!match)
> return NULL;
>
> - if (fwnode) {
> - ret = fwnode_graph_devcon_match(fwnode, con_id, data,
> match);
> - if (ret)
> - return ret;
> - }
> + ret = fwnode_connection_find_match(fwnode, con_id, data,
> match);
> + if (ret)
> + return ret;
>
> mutex_lock(&devcon_lock);
>
> diff --git a/include/linux/device.h b/include/linux/device.h index
> e85264fb6616..9445f068602f 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -772,10 +772,14 @@ struct device_connection {
> struct list_head list;
> };
>
> +typedef void *(*devcon_match_fn_t)(struct device_connection *con, int
> ep,
> + void *data);
> +
> +void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
> + const char *con_id, void *data,
> + devcon_match_fn_t match);
> void *device_connection_find_match(struct device *dev, const char
> *con_id,
> - void *data,
> - void *(*match)(struct device_connection
> *con,
> - int ep, void *data));
> + void *data, devcon_match_fn_t match);
>
> struct device *device_connection_find(struct device *dev, const char
> *con_id);
>
> --
> 2.21.0

2019-05-31 07:44:25

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH v6 07/10] usb: roles: Add fwnode_usb_role_switch_get() function

Hi,

Thanks for the patch.

> Subject: [PATCH v6 07/10] usb: roles: Add fwnode_usb_role_switch_get()
> function
>
> From: Heikki Krogerus <[email protected]>
>
> The fwnode_usb_role_switch_get() function is exactly the same as
> usb_role_switch_get(), except that it takes struct fwnode_handle as
> parameter instead of struct device.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
Tested-by: Biju Das <[email protected]>

> ---
> v6:
> new patch
> ---
> drivers/usb/roles/class.c | 20 ++++++++++++++++++++
> include/linux/usb/role.h | 7 +++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c index
> f45d8df5cfb8..aab795b54c7f 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -135,6 +135,26 @@ struct usb_role_switch *usb_role_switch_get(struct
> device *dev) } EXPORT_SYMBOL_GPL(usb_role_switch_get);
>
> +/**
> + * fwnode_usb_role_switch_get - Find USB role switch linked with the
> +caller
> + * @fwnode: The caller device node
> + *
> + * This is similar to the usb_role_switch_get() function above, but it
> +searches
> + * the switch using fwnode instead of device entry.
> + */
> +struct usb_role_switch *fwnode_usb_role_switch_get(struct
> fwnode_handle
> +*fwnode) {
> + struct usb_role_switch *sw;
> +
> + sw = fwnode_connection_find_match(fwnode, "usb-role-switch",
> NULL,
> + usb_role_switch_match);
> + if (!IS_ERR_OR_NULL(sw))
> + WARN_ON(!try_module_get(sw->dev.parent->driver-
> >owner));
> +
> + return sw;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_usb_role_switch_get);
> +
> /**
> * usb_role_switch_put - Release handle to a switch
> * @sw: USB Role Switch
> diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h index
> da2b9641b877..2d77f97df72d 100644
> --- a/include/linux/usb/role.h
> +++ b/include/linux/usb/role.h
> @@ -47,6 +47,7 @@ struct usb_role_switch_desc { int
> usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
> enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
> struct usb_role_switch *usb_role_switch_get(struct device *dev);
> +struct usb_role_switch *fwnode_usb_role_switch_get(struct
> fwnode_handle
> +*node);
> void usb_role_switch_put(struct usb_role_switch *sw);
>
> struct usb_role_switch *
> @@ -70,6 +71,12 @@ static inline struct usb_role_switch
> *usb_role_switch_get(struct device *dev)
> return ERR_PTR(-ENODEV);
> }
>
> +static inline struct usb_role_switch *
> +fwnode_usb_role_switch_get(struct fwnode_handle *node) {
> + return ERR_PTR(-ENODEV);
> +}
> +
> static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
>
> static inline struct usb_role_switch *
> --
> 2.21.0

2019-06-03 16:59:47

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 05/10] usb: roles: Introduce stubs for the exiting functions in role.h.

On Wed, May 29, 2019 at 03:43:43PM +0800, Chunfeng Yun wrote:
> From: Yu Chen <[email protected]>
>
> This patch adds stubs for the exiting functions while
> CONFIG_USB_ROLE_SWITCH does not enabled.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Heikki Krogerus <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: John Stultz <[email protected]>
> Reviewed-by: Heikki Krogerus <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>

Same here, you need to sign off on it too.

thanks,

greg k-h

2019-06-03 17:38:13

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 07/10] usb: roles: Add fwnode_usb_role_switch_get() function

On Wed, May 29, 2019 at 03:43:45PM +0800, Chunfeng Yun wrote:
> From: Heikki Krogerus <[email protected]>
>
> The fwnode_usb_role_switch_get() function is exactly the
> same as usb_role_switch_get(), except that it takes struct
> fwnode_handle as parameter instead of struct device.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> ---
> v6:
> new patch

If you are forwarding on a patch from someone else, like this, you need
to put your signed-off-by line on it as well.

Please fix that up, and add the tested-by one, when you resend this
series.

thanks,

greg k-h

2019-06-04 06:44:35

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH v6 07/10] usb: roles: Add fwnode_usb_role_switch_get() function

On Mon, 2019-06-03 at 15:19 +0200, Greg Kroah-Hartman wrote:
> On Wed, May 29, 2019 at 03:43:45PM +0800, Chunfeng Yun wrote:
> > From: Heikki Krogerus <[email protected]>
> >
> > The fwnode_usb_role_switch_get() function is exactly the
> > same as usb_role_switch_get(), except that it takes struct
> > fwnode_handle as parameter instead of struct device.
> >
> > Signed-off-by: Heikki Krogerus <[email protected]>
> > ---
> > v6:
> > new patch
>
> If you are forwarding on a patch from someone else, like this, you need
> to put your signed-off-by line on it as well.
>
> Please fix that up, and add the tested-by one, when you resend this
> series.
Got it, thanks

>
> thanks,
>
> greg k-h


2019-06-04 06:45:58

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH v6 05/10] usb: roles: Introduce stubs for the exiting functions in role.h.

On Mon, 2019-06-03 at 15:19 +0200, Greg Kroah-Hartman wrote:
> On Wed, May 29, 2019 at 03:43:43PM +0800, Chunfeng Yun wrote:
> > From: Yu Chen <[email protected]>
> >
> > This patch adds stubs for the exiting functions while
> > CONFIG_USB_ROLE_SWITCH does not enabled.
> >
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Heikki Krogerus <[email protected]>
> > Cc: Hans de Goede <[email protected]>
> > Cc: Andy Shevchenko <[email protected]>
> > Cc: John Stultz <[email protected]>
> > Reviewed-by: Heikki Krogerus <[email protected]>
> > Signed-off-by: Yu Chen <[email protected]>
>
> Same here, you need to sign off on it too.
Ok, thanks
>
> thanks,
>
> greg k-h


2019-06-05 08:47:33

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

On Wed, May 29, 2019 at 10:44 AM Chunfeng Yun <[email protected]> wrote:
>
> Due to the requirement of usb-connector.txt binding, the old way
> using extcon to support USB Dual-Role switch is now deprecated
> when use Type-B connector.
> This patch introduces a driver of Type-B connector which typically
> uses an input GPIO to detect USB ID pin, and try to replace the
> function provided by extcon-usb-gpio driver

> +static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
> + usb_conn_suspend, usb_conn_resume);
> +
> +#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)

Why this macro is needed?

--
With Best Regards,
Andy Shevchenko

2019-06-06 02:55:28

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

On Wed, 2019-06-05 at 11:45 +0300, Andy Shevchenko wrote:
> On Wed, May 29, 2019 at 10:44 AM Chunfeng Yun <[email protected]> wrote:
> >
> > Due to the requirement of usb-connector.txt binding, the old way
> > using extcon to support USB Dual-Role switch is now deprecated
> > when use Type-B connector.
> > This patch introduces a driver of Type-B connector which typically
> > uses an input GPIO to detect USB ID pin, and try to replace the
> > function provided by extcon-usb-gpio driver
>
> > +static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
> > + usb_conn_suspend, usb_conn_resume);
> > +
> > +#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)
>
> Why this macro is needed?
Want to set .pm as NULL when CONFIG_PM_SLEEP is not enabled.

Thanks
>


2019-06-06 06:33:24

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

On Thu, Jun 6, 2019 at 5:53 AM Chunfeng Yun <[email protected]> wrote:
>
> On Wed, 2019-06-05 at 11:45 +0300, Andy Shevchenko wrote:
> > On Wed, May 29, 2019 at 10:44 AM Chunfeng Yun <[email protected]> wrote:
> > >
> > > Due to the requirement of usb-connector.txt binding, the old way
> > > using extcon to support USB Dual-Role switch is now deprecated
> > > when use Type-B connector.
> > > This patch introduces a driver of Type-B connector which typically
> > > uses an input GPIO to detect USB ID pin, and try to replace the
> > > function provided by extcon-usb-gpio driver
> >
> > > +static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
> > > + usb_conn_suspend, usb_conn_resume);
> > > +
> > > +#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)
> >
> > Why this macro is needed?
> Want to set .pm as NULL when CONFIG_PM_SLEEP is not enabled.

Doesn't SIMPLE_DEV_PM_OPS do this for you?

--
With Best Regards,
Andy Shevchenko

2019-06-07 12:59:20

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v6 06/10] device connection: Add fwnode_connection_find_match()

Hi,

On Wed, May 29, 2019 at 03:43:44PM +0800, Chunfeng Yun wrote:
> From: Heikki Krogerus <[email protected]>
>
> The fwnode_connection_find_match() function is exactly the
> same as device_connection_find_match(), except it takes
> struct fwnode_handle as parameter instead of struct device.
> That allows locating device connections before the device
> entries have been created.
>
> Signed-off-by: Heikki Krogerus <[email protected]>

This one is also missing your SoB.

There are now some other changes to the devcon API in Rafael's tree
[1] that will conflict with this one. I'm attaching a modified version
of the patch that is rebased on top of today's linux-next. If you use
it, you should make a note (probable in the cover letter) that the
series now depends on Rafael's tree.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=linux-next


thanks,

--
heikki


Attachments:
(No filename) (959.00 B)
0001-device-connection-Add-fwnode_connection_find_match.patch (3.60 kB)
Download all attachments

2019-06-10 01:54:19

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH v6 06/10] device connection: Add fwnode_connection_find_match()

Hi Heikki,

On Fri, 2019-06-07 at 13:30 +0300, Heikki Krogerus wrote:
> Hi,
>
> On Wed, May 29, 2019 at 03:43:44PM +0800, Chunfeng Yun wrote:
> > From: Heikki Krogerus <[email protected]>
> >
> > The fwnode_connection_find_match() function is exactly the
> > same as device_connection_find_match(), except it takes
> > struct fwnode_handle as parameter instead of struct device.
> > That allows locating device connections before the device
> > entries have been created.
> >
> > Signed-off-by: Heikki Krogerus <[email protected]>
>
> This one is also missing your SoB.
>
> There are now some other changes to the devcon API in Rafael's tree
> [1] that will conflict with this one. I'm attaching a modified version
> of the patch that is rebased on top of today's linux-next. If you use
> it, you should make a note (probable in the cover letter) that the
> series now depends on Rafael's tree.
Got it, thanks

>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=linux-next
>
>
> thanks,
>


2019-06-10 10:37:16

by Nagarjuna Kristam

[permalink] [raw]
Subject: Re: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

Tested-by: Nagarjuna Kristam <[email protected]>

On 29-05-2019 13:13, Chunfeng Yun wrote:
> Due to the requirement of usb-connector.txt binding, the old way
> using extcon to support USB Dual-Role switch is now deprecated
> when use Type-B connector.
> This patch introduces a driver of Type-B connector which typically
> uses an input GPIO to detect USB ID pin, and try to replace the
> function provided by extcon-usb-gpio driver
>
> Signed-off-by: Chunfeng Yun <[email protected]>
> ---
> v6 changes:
> 1. get usb-role-swtich by usb_role_switch_get()
>
> v5 changes:
> 1. put usb_role_switch when error happens suggested by Biju
> 2. don't treat bype-B connector as a virtual device suggested by Rob
>
> v4 changes:
> 1. remove linux/gpio.h suggested by Linus
> 2. put node when error happens
>
> v3 changes:
> 1. treat bype-B connector as a virtual device;
> 2. change file name again
>
> v2 changes:
> 1. file name is changed
> 2. use new compatible
> ---
> drivers/usb/roles/Kconfig | 11 ++
> drivers/usb/roles/Makefile | 1 +
> drivers/usb/roles/typeb-conn-gpio.c | 286 ++++++++++++++++++++++++++++
> 3 files changed, 298 insertions(+)
> create mode 100644 drivers/usb/roles/typeb-conn-gpio.c
>
> diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig
> index f8b31aa67526..d1156e18a81a 100644
> --- a/drivers/usb/roles/Kconfig
> +++ b/drivers/usb/roles/Kconfig
> @@ -26,4 +26,15 @@ config USB_ROLES_INTEL_XHCI
> To compile the driver as a module, choose M here: the module will
> be called intel-xhci-usb-role-switch.
>
> +config TYPEB_CONN_GPIO
> + tristate "USB Type-B GPIO Connector"
> + depends on GPIOLIB
> + help
> + The driver supports USB role switch between host and device via GPIO
> + based USB cable detection, used typically if an input GPIO is used
> + to detect USB ID pin.
> +
> + To compile the driver as a module, choose M here: the module will
> + be called typeb-conn-gpio.ko
> +
> endif # USB_ROLE_SWITCH
> diff --git a/drivers/usb/roles/Makefile b/drivers/usb/roles/Makefile
> index 757a7d2797eb..5d5620d9d113 100644
> --- a/drivers/usb/roles/Makefile
> +++ b/drivers/usb/roles/Makefile
> @@ -3,3 +3,4 @@
> obj-$(CONFIG_USB_ROLE_SWITCH) += roles.o
> roles-y := class.o
> obj-$(CONFIG_USB_ROLES_INTEL_XHCI) += intel-xhci-usb-role-switch.o
> +obj-$(CONFIG_TYPEB_CONN_GPIO) += typeb-conn-gpio.o
> diff --git a/drivers/usb/roles/typeb-conn-gpio.c b/drivers/usb/roles/typeb-conn-gpio.c
> new file mode 100644
> index 000000000000..c66f852365ef
> --- /dev/null
> +++ b/drivers/usb/roles/typeb-conn-gpio.c
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * USB Type-B GPIO Connector Driver
> + *
> + * Copyright (C) 2019 MediaTek Inc.
> + *
> + * Author: Chunfeng Yun <[email protected]>
> + *
> + * Some code borrowed from drivers/extcon/extcon-usb-gpio.c
> + */
> +
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/role.h>
> +
> +#define USB_GPIO_DEB_MS 20 /* ms */
> +#define USB_GPIO_DEB_US ((USB_GPIO_DEB_MS) * 1000) /* us */
> +
> +#define USB_CONN_IRQF \
> + (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT)
> +
> +struct usb_conn_info {
> + struct device *dev;
> + struct usb_role_switch *role_sw;
> + enum usb_role last_role;
> + struct regulator *vbus;
> + struct delayed_work dw_det;
> + unsigned long debounce_jiffies;
> +
> + struct gpio_desc *id_gpiod;
> + struct gpio_desc *vbus_gpiod;
> + int id_irq;
> + int vbus_irq;
> +};
> +
> +/**
> + * "DEVICE" = VBUS and "HOST" = !ID, so we have:
> + * Both "DEVICE" and "HOST" can't be set as active at the same time
> + * so if "HOST" is active (i.e. ID is 0) we keep "DEVICE" inactive
> + * even if VBUS is on.
> + *
> + * Role | ID | VBUS
> + * ------------------------------------
> + * [1] DEVICE | H | H
> + * [2] NONE | H | L
> + * [3] HOST | L | H
> + * [4] HOST | L | L
> + *
> + * In case we have only one of these signals:
> + * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1
> + * - ID only - we want to distinguish between [1] and [4], so VBUS = ID
> + */
> +static void usb_conn_detect_cable(struct work_struct *work)
> +{
> + struct usb_conn_info *info;
> + enum usb_role role;
> + int id, vbus, ret;
> +
> + info = container_of(to_delayed_work(work),
> + struct usb_conn_info, dw_det);
> +
> + /* check ID and VBUS */
> + id = info->id_gpiod ?
> + gpiod_get_value_cansleep(info->id_gpiod) : 1;
> + vbus = info->vbus_gpiod ?
> + gpiod_get_value_cansleep(info->vbus_gpiod) : id;
> +
> + if (!id)
> + role = USB_ROLE_HOST;
> + else if (vbus)
> + role = USB_ROLE_DEVICE;
> + else
> + role = USB_ROLE_NONE;
> +
> + dev_dbg(info->dev, "role %d/%d, gpios: id %d, vbus %d\n",
> + info->last_role, role, id, vbus);
> +
> + if (info->last_role == role) {
> + dev_warn(info->dev, "repeated role: %d\n", role);
> + return;
> + }
> +
> + if (info->last_role == USB_ROLE_HOST)
> + regulator_disable(info->vbus);
> +
> + ret = usb_role_switch_set_role(info->role_sw, role);
> + if (ret)
> + dev_err(info->dev, "failed to set role: %d\n", ret);
> +
> + if (role == USB_ROLE_HOST) {
> + ret = regulator_enable(info->vbus);
> + if (ret)
> + dev_err(info->dev, "enable vbus regulator failed\n");
> + }
> +
> + info->last_role = role;
> +
> + dev_dbg(info->dev, "vbus regulator is %s\n",
> + regulator_is_enabled(info->vbus) ? "enabled" : "disabled");
> +}
> +
> +static void usb_conn_queue_dwork(struct usb_conn_info *info,
> + unsigned long delay)
> +{
> + queue_delayed_work(system_power_efficient_wq, &info->dw_det, delay);
> +}
> +
> +static irqreturn_t usb_conn_isr(int irq, void *dev_id)
> +{
> + struct usb_conn_info *info = dev_id;
> +
> + usb_conn_queue_dwork(info, info->debounce_jiffies);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int usb_conn_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct usb_conn_info *info;
> + int ret = 0;
> +
> + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->dev = dev;
> + info->id_gpiod = devm_gpiod_get_optional(dev, "id", GPIOD_IN);
> + if (IS_ERR(info->id_gpiod))
> + return PTR_ERR(info->id_gpiod);
> +
> + info->vbus_gpiod = devm_gpiod_get_optional(dev, "vbus", GPIOD_IN);
> + if (IS_ERR(info->vbus_gpiod))
> + return PTR_ERR(info->vbus_gpiod);
> +
> + if (!info->id_gpiod && !info->vbus_gpiod) {
> + dev_err(dev, "failed to get gpios\n");
> + return -ENODEV;
> + }
> +
> + if (info->id_gpiod)
> + ret = gpiod_set_debounce(info->id_gpiod, USB_GPIO_DEB_US);
> + if (!ret && info->vbus_gpiod)
> + ret = gpiod_set_debounce(info->vbus_gpiod, USB_GPIO_DEB_US);
> + if (ret < 0)
> + info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEB_MS);
> +
> + INIT_DELAYED_WORK(&info->dw_det, usb_conn_detect_cable);
> +
> + info->vbus = devm_regulator_get(dev, "vbus");
> + if (IS_ERR(info->vbus)) {
> + dev_err(dev, "failed to get vbus\n");
> + return PTR_ERR(info->vbus);
> + }
> +
> + info->role_sw = usb_role_switch_get(dev);
> + if (IS_ERR(info->role_sw)) {
> + if (PTR_ERR(info->role_sw) != -EPROBE_DEFER)
> + dev_err(dev, "failed to get role switch\n");
> +
> + return PTR_ERR(info->role_sw);
> + }
> +
> + if (info->id_gpiod) {
> + info->id_irq = gpiod_to_irq(info->id_gpiod);
> + if (info->id_irq < 0) {
> + dev_err(dev, "failed to get ID IRQ\n");
> + ret = info->id_irq;
> + goto put_role_sw;
> + }
> +
> + ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
> + usb_conn_isr, USB_CONN_IRQF,
> + pdev->name, info);
> + if (ret < 0) {
> + dev_err(dev, "failed to request ID IRQ\n");
> + goto put_role_sw;
> + }
> + }
> +
> + if (info->vbus_gpiod) {
> + info->vbus_irq = gpiod_to_irq(info->vbus_gpiod);
> + if (info->vbus_irq < 0) {
> + dev_err(dev, "failed to get VBUS IRQ\n");
> + ret = info->vbus_irq;
> + goto put_role_sw;
> + }
> +
> + ret = devm_request_threaded_irq(dev, info->vbus_irq, NULL,
> + usb_conn_isr, USB_CONN_IRQF,
> + pdev->name, info);
> + if (ret < 0) {
> + dev_err(dev, "failed to request VBUS IRQ\n");
> + goto put_role_sw;
> + }
> + }
> +
> + platform_set_drvdata(pdev, info);
> +
> + /* Perform initial detection */
> + usb_conn_queue_dwork(info, 0);
> +
> + return 0;
> +
> +put_role_sw:
> + usb_role_switch_put(info->role_sw);
> + return ret;
> +}
> +
> +static int usb_conn_remove(struct platform_device *pdev)
> +{
> + struct usb_conn_info *info = platform_get_drvdata(pdev);
> +
> + cancel_delayed_work_sync(&info->dw_det);
> +
> + if (info->last_role == USB_ROLE_HOST)
> + regulator_disable(info->vbus);
> +
> + usb_role_switch_put(info->role_sw);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused usb_conn_suspend(struct device *dev)
> +{
> + struct usb_conn_info *info = dev_get_drvdata(dev);
> +
> + if (info->id_gpiod)
> + disable_irq(info->id_irq);
> + if (info->vbus_gpiod)
> + disable_irq(info->vbus_irq);
> +
> + pinctrl_pm_select_sleep_state(dev);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused usb_conn_resume(struct device *dev)
> +{
> + struct usb_conn_info *info = dev_get_drvdata(dev);
> +
> + pinctrl_pm_select_default_state(dev);
> +
> + if (info->id_gpiod)
> + enable_irq(info->id_irq);
> + if (info->vbus_gpiod)
> + enable_irq(info->vbus_irq);
> +
> + usb_conn_queue_dwork(info, 0);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
> + usb_conn_suspend, usb_conn_resume);
> +
> +#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)
> +
> +static const struct of_device_id usb_conn_dt_match[] = {
> + { .compatible = "linux,typeb-conn-gpio", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, usb_conn_dt_match);
> +
> +static struct platform_driver usb_conn_driver = {
> + .probe = usb_conn_probe,
> + .remove = usb_conn_remove,
> + .driver = {
> + .name = "typeb-conn-gpio",
> + .pm = DEV_PMS_OPS,
> + .of_match_table = usb_conn_dt_match,
> + },
> +};
> +
> +module_platform_driver(usb_conn_driver);
> +
> +MODULE_AUTHOR("Chunfeng Yun <[email protected]>");
> +MODULE_DESCRIPTION("USB Type-B GPIO connector driver");
> +MODULE_LICENSE("GPL v2");
>

2019-06-11 05:51:14

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

On Thu, 2019-06-06 at 09:31 +0300, Andy Shevchenko wrote:
> On Thu, Jun 6, 2019 at 5:53 AM Chunfeng Yun <[email protected]> wrote:
> >
> > On Wed, 2019-06-05 at 11:45 +0300, Andy Shevchenko wrote:
> > > On Wed, May 29, 2019 at 10:44 AM Chunfeng Yun <[email protected]> wrote:
> > > >
> > > > Due to the requirement of usb-connector.txt binding, the old way
> > > > using extcon to support USB Dual-Role switch is now deprecated
> > > > when use Type-B connector.
> > > > This patch introduces a driver of Type-B connector which typically
> > > > uses an input GPIO to detect USB ID pin, and try to replace the
> > > > function provided by extcon-usb-gpio driver
> > >
> > > > +static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
> > > > + usb_conn_suspend, usb_conn_resume);
> > > > +
> > > > +#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)
> > >
> > > Why this macro is needed?
> > Want to set .pm as NULL when CONFIG_PM_SLEEP is not enabled.
>
> Doesn't SIMPLE_DEV_PM_OPS do this for you?
Yes, you are right, it provides an empty dev_pm_ops struct, I'll remove
DEV_PMS_OPS, thanks a lot

>


2019-06-11 06:00:01

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH v6 09/10] usb: roles: add USB Type-B GPIO connector driver

On Mon, 2019-06-10 at 16:03 +0530, Nagarjuna Kristam wrote:
> Tested-by: Nagarjuna Kristam <[email protected]>
>
Thanks

> On 29-05-2019 13:13, Chunfeng Yun wrote:
> > Due to the requirement of usb-connector.txt binding, the old way
> > using extcon to support USB Dual-Role switch is now deprecated
> > when use Type-B connector.
> > This patch introduces a driver of Type-B connector which typically
> > uses an input GPIO to detect USB ID pin, and try to replace the
> > function provided by extcon-usb-gpio driver
> >
> > Signed-off-by: Chunfeng Yun <[email protected]>
> > ---
> > v6 changes:
> > 1. get usb-role-swtich by usb_role_switch_get()
> >
> > v5 changes:
> > 1. put usb_role_switch when error happens suggested by Biju
> > 2. don't treat bype-B connector as a virtual device suggested by Rob
> >
> > v4 changes:
> > 1. remove linux/gpio.h suggested by Linus
> > 2. put node when error happens
> >
> > v3 changes:
> > 1. treat bype-B connector as a virtual device;
> > 2. change file name again
> >
> > v2 changes:
> > 1. file name is changed
> > 2. use new compatible
> > ---
> > drivers/usb/roles/Kconfig | 11 ++
> > drivers/usb/roles/Makefile | 1 +
> > drivers/usb/roles/typeb-conn-gpio.c | 286 ++++++++++++++++++++++++++++
> > 3 files changed, 298 insertions(+)
> > create mode 100644 drivers/usb/roles/typeb-conn-gpio.c
> >
> > diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig
> > index f8b31aa67526..d1156e18a81a 100644
> > --- a/drivers/usb/roles/Kconfig
> > +++ b/drivers/usb/roles/Kconfig
> >