2014-01-09 14:17:46

by Pali Rohár

[permalink] [raw]
Subject: Re: BUG: usb: obex in g_nokia.ko causing kernel panic

On Wednesday 27 November 2013 18:39:04 Felipe Balbi wrote:
> Hi,
>
> On Wed, Nov 27, 2013 at 05:40:42PM +0100, Pali Rohár wrote:
> > > > > > @@ -282,10 +284,12 @@ static void
> > > > > > obex_disconnect(struct gserial *g) if
> > > > > > (!obex->can_activate)
> > > > > >
> > > > > > return;
> > > > > >
> > > > > > +#if 0
> > > > > >
> > > > > > status = usb_function_deactivate(&g->func);
> > > > > > if (status)
> > > > > >
> > > > > > DBG(cdev, "obex ttyGS%d function deactivate -->
> > > > > > %d\n",
> > > > > >
> > > > > > obex->port_num, status);
> > > > > >
> > > > > > +#endif
> > > > > >
> > > > > > }
> > > > > >
> > > > > > /*-------------------------------------------------
> > > > > > ---- ------
> > > > > >
> > > > > > --------------*/ @@ -372,6 +376,7 @@ static int
> > > > > > obex_bind(struct usb_configuration *c, struct
> > > > > > usb_function *f) if (status)
> > > > > >
> > > > > > goto fail;
> > > > > >
> > > > > > +#if 0
> > > > > >
> > > > > > /* Avoid letting this gadget enumerate until the
> > > > > > userspace
> > > > > >
> > > > > > * OBEX server is active.
> > > > > > */
> > > > > >
> > > > > > @@ -381,6 +386,7 @@ static int obex_bind(struct
> > > > > > usb_configuration *c, struct usb_function *f)
> > > > > > obex->port_num, status);
> > > > > >
> > > > > > else
> > > > > >
> > > > > > obex->can_activate = true;
> > > > > >
> > > > > > +#endif
> > > > > >
> > > > > > DBG(cdev, "obex ttyGS%d: %s speed IN/%s OUT/%s\n",
> > > > >
> > > > > Hi, with above patch g_nokia.ko working and not
> > > > > crashing.
> > > >
> > > > yeah, makes sense. We shouldn't call phy operations in
> > > > atomic context. I'll see how easy it would be to fix
> > > > that. Real patch coming soon(-ish).
> > >
> > > I think we could send diff below for the -rc cycle and
> > > figure out a better to handle this for the merge window.
> > > Can you check if it also works with patch below ?
> > >
> > > diff --git a/drivers/usb/musb/omap2430.c
> > > b/drivers/usb/musb/omap2430.c index 2a408cd..8aa59a2
> > > 100644 --- a/drivers/usb/musb/omap2430.c
> > > +++ b/drivers/usb/musb/omap2430.c
> > > @@ -659,7 +659,6 @@ static int
> > > omap2430_runtime_suspend(struct device *dev)
> > > OTG_INTERFSEL);
> > >
> > > omap2430_low_level_exit(musb);
> > >
> > > - phy_power_off(musb->phy);
> > >
> > > }
> > >
> > > return 0;
> > >
> > > @@ -674,7 +673,6 @@ static int
> > > omap2430_runtime_resume(struct device *dev)
> > > omap2430_low_level_init(musb);
> > >
> > > musb_writel(musb->mregs, OTG_INTERFSEL,
> > >
> > > musb->context.otg_interfsel);
> > >
> > > - phy_power_on(musb->phy);
> > >
> > > }
> > >
> > > return 0;
> >
> > Hi, I applied similar patch (commented usb_phy_set_suspend
> > instead phy_power_on/off) on 3.12-rc5 and it working too, no
> > crash. I do not have rebased n900 patches on top of 3.13
> > tree, so I cannot check it. But I think phy_power_on/off
> > doing same as usb_phy_set_suspend in 3.12.
>
> cool, I'll send this during the -rc and Cc stable, then I'll
> manually backport it to stable later.

PING!

I still do not see this patch in linus tree. What happened?

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2014-01-26 22:06:47

by Pali Rohár

[permalink] [raw]
Subject: Re: BUG: usb: obex in g_nokia.ko causing kernel panic

2014-01-09 Pali Rohár <[email protected]>:
> On Wednesday 27 November 2013 18:39:04 Felipe Balbi wrote:
>> Hi,
>>
>> On Wed, Nov 27, 2013 at 05:40:42PM +0100, Pali Rohár wrote:
>> > > > > > @@ -282,10 +284,12 @@ static void
>> > > > > > obex_disconnect(struct gserial *g) if
>> > > > > > (!obex->can_activate)
>> > > > > >
>> > > > > > return;
>> > > > > >
>> > > > > > +#if 0
>> > > > > >
>> > > > > > status = usb_function_deactivate(&g->func);
>> > > > > > if (status)
>> > > > > >
>> > > > > > DBG(cdev, "obex ttyGS%d function deactivate -->
>> > > > > > %d\n",
>> > > > > >
>> > > > > > obex->port_num, status);
>> > > > > >
>> > > > > > +#endif
>> > > > > >
>> > > > > > }
>> > > > > >
>> > > > > > /*-------------------------------------------------
>> > > > > > ---- ------
>> > > > > >
>> > > > > > --------------*/ @@ -372,6 +376,7 @@ static int
>> > > > > > obex_bind(struct usb_configuration *c, struct
>> > > > > > usb_function *f) if (status)
>> > > > > >
>> > > > > > goto fail;
>> > > > > >
>> > > > > > +#if 0
>> > > > > >
>> > > > > > /* Avoid letting this gadget enumerate until the
>> > > > > > userspace
>> > > > > >
>> > > > > > * OBEX server is active.
>> > > > > > */
>> > > > > >
>> > > > > > @@ -381,6 +386,7 @@ static int obex_bind(struct
>> > > > > > usb_configuration *c, struct usb_function *f)
>> > > > > > obex->port_num, status);
>> > > > > >
>> > > > > > else
>> > > > > >
>> > > > > > obex->can_activate = true;
>> > > > > >
>> > > > > > +#endif
>> > > > > >
>> > > > > > DBG(cdev, "obex ttyGS%d: %s speed IN/%s OUT/%s\n",
>> > > > >
>> > > > > Hi, with above patch g_nokia.ko working and not
>> > > > > crashing.
>> > > >
>> > > > yeah, makes sense. We shouldn't call phy operations in
>> > > > atomic context. I'll see how easy it would be to fix
>> > > > that. Real patch coming soon(-ish).
>> > >
>> > > I think we could send diff below for the -rc cycle and
>> > > figure out a better to handle this for the merge window.
>> > > Can you check if it also works with patch below ?
>> > >
>> > > diff --git a/drivers/usb/musb/omap2430.c
>> > > b/drivers/usb/musb/omap2430.c index 2a408cd..8aa59a2
>> > > 100644 --- a/drivers/usb/musb/omap2430.c
>> > > +++ b/drivers/usb/musb/omap2430.c
>> > > @@ -659,7 +659,6 @@ static int
>> > > omap2430_runtime_suspend(struct device *dev)
>> > > OTG_INTERFSEL);
>> > >
>> > > omap2430_low_level_exit(musb);
>> > >
>> > > - phy_power_off(musb->phy);
>> > >
>> > > }
>> > >
>> > > return 0;
>> > >
>> > > @@ -674,7 +673,6 @@ static int
>> > > omap2430_runtime_resume(struct device *dev)
>> > > omap2430_low_level_init(musb);
>> > >
>> > > musb_writel(musb->mregs, OTG_INTERFSEL,
>> > >
>> > > musb->context.otg_interfsel);
>> > >
>> > > - phy_power_on(musb->phy);
>> > >
>> > > }
>> > >
>> > > return 0;
>> >
>> > Hi, I applied similar patch (commented usb_phy_set_suspend
>> > instead phy_power_on/off) on 3.12-rc5 and it working too, no
>> > crash. I do not have rebased n900 patches on top of 3.13
>> > tree, so I cannot check it. But I think phy_power_on/off
>> > doing same as usb_phy_set_suspend in 3.12.
>>
>> cool, I'll send this during the -rc and Cc stable, then I'll
>> manually backport it to stable later.
>
> PING!
>
> I still do not see this patch in linus tree. What happened?
>
> --
> Pali Rohár
> [email protected]

BUMP

Is there any problem with above patch?

--
Pali Rohár
[email protected]

2014-02-05 22:25:34

by Ivaylo Dimitrov

[permalink] [raw]
Subject: Re: BUG: usb: obex in g_nokia.ko causing kernel panic

Hi Felipe,

>>>
>>> cool, I'll send this during the -rc and Cc stable, then I'll
>>> manually backport it to stable later.
>>
>> PING!
>>
>> I still do not see this patch in linus tree. What happened?
>>
>> --
>> Pali Roh?r
>> [email protected]
>
> BUMP
>
> Is there any problem with above patch?
>

If you are short on time or have some another reasons preventing you
pushing that patch, I can send that patch for you, are you ok with that?

Ivo

2014-02-05 23:05:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: BUG: usb: obex in g_nokia.ko causing kernel panic

On Thu, Feb 06, 2014 at 12:25:21AM +0200, Ivaylo Dimitrov wrote:
> Hi Felipe,
>
> >>>
> >>>cool, I'll send this during the -rc and Cc stable, then I'll
> >>>manually backport it to stable later.
> >>
> >>PING!
> >>
> >>I still do not see this patch in linus tree. What happened?
> >>
> >>--
> >>Pali Roh?r
> >>[email protected]
> >
> >BUMP
> >
> >Is there any problem with above patch?
> >
>
> If you are short on time or have some another reasons preventing you pushing
> that patch, I can send that patch for you, are you ok with that?

Felipe is on vacation, care to resend the patch and cc: me?

thanks,

greg k-h

2014-02-06 19:25:50

by Ivaylo Dimitrov

[permalink] [raw]
Subject: [PATCH] usb: musb: Fix obex in g_nokia.ko causing kernel panic

From: Felipe Balbi <[email protected]>

[ 18.606414] [<c037eac0>] (__schedule+0x5c/0x50c) from [<c037d3bc>] (schedule_timeout+0x1f4/0x25c)
[ 18.623809] [<c037d3bc>] (schedule_timeout+0x1f4/0x25c) from [<c037f12c>] (wait_for_common+0xc8/0x1ac)
[ 18.649291] [<c037f12c>] (wait_for_common+0xc8/0x1ac) from [<c028c1c0>] (omap_i2c_xfer+0x338/0x488)
[ 18.674499] [<c028c1c0>] (omap_i2c_xfer+0x338/0x488) from [<c0288144>] (__i2c_transfer+0x40/0x74)
[ 18.692047] [<c0288144>] (__i2c_transfer+0x40/0x74) from [<c0288a2c>] (i2c_transfer+0x6c/0x90)
[ 18.709320] [<c0288a2c>] (i2c_transfer+0x6c/0x90) from [<c02351c8>] (regmap_i2c_read+0x48/0x68)
[ 18.726715] [<c02351c8>] (regmap_i2c_read+0x48/0x68) from [<c023161c>] (_regmap_raw_read+0x128/0x220)
[ 18.752685] [<c023161c>] (_regmap_raw_read+0x128/0x220) from [<c02317b4>] (regmap_raw_read+0xa0/0x130)
[ 18.779052] [<c02317b4>] (regmap_raw_read+0xa0/0x130) from [<c023193c>] (regmap_bulk_read+0xf8/0x16c)
[ 18.805694] [<c023193c>] (regmap_bulk_read+0xf8/0x16c) from [<c0238ea8>] (twl_i2c_read+0xa4/0xe0)
[ 18.823730] [<c0238ea8>] (twl_i2c_read+0xa4/0xe0) from [<c0274d34>] (__twl4030_phy_power.isra.12+0x1c/0x58)
[ 18.850921] [<c0274d34>] (__twl4030_phy_power.isra.12+0x1c/0x58) from [<c0274df0>]
(twl4030_phy_power.part.14+0x80/0xc8)
[ 18.879699] [<c0274df0>] (twl4030_phy_power.part.14+0x80/0xc8) from [<c0274f9c>]
(twl4030_set_suspend+0x54/0x1e8)
[ 18.908325] [<c0274f9c>] (twl4030_set_suspend+0x54/0x1e8) from [<c027c8c4>]
(omap2430_runtime_resume+0x5c/0x64)
[ 18.937042] [<c027c8c4>] (omap2430_runtime_resume+0x5c/0x64) from [<c0225dd0>]
(pm_generic_runtime_resume+0x2c/0x38)
[ 18.966461] [<c0225dd0>] (pm_generic_runtime_resume+0x2c/0x38) from [<c0229fe0>] (__rpm_callback+0x54/0x80)
[ 18.995117] [<c0229fe0>] (__rpm_callback+0x54/0x80) from [<c022a04c>] (rpm_callback+0x40/0x74)
[ 19.013610] [<c022a04c>] (rpm_callback+0x40/0x74) from [<c022b3c8>] (rpm_resume+0x448/0x63c)
[ 19.031921] [<c022b3c8>] (rpm_resume+0x448/0x63c) from [<c022b2e4>] (rpm_resume+0x364/0x63c)
[ 19.050140] [<c022b2e4>] (rpm_resume+0x364/0x63c) from [<c022b874>] (__pm_runtime_resume+0x48/0x74)
[ 19.077728] [<c022b874>] (__pm_runtime_resume+0x48/0x74) from [<c027b4fc>] (musb_gadget_pullup+0x1c/0xb4)
[ 19.105895] [<c027b4fc>] (musb_gadget_pullup+0x1c/0xb4) from [<bf025c14>] (usb_function_deactivate+0x54/0xa4
[libcomposite])
[ 19.135955] [<bf025c14>] (usb_function_deactivate+0x54/0xa4 [libcomposite]) from [<bf05b3b8>]
(obex_bind+0x124/0x1d8 [usb_f_obex])
[ 19.166870] [<bf05b3b8>] (obex_bind+0x124/0x1d8 [usb_f_obex]) from [<bf025794>] (usb_add_function+0x58/0xf4
[libcomposite])
[ 19.197143] [<bf025794>] (usb_add_function+0x58/0xf4 [libcomposite]) from [<bf037420>]
(nokia_bind_config+0x204/0x250 [g_nokia])
[ 19.227905] [<bf037420>] (nokia_bind_config+0x204/0x250 [g_nokia]) from [<bf0263fc>] (usb_add_config+0x28/0xc0
[libcomposite])
[ 19.258483] [<bf0263fc>] (usb_add_config+0x28/0xc0 [libcomposite]) from [<bf03709c>] (nokia_bind+0x9c/0x21c
[g_nokia])
[ 19.288421] [<bf03709c>] (nokia_bind+0x9c/0x21c [g_nokia]) from [<bf0275bc>] (composite_bind+0x74/0x180
[libcomposite])
[ 19.318420] [<bf0275bc>] (composite_bind+0x74/0x180 [libcomposite]) from [<c027d658>]
(udc_bind_to_driver+0x2c/0xc4)
[ 19.348114] [<c027d658>] (udc_bind_to_driver+0x2c/0xc4) from [<c027d764>] (usb_gadget_probe_driver+0x74/0x94)
[ 19.377166] [<c027d764>] (usb_gadget_probe_driver+0x74/0x94) from [<c00086f8>] (do_one_initcall+0x94/0x138)
[ 19.406005] [<c00086f8>] (do_one_initcall+0x94/0x138) from [<c007a460>] (load_module+0x113c/0x13c4)
[ 19.434051] [<c007a460>] (load_module+0x113c/0x13c4) from [<c007a7b4>] (SyS_init_module+0xcc/0xec)
[ 19.462127] [<c007a7b4>] (SyS_init_module+0xcc/0xec) from [<c000dd40>] (ret_fast_syscall+0x0/0x30)
[ 19.490753] Code: 0a00002e e1a00004 eb001438 e598300c (e5d3202c)
[ 19.506805] ---[ end trace 060b62ec0d68a78b ]---
[ 19.523132] Kernel panic - not syncing: Fatal exception in interrupt

dump is from 3.12-rc5 kernel

Reported-by: Pali Rohár <[email protected]>
Signed-off-by: Ivaylo Dimitrov <[email protected]>
---
drivers/usb/musb/omap2430.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 2a408cd..8aa59a2 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -659,7 +659,6 @@ static int omap2430_runtime_suspend(struct device *dev)
OTG_INTERFSEL);

omap2430_low_level_exit(musb);
- phy_power_off(musb->phy);
}

return 0;
@@ -674,7 +673,6 @@ static int omap2430_runtime_resume(struct device *dev)
omap2430_low_level_init(musb);
musb_writel(musb->mregs, OTG_INTERFSEL,
musb->context.otg_interfsel);
- phy_power_on(musb->phy);
}

return 0;
--
1.7.9.5

2014-02-06 19:32:21

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Fix obex in g_nokia.ko causing kernel panic

On Thu, Feb 06, 2014 at 09:25:37PM +0200, Ivaylo Dimitrov wrote:
> From: Felipe Balbi <[email protected]>

I can't take a patch that the author doesn't sign off on, sorry.

Please wait a week or so so that Felipe can return from vacation.

thanks,

greg k-h

2014-04-15 21:37:30

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Fix obex in g_nokia.ko causing kernel panic

Hi!

> > From: Felipe Balbi <[email protected]>
>
> This patch, which is present in 3.14-rc4 as 30a70b026 ("usb: musb: fix
> obex in g_nokia.ko causing kernel panic"), breaks USB gadget support
> on my Pandaboard. Bisecting points to this commit, reverting it makes
> USB gadget support work again. The problem is that this patch deletes
> the call which turns on the PHY.
>
> Config is attached.

Can you try adding

static int omap2430_musb_init(struct musb *musb)
{
...
+ phy_power_on(musb->phy);
return 0;
...
}


?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html