Re: [PATCH v12 2/5] usb: renesas-xhci: Add the renesas xhci driver
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
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
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
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
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
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