Re: [PATCH v10 3/5] usb: xhci: Add support for Renesas controller with memory
On 30-04-20, 11:16, Mathias Nyman wrote: > On 30.4.2020 9.20, Vinod Koul wrote: > > On 29-04-20, 19:58, Vinod Koul wrote: > >> On 29-04-20, 16:53, Mathias Nyman wrote: > >>> On 24.4.2020 13.14, Vinod Koul wrote: > > > /* Prevent runtime suspending between USB-2 and USB-3 > initialization */ > pm_runtime_get_noresume(&dev->dev); > @@ -388,6 +401,9 @@ static void xhci_pci_remove(struct pci_dev *dev) > { > struct xhci_hcd *xhci; > > +if (renesas_device) > +renesas_xhci_pci_exit(dev); > + > >>> > >>> Ah, I see, what we really should do is make sure the quirks in the driver > >>> data get > >>> added to xhci->quirks, and then just check for the correct quirk in > >>> xhci_pci_remove. > >> > >> Ah sure that does sound better, I will update this as well and send an > >> update with these changes > > > > This works for me.. But I have kept the code as in the xhci_pci_probe(), > > ofcourse removed bool renesas_device. > > That's fine, xhci is just hcd->hcd_priv, and it doesn't exists before > usb_hcd_pci_probe() is called > > usb_hcd_pci_probe() > usb_create_hcd() > hcd = kzalloc(sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL); Right, thanks for confirming -- ~Vinod
Re: [PATCH v10 3/5] usb: xhci: Add support for Renesas controller with memory
On 30.4.2020 9.20, Vinod Koul wrote: > On 29-04-20, 19:58, Vinod Koul wrote: >> On 29-04-20, 16:53, Mathias Nyman wrote: >>> On 24.4.2020 13.14, Vinod Koul wrote: > /* Prevent runtime suspending between USB-2 and USB-3 initialization */ pm_runtime_get_noresume(&dev->dev); @@ -388,6 +401,9 @@ static void xhci_pci_remove(struct pci_dev *dev) { struct xhci_hcd *xhci; + if (renesas_device) + renesas_xhci_pci_exit(dev); + >>> >>> Ah, I see, what we really should do is make sure the quirks in the driver >>> data get >>> added to xhci->quirks, and then just check for the correct quirk in >>> xhci_pci_remove. >> >> Ah sure that does sound better, I will update this as well and send an >> update with these changes > > This works for me.. But I have kept the code as in the xhci_pci_probe(), > ofcourse removed bool renesas_device. That's fine, xhci is just hcd->hcd_priv, and it doesn't exists before usb_hcd_pci_probe() is called usb_hcd_pci_probe() usb_create_hcd() hcd = kzalloc(sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL); -Mathias
Re: [PATCH v10 3/5] usb: xhci: Add support for Renesas controller with memory
On 29-04-20, 19:58, Vinod Koul wrote: > On 29-04-20, 16:53, Mathias Nyman wrote: > > On 24.4.2020 13.14, Vinod Koul wrote: > > > /* Prevent runtime suspending between USB-2 and USB-3 initialization */ > > > pm_runtime_get_noresume(&dev->dev); > > > @@ -388,6 +401,9 @@ static void xhci_pci_remove(struct pci_dev *dev) > > > { > > > struct xhci_hcd *xhci; > > > > > > + if (renesas_device) > > > + renesas_xhci_pci_exit(dev); > > > + > > > > Ah, I see, what we really should do is make sure the quirks in the driver > > data get > > added to xhci->quirks, and then just check for the correct quirk in > > xhci_pci_remove. > > Ah sure that does sound better, I will update this as well and send an > update with these changes This works for me.. But I have kept the code as in the xhci_pci_probe(), ofcourse removed bool renesas_device. the xhci is not valid before invoking renesas_xhci_check_request_fw(). Is there another way to get xhci from dev without calling usb_hcd_pci_probe(). xhci = hcd_to_xhci(hcd) doesn't seem to work before that! -- ~Vinod
Re: [PATCH v10 3/5] usb: xhci: Add support for Renesas controller with memory
On 29-04-20, 16:53, Mathias Nyman wrote: > On 24.4.2020 13.14, 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 > > --- > > drivers/usb/host/xhci-pci.c | 28 > > drivers/usb/host/xhci.h | 1 + > > 2 files changed, 29 insertions(+) > > > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > > index b6c2f5c530e3..f26cf072836d 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 > > @@ -319,6 +320,8 @@ static int xhci_pci_setup(struct usb_hcd *hcd) > > return xhci_pci_reinit(xhci, pdev); > > } > > > > +static bool renesas_device; > > hmm, we shouldn't need this > > > + > > /* > > * We need to register our own PCI probe function (instead of the USB > > core's > > * function) in order to create a second roothub under xHCI. > > @@ -328,6 +331,16 @@ 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; > > + > > + renesas_device = false; > > + 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; > > + renesas_device = true; > > + } > > > > /* Prevent runtime suspending between USB-2 and USB-3 initialization */ > > pm_runtime_get_noresume(&dev->dev); > > @@ -388,6 +401,9 @@ static void xhci_pci_remove(struct pci_dev *dev) > > { > > struct xhci_hcd *xhci; > > > > + if (renesas_device) > > + renesas_xhci_pci_exit(dev); > > + > > Ah, I see, what we really should do is make sure the quirks in the driver > data get > added to xhci->quirks, and then just check for the correct quirk in > xhci_pci_remove. Ah sure that does sound better, I will update this as well and send an update with these changes > > if (xhci->quirks & XHCI_RENESAS_FW_QUIRK) > renesas_xhci_pci_exit(dev); > > > Heikki Krogerus did some work on this a long time ago, below code is based on > his > work. It needs to be added: > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index f26cf072836d..5ae4fc10fc31 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -88,8 +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 && > (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK || > > > -Mathias -- ~Vinod
Re: [PATCH v10 3/5] usb: xhci: Add support for Renesas controller with memory
On 24.4.2020 13.14, 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 > --- > drivers/usb/host/xhci-pci.c | 28 > drivers/usb/host/xhci.h | 1 + > 2 files changed, 29 insertions(+) > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index b6c2f5c530e3..f26cf072836d 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_NUM2 > #define SSIC_PORT_CFG2 0x880c > @@ -319,6 +320,8 @@ static int xhci_pci_setup(struct usb_hcd *hcd) > return xhci_pci_reinit(xhci, pdev); > } > > +static bool renesas_device; hmm, we shouldn't need this > + > /* > * We need to register our own PCI probe function (instead of the USB core's > * function) in order to create a second roothub under xHCI. > @@ -328,6 +331,16 @@ 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; > + > + renesas_device = false; > + 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; > + renesas_device = true; > + } > > /* Prevent runtime suspending between USB-2 and USB-3 initialization */ > pm_runtime_get_noresume(&dev->dev); > @@ -388,6 +401,9 @@ static void xhci_pci_remove(struct pci_dev *dev) > { > struct xhci_hcd *xhci; > > + if (renesas_device) > + renesas_xhci_pci_exit(dev); > + Ah, I see, what we really should do is make sure the quirks in the driver data get added to xhci->quirks, and then just check for the correct quirk in xhci_pci_remove. if (xhci->quirks & XHCI_RENESAS_FW_QUIRK) renesas_xhci_pci_exit(dev); Heikki Krogerus did some work on this a long time ago, below code is based on his work. It needs to be added: diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index f26cf072836d..5ae4fc10fc31 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -88,8 +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 && (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK || -Mathias