On Tue, Nov 18, 2014 at 12:05:45AM +0400, Dmitry Eremin-Solenikov wrote:
> Hello,
>
> 2014-11-17 21:44 GMT+03:00 Robert Jarzmik <robert.jarzmik@xxxxxxx>:
> > Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx> writes:
> >
> >> Change clk_enable/disable() calls to clk_prepare_enable() and
> >> clk_disable_unprepare().
> >>
> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>
> >> ---
> >> drivers/usb/gadget/udc/pxa25x_udc.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
> >> index 42f7eeb..e4964ee 100644
> >> --- a/drivers/usb/gadget/udc/pxa25x_udc.c
> >> +++ b/drivers/usb/gadget/udc/pxa25x_udc.c
> >> @@ -103,8 +103,8 @@ static const char ep0name [] = "ep0";
> >>
> >> /* IXP doesn't yet support <linux/clk.h> */
> >> #define clk_get(dev,name) NULL
> >> -#define clk_enable(clk) do { } while (0)
> >> -#define clk_disable(clk) do { } while (0)
> >> +#define clk_prepare_enable(clk) do { } while (0)
> >> +#define clk_disable_unprepare(clk) do { } while (0)
> >> #define clk_put(clk) do { } while (0)
> >>
> >> #endif
> >> @@ -932,7 +932,7 @@ static int pullup(struct pxa25x_udc *udc)
> >> if (!udc->active) {
> >> udc->active = 1;
> >> /* Enable clock for USB device */
> >> - clk_enable(udc->clk);
> >> + clk_prepare_enable(udc->clk);
> >
> > Guess what, Russell's remark on i2c and serial made me cross-check. And there
> > is a case where this will be called in irq context too ...
> >
> > See :
> > -> do_IRQ
> > -> lubbock_vbus_irq()
> > -> pxa25x_udc_vbus_session()
> > -> pullup()
> > -> clk_prepare_enable()
> > -> CRACK
> >
> > Note that your patch is not really the faulty one, I think a threaded irq
> > instead of the "raw" irq should do the trick. And it is granted on UDC api that
> > vbus function is called in a "sleeping" context (Felipe correct me if I'm
> > wrong), so a patch to fix this before your current code would be fine.
Well, from the framework point of view, ->pullup() is only called
outside of IRQ context. But I see you're calling it from vbus_irq(), so
you brought this upon yourself :-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
