2020-04-14 16:57:16

by Vinod Koul

[permalink] [raw]
Subject: [PATCH v9 3/5] usb: xhci: Add support for Renesas controller with memory

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 <[email protected]>
---
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;
+ 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;
--
2.25.1


2020-04-23 14:25:48

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH v9 3/5] usb: xhci: Add support for Renesas controller with memory

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 <[email protected]>
> ---
> 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

2020-04-23 14:33:24

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v9 3/5] usb: xhci: Add support for Renesas controller with memory

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 <[email protected]>
> > ---
> > 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

--
~Vinod