On Thursday 12 December 2013 02:41 PM, Felipe Balbi wrote: > On Thu, Dec 12, 2013 at 02:36:01PM -0500, Santosh Shilimkar wrote: >> On Thursday 12 December 2013 12:48 PM, Felipe Balbi wrote: >>> Hi, >>> >>> On Tue, Dec 10, 2013 at 10:11:36AM -0500, Santosh Shilimkar wrote: >>>>>> +static int kdwc3_remove(struct platform_device *pdev) >>>>>> +{ >>>>>> + struct dwc3_keystone *kdwc = platform_get_drvdata(pdev); >>>>>> + >>>>>> + kdwc3_disable_irqs(kdwc); >>>>>> + clk_disable_unprepare(kdwc->clk); >>>>> >>>>> I hope the clock isn't shared between core and wrapper, otherwise you >>>>> could run into some troubles here. Can you confirm ? >>>>> >>>> Yes. the clock isn't shared. Thanks for taking care of other parts. >>> >>> so clock for core is always running too ? >>> >> I take that back. The clock is actually common so we should disable >> it after removing the kdwc3_remove_core() as you suggested. >> >> You won't see issue since the kdwc3_remove_core() not doing >> any register access but moving the clock disable after >> the core remove is right thing to do. > > the problem is not kdwc3_remove_core() accessing registers, but > dwc3_remove() _does_ access registers during remove. If you just > mopdrobe -r dwc3-keystone without removing dwc3.ko first, then > kdwc3_remove_core() will cause dwc3.ko to be removed (because of > platform_driver_unregister()) and, since clocks have already been > disabled, then we'd die :-) > Oh yes, you are right. I see Wingman already posted the updated patch. Regards, Santosh -- 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