[cip-dev] [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C Backport

[cip-dev] [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C Backport

[Marian-Cristian Rotariu]

This patchset revolves around the following patch set from upstream:
https://patchwork.kernel.org/cover/10969899/

Unfortunately, the driver is using a slightly different version of the USB
API. In upstream, the connection between the fwnode of the USB controller
device tree node, that is the connector device tree node, and the USB
peripheral device tree node is nicely done via some new function calls that
are basically some extensions to the graph traverse and graph discovery
methods for the device tree parser. This is needed for role switch feature.

I tried to create the minimum set for the USB API extension and it should
not affect at all the current behavior. All the modifications are additions
and some rewritings of some of the current functions.

I also backported some fixes where I have found them.

Tested on Si-Linux EK874 (with RZ/G2E SoC) that we have in house.

Biju Das (8):
  dt-bindings: usb: hd3ss3220 device tree binding document
  dt-bindings: usb: renesas_usb3: Document usb role switch support
  usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
  usb: typec: hd3ss3220: hd3ss3220_probe() warn: passing zero to
    'PTR_ERR'
  usb: gadget: udc: renesas_usb3: Enhance role switch support
  arm64: defconfig: enable TYPEC_HD3SS3220 config option
  arm64: dts: renesas: cat874: Enable USB3.0 host/peripheral device node
  arm64: dts: renesas: cat874: Enable usb role switch support

Dan Carpenter (1):
  usb: typec: fix an IS_ERR() vs NULL bug in hd3ss3220_probe()

Heikki Krogerus (10):
  device connection: Add fwnode member to struct device_connection
  usb: typec: mux: Find the muxes by also matching against the device
    node
  usb: roles: Find the muxes by also matching against the device node
  usb: typec: Find the ports by also matching against the device node
  device connection: Prepare support for firmware described connections
  device connection: Find device connections also from device graphs
  device property: Introduce fwnode_find_reference()
  device connection: Find connections also by checking the references
  device connection: Add fwnode_connection_find_match()
  usb: roles: Add fwnode_usb_role_switch_get() function

Mao Wenan (1):
  usb: typec: add dependency for TYPEC_HD3SS3220

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

YueHaibing (1):
  usb: typec: mux: Fix unsigned comparison with less than zero

kbuild test robot (1):
  usb: typec: hd3ss3220_irq() can be static

 .../devicetree/bindings/usb/renesas,usb3-peri.txt  |  23 ++
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       |  38 +++
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts    |  49 ++++
 arch/arm64/configs/defconfig                       |   2 +
 drivers/base/devcon.c                              | 107 ++++++++-
 drivers/base/property.c                            |  24 ++
 drivers/usb/gadget/udc/renesas_usb3.c              |  91 ++++++-
 drivers/usb/roles/class.c                          |  41 +++-
 drivers/usb/typec/Kconfig                          |  11 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/class.c                          |  24 +-
 drivers/usb/typec/hd3ss3220.c                      | 262 +++++++++++++++++++++
 drivers/usb/typec/mux.c                            |  86 ++++++-
 include/linux/device.h                             |  16 +-
 include/linux/property.h                           |   4 +
 include/linux/usb/role.h                           |  38 +++
 16 files changed, 786 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
 create mode 100644 drivers/usb/typec/hd3ss3220.c

-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 01/23] device connection: Add fwnode member to struct device_connection

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 09aa11cfda9d8186046bcd1adcd6498b688114f4 upstream.

This will prepare the device connection API for connections
described in firmware.

Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 include/linux/device.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 19dd885..ceb69b9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -753,11 +753,17 @@ struct device_dma_parameters {
 
 /**
  * struct device_connection - Device Connection Descriptor
+ * @fwnode: The device node of the connected device
  * @endpoint: The names of the two devices connected together
  * @id: Unique identifier for the connection
  * @list: List head, private, for internal use only
+ *
+ * NOTE: @fwnode is not used together with @endpoint. @fwnode is used when
+ * platform firmware defines the connection. When the connection is registered
+ * with device_connection_add() @endpoint is used instead.
  */
 struct device_connection {
+	struct fwnode_handle	*fwnode;
 	const char		*endpoint[2];
 	const char		*id;
 	struct list_head	list;
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 02/23] usb: typec: mux: Find the muxes by also matching against the device node

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 96a6d031ca9930938bd66d0052fc7ed2b56e3583 upstream.

When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.

Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/mux.c | 86 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 74 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index d990aa5..0c4e0b5 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -11,6 +11,8 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/slab.h>
 #include <linux/usb/typec_mux.h>
 
 static DEFINE_MUTEX(switch_lock);
@@ -23,15 +25,25 @@ static void *typec_switch_match(struct device_connection *con, int ep,
 {
 	struct typec_switch *sw;
 
-	list_for_each_entry(sw, &switch_list, entry)
-		if (!strcmp(con->endpoint[ep], dev_name(sw->dev)))
-			return sw;
+	if (!con->fwnode) {
+		list_for_each_entry(sw, &switch_list, entry)
+			if (!strcmp(con->endpoint[ep], dev_name(sw->dev)))
+				return sw;
+		return ERR_PTR(-EPROBE_DEFER);
+	}
 
 	/*
-	 * We only get called if a connection was found, tell the caller to
-	 * wait for the switch to show up.
+	 * With OF graph the mux node must have a boolean device property named
+	 * "orientation-switch".
 	 */
-	return ERR_PTR(-EPROBE_DEFER);
+	if (con->id && !fwnode_property_present(con->fwnode, con->id))
+		return NULL;
+
+	list_for_each_entry(sw, &switch_list, entry)
+		if (dev_fwnode(sw->dev) == con->fwnode)
+			return sw;
+
+	return con->id ? ERR_PTR(-EPROBE_DEFER) : NULL;
 }
 
 /**
@@ -112,17 +124,67 @@ EXPORT_SYMBOL_GPL(typec_switch_unregister);
 
 static void *typec_mux_match(struct device_connection *con, int ep, void *data)
 {
+	const struct typec_altmode_desc *desc = data;
 	struct typec_mux *mux;
+	size_t nval;
+	bool match;
+	u16 *val;
+	int i;
 
-	list_for_each_entry(mux, &mux_list, entry)
-		if (!strcmp(con->endpoint[ep], dev_name(mux->dev)))
-			return mux;
+	if (!con->fwnode) {
+		list_for_each_entry(mux, &mux_list, entry)
+			if (!strcmp(con->endpoint[ep], dev_name(mux->dev)))
+				return mux;
+		return ERR_PTR(-EPROBE_DEFER);
+	}
 
 	/*
-	 * We only get called if a connection was found, tell the caller to
-	 * wait for the switch to show up.
+	 * Check has the identifier already been "consumed". If it
+	 * has, no need to do any extra connection identification.
 	 */
-	return ERR_PTR(-EPROBE_DEFER);
+	match = !con->id;
+	if (match)
+		goto find_mux;
+
+	/* Accessory Mode muxes */
+	if (!desc) {
+		match = fwnode_property_present(con->fwnode, "accessory");
+		if (match)
+			goto find_mux;
+		return NULL;
+	}
+
+	/* Alternate Mode muxes */
+	nval = fwnode_property_read_u16_array(con->fwnode, "svid", NULL, 0);
+	if (nval <= 0)
+		return NULL;
+
+	val = kcalloc(nval, sizeof(*val), GFP_KERNEL);
+	if (!val)
+		return ERR_PTR(-ENOMEM);
+
+	nval = fwnode_property_read_u16_array(con->fwnode, "svid", val, nval);
+	if (nval < 0) {
+		kfree(val);
+		return ERR_PTR(nval);
+	}
+
+	for (i = 0; i < nval; i++) {
+		match = val[i] == desc->svid;
+		if (match) {
+			kfree(val);
+			goto find_mux;
+		}
+	}
+	kfree(val);
+	return NULL;
+
+find_mux:
+	list_for_each_entry(mux, &mux_list, entry)
+		if (dev_fwnode(mux->dev) == con->fwnode)
+			return mux;
+
+	return match ? ERR_PTR(-EPROBE_DEFER) : NULL;
 }
 
 /**
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 03/23] usb: typec: mux: Fix unsigned comparison with less than zero

[Marian-Cristian Rotariu]

From: YueHaibing <yuehaibing@huawei.com>

commit 4e46f271c37415323f2b650d02ef50c87ff8d092 upstream.

The return from the call to fwnode_property_read_u16_array is int,
it can be a negative error code however this is being assigned to
an size_t variable 'nval', hence the check is always false.
Fix this by making 'nval' an int.

Detected by Coccinelle ("Unsigned expression compared with
zero: nval < 0")

Fixes: 96a6d031ca99 ("usb: typec: mux: Find the muxes by also matching against the device node")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 0c4e0b5..e49c138 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -126,7 +126,7 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
 {
 	const struct typec_altmode_desc *desc = data;
 	struct typec_mux *mux;
-	size_t nval;
+	int nval;
 	bool match;
 	u16 *val;
 	int i;
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 04/23] usb: roles: Find the muxes by also matching against the device node

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit ec69e9533c4879c81eb7122771792864eb49af35 upstream.

When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.

Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Tested-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/roles/class.c | 21 ++++++++++++++++++---
 include/linux/usb/role.h  |  2 ++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 1dd492e..44a4af2 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/usb/role.h>
+#include <linux/property.h>
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
 
-static int __switch_match(struct device *dev, const void *name)
+static int switch_fwnode_match(struct device *dev, const void *fwnode)
+{
+	return dev_fwnode(dev) == fwnode;
+}
+
+static int switch_name_match(struct device *dev, const void *name)
 {
 	return !strcmp((const char *)name, dev_name(dev));
 }
@@ -94,8 +100,16 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
 {
 	struct device *dev;
 
-	dev = class_find_device(role_class, NULL, con->endpoint[ep],
-				__switch_match);
+	if (con->fwnode) {
+		if (!fwnode_property_present(con->fwnode, con->id))
+			return NULL;
+
+		dev = class_find_device(role_class, NULL, con->fwnode,
+					switch_fwnode_match);
+	} else {
+		dev = class_find_device(role_class, NULL, con->endpoint[ep],
+					switch_name_match);
+	}
 
 	return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
 }
@@ -266,6 +280,7 @@ usb_role_switch_register(struct device *parent,
 	sw->get = desc->get;
 
 	sw->dev.parent = parent;
+	sw->dev.fwnode = desc->fwnode;
 	sw->dev.class = role_class;
 	sw->dev.type = &usb_role_dev_type;
 	dev_set_name(&sw->dev, "%s-role-switch", dev_name(parent));
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index edc51be..c05ffa6 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -18,6 +18,7 @@ typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
 
 /**
  * struct usb_role_switch_desc - USB Role Switch Descriptor
+ * @fwnode: The device node to be associated with the role switch
  * @usb2_port: Optional reference to the host controller port device (USB2)
  * @usb3_port: Optional reference to the host controller port device (USB3)
  * @udc: Optional reference to the peripheral controller device
@@ -32,6 +33,7 @@ typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
  * usb_role_switch_register() before registering the switch.
  */
 struct usb_role_switch_desc {
+	struct fwnode_handle *fwnode;
 	struct device *usb2_port;
 	struct device *usb3_port;
 	struct device *udc;
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 05/23] usb: typec: Find the ports by also matching against the device node

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 6a0bbcf96b2273f110a14d11a5952527c5921191 upstream.

When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.

Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/class.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 1916ee1..65e95b6 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include "bus.h"
@@ -204,15 +205,32 @@ static void typec_altmode_put_partner(struct altmode *altmode)
 	put_device(&adev->dev);
 }
 
-static int __typec_port_match(struct device *dev, const void *name)
+static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
+{
+	return dev_fwnode(dev) == fwnode;
+}
+
+static int typec_port_name_match(struct device *dev, const void *name)
 {
 	return !strcmp((const char *)name, dev_name(dev));
 }
 
 static void *typec_port_match(struct device_connection *con, int ep, void *data)
 {
-	return class_find_device(typec_class, NULL, con->endpoint[ep],
-				 __typec_port_match);
+	struct device *dev;
+
+	/*
+	 * FIXME: Check does the fwnode supports the requested SVID. If it does
+	 * we need to return ERR_PTR(-PROBE_DEFER) when there is no device.
+	 */
+	if (con->fwnode)
+		return class_find_device(typec_class, NULL, con->fwnode,
+					 typec_port_fwnode_match);
+
+	dev = class_find_device(typec_class, NULL, con->endpoint[ep],
+				typec_port_name_match);
+
+	return dev ? dev : ERR_PTR(-EPROBE_DEFER);
 }
 
 struct typec_altmode *
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 06/23] device connection: Prepare support for firmware described connections

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 80e04837a40f6f1c8e93437be3f772c967a41d2f upstream.

When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device. The endpoint member for the device names will not be
used at all in that case.

Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Tested-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/base/devcon.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index d427e80..858b8d2 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -75,12 +75,36 @@ static struct bus_type *generic_match_buses[] = {
 	NULL,
 };
 
+static int device_fwnode_match(struct device *dev, void *fwnode)
+{
+	return dev_fwnode(dev) == fwnode;
+}
+
+static void *device_connection_fwnode_match(struct device_connection *con)
+{
+	struct bus_type *bus;
+	struct device *dev;
+
+	for (bus = generic_match_buses[0]; bus; bus++) {
+		dev = bus_find_device(bus, NULL, (void *)con->fwnode,
+				      device_fwnode_match);
+		if (dev && !strncmp(dev_name(dev), con->id, strlen(con->id)))
+			return dev;
+
+		put_device(dev);
+	}
+	return NULL;
+}
+
 /* This tries to find the device from the most common bus types by name. */
 static void *generic_match(struct device_connection *con, int ep, void *data)
 {
 	struct bus_type *bus;
 	struct device *dev;
 
+	if (con->fwnode)
+		return device_connection_fwnode_match(con);
+
 	for (bus = generic_match_buses[0]; bus; bus++) {
 		dev = bus_find_device_by_name(bus, NULL, con->endpoint[ep]);
 		if (dev)
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 07/23] device connection: Find device connections also from device graphs

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 637e9e52b185e5b10372366bd6db97f27cf3f202 upstream.

If connections between devices are described in OF graph or
ACPI device graph, we can find them by using the
fwnode_graph_*() functions.

Acked-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jun Li <jun.li@nxp.com>
Tested-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/base/devcon.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 858b8d2..04db9ae 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -7,10 +7,37 @@
  */
 
 #include <linux/device.h>
+#include <linux/property.h>
 
 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)
+{
+	struct device_connection con = { .id = con_id };
+	struct fwnode_handle *ep;
+	void *ret;
+
+	fwnode_graph_for_each_endpoint(fwnode, ep) {
+		con.fwnode = fwnode_graph_get_remote_port_parent(ep);
+		if (!fwnode_device_is_available(con.fwnode))
+			continue;
+
+		ret = match(&con, -1, data);
+		fwnode_handle_put(con.fwnode);
+		if (ret) {
+			fwnode_handle_put(ep);
+			return ret;
+		}
+	}
+	return NULL;
+}
+
 /**
  * device_connection_find_match - Find physical connection to a device
  * @dev: Device with the connection
@@ -23,10 +50,9 @@ static LIST_HEAD(devcon_list);
  * caller is expecting to be returned.
  */
 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 fwnode_handle *fwnode = dev_fwnode(dev);
 	const char *devname = dev_name(dev);
 	struct device_connection *con;
 	void *ret = NULL;
@@ -35,6 +61,12 @@ 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;
+	}
+
 	mutex_lock(&devcon_lock);
 
 	list_for_each_entry(con, &devcon_list, list) {
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 08/23] device property: Introduce fwnode_find_reference()

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 83b34afb6b79c69f5478a7249451cab858af97d6 upstream.

In most cases the references that the drivers look for don't
have any arguments. This introduces a wrapper function for
fwnode_property_get_reference_args() that looks for
references by using only the name and index.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/base/property.c  | 24 ++++++++++++++++++++++++
 include/linux/property.h |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 240ab52..52be8c8 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -946,6 +946,30 @@ static struct property_set *pset_copy_set(const struct property_set *pset)
 }
 
 /**
+ * fwnode_find_reference - Find named reference to a fwnode_handle
+ * @fwnode: Firmware node where to look for the reference
+ * @name: The name of the reference
+ * @index: Index of the reference
+ *
+ * @index can be used when the named reference holds a table of references.
+ *
+ * Returns pointer to the reference fwnode, or ERR_PTR. Caller is responsible to
+ * call fwnode_handle_put() on the returned fwnode pointer.
+ */
+struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
+					    const char *name,
+					    unsigned int index)
+{
+	struct fwnode_reference_args args;
+	int ret;
+
+	ret = fwnode_property_get_reference_args(fwnode, name, NULL, 0, index,
+						 &args);
+	return ret ? ERR_PTR(ret) : args.fwnode;
+}
+EXPORT_SYMBOL_GPL(fwnode_find_reference);
+
+/**
  * device_remove_properties - Remove properties from a device object.
  * @dev: Device whose properties to remove.
  *
diff --git a/include/linux/property.h b/include/linux/property.h
index 1a12364..216dcfe 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -78,6 +78,10 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
 				       unsigned int nargs, unsigned int index,
 				       struct fwnode_reference_args *args);
 
+struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
+					    const char *name,
+					    unsigned int index);
+
 struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_next_parent(
 	struct fwnode_handle *fwnode);
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 09/23] device connection: Find connections also by checking the references

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit fde777791eb83f6c5845b81af89de6be60a2b2ce upstream.

We can also use this API to find named references that the
device nodes have by using fwnode_property_get_reference_args()
function.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/base/devcon.c     | 26 ++++++++++++++++++++++++++
 drivers/usb/roles/class.c |  2 +-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 04db9ae..f7035fc 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -38,6 +38,28 @@ fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
 	return NULL;
 }
 
+static void *
+fwnode_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
+		    void *data, devcon_match_fn_t match)
+{
+	struct device_connection con = { };
+	void *ret;
+	int i;
+
+	for (i = 0; ; i++) {
+		con.fwnode = fwnode_find_reference(fwnode, con_id, i);
+		if (IS_ERR(con.fwnode))
+			break;
+
+		ret = match(&con, -1, data);
+		fwnode_handle_put(con.fwnode);
+		if (ret)
+			return ret;
+	}
+
+	return NULL;
+}
+
 /**
  * device_connection_find_match - Find physical connection to a device
  * @dev: Device with the connection
@@ -65,6 +87,10 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
 		ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
 		if (ret)
 			return ret;
+
+		ret = fwnode_devcon_match(fwnode, con_id, data, match);
+		if (ret)
+			return ret;
 	}
 
 	mutex_lock(&devcon_lock);
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 44a4af2..6b3c087 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -101,7 +101,7 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
 	struct device *dev;
 
 	if (con->fwnode) {
-		if (!fwnode_property_present(con->fwnode, con->id))
+		if (con->id && !fwnode_property_present(con->fwnode, con->id))
 			return NULL;
 
 		dev = class_find_device(role_class, NULL, con->fwnode,
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 10/23] usb: roles: Introduce stubs for the exiting functions in role.h

[Marian-Cristian Rotariu]

From: Yu Chen <chenyu56@huawei.com>

commit 977607653cdd06a2c4d1d0f179ff46cc0cbe83a5 upstream.

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

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Link: https://lore.kernel.org/r/1567070558-29417-6-git-send-email-chunfeng.yun at mediatek.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(fixed checkpatch alignment warnings)
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 include/linux/usb/role.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index c05ffa6..b880e6a 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -42,6 +42,7 @@ 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 +52,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.7.4

[cip-dev] [PATCH 4.19.y-cip 11/23] device connection: Add fwnode_connection_find_match()

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit 44493062abc38e5895e0cf8d22698f2ca39a1e4d upstream.

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 <heikki.krogerus@linux.intel.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Link: https://lore.kernel.org/r/1567070558-29417-7-git-send-email-chunfeng.yun at mediatek.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/base/devcon.c  | 43 +++++++++++++++++++++++++++++++------------
 include/linux/device.h | 10 +++++++---
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index f7035fc..5bf9537bd7 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)
@@ -61,6 +58,34 @@ fwnode_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
 }
 
 /**
+ * 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)
+{
+	void *ret;
+
+	if (!fwnode || !match)
+		return NULL;
+
+	ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
+	if (ret)
+		return ret;
+
+	return fwnode_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
  * @con_id: Identifier for the connection
@@ -83,15 +108,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_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 ceb69b9..f4c89e7 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -769,10 +769,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.7.4

[cip-dev] [PATCH 4.19.y-cip 12/23] usb: roles: Add fwnode_usb_role_switch_get() function

[Marian-Cristian Rotariu]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

commit a31f01777bc54a0a9d57628956d05f8d454d3418 upstream.

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 <heikki.krogerus@linux.intel.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Tested-by: Biju Das <biju.das@bp.renesas.com>
Link: https://lore.kernel.org/r/1567070558-29417-8-git-send-email-chunfeng.yun at mediatek.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 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 6b3c087..a4e498e 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -136,6 +136,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 b880e6a..8029d1b 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -46,6 +46,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 *
@@ -69,6 +70,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.7.4

[cip-dev] [PATCH 4.19.y-cip 13/23] dt-bindings: usb: hd3ss3220 device tree binding document

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit 1918fa0d44985cbc8ab2f315c120525b207b2195 upstream.

Add device tree binding document for TI HD3SS3220 Type-C DRP port
controller driver.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/1567584941-13690-2-git-send-email-biju.das at bp.renesas.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt

diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
new file mode 100644
index 0000000..25780e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,38 @@
+TI HD3SS3220 TypeC DRP Port Controller.
+
+Required properties:
+ - compatible: Must be "ti,hd3ss3220".
+ - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
+ - interrupts: An interrupt specifier.
+
+Required sub-node:
+ - connector: The "usb-c-connector" attached to the hd3ss3220 chip. The
+   bindings of the connector node are specified in:
+
+	Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+hd3ss3220 at 47 {
+	compatible = "ti,hd3ss3220";
+	reg = <0x47>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+	connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+		data-role = "dual";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 1 {
+				reg = <1>;
+				hd3ss3220_ep: endpoint {
+					remote-endpoint = <&usb3_role_switch>;
+				};
+			};
+		};
+	};
+};
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 14/23] dt-bindings: usb: renesas_usb3: Document usb role switch support

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit 4aec2393f51418e63dd1e9090063aeda5a6b03d7 upstream.

Update the DT bindings documentation to support usb role switch
for USB Type-C connector using USB role switch class framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/1567584941-13690-4-git-send-email-biju.das at bp.renesas.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 .../devicetree/bindings/usb/renesas,usb3-peri.txt  | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
index 51a7b37d..9e463b0 100644
--- a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
+++ b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
@@ -22,6 +22,12 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: support role switch. see usb/generic.txt
+
+Sub-nodes:
+- any connector to the data bus of this controller should be modelled using the
+  OF graph bindings specified in bindings/graph.txt, if the "usb-role-switch"
+  property is used.
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb at ee020000 {
@@ -39,3 +45,20 @@ Example of R-Car H3 ES1.x:
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cpg CPG_MOD 327>;
 	};
+
+Example of RZ/G2E:
+	usb3_peri0: usb at ee020000 {
+		compatible = "renesas,r8a774c0-usb3-peri",
+			     "renesas,rcar-gen3-usb3-peri";
+		reg = <0 0xee020000 0 0x400>;
+		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 328>;
+		companion = <&xhci0>;
+		usb-role-switch;
+
+		port {
+			usb3_role_switch: endpoint {
+				remote-endpoint = <&hd3ss3220_ep>;
+			};
+		};
+	};
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit 1c48c759ef4bb9031b3347277f04484e07e27d97 upstream.

Driver for TI HD3SS3220 USB Type-C DRP port controller.

The driver currently registers the port and supports data role swapping.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://lore.kernel.org/r/1567584941-13690-3-git-send-email-biju.das at bp.renesas.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(fixed checkpatch alignment warnings)
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/Kconfig     |  10 ++
 drivers/usb/typec/Makefile    |   1 +
 drivers/usb/typec/hd3ss3220.c | 259 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 270 insertions(+)
 create mode 100644 drivers/usb/typec/hd3ss3220.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 00878c3..fde51c5 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -92,6 +92,16 @@ endif # TYPEC_TCPM
 
 source "drivers/usb/typec/ucsi/Kconfig"
 
+config TYPEC_HD3SS3220
+	tristate "TI HD3SS3220 Type-C DRP Port controller driver"
+	depends on I2C
+	help
+	  Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
+	  controller driver.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called hd3ss3220.ko.
+
 config TYPEC_TPS6598X
 	tristate "TI TPS6598x USB Power Delivery controller driver"
 	depends on I2C
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 45b0aef..ff58cea 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TYPEC_TCPM)	+= tcpm.o
 obj-y				+= fusb302/
 obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
+obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tps6598x.o
 obj-$(CONFIG_TYPEC)		+= mux/
 obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
new file mode 100644
index 0000000..0ece5ed
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI HD3SS3220 Type-C DRP Port Controller Driver
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/usb/role.h>
+#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/usb/typec.h>
+#include <linux/delay.h>
+
+#define HD3SS3220_REG_CN_STAT_CTRL	0x09
+#define HD3SS3220_REG_GEN_CTRL		0x0A
+#define HD3SS3220_REG_DEV_REV		0xA0
+
+/* Register HD3SS3220_REG_CN_STAT_CTRL*/
+#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK	(BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP		BIT(6)
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP		BIT(7)
+#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY		(BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS		BIT(4)
+
+/* Register HD3SS3220_REG_GEN_CTRL*/
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK		(BIT(2) | BIT(1))
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT	0x00
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK	BIT(1)
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC	(BIT(2) | BIT(1))
+
+struct hd3ss3220 {
+	struct device *dev;
+	struct regmap *regmap;
+	struct usb_role_switch	*role_sw;
+	struct typec_port *port;
+	struct typec_capability typec_cap;
+};
+
+static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
+{
+	return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
+				  HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
+				  src_pref);
+}
+
+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
+{
+	unsigned int reg_val;
+	enum usb_role attached_state;
+	int ret;
+
+	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+			  &reg_val);
+	if (ret < 0)
+		return ret;
+
+	switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+		attached_state = USB_ROLE_HOST;
+		break;
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+		attached_state = USB_ROLE_DEVICE;
+		break;
+	default:
+		attached_state = USB_ROLE_NONE;
+		break;
+	}
+
+	return attached_state;
+}
+
+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+			    enum typec_data_role role)
+{
+	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
+						   typec_cap);
+	enum usb_role role_val;
+	int pref, ret = 0;
+
+	if (role == TYPEC_HOST) {
+		role_val = USB_ROLE_HOST;
+		pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
+	} else {
+		role_val = USB_ROLE_DEVICE;
+		pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
+	}
+
+	ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+	usleep_range(10, 100);
+
+	usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
+	typec_set_data_role(hd3ss3220->port, role);
+
+	return ret;
+}
+
+static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
+{
+	enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
+
+	usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
+	if (role_state == USB_ROLE_NONE)
+		hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+
+	switch (role_state) {
+	case USB_ROLE_HOST:
+		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+		break;
+	case USB_ROLE_DEVICE:
+		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+		break;
+	default:
+		break;
+	}
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+	int err;
+
+	hd3ss3220_set_role(hd3ss3220);
+	err = regmap_update_bits_base(hd3ss3220->regmap,
+				      HD3SS3220_REG_CN_STAT_CTRL,
+				      HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+				      HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+				      NULL, false, true);
+	if (err < 0)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
+{
+	struct i2c_client *client = to_i2c_client(data);
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	return hd3ss3220_irq(hd3ss3220);
+}
+
+static const struct regmap_config config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x0A,
+};
+
+static int hd3ss3220_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct hd3ss3220 *hd3ss3220;
+	struct fwnode_handle *connector;
+	int ret;
+	unsigned int data;
+
+	hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
+				 GFP_KERNEL);
+	if (!hd3ss3220)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, hd3ss3220);
+
+	hd3ss3220->dev = &client->dev;
+	hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
+	if (IS_ERR(hd3ss3220->regmap))
+		return PTR_ERR(hd3ss3220->regmap);
+
+	hd3ss3220_set_source_pref(hd3ss3220,
+				  HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+	connector = device_get_named_child_node(hd3ss3220->dev, "connector");
+	if (IS_ERR(connector))
+		return PTR_ERR(connector);
+
+	hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
+	fwnode_handle_put(connector);
+	if (IS_ERR_OR_NULL(hd3ss3220->role_sw))
+		return PTR_ERR(hd3ss3220->role_sw);
+
+	hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+	hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
+	hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
+	hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
+
+	hd3ss3220->port = typec_register_port(&client->dev,
+					      &hd3ss3220->typec_cap);
+	if (IS_ERR(hd3ss3220->port))
+		return PTR_ERR(hd3ss3220->port);
+
+	hd3ss3220_set_role(hd3ss3220);
+	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data);
+	if (ret < 0)
+		goto error;
+
+	if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
+		ret = regmap_write(hd3ss3220->regmap,
+				   HD3SS3220_REG_CN_STAT_CTRL,
+				data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+		if (ret < 0)
+			goto error;
+	}
+
+	if (client->irq > 0) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+					hd3ss3220_irq_handler,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					"hd3ss3220", &client->dev);
+		if (ret)
+			goto error;
+	}
+
+	ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
+	if (ret < 0)
+		goto error;
+
+	dev_info(&client->dev, "probed revision=0x%x\n", ret);
+
+	return 0;
+error:
+	typec_unregister_port(hd3ss3220->port);
+	usb_role_switch_put(hd3ss3220->role_sw);
+
+	return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	typec_unregister_port(hd3ss3220->port);
+	usb_role_switch_put(hd3ss3220->role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id dev_ids[] = {
+	{ .compatible = "ti,hd3ss3220"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, dev_ids);
+
+static struct i2c_driver hd3ss3220_driver = {
+	.driver = {
+		.name = "hd3ss3220",
+		.of_match_table = of_match_ptr(dev_ids),
+	},
+	.probe = hd3ss3220_probe,
+	.remove =  hd3ss3220_remove,
+};
+
+module_i2c_driver(hd3ss3220_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
+MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 16/23] usb: typec: hd3ss3220_irq() can be static

[Marian-Cristian Rotariu]

From: kbuild test robot <lkp@intel.com>

commit dd3fd317e2beb899cbffcf364de049b9f9a02db5 upstream.

Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller")
Signed-off-by: kbuild test robot <lkp@intel.com>
Link: https://lore.kernel.org/r/20191005215727.qfypxoswkiyu45ak at 332d0cec05f4
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/hd3ss3220.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 0ece5ed..658e6aa 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -120,7 +120,7 @@ static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
 	}
 }
 
-irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+static irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
 {
 	int err;
 
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 17/23] usb: typec: add dependency for TYPEC_HD3SS3220

[Marian-Cristian Rotariu]

From: Mao Wenan <maowenan@huawei.com>

commit da4b5d18dd949abdda7c8ea76c9483b5edd49616 upstream.

If CONFIG_TYPEC_HD3SS3220=y, CONFIG_USB_ROLE_SWITCH=m, below errors
can be found:
drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_remove':
hd3ss3220.c:(.text+0x64): undefined reference to `usb_role_switch_put'
drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_dr_set':
hd3ss3220.c:(.text+0x154): undefined reference to `usb_role_switch_set_role'
drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_set_role':
hd3ss3220.c:(.text+0x294): undefined reference to `usb_role_switch_set_role'
hd3ss3220.c:(.text+0x2f4): undefined reference to `usb_role_switch_set_role'
hd3ss3220.c:(.text+0x348): undefined reference to `usb_role_switch_set_role'
hd3ss3220.c:(.text+0x390): undefined reference to `usb_role_switch_set_role'
drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_probe':
hd3ss3220.c:(.text+0x5e8): undefined reference to `fwnode_usb_role_switch_get'
hd3ss3220.c:(.text+0x8a4): undefined reference to `usb_role_switch_put'
make: *** [vmlinux] Error 1

This patch add dependency USB_ROLE_SWITCH for TYPEC_HD3SS3220.

Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Biju Das <biju.das@bp.renesas.com>
Link: https://lore.kernel.org/r/20191009014707.38716-1-maowenan at huawei.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index fde51c5..a4bdb5c 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -95,6 +95,7 @@ source "drivers/usb/typec/ucsi/Kconfig"
 config TYPEC_HD3SS3220
 	tristate "TI HD3SS3220 Type-C DRP Port controller driver"
 	depends on I2C
+	depends on USB_ROLE_SWITCH
 	help
 	  Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
 	  controller driver.
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 18/23] usb: typec: hd3ss3220: hd3ss3220_probe() warn: passing zero to 'PTR_ERR'

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit 5a9a8a4c505851cfb1fe5772851fb528476575bb upstream.

This patch fixes the warning passing zero to 'PTR_ERR' by changing the
check from 'IS_ERR_OR_NULL' to 'IS_ERR'. Also improved the error handling
on probe function.

Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller")
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://lore.kernel.org/r/1570462729-25722-1-git-send-email-biju.das at bp.renesas.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/hd3ss3220.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 658e6aa..0b28ca9 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -178,7 +178,7 @@ static int hd3ss3220_probe(struct i2c_client *client,
 
 	hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
 	fwnode_handle_put(connector);
-	if (IS_ERR_OR_NULL(hd3ss3220->role_sw))
+	if (IS_ERR(hd3ss3220->role_sw))
 		return PTR_ERR(hd3ss3220->role_sw);
 
 	hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
@@ -188,20 +188,22 @@ static int hd3ss3220_probe(struct i2c_client *client,
 
 	hd3ss3220->port = typec_register_port(&client->dev,
 					      &hd3ss3220->typec_cap);
-	if (IS_ERR(hd3ss3220->port))
-		return PTR_ERR(hd3ss3220->port);
+	if (IS_ERR(hd3ss3220->port)) {
+		ret = PTR_ERR(hd3ss3220->port);
+		goto err_put_role;
+	}
 
 	hd3ss3220_set_role(hd3ss3220);
 	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data);
 	if (ret < 0)
-		goto error;
+		goto err_unreg_port;
 
 	if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
 		ret = regmap_write(hd3ss3220->regmap,
 				   HD3SS3220_REG_CN_STAT_CTRL,
 				data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
 		if (ret < 0)
-			goto error;
+			goto err_unreg_port;
 	}
 
 	if (client->irq > 0) {
@@ -210,18 +212,19 @@ static int hd3ss3220_probe(struct i2c_client *client,
 					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 					"hd3ss3220", &client->dev);
 		if (ret)
-			goto error;
+			goto err_unreg_port;
 	}
 
 	ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
 	if (ret < 0)
-		goto error;
+		goto err_unreg_port;
 
 	dev_info(&client->dev, "probed revision=0x%x\n", ret);
 
 	return 0;
-error:
+err_unreg_port:
 	typec_unregister_port(hd3ss3220->port);
+err_put_role:
 	usb_role_switch_put(hd3ss3220->role_sw);
 
 	return ret;
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 19/23] usb: typec: fix an IS_ERR() vs NULL bug in hd3ss3220_probe()

[Marian-Cristian Rotariu]

From: Dan Carpenter <dan.carpenter@oracle.com>

commit c8dd192be821039d7023e0a5869d2cdf3acdef7e upstream.

The device_get_named_child_node() function doesn't return error
pointers, it returns NULL on error.

Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://lore.kernel.org/r/20191011185055.GA20972 at mwanda
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/typec/hd3ss3220.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 0b28ca9..ac4bd17 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -173,8 +173,8 @@ static int hd3ss3220_probe(struct i2c_client *client,
 	hd3ss3220_set_source_pref(hd3ss3220,
 				  HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
 	connector = device_get_named_child_node(hd3ss3220->dev, "connector");
-	if (IS_ERR(connector))
-		return PTR_ERR(connector);
+	if (!connector)
+		return -ENODEV;
 
 	hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
 	fwnode_handle_put(connector);
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 20/23] usb: gadget: udc: renesas_usb3: Enhance role switch support

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit 0604160d8c0b6082727182398d432a68bdd58872 upstream.

The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220 usb
type-c drp port controller. Enhance role switch support to assign the role
requested by connector device using the usb role switch class framework.

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
(fixed checkpatch alignment warnings)
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 drivers/usb/gadget/udc/renesas_usb3.c | 91 ++++++++++++++++++++++++++++++++---
 1 file changed, 84 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index b6eec81..d9668f8 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -352,6 +352,8 @@ struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	enum usb_role connection_state;
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;
@@ -360,6 +362,7 @@ struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB */
 	bool forced_b_device;
 	bool start_to_connect;
+	bool role_sw_by_connector;
 };
 
 #define gadget_to_renesas_usb3(_gadget)	\
@@ -700,8 +703,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
 	unsigned long flags;
 
 	spin_lock_irqsave(&usb3->lock, flags);
-	usb3_set_mode_by_role_sw(usb3, host);
-	usb3_vbus_out(usb3, a_dev);
+	if (!usb3->role_sw_by_connector ||
+	    usb3->connection_state != USB_ROLE_NONE) {
+		usb3_set_mode_by_role_sw(usb3, host);
+		usb3_vbus_out(usb3, a_dev);
+	}
 	/* for A-Peripheral or forced B-device mode */
 	if ((!host && a_dev) || usb3->start_to_connect)
 		usb3_connect(usb3);
@@ -717,7 +723,8 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
 {
 	usb3->extcon_host = usb3_is_a_device(usb3);
 
-	if (usb3->extcon_host && !usb3->forced_b_device)
+	if ((!usb3->role_sw_by_connector && usb3->extcon_host &&
+	     !usb3->forced_b_device) || usb3->connection_state == USB_ROLE_HOST)
 		usb3_mode_config(usb3, true, true);
 	else
 		usb3_mode_config(usb3, false, false);
@@ -2344,14 +2351,65 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
 	return cur_role;
 }
 
-static int renesas_usb3_role_switch_set(struct device *dev,
-					enum usb_role role)
+static void handle_ext_role_switch_states(struct device *dev,
+					  enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+	struct device *host = usb3->host_dev;
+	enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+
+	switch (role) {
+	case USB_ROLE_NONE:
+		usb3->connection_state = USB_ROLE_NONE;
+		if (usb3->driver)
+			usb3_disconnect(usb3);
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_DEVICE:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			usb3->connection_state = USB_ROLE_DEVICE;
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		} else if (cur_role == USB_ROLE_HOST)  {
+			device_release_driver(host);
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		}
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_HOST:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			if (usb3->driver)
+				usb3_disconnect(usb3);
+
+			usb3->connection_state = USB_ROLE_HOST;
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host) failed\n");
+		} else if (cur_role == USB_ROLE_DEVICE) {
+			usb3_disconnect(usb3);
+			/* Must set the mode before device_attach of the host */
+			usb3_set_mode(usb3, true);
+			/* This device_attach() might sleep */
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host) failed\n");
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static void handle_role_switch_states(struct device *dev,
+				      enum usb_role role)
 {
 	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
 	struct device *host = usb3->host_dev;
 	enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
 
-	pm_runtime_get_sync(dev);
 	if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
 		device_release_driver(host);
 		usb3_set_mode(usb3, false);
@@ -2362,6 +2420,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
 		if (device_attach(host) < 0)
 			dev_err(dev, "device_attach(host) failed\n");
 	}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+					enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+
+	if (usb3->role_sw_by_connector)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);
 
 	return 0;
@@ -2651,7 +2723,7 @@ static const unsigned int renesas_usb3_cable[] = {
 	EXTCON_NONE,
 };
 
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
 	.set = renesas_usb3_role_switch_set,
 	.get = renesas_usb3_role_switch_get,
 	.allow_userspace_control = true,
@@ -2742,6 +2814,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
+		usb3->role_sw_by_connector = true;
+		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+	}
+
 	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
 	usb3->role_sw = usb_role_switch_register(&pdev->dev,
 					&renesas_usb3_role_switch_desc);
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 21/23] arm64: defconfig: enable TYPEC_HD3SS3220 config option

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit c24f6886b2ee321b50543d3683573689a1817336 upstream

Enable support for the TI HD3SS320 USB Type-C DRP Port controller driver
by turning on CONFIG_TYPEC and CONFIG_TYPEC_HD3SS3220 as modules.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
(fixed context of patch as CONFIG_USB_ULPI_BUS was removed from upstream)
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0165a2f..3ffac8d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -510,6 +510,8 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_RENESAS_USBHS_UDC=m
 CONFIG_USB_RENESAS_USB3=m
 CONFIG_USB_ULPI_BUS=y
+CONFIG_TYPEC=m
+CONFIG_TYPEC_HD3SS3220=m
 CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=32
 CONFIG_MMC_ARMMMCI=y
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 22/23] arm64: dts: renesas: cat874: Enable USB3.0 host/peripheral device node

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit cf7b175ae4deee43e3408bece10246d6a54cbe4c upstream.

This patch enables USB3.0 host/peripheral device node for the cat874
board.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
(fixed cherry-pick confict as patch has different context in upstream)
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 5be3bad..6b48f87 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -297,6 +297,11 @@
 		groups = "ssi01239_ctrl", "ssi0_data";
 		function = "ssi";
 	};
+
+	usb30_pins: usb30 {
+		groups = "usb30", "usb30_id";
+		function = "usb30";
+	};
 };
 
 &rcar_sound {
@@ -370,3 +375,15 @@
 	renesas,no-otg-pins;
 	status = "okay";
 };
+
+&usb3_peri0 {
+	companion = <&xhci0>;
+	status = "okay";
+};
+
+&xhci0 {
+	pinctrl-0 = <&usb30_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
2.7.4

[cip-dev] [PATCH 4.19.y-cip 23/23] arm64: dts: renesas: cat874: Enable usb role switch support

[Marian-Cristian Rotariu]

From: Biju Das <biju.das@bp.renesas.com>

commit ec0a286a339e0fff8666d48ccce48263488e64fb upstream.

This patch enables TI HD3SS3220 device and support usb role switch
for the CAT 874 platform.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 6b48f87..c99b1de 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -181,6 +181,31 @@
 	status = "okay";
 	clock-frequency = <100000>;
 
+	hd3ss3220 at 47 {
+		compatible = "ti,hd3ss3220";
+		reg = <0x47>;
+		interrupt-parent = <&gpio6>;
+		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+		connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port at 1 {
+					reg = <1>;
+					hd3ss3220_ep: endpoint {
+						remote-endpoint = <&usb3_role_switch>;
+					};
+				};
+			};
+		};
+	};
+
 	tda19988: tda19988 at 70 {
 		compatible = "nxp,tda998x";
 		reg = <0x70>;
@@ -379,6 +404,13 @@
 &usb3_peri0 {
 	companion = <&xhci0>;
 	status = "okay";
+	usb-role-switch;
+
+	port {
+		usb3_role_switch: endpoint {
+			remote-endpoint = <&hd3ss3220_ep>;
+		};
+	};
 };
 
 &xhci0 {
-- 
2.7.4

Re: [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C Backport

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Hi!

> This patchset revolves around the following patch set from upstream:
> https://patchwork.kernel.org/cover/10969899/
> 
> Unfortunately, the driver is using a slightly different version of the USB
> API. In upstream, the connection between the fwnode of the USB controller
> device tree node, that is the connector device tree node, and the USB
> peripheral device tree node is nicely done via some new function calls that
> are basically some extensions to the graph traverse and graph discovery
> methods for the device tree parser. This is needed for role switch feature.
> 
> I tried to create the minimum set for the USB API extension and it should
> not affect at all the current behavior. All the modifications are additions
> and some rewritings of some of the current functions.
> 
> I also backported some fixes where I have found them.
> 
> Tested on Si-Linux EK874 (with RZ/G2E SoC) that we have in house.

Ok, so this does not look bad, but it affects other people's boards,
too. I went through it and will have some questions/comments.

Is someone else in -cip project using usb-otg? Are we reasonably sure
this won't cause problems elsewhere?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 01/23] device connection: Add fwnode member to struct device_connection

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

Hi!

> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> commit 09aa11cfda9d8186046bcd1adcd6498b688114f4 upstream.
> 
> This will prepare the device connection API for connections
> described in firmware.

> @@ -753,11 +753,17 @@ struct device_dma_parameters {
>   */
>  struct device_connection {
> +	struct fwnode_handle	*fwnode;
>  	const char		*endpoint[2];
>  	const char		*id;
>  	struct list_head	list;

Ok, so this really needs to be 0-initialized in all cases, or we'll be
breaking existing setups. Are we reasonably sure that's the case?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 04/23] usb: roles: Find the muxes by also matching against the device node

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

Hi!

> @@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
>  }
>  EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
>  
> -static int __switch_match(struct device *dev, const void *name)
> +static int switch_fwnode_match(struct device *dev, const void *fwnode)
> +{
> +	return dev_fwnode(dev) == fwnode;
> +}
> +
> +static int switch_name_match(struct device *dev, const void *name)
>  {
>  	return !strcmp((const char *)name, dev_name(dev));
>  }

This renames  __switch_match -> switch_name_match. That is welcome
cleanup for mainline, but not quite neccessary change here. As the
function is static and local to the file, we can live with that,
but I hope you have not had to do extra effort to backport it :-).

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 05/23] usb: typec: Find the ports by also matching against the device node

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

Hi!

> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> commit 6a0bbcf96b2273f110a14d11a5952527c5921191 upstream.
> 
> When the connections are defined in firmware, struct
> device_connection will have the fwnode member pointing to
> the device node (struct fwnode_handle) of the requested
> device, and the endpoint will not be used at all in that
> case.

>  static void *typec_port_match(struct device_connection *con, int ep, void *data)
>  {
> -	return class_find_device(typec_class, NULL, con->endpoint[ep],
> -				 __typec_port_match);
...
> +	dev = class_find_device(typec_class, NULL, con->endpoint[ep],
> +				typec_port_name_match);
> +
> +	return dev ? dev : ERR_PTR(-EPROBE_DEFER);
>  }

So... this adds handling in fwnode != NULL (expected, okay with me),
but it also changes behaviour in fwnode == NULL case: return value
changed from NULL to ERR_PTR(-EPROBE_DEFER). Are all callers ready to
handle the changed situation in -cip? Could we get some explanation
why it is neccessary/good idea?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

Hi!

> From: Biju Das <biju.das@bp.renesas.com>
> 
> commit 1c48c759ef4bb9031b3347277f04484e07e27d97 upstream.
> 
> Driver for TI HD3SS3220 USB Type-C DRP port controller.
> 
> The driver currently registers the port and supports data role
> swapping.

Ok, so this has few problems that are fixed by subsequent patches, but
I believe I see some more:

> +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
> +{
> +	unsigned int reg_val;
> +	enum usb_role attached_state;
> +	int ret;
> +
> +	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
> +			  &reg_val);
> +	if (ret < 0)
> +		return ret;

This function claims to return "enum usb_role", but here it returns
errno from regmap_read.

> +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> +			    enum typec_data_role role)
> +{
...
> +	ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> +	usleep_range(10, 100);

Would udelay() make more sense here? Are your CPUs / timer subsystem
so fast that sleeping for 10usec is possible and reasonable to do?

Best regards,
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 17/23] usb: typec: add dependency for TYPEC_HD3SS3220

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

On Tue 2020-02-18 14:05:14, Marian-Cristian Rotariu wrote:
> From: Mao Wenan <maowenan@huawei.com>
> 
> commit da4b5d18dd949abdda7c8ea76c9483b5edd49616 upstream.
> 
> If CONFIG_TYPEC_HD3SS3220=y, CONFIG_USB_ROLE_SWITCH=m, below errors
> can be found:
> drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_remove':
> hd3ss3220.c:(.text+0x64): undefined reference to `usb_role_switch_put'
> drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_dr_set':
> hd3ss3220.c:(.text+0x154): undefined reference to `usb_role_switch_set_role'
> drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_set_role':
> hd3ss3220.c:(.text+0x294): undefined reference to `usb_role_switch_set_role'
> hd3ss3220.c:(.text+0x2f4): undefined reference to `usb_role_switch_set_role'
> hd3ss3220.c:(.text+0x348): undefined reference to `usb_role_switch_set_role'
> hd3ss3220.c:(.text+0x390): undefined reference to `usb_role_switch_set_role'
> drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_probe':
> hd3ss3220.c:(.text+0x5e8): undefined reference to `fwnode_usb_role_switch_get'
> hd3ss3220.c:(.text+0x8a4): undefined reference to `usb_role_switch_put'
> make: *** [vmlinux] Error 1

I guess it is okay here, but for more serious bugs it would be nice to
merge the buggy patch with a fix -- not to break bisect.

Best regards,
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 21/23] arm64: defconfig: enable TYPEC_HD3SS3220 config option

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

On Tue 2020-02-18 14:05:18, Marian-Cristian Rotariu wrote:
> From: Biju Das <biju.das@bp.renesas.com>
> 
> commit c24f6886b2ee321b50543d3683573689a1817336 upstream
> 
> Enable support for the TI HD3SS320 USB Type-C DRP Port controller driver
> by turning on CONFIG_TYPEC and CONFIG_TYPEC_HD3SS3220 as modules.

No, just no.

Lets enable new code for your boards (cip-kernel-config repository),
but keep it disabled for other people. We don't want to break build by
mistake, or something like that.

Best regards,
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 16/23] usb: typec: hd3ss3220_irq() can be static

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

Hi!

> From: kbuild test robot <lkp@intel.com>
> 
> commit dd3fd317e2beb899cbffcf364de049b9f9a02db5 upstream.
> 
> Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller")
> Signed-off-by: kbuild test robot <lkp@intel.com>

This is not your fault and we should not attempt to fix it here, but
we normally require real, human names for Signed-off-by.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C Backport

[Marian-Cristian Rotariu]

Hi Pavel!

First of all, thank you for the review. 

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 07:42
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C
> Backport
> 
> Hi!
> 
> > This patchset revolves around the following patch set from upstream:
> > https://patchwork.kernel.org/cover/10969899/
> >
> > Unfortunately, the driver is using a slightly different version of the
> > USB API. In upstream, the connection between the fwnode of the USB
> > controller device tree node, that is the connector device tree node,
> > and the USB peripheral device tree node is nicely done via some new
> > function calls that are basically some extensions to the graph
> > traverse and graph discovery methods for the device tree parser. This is
> needed for role switch feature.
> >
> > I tried to create the minimum set for the USB API extension and it
> > should not affect at all the current behavior. All the modifications
> > are additions and some rewritings of some of the current functions.
> >
> > I also backported some fixes where I have found them.
> >
> > Tested on Si-Linux EK874 (with RZ/G2E SoC) that we have in house.
> 
> Ok, so this does not look bad, but it affects other people's boards, too. I went
> through it and will have some questions/comments.
> 
> Is someone else in -cip project using usb-otg? Are we reasonably sure this
> won't cause problems elsewhere?

There are not many USB Type-C connectors, not even in upstream. For the common
area of the code (drivers/usb/roles/class.c) I see only two drivers that call
usb_role_switch_register(), Renesas's and x86 Intel xHCI driver.

Does linux-cip support x86 platforms?

I am actively trying to find what could go wrong with this implementation. Also, there
might be another way of doing this by attaching the graph connection to the port
controller device tree node and create a device connection between the two devices
(the role switch device and the port controller device). This might get rid of all the extra
additional patches in the general area of the code (no reference to fwnode), but might
introduce modifications into the port controller driver and into the role switch driver of
Renesas.

I will attempt to answer to all your comments, maybe this will look better in a v2 patchset.

Best regards,
Marian

  

Re: [PATCH 4.19.y-cip 04/23] usb: roles: Find the muxes by also matching against the device node

[Marian-Cristian Rotariu]

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 07:51
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 04/23] usb: roles: Find the muxes by
> also matching against the device node
> 
> Hi!
> 
> > @@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct
> > usb_role_switch *sw)  }
> EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
> >
> > -static int __switch_match(struct device *dev, const void *name)
> > +static int switch_fwnode_match(struct device *dev, const void
> > +*fwnode) {
> > +	return dev_fwnode(dev) == fwnode;
> > +}
> > +
> > +static int switch_name_match(struct device *dev, const void *name)
> >  {
> >  	return !strcmp((const char *)name, dev_name(dev));  }
> 
> This renames  __switch_match -> switch_name_match. That is welcome
> cleanup for mainline, but not quite neccessary change here. As the function
> is static and local to the file, we can live with that, but I hope you have not
> had to do extra effort to backport it :-).

Unfortunately, this patch is necessary for the following patches as it introduces
the necessary fwnode. Generally, yes, I had to find the minimum set of changes,
not to modify too much the USB core code. I opted for complete backport
of the upstream patches, not to chop them off and also not to add much code on
top of them (only some minor checkpatch fixes). I think it is better not to increase
the delta between linux-cip and upstream too much.

But, looking retrospectively, this might not be the best strategy.

> 
> Best regards,
> 
> 									Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 4.19.y-cip 01/23] device connection: Add fwnode member to struct device_connection

[Marian-Cristian Rotariu]

> > From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >
> > commit 09aa11cfda9d8186046bcd1adcd6498b688114f4 upstream.
> >
> > This will prepare the device connection API for connections described
> > in firmware.
> 
> > @@ -753,11 +753,17 @@ struct device_dma_parameters {
> >   */
> >  struct device_connection {
> > +	struct fwnode_handle	*fwnode;
> >  	const char		*endpoint[2];
> >  	const char		*id;
> >  	struct list_head	list;
> 
> Ok, so this really needs to be 0-initialized in all cases, or we'll be breaking
> existing setups. Are we reasonably sure that's the case?

Yes, I need to add two 0-initialization to the following drivers:
drivers/platform/x86/intel_cht_int33fe.c
drivers/extcon/extcon-axp288.c

In upstream, these drivers opt for fwnode node discovery instead of device_connection definitions.
I will do the modifications in v2.

Thanks,
Marian

Re: [PATCH 4.19.y-cip 16/23] usb: typec: hd3ss3220_irq() can be static

[Marian-Cristian Rotariu]

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 08:21
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 16/23] usb: typec: hd3ss3220_irq()
> can be static
> 
> Hi!
> 
> > From: kbuild test robot <lkp@intel.com>
> >
> > commit dd3fd317e2beb899cbffcf364de049b9f9a02db5 upstream.
> >
> > Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C
> > DRP port controller")
> > Signed-off-by: kbuild test robot <lkp@intel.com>
> 
> This is not your fault and we should not attempt to fix it here, but we
> normally require real, human names for Signed-off-by.

I will drop this in v2.

Thanks,
Marian

Re: [PATCH 4.19.y-cip 21/23] arm64: defconfig: enable TYPEC_HD3SS3220 config option

[Marian-Cristian Rotariu]

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 08:19
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 21/23] arm64: defconfig: enable
> TYPEC_HD3SS3220 config option
> 
> On Tue 2020-02-18 14:05:18, Marian-Cristian Rotariu wrote:
> > From: Biju Das <biju.das@bp.renesas.com>
> >
> > commit c24f6886b2ee321b50543d3683573689a1817336 upstream
> >
> > Enable support for the TI HD3SS320 USB Type-C DRP Port controller
> > driver by turning on CONFIG_TYPEC and CONFIG_TYPEC_HD3SS3220 as
> modules.
> 
> No, just no.
> 
> Lets enable new code for your boards (cip-kernel-config repository), but
> keep it disabled for other people. We don't want to break build by mistake,
> or something like that.

Ok, I will drop this for v2.

Thanks,
Marian

Re: [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C Backport

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

Hi!

> First of all, thank you for the review.

You are welcome :-).

> > > I also backported some fixes where I have found them.
> > >
> > > Tested on Si-Linux EK874 (with RZ/G2E SoC) that we have in house.
> > 
> > Ok, so this does not look bad, but it affects other people's boards, too. I went
> > through it and will have some questions/comments.
> > 
> > Is someone else in -cip project using usb-otg? Are we reasonably sure this
> > won't cause problems elsewhere?
> 
> There are not many USB Type-C connectors, not even in upstream. For the common
> area of the code (drivers/usb/roles/class.c) I see only two drivers that call
> usb_role_switch_register(), Renesas's and x86 Intel xHCI driver.
> 
> Does linux-cip support x86 platforms?

Yes, we have several x86 configurations. But you may be right that
Type-C is less common than I thought.

cip-kernel-config/4.19.y-cip-rt/x86:
siemens_i386-rt.config	siemens_i386-rt.sources

cip-kernel-config/4.19.y-cip/x86:
plathome_obsvx2.config	   siemens_ipc227e.sources
plathome_obsvx2.sources    siemens_server_defconfig
siemens_iot2000.config	      siemens_server.sources
siemens_iot2000.sources    toshiba_atom_baytrail_cip.config
siemens_ipc227e_defconfig  toshiba_atom_baytrail_cip.sources

> I am actively trying to find what could go wrong with this implementation. Also, there
> might be another way of doing this by attaching the graph connection to the port
> controller device tree node and create a device connection between the two devices
> (the role switch device and the port controller device). This might get rid of all the extra
> additional patches in the general area of the code (no reference to fwnode), but might
> introduce modifications into the port controller driver and into the role switch driver of
> Renesas.

I believe that general approach of the patches is okay. You should not
need to do big changes.

Best regards,

								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 04/23] usb: roles: Find the muxes by also matching against the device node

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

Hi!

> > > @@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct
> > > usb_role_switch *sw)  }
> > EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
> > >
> > > -static int __switch_match(struct device *dev, const void *name)
> > > +static int switch_fwnode_match(struct device *dev, const void
> > > +*fwnode) {
> > > +	return dev_fwnode(dev) == fwnode;
> > > +}
> > > +
> > > +static int switch_name_match(struct device *dev, const void *name)
> > >  {
> > >  	return !strcmp((const char *)name, dev_name(dev));  }
> > 
> > This renames  __switch_match -> switch_name_match. That is welcome
> > cleanup for mainline, but not quite neccessary change here. As the function
> > is static and local to the file, we can live with that, but I hope you have not
> > had to do extra effort to backport it :-).
> 
> Unfortunately, this patch is necessary for the following patches as it introduces
> the necessary fwnode. Generally, yes, I had to find the minimum set of changes,
> not to modify too much the USB core code. I opted for complete backport
> of the upstream patches, not to chop them off and also not to add much code on
> top of them (only some minor checkpatch fixes). I think it is better not to increase
> the delta between linux-cip and upstream too much.
> 
> But, looking retrospectively, this might not be the best strategy.

Just to be clear, I believe we can use this patch as-is. No need to
change it.

Best regards,
       	       			     	       	      	   Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4.19.y-cip 16/23] usb: typec: hd3ss3220_irq() can be static

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

Hi!

> > > From: kbuild test robot <lkp@intel.com>
> > >
> > > commit dd3fd317e2beb899cbffcf364de049b9f9a02db5 upstream.
> > >
> > > Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C
> > > DRP port controller")
> > > Signed-off-by: kbuild test robot <lkp@intel.com>
> > 
> > This is not your fault and we should not attempt to fix it here, but we
> > normally require real, human names for Signed-off-by.
> 
> I will drop this in v2.

You can merge it with the previous patch, or you can keep it as is, it
is your choice. If mainline is okay with it, it should not create
problems for us, even if it is a bit non-standard.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [cip-dev] [PATCH 4.19.y-cip 05/23] usb: typec: Find the ports by also matching against the device node

[Marian-Cristian Rotariu]

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 07:57
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 05/23] usb: typec: Find the ports by
> also matching against the device node
> 
> Hi!
> 
> > From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >
> > commit 6a0bbcf96b2273f110a14d11a5952527c5921191 upstream.
> >
> > When the connections are defined in firmware, struct device_connection
> > will have the fwnode member pointing to the device node (struct
> > fwnode_handle) of the requested device, and the endpoint will not be
> > used at all in that case.
> 
> >  static void *typec_port_match(struct device_connection *con, int ep,
> > void *data)  {
> > -	return class_find_device(typec_class, NULL, con->endpoint[ep],
> > -				 __typec_port_match);
> ...
> > +	dev = class_find_device(typec_class, NULL, con->endpoint[ep],
> > +				typec_port_name_match);
> > +
> > +	return dev ? dev : ERR_PTR(-EPROBE_DEFER);
> >  }
> 
> So... this adds handling in fwnode != NULL (expected, okay with me), but it
> also changes behaviour in fwnode == NULL case: return value changed from
> NULL to ERR_PTR(-EPROBE_DEFER). Are all callers ready to handle the
> changed situation in -cip? Could we get some explanation why it is
> neccessary/good idea?

This is part of the Type-C Alternate Modes. This callback is used only by the
typec_altmode_register_notifier() that should be called by the upper driver
that uses the USB Type-C port in the Alternate Mode. In this way it can receive
notifications from it. 

In its initial form, this function explores the graph child nodes and finds the
adequate USB port controller device. But, this device might not be ready/probed
yet. Hence, the EPROBE_DEFER addition of this patch.

Strangely enough, no one is using this notification chain so far, not even in
mainstream. I guess Type-C is still in its infancy. 
_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev

Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Marian-Cristian Rotariu]

Hi!

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 08:13
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI
> HD3SS3220 USB Type-C DRP port controller
> 
> Hi!
> 
> > From: Biju Das <biju.das@bp.renesas.com>
> >
> > commit 1c48c759ef4bb9031b3347277f04484e07e27d97 upstream.
> >
> > Driver for TI HD3SS3220 USB Type-C DRP port controller.
> >
> > The driver currently registers the port and supports data role
> > swapping.
> 
> Ok, so this has few problems that are fixed by subsequent patches, but I
> believe I see some more:
> 
> > +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220
> > +*hd3ss3220) {
> > +	unsigned int reg_val;
> > +	enum usb_role attached_state;
> > +	int ret;
> > +
> > +	ret = regmap_read(hd3ss3220->regmap,
> HD3SS3220_REG_CN_STAT_CTRL,
> > +			  &reg_val);
> > +	if (ret < 0)
> > +		return ret;
> 
> This function claims to return "enum usb_role", but here it returns errno
> from regmap_read.
> 
> > +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> > +			    enum typec_data_role role)
> > +{
> ...
> > +	ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> > +	usleep_range(10, 100);
> 
> Would udelay() make more sense here? Are your CPUs / timer subsystem so
> fast that sleeping for 10usec is possible and reasonable to do?

I think Biju followed the overall indications from:
Documentation/timers-howto.txt (.rst)

According to the documentation the inflection point is at 10us, so this is kind
of a grey area. Just to put it in context, the RZ/G2E has 2x1.2Ghz ARM Cortex A53.
Therefore this is kind of a high-performance embedded platform. At least, we
like to advertise it like that 😊

So, I am inclined to keep this as it is if you agree. 

Cheers,
Marian.
_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev

Re: [cip-dev] [PATCH 4.19.y-cip 17/23] usb: typec: add dependency for TYPEC_HD3SS3220

[Marian-Cristian Rotariu]

Hi!

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 08:16
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 17/23] usb: typec: add dependency
> for TYPEC_HD3SS3220
> 
> On Tue 2020-02-18 14:05:14, Marian-Cristian Rotariu wrote:
> > From: Mao Wenan <maowenan@huawei.com>
> >
> > commit da4b5d18dd949abdda7c8ea76c9483b5edd49616 upstream.
> >
> > If CONFIG_TYPEC_HD3SS3220=y, CONFIG_USB_ROLE_SWITCH=m, below
> errors
> > can be found:
> > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_remove':
> > hd3ss3220.c:(.text+0x64): undefined reference to `usb_role_switch_put'
> > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_dr_set':
> > hd3ss3220.c:(.text+0x154): undefined reference to
> `usb_role_switch_set_role'
> > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_set_role':
> > hd3ss3220.c:(.text+0x294): undefined reference to
> `usb_role_switch_set_role'
> > hd3ss3220.c:(.text+0x2f4): undefined reference to
> `usb_role_switch_set_role'
> > hd3ss3220.c:(.text+0x348): undefined reference to
> `usb_role_switch_set_role'
> > hd3ss3220.c:(.text+0x390): undefined reference to
> `usb_role_switch_set_role'
> > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_probe':
> > hd3ss3220.c:(.text+0x5e8): undefined reference to
> `fwnode_usb_role_switch_get'
> > hd3ss3220.c:(.text+0x8a4): undefined reference to `usb_role_switch_put'
> > make: *** [vmlinux] Error 1
> 
> I guess it is okay here, but for more serious bugs it would be nice to merge
> the buggy patch with a fix -- not to break bisect.

I kind of forgot about bisect. I was mostly focused on keeping the original patches
from upstream (in this case, it is more about the commit message as the fix is trivial).
I will send this patch in v2 also, since you mostly agreed with it. Please let me know
if otherwise. 

Cheers,
Marian
_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev

Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Pavel Machek]

[-- Attachment #1.1: Type: text/plain, Size: 1617 bytes --]

Hi!

> > > +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220
> > > +*hd3ss3220) {
> > > +	unsigned int reg_val;
> > > +	enum usb_role attached_state;
> > > +	int ret;
> > > +
> > > +	ret = regmap_read(hd3ss3220->regmap,
> > HD3SS3220_REG_CN_STAT_CTRL,
> > > +			  &reg_val);
> > > +	if (ret < 0)
> > > +		return ret;
> > 
> > This function claims to return "enum usb_role", but here it returns errno
> > from regmap_read.

######### HERE

> > > +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> > > +			    enum typec_data_role role)
> > > +{
> > ...
> > > +	ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> > > +	usleep_range(10, 100);
> > 
> > Would udelay() make more sense here? Are your CPUs / timer subsystem so
> > fast that sleeping for 10usec is possible and reasonable to do?
> 
> I think Biju followed the overall indications from:
> Documentation/timers-howto.txt (.rst)
> 
> According to the documentation the inflection point is at 10us, so this is kind
> of a grey area. Just to put it in context, the RZ/G2E has 2x1.2Ghz ARM Cortex A53.
> Therefore this is kind of a high-performance embedded platform. At least, we
> like to advertise it like that 😊
> 
> So, I am inclined to keep this as it is if you agree. 

It is not a big deal to keep it. What about the "returning -errno but
return type is enum" issue above? (Marked with "HERE").

Best regards,
								Pavel

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev

Re: [cip-dev] [PATCH 4.19.y-cip 17/23] usb: typec: add dependency for TYPEC_HD3SS3220

[Pavel Machek]

[-- Attachment #1.1: Type: text/plain, Size: 1944 bytes --]

Hi!

> > On Tue 2020-02-18 14:05:14, Marian-Cristian Rotariu wrote:
> > > From: Mao Wenan <maowenan@huawei.com>
> > >
> > > commit da4b5d18dd949abdda7c8ea76c9483b5edd49616 upstream.
> > >
> > > If CONFIG_TYPEC_HD3SS3220=y, CONFIG_USB_ROLE_SWITCH=m, below
> > errors
> > > can be found:
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_remove':
> > > hd3ss3220.c:(.text+0x64): undefined reference to `usb_role_switch_put'
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_dr_set':
> > > hd3ss3220.c:(.text+0x154): undefined reference to
> > `usb_role_switch_set_role'
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_set_role':
> > > hd3ss3220.c:(.text+0x294): undefined reference to
> > `usb_role_switch_set_role'
> > > hd3ss3220.c:(.text+0x2f4): undefined reference to
> > `usb_role_switch_set_role'
> > > hd3ss3220.c:(.text+0x348): undefined reference to
> > `usb_role_switch_set_role'
> > > hd3ss3220.c:(.text+0x390): undefined reference to
> > `usb_role_switch_set_role'
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_probe':
> > > hd3ss3220.c:(.text+0x5e8): undefined reference to
> > `fwnode_usb_role_switch_get'
> > > hd3ss3220.c:(.text+0x8a4): undefined reference to `usb_role_switch_put'
> > > make: *** [vmlinux] Error 1
> > 
> > I guess it is okay here, but for more serious bugs it would be nice to merge
> > the buggy patch with a fix -- not to break bisect.
> 
> I kind of forgot about bisect. I was mostly focused on keeping the original patches
> from upstream (in this case, it is more about the commit message as the fix is trivial).
> I will send this patch in v2 also, since you mostly agreed with it. Please let me know
> if otherwise. 

This patch is okay.

Best regards,

								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev

Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Marian-Cristian Rotariu]

Hi!

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 20 February 2020 15:47
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: Pavel Machek <pavel@denx.de>; cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI
> HD3SS3220 USB Type-C DRP port controller
> 
> Hi!
> 
> > > > +static enum usb_role hd3ss3220_get_attached_state(struct
> > > > +hd3ss3220
> > > > +*hd3ss3220) {
> > > > +	unsigned int reg_val;
> > > > +	enum usb_role attached_state;
> > > > +	int ret;
> > > > +
> > > > +	ret = regmap_read(hd3ss3220->regmap,
> > > HD3SS3220_REG_CN_STAT_CTRL,
> > > > +			  &reg_val);
> > > > +	if (ret < 0)
> > > > +		return ret;
> > >
> > > This function claims to return "enum usb_role", but here it returns
> > > errno from regmap_read.
> 

Yes, this a mistake. The error should be properly propagated to the callers
(that is hd3ss3220_probe)

Also in the mainline. I will fix it in v2 and in mainline.

Thank you for the good catch,
Marian
_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev