On 4/18/2023 8:36 PM, Alan Stern wrote:
> On Tue, Apr 18, 2023 at 07:38:16PM +0530, Basavaraj Natikar wrote:
>> Currently, the pci_resume method has only a flag indicating whether the
>> system is resuming from hibernation. In order to handle all PM events like
>> AUTO_RESUME, SUSPEND etc change the pci_resume method to handle all PM
>> events.
> You might want to make a different kind of distinction between the
> various sorts of resume. For example, a resume from runtime suspend
> can occur either because of a request from the system (it needs to start
> using the device) or a remote wakeup request from an attached device.
> The different sorts of resume might have different requirements.
yes correct.
>
>> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
>> index 4b148fe5e43b..1145c6e7fae5 100644
>> --- a/drivers/usb/host/ehci-pci.c
>> +++ b/drivers/usb/host/ehci-pci.c
>> @@ -354,10 +354,11 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
>> * Also they depend on separate root hub suspend/resume.
>> */
>>
>> -static int ehci_pci_resume(struct usb_hcd *hcd, bool hibernated)
>> +static int ehci_pci_resume(struct usb_hcd *hcd, int event)
>> {
>> struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>> struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
>> + bool hibernated = event == PM_EVENT_RESTORE;
> Please use the same indentation style as the surrounding code. Also,
> when a boolean expression is used in an assignment, I prefer to put it
> in parentheses to help set it off from the assignment operator:
>
> bool hibernated = (event == PM_EVENT_RESTORE);
Sure will change it accordingly.
>
>> diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
>> index 3592f757fe05..9b90c3221bd8 100644
>> --- a/drivers/usb/host/uhci-pci.c
>> +++ b/drivers/usb/host/uhci-pci.c
>> @@ -167,7 +167,7 @@ static void uhci_shutdown(struct pci_dev *pdev)
>>
>> #ifdef CONFIG_PM
>>
>> -static int uhci_pci_resume(struct usb_hcd *hcd, bool hibernated);
>> +static int uhci_resume(struct usb_hcd *hcd, bool hibernated);
> There's no need to change the function's name. After all, it is static.
sure will keep same function name.
>
>>
>> static int uhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
>> {
>> @@ -202,13 +202,13 @@ static int uhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
>>
>> /* Check for race with a wakeup request */
>> if (do_wakeup && HCD_WAKEUP_PENDING(hcd)) {
>> - uhci_pci_resume(hcd, false);
>> + uhci_resume(hcd, false);
>> rc = -EBUSY;
>> }
>> return rc;
>> }
>>
>> -static int uhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
>> +static int uhci_resume(struct usb_hcd *hcd, bool hibernated)
>> {
>> struct uhci_hcd *uhci = hcd_to_uhci(hcd);
>>
>> @@ -252,6 +252,10 @@ static int uhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
>> return 0;
>> }
>>
>> +static int uhci_pci_resume(struct usb_hcd *hcd, int event)
>> +{
>> + return uhci_resume(hcd, event == PM_EVENT_RESTORE);
>> +}
> Again, try to avoid this wrapper.
Sure will avoid this change.
Thanks,
--
Basavaraj
>
> Alan Stern
