[PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[Akash Kumar]

Skip usb_early_handoff for the Renesas PCI USB controller due to
the firmware not being loaded beforehand, which impacts the bootup
time.

Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
---
 drivers/usb/host/pci-quirks.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 0b949acfa258..a0770ecc0861 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -1264,6 +1264,11 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev)
 		}
 	}
 
+	/* Skip handoff for Renesas PCI USB controller on QCOM SOC */
+	if ((pdev->vendor == PCI_VENDOR_ID_RENESAS) &&
+			(pcie_find_root_port(pdev)->vendor == PCI_VENDOR_ID_QCOM))
+		return;
+
 	if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
 			pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
 			pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
-- 
2.17.1

Re: [PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[Greg Kroah-Hartman]

On Tue, May 21, 2024 at 01:16:35PM +0530, Akash Kumar wrote:
> Skip usb_early_handoff for the Renesas PCI USB controller due to
> the firmware not being loaded beforehand, which impacts the bootup
> time.
> 
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>

What commit id does this fix?  Should it go to stable kernels?

> ---
>  drivers/usb/host/pci-quirks.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 0b949acfa258..a0770ecc0861 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -1264,6 +1264,11 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev)
>  		}
>  	}
>  
> +	/* Skip handoff for Renesas PCI USB controller on QCOM SOC */
> +	if ((pdev->vendor == PCI_VENDOR_ID_RENESAS) &&
> +			(pcie_find_root_port(pdev)->vendor == PCI_VENDOR_ID_QCOM))

Why are all Renesas PCI devices on a QCOM host to be marked this way?
That's a very big hammer for potentially lots of devices.  Have you
tested them all?

thanks,

greg k-h

Re: [PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[AKASH KUMAR]

Hi Greg,On 5/21/2024 1:35 PM, Greg Kroah-Hartman wrote: > On Tue, May 21, 2024 
at 01:16:35PM +0530, Akash Kumar wrote: >> Skip usb_early_handoff for 
the Renesas PCI USB controller due to >> the firmware not being loaded 
beforehand, which impacts the bootup >> time. >> >> Signed-off-by: Akash 
Kumar <quic_akakum@quicinc.com> > What commit id does this fix? Should 
it go to stable kernels? yes it can go to stable kernels, issue is seen on every target with usb over pcie support. >> --- >> drivers/usb/host/pci-quirks.c | 5 +++++ >> 1 file changed, 5 
insertions(+) >> >> diff --git a/drivers/usb/host/pci-quirks.c 
b/drivers/usb/host/pci-quirks.c >> index 0b949acfa258..a0770ecc0861 
100644 >> --- a/drivers/usb/host/pci-quirks.c >> +++ 
b/drivers/usb/host/pci-quirks.c >> @@ -1264,6 +1264,11 @@ static void 
quirk_usb_early_handoff(struct pci_dev *pdev) >> } >> } >> >> + /* Skip 
handoff for Renesas PCI USB controller on QCOM SOC */ >> + if 
((pdev->vendor == PCI_VENDOR_ID_RENESAS) && >> + 
(pcie_find_root_port(pdev)->vendor == PCI_VENDOR_ID_QCOM)) > Why are all 
Renesas PCI devices on a QCOM host to be marked this way? > That's a 
very big hammer for potentially lots of devices. Have you > tested them 
all? firmware loading is being done in HLOS, not UEFI,
if firmware loading is done in UEFI, then calling early_handoff() API makes sense,
else it is checking for controller ready without firmware loaded which is impacting boot up time by 5 sec roughly.
We are seeing problem in all targets having usb over pcie support. > thanks, > > greg k-h Thanks,
Akash

Re: [PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[Greg Kroah-Hartman]

On Tue, May 21, 2024 at 02:55:13PM +0530, AKASH KUMAR wrote:
> Hi Greg,
> 
> On 5/21/2024 1:35 PM, Greg Kroah-Hartman wrote:
> > On Tue, May 21, 2024 at 01:16:35PM +0530, Akash Kumar wrote:
> > > Skip usb_early_handoff for the Renesas PCI USB controller due to
> > > the firmware not being loaded beforehand, which impacts the bootup
> > > time.
> > > 
> > > Signed-off-by: Akash Kumar<quic_akakum@quicinc.com>
> > What commit id does this fix?  Should it go to stable kernels?
> > yes it can go to stable kernels, issue is seen on every target with usb
> > over pcie support.
> > > ---
> > >   drivers/usb/host/pci-quirks.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> > > index 0b949acfa258..a0770ecc0861 100644
> > > --- a/drivers/usb/host/pci-quirks.c
> > > +++ b/drivers/usb/host/pci-quirks.c
> > > @@ -1264,6 +1264,11 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev)
> > >   		}
> > >   	}
> > > +	/* Skip handoff for Renesas PCI USB controller on QCOM SOC */
> > > +	if ((pdev->vendor == PCI_VENDOR_ID_RENESAS) &&
> > > +			(pcie_find_root_port(pdev)->vendor == PCI_VENDOR_ID_QCOM))
> > Why are all Renesas PCI devices on a QCOM host to be marked this way?
> > That's a very big hammer for potentially lots of devices.  Have you
> > tested them all?
> 
> firmware loading is being done in HLOS, not UEFI, if firmware loading is
> done in UEFI, then calling early_handoff() API makes sense, else it is
> checking for controller ready without firmware loaded which is impacting
> boot up time by 5 sec roughly. We are seeing problem in all targets having
> usb over pcie support.

But the bootloader has nothing to do with the device type of the devices
here, right?  Why not properly trigger this off of the needed firmware
location instead of here?  What happens when you have a system using
UEFI that matches these two devices and the change causes them to break?

In other words, test the proper thing, and only for the specific devices
you need to have the change for, don't be overly broad like you are
doing here, as you might break other systems that you do not have in
front of you at the moment.

thanks,

greg k-h

Re: [PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[AKASH KUMAR]

On 5/21/2024 3:08 PM, Greg Kroah-Hartman wrote:

> On Tue, May 21, 2024 at 02:55:13PM +0530, AKASH KUMAR wrote:
>> Hi Greg, On 5/21/2024 1:35 PM, Greg Kroah-Hartman wrote:
>>> On Tue, May 21, 2024 at 01:16:35PM +0530, Akash Kumar wrote:
>>>> Skip usb_early_handoff for the Renesas PCI USB controller due to 
>>>> the firmware not being loaded beforehand, which impacts the bootup 
>>>> time. Signed-off-by: Akash Kumar<quic_akakum@quicinc.com>
>>> What commit id does this fix? Should it go to stable kernels? yes it 
>>> can go to stable kernels, issue is seen on every target with usb 
>>> over pcie support.
>>>> --- drivers/usb/host/pci-quirks.c | 5 +++++ 1 file changed, 5 
>>>> insertions(+) diff --git a/drivers/usb/host/pci-quirks.c 
>>>> b/drivers/usb/host/pci-quirks.c index 0b949acfa258..a0770ecc0861 
>>>> 100644 --- a/drivers/usb/host/pci-quirks.c +++ 
>>>> b/drivers/usb/host/pci-quirks.c @@ -1264,6 +1264,11 @@ static void 
>>>> quirk_usb_early_handoff(struct pci_dev *pdev) } } + /* Skip handoff 
>>>> for Renesas PCI USB controller on QCOM SOC */ + if ((pdev->vendor 
>>>> == PCI_VENDOR_ID_RENESAS) && + (pcie_find_root_port(pdev)->vendor 
>>>> == PCI_VENDOR_ID_QCOM))
>>> Why are all Renesas PCI devices on a QCOM host to be marked this 
>>> way? That's a very big hammer for potentially lots of devices. Have 
>>> you tested them all?
>> firmware loading is being done in HLOS, not UEFI, if firmware loading 
>> is done in UEFI, then calling early_handoff() API makes sense, else 
>> it is checking for controller ready without firmware loaded which is 
>> impacting boot up time by 5 sec roughly. We are seeing problem in all 
>> targets having usb over pcie support.
> But the bootloader has nothing to do with the device type of the 
> devices here, right? Why not properly trigger this off of the needed 
> firmware location instead of here? What happens when you have a system 
> using UEFI that matches these two devices and the change causes them 
> to break? In other words, test the proper thing, and only for the 
> specific devices you need to have the change for, don't be overly 
> broad like you are doing here, as you might break other systems that 
> you do not have in front of you at the moment.

yeah currently we don't have any uefi based targets, will add target specific check.

Thanks,

Akash

Re: [PATCH] USB: pci-quirks: Skip usb_early_handoff for Renesas PCI USB

[Bjorn Andersson]

On Wed, May 22, 2024 at 03:00:32PM +0530, AKASH KUMAR wrote:
> 
> On 5/21/2024 3:08 PM, Greg Kroah-Hartman wrote:
> 
> > On Tue, May 21, 2024 at 02:55:13PM +0530, AKASH KUMAR wrote:
> > > Hi Greg, On 5/21/2024 1:35 PM, Greg Kroah-Hartman wrote:
> > > > On Tue, May 21, 2024 at 01:16:35PM +0530, Akash Kumar wrote:
> > > > > Skip usb_early_handoff for the Renesas PCI USB controller
> > > > > due to the firmware not being loaded beforehand, which
> > > > > impacts the bootup time. Signed-off-by: Akash
> > > > > Kumar<quic_akakum@quicinc.com>
> > > > What commit id does this fix? Should it go to stable kernels?
> > > > yes it can go to stable kernels, issue is seen on every target
> > > > with usb over pcie support.
> > > > > --- drivers/usb/host/pci-quirks.c | 5 +++++ 1 file changed,
> > > > > 5 insertions(+) diff --git a/drivers/usb/host/pci-quirks.c
> > > > > b/drivers/usb/host/pci-quirks.c index
> > > > > 0b949acfa258..a0770ecc0861 100644 ---
> > > > > a/drivers/usb/host/pci-quirks.c +++
> > > > > b/drivers/usb/host/pci-quirks.c @@ -1264,6 +1264,11 @@
> > > > > static void quirk_usb_early_handoff(struct pci_dev *pdev) }
> > > > > } + /* Skip handoff for Renesas PCI USB controller on QCOM
> > > > > SOC */ + if ((pdev->vendor == PCI_VENDOR_ID_RENESAS) && +
> > > > > (pcie_find_root_port(pdev)->vendor == PCI_VENDOR_ID_QCOM))
> > > > Why are all Renesas PCI devices on a QCOM host to be marked this
> > > > way? That's a very big hammer for potentially lots of devices.
> > > > Have you tested them all?
> > > firmware loading is being done in HLOS, not UEFI, if firmware
> > > loading is done in UEFI, then calling early_handoff() API makes
> > > sense, else it is checking for controller ready without firmware
> > > loaded which is impacting boot up time by 5 sec roughly. We are
> > > seeing problem in all targets having usb over pcie support.
> > But the bootloader has nothing to do with the device type of the devices
> > here, right? Why not properly trigger this off of the needed firmware
> > location instead of here? What happens when you have a system using UEFI
> > that matches these two devices and the change causes them to break? In
> > other words, test the proper thing, and only for the specific devices
> > you need to have the change for, don't be overly broad like you are
> > doing here, as you might break other systems that you do not have in
> > front of you at the moment.
> 
> yeah currently we don't have any uefi based targets, will add target specific check.
> 

Sure we do.

And please fix your email client.

Regards,
Bjorn