Hi, On Tue, Dec 17, 2013 at 02:31:00AM +0100, Apelete Seketeli wrote: > On 16-Dec-13, Felipe Balbi wrote: > > On Sat, Dec 14, 2013 at 04:48:38AM +0100, Apelete Seketeli wrote: > > > JZ4740 USB Device Controller is not OTG compatible and does not have > > > DEVCTL > > > register in silicon. > > > > > > During ethernet-over-usb transactions, on reset, musb driver tries to > > > read from DEVCTL and consequently sets device as host (A-Device) > > > instead of peripheral (B-Device), which makes it a composite device to > > > the USB gadget driver. > > > This induces a kernel panic during power down where the USB gadget > > > driver does a null pointer dereference when trying to access the > > > composite device configuration. > > > > > > On reset, do not rely on DEVCTL value for setting gadget peripheral > > > mode: hardcode it instead to B-Device. > > > > > > Signed-off-by: Apelete Seketeli <apel...@seketeli.net> > > > --- > > > drivers/usb/musb/musb_gadget.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/drivers/usb/musb/musb_gadget.c > > > b/drivers/usb/musb/musb_gadget.c > > > index 32fb057..b4bea7a 100644 > > > --- a/drivers/usb/musb/musb_gadget.c > > > +++ b/drivers/usb/musb/musb_gadget.c > > > @@ -2119,6 +2119,14 @@ __acquires(musb->lock) > > > /* Normal reset, as B-Device; > > > * or else after HNP, as A-Device > > > */ > > > +#if defined(CONFIG_USB_MUSB_JZ4740) || > > > defined(CONFIG_USB_MUSB_JZ4740_MODULE) > > > > NAK, no ifdefs in this driver. Pass a quirk flag through platform_data > > or something similar. > > I get that, makes sense to me, but problem is the driver has to read a > valid value from DEVCTL hardware register when musb_g_reset() is > called, and I do not see how this can be achieved through > platform_data.
why not ?
/*
* JZ4740 UDC Controller is not OTG compatible as does not
* have a DEVCTL register in silicon. Due to that, we must
* NOT rely on that register for setting peripheral mode.
*/
if (unlikely(musb->quirk_broken_devctl)) {
musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
musb->g_is_a_peripheral = 0;
else if (devctl & MUSB_DEVCTL_BDEVICE) {
musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
musb->g_is_a_peripheral = 0;
} else {
musb->xceiv->state = OTG_STATE_A_PERIPHERAL;
musb->g_is_a_peripheral = 1;
}
> Is it ok to use ifdefs in musb_regs.h to add specific hardware
> register indexes for JZ4740 instead ?
you guys changed the register file ? Why ? Is my pain not enough
already ? :-p
> I am actually thinking about fooling the musb driver into reading a
> valid value from another register instead of DEVCTL.
which register would that be ? If the register file is different, we
need to find a way to support it, but you gotta fix a few other things
first before I look into that, I don't want to see any more hacks and
ifdeffery hell around this driver. It's already painful enough to
support all HW variants it already supports.
cheers
--
balbi
signature.asc
Description: Digital signature
