[PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Vinod Koul]

This series add support for Renesas USB controllers uPD720201 and uPD720202.
These require firmware to be loaded and in case devices have ROM those can
also be programmed if empty. If ROM is programmed, it runs from ROM as well.

This includes two patches from Christian which supported these controllers
w/o ROM and later my patches for ROM support and multiple firmware versions,
debugfs hook for rom erase and export of xhci-pci functions.

Changes in v6:
 Move the renesas code into a seprate driver which invokes xhci-pci functions.

Changes in v5:
 Added a debugfs rom erase patch, helps in debugging
 Squashed patch 1 & 2 as requested by Mathias

Changes in v4:
 Rollback the delay values as we got device failures

Changes in v3:
  Dropped patch 2 as discussed with Christian
  Removed aligned 8 bytes check
  Change order for firware search from highest version to lowest
  Added entry for new firmware for device 0x14 as well
  Add tested by Christian

Changes in v2:
  used macros for timeout count and delay
  removed renesas_fw_alive_check
  cleaned renesas_fw_callback
  removed recurion for renesas_fw_download
  added MODULE_FIRMWARE
  added comment for multiple fw order

Christian Lamparter (1):
  usb: renesas-xhci: Add the renesas xhci driver

Vinod Koul (4):
  usb: xhci: export few functions
  usb: renesas-xhci: Add ROM loader for uPD720201
  usb: renesas-xhci: allow multiple firmware versions
  usb: xhci: provide a debugfs hook for erasing rom

 drivers/usb/host/Kconfig            |   9 +
 drivers/usb/host/Makefile           |   1 +
 drivers/usb/host/xhci-pci-renesas.c | 985 ++++++++++++++++++++++++++++
 drivers/usb/host/xhci-pci.c         |  18 +-
 drivers/usb/host/xhci-pci.h         |  18 +
 5 files changed, 1024 insertions(+), 7 deletions(-)
 create mode 100644 drivers/usb/host/xhci-pci-renesas.c
 create mode 100644 drivers/usb/host/xhci-pci.h

-- 
2.24.1

[PATCH v6 1/5] usb: xhci: export few functions

[Vinod Koul]

Export the xhci_pci_setup(), xhci_pci_probe() xhci_pci_remove(),
xhci_pci_suspend() and xhci_pci_resume() functions as they would be used
by renesas-xhci driver.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/usb/host/xhci-pci.c | 18 +++++++++++-------
 drivers/usb/host/xhci-pci.h | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+), 7 deletions(-)
 create mode 100644 drivers/usb/host/xhci-pci.h

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a0025d23b257..5115871b50af 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -15,6 +15,7 @@
 
 #include "xhci.h"
 #include "xhci-trace.h"
+#include "xhci-pci.h"
 
 #define SSIC_PORT_NUM		2
 #define SSIC_PORT_CFG2		0x880c
@@ -60,8 +61,6 @@ static const char hcd_name[] = "xhci_hcd";
 
 static struct hc_driver __read_mostly xhci_pci_hc_driver;
 
-static int xhci_pci_setup(struct usb_hcd *hcd);
-
 static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
 	.reset = xhci_pci_setup,
 };
@@ -282,7 +281,7 @@ static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev) { }
 #endif /* CONFIG_ACPI */
 
 /* called during probe() after chip reset completes */
-static int xhci_pci_setup(struct usb_hcd *hcd)
+int xhci_pci_setup(struct usb_hcd *hcd)
 {
 	struct xhci_hcd		*xhci;
 	struct pci_dev		*pdev = to_pci_dev(hcd->self.controller);
@@ -307,12 +306,13 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
 	/* Find any debug ports */
 	return xhci_pci_reinit(xhci, pdev);
 }
+EXPORT_SYMBOL_GPL(xhci_pci_setup);
 
 /*
  * We need to register our own PCI probe function (instead of the USB core's
  * function) in order to create a second roothub under xHCI.
  */
-static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int retval;
 	struct xhci_hcd *xhci;
@@ -378,8 +378,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	pm_runtime_put_noidle(&dev->dev);
 	return retval;
 }
+EXPORT_SYMBOL_GPL(xhci_pci_probe);
 
-static void xhci_pci_remove(struct pci_dev *dev)
+void xhci_pci_remove(struct pci_dev *dev)
 {
 	struct xhci_hcd *xhci;
 
@@ -401,6 +402,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
 
 	usb_hcd_pci_remove(dev);
 }
+EXPORT_SYMBOL_GPL(xhci_pci_remove);
 
 #ifdef CONFIG_PM
 /*
@@ -457,7 +459,7 @@ static void xhci_pme_quirk(struct usb_hcd *hcd)
 	readl(reg);
 }
 
-static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
+int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
 {
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct pci_dev		*pdev = to_pci_dev(hcd->self.controller);
@@ -482,8 +484,9 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(xhci_pci_suspend);
 
-static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
+int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
 {
 	struct xhci_hcd		*xhci = hcd_to_xhci(hcd);
 	struct pci_dev		*pdev = to_pci_dev(hcd->self.controller);
@@ -519,6 +522,7 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
 	retval = xhci_resume(xhci, hibernated);
 	return retval;
 }
+EXPORT_SYMBOL_GPL(xhci_pci_resume);
 #endif /* CONFIG_PM */
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/host/xhci-pci.h b/drivers/usb/host/xhci-pci.h
new file mode 100644
index 000000000000..587f71dc5e35
--- /dev/null
+++ b/drivers/usb/host/xhci-pci.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2008 Intel Corp. */
+
+#ifndef XHCI_PCI_H
+#define XHCI_PCI_H
+
+int xhci_pci_setup(struct usb_hcd *hcd);
+
+int xhci_pci_probe(struct pci_dev *pdev,
+		   const struct pci_device_id *id);
+
+void xhci_pci_remove(struct pci_dev *dev);
+
+int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup);
+
+int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated);
+
+#endif
-- 
2.24.1

[PATCH v6 2/5] usb: renesas-xhci: Add the renesas xhci driver

[Vinod Koul]

From: Christian Lamparter <chunkeey@googlemail.com>

This add a new driver for renesas xhci which is basically a firmware
loader for uPD720201 and uPD720202 w/o ROM. It uses xhci-pci driver for
most of the work.

This is added in Makefile before the xhci-pci driver so that it first
get probed for renesas devices before the xhci-pci driver has a chance
to claim any device.

This patch adds a firmware loader for the uPD720201K8-711-BAC-A
and uPD720202K8-711-BAA-A variant. Both of these chips are listed
in Renesas' R19UH0078EJ0500 Rev.5.00 "User's Manual: Hardware" as
devices which need the firmware loader on page 2 in order to
work as they "do not support the External ROM".

The "Firmware Download Sequence" is describe in chapter
"7.1 FW Download Interface" R19UH0078EJ0500 Rev.5.00 page 131.

The firmware "K2013080.mem" is available from a USB3.0 Host to
PCIe Adapter (PP2U-E card) "Firmware download" archive. An
alternative version can be sourced from Netgear's WNDR4700 GPL
archives.

The release notes of the PP2U-E's "Firmware Download" ver 2.0.1.3
(2012-06-15) state that the firmware is for the following devices:
 - uPD720201 ES 2.0 sample whose revision ID is 2.
 - uPD720201 ES 2.1 sample & CS sample & Mass product, ID is 3.
 - uPD720202 ES 2.0 sample & CS sample & Mass product, ID is 2.

Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[vkoul: fixed comments:
	used macros for timeout count and delay
	removed renesas_fw_alive_check
	cleaned renesas_fw_callback
	removed recurion for renesas_fw_download
	added MODULE_FIRMWARE
	add register defines and field names
	move to a separate driver]
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/usb/host/Kconfig            |   9 +
 drivers/usb/host/Makefile           |   1 +
 drivers/usb/host/xhci-pci-renesas.c | 557 ++++++++++++++++++++++++++++
 3 files changed, 567 insertions(+)
 create mode 100644 drivers/usb/host/xhci-pci-renesas.c

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 8d730180db06..77466b51bc7c 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -42,6 +42,15 @@ config USB_XHCI_PCI
 	depends on USB_PCI
 	default y
 
+config USB_XHCI_PCI_RENESAS
+	tristate "Renesas USB XHCI Driver"
+	depends on USB_XHCI_PCI
+	---help---
+	  Say 'Y' to enable the support for renesas USB XHCI Driver if
+	  you have such a device. These devices need additional firmware,
+	  make sure that is available.
+	  If unsure, say 'N'.
+
 config USB_XHCI_PLATFORM
 	tristate "Generic xHCI driver for a platform device"
 	select USB_XHCI_RCAR if ARCH_RENESAS
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index b191361257cc..b6955d5235b0 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)	+= ohci-da8xx.o
 obj-$(CONFIG_USB_UHCI_HCD)	+= uhci-hcd.o
 obj-$(CONFIG_USB_FHCI_HCD)	+= fhci.o
 obj-$(CONFIG_USB_XHCI_HCD)	+= xhci-hcd.o
+obj-$(CONFIG_USB_XHCI_PCI_RENESAS)	+= xhci-pci-renesas.o
 obj-$(CONFIG_USB_XHCI_PCI)	+= xhci-pci.o
 obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
 obj-$(CONFIG_USB_XHCI_HISTB)	+= xhci-histb.o
diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
new file mode 100644
index 000000000000..110c914ea7ef
--- /dev/null
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -0,0 +1,557 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2019-2020 Linaro Limited */
+
+#include <linux/acpi.h>
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/unaligned/access_ok.h>
+
+#include "xhci.h"
+#include "xhci-trace.h"
+#include "xhci-pci.h"
+
+#define RENESAS_FW_VERSION				0x6C
+#define RENESAS_ROM_CONFIG				0xF0
+#define RENESAS_FW_STATUS				0xF4
+#define RENESAS_FW_STATUS_MSB				0xF5
+#define RENESAS_ROM_STATUS				0xF6
+#define RENESAS_ROM_STATUS_MSB				0xF7
+#define RENESAS_DATA0					0xF8
+#define RENESAS_DATA1					0xFC
+
+#define RENESAS_FW_VERSION_FIELD			GENMASK(23, 7)
+#define RENESAS_FW_VERSION_OFFSET			8
+
+#define RENESAS_FW_STATUS_DOWNLOAD_ENABLE		BIT(0)
+#define RENESAS_FW_STATUS_LOCK				BIT(1)
+#define RENESAS_FW_STATUS_RESULT			GENMASK(6, 4)
+  #define RENESAS_FW_STATUS_INVALID			0
+  #define RENESAS_FW_STATUS_SUCCESS			BIT(4)
+  #define RENESAS_FW_STATUS_ERROR			BIT(5)
+#define RENESAS_FW_STATUS_SET_DATA0			BIT(8)
+#define RENESAS_FW_STATUS_SET_DATA1			BIT(9)
+
+#define RENESAS_RETRY	10000
+#define RENESAS_DELAY	10
+
+static struct hc_driver __read_mostly xhci_pci_hc_driver;
+
+static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
+	.reset = xhci_pci_setup,
+};
+
+static const struct renesas_fw_entry {
+	const char *firmware_name;
+	u16 device;
+	u8 revision;
+	u16 expected_version;
+} renesas_fw_table[] = {
+	/*
+	 * Only the uPD720201K8-711-BAC-A or uPD720202K8-711-BAA-A
+	 * are listed in R19UH0078EJ0500 Rev.5.00 as devices which
+	 * need the software loader.
+	 *
+	 * PP2U/ReleaseNote_USB3-201-202-FW.txt:
+	 *
+	 * Note: This firmware is for the following devices.
+	 *  - uPD720201 ES 2.0 sample whose revision ID is 2.
+	 *  - uPD720201 ES 2.1 sample & CS sample & Mass product, ID is 3.
+	 *  - uPD720202 ES 2.0 sample & CS sample & Mass product, ID is 2.
+	 */
+	{ "K2013080.mem", 0x0014, 0x02, 0x2013 },
+	{ "K2013080.mem", 0x0014, 0x03, 0x2013 },
+	{ "K2013080.mem", 0x0015, 0x02, 0x2013 },
+};
+
+MODULE_FIRMWARE("K2013080.mem");
+
+static const struct renesas_fw_entry *renesas_needs_fw_dl(struct pci_dev *dev)
+{
+	const struct renesas_fw_entry *entry;
+	size_t i;
+
+	/* This loader will only work with a RENESAS device. */
+	if (!(dev->vendor == PCI_VENDOR_ID_RENESAS))
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(renesas_fw_table); i++) {
+		entry = &renesas_fw_table[i];
+		if (entry->device == dev->device &&
+		    entry->revision == dev->revision)
+			return entry;
+	}
+
+	return NULL;
+}
+
+static int renesas_fw_download_image(struct pci_dev *dev,
+				     const u32 *fw,
+				     size_t step)
+{
+	size_t i;
+	int err;
+	u8 fw_status;
+	bool data0_or_data1;
+
+	/*
+	 * The hardware does alternate between two 32-bit pages.
+	 * (This is because each row of the firmware is 8 bytes).
+	 *
+	 * for even steps we use DATA0, for odd steps DATA1.
+	 */
+	data0_or_data1 = (step & 1) == 1;
+
+	/* step+1. Read "Set DATAX" and confirm it is cleared. */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(dev, RENESAS_FW_STATUS_MSB,
+					   &fw_status);
+		if (err)
+			return pcibios_err_to_errno(err);
+		if (!(fw_status & BIT(data0_or_data1)))
+			break;
+
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY)
+		return -ETIMEDOUT;
+
+	/*
+	 * step+2. Write FW data to "DATAX".
+	 * "LSB is left" => force little endian
+	 */
+	err = pci_write_config_dword(dev, data0_or_data1 ?
+				     RENESAS_DATA1 : RENESAS_DATA0,
+				     (__force u32)cpu_to_le32(fw[step]));
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	udelay(100);
+
+	/* step+3. Set "Set DATAX". */
+	err = pci_write_config_byte(dev, RENESAS_FW_STATUS_MSB,
+				    BIT(data0_or_data1));
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	return 0;
+}
+
+static int renesas_fw_verify(struct pci_dev *dev,
+			     const void *fw_data,
+			     size_t length)
+{
+	const struct renesas_fw_entry *entry = renesas_needs_fw_dl(dev);
+	u16 fw_version_pointer;
+	u16 fw_version;
+
+	if (!entry)
+		return -EINVAL;
+
+	/*
+	 * The Firmware's Data Format is describe in
+	 * "6.3 Data Format" R19UH0078EJ0500 Rev.5.00 page 124
+	 */
+
+	/*
+	 * The bootrom chips of the big brother have sizes up to 64k, let's
+	 * assume that's the biggest the firmware can get.
+	 */
+	if (length < 0x1000 || length >= 0x10000) {
+		dev_err(&dev->dev, "firmware is size %zd is not (4k - 64k).",
+			length);
+		return -EINVAL;
+	}
+
+	/* The First 2 bytes are fixed value (55aa). "LSB on Left" */
+	if (get_unaligned_le16(fw_data) != 0x55aa) {
+		dev_err(&dev->dev, "no valid firmware header found.");
+		return -EINVAL;
+	}
+
+	/* verify the firmware version position and print it. */
+	fw_version_pointer = get_unaligned_le16(fw_data + 4);
+	if (fw_version_pointer + 2 >= length) {
+		dev_err(&dev->dev,
+			"firmware version pointer is outside of the firmware image.");
+		return -EINVAL;
+	}
+
+	fw_version = get_unaligned_le16(fw_data + fw_version_pointer);
+	dev_dbg(&dev->dev, "got firmware version: %02x.", fw_version);
+
+	if (fw_version != entry->expected_version) {
+		dev_err(&dev->dev,
+			"firmware version mismatch, expected version: %02x.",
+			entry->expected_version);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int renesas_fw_check_running(struct pci_dev *pdev)
+{
+	int err;
+	u8 fw_state;
+
+	/*
+	 * Test if the device is actually needing the firmware. As most
+	 * BIOSes will initialize the device for us. If the device is
+	 * initialized.
+	 */
+	err = pci_read_config_byte(pdev, RENESAS_FW_STATUS, &fw_state);
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	/*
+	 * Check if "FW Download Lock" is locked. If it is and the FW is
+	 * ready we can simply continue. If the FW is not ready, we have
+	 * to give up.
+	 */
+	if (fw_state & RENESAS_FW_STATUS_LOCK) {
+		dev_dbg(&pdev->dev, "FW Download Lock is engaged.");
+
+		if (fw_state & RENESAS_FW_STATUS_SUCCESS)
+			return 0;
+
+		dev_err(&pdev->dev,
+			"FW Download Lock is set and FW is not ready. Giving Up.");
+		return -EIO;
+	}
+
+	/*
+	 * Check if "FW Download Enable" is set. If someone (us?) tampered
+	 * with it and it can't be resetted, we have to give up too... and
+	 * ask for a forgiveness and a reboot.
+	 */
+	if (fw_state & RENESAS_FW_STATUS_DOWNLOAD_ENABLE) {
+		dev_err(&pdev->dev,
+			"FW Download Enable is stale. Giving Up (poweroff/reboot needed).");
+		return -EIO;
+	}
+
+	/* Otherwise, Check the "Result Code" Bits (6:4) and act accordingly */
+	switch (fw_state & RENESAS_FW_STATUS_RESULT) {
+	case 0: /* No result yet */
+		dev_dbg(&pdev->dev, "FW is not ready/loaded yet.");
+
+		/* tell the caller, that this device needs the firmware. */
+		return 1;
+
+	case RENESAS_FW_STATUS_SUCCESS: /* Success, device should be working. */
+		dev_dbg(&pdev->dev, "FW is ready.");
+		return 0;
+
+	case RENESAS_FW_STATUS_ERROR: /* Error State */
+		dev_err(&pdev->dev,
+			"hardware is in an error state. Giving up (poweroff/reboot needed).");
+		return -ENODEV;
+
+	default: /* All other states are marked as "Reserved states" */
+		dev_err(&pdev->dev,
+			"hardware is in an invalid state %lx. Giving up (poweroff/reboot needed).",
+			(fw_state & RENESAS_FW_STATUS_RESULT) >> 4);
+		return -EINVAL;
+	}
+}
+
+static int renesas_fw_download(struct pci_dev *pdev,
+			       const struct firmware *fw)
+{
+	const u32 *fw_data = (const u32 *)fw->data;
+	size_t i;
+	int err;
+	u8 fw_status;
+
+	/*
+	 * For more information and the big picture: please look at the
+	 * "Firmware Download Sequence" in "7.1 FW Download Interface"
+	 * of R19UH0078EJ0500 Rev.5.00 page 131
+	 */
+
+	/*
+	 * 0. Set "FW Download Enable" bit in the
+	 * "FW Download Control & Status Register" at 0xF4
+	 */
+	err = pci_write_config_byte(pdev, RENESAS_FW_STATUS,
+				    RENESAS_FW_STATUS_DOWNLOAD_ENABLE);
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	/* 1 - 10 follow one step after the other. */
+	for (i = 0; i < fw->size / 4; i++) {
+		err = renesas_fw_download_image(pdev, fw_data, i);
+		if (err) {
+			dev_err(&pdev->dev,
+				"Firmware Download Step %zd failed at position %zd bytes with (%d).",
+				i, i * 4, err);
+			return err;
+		}
+	}
+
+	/*
+	 * This sequence continues until the last data is written to
+	 * "DATA0" or "DATA1". Naturally, we wait until "SET DATA0/1"
+	 * is cleared by the hardware beforehand.
+	 */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(pdev, RENESAS_FW_STATUS_MSB,
+					   &fw_status);
+		if (err)
+			return pcibios_err_to_errno(err);
+		if (!(fw_status & (BIT(0) | BIT(1))))
+			break;
+
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY)
+		dev_warn(&pdev->dev, "Final Firmware Download step timed out.");
+
+	/*
+	 * 11. After finishing writing the last data of FW, the
+	 * System Software must clear "FW Download Enable"
+	 */
+	err = pci_write_config_byte(pdev, RENESAS_FW_STATUS, 0);
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	/* 12. Read "Result Code" and confirm it is good. */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(pdev, RENESAS_FW_STATUS, &fw_status);
+		if (err)
+			return pcibios_err_to_errno(err);
+		if (fw_status & RENESAS_FW_STATUS_SUCCESS)
+			break;
+
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY) {
+		/* Timed out / Error - let's see if we can fix this */
+		err = renesas_fw_check_running(pdev);
+		switch (err) {
+		case 0: /*
+			 * we shouldn't end up here.
+			 * maybe it took a little bit longer.
+			 * But all should be well?
+			 */
+			break;
+
+		case 1: /* (No result yet! */
+			return -ETIMEDOUT;
+
+		default:
+			return err;
+		}
+	}
+	/*
+	 * Optional last step: Engage Firmware Lock
+	 *
+	 * err = pci_write_config_byte(pdev, 0xF4, BIT(2));
+	 * if (err)
+	 *	return pcibios_err_to_errno(err);
+	 */
+
+	return 0;
+}
+
+struct renesas_fw_ctx {
+	struct pci_dev *pdev;
+	const struct pci_device_id *id;
+	bool resume;
+	const struct renesas_fw_entry *entry;
+};
+
+static void renesas_fw_callback(const struct firmware *fw,
+				void *context)
+{
+	struct renesas_fw_ctx *ctx = context;
+	struct pci_dev *pdev = ctx->pdev;
+	struct device *parent = pdev->dev.parent;
+	int err;
+
+	if (!fw) {
+		dev_err(&pdev->dev, "firmware failed to load\n");
+
+		goto cleanup;
+	}
+
+	err = renesas_fw_verify(pdev, fw->data, fw->size);
+	if (err)
+		goto cleanup;
+
+	err = renesas_fw_download(pdev, fw);
+	release_firmware(fw);
+	if (err) {
+		dev_err(&pdev->dev, "firmware failed to download (%d).", err);
+		goto cleanup;
+	}
+
+	if (ctx->resume)
+		return;
+
+	err = xhci_pci_probe(pdev, ctx->id);
+	if (!err) {
+		/* everything worked */
+		devm_kfree(&pdev->dev, ctx);
+		return;
+	}
+
+cleanup:
+	/* in case of an error - fall through */
+	dev_info(&pdev->dev, "Unloading driver");
+
+	if (parent)
+		device_lock(parent);
+
+	device_release_driver(&pdev->dev);
+
+	if (parent)
+		device_unlock(parent);
+
+	pci_dev_put(pdev);
+}
+
+static int renesas_fw_alive_check(struct pci_dev *pdev)
+{
+	const struct renesas_fw_entry *entry;
+
+	/* check if we have a eligible RENESAS' uPD720201/2 w/o FW. */
+	entry = renesas_needs_fw_dl(pdev);
+	if (!entry)
+		return 0;
+
+	return renesas_fw_check_running(pdev);
+}
+
+static int renesas_fw_download_to_hw(struct pci_dev *pdev,
+				     const struct pci_device_id *id,
+				     bool do_resume)
+{
+	const struct renesas_fw_entry *entry;
+	struct renesas_fw_ctx *ctx;
+	int err;
+
+	/* check if we have a eligible RENESAS' uPD720201/2 w/o FW. */
+	entry = renesas_needs_fw_dl(pdev);
+	if (!entry)
+		return 0;
+
+	err = renesas_fw_check_running(pdev);
+	/* Continue ahead, if the firmware is already running. */
+	if (err == 0)
+		return 0;
+
+	if (err != 1)
+		return err;
+
+	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	ctx->pdev = pdev;
+	ctx->resume = do_resume;
+	ctx->id = id;
+	ctx->entry = entry;
+
+	pci_dev_get(pdev);
+	err = request_firmware_nowait(THIS_MODULE, 1, entry->firmware_name,
+				      &pdev->dev, GFP_KERNEL,
+				      ctx, renesas_fw_callback);
+	if (err) {
+		pci_dev_put(pdev);
+		return err;
+	}
+
+	/*
+	 * The renesas_fw_callback() callback will continue the probe
+	 * process, once it aquires the firmware.
+	 */
+	return 1;
+}
+
+static int renesas_xhci_pci_probe(struct pci_dev *dev,
+				  const struct pci_device_id *id)
+{
+	int retval;
+
+	/*
+	 * Check if this device is a RENESAS uPD720201/2 device.
+	 * Otherwise, we can continue with xhci_pci_probe as usual.
+	 */
+	retval = renesas_fw_download_to_hw(dev, id, false);
+	switch (retval) {
+	case 0:
+		break;
+
+	case 1: /* let it load the firmware and recontinue the probe. */
+		return 0;
+
+	default:
+		return retval;
+	};
+
+	return xhci_pci_probe(dev, id);
+}
+
+static void renesas_xhci_pci_remove(struct pci_dev *dev)
+{
+	if (renesas_fw_alive_check(dev)) {
+		/*
+		 * bail out early, if this was a renesas device w/o FW.
+		 * Else we might hit the NMI watchdog in xhci_handsake
+		 * during xhci_reset as part of the driver's unloading.
+		 * which we forced in the renesas_fw_callback().
+		 */
+		return;
+	}
+
+	xhci_pci_remove(dev);
+}
+
+static const struct pci_device_id pci_ids[] = {
+	{ PCI_DEVICE(0x1912, 0x0014),
+		.driver_data =	(unsigned long)&xhci_pci_hc_driver,
+	},
+	{ PCI_DEVICE(0x1912, 0x0015),
+		.driver_data =	(unsigned long)&xhci_pci_hc_driver,
+	},
+	{ /* sentinal */ }
+};
+MODULE_DEVICE_TABLE(pci, pci_ids);
+
+static struct pci_driver renesas_xhci_pci_driver = {
+	.name =		"renesas xhci",
+	.id_table =	pci_ids,
+
+	.probe =	renesas_xhci_pci_probe,
+	.remove =	renesas_xhci_pci_remove,
+	/* suspend and resume implemented later */
+
+	.shutdown =	usb_hcd_pci_shutdown,
+#ifdef CONFIG_PM
+	.driver = {
+		.pm = &usb_hcd_pci_pm_ops
+	},
+#endif
+};
+
+static int __init xhci_pci_init(void)
+{
+	xhci_init_driver(&xhci_pci_hc_driver, &xhci_pci_overrides);
+#ifdef CONFIG_PM
+	xhci_pci_hc_driver.pci_suspend = xhci_pci_suspend;
+	xhci_pci_hc_driver.pci_resume = xhci_pci_resume;
+#endif
+	return pci_register_driver(&renesas_xhci_pci_driver);
+}
+module_init(xhci_pci_init);
+
+static void __exit xhci_pci_exit(void)
+{
+	pci_unregister_driver(&renesas_xhci_pci_driver);
+}
+module_exit(xhci_pci_exit);
+
+MODULE_DESCRIPTION("xHCI PCI Host Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.24.1

[PATCH v6 3/5] usb: renesas-xhci: Add ROM loader for uPD720201

[Vinod Koul]

uPD720201 supports ROM and allows software to program the ROM and boot
from it. Add support for detecting if ROM is present, if so load the ROM
if not programmed earlier.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/usb/host/xhci-pci-renesas.c | 352 ++++++++++++++++++++++++++++
 1 file changed, 352 insertions(+)

diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index 110c914ea7ef..1d073c5637c4 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -33,6 +33,20 @@
 #define RENESAS_FW_STATUS_SET_DATA0			BIT(8)
 #define RENESAS_FW_STATUS_SET_DATA1			BIT(9)
 
+#define RENESAS_ROM_STATUS_ACCESS			BIT(0)
+#define RENESAS_ROM_STATUS_ERASE			BIT(1)
+#define RENESAS_ROM_STATUS_RELOAD			BIT(2)
+#define RENESAS_ROM_STATUS_RESULT			GENMASK(6, 4)
+  #define RENESAS_ROM_STATUS_INVALID			0
+  #define RENESAS_ROM_STATUS_SUCCESS			BIT(4)
+  #define RENESAS_ROM_STATUS_ERROR			BIT(5)
+#define RENESAS_ROM_STATUS_SET_DATA0			BIT(8)
+#define RENESAS_ROM_STATUS_SET_DATA1			BIT(9)
+#define RENESAS_ROM_STATUS_ROM_EXISTS			BIT(15)
+
+#define RENESAS_ROM_ERASE_MAGIC				0x5A65726F
+#define RENESAS_ROM_WRITE_MAGIC				0x53524F4D
+
 #define RENESAS_RETRY	10000
 #define RENESAS_DELAY	10
 
@@ -190,12 +204,81 @@ static int renesas_fw_verify(struct pci_dev *dev,
 
 	return 0;
 }
+static int renesas_check_rom_state(struct pci_dev *pdev)
+{
+	const struct renesas_fw_entry *entry;
+	u16 rom_state;
+	u32 version;
+	bool valid_version = false;
+	int err, i;
+
+	/* check FW version */
+	err = pci_read_config_dword(pdev, RENESAS_FW_VERSION, &version);
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	version &= RENESAS_FW_VERSION_FIELD;
+	version = version >> RENESAS_FW_VERSION_OFFSET;
+	dev_dbg(&pdev->dev, "Found FW version loaded is %x\n", version);
+
+	/* treat version in renesas_fw_table as correct ones */
+	for (i = 0; i < ARRAY_SIZE(renesas_fw_table); i++) {
+		entry = &renesas_fw_table[i];
+		if (version == entry->expected_version) {
+			dev_dbg(&pdev->dev, "Detected valid ROM version..\n");
+			valid_version = true;
+		}
+	}
+	if (valid_version == false)
+		dev_dbg(&pdev->dev, "Didn't find valid ROM version\n");
+
+	/*
+	 * Test if ROM is present and loaded, if so we can skip everything
+	 */
+	err = pci_read_config_word(pdev, RENESAS_ROM_STATUS, &rom_state);
+	if (err)
+		return pcibios_err_to_errno(err);
+
+	if (rom_state & BIT(15)) {
+		/* ROM exists */
+		dev_dbg(&pdev->dev, "ROM exists\n");
+
+		/* Check the "Result Code" Bits (6:4) and act accordingly */
+		switch (rom_state & RENESAS_ROM_STATUS_RESULT) {
+		case RENESAS_ROM_STATUS_SUCCESS:
+			dev_dbg(&pdev->dev, "Success ROM load...");
+			/* we have valid version and status so success */
+			if (valid_version)
+				return 0;
+			break;
+
+		case RENESAS_ROM_STATUS_INVALID: /* No result yet */
+			dev_dbg(&pdev->dev, "No result as it is ROM...");
+			/* we have valid version and status so success */
+			if (valid_version)
+				return 0;
+			break;
+
+		case RENESAS_ROM_STATUS_ERROR: /* Error State */
+		default: /* All other states are marked as "Reserved states" */
+			dev_err(&pdev->dev, "Invalid ROM..");
+			break;
+		}
+	}
+
+	return -EIO;
+}
 
 static int renesas_fw_check_running(struct pci_dev *pdev)
 {
 	int err;
 	u8 fw_state;
 
+	/* Check if device has ROM and loaded, if so skip everything */
+	err = renesas_check_rom_state(pdev);
+	if (!err)
+		return err;
+
 	/*
 	 * Test if the device is actually needing the firmware. As most
 	 * BIOSes will initialize the device for us. If the device is
@@ -363,12 +446,261 @@ struct renesas_fw_ctx {
 	const struct renesas_fw_entry *entry;
 };
 
+static bool renesas_check_rom(struct pci_dev *pdev)
+{
+	u16 rom_status;
+	int retval;
+
+	/* 1. Check if external ROM exists */
+	retval = pci_read_config_word(pdev, RENESAS_ROM_STATUS, &rom_status);
+	if (retval)
+		return false;
+
+	rom_status &= RENESAS_ROM_STATUS_ROM_EXISTS;
+	if (rom_status) {
+		dev_dbg(&pdev->dev, "External ROM exists\n");
+		return true; /* External ROM exists */
+	}
+
+	return false;
+}
+
+static void renesas_rom_erase(struct pci_dev *pdev)
+{
+	int retval, i;
+	u8 status;
+
+	dev_dbg(&pdev->dev, "Performing ROM Erase...\n");
+	retval = pci_write_config_dword(pdev, RENESAS_DATA0,
+					RENESAS_ROM_ERASE_MAGIC);
+	if (retval) {
+		dev_err(&pdev->dev, "ROM erase, magic word write failed: %d\n",
+			pcibios_err_to_errno(retval));
+		return;
+	}
+
+	retval = pci_read_config_byte(pdev, RENESAS_ROM_STATUS, &status);
+	if (retval) {
+		dev_err(&pdev->dev, "ROM status read failed: %d\n",
+			pcibios_err_to_errno(retval));
+		return;
+	}
+	status |= RENESAS_ROM_STATUS_ERASE;
+	retval = pci_write_config_byte(pdev, RENESAS_ROM_STATUS, status);
+	if (retval) {
+		dev_err(&pdev->dev, "ROM erase set word write failed\n");
+		return;
+	}
+
+	/* sleep a bit while ROM is erased */
+	msleep(20);
+
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		retval = pci_read_config_byte(pdev, RENESAS_ROM_STATUS,
+					      &status);
+		status &= RENESAS_ROM_STATUS_ERASE;
+		if (!status)
+			break;
+
+		mdelay(RENESAS_DELAY);
+	}
+
+	if (i == RENESAS_RETRY)
+		dev_dbg(&pdev->dev, "Chip erase timedout: %x\n", status);
+
+	dev_dbg(&pdev->dev, "ROM Erase... Done success\n");
+}
+
+static bool renesas_download_rom(struct pci_dev *pdev,
+				 const u32 *fw, size_t step)
+{
+	bool data0_or_data1;
+	u8 fw_status;
+	size_t i;
+	int err;
+
+	/*
+	 * The hardware does alternate between two 32-bit pages.
+	 * (This is because each row of the firmware is 8 bytes).
+	 *
+	 * for even steps we use DATA0, for odd steps DATA1.
+	 */
+	data0_or_data1 = (step & 1) == 1;
+
+	/* Read "Set DATAX" and confirm it is cleared. */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(pdev, RENESAS_ROM_STATUS_MSB,
+					   &fw_status);
+		if (err) {
+			dev_err(&pdev->dev, "Read ROM Status failed: %d\n",
+				pcibios_err_to_errno(err));
+			return false;
+		}
+		if (!(fw_status & BIT(data0_or_data1)))
+			break;
+
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY) {
+		dev_err(&pdev->dev, "Timeout for Set DATAX step: %zd\n", step);
+		return false;
+	}
+
+	/*
+	 * Write FW data to "DATAX".
+	 * "LSB is left" => force little endian
+	 */
+	err = pci_write_config_dword(pdev, data0_or_data1 ?
+				     RENESAS_DATA1 : RENESAS_DATA0,
+				     (__force u32)cpu_to_le32(fw[step]));
+	if (err) {
+		dev_err(&pdev->dev, "Write to DATAX failed: %d\n",
+			pcibios_err_to_errno(err));
+		return false;
+	}
+
+	udelay(100);
+
+	/* Set "Set DATAX". */
+	err = pci_write_config_byte(pdev, RENESAS_ROM_STATUS_MSB,
+				    BIT(data0_or_data1));
+	if (err) {
+		dev_err(&pdev->dev, "Write config for DATAX failed: %d\n",
+			pcibios_err_to_errno(err));
+		return false;
+	}
+
+	return true;
+}
+
+static bool renesas_setup_rom(struct pci_dev *pdev, const struct firmware *fw)
+{
+	const u32 *fw_data = (const u32 *)fw->data;
+	int err, i;
+	u8 status;
+
+	/* 2. Write magic word to Data0 */
+	err = pci_write_config_dword(pdev, RENESAS_DATA0,
+				     RENESAS_ROM_WRITE_MAGIC);
+	if (err)
+		return false;
+
+	/* 3. Set External ROM access */
+	err = pci_write_config_byte(pdev, RENESAS_ROM_STATUS,
+				    RENESAS_ROM_STATUS_ACCESS);
+	if (err)
+		goto remove_bypass;
+
+	/* 4. Check the result */
+	err = pci_read_config_byte(pdev, RENESAS_ROM_STATUS, &status);
+	if (err)
+		goto remove_bypass;
+	status &= GENMASK(6, 4);
+	if (status) {
+		dev_err(&pdev->dev,
+			"setting external rom failed: %x\n", status);
+		goto remove_bypass;
+	}
+
+	/* 5 to 16 Write FW to DATA0/1 while checking SetData0/1 */
+	for (i = 0; i < fw->size / 4; i++) {
+		err = renesas_download_rom(pdev, fw_data, i);
+		if (!err) {
+			dev_err(&pdev->dev,
+				"ROM Download Step %d failed at position %d bytes\n",
+				 i, i * 4);
+			goto remove_bypass;
+		}
+	}
+
+	/*
+	 * wait till DATA0/1 is cleared
+	 */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(pdev, RENESAS_ROM_STATUS_MSB,
+					   &status);
+		if (err)
+			goto remove_bypass;
+		if (!(status & (BIT(0) | BIT(1))))
+			break;
+
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY) {
+		dev_err(&pdev->dev, "Final Firmware ROM Download step timed out\n");
+		goto remove_bypass;
+	}
+
+	/* 17. Remove bypass */
+	err = pci_write_config_byte(pdev, RENESAS_ROM_STATUS, 0);
+	if (err)
+		return false;
+
+	udelay(10);
+
+	/* 18. check result */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(pdev, RENESAS_ROM_STATUS, &status);
+		if (err) {
+			dev_err(&pdev->dev, "Read ROM status failed:%d\n",
+				pcibios_err_to_errno(err));
+			return false;
+		}
+		status &= RENESAS_ROM_STATUS_RESULT;
+		if (status ==  RENESAS_ROM_STATUS_SUCCESS) {
+			dev_dbg(&pdev->dev, "Download ROM success\n");
+			break;
+		}
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY) { /* Timed out */
+		dev_err(&pdev->dev,
+			"Download to external ROM TO: %x\n", status);
+		return false;
+	}
+
+	dev_dbg(&pdev->dev, "Download to external ROM scuceeded\n");
+
+	/* Last step set Reload */
+	err = pci_write_config_byte(pdev, RENESAS_ROM_STATUS,
+				    RENESAS_ROM_STATUS_RELOAD);
+	if (err) {
+		dev_err(&pdev->dev, "Set ROM execute failed: %d\n",
+			pcibios_err_to_errno(err));
+		return false;
+	}
+
+	/*
+	 * wait till Reload is cleared
+	 */
+	for (i = 0; i < RENESAS_RETRY; i++) {
+		err = pci_read_config_byte(pdev, RENESAS_ROM_STATUS, &status);
+		if (err)
+			return false;
+		if (!(status & RENESAS_ROM_STATUS_RELOAD))
+			break;
+
+		udelay(RENESAS_DELAY);
+	}
+	if (i == RENESAS_RETRY) {
+		dev_err(&pdev->dev, "ROM Exec timed out: %x\n", status);
+		return false;
+	}
+
+	return true;
+
+remove_bypass:
+	pci_write_config_byte(pdev, RENESAS_ROM_STATUS, 0);
+	return false;
+}
+
 static void renesas_fw_callback(const struct firmware *fw,
 				void *context)
 {
 	struct renesas_fw_ctx *ctx = context;
 	struct pci_dev *pdev = ctx->pdev;
 	struct device *parent = pdev->dev.parent;
+	bool rom;
 	int err;
 
 	if (!fw) {
@@ -381,6 +713,25 @@ static void renesas_fw_callback(const struct firmware *fw,
 	if (err)
 		goto cleanup;
 
+	/* Check if the device has external ROM */
+	rom = renesas_check_rom(pdev);
+	if (rom) {
+		/* perfrom chip erase first */
+		renesas_rom_erase(pdev);
+
+		/* lets try loading fw on ROM first */
+		rom = renesas_setup_rom(pdev, fw);
+		if (!rom) {
+			dev_err(&pdev->dev,
+				"ROM load failed, falling back on FW load\n");
+		} else {
+			dev_dbg(&pdev->dev, "ROM load done..\n");
+
+			release_firmware(fw);
+			goto do_probe;
+		}
+	}
+
 	err = renesas_fw_download(pdev, fw);
 	release_firmware(fw);
 	if (err) {
@@ -388,6 +739,7 @@ static void renesas_fw_callback(const struct firmware *fw,
 		goto cleanup;
 	}
 
+do_probe:
 	if (ctx->resume)
 		return;
 
-- 
2.24.1

[PATCH v6 4/5] usb: renesas-xhci: allow multiple firmware versions

[Vinod Koul]

Allow multiple firmware file versions in table and load them in
increasing order as we find them in the file system.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/usb/host/xhci-pci-renesas.c | 45 +++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index 1d073c5637c4..fe95487ca888 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -73,13 +73,20 @@ static const struct renesas_fw_entry {
 	 *  - uPD720201 ES 2.0 sample whose revision ID is 2.
 	 *  - uPD720201 ES 2.1 sample & CS sample & Mass product, ID is 3.
 	 *  - uPD720202 ES 2.0 sample & CS sample & Mass product, ID is 2.
+	 *
+	 *  Entry expected_version should be kept in decreasing order for a
+	 *  chip, so that driver will pick latest version and if that fails
+	 *  then next one will be picked
 	 */
 	{ "K2013080.mem", 0x0014, 0x02, 0x2013 },
+	{ "K2026090.mem", 0x0014, 0x03, 0x2026 },
 	{ "K2013080.mem", 0x0014, 0x03, 0x2013 },
+	{ "K2026090.mem", 0x0015, 0x02, 0x2026 },
 	{ "K2013080.mem", 0x0015, 0x02, 0x2013 },
 };
 
 MODULE_FIRMWARE("K2013080.mem");
+MODULE_FIRMWARE("K2026090.mem");
 
 static const struct renesas_fw_entry *renesas_needs_fw_dl(struct pci_dev *dev)
 {
@@ -100,6 +107,24 @@ static const struct renesas_fw_entry *renesas_needs_fw_dl(struct pci_dev *dev)
 	return NULL;
 }
 
+static const struct
+renesas_fw_entry *renesas_get_next_entry(struct pci_dev *dev,
+					 const struct renesas_fw_entry *entry)
+{
+	const struct renesas_fw_entry *next_entry;
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(renesas_fw_table); i++) {
+		next_entry = &renesas_fw_table[i];
+		if (next_entry->device == dev->device &&
+		    next_entry->revision == dev->revision &&
+		    next_entry->expected_version < entry->expected_version)
+			return next_entry;
+	}
+
+	return NULL;
+}
+
 static int renesas_fw_download_image(struct pci_dev *dev,
 				     const u32 *fw,
 				     size_t step)
@@ -700,13 +725,29 @@ static void renesas_fw_callback(const struct firmware *fw,
 	struct renesas_fw_ctx *ctx = context;
 	struct pci_dev *pdev = ctx->pdev;
 	struct device *parent = pdev->dev.parent;
+	const struct renesas_fw_entry *next_entry;
 	bool rom;
 	int err;
 
 	if (!fw) {
 		dev_err(&pdev->dev, "firmware failed to load\n");
-
-		goto cleanup;
+		/*
+		 * we didn't find firmware, check if we have another
+		 * entry for this device
+		 */
+		next_entry = renesas_get_next_entry(ctx->pdev, ctx->entry);
+		if (next_entry) {
+			ctx->entry = next_entry;
+			dev_dbg(&pdev->dev, "Found next entry, requesting: %s\n",
+				next_entry->firmware_name);
+			request_firmware_nowait(THIS_MODULE, 1,
+						next_entry->firmware_name,
+						&pdev->dev, GFP_KERNEL,
+						ctx, renesas_fw_callback);
+			return;
+		} else {
+			goto cleanup;
+		}
 	}
 
 	err = renesas_fw_verify(pdev, fw->data, fw->size);
-- 
2.24.1

[PATCH v6 5/5] usb: xhci: provide a debugfs hook for erasing rom

[Vinod Koul]

run "echo 1 > /sys/kernel/debug/renesas-usb/rom_erase" to erase firmware
when driver is loaded.

Subsequent init of driver shall reload the firmware

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/usb/host/xhci-pci-renesas.c | 35 +++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index fe95487ca888..be2e7eff492f 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -2,6 +2,7 @@
 /* Copyright (C) 2019-2020 Linaro Limited */
 
 #include <linux/acpi.h>
+#include <linux/debugfs.h>
 #include <linux/firmware.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -229,6 +230,9 @@ static int renesas_fw_verify(struct pci_dev *dev,
 
 	return 0;
 }
+
+static void debugfs_init(struct pci_dev *pdev);
+
 static int renesas_check_rom_state(struct pci_dev *pdev)
 {
 	const struct renesas_fw_entry *entry;
@@ -252,6 +256,7 @@ static int renesas_check_rom_state(struct pci_dev *pdev)
 		if (version == entry->expected_version) {
 			dev_dbg(&pdev->dev, "Detected valid ROM version..\n");
 			valid_version = true;
+			debugfs_init(pdev);
 		}
 	}
 	if (valid_version == false)
@@ -536,6 +541,34 @@ static void renesas_rom_erase(struct pci_dev *pdev)
 	dev_dbg(&pdev->dev, "ROM Erase... Done success\n");
 }
 
+static int debugfs_rom_erase(void *data, u64 value)
+{
+	struct pci_dev *pdev = data;
+
+	if (value == 1) {
+		dev_dbg(&pdev->dev, "Userspace requested ROM erase\n");
+		renesas_rom_erase(pdev);
+		return 0;
+	}
+	return -EINVAL;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(rom_erase_ops, NULL, debugfs_rom_erase, "%llu\n");
+
+static struct dentry *debugfs_root;
+
+static void debugfs_init(struct pci_dev *pdev)
+{
+	debugfs_root = debugfs_create_dir("renesas-usb", NULL);
+
+	debugfs_create_file("rom_erase", 0200, debugfs_root,
+			    pdev, &rom_erase_ops);
+}
+
+static void debugfs_exit(void)
+{
+	debugfs_remove_recursive(debugfs_root);
+}
+
 static bool renesas_download_rom(struct pci_dev *pdev,
 				 const u32 *fw, size_t step)
 {
@@ -889,6 +922,8 @@ static int renesas_xhci_pci_probe(struct pci_dev *dev,
 
 static void renesas_xhci_pci_remove(struct pci_dev *dev)
 {
+	debugfs_exit();
+
 	if (renesas_fw_alive_check(dev)) {
 		/*
 		 * bail out early, if this was a renesas device w/o FW.
-- 
2.24.1

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[John Stultz]

On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> This series add support for Renesas USB controllers uPD720201 and uPD720202.
> These require firmware to be loaded and in case devices have ROM those can
> also be programmed if empty. If ROM is programmed, it runs from ROM as well.
>
> This includes two patches from Christian which supported these controllers
> w/o ROM and later my patches for ROM support and multiple firmware versions,
> debugfs hook for rom erase and export of xhci-pci functions.
>

Thanks so much for updating these! They are working ok for me in my
testing on db845c.

Tested-by: John Stultz <john.stultz@linaro.org>

thanks again!
-john

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Christian Lamparter]

On Mon, Jan 13, 2020 at 9:10 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > These require firmware to be loaded and in case devices have ROM those can
> > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> >
> > This includes two patches from Christian which supported these controllers
> > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > debugfs hook for rom erase and export of xhci-pci functions.
> >
>
> Thanks so much for updating these! They are working ok for me in my
> testing on db845c.
>
> Tested-by: John Stultz <john.stultz@linaro.org>

Nice! I'll definitely give this series another try on my WNDR4700 too
(PowerPC Arch)
this weekend.

and from me: Thanks!

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Vinod Koul]

hey Christian,

On 13-01-20, 21:33, Christian Lamparter wrote:
> On Mon, Jan 13, 2020 at 9:10 PM John Stultz <john.stultz@linaro.org> wrote:
> >
> > On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > > These require firmware to be loaded and in case devices have ROM those can
> > > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> > >
> > > This includes two patches from Christian which supported these controllers
> > > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > > debugfs hook for rom erase and export of xhci-pci functions.
> > >
> >
> > Thanks so much for updating these! They are working ok for me in my
> > testing on db845c.
> >
> > Tested-by: John Stultz <john.stultz@linaro.org>
> 
> Nice! I'll definitely give this series another try on my WNDR4700 too
> (PowerPC Arch)
> this weekend.
> 
> and from me: Thanks!

Did you get around to test these?

-- 
~Vinod

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Christian Lamparter]

Hello,

On Tue, Jan 21, 2020 at 7:46 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> hey Christian,
>
> On 13-01-20, 21:33, Christian Lamparter wrote:
> > On Mon, Jan 13, 2020 at 9:10 PM John Stultz <john.stultz@linaro.org> wrote:
> > >
> > > On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> > > >
> > > > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > > > These require firmware to be loaded and in case devices have ROM those can
> > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> > > >
> > > > This includes two patches from Christian which supported these controllers
> > > > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > > > debugfs hook for rom erase and export of xhci-pci functions.
> > > >
> > >
> > > Thanks so much for updating these! They are working ok for me in my
> > > testing on db845c.
> > >
> > > Tested-by: John Stultz <john.stultz@linaro.org>
> >
> > Nice! I'll definitely give this series another try on my WNDR4700 too
> > (PowerPC Arch)
> > this weekend.
> >
> > and from me: Thanks!
>
> Did you get around to test these?

Not yet, I was too optimistic that I could get current linux-usb with the
patches running on the WNDR4700 (due to APM82181) over the
weekend. Do you think that It still counts, if I'm going with 5.4.11 on
OpenWrt instead? Because then I just swap out the old patches from
my OpenWrt APM821XX branch:
<https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=4dd6f62a36a3724f0363d639cd9e29e04d7b62c0>

and don't have to figure out what broke with linux-usb on the APM821xx.

Cheers,
Christian

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Christian Lamparter]

On Tuesday, 21 January 2020 21:26:34 CET Christian Lamparter wrote:
> Hello,
> 
> On Tue, Jan 21, 2020 at 7:46 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > hey Christian,
> >
> > On 13-01-20, 21:33, Christian Lamparter wrote:
> > > On Mon, Jan 13, 2020 at 9:10 PM John Stultz <john.stultz@linaro.org> wrote:
> > > >
> > > > On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> > > > >
> > > > > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > > > > These require firmware to be loaded and in case devices have ROM those can
> > > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> > > > >
> > > > > This includes two patches from Christian which supported these controllers
> > > > > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > > > > debugfs hook for rom erase and export of xhci-pci functions.
> > > > >
> > > >
> > > > Thanks so much for updating these! They are working ok for me in my
> > > > testing on db845c.
> > > >
> > > > Tested-by: John Stultz <john.stultz@linaro.org>
> > >
> > > Nice! I'll definitely give this series another try on my WNDR4700 too
> > > (PowerPC Arch)
> > > this weekend.
> > >
> > > and from me: Thanks!
> >
> > Did you get around to test these?
> 
> Not yet, I was too optimistic that I could get current linux-usb with the
> patches running on the WNDR4700 (due to APM82181) over the
> weekend. Do you think that It still counts, if I'm going with 5.4.11 on
> OpenWrt instead? Because then I just swap out the old patches from
> my OpenWrt APM821XX branch:
> <https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=4dd6f62a36a3724f0363d639cd9e29e04d7b62c0>
> 
> and don't have to figure out what broke with linux-usb on the APM821xx.

I could get 5.4.11 to boot on the Netgear WNDR4700 :-).
(This has a APM82181 SoC (PowerPC 464))

Here's faillog from the "plain xhci-pci" driver: 

[  375.481868] xhci_hcd 0000:45:00.0: xHCI Host Controller
[  375.487149] xhci_hcd 0000:45:00.0: new USB bus registered, assigned bus number 1
[  385.494590] xhci_hcd 0000:45:00.0: can't setup: -110
[  385.499558] xhci_hcd 0000:45:00.0: USB bus 1 deregistered
[  385.504963] xhci_hcd 0000:45:00.0: init 0000:45:00.0 fail, -110
[  385.510889] xhci_hcd: probe of 0000:45:00.0 failed with error -110

(Notice how it gets stuck for 10 seconds there).

And this is the successlog from the xhci-pci-renesas module

[  391.555559] renesas xhci 0000:45:00.0: xHCI Host Controller
[  391.561171] renesas xhci 0000:45:00.0: new USB bus registered, assigned bus number 1
[  391.575068] renesas xhci 0000:45:00.0: hcc params 0x014051cf hci version 0x100 quirks 0x0000000101000090
[  391.586750] hub 1-0:1.0: USB hub found
[  391.592601] hub 1-0:1.0: 2 ports detected
[  391.597199] renesas xhci 0000:45:00.0: xHCI Host Controller
[  391.602797] renesas xhci 0000:45:00.0: new USB bus registered, assigned bus number 2
[  391.610537] renesas xhci 0000:45:00.0: Host supports USB 3.0 SuperSpeed
[  391.617719] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
[  391.626495] hub 2-0:1.0: USB hub found
[  391.630570] hub 2-0:1.0: 2 ports detected

this is when I added the usb 3.0-stick:

[  775.403928] usb 2-2: new SuperSpeed Gen 1 USB device number 3 using renesas xhci
[  775.432684] usb-storage 2-2:1.0: USB Mass Storage device detected
[  775.439238] scsi host1: usb-storage 2-2:1.0
[  776.482556] scsi 1:0:0:0: Direct-Access     SanDisk  Ultra            1.00 PQ: 0 ANSI: 6
[  776.492181] sd 1:0:0:0: [sda] 60063744 512-byte logical blocks: (30.8 GB/28.6 GiB)
[  776.501193] sd 1:0:0:0: [sda] Write Protect is off
[  776.507047] sd 1:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
[  776.524893]  sda: sda1 sda2
[  776.531062] sd 1:0:0:0: [sda] Attached SCSI removable disk

root@(none):/dev# hdparm -t /dev/sda

/dev/sda:
 Timing buffered disk reads: 466 MB in  3.01 seconds = 154.98 MB/sec

and this is the log from my usb 2.0-memorystick:

[ 1187.113650] usb 2-2: USB disconnect, device number 3
[ 1195.867397] usb 1-2: new high-speed USB device number 2 using renesas xhci
[ 1195.895171] usb-storage 1-2:1.0: USB Mass Storage device detected
[ 1195.901848] scsi host1: usb-storage 1-2:1.0
[ 1196.962583] scsi 1:0:0:0: Direct-Access     SanDisk  Cruzer Blade     1.00 PQ: 0 ANSI: 6
[ 1196.978772] sd 1:0:0:0: [sda] 30031872 512-byte logical blocks: (15.4 GB/14.3 GiB)
[ 1196.988529] sd 1:0:0:0: [sda] Write Protect is off
[ 1196.994498] sd 1:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
[ 1197.020407]  sda: sda1
[ 1197.030458] sd 1:0:0:0: [sda] Attached SCSI removable disk

root@(none):/dev# hdparm -t /dev/sda

/dev/sda:
 Timing buffered disk reads:  64 MB in  3.01 seconds =  21.28 MB/sec

These speeds for usb3 and usb2 are within what the device can do.
So, everything is working fine with the v6.

Tested-by: Christian Lamparter <chunkeey@gmail.com>

Cheers,
Christian

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Vinod Koul]

On 24-01-20, 22:38, Christian Lamparter wrote:
> On Tuesday, 21 January 2020 21:26:34 CET Christian Lamparter wrote:
> > Hello,
> > 
> > On Tue, Jan 21, 2020 at 7:46 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > hey Christian,
> > >
> > > On 13-01-20, 21:33, Christian Lamparter wrote:
> > > > On Mon, Jan 13, 2020 at 9:10 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > >
> > > > > On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> > > > > >
> > > > > > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > > > > > These require firmware to be loaded and in case devices have ROM those can
> > > > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> > > > > >
> > > > > > This includes two patches from Christian which supported these controllers
> > > > > > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > > > > > debugfs hook for rom erase and export of xhci-pci functions.
> > > > > >
> > > > >
> > > > > Thanks so much for updating these! They are working ok for me in my
> > > > > testing on db845c.
> > > > >
> > > > > Tested-by: John Stultz <john.stultz@linaro.org>
> > > >
> > > > Nice! I'll definitely give this series another try on my WNDR4700 too
> > > > (PowerPC Arch)
> > > > this weekend.
> > > >
> > > > and from me: Thanks!
> > >
> > > Did you get around to test these?
> > 
> > Not yet, I was too optimistic that I could get current linux-usb with the
> > patches running on the WNDR4700 (due to APM82181) over the
> > weekend. Do you think that It still counts, if I'm going with 5.4.11 on
> > OpenWrt instead? Because then I just swap out the old patches from
> > my OpenWrt APM821XX branch:
> > <https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=4dd6f62a36a3724f0363d639cd9e29e04d7b62c0>
> > 
> > and don't have to figure out what broke with linux-usb on the APM821xx.
> 
> I could get 5.4.11 to boot on the Netgear WNDR4700 :-).
> (This has a APM82181 SoC (PowerPC 464))
> 
> Here's faillog from the "plain xhci-pci" driver: 
> 
> [  375.481868] xhci_hcd 0000:45:00.0: xHCI Host Controller
> [  375.487149] xhci_hcd 0000:45:00.0: new USB bus registered, assigned bus number 1
> [  385.494590] xhci_hcd 0000:45:00.0: can't setup: -110
> [  385.499558] xhci_hcd 0000:45:00.0: USB bus 1 deregistered
> [  385.504963] xhci_hcd 0000:45:00.0: init 0000:45:00.0 fail, -110
> [  385.510889] xhci_hcd: probe of 0000:45:00.0 failed with error -110
> 
> (Notice how it gets stuck for 10 seconds there).
> 
> And this is the successlog from the xhci-pci-renesas module
> 
> [  391.555559] renesas xhci 0000:45:00.0: xHCI Host Controller
> [  391.561171] renesas xhci 0000:45:00.0: new USB bus registered, assigned bus number 1
> [  391.575068] renesas xhci 0000:45:00.0: hcc params 0x014051cf hci version 0x100 quirks 0x0000000101000090
> [  391.586750] hub 1-0:1.0: USB hub found
> [  391.592601] hub 1-0:1.0: 2 ports detected
> [  391.597199] renesas xhci 0000:45:00.0: xHCI Host Controller
> [  391.602797] renesas xhci 0000:45:00.0: new USB bus registered, assigned bus number 2
> [  391.610537] renesas xhci 0000:45:00.0: Host supports USB 3.0 SuperSpeed
> [  391.617719] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
> [  391.626495] hub 2-0:1.0: USB hub found
> [  391.630570] hub 2-0:1.0: 2 ports detected
> 
> this is when I added the usb 3.0-stick:
> 
> [  775.403928] usb 2-2: new SuperSpeed Gen 1 USB device number 3 using renesas xhci
> [  775.432684] usb-storage 2-2:1.0: USB Mass Storage device detected
> [  775.439238] scsi host1: usb-storage 2-2:1.0
> [  776.482556] scsi 1:0:0:0: Direct-Access     SanDisk  Ultra            1.00 PQ: 0 ANSI: 6
> [  776.492181] sd 1:0:0:0: [sda] 60063744 512-byte logical blocks: (30.8 GB/28.6 GiB)
> [  776.501193] sd 1:0:0:0: [sda] Write Protect is off
> [  776.507047] sd 1:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
> [  776.524893]  sda: sda1 sda2
> [  776.531062] sd 1:0:0:0: [sda] Attached SCSI removable disk
> 
> root@(none):/dev# hdparm -t /dev/sda
> 
> /dev/sda:
>  Timing buffered disk reads: 466 MB in  3.01 seconds = 154.98 MB/sec
> 
> and this is the log from my usb 2.0-memorystick:
> 
> [ 1187.113650] usb 2-2: USB disconnect, device number 3
> [ 1195.867397] usb 1-2: new high-speed USB device number 2 using renesas xhci
> [ 1195.895171] usb-storage 1-2:1.0: USB Mass Storage device detected
> [ 1195.901848] scsi host1: usb-storage 1-2:1.0
> [ 1196.962583] scsi 1:0:0:0: Direct-Access     SanDisk  Cruzer Blade     1.00 PQ: 0 ANSI: 6
> [ 1196.978772] sd 1:0:0:0: [sda] 30031872 512-byte logical blocks: (15.4 GB/14.3 GiB)
> [ 1196.988529] sd 1:0:0:0: [sda] Write Protect is off
> [ 1196.994498] sd 1:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
> [ 1197.020407]  sda: sda1
> [ 1197.030458] sd 1:0:0:0: [sda] Attached SCSI removable disk
> 
> root@(none):/dev# hdparm -t /dev/sda
> 
> /dev/sda:
>  Timing buffered disk reads:  64 MB in  3.01 seconds =  21.28 MB/sec
> 
> These speeds for usb3 and usb2 are within what the device can do.
> So, everything is working fine with the v6.
> 
> Tested-by: Christian Lamparter <chunkeey@gmail.com>

Thanks a lot Christian for again testing this.

Mathias, any comments on this series..?

-- 
~Vinod

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Mathias Nyman]

On 25.1.2020 7.32, Vinod Koul wrote:
>>>>>>
>>>>>> On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
>>>>>>>
>>>>>>> This series add support for Renesas USB controllers uPD720201 and uPD720202.
>>>>>>> These require firmware to be loaded and in case devices have ROM those can
>>>>>>> also be programmed if empty. If ROM is programmed, it runs from ROM as well.
>>>>>>>
>>>>>>> This includes two patches from Christian which supported these controllers
>>>>>>> w/o ROM and later my patches for ROM support and multiple firmware versions,
>>>>>>> debugfs hook for rom erase and export of xhci-pci functions.
>>>>>>>
...
> 
> Mathias, any comments on this series..?
> 

Hi Vinod

Sorry about the delay.

Maybe a firmware loading driver like this that wraps the xhci pci driver could
work.

One benefit is that we could skip searching for the right firmware name based
on PCI ID. Each of these Renesas controllers now have their own pci_device_id
entry in the pci_ids[] table, and could have the supported firmware name(s)
in .driver_data. This way we wouldn't need to add the renesas_fw_table[] or
maybe even the renesas_needs_fw_dl() function in this series.

I realize this can't be easily changed because usb_hcd_pci_probe() takes the
pci_device_id pointer as an argument, and expects id.driver_data to be a
HC driver pointer.

So this turns out to be a question for Greg and Alan:

Would it make sense to change usb_hcd_pci_probe() to take a HC driver pointer
as an argument instead of a pointer to pci_device_id?
pci_device_id pointer is only used to extract the HC driver handle.
This way the driver_data could be used for, well, driver data.

Heikki actually suggested this some time ago to me, back then the idea was to
improve xhci quirks code by using driver_data for quirk flags instead of
finding and setting them later.

There are a few other opens regarding this series. Mostly because I'm not (yet)
familiar with all the details, so I'll just just list them here.

- Is it really enough to add the Renesas driver to Makefile before xhci-pci
   driver to make sure it gets matched and probed based on vendor/device id
   before xhci-pci driver is matched and probed based on pci class?
   What if the Renesas driver is a module and xhci-pci compiled in?

- Previously probe didn't return before hcd's were added and everything set up.
   Now with request_firmware_nowait() probe returns early successfully, and the
   old xhci_pci_probe() which sets up everything is called later by the request
   firmware callback. So there could be whole new set of races possible.
   For example pci remove can be called mid firmware loading, or when the old
   xhci_pci_probe is still setting up things.

   I understood that a synchronous request_firmware() in probe has its own
   issues, not sure if there is a good solution for this.

- Before the firmware is written to the controller the firmware version is
   compared against a hardcoded number in the drivers renesas_fw_table[].
   This means new firmware versions can't be supported without patching the driver.
   Is this intentional?

- Mathias

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Vinod Koul]

Hi Mathias,

On 30-01-20, 19:07, Mathias Nyman wrote:
> On 25.1.2020 7.32, Vinod Koul wrote:
> > > > > > > 
> > > > > > > On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
> > > > > > > > 
> > > > > > > > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > > > > > > > These require firmware to be loaded and in case devices have ROM those can
> > > > > > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> > > > > > > > 
> > > > > > > > This includes two patches from Christian which supported these controllers
> > > > > > > > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > > > > > > > debugfs hook for rom erase and export of xhci-pci functions.
> > > > > > > > 
> ...
> > 
> > Mathias, any comments on this series..?
> > 
> 
> Hi Vinod
> 
> Sorry about the delay.
> 
> Maybe a firmware loading driver like this that wraps the xhci pci driver could
> work.
> 
> One benefit is that we could skip searching for the right firmware name based
> on PCI ID. Each of these Renesas controllers now have their own pci_device_id
> entry in the pci_ids[] table, and could have the supported firmware name(s)
> in .driver_data. This way we wouldn't need to add the renesas_fw_table[] or
> maybe even the renesas_needs_fw_dl() function in this series.
> 
> I realize this can't be easily changed because usb_hcd_pci_probe() takes the
> pci_device_id pointer as an argument, and expects id.driver_data to be a
> HC driver pointer.
> 
> So this turns out to be a question for Greg and Alan:
> 
> Would it make sense to change usb_hcd_pci_probe() to take a HC driver pointer
> as an argument instead of a pointer to pci_device_id?
> pci_device_id pointer is only used to extract the HC driver handle.
> This way the driver_data could be used for, well, driver data.
> 
> Heikki actually suggested this some time ago to me, back then the idea was to
> improve xhci quirks code by using driver_data for quirk flags instead of
> finding and setting them later.
> 
> There are a few other opens regarding this series. Mostly because I'm not (yet)
> familiar with all the details, so I'll just just list them here.
> 
> - Is it really enough to add the Renesas driver to Makefile before xhci-pci
>   driver to make sure it gets matched and probed based on vendor/device id
>   before xhci-pci driver is matched and probed based on pci class?
>   What if the Renesas driver is a module and xhci-pci compiled in?

The driver loading rules are based on level and makefile order. So
renesas will always be loaded before xhci driver. This is required since
xhci claims all devices, so we need to make sure it loads before.

I think we should make it inbuilt driver rather than a module. xhci
driver is only inbuilt.

If there is a better way to handle this, am all for it.

> - Previously probe didn't return before hcd's were added and everything set up.
>   Now with request_firmware_nowait() probe returns early successfully, and the
>   old xhci_pci_probe() which sets up everything is called later by the request
>   firmware callback. So there could be whole new set of races possible.
>   For example pci remove can be called mid firmware loading, or when the old
>   xhci_pci_probe is still setting up things.

I think this is a valid concern. Let me think about how to solve this..
maybe add a flag in remove which ensure remove doesnt run untill the
probe/firmware callback is completed.

>   I understood that a synchronous request_firmware() in probe has its own
>   issues, not sure if there is a good solution for this.

Yeah with userspace not available, that can be tricky!

> - Before the firmware is written to the controller the firmware version is
>   compared against a hardcoded number in the drivers renesas_fw_table[].
>   This means new firmware versions can't be supported without patching the driver.
>   Is this intentional?

Yes, we have only bunch of validated versions. There maybe more in the
wild but we dont know. The vendor is not very helpful here :(

Across all folks using this, there seems to be only few versions and
vendor is not supporting it anymore, so chances of new versions seems
remote


Thanks
-- 
~Vinod

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Mathias Nyman]

On 31.1.2020 10.40, Vinod Koul wrote:
> Hi Mathias,
> 
> On 30-01-20, 19:07, Mathias Nyman wrote:
>> On 25.1.2020 7.32, Vinod Koul wrote:
>>>>>>>>
>>>>>>>> On Mon, Jan 13, 2020 at 12:42 AM Vinod Koul <vkoul@kernel.org> wrote:
>>>>>>>>>
>>>>>>>>> This series add support for Renesas USB controllers uPD720201 and uPD720202.
>>>>>>>>> These require firmware to be loaded and in case devices have ROM those can
>>>>>>>>> also be programmed if empty. If ROM is programmed, it runs from ROM as well.
>>>>>>>>>
>>>>>>>>> This includes two patches from Christian which supported these controllers
>>>>>>>>> w/o ROM and later my patches for ROM support and multiple firmware versions,
>>>>>>>>> debugfs hook for rom erase and export of xhci-pci functions.
>>>>>>>>>

...

>>
>> There are a few other opens regarding this series. Mostly because I'm not (yet)
>> familiar with all the details, so I'll just just list them here.
>>
>> - Is it really enough to add the Renesas driver to Makefile before xhci-pci
>>    driver to make sure it gets matched and probed based on vendor/device id
>>    before xhci-pci driver is matched and probed based on pci class?
>>    What if the Renesas driver is a module and xhci-pci compiled in?
> 
> The driver loading rules are based on level and makefile order. So
> renesas will always be loaded before xhci driver. This is required since
> xhci claims all devices, so we need to make sure it loads before.
> 
> I think we should make it inbuilt driver rather than a module. xhci
> driver is only inbuilt.
> 
> If there is a better way to handle this, am all for it.
> 
>> - Previously probe didn't return before hcd's were added and everything set up.
>>    Now with request_firmware_nowait() probe returns early successfully, and the
>>    old xhci_pci_probe() which sets up everything is called later by the request
>>    firmware callback. So there could be whole new set of races possible.
>>    For example pci remove can be called mid firmware loading, or when the old
>>    xhci_pci_probe is still setting up things.
> 
> I think this is a valid concern. Let me think about how to solve this..
> maybe add a flag in remove which ensure remove doesnt run untill the
> probe/firmware callback is completed.

How about initiating async firmware loading before probe is called by using
DECLARE_PCI_FIXUP_*() hooks?

Probe would then just check if there is a valid running firmware, if not just
defer probe until later (return -EPROBE_DEFER).

No need for a separate Renesas xhci driver, no worries about which driver
claims the device first, no races because of probe returning successfully
too early.

Can we check the device for a valid running firmware without disrupting a
ongoing firmware write?

-Mathias

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Vinod Koul]

On 04-02-20, 18:33, Mathias Nyman wrote:

> > > 
> > > There are a few other opens regarding this series. Mostly because I'm not (yet)
> > > familiar with all the details, so I'll just just list them here.
> > > 
> > > - Is it really enough to add the Renesas driver to Makefile before xhci-pci
> > >    driver to make sure it gets matched and probed based on vendor/device id
> > >    before xhci-pci driver is matched and probed based on pci class?
> > >    What if the Renesas driver is a module and xhci-pci compiled in?
> > 
> > The driver loading rules are based on level and makefile order. So
> > renesas will always be loaded before xhci driver. This is required since
> > xhci claims all devices, so we need to make sure it loads before.
> > 
> > I think we should make it inbuilt driver rather than a module. xhci
> > driver is only inbuilt.
> > 
> > If there is a better way to handle this, am all for it.
> > 
> > > - Previously probe didn't return before hcd's were added and everything set up.
> > >    Now with request_firmware_nowait() probe returns early successfully, and the
> > >    old xhci_pci_probe() which sets up everything is called later by the request
> > >    firmware callback. So there could be whole new set of races possible.
> > >    For example pci remove can be called mid firmware loading, or when the old
> > >    xhci_pci_probe is still setting up things.
> > 
> > I think this is a valid concern. Let me think about how to solve this..
> > maybe add a flag in remove which ensure remove doesnt run untill the
> > probe/firmware callback is completed.
> 
> How about initiating async firmware loading before probe is called by using
> DECLARE_PCI_FIXUP_*() hooks?

Well somehow that does not work :(

I tried using DECLARE_PCI_FIXUP_FINAL hook to request the firmware.
Doing so from probe works but from fixup fails always!

So expect this one, I have done the rest of the changes requested, will
send them over soon

Thanks
-- 
~Vinod

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Alan Stern]

On Thu, 30 Jan 2020, Mathias Nyman wrote:

> I realize this can't be easily changed because usb_hcd_pci_probe() takes the
> pci_device_id pointer as an argument, and expects id.driver_data to be a
> HC driver pointer.
> 
> So this turns out to be a question for Greg and Alan:
> 
> Would it make sense to change usb_hcd_pci_probe() to take a HC driver pointer
> as an argument instead of a pointer to pci_device_id?
> pci_device_id pointer is only used to extract the HC driver handle.
> This way the driver_data could be used for, well, driver data.

That seems like a good idea to me.  There aren't very many drivers that 
use usb_hcd_pci_probe(); changing them all should be fairly easy.

Alan Stern

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Vinod Koul]

On 31-01-20, 10:47, Alan Stern wrote:
> On Thu, 30 Jan 2020, Mathias Nyman wrote:
> 
> > I realize this can't be easily changed because usb_hcd_pci_probe() takes the
> > pci_device_id pointer as an argument, and expects id.driver_data to be a
> > HC driver pointer.
> > 
> > So this turns out to be a question for Greg and Alan:
> > 
> > Would it make sense to change usb_hcd_pci_probe() to take a HC driver pointer
> > as an argument instead of a pointer to pci_device_id?
> > pci_device_id pointer is only used to extract the HC driver handle.
> > This way the driver_data could be used for, well, driver data.
> 
> That seems like a good idea to me.  There aren't very many drivers that 
> use usb_hcd_pci_probe(); changing them all should be fairly easy.

Yup it was easy to do :) I have done this and tested it. Now we can use
driver_data for driver data.

Though couldn't compile the uhci, seems to have missing Makefile entry.

-- 
~Vinod

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Andreas Böhler]

On 13/01/2020 09:40, Vinod Koul wrote:
> This series add support for Renesas USB controllers uPD720201 and uPD720202.
> These require firmware to be loaded and in case devices have ROM those can
> also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> 
> This includes two patches from Christian which supported these controllers
> w/o ROM and later my patches for ROM support and multiple firmware versions,
> debugfs hook for rom erase and export of xhci-pci functions.
> 
I tested this on an AVM FRITZ!Box 3490, backported to 4.19: Firmware
upload works fine.

However, I'm seeing read errors afterwards which, I suppose, are a
different story.

Here is the log:

[  498.115808] ifx_pcie_bios_map_irq port 0 dev 0000:01:00.0 slot 0 pin 1
[  498.121154] ifx_pcie_bios_map_irq dev 0000:01:00.0 irq 144 assigned
[  498.488541] renesas xhci 0000:01:00.0: xHCI Host Controller
[  498.492820] renesas xhci 0000:01:00.0: new USB bus registered,
assigned bus number 1
[  498.506123] renesas xhci 0000:01:00.0: hcc params 0x014051cf hci
version 0x100 quirks 0x0000000101000090
[  498.516869] hub 1-0:1.0: USB hub found
[  498.519631] hub 1-0:1.0: 2 ports detected
[  498.525641] renesas xhci 0000:01:00.0: xHCI Host Controller
[  498.530217] renesas xhci 0000:01:00.0: new USB bus registered,
assigned bus number 2
[  498.537846] renesas xhci 0000:01:00.0: Host supports USB 3.0 SuperSpeed
[  498.545095] usb usb2: We don't know the algorithms for LPM for this
host, disabling LPM.
[  498.554921] hub 2-0:1.0: USB hub found
[  498.557588] hub 2-0:1.0: 2 ports detected
[  523.013361] usb 1-1: new full-speed USB device number 2 using renesas
xhci
[  523.182725] usb 1-1: no configurations
[  523.185085] usb 1-1: can't read configurations, error -22
[  523.317423] usb 1-1: new full-speed USB device number 3 using renesas
xhci
[  523.493710] usb 1-1: no configurations
[  523.496069] usb 1-1: can't read configurations, error -22
[  523.501951] usb usb1-port1: attempt power cycle

Regards and thanks for the patch,
Andreas

Re: [PATCH v6 0/5] usb: xhci: Add support for Renesas USB controllers

[Christian Lamparter]

On Sunday, 26 January 2020 01:11:35 CET Andreas Böhler wrote:
> 
> On 13/01/2020 09:40, Vinod Koul wrote:
> > This series add support for Renesas USB controllers uPD720201 and uPD720202.
> > These require firmware to be loaded and in case devices have ROM those can
> > also be programmed if empty. If ROM is programmed, it runs from ROM as well.
> > 
> > This includes two patches from Christian which supported these controllers
> > w/o ROM and later my patches for ROM support and multiple firmware versions,
> > debugfs hook for rom erase and export of xhci-pci functions.
> > 
> I tested this on an AVM FRITZ!Box 3490, backported to 4.19: Firmware
> upload works fine.
> 
> However, I'm seeing read errors afterwards which, I suppose, are a
> different story.
> 
> Here is the log:
> 
> [  498.115808] ifx_pcie_bios_map_irq port 0 dev 0000:01:00.0 slot 0 pin 1
> [  498.121154] ifx_pcie_bios_map_irq dev 0000:01:00.0 irq 144 assigned
> [  498.488541] renesas xhci 0000:01:00.0: xHCI Host Controller
> [  498.492820] renesas xhci 0000:01:00.0: new USB bus registered,
> assigned bus number 1
> [  498.506123] renesas xhci 0000:01:00.0: hcc params 0x014051cf hci
> version 0x100 quirks 0x0000000101000090
> [  498.516869] hub 1-0:1.0: USB hub found
> [  498.519631] hub 1-0:1.0: 2 ports detected
> [  498.525641] renesas xhci 0000:01:00.0: xHCI Host Controller
> [  498.530217] renesas xhci 0000:01:00.0: new USB bus registered,
> assigned bus number 2
> [  498.537846] renesas xhci 0000:01:00.0: Host supports USB 3.0 SuperSpeed
> [  498.545095] usb usb2: We don't know the algorithms for LPM for this
> host, disabling LPM.
> [  498.554921] hub 2-0:1.0: USB hub found
> [  498.557588] hub 2-0:1.0: 2 ports detected
> [  523.013361] usb 1-1: new full-speed USB device number 2 using renesas
> xhci
> [  523.182725] usb 1-1: no configurations
> [  523.185085] usb 1-1: can't read configurations, error -22
> [  523.317423] usb 1-1: new full-speed USB device number 3 using renesas
> xhci
> [  523.493710] usb 1-1: no configurations
> [  523.496069] usb 1-1: can't read configurations, error -22
> [  523.501951] usb usb1-port1: attempt power cycle
> 
Hm, I don't think lantiq's PCI code is upstream... And now that I've seen
more errors from your forum post at: 
<https://forum.openwrt.org/t/fix-xhci-errors-on-renesas-upd70202-fritz-box-3490/53620>

I wonder if this has something to do with a similar issue I was facing with
the ath9k chip loader in commit:
5a4f2040fd07 ("ath9k: add loader for AR92XX (and older) pci(e)")

which later needed a fix for a specifc lantiq byteswap problem in commit:
22d0d5ae7a08 ("ath9k: use iowrite32 over __raw_writel"):
|    This patch changes the ath9k_pci_owl_loader to use the
|    same iowrite32 memory accessor that ath9k_pci is using
|    to communicate with the PCI(e) chip.
|   
|   This will fix endian issues that came up during testing
|   with loaned AVM Fritz!Box 7360 (Lantiq MIPS SoCs + AR9287).


The reason was that apparently (I gave back the loaned device), the lantiq
PCI silicon does some sneaky byteswaps in special cases. Could this be
related? You mentioned in another post that AVM did changes to the xhci
driver, can you look if they added changes to the memory accessors?
Because this would explain why the APM82181 (PowerPC which is also a
BigEndian) had no issues (as it's using a entirely different pcie hardware
and code).

Cheers,
Christian