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

2020-05-05 Thread Greg Kroah-Hartman
On Tue, May 05, 2020 at 05:03:54PM +0530, Vinod Koul wrote:
> On 05-05-20, 13:04, Greg Kroah-Hartman wrote:
> > On Mon, May 04, 2020 at 08:04:38PM +0530, Vinod Koul wrote:
> 
> > > > > --- a/drivers/usb/host/Makefile
> > > > > +++ b/drivers/usb/host/Makefile
> > > > > @@ -70,7 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)  += ohci-da8xx.o
> > > > >  obj-$(CONFIG_USB_UHCI_HCD)   += uhci-hcd.o
> > > > >  obj-$(CONFIG_USB_FHCI_HCD)   += fhci.o
> > > > >  obj-$(CONFIG_USB_XHCI_HCD)   += xhci-hcd.o
> > > > > -obj-$(CONFIG_USB_XHCI_PCI)   += xhci-pci.o
> > > > > +obj-$(CONFIG_USB_XHCI_PCI)   += xhci-pci.o xhci-pci-renesas.o
> > > > 
> > > > Hmm, now we end up with two modules, xhci-pci and xhci-pci-renesas, 
> > > > even if
> > > > xhci-pci-renesas just includes helper functions to load firmware for 
> > > > renesas.
> > > 
> > > Right, these are two modules. Do you forsee an issue with two ko's
> > 
> > Two kos should be fine, but as you aren't giving people the option to
> > not select this, it's a bit harsh to add it.
> > 
> > Can this be a separate module/config option?  Why force everyone to need
> > this additional code if they do not have this hardware?
> 
> Since the code is moved out and is based on PCI ID of the device, this
> wont be invoked at all for folks not having this hardware. But adding a
> config option would work too and avoid renaming file.

Yes, it would not be "invoked", but it still would always be loaded into
memory.  Please only load this code if the hardware is present in the
system.

thanks,

greg k-h


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

2020-05-05 Thread Vinod Koul
On 05-05-20, 13:04, Greg Kroah-Hartman wrote:
> On Mon, May 04, 2020 at 08:04:38PM +0530, Vinod Koul wrote:

> > > > --- a/drivers/usb/host/Makefile
> > > > +++ b/drivers/usb/host/Makefile
> > > > @@ -70,7 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)+= ohci-da8xx.o
> > > >  obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
> > > >  obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
> > > >  obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
> > > > -obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
> > > > +obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-pci-renesas.o
> > > 
> > > Hmm, now we end up with two modules, xhci-pci and xhci-pci-renesas, even 
> > > if
> > > xhci-pci-renesas just includes helper functions to load firmware for 
> > > renesas.
> > 
> > Right, these are two modules. Do you forsee an issue with two ko's
> 
> Two kos should be fine, but as you aren't giving people the option to
> not select this, it's a bit harsh to add it.
> 
> Can this be a separate module/config option?  Why force everyone to need
> this additional code if they do not have this hardware?

Since the code is moved out and is based on PCI ID of the device, this
wont be invoked at all for folks not having this hardware. But adding a
config option would work too and avoid renaming file.

I think this looks as a better option to me atm. We no longer have load
order issue with current approach so we dont care about that as well.

Mathias, let me know if you are okay with approach, I can respin this,
or if you have better idea do let us know

Thanks
-- 
~Vinod


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

2020-05-05 Thread Greg Kroah-Hartman
On Mon, May 04, 2020 at 08:04:38PM +0530, Vinod Koul wrote:
> Hi Mathias,
> 
> On 04-05-20, 16:01, Mathias Nyman wrote:
> > On 30.4.2020 19.59, Vinod Koul wrote:
> > > From: Christian Lamparter 
> > > 
> > > This add a new driver for renesas xhci which is basically a firmware
> > > loader for uPD720201 and uPD720202 w/o ROM. The xhci-pci driver will
> > > invoke this driver for loading/unloading on relevant devices.
> > > 
> > > 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.
> > > 
> > > Signed-off-by: Christian Lamparter 
> > > Signed-off-by: Bjorn Andersson 
> > > [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
> > >   make fw loader as sync probe so that we execute in probe and
> > > prevent race
> > >   export symbols for xhci-pci to use]
> > > Signed-off-by: Vinod Koul 
> > > ---
> > >  drivers/usb/host/Makefile   |   2 +-
> > >  drivers/usb/host/xhci-pci-renesas.c | 365 
> > >  drivers/usb/host/xhci-pci.h |  16 ++
> > >  3 files changed, 382 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..f3a5a2f01874 100644
> > > --- a/drivers/usb/host/Makefile
> > > +++ b/drivers/usb/host/Makefile
> > > @@ -70,7 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)  += ohci-da8xx.o
> > >  obj-$(CONFIG_USB_UHCI_HCD)   += uhci-hcd.o
> > >  obj-$(CONFIG_USB_FHCI_HCD)   += fhci.o
> > >  obj-$(CONFIG_USB_XHCI_HCD)   += xhci-hcd.o
> > > -obj-$(CONFIG_USB_XHCI_PCI)   += xhci-pci.o
> > > +obj-$(CONFIG_USB_XHCI_PCI)   += xhci-pci.o xhci-pci-renesas.o
> > 
> > Hmm, now we end up with two modules, xhci-pci and xhci-pci-renesas, even if
> > xhci-pci-renesas just includes helper functions to load firmware for 
> > renesas.
> 
> Right, these are two modules. Do you forsee an issue with two ko's

Two kos should be fine, but as you aren't giving people the option to
not select this, it's a bit harsh to add it.

Can this be a separate module/config option?  Why force everyone to need
this additional code if they do not have this hardware?

thanks,

greg k-h


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

2020-05-04 Thread Vinod Koul
Hi Mathias,

On 04-05-20, 16:01, Mathias Nyman wrote:
> On 30.4.2020 19.59, Vinod Koul wrote:
> > From: Christian Lamparter 
> > 
> > This add a new driver for renesas xhci which is basically a firmware
> > loader for uPD720201 and uPD720202 w/o ROM. The xhci-pci driver will
> > invoke this driver for loading/unloading on relevant devices.
> > 
> > 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.
> > 
> > Signed-off-by: Christian Lamparter 
> > Signed-off-by: Bjorn Andersson 
> > [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
> > make fw loader as sync probe so that we execute in probe and
> > prevent race
> > export symbols for xhci-pci to use]
> > Signed-off-by: Vinod Koul 
> > ---
> >  drivers/usb/host/Makefile   |   2 +-
> >  drivers/usb/host/xhci-pci-renesas.c | 365 
> >  drivers/usb/host/xhci-pci.h |  16 ++
> >  3 files changed, 382 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..f3a5a2f01874 100644
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
> > @@ -70,7 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)+= ohci-da8xx.o
> >  obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
> >  obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
> >  obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
> > -obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
> > +obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-pci-renesas.o
> 
> Hmm, now we end up with two modules, xhci-pci and xhci-pci-renesas, even if
> xhci-pci-renesas just includes helper functions to load firmware for renesas.

Right, these are two modules. Do you forsee an issue with two ko's
> 
> My kbuild knowledge is limited, but one way to solve this would be to 
> rename xhci-pci.c to xhci-pci-core.c and add:

Yeah i wanted to avoid the rename (stopped short of suggesting that)..
> 
> xhci-pci-y := xhci-pci-core.o xhci-pci-renesas.o
> 
> unless someone can suggest a better way to solve this

I dont have any better idea atm!

So if you are okay with rename of file, I can do that first and then
these patches

Thanks
-- 
~Vinod


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

2020-05-04 Thread Mathias Nyman
On 30.4.2020 19.59, Vinod Koul wrote:
> From: Christian Lamparter 
> 
> This add a new driver for renesas xhci which is basically a firmware
> loader for uPD720201 and uPD720202 w/o ROM. The xhci-pci driver will
> invoke this driver for loading/unloading on relevant devices.
> 
> 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.
> 
> Signed-off-by: Christian Lamparter 
> Signed-off-by: Bjorn Andersson 
> [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
>   make fw loader as sync probe so that we execute in probe and
> prevent race
>   export symbols for xhci-pci to use]
> Signed-off-by: Vinod Koul 
> ---
>  drivers/usb/host/Makefile   |   2 +-
>  drivers/usb/host/xhci-pci-renesas.c | 365 
>  drivers/usb/host/xhci-pci.h |  16 ++
>  3 files changed, 382 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..f3a5a2f01874 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -70,7 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)  += ohci-da8xx.o
>  obj-$(CONFIG_USB_UHCI_HCD)   += uhci-hcd.o
>  obj-$(CONFIG_USB_FHCI_HCD)   += fhci.o
>  obj-$(CONFIG_USB_XHCI_HCD)   += xhci-hcd.o
> -obj-$(CONFIG_USB_XHCI_PCI)   += xhci-pci.o
> +obj-$(CONFIG_USB_XHCI_PCI)   += xhci-pci.o xhci-pci-renesas.o

Hmm, now we end up with two modules, xhci-pci and xhci-pci-renesas, even if
xhci-pci-renesas just includes helper functions to load firmware for renesas.

My kbuild knowledge is limited, but one way to solve this would be to 
rename xhci-pci.c to xhci-pci-core.c and add:

xhci-pci-y := xhci-pci-core.o xhci-pci-renesas.o

unless someone can suggest a better way to solve this

-Mathias



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

2020-04-30 Thread Vinod Koul
From: Christian Lamparter 

This add a new driver for renesas xhci which is basically a firmware
loader for uPD720201 and uPD720202 w/o ROM. The xhci-pci driver will
invoke this driver for loading/unloading on relevant devices.

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.

Signed-off-by: Christian Lamparter 
Signed-off-by: Bjorn Andersson 
[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
make fw loader as sync probe so that we execute in probe and
prevent race
export symbols for xhci-pci to use]
Signed-off-by: Vinod Koul 
---
 drivers/usb/host/Makefile   |   2 +-
 drivers/usb/host/xhci-pci-renesas.c | 365 
 drivers/usb/host/xhci-pci.h |  16 ++
 3 files changed, 382 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..f3a5a2f01874 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -70,7 +70,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)+= ohci-da8xx.o
 obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
 obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
 obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
-obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
+obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-pci-renesas.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 ..88341d79b651
--- /dev/null
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -0,0 +1,365 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2019-2020 Linaro Limited */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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_INVALID0
+  #define RENESAS_FW_STATUS_SUCCESSBIT(4)
+  #define RENESAS_FW_STATUS_ERROR  BIT(5)
+#define RENESAS_FW_STATUS_SET_DATA0BIT(8)
+#define RENESAS_FW_STATUS_SET_DATA1BIT(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_MAGIC0x5A65726F
+#define RENESAS_ROM_WRITE_MAGIC0x53524F4D
+
+#define