diff mbox series

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

Message ID 20200506060025.1535960-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 May 6, 2020, 6 a.m. UTC
Some rensas controller like uPD720201 and uPD720202 need firmware to be
loaded. Add these devices in pci 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 | 35 ++++++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci.h     |  1 +
 2 files changed, 35 insertions(+), 1 deletion(-)

Comments

Anders Roxell May 18, 2020, 5:53 p.m. UTC | #1
On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
>
> Some rensas controller like uPD720201 and uPD720202 need firmware to be
> loaded. Add these devices in pci 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>

Hi, I got a build error when I built an arm64 allmodconfig kernel.

building obj-arm64-next-20200518

aarch64-linux-gnu-ld: drivers/usb/host/xhci-pci.o: in function
`xhci_pci_remove':
/srv/src/kernel/next/obj-arm64-next-20200518/../drivers/usb/host/xhci-pci.c:411:
undefined reference to `renesas_xhci_pci_exit'
aarch64-linux-gnu-ld:
/srv/src/kernel/next/obj-arm64-next-20200518/../drivers/usb/host/xhci-pci.c:411:(.text+0xd8):
relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
`renesas_xhci_pci_exit'
aarch64-linux-gnu-ld: drivers/usb/host/xhci-pci.o: in function `xhci_pci_probe':
/srv/src/kernel/next/obj-arm64-next-20200518/../drivers/usb/host/xhci-pci.c:345:
undefined reference to `renesas_xhci_check_request_fw'
aarch64-linux-gnu-ld:
/srv/src/kernel/next/obj-arm64-next-20200518/../drivers/usb/host/xhci-pci.c:345:(.text+0x2298):
relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
`renesas_xhci_check_request_fw'
make[1]: *** [/srv/src/kernel/next/Makefile:1126: vmlinux] Error 1
make[1]: Target 'Image' not remade because of errors.
make: *** [Makefile:185: __sub-make] Error 2
make: Target 'Image' not remade because of errors.

When I reverted this patch from todays next tag next-20200518 I
managed to build.


Cheers,
Anders

> ---
>  drivers/usb/host/xhci-pci.c | 35 ++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/xhci.h     |  1 +
>  2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index b6c2f5c530e3..ef513c2fb843 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
> @@ -87,7 +88,16 @@ static int xhci_pci_reinit(struct xhci_hcd *xhci, struct pci_dev *pdev)
>
>  static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>  {
> -       struct pci_dev          *pdev = to_pci_dev(dev);
> +       struct pci_dev                  *pdev = to_pci_dev(dev);
> +       struct xhci_driver_data         *driver_data;
> +       const struct pci_device_id      *id;
> +
> +       id = pci_match_id(pdev->driver->id_table, pdev);
> +
> +       if (id && id->driver_data) {
> +               driver_data = (struct xhci_driver_data *)id->driver_data;
> +               xhci->quirks |= driver_data->quirks;
> +       }
>
>         /* Look for vendor-specific quirks */
>         if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
> @@ -328,6 +338,14 @@ 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_check_request_fw(dev, id);
> +               if (retval)
> +                       return retval;
> +       }
>
>         /* Prevent runtime suspending between USB-2 and USB-3 initialization */
>         pm_runtime_get_noresume(&dev->dev);
> @@ -389,6 +407,9 @@ static void xhci_pci_remove(struct pci_dev *dev)
>         struct xhci_hcd *xhci;
>
>         xhci = hcd_to_xhci(pci_get_drvdata(dev));
> +       if (xhci->quirks & XHCI_RENESAS_FW_QUIRK)
> +               renesas_xhci_pci_exit(dev);
> +
>         xhci->xhc_state |= XHCI_STATE_REMOVING;
>
>         if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
> @@ -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.4
>
Vinod Koul May 18, 2020, 7:57 p.m. UTC | #2
Hi Anders,

On 18-05-20, 19:53, Anders Roxell wrote:
> On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
> >
> > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > loaded. Add these devices in pci 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>
> 
> Hi, I got a build error when I built an arm64 allmodconfig kernel.

Thanks for this. This is happening as we have default y for USB_XHCI_PCI
and then we make USB_XHCI_PCI_RENESAS=m. That should be not allowed as
we export as symbol so both can be inbuilt or modules but USB_XHCI_PCI=y
and USB_XHCI_PCI_RENESAS=m cant. While it is valid that USB_XHCI_PCI=y|m
and USB_XHCI_PCI_RENESAS=n

So this seems to get fixed by below for me. I have tested with
 - both y and m (easy)
 - make USB_XHCI_PCI_RENESAS=n, USB_XHCI_PCI=y|m works
 - try making USB_XHCI_PCI=y and USB_XHCI_PCI_RENESAS=m, then
   USB_XHCI_PCI=m by kbuild :)
 - try making USB_XHCI_PCI=m and USB_XHCI_PCI_RENESAS=y, kbuild gives
   error prompt that it will be m due to depends

Thanks to all the fixes done by Arnd which pointed me to this. Pls
verify and I will send the fix with you as reported :)

---- >8 ----

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b5c542d6a1c5..92783d175b3f 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -40,11 +40,11 @@ config USB_XHCI_DBGCAP
 config USB_XHCI_PCI
        tristate
        depends on USB_PCI
+       depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS
        default y
 
 config USB_XHCI_PCI_RENESAS
        tristate "Support for additional Renesas xHCI controller with firwmare"
-       depends on USB_XHCI_PCI
        ---help---
          Say 'Y' to enable the support for the Renesas xHCI controller with
          firwmare. Make sure you have the firwmare for the device and
Anders Roxell May 18, 2020, 10:37 p.m. UTC | #3
On Mon, 18 May 2020 at 21:57, Vinod Koul <vkoul@kernel.org> wrote:
>
> Hi Anders,

Hi Vinod,

>
> On 18-05-20, 19:53, Anders Roxell wrote:
> > On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > loaded. Add these devices in pci 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>
> >
> > Hi, I got a build error when I built an arm64 allmodconfig kernel.
>
> Thanks for this. This is happening as we have default y for USB_XHCI_PCI
> and then we make USB_XHCI_PCI_RENESAS=m. That should be not allowed as
> we export as symbol so both can be inbuilt or modules but USB_XHCI_PCI=y
> and USB_XHCI_PCI_RENESAS=m cant. While it is valid that USB_XHCI_PCI=y|m
> and USB_XHCI_PCI_RENESAS=n
>
> So this seems to get fixed by below for me. I have tested with
>  - both y and m (easy)
>  - make USB_XHCI_PCI_RENESAS=n, USB_XHCI_PCI=y|m works
>  - try making USB_XHCI_PCI=y and USB_XHCI_PCI_RENESAS=m, then
>    USB_XHCI_PCI=m by kbuild :)
>  - try making USB_XHCI_PCI=m and USB_XHCI_PCI_RENESAS=y, kbuild gives
>    error prompt that it will be m due to depends
>
> Thanks to all the fixes done by Arnd which pointed me to this. Pls
> verify

I was able to build an arm64 allmodconfig kernel with this change.

Cheers,
Anders

> and I will send the fix with you as reported :)
>
> ---- >8 ----
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index b5c542d6a1c5..92783d175b3f 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -40,11 +40,11 @@ config USB_XHCI_DBGCAP
>  config USB_XHCI_PCI
>         tristate
>         depends on USB_PCI
> +       depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS
>         default y
>
>  config USB_XHCI_PCI_RENESAS
>         tristate "Support for additional Renesas xHCI controller with firwmare"
> -       depends on USB_XHCI_PCI
>         ---help---
>           Say 'Y' to enable the support for the Renesas xHCI controller with
>           firwmare. Make sure you have the firwmare for the device and
>
> --
> ~Vinod
Bjorn Andersson May 18, 2020, 10:42 p.m. UTC | #4
On Mon 18 May 12:57 PDT 2020, Vinod Koul wrote:

> Hi Anders,
> 
> On 18-05-20, 19:53, Anders Roxell wrote:
> > On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > loaded. Add these devices in pci 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>
> > 
> > Hi, I got a build error when I built an arm64 allmodconfig kernel.
> 
> Thanks for this. This is happening as we have default y for USB_XHCI_PCI
> and then we make USB_XHCI_PCI_RENESAS=m. That should be not allowed as
> we export as symbol so both can be inbuilt or modules but USB_XHCI_PCI=y
> and USB_XHCI_PCI_RENESAS=m cant. While it is valid that USB_XHCI_PCI=y|m
> and USB_XHCI_PCI_RENESAS=n
> 
> So this seems to get fixed by below for me. I have tested with
>  - both y and m (easy)
>  - make USB_XHCI_PCI_RENESAS=n, USB_XHCI_PCI=y|m works
>  - try making USB_XHCI_PCI=y and USB_XHCI_PCI_RENESAS=m, then
>    USB_XHCI_PCI=m by kbuild :)
>  - try making USB_XHCI_PCI=m and USB_XHCI_PCI_RENESAS=y, kbuild gives
>    error prompt that it will be m due to depends
> 
> Thanks to all the fixes done by Arnd which pointed me to this. Pls
> verify and I will send the fix with you as reported :)
> 
> ---- >8 ----
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index b5c542d6a1c5..92783d175b3f 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -40,11 +40,11 @@ config USB_XHCI_DBGCAP
>  config USB_XHCI_PCI
>         tristate
>         depends on USB_PCI
> +       depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS

This correctly captures the variations you describe above.

Regards,
Bjorn

>         default y
>  
>  config USB_XHCI_PCI_RENESAS
>         tristate "Support for additional Renesas xHCI controller with firwmare"
> -       depends on USB_XHCI_PCI
>         ---help---
>           Say 'Y' to enable the support for the Renesas xHCI controller with
>           firwmare. Make sure you have the firwmare for the device and
> 
> -- 
> ~Vinod
Vinod Koul May 19, 2020, 4:53 a.m. UTC | #5
On 19-05-20, 00:37, Anders Roxell wrote:
> On Mon, 18 May 2020 at 21:57, Vinod Koul <vkoul@kernel.org> wrote:
> >
> > Hi Anders,
> 
> Hi Vinod,
> 
> >
> > On 18-05-20, 19:53, Anders Roxell wrote:
> > > On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
> > > >
> > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > > loaded. Add these devices in pci 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>
> > >
> > > Hi, I got a build error when I built an arm64 allmodconfig kernel.
> >
> > Thanks for this. This is happening as we have default y for USB_XHCI_PCI
> > and then we make USB_XHCI_PCI_RENESAS=m. That should be not allowed as
> > we export as symbol so both can be inbuilt or modules but USB_XHCI_PCI=y
> > and USB_XHCI_PCI_RENESAS=m cant. While it is valid that USB_XHCI_PCI=y|m
> > and USB_XHCI_PCI_RENESAS=n
> >
> > So this seems to get fixed by below for me. I have tested with
> >  - both y and m (easy)
> >  - make USB_XHCI_PCI_RENESAS=n, USB_XHCI_PCI=y|m works
> >  - try making USB_XHCI_PCI=y and USB_XHCI_PCI_RENESAS=m, then
> >    USB_XHCI_PCI=m by kbuild :)
> >  - try making USB_XHCI_PCI=m and USB_XHCI_PCI_RENESAS=y, kbuild gives
> >    error prompt that it will be m due to depends
> >
> > Thanks to all the fixes done by Arnd which pointed me to this. Pls
> > verify
> 
> I was able to build an arm64 allmodconfig kernel with this change.

I will send the formal patch and add your name in reported and
tested. Thanks for the quick verification

> 
> Cheers,
> Anders
> 
> > and I will send the fix with you as reported :)
> >
> > ---- >8 ----
> >
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > index b5c542d6a1c5..92783d175b3f 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -40,11 +40,11 @@ config USB_XHCI_DBGCAP
> >  config USB_XHCI_PCI
> >         tristate
> >         depends on USB_PCI
> > +       depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS
> >         default y
> >
> >  config USB_XHCI_PCI_RENESAS
> >         tristate "Support for additional Renesas xHCI controller with firwmare"
> > -       depends on USB_XHCI_PCI
> >         ---help---
> >           Say 'Y' to enable the support for the Renesas xHCI controller with
> >           firwmare. Make sure you have the firwmare for the device and
> >
> > --
> > ~Vinod
Arnd Bergmann May 19, 2020, 7:44 a.m. UTC | #6
On Tue, May 19, 2020 at 6:53 AM Vinod Koul <vkoul@kernel.org> wrote:
> On 19-05-20, 00:37, Anders Roxell wrote:
> > On Mon, 18 May 2020 at 21:57, Vinod Koul <vkoul@kernel.org> wrote:
> > > On 18-05-20, 19:53, Anders Roxell wrote:
> > > > On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
> > > > >
> > > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > > > loaded. Add these devices in pci 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>
> > > >
> > > > Hi, I got a build error when I built an arm64 allmodconfig kernel.
> > >
> > > Thanks for this. This is happening as we have default y for USB_XHCI_PCI
> > > and then we make USB_XHCI_PCI_RENESAS=m. That should be not allowed as
> > > we export as symbol so both can be inbuilt or modules but USB_XHCI_PCI=y
> > > and USB_XHCI_PCI_RENESAS=m cant. While it is valid that USB_XHCI_PCI=y|m
> > > and USB_XHCI_PCI_RENESAS=n
> > >
> > > So this seems to get fixed by below for me. I have tested with
> > >  - both y and m (easy)
> > >  - make USB_XHCI_PCI_RENESAS=n, USB_XHCI_PCI=y|m works
> > >  - try making USB_XHCI_PCI=y and USB_XHCI_PCI_RENESAS=m, then
> > >    USB_XHCI_PCI=m by kbuild :)
> > >  - try making USB_XHCI_PCI=m and USB_XHCI_PCI_RENESAS=y, kbuild gives
> > >    error prompt that it will be m due to depends
> > >
> > > Thanks to all the fixes done by Arnd which pointed me to this. Pls
> > > verify
> >
> > I was able to build an arm64 allmodconfig kernel with this change.
>
> I will send the formal patch and add your name in reported and
> tested. Thanks for the quick verification

I just checked the patch and I think this will work correctly in all cases,
but it still seems a bit backwards:

> > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > > index b5c542d6a1c5..92783d175b3f 100644
> > > --- a/drivers/usb/host/Kconfig
> > > +++ b/drivers/usb/host/Kconfig
> > > @@ -40,11 +40,11 @@ config USB_XHCI_DBGCAP
> > >  config USB_XHCI_PCI
> > >         tristate
> > >         depends on USB_PCI
> > > +       depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS
> > >         default y
> > >
> > >  config USB_XHCI_PCI_RENESAS
> > >         tristate "Support for additional Renesas xHCI controller with firwmare"
> > > -       depends on USB_XHCI_PCI
> > >         ---help---
> > >           Say 'Y' to enable the support for the Renesas xHCI controller with
> > >           firwmare. Make sure you have the firwmare for the device and
> > >

I think it would have been better to follow the normal driver abstraction here
and make the renesas xhci a specialized version of the xhci driver with
its own platform_driver instance that calls into the generic xhci_pci module,
rather than having the generic code treat it as a quirk.

That would be more like how we handle all the ehci and ohci variants, though
I'm not sure how exactly it would work with two drivers having pci_device_id
tables with non-exclusive members. Presumably the generic driver would
still have to know that it needs to fail its probe() function on devices that
need the firmware.

        Arnd
Vinod Koul May 19, 2020, 12:42 p.m. UTC | #7
HI Arnd,

On 19-05-20, 09:44, Arnd Bergmann wrote:
> On Tue, May 19, 2020 at 6:53 AM Vinod Koul <vkoul@kernel.org> wrote:
> > On 19-05-20, 00:37, Anders Roxell wrote:
> > > On Mon, 18 May 2020 at 21:57, Vinod Koul <vkoul@kernel.org> wrote:
> > > > On 18-05-20, 19:53, Anders Roxell wrote:
> > > > > On Wed, 6 May 2020 at 08:01, Vinod Koul <vkoul@kernel.org> wrote:
> > > > > >
> > > > > > Some rensas controller like uPD720201 and uPD720202 need firmware to be
> > > > > > loaded. Add these devices in pci 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>
> > > > >
> > > > > Hi, I got a build error when I built an arm64 allmodconfig kernel.
> > > >
> > > > Thanks for this. This is happening as we have default y for USB_XHCI_PCI
> > > > and then we make USB_XHCI_PCI_RENESAS=m. That should be not allowed as
> > > > we export as symbol so both can be inbuilt or modules but USB_XHCI_PCI=y
> > > > and USB_XHCI_PCI_RENESAS=m cant. While it is valid that USB_XHCI_PCI=y|m
> > > > and USB_XHCI_PCI_RENESAS=n
> > > >
> > > > So this seems to get fixed by below for me. I have tested with
> > > >  - both y and m (easy)
> > > >  - make USB_XHCI_PCI_RENESAS=n, USB_XHCI_PCI=y|m works
> > > >  - try making USB_XHCI_PCI=y and USB_XHCI_PCI_RENESAS=m, then
> > > >    USB_XHCI_PCI=m by kbuild :)
> > > >  - try making USB_XHCI_PCI=m and USB_XHCI_PCI_RENESAS=y, kbuild gives
> > > >    error prompt that it will be m due to depends
> > > >
> > > > Thanks to all the fixes done by Arnd which pointed me to this. Pls
> > > > verify
> > >
> > > I was able to build an arm64 allmodconfig kernel with this change.
> >
> > I will send the formal patch and add your name in reported and
> > tested. Thanks for the quick verification
> 
> I just checked the patch and I think this will work correctly in all cases,
> but it still seems a bit backwards:
> 
> > > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > > > index b5c542d6a1c5..92783d175b3f 100644
> > > > --- a/drivers/usb/host/Kconfig
> > > > +++ b/drivers/usb/host/Kconfig
> > > > @@ -40,11 +40,11 @@ config USB_XHCI_DBGCAP
> > > >  config USB_XHCI_PCI
> > > >         tristate
> > > >         depends on USB_PCI
> > > > +       depends on USB_XHCI_PCI_RENESAS || !USB_XHCI_PCI_RENESAS
> > > >         default y
> > > >
> > > >  config USB_XHCI_PCI_RENESAS
> > > >         tristate "Support for additional Renesas xHCI controller with firwmare"
> > > > -       depends on USB_XHCI_PCI
> > > >         ---help---
> > > >           Say 'Y' to enable the support for the Renesas xHCI controller with
> > > >           firwmare. Make sure you have the firwmare for the device and
> > > >
> 
> I think it would have been better to follow the normal driver abstraction here
> and make the renesas xhci a specialized version of the xhci driver with
> its own platform_driver instance that calls into the generic xhci_pci module,
> rather than having the generic code treat it as a quirk.
> 
> That would be more like how we handle all the ehci and ohci variants, though
> I'm not sure how exactly it would work with two drivers having pci_device_id
> tables with non-exclusive members. Presumably the generic driver would
> still have to know that it needs to fail its probe() function on devices that
> need the firmware.

Yeah one of the earlier versions did try this and it wasn't nice. The
xhci driver claims the devices as it registers for the class. Now only
solution is to ensure we load the renesas first and resort to makefile
hacks..
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index b6c2f5c530e3..ef513c2fb843 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
@@ -87,7 +88,16 @@  static int xhci_pci_reinit(struct xhci_hcd *xhci, struct pci_dev *pdev)
 
 static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
-	struct pci_dev		*pdev = to_pci_dev(dev);
+	struct pci_dev                  *pdev = to_pci_dev(dev);
+	struct xhci_driver_data         *driver_data;
+	const struct pci_device_id      *id;
+
+	id = pci_match_id(pdev->driver->id_table, pdev);
+
+	if (id && id->driver_data) {
+		driver_data = (struct xhci_driver_data *)id->driver_data;
+		xhci->quirks |= driver_data->quirks;
+	}
 
 	/* Look for vendor-specific quirks */
 	if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
@@ -328,6 +338,14 @@  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_check_request_fw(dev, id);
+		if (retval)
+			return retval;
+	}
 
 	/* Prevent runtime suspending between USB-2 and USB-3 initialization */
 	pm_runtime_get_noresume(&dev->dev);
@@ -389,6 +407,9 @@  static void xhci_pci_remove(struct pci_dev *dev)
 	struct xhci_hcd *xhci;
 
 	xhci = hcd_to_xhci(pci_get_drvdata(dev));
+	if (xhci->quirks & XHCI_RENESAS_FW_QUIRK)
+		renesas_xhci_pci_exit(dev);
+
 	xhci->xhc_state |= XHCI_STATE_REMOVING;
 
 	if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
@@ -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;