Re: [PATCH] Fix clk->enabled counter

[Dmitry Baryshkov <dbaryshkov@xxxxxxxxx>]

Hi,
David Brownell wrote:
> On Saturday 19 January 2008, Dmitry Baryshkov wrote:>> > Minor issues as noted below.  You tested this?  With suspend, resume,>> > remote wakeup, unplug-while-active, and rmmod g_<what> scenarios to>> > make sure the main code paths behave OK?>> >> I've tested with g_ether the suspend/resume of gadget (got log messages>> as found bellow, but otherwise OK),> > Hmm, looks like you were testing system suspend/resume (initiated by> writing to /sys/power/state) when that lockdep problem showed up, not> USB suspend/resume (initiated by the host).  Actually *BOTH* of those> sequences need to be tested!
Yup.
> > The lockdep warning should go away.  It's basically saying that this> code predates lockdep, and has an associated bug.  The bug fix looks to> be along the lines of ensuring that IRQs are blocked during> pxa2xx_udc_{suspend,resume}() methods ... the easiest way to fix that is> updating the udc_driver struct to make those become the late_suspend and> early_resume methods, which are always called with IRQs disabled (so you> won't need to do it yourself).> > (And kudos on running -- or at least testing! -- with lockdep. That's a> practice all developers should follow, and too few of them do so.)
:)
After running into pair of locking problems just when I've started touchingkernel development I tend to write & test code with lockdep enabled.
>> plug-unplug also works. Can't test>> rmmod/modrobe as I don't build modular kernels for my device.> > Well, if you can't do that then you can't.  The trick is rmmod while the> device is active.  That can be problematic, since it forces a clean> shutdown in a different way than disconnect.
Yes. I understood it. Actually I've built and tested the kernel with g_etherbuilt as a module. modprobe/rmmod worked.
> >> Also what do you mean by remote wakeup?> > See the USB 2.0 spec for definition.  The easy way to test it is as> described in http://www.linux-usb.org/usbtest where it says how to set> up g_zero with "autoresume=5" to ensure the remote wakeup tests can run> with USBCV.  There are no easy automated tests to run on Linux.> > In fact, running USBCV is also the easiest way to test USB suspend and> resume too.
Thanks. I'll look into it and post results.
>> I'll try making modules to work, fix things you noted and resubmit the>> patch.> > Thanks.  It basically looks OK, but that's code I've learned to be> cautious with.  It's easy for subtle bugs to creep in ... as you may> have deduced by noticing that lockdep bug.
:)Finding all possible "duplicate clk_disable" was also a bit of work.
-- With best wishesDmitry


-------------------------------------------------------------------------This SF.net email is sponsored by: MicrosoftDefy all challenges. Microsoft(R) Visual Studio 2008.http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/_______________________________________________linux-usb-devel@xxxxxxxxxxxxxxxxxxxxxxx unsubscribe, use the last form field at:https://lists.sourceforge.net/lists/listinfo/linux-usb-devel