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 <apelete@xxxxxxxxxxxx>
> > > ---
> > > 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
Attachment:
signature.asc
Description: Digital signature
