Hi,

On 11/08/2019 14:59, Pawel Laszczak wrote:

quoted

Hi,

quoted

On 21/07/2019 21:32, Pawel Laszczak wrote:

quoted

This patch introduce new Cadence USBSS DRD driver to Linux kernel.

The Cadence USBSS DRD Controller is a highly configurable IP Core which
can be instantiated as Dual-Role Device (DRD), Peripheral Only and
Host Only (XHCI)configurations.

The current driver has been validated with FPGA platform. We have
support for PCIe bus, which is used on FPGA prototyping.

The host side of USBSS-DRD controller is compliant with XHCI
specification, so it works with standard XHCI Linux driver.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 drivers/usb/Kconfig                |    2 +
 drivers/usb/Makefile               |    2 +
 drivers/usb/cdns3/Kconfig          |   46 +
 drivers/usb/cdns3/Makefile         |   17 +
 drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
 drivers/usb/cdns3/core.c           |  554 +++++++
 drivers/usb/cdns3/core.h           |  109 ++
 drivers/usb/cdns3/debug.h          |  171 ++
 drivers/usb/cdns3/debugfs.c        |   87 ++
 drivers/usb/cdns3/drd.c            |  390 +++++
 drivers/usb/cdns3/drd.h            |  166 ++
 drivers/usb/cdns3/ep0.c            |  914 +++++++++++
 drivers/usb/cdns3/gadget-export.h  |   28 +
 drivers/usb/cdns3/gadget.c         | 2338 ++++++++++++++++++++++++++++
 drivers/usb/cdns3/gadget.h         | 1321 ++++++++++++++++
 drivers/usb/cdns3/host-export.h    |   28 +
 drivers/usb/cdns3/host.c           |   71 +
 drivers/usb/cdns3/trace.c          |   11 +
 drivers/usb/cdns3/trace.h          |  493 ++++++
 19 files changed, 6951 insertions(+)
 create mode 100644 drivers/usb/cdns3/Kconfig
 create mode 100644 drivers/usb/cdns3/Makefile
 create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
 create mode 100644 drivers/usb/cdns3/core.c
 create mode 100644 drivers/usb/cdns3/core.h
 create mode 100644 drivers/usb/cdns3/debug.h
 create mode 100644 drivers/usb/cdns3/debugfs.c
 create mode 100644 drivers/usb/cdns3/drd.c
 create mode 100644 drivers/usb/cdns3/drd.h
 create mode 100644 drivers/usb/cdns3/ep0.c
 create mode 100644 drivers/usb/cdns3/gadget-export.h
 create mode 100644 drivers/usb/cdns3/gadget.c
 create mode 100644 drivers/usb/cdns3/gadget.h
 create mode 100644 drivers/usb/cdns3/host-export.h
 create mode 100644 drivers/usb/cdns3/host.c
 create mode 100644 drivers/usb/cdns3/trace.c
 create mode 100644 drivers/usb/cdns3/trace.h

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e4b27413f528..c2e78882f8c2 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig

@@ -113,6 +113,8 @@ source "drivers/usb/usbip/Kconfig"

 endif

<snip>

quoted

quoted

quoted

+	real_role = cdsn3_real_role_switch_get(cdns->dev);
+
+	current_role = role;
+	dev_dbg(cdns->dev, "Switching role");
+
+	ret = cdns3_role_start(cdns, real_role);
+	if (ret) {
+		/* Back to current role */
+		dev_err(cdns->dev, "set %d has failed, back to %d\n",
+			role, current_role);
+		ret = cdns3_role_start(cdns, current_role);
+		if (ret)
+			dev_err(cdns->dev, "back to %d failed too\n",
+				current_role);
+	}
+exit:
+	pm_runtime_put_sync(cdns->dev);
+	return ret;
+}
+
+static const struct usb_role_switch_desc cdns3_switch_desc = {
+	.set = cdns3_role_switch_set,
+	.get = cdsn3_real_role_switch_get,
+	.allow_userspace_control = true,

how does user initiated cdns3_role_switch_set() via sysfs co-exist with role
changes done by hardware events. e.g. ID/VBUS?

Do you expect any issues whit this,  have you seen any problem with this
on your  platform ?

I assume that it should work in this way:
1. user change role by sysfs
2. Driver change the role according with user request.
3. If we receive correct ID/VBUS then role should not be changed
    because new role is the same as current set in point 2.

I have not tested this series yet.
My understanding is that if user sets role to "host" or "device" then it should
remain in that role irrespective of ID/VBUS. Once user sets it to "none" then
port should set role based on ID/VBUS.

According with your understanding it works the same way as by debugfs. 
Now I have no doubts to remove debugfs.c file :)

quoted

quoted

quoted

+};
+
+/**
+ * cdns3_probe - probe for cdns3 core device
+ * @pdev: Pointer to cdns3 core platform device
+ *
+ * Returns 0 on success otherwise negative errno
+ */
+static int cdns3_probe(struct platform_device *pdev)

<snip>

quoted

quoted

quoted

+ * Returns 0 on success otherwise negative errno
+ */
+int cdns3_drd_update_mode(struct cdns3 *cdns)
+{
+	int ret = 0;
+
+	if (cdns->desired_dr_mode == cdns->current_dr_mode)
+		return ret;
+
+	switch (cdns->desired_dr_mode) {
+	case USB_DR_MODE_PERIPHERAL:
+		ret = cdns3_set_mode(cdns, USB_DR_MODE_PERIPHERAL);
+		break;
+	case USB_DR_MODE_HOST:
+		ret = cdns3_set_mode(cdns, USB_DR_MODE_HOST);
+		break;
+	case USB_DR_MODE_OTG:
+		ret = cdns3_init_otg_mode(cdns);
+		break;
+	default:
+		dev_err(cdns->dev, "Unsupported mode of operation %d\n",
+			cdns->dr_mode);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static irqreturn_t cdns3_drd_thread_irq(int irq, void *data)
+{
+	struct cdns3 *cdns = data;
+
+	usb_role_switch_set_role(cdns->role_sw, cdns->role);

How to ensure that HW events don't step over user chosen role?

I need to think about this and find out how to test it and eventually force
such cases.

But I assume that after attaching/detaching plug the user chosen role can
be forgotten.

No. Only when user sets role to none then role should be based on HW.

Ok, I will try to test it in next week.

quoted

quoted

<snip>

Cheers,
Pawell