diff mbox series

[v9,3/5] usb: xhci: Add support for Renesas controller with memory

Message ID 20200414164152.2786474-4-vkoul@kernel.org (mailing list archive)
State Superseded
Headers show
Series usb: xhci: Add support for Renesas USB controllers | expand

Commit Message

Vinod Koul April 14, 2020, 4:41 p.m. UTC
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.c | 33 +++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h     |  1 +
 2 files changed, 34 insertions(+)

Comments

Mathias Nyman April 23, 2020, 2:07 p.m. UTC | #1
On 14.4.2020 19.41, 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.c | 33 +++++++++++++++++++++++++++++++++
>  drivers/usb/host/xhci.h     |  1 +
>  2 files changed, 34 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index b6c2f5c530e3..11521e2e1720 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
> @@ -328,6 +329,21 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	int retval;
>  	struct xhci_hcd *xhci;
>  	struct usb_hcd *hcd;
> +	struct xhci_driver_data *driver_data;
> +
> +	driver_data = (struct xhci_driver_data *)id->driver_data;
> +
> +	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> +		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;

This is no longer true, right?

To me it looks like renesas_xhci_pci_probe() returns 0 on success, both if
firmware was already running or if successfully loaded, and negative on error

While changing this the function name "renesas_xhci_pci_probe()" should be
changed as well. This isn't anymore a separate firmware loading driver, just a
a lot of renesas firmware loading code.

You could call renesas_xhci_check_request_fw() directly instead:

	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
		retval = renesas_xhci_check_request_fw(dev, id);
		if (retval)
			return retval;
	}

-Mathias
Vinod Koul April 23, 2020, 2:31 p.m. UTC | #2
On 23-04-20, 17:07, Mathias Nyman wrote:
> On 14.4.2020 19.41, 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.c | 33 +++++++++++++++++++++++++++++++++
> >  drivers/usb/host/xhci.h     |  1 +
> >  2 files changed, 34 insertions(+)
> > 
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index b6c2f5c530e3..11521e2e1720 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
> > @@ -328,6 +329,21 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >  	int retval;
> >  	struct xhci_hcd *xhci;
> >  	struct usb_hcd *hcd;
> > +	struct xhci_driver_data *driver_data;
> > +
> > +	driver_data = (struct xhci_driver_data *)id->driver_data;
> > +
> > +	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> > +		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;
> 
> This is no longer true, right?

Correct.

> To me it looks like renesas_xhci_pci_probe() returns 0 on success, both if
> firmware was already running or if successfully loaded, and negative on error

Yes now it does that and I will update this part..
> 
> While changing this the function name "renesas_xhci_pci_probe()" should be
> changed as well. This isn't anymore a separate firmware loading driver, just a
> a lot of renesas firmware loading code.
> 
> You could call renesas_xhci_check_request_fw() directly instead:
> 
> 	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> 		retval = renesas_xhci_check_request_fw(dev, id);
> 		if (retval)
> 			return retval;
> 	}

Yes I can remove this layer and directly invoke the internal function..

Thanks for the comments
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b6c2f5c530e3..11521e2e1720 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
@@ -328,6 +329,21 @@  static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	int retval;
 	struct xhci_hcd *xhci;
 	struct usb_hcd *hcd;
+	struct xhci_driver_data *driver_data;
+
+	driver_data = (struct xhci_driver_data *)id->driver_data;
+
+	if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
+		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);
@@ -387,6 +403,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;
@@ -540,14 +561,26 @@  static void xhci_pci_shutdown(struct usb_hcd *hcd)
 
 /*-------------------------------------------------------------------------*/
 
+static const struct xhci_driver_data reneses_data = {
+	.quirks  = XHCI_RENESAS_FW_QUIRK,
+	.firmware = "renesas_usb_fw.mem",
+};
+
 /* PCI driver selection metadata; PCI hotplugging uses this */
 static const struct pci_device_id pci_ids[] = {
+	{ PCI_DEVICE(0x1912, 0x0014),
+		.driver_data =  (unsigned long)&reneses_data,
+	},
+	{ PCI_DEVICE(0x1912, 0x0015),
+		.driver_data =  (unsigned long)&reneses_data,
+	},
 	/* 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.h b/drivers/usb/host/xhci.h
index 3289bb516201..4047363c7423 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1873,6 +1873,7 @@  struct xhci_hcd {
 #define XHCI_DEFAULT_PM_RUNTIME_ALLOW	BIT_ULL(33)
 #define XHCI_RESET_PLL_ON_DISCONNECT	BIT_ULL(34)
 #define XHCI_SNPS_BROKEN_SUSPEND    BIT_ULL(35)
+#define XHCI_RENESAS_FW_QUIRK	BIT_ULL(36)
 
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;