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

[PATCH v8 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 patches from Christian which supported these controllers w/o
ROM and later my patches for ROM support and debugfs hook for rom erase and
export of xhci-pci functions.

Changes in v8:
 Fix compile error reported by Kbuild-bot by making usb_hcd_pci_probe() take
 const struct hc_driver * as argument

Changes in v7:
 Make a single module which removes issues with module loading
 Keep the renesas code in renesas file
 Add hc_driver as argument for usb_hcd_pci_probe and modify hdc drivers to
   pass this and not use driver_data
 Use driver data for fw name
 Remove code to check if we need to load firmware or not
 remove multiple fw version support, we can do that with symlink in
   userspace

Changes in v6:
 Move the renesas code into a separate 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 firmware 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: hci: add hc_driver as argument for usb_hcd_pci_probe
  usb: xhci: Add support for Renesas controller with memory
  usb: renesas-xhci: Add ROM loader for uPD720201
  usb: xhci: provide a debugfs hook for erasing rom

 drivers/usb/core/hcd-pci.c          |   7 +-
 drivers/usb/host/Makefile           |   3 +-
 drivers/usb/host/ehci-pci.c         |   6 +-
 drivers/usb/host/ohci-pci.c         |   9 +-
 drivers/usb/host/uhci-pci.c         |   8 +-
 drivers/usb/host/xhci-pci-renesas.c | 802 ++++++++++++++++++++++++++++
 drivers/usb/host/xhci-pci.c         |  43 +-
 drivers/usb/host/xhci-pci.h         |  14 +
 include/linux/usb/hcd.h             |   3 +-
 9 files changed, 871 insertions(+), 24 deletions(-)
 create mode 100644 drivers/usb/host/xhci-pci-renesas.c
 create mode 100644 drivers/usb/host/xhci-pci.h

-- 
2.25.1

[PATCH v8 1/5] usb: hci: add hc_driver as argument for usb_hcd_pci_probe

[Vinod Koul]

usb_hcd_pci_probe expects users to call this with driver_data set as
hc_driver, that limits the possibility of using the driver_data for
driver data.

Add hc_driver as argument to usb_hcd_pci_probe and modify the callers
ehci/ohci/xhci/uhci to pass hc_driver as argument and freeup the
driver_data used

Tested xhci driver on Dragon-board RB3, compile tested ehci and ohci.
Couldn't compile uhci

Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/usb/core/hcd-pci.c  |  7 ++++---
 drivers/usb/host/ehci-pci.c |  6 ++----
 drivers/usb/host/ohci-pci.c |  9 ++++++---
 drivers/usb/host/uhci-pci.c |  8 ++++++--
 drivers/usb/host/xhci-pci.c | 14 +++++---------
 include/linux/usb/hcd.h     |  3 ++-
 6 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index f0a259937da8..1547aa6e5314 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -159,6 +159,7 @@ static void ehci_wait_for_companions(struct pci_dev *pdev, struct usb_hcd *hcd,
  * usb_hcd_pci_probe - initialize PCI-based HCDs
  * @dev: USB Host Controller being probed
  * @id: pci hotplug id connecting controller to HCD framework
+ * @driver: USB HC driver handle
  * Context: !in_interrupt()
  *
  * Allocates basic PCI resources for this USB host controller, and
@@ -169,9 +170,9 @@ static void ehci_wait_for_companions(struct pci_dev *pdev, struct usb_hcd *hcd,
  *
  * Return: 0 if successful.
  */
-int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id,
+		      const struct hc_driver *driver)
 {
-	struct hc_driver	*driver;
 	struct usb_hcd		*hcd;
 	int			retval;
 	int			hcd_irq = 0;
@@ -181,7 +182,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	if (!id)
 		return -EINVAL;
-	driver = (struct hc_driver *)id->driver_data;
+
 	if (!driver)
 		return -EINVAL;
 
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index b0882c13a1d1..daaddfbbf797 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -360,23 +360,21 @@ static int ehci_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	if (is_bypassed_id(pdev))
 		return -ENODEV;
-	return usb_hcd_pci_probe(pdev, id);
+	return usb_hcd_pci_probe(pdev, id, &ehci_pci_hc_driver);
 }
 
 static void ehci_pci_remove(struct pci_dev *pdev)
 {
 	pci_clear_mwi(pdev);
-	usb_hcd_pci_remove(pdev);	
+	usb_hcd_pci_remove(pdev);
 }
 
 /* PCI driver selection metadata; PCI hotplugging uses this */
 static const struct pci_device_id pci_ids [] = { {
 	/* handle any USB 2.0 EHCI controller */
 	PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_EHCI, ~0),
-	.driver_data =	(unsigned long) &ehci_pci_hc_driver,
 	}, {
 	PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_USB_HOST),
-	.driver_data = (unsigned long) &ehci_pci_hc_driver,
 	},
 	{ /* end: all zeroes */ }
 };
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index f4e13a3fddee..78b0d84c7675 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -277,21 +277,24 @@ static const struct ohci_driver_overrides pci_overrides __initconst = {
 static const struct pci_device_id pci_ids[] = { {
 	/* handle any USB OHCI controller */
 	PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_OHCI, ~0),
-	.driver_data =	(unsigned long) &ohci_pci_hc_driver,
 	}, {
 	/* The device in the ConneXT I/O hub has no class reg */
 	PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_USB_OHCI),
-	.driver_data =	(unsigned long) &ohci_pci_hc_driver,
 	}, { /* end: all zeroes */ }
 };
 MODULE_DEVICE_TABLE (pci, pci_ids);
 
+static int ohci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	return usb_hcd_pci_probe(dev, id, &ohci_pci_hc_driver);
+}
+
 /* pci driver glue; this is a "new style" PCI driver module */
 static struct pci_driver ohci_pci_driver = {
 	.name =		(char *) hcd_name,
 	.id_table =	pci_ids,
 
-	.probe =	usb_hcd_pci_probe,
+	.probe =	ohci_pci_probe,
 	.remove =	usb_hcd_pci_remove,
 	.shutdown =	usb_hcd_pci_shutdown,
 
diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
index 0fa3d72bae26..d59b9257c52b 100644
--- a/drivers/usb/host/uhci-pci.c
+++ b/drivers/usb/host/uhci-pci.c
@@ -287,17 +287,21 @@ static const struct hc_driver uhci_driver = {
 static const struct pci_device_id uhci_pci_ids[] = { {
 	/* handle any USB UHCI controller */
 	PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_UHCI, ~0),
-	.driver_data =	(unsigned long) &uhci_driver,
 	}, { /* end: all zeroes */ }
 };
 
 MODULE_DEVICE_TABLE(pci, uhci_pci_ids);
 
+static int uhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	return usb_hcd_pci_probe(dev, id, &uhci_driver);
+}
+
 static struct pci_driver uhci_pci_driver = {
 	.name =		(char *)hcd_name,
 	.id_table =	uhci_pci_ids,
 
-	.probe =	usb_hcd_pci_probe,
+	.probe =	uhci_pci_probe,
 	.remove =	usb_hcd_pci_remove,
 	.shutdown =	uhci_shutdown,
 
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 4917c5b033fa..a19752178216 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -316,11 +316,8 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	int retval;
 	struct xhci_hcd *xhci;
-	struct hc_driver *driver;
 	struct usb_hcd *hcd;
 
-	driver = (struct hc_driver *)id->driver_data;
-
 	/* Prevent runtime suspending between USB-2 and USB-3 initialization */
 	pm_runtime_get_noresume(&dev->dev);
 
@@ -330,7 +327,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	 * to say USB 2.0, but I'm not sure what the implications would be in
 	 * the other parts of the HCD code.
 	 */
-	retval = usb_hcd_pci_probe(dev, id);
+	retval = usb_hcd_pci_probe(dev, id, &xhci_pci_hc_driver);
 
 	if (retval)
 		goto put_runtime_pm;
@@ -338,8 +335,8 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* USB 2.0 roothub is stored in the PCI device now. */
 	hcd = dev_get_drvdata(&dev->dev);
 	xhci = hcd_to_xhci(hcd);
-	xhci->shared_hcd = usb_create_shared_hcd(driver, &dev->dev,
-				pci_name(dev), hcd);
+	xhci->shared_hcd = usb_create_shared_hcd(&xhci_pci_hc_driver, &dev->dev,
+						 pci_name(dev), hcd);
 	if (!xhci->shared_hcd) {
 		retval = -ENOMEM;
 		goto dealloc_usb2_hcd;
@@ -536,10 +533,9 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd)
 /*-------------------------------------------------------------------------*/
 
 /* PCI driver selection metadata; PCI hotplugging uses this */
-static const struct pci_device_id pci_ids[] = { {
+static const struct pci_device_id pci_ids[] = {
 	/* handle any USB 3.0 xHCI controller */
-	PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_XHCI, ~0),
-	.driver_data =	(unsigned long) &xhci_pci_hc_driver,
+	{ PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_XHCI, ~0),
 	},
 	{ /* end: all zeroes */ }
 };
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 712b2a603645..d82f3c849b6b 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -479,7 +479,8 @@ extern void usb_hcd_platform_shutdown(struct platform_device *dev);
 struct pci_dev;
 struct pci_device_id;
 extern int usb_hcd_pci_probe(struct pci_dev *dev,
-				const struct pci_device_id *id);
+			     const struct pci_device_id *id,
+			     const struct hc_driver *driver);
 extern void usb_hcd_pci_remove(struct pci_dev *dev);
 extern void usb_hcd_pci_shutdown(struct pci_dev *dev);
 
-- 
2.25.1

[PATCH v8 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 recursion for renesas_fw_download
	add register defines and field names
	move to a separate file]
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/usb/host/Makefile           |   3 +-
 drivers/usb/host/xhci-pci-renesas.c | 431 ++++++++++++++++++++++++++++
 drivers/usb/host/xhci-pci.h         |  11 +
 3 files changed, 444 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/host/xhci-pci-renesas.c
 create mode 100644 drivers/usb/host/xhci-pci.h

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index b191361257cc..c3a79f626393 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -70,7 +70,8 @@ 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)	+= xhci-pci.o
+usb-xhci-pci-objs		:= xhci-pci.o xhci-pci-renesas.o
+obj-$(CONFIG_USB_XHCI_PCI)	+= usb-xhci-pci.o
 obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
 obj-$(CONFIG_USB_XHCI_HISTB)	+= xhci-histb.o
 obj-$(CONFIG_USB_XHCI_MTK)	+= xhci-mtk.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..c588277ac9b8
--- /dev/null
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -0,0 +1,431 @@
+// 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_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_NO_RESULT			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
+
+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) {
+		dev_err(&dev->dev, "FW Load timedout");
+		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)
+{
+	u16 fw_version_pointer;
+	u16 fw_version;
+
+	/*
+	 * 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);
+
+	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 reset, 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! */
+			dev_err(&pdev->dev, "FW Load timedout");
+			return -ETIMEDOUT;
+
+		default:
+			return 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;
+	}
+
+	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_download_to_hw(struct pci_dev *pdev,
+				     const struct pci_device_id *id)
+{
+	struct renesas_fw_ctx *ctx;
+	char *fw_name = (char *)id->driver_data;
+	int err;
+
+	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->id = id;
+
+	pci_dev_get(pdev);
+	err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
+				      &pdev->dev, GFP_KERNEL,
+				      ctx, renesas_fw_callback);
+	if (err) {
+		dev_err(&pdev->dev, "request_firmware failed: %d\n", err);
+		pci_dev_put(pdev);
+		return err;
+	}
+
+	/*
+	 * The renesas_fw_callback() callback will continue the probe
+	 * process, once it acquires the firmware.
+	 */
+	return 1;
+}
+
+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);
+	switch (retval) {
+	case 0:
+		break;
+
+	case 1: /* let it load the firmware and recontinue the probe. */
+		retval = 1;
+		break;
+
+	default:
+		break;
+	};
+
+	return retval;
+}
+
+int renesas_xhci_pci_remove(struct pci_dev *dev)
+{
+	if (renesas_fw_check_running(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 -EIO;
+	}
+	return 0;
+}
diff --git a/drivers/usb/host/xhci-pci.h b/drivers/usb/host/xhci-pci.h
new file mode 100644
index 000000000000..092909dd85a0
--- /dev/null
+++ b/drivers/usb/host/xhci-pci.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2019-2020 Linaro Limited */
+
+#ifndef XHCI_PCI_H
+#define XHCI_PCI_H
+
+int renesas_xhci_pci_probe(struct pci_dev *dev,
+			   const struct pci_device_id *id);
+int renesas_xhci_pci_remove(struct pci_dev *dev);
+#endif
+
-- 
2.25.1

[PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Vinod Koul]

Some rensas controller like uPD720201 and uPD720202 need firmware to be
loaded. Add these devices in table and invoke renesas firmware loader
functions to check and load the firmware into device memory when
required.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/usb/host/xhci-pci-renesas.c |  1 +
 drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
 drivers/usb/host/xhci-pci.h         |  3 +++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index c588277ac9b8..d413d53df94b 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
 		goto cleanup;
 	}
 
+	xhci_pci_probe(pdev, ctx->id);
 	return;
 
 cleanup:
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a19752178216..7e63658542ac 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
@@ -312,11 +313,25 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
  * 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;
 	struct usb_hcd *hcd;
+	char *renesas_fw;
+
+	renesas_fw = (char *)id->driver_data;
+	if (renesas_fw) {
+		retval = renesas_xhci_pci_probe(dev, id);
+		switch (retval) {
+		case 0: /* fw check success, continue */
+			break;
+		case 1: /* fw will be loaded by async load */
+			return 0;
+		default: /* error */
+			return retval;
+		}
+	}
 
 	/* Prevent runtime suspending between USB-2 and USB-3 initialization */
 	pm_runtime_get_noresume(&dev->dev);
@@ -379,6 +394,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 static void xhci_pci_remove(struct pci_dev *dev)
 {
 	struct xhci_hcd *xhci;
+	int err;
+
+	err = renesas_xhci_pci_remove(dev);
+	if (err)
+		return;
 
 	xhci = hcd_to_xhci(pci_get_drvdata(dev));
 	xhci->xhc_state |= XHCI_STATE_REMOVING;
@@ -534,12 +554,19 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd)
 
 /* PCI driver selection metadata; PCI hotplugging uses this */
 static const struct pci_device_id pci_ids[] = {
+	{ PCI_DEVICE(0x1912, 0x0014),
+		.driver_data =  (unsigned long)"renesas_usb_fw.mem",
+	},
+	{ PCI_DEVICE(0x1912, 0x0015),
+		.driver_data =  (unsigned long)"renesas_usb_fw.mem",
+	},
 	/* handle any USB 3.0 xHCI controller */
 	{ PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_XHCI, ~0),
 	},
 	{ /* end: all zeroes */ }
 };
 MODULE_DEVICE_TABLE(pci, pci_ids);
+MODULE_FIRMWARE("renesas_usb_fw.mem");
 
 /* pci driver glue; this is a "new style" PCI driver module */
 static struct pci_driver xhci_pci_driver = {
diff --git a/drivers/usb/host/xhci-pci.h b/drivers/usb/host/xhci-pci.h
index 092909dd85a0..1b4330f893fa 100644
--- a/drivers/usb/host/xhci-pci.h
+++ b/drivers/usb/host/xhci-pci.h
@@ -7,5 +7,8 @@
 int renesas_xhci_pci_probe(struct pci_dev *dev,
 			   const struct pci_device_id *id);
 int renesas_xhci_pci_remove(struct pci_dev *dev);
+
+int xhci_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id);
+
 #endif
 
-- 
2.25.1

[PATCH v8 4/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 | 341 +++++++++++++++++++++++++++-
 1 file changed, 338 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index d413d53df94b..6301730b1f8d 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -50,6 +50,22 @@
 #define RENESAS_RETRY	10000
 #define RENESAS_DELAY	10
 
+#define ROM_VALID_01 0x2013
+#define ROM_VALID_02 0x2026
+
+static int renesas_verify_fw_version(struct pci_dev *pdev, u32 version)
+{
+	switch (version) {
+	case ROM_VALID_01:
+	case ROM_VALID_02:
+		return 0;
+	default:
+		dev_err(&pdev->dev, "FW has invalid version :%d\n", version);
+		return 1;
+	}
+	return -EINVAL;
+}
+
 static int renesas_fw_download_image(struct pci_dev *dev,
 				     const u32 *fw,
 				     size_t step)
@@ -146,10 +162,62 @@ static int renesas_fw_verify(struct pci_dev *dev,
 	return 0;
 }
 
-static int renesas_fw_check_running(struct pci_dev *pdev)
+static int renesas_check_rom_state(struct pci_dev *pdev)
 {
+	u16 rom_state;
+	u32 version;
 	int err;
+
+	/* 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;
+
+	err = renesas_verify_fw_version(pdev, version);
+	if (err)
+		return err;
+
+	/*
+	 * 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:
+			return 0;
+
+		case RENESAS_ROM_STATUS_NO_RESULT: /* No result yet */
+			return 0;
+
+		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)
+{
 	u8 fw_state;
+	int err;
+
+	/* 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
@@ -308,16 +376,263 @@ static int renesas_fw_download(struct pci_dev *pdev,
 struct renesas_fw_ctx {
 	struct pci_dev *pdev;
 	const struct pci_device_id *id;
-	bool resume;
-	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 succeeded\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) {
@@ -329,6 +644,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) {
+		/* perform 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_err(&pdev->dev,
+				"ROM load success\n");
+			release_firmware(fw);
+			goto do_probe;
+		}
+	}
+
 	err = renesas_fw_download(pdev, fw);
 	release_firmware(fw);
 	if (err) {
@@ -336,6 +670,7 @@ static void renesas_fw_callback(const struct firmware *fw,
 		goto cleanup;
 	}
 
+do_probe:
 	xhci_pci_probe(pdev, ctx->id);
 	return;
 
-- 
2.25.1

[PATCH v8 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 6301730b1f8d..714932eaac90 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>
@@ -162,6 +163,8 @@ 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)
 {
 	u16 rom_state;
@@ -194,9 +197,11 @@ static int renesas_check_rom_state(struct pci_dev *pdev)
 		/* Check the "Result Code" Bits (6:4) and act accordingly */
 		switch (rom_state & RENESAS_ROM_STATUS_RESULT) {
 		case RENESAS_ROM_STATUS_SUCCESS:
+			debugfs_init(pdev);
 			return 0;
 
 		case RENESAS_ROM_STATUS_NO_RESULT: /* No result yet */
+			debugfs_init(pdev);
 			return 0;
 
 		case RENESAS_ROM_STATUS_ERROR: /* Error State */
@@ -443,6 +448,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)
 {
@@ -754,6 +787,8 @@ int renesas_xhci_pci_probe(struct pci_dev *dev,
 
 int renesas_xhci_pci_remove(struct pci_dev *dev)
 {
+	debugfs_exit();
+
 	if (renesas_fw_check_running(dev)) {
 		/*
 		 * bail out early, if this was a renesas device w/o FW.
-- 
2.25.1

Re: [PATCH v8 1/5] usb: hci: add hc_driver as argument for usb_hcd_pci_probe

[Mathias Nyman]

On 23.3.2020 19.05, Vinod Koul wrote:
> usb_hcd_pci_probe expects users to call this with driver_data set as
> hc_driver, that limits the possibility of using the driver_data for
> driver data.
> 
> Add hc_driver as argument to usb_hcd_pci_probe and modify the callers
> ehci/ohci/xhci/uhci to pass hc_driver as argument and freeup the
> driver_data used
> 
> Tested xhci driver on Dragon-board RB3, compile tested ehci and ohci.
> Couldn't compile uhci
> 
> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  drivers/usb/core/hcd-pci.c  |  7 ++++---
>  drivers/usb/host/ehci-pci.c |  6 ++----
>  drivers/usb/host/ohci-pci.c |  9 ++++++---
>  drivers/usb/host/uhci-pci.c |  8 ++++++--
>  drivers/usb/host/xhci-pci.c | 14 +++++---------
>  include/linux/usb/hcd.h     |  3 ++-
>  6 files changed, 25 insertions(+), 22 deletions(-)
> 

For the xhci part of this 1/5 patch only:
Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>

Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Mathias Nyman]

Hi Vinod

On 23.3.2020 19.05, Vinod Koul wrote:
> Some rensas controller like uPD720201 and uPD720202 need firmware to be
> loaded. Add these devices in table and invoke renesas firmware loader
> functions to check and load the firmware into device memory when
> required.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  drivers/usb/host/xhci-pci-renesas.c |  1 +
>  drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
>  drivers/usb/host/xhci-pci.h         |  3 +++
>  3 files changed, 32 insertions(+), 1 deletion(-)
> 

It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook
for this Renesas controller. What was the reason it failed?

Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4
where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff()

https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de

Is he doing something different than what was done for the Renesas controller?


> diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> index c588277ac9b8..d413d53df94b 100644
> --- a/drivers/usb/host/xhci-pci-renesas.c
> +++ b/drivers/usb/host/xhci-pci-renesas.c
> @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
>  		goto cleanup;
>  	}
>  
> +	xhci_pci_probe(pdev, ctx->id);
>  	return;

I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw
loading callback could we just return -EPROBE_DEFER until firmware is loaded when
xhci_pci_probe() is originally called?

>  
>  cleanup:
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index a19752178216..7e63658542ac 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
> @@ -312,11 +313,25 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
>   * 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;
>  	struct usb_hcd *hcd;
> +	char *renesas_fw;
> +
> +	renesas_fw = (char *)id->driver_data;

driver_data is useful for other things than just renesas firmware loading.
Heikki suggested a long time ago to use it for passing the quirk flags as well, which
makes sense.

We probably need a structure, something like

struct xhci_driver_data = {
	u64 quirks;
	const char *firmware;
};

> +	if (renesas_fw) {
> +		retval = renesas_xhci_pci_probe(dev, id);
> +		switch (retval) {
> +		case 0: /* fw check success, continue */
> +			break;
> +		case 1: /* fw will be loaded by async load */
> +			return 0;
> +		default: /* error */
> +			return retval;
> +		}
> +	}
>  

If returning -EPROBE_DEFER until firmware is loaded is an option then we would prevent probe
from returning success while the renesas controller is still loading firmware.

So we would end up with something like this:
(we can add a quirk flag for renesas firmware loading)

int xhci_pci_probe(..)
{
	...
	struct xhci_driver_data *data = id->driver_data;
	if (data && data->quirks & XHCI_RENESAS_FW_QUIRK) { 
		if (!xhci_renesas_fw_ready(...))
			return -EPROBE_DEFER
	}
}

xhci_renesas_fw_ready() would need to initiate firmware loading unless
firmware is already running or loading.

Would that work for you?

-Mathias

Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Vinod Koul]

On 26-03-20, 13:29, Mathias Nyman wrote:
> Hi Vinod
> 
> On 23.3.2020 19.05, Vinod Koul wrote:
> > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > loaded. Add these devices in table and invoke renesas firmware loader
> > functions to check and load the firmware into device memory when
> > required.
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  drivers/usb/host/xhci-pci-renesas.c |  1 +
> >  drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
> >  drivers/usb/host/xhci-pci.h         |  3 +++
> >  3 files changed, 32 insertions(+), 1 deletion(-)
> > 
> 
> It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook
> for this Renesas controller. What was the reason it failed?
> 
> Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4
> where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff()
> 
> https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de
> 
> Is he doing something different than what was done for the Renesas controller?

I tried and everytime ended up not getting firmware. Though I did not
investigate a lot. Christian seemed to have tested sometime back as
well.

Another problem is that we dont get driver_data in the quirk and there
didnt seem a way to find the firmware name.

> > diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> > index c588277ac9b8..d413d53df94b 100644
> > --- a/drivers/usb/host/xhci-pci-renesas.c
> > +++ b/drivers/usb/host/xhci-pci-renesas.c
> > @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
> >  		goto cleanup;
> >  	}
> >  
> > +	xhci_pci_probe(pdev, ctx->id);
> >  	return;
> 
> I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw
> loading callback could we just return -EPROBE_DEFER until firmware is loaded when
> xhci_pci_probe() is originally called?

Hmm, initially my thinking was how to tell device core to probe again,
and then digging up I saw wait_for_device_probe() which can be used, let
me try that

> >  cleanup:
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index a19752178216..7e63658542ac 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
> > @@ -312,11 +313,25 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
> >   * 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;
> >  	struct usb_hcd *hcd;
> > +	char *renesas_fw;
> > +
> > +	renesas_fw = (char *)id->driver_data;
> 
> driver_data is useful for other things than just renesas firmware loading.
> Heikki suggested a long time ago to use it for passing the quirk flags as well, which
> makes sense.
> 
> We probably need a structure, something like
> 
> struct xhci_driver_data = {
> 	u64 quirks;
> 	const char *firmware;
> };
> 
> > +	if (renesas_fw) {
> > +		retval = renesas_xhci_pci_probe(dev, id);
> > +		switch (retval) {
> > +		case 0: /* fw check success, continue */
> > +			break;
> > +		case 1: /* fw will be loaded by async load */
> > +			return 0;
> > +		default: /* error */
> > +			return retval;
> > +		}
> > +	}
> >  
> 
> If returning -EPROBE_DEFER until firmware is loaded is an option then we would prevent probe
> from returning success while the renesas controller is still loading firmware.
> 
> So we would end up with something like this:
> (we can add a quirk flag for renesas firmware loading)
> 
> int xhci_pci_probe(..)
> {
> 	...
> 	struct xhci_driver_data *data = id->driver_data;
> 	if (data && data->quirks & XHCI_RENESAS_FW_QUIRK) { 
> 		if (!xhci_renesas_fw_ready(...))
> 			return -EPROBE_DEFER
> 	}
> }
> 
> xhci_renesas_fw_ready() would need to initiate firmware loading unless
> firmware is already running or loading.
> 
> Would that work for you?

I think yes that should work, let me try that..

-- 
~Vinod

Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Vinod Koul]

On 26-03-20, 17:21, Vinod Koul wrote:
> On 26-03-20, 13:29, Mathias Nyman wrote:
> > Hi Vinod
> > 
> > On 23.3.2020 19.05, Vinod Koul wrote:
> > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > loaded. Add these devices in table and invoke renesas firmware loader
> > > functions to check and load the firmware into device memory when
> > > required.
> > > 
> > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > ---
> > >  drivers/usb/host/xhci-pci-renesas.c |  1 +
> > >  drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
> > >  drivers/usb/host/xhci-pci.h         |  3 +++
> > >  3 files changed, 32 insertions(+), 1 deletion(-)
> > > 
> > 
> > It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook
> > for this Renesas controller. What was the reason it failed?
> > 
> > Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4
> > where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff()
> > 
> > https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de
> > 
> > Is he doing something different than what was done for the Renesas controller?
> 
> I tried and everytime ended up not getting firmware. Though I did not
> investigate a lot. Christian seemed to have tested sometime back as
> well.
> 
> Another problem is that we dont get driver_data in the quirk and there
> didnt seem a way to find the firmware name.
> 
> > > diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> > > index c588277ac9b8..d413d53df94b 100644
> > > --- a/drivers/usb/host/xhci-pci-renesas.c
> > > +++ b/drivers/usb/host/xhci-pci-renesas.c
> > > @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
> > >  		goto cleanup;
> > >  	}
> > >  
> > > +	xhci_pci_probe(pdev, ctx->id);
> > >  	return;
> > 
> > I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw
> > loading callback could we just return -EPROBE_DEFER until firmware is loaded when
> > xhci_pci_probe() is originally called?
> 
> Hmm, initially my thinking was how to tell device core to probe again,
> and then digging up I saw wait_for_device_probe() which can be used, let
> me try that

Sorry to report back that it doesn't work as planned :(

I modified the code to invoke the request_firmware_nowait() which will load
the firmware and provide the firmware in callback. Meanwhile return -EPROBE_DEFER.

After a bit, the core invokes the driver probe again and we hit the
roadblock. The request_firmware uses devres and allocates resources for
loading the firmware. The problem is that device core checks for this:

bus: 'pci': really_probe: probing driver xhci_hcd_pci with device 0000:01:00.0
pci 0000:01:00.0: Resources present before probing

And here the probe fails. In some cases the firmware_callback finishes
before this and we can probe again, but that is not very reliable.

I tested another way to use request_firmware() (sync version) and then
load the firmware in probe and load. The request is done only for
renesas devices if they dont have firmware already running.
So rest of the devices wont have any impact.

Now should we continue this way in the patchset or move to sync version.
Am okay either way.

-- 
~Vinod

Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Christian Lamparter]

Hello,

On Wednesday, 1 April 2020 14:57:48 CEST Vinod Koul wrote:
> On 26-03-20, 17:21, Vinod Koul wrote:
> > On 26-03-20, 13:29, Mathias Nyman wrote:
> > > On 23.3.2020 19.05, Vinod Koul wrote:
> > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > > loaded. Add these devices in table and invoke renesas firmware loader
> > > > functions to check and load the firmware into device memory when
> > > > required.
> > > > 
> > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > ---
> > > >  drivers/usb/host/xhci-pci-renesas.c |  1 +
> > > >  drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
> > > >  drivers/usb/host/xhci-pci.h         |  3 +++
> > > >  3 files changed, 32 insertions(+), 1 deletion(-)
> > > > 
> > > 
> > > It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook
> > > for this Renesas controller. What was the reason it failed?
> > > 
> > > Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4
> > > where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff()
> > > 
> > > https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de
> > > 
> > > Is he doing something different than what was done for the Renesas controller?
> > 
> > I tried and everytime ended up not getting firmware. Though I did not
> > investigate a lot. Christian seemed to have tested sometime back as
> > well.
> > 
> > Another problem is that we dont get driver_data in the quirk and there
> > didnt seem a way to find the firmware name.
> > 
> > > > diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> > > > index c588277ac9b8..d413d53df94b 100644
> > > > --- a/drivers/usb/host/xhci-pci-renesas.c
> > > > +++ b/drivers/usb/host/xhci-pci-renesas.c
> > > > @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
> > > >  		goto cleanup;
> > > >  	}
> > > >  
> > > > +	xhci_pci_probe(pdev, ctx->id);
> > > >  	return;
> > > 
> > > I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw
> > > loading callback could we just return -EPROBE_DEFER until firmware is loaded when
> > > xhci_pci_probe() is originally called?
> > 
> > Hmm, initially my thinking was how to tell device core to probe again,
> > and then digging up I saw wait_for_device_probe() which can be used, let
> > me try that
> 
> Sorry to report back that it doesn't work as planned :(
> 
> I modified the code to invoke the request_firmware_nowait() which will load
> the firmware and provide the firmware in callback. Meanwhile return -EPROBE_DEFER.
> 
> After a bit, the core invokes the driver probe again and we hit the
> roadblock. The request_firmware uses devres and allocates resources for
> loading the firmware. The problem is that device core checks for this:
> 
> bus: 'pci': really_probe: probing driver xhci_hcd_pci with device 0000:01:00.0
> pci 0000:01:00.0: Resources present before probing
> 
> And here the probe fails. In some cases the firmware_callback finishes
> before this and we can probe again, but that is not very reliable.
> 
> I tested another way to use request_firmware() (sync version) and then
> load the firmware in probe and load. The request is done only for
> renesas devices if they dont have firmware already running.
> So rest of the devices wont have any impact.
> 
> Now should we continue this way in the patchset or move to sync version.
> Am okay either way.

Just a word of caution.

The problem with the usage of "sync" request_firmware in drivers is that if the
code is built into the kernel the request_firmware() could be called before the
(root) filesystem on which the firmware resides is ready.... So this will get
weird during boot because what is the sync request_firmware() going to do? From what
I know, this is why the funny _async firmware request APIs are even a thing...

(I took a quick peek into the RPI 4 code, but unlike this code it seems to fetch
from nvmem/eeprom, is this right? I had a tons-of-fun dealing with caldata and
firmware on UBIFS in UBI Volumes. So I'm prepared to test this cases. :D )

(Another possibility would be to use request_firmware_direct() here.
Though, I don't know if it would be considered API Abuse to -EPROBE_DEFER
on errors of request_firmware_direct() in order to wait for FSes to appear )

Regards,
Christian

Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Vinod Koul]

Hi Christian,

On 01-04-20, 17:39, Christian Lamparter wrote:
> On Wednesday, 1 April 2020 14:57:48 CEST Vinod Koul wrote:
> > On 26-03-20, 17:21, Vinod Koul wrote:
> > > On 26-03-20, 13:29, Mathias Nyman wrote:
> > > > On 23.3.2020 19.05, Vinod Koul wrote:
> > > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > > > loaded. Add these devices in table and invoke renesas firmware loader
> > > > > functions to check and load the firmware into device memory when
> > > > > required.
> > > > > 
> > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > > ---
> > > > >  drivers/usb/host/xhci-pci-renesas.c |  1 +
> > > > >  drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
> > > > >  drivers/usb/host/xhci-pci.h         |  3 +++
> > > > >  3 files changed, 32 insertions(+), 1 deletion(-)
> > > > > 
> > > > 
> > > > It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook
> > > > for this Renesas controller. What was the reason it failed?
> > > > 
> > > > Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4
> > > > where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff()
> > > > 
> > > > https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de
> > > > 
> > > > Is he doing something different than what was done for the Renesas controller?
> > > 
> > > I tried and everytime ended up not getting firmware. Though I did not
> > > investigate a lot. Christian seemed to have tested sometime back as
> > > well.
> > > 
> > > Another problem is that we dont get driver_data in the quirk and there
> > > didnt seem a way to find the firmware name.
> > > 
> > > > > diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> > > > > index c588277ac9b8..d413d53df94b 100644
> > > > > --- a/drivers/usb/host/xhci-pci-renesas.c
> > > > > +++ b/drivers/usb/host/xhci-pci-renesas.c
> > > > > @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
> > > > >  		goto cleanup;
> > > > >  	}
> > > > >  
> > > > > +	xhci_pci_probe(pdev, ctx->id);
> > > > >  	return;
> > > > 
> > > > I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw
> > > > loading callback could we just return -EPROBE_DEFER until firmware is loaded when
> > > > xhci_pci_probe() is originally called?
> > > 
> > > Hmm, initially my thinking was how to tell device core to probe again,
> > > and then digging up I saw wait_for_device_probe() which can be used, let
> > > me try that
> > 
> > Sorry to report back that it doesn't work as planned :(
> > 
> > I modified the code to invoke the request_firmware_nowait() which will load
> > the firmware and provide the firmware in callback. Meanwhile return -EPROBE_DEFER.
> > 
> > After a bit, the core invokes the driver probe again and we hit the
> > roadblock. The request_firmware uses devres and allocates resources for
> > loading the firmware. The problem is that device core checks for this:
> > 
> > bus: 'pci': really_probe: probing driver xhci_hcd_pci with device 0000:01:00.0
> > pci 0000:01:00.0: Resources present before probing
> > 
> > And here the probe fails. In some cases the firmware_callback finishes
> > before this and we can probe again, but that is not very reliable.
> > 
> > I tested another way to use request_firmware() (sync version) and then
> > load the firmware in probe and load. The request is done only for
> > renesas devices if they dont have firmware already running.
> > So rest of the devices wont have any impact.
> > 
> > Now should we continue this way in the patchset or move to sync version.
> > Am okay either way.
> 
> Just a word of caution.
> 
> The problem with the usage of "sync" request_firmware in drivers is that if the
> code is built into the kernel the request_firmware() could be called before the
> (root) filesystem on which the firmware resides is ready.... So this will get
> weird during boot because what is the sync request_firmware() going to do? From what
> I know, this is why the funny _async firmware request APIs are even a thing...

So is your usage a module or inbuilt. I am using it as a module, so
seems okay. For inbuilt, someone needs to do make it in kernel or make
sure the initramfs has this :)

> (I took a quick peek into the RPI 4 code, but unlike this code it seems to fetch
> from nvmem/eeprom, is this right? I had a tons-of-fun dealing with caldata and
> firmware on UBIFS in UBI Volumes. So I'm prepared to test this cases. :D )

Okay I will send you this version to play around with.

> (Another possibility would be to use request_firmware_direct() here.
> Though, I don't know if it would be considered API Abuse to -EPROBE_DEFER
> on errors of request_firmware_direct() in order to wait for FSes to appear )

As I said we need to either accept the current change of having async
version and have firmware callback invoke probe to complete the load
action or move to sync. Which of the lesser devils are we going to be
happy with :)

-- 
~Vinod

Re: [PATCH v8 3/5] usb: xhci: Add support for Renesas controller with memory

[Christian Lamparter]

Hi,

On Wednesday, 1 April 2020 18:18:52 CEST Vinod Koul wrote:
> On 01-04-20, 17:39, Christian Lamparter wrote:
> > On Wednesday, 1 April 2020 14:57:48 CEST Vinod Koul wrote:
> > > On 26-03-20, 17:21, Vinod Koul wrote:
> > > > On 26-03-20, 13:29, Mathias Nyman wrote:
> > > > > On 23.3.2020 19.05, Vinod Koul wrote:
> > > > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > > > > loaded. Add these devices in table and invoke renesas firmware loader
> > > > > > functions to check and load the firmware into device memory when
> > > > > > required.
> > > > > > 
> > > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > > > ---
> > > > > >  drivers/usb/host/xhci-pci-renesas.c |  1 +
> > > > > >  drivers/usb/host/xhci-pci.c         | 29 ++++++++++++++++++++++++++++-
> > > > > >  drivers/usb/host/xhci-pci.h         |  3 +++
> > > > > >  3 files changed, 32 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > 
> > > > > It's unfortunate if firmware loading couldn't be initiated in a PCI fixup hook
> > > > > for this Renesas controller. What was the reason it failed?
> > > > > 
> > > > > Nicolas Saenz Julienne just submitted a solution like that for Raspberry Pi 4
> > > > > where firmware loading is initiated in pci-quirks.c quirk_usb_early_handoff()
> > > > > 
> > > > > https://lore.kernel.org/lkml/20200324182812.20420-1-nsaenzjulienne@suse.de
> > > > > 
> > > > > Is he doing something different than what was done for the Renesas controller?
> > > > 
> > > > I tried and everytime ended up not getting firmware. Though I did not
> > > > investigate a lot. Christian seemed to have tested sometime back as
> > > > well.
> > > > 
> > > > Another problem is that we dont get driver_data in the quirk and there
> > > > didnt seem a way to find the firmware name.
> > > > 
> > > > > > diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> > > > > > index c588277ac9b8..d413d53df94b 100644
> > > > > > --- a/drivers/usb/host/xhci-pci-renesas.c
> > > > > > +++ b/drivers/usb/host/xhci-pci-renesas.c
> > > > > > @@ -336,6 +336,7 @@ static void renesas_fw_callback(const struct firmware *fw,
> > > > > >  		goto cleanup;
> > > > > >  	}
> > > > > >  
> > > > > > +	xhci_pci_probe(pdev, ctx->id);
> > > > > >  	return;
> > > > > 
> > > > > I haven't looked into this but instead of calling xhci_pci_probe() here in the async fw
> > > > > loading callback could we just return -EPROBE_DEFER until firmware is loaded when
> > > > > xhci_pci_probe() is originally called?
> > > > 
> > > > Hmm, initially my thinking was how to tell device core to probe again,
> > > > and then digging up I saw wait_for_device_probe() which can be used, let
> > > > me try that
> > > 
> > > Sorry to report back that it doesn't work as planned :(
> > > 
> > > I modified the code to invoke the request_firmware_nowait() which will load
> > > the firmware and provide the firmware in callback. Meanwhile return -EPROBE_DEFER.
> > > 
> > > After a bit, the core invokes the driver probe again and we hit the
> > > roadblock. The request_firmware uses devres and allocates resources for
> > > loading the firmware. The problem is that device core checks for this:
> > > 
> > > bus: 'pci': really_probe: probing driver xhci_hcd_pci with device 0000:01:00.0
> > > pci 0000:01:00.0: Resources present before probing
> > > 
> > > And here the probe fails. In some cases the firmware_callback finishes
> > > before this and we can probe again, but that is not very reliable.
> > > 
> > > I tested another way to use request_firmware() (sync version) and then
> > > load the firmware in probe and load. The request is done only for
> > > renesas devices if they dont have firmware already running.
> > > So rest of the devices wont have any impact.
> > > 
> > > Now should we continue this way in the patchset or move to sync version.
> > > Am okay either way.
> > 
> > Just a word of caution.
> > 
> > The problem with the usage of "sync" request_firmware in drivers is that if the
> > code is built into the kernel the request_firmware() could be called before the
> > (root) filesystem on which the firmware resides is ready.... So this will get
> > weird during boot because what is the sync request_firmware() going to do? From what
> > I know, this is why the funny _async firmware request APIs are even a thing...
> 
> So is your usage a module or inbuilt. I am using it as a module, so
> seems okay. For inbuilt, someone needs to do make it in kernel or make
> sure the initramfs has this :)

Yes, after testing on the device, I can state that everything went as intended :-).

Cheers,
Christian