Hi, On Wednesday 14 May 2014 02:51 PM, Kamil Debski wrote: > Hi Vivek, Kishon, > >> From: gautamvivek1987@xxxxxxxxx [mailto:gautamvivek1987@xxxxxxxxx] On >> Behalf Of Vivek Gautam >> Sent: Tuesday, May 13, 2014 12:08 PM >> To: Marek Szyprowski >> Cc: Kamil Debski; Linux USB Mailing List; r.baldyga@xxxxxxxxxxx >> Subject: Re: [PATCH] usb: gadget: s3c-hsotg: fix phy disable sequence >> >> Hi, >> >> >> On Tue, May 13, 2014 at 12:30 PM, Marek Szyprowski >> <m.szyprowski@xxxxxxxxxxx> wrote: >>> Hello, >>> >>> >>> On 2014-05-13 08:21, Vivek Gautam wrote: >>>> >>>> On Mon, May 12, 2014 at 10:19 PM, Kamil Debski <k.debski@xxxxxxxxxxx> >>>> wrote: >>>>> When the driver is removed s3c_hsotg_phy_disable is called three >>>>> times instead of once. This results in decreasing of the phy >>>>> reference counter below zero and thus consecutive inserts of the >> module fails. >>>>> >>>>> This patch removes calls to s3c_hsotg_phy_disable from >>>>> s3c_hsotg_remove and s3c_hsotg_udc_stop. >>>>> >>>>> s3c_hsotg_udc_stop is called from udc-core.c only after >>>>> usb_gadget_disconnect, which in turn calls s3c_hsotg_pullup, which >>>>> already calls s3c_hsotg_phy_disable. >>>>> >>>>> s3c_hsotg_remove must be called only after udc_stop, so there is >> no >>>>> point in disabling phy once again there. >>>>> >>>>> Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx> >>>>> --- >>>>> drivers/usb/gadget/s3c-hsotg.c | 3 --- >> >> Please rebase on 'usb-next'. This file is moved to dwc2. >> "drivers/usb/dwc2/gadget.c" > > I will, thank you for pointing this out. > >> >>>>> 1 file changed, 3 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/gadget/s3c-hsotg.c >>>>> b/drivers/usb/gadget/s3c-hsotg.c index 2a9cb67..29d70a7 100644 >>>>> --- a/drivers/usb/gadget/s3c-hsotg.c >>>>> +++ b/drivers/usb/gadget/s3c-hsotg.c >>>>> @@ -3078,8 +3078,6 @@ static int s3c_hsotg_udc_stop(struct >>>>> usb_gadget *gadget, >>>>> >>>>> spin_lock_irqsave(&hsotg->lock, flags); >>>>> >>>>> - s3c_hsotg_phy_disable(hsotg); >>>>> - >>>>> if (!driver) >>>>> hsotg->driver = NULL; >>>>> >>>>> @@ -3766,7 +3764,6 @@ static int s3c_hsotg_remove(struct >>>>> platform_device >>>>> *pdev) >>>>> usb_gadget_unregister_driver(hsotg->driver); >>>>> } >>>>> >>>>> - s3c_hsotg_phy_disable(hsotg); >>>>> if (hsotg->phy) >>>>> phy_exit(hsotg->phy); >>>> >>>> We may want to remove above two lines too, isn't it ? >>> >>> >>> Hmm, I think that more appropriate will be to remove calls to >>> phy_init()/phy_exit() >>> from s3c_hsotg_phy_enable()/s3c_hsotg_phy_disable() functions and >>> leave >>> phy_init() >>> and phy_exit() only in s3c_hsotg_probe()/s3c_hsotg_remove() pair. >> >> Yea, that's one way. >> But i was just wondering, do the phy_init() and phy_exit() callbacks >> have any PHY's suspend/resume related settings. It shouldn't have suspend/resume settings. For suspend/resume, phy_pm_runtime_* calls should be used and if it is for poweron/poweroff, phy_power_on/phy_power_off should be used. Thanks Kishon -- 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
