On Thu, 16 Jun 2011, Sebastian Andrzej Siewior wrote:
> * Alan Stern | 2011-06-16 15:35:13 [-0400]:
>
> >On Thu, 16 Jun 2011, Sebastian Andrzej Siewior wrote:
> >
> >> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> >> ---
> >> drivers/usb/gadget/dummy_hcd.c | 26 ++++++++++----------------
> >> 1 files changed, 10 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
> >> index 865faf0..0bdc035 100644
> >> --- a/drivers/usb/gadget/dummy_hcd.c
> >> +++ b/drivers/usb/gadget/dummy_hcd.c
> >> @@ -1040,36 +1040,30 @@ static int dummy_udc_remove (struct platform_device *pdev)
> >> return 0;
> >> }
> >>
> >> -static int dummy_udc_suspend (struct platform_device *pdev, pm_message_t state)
> >> +static int dummy_udc_pm(struct platform_device *pdev, int suspend)
> >> {
> >> struct dummy *dum = platform_get_drvdata(pdev);
> >> struct dummy_hcd *dum_hcd;
> >>
> >> - dev_dbg (&pdev->dev, "%s\n", __func__);
> >> dum_hcd = gadget_to_dummy_hcd(&dum->gadget);
> >> spin_lock_irq (&dum->lock);
> >> - dum->udc_suspended = 1;
> >> + dum->udc_suspended = suspend;
> >> set_link_state(dum_hcd);
> >> spin_unlock_irq (&dum->lock);
> >> -
> >> usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd));
> >
> >This last line belongs in dummy_udc_suspend(), not dummy_udc_pm().
>
> Sorry, I'm not getting in. Do you want to keep the space between
> spin_unlock_irq() and usb_hcd_poll_rh_status?
No, no, I'm not talking about the blank line.
> The result of dummy_udc_suspend() is:
> |static int dummy_udc_suspend(struct platform_device *pdev, pm_message_t state)
> |{
> | dev_dbg(&pdev->dev, "%s\n", __func__);
> | return dummy_udc_pm(pdev, 1);
> |}
>
> >> return 0;
> >> }
> >
> >Why declare a return value if you're always going to return 0?
>
> The prototype of dummy_udc_suspend()/dummy_udc_resume() forces me to
> return something. So instead
> dummy_udc_pm()
> return 0;
>
> I concated it into one line. If, something could make
> dummy_udc_suspend() forbid to enter suspend then it would happen in
> dummy_udc_pm() :)
> Do you want the two line edition instead?
I meant you should do this:
static int dummy_udc_suspend(struct platform_device *pdev, pm_message_t state)
{
dev_dbg(&pdev->dev, "%s\n", __func__);
dummy_udc_pm(pdev, 1);
usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd));
return 0;
}
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
