From: Phil Edworthy <phil.edworthy@renesas.com>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
CC: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Felipe Balbi <balbi@ti.com>, Ulrich Hecht <ulrich.hecht@gmail.com>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        "Sergei Shtylyov" <sergei.shtylyov@cogentembedded.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-sh@vger.kernel.org" <linux-sh@vger.kernel.org>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: RE: [PATCH v3] usb: renesas_usbhs: Allow an OTG PHY driver to provide
 VBUS
Thread-Topic: [PATCH v3] usb: renesas_usbhs: Allow an OTG PHY driver to
 provide VBUS
Thread-Index: AQHQt709t0fxPg3S/kucP8bohKfBep3Pti9g
Date: Tue, 7 Jul 2015 08:59:01 +0000
Message-ID: <SIXPR06MB0639AC576AF79B409EDDBCCFF5920@SIXPR06MB0639.apcprd06.prod.outlook.com>
References: <2896957.Pcl0CbmdvL@avalon>
 <1435832793-9999-1-git-send-email-phil.edworthy@renesas.com>
 <SG2PR06MB09198FBCF2401A2DFC1DDC4FD8930@SG2PR06MB0919.apcprd06.prod.outlook.com>
In-Reply-To: <SG2PR06MB09198FBCF2401A2DFC1DDC4FD8930@SG2PR06MB0919.apcprd06.prod.outlook.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
spamdiagnosticoutput: 1:23
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jul 2015 08:59:01.4325
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SIXPR06MB0607
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4976
Lines: 120

Hi Shimoda-san,

On 06 July 2015 08:28, Shimoda-san wrote:
> Hi Phil-san,
> 
> > Sent: Thursday, July 02, 2015 7:27 PM
> >
> > These changes allow a PHY driver to trigger a VBUS interrupt and
> > to provide the value of VBUS.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> Thank you for the patch!
> 
> However, if I tested this patch, a kernel panic happened when
> I did "rmmod g_mass_storage" on koelsch:
> 
> root@192:~/usb# rmmod g_mass_storage
> Unable to handle kernel NULL pointer dereference at virtual address 00000004
> pgd = eebd4bc0
> [00000004] *pgd=00000000
> Internal error: Oops: 205 [#1] SMP ARM
> Modules linked in: g_mass_storage(-) usb_f_mass_storage libcomposite
> CPU: 0 PID: 2496 Comm: rmmod Not tainted 4.1.0-dirty #32
> Hardware name: Generic R8A7791 (Flattened Device Tree)
> task: eeb64ac0 ti: ee022000 task.ti: ee022000
> PC is at usbhs_pkt_pop+0x1c/0x100
> LR is at usbhsg_ep_dequeue+0x28/0x40
> pc : [<c036d934>]    lr : [<c036e364>]    psr: 20000013
> sp : ee023e70  ip : ee023e98  fp : ee023e94
> r10: 00000000  r9 : ee022000  r8 : c00101e4
> r7 : 00200200  r6 : 00100100  r5 : 00000000  r4 : eeb917f8
> r3 : c036e33c  r2 : 00000041  r1 : eeb917f8  r0 : 00000000
> Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> Control: 30c5307d  Table: 6ebd4bc0  DAC: fffffffd
> Process rmmod (pid: 2496, stack limit = 0xee022210)
> Stack: (0xee023e70 to 0xee024000)
> 3e60:                                     ee1b1400 eeb917c0 00100100 00200200
> 3e80: c00101e4 ee022000 ee023eac ee023e98 c036e364 c036d924 eeb34180
> eeb341c0
> 3ea0: ee023ecc ee023eb0 bf002f20 c036e348 eeb34180 00000001 ee1b1800
> eeb341b8
> 3ec0: ee023eec ee023ed0 bf002fec bf002e5c ee181600 bf01d624 bed3ff0d
> 00000081
> 3ee0: ee023efc ee023ef0 bf003028 bf002f78 ee023f14 ee023f00 c036fb88
> bf003018
> 3f00: ee181600 bf01d624 ee023f2c ee023f18 c036fc10 c036fb34 bf01d694 7f64cd30
> 3f20: ee023f3c ee023f30 bf0013d0 c036fbbc ee023f4c ee023f40 bf01d26c bf0013c0
> 3f40: ee023fa4 ee023f50 c0090bd8 bf01d248 ee023f6c 616d5f67 735f7373
> 61726f74
> 3f60: 00006567 ee023f70 c00460f4 c052a8c0 c00101e4 ee022010 ee023fb0
> ee022000
> 3f80: ee023fac ee023f90 c00136c4 00046038 00000001 7f64cd00 00000000
> ee023fa8
> 3fa0: c0010040 c0090adc 00000001 7f64cd00 7f64cd30 00000800 ccd5c100 ccd5c100
> 3fc0: 00000001 7f64cd00 bed3ff0d 00000081 7f64c008 00000000 00000002 00000800
> 3fe0: bed3fe0c bed3fb98 b6eeb068 b6e4f05c 60000010 7f64cd30 00000000
> 00000000
> Backtrace:
> [<c036d918>] (usbhs_pkt_pop) from [<c036e364>]
> (usbhsg_ep_dequeue+0x28/0x40)
>  r9:ee022000 r8:c00101e4 r7:00200200 r6:00100100 r5:eeb917c0 r4:ee1b1400
> [<c036e33c>] (usbhsg_ep_dequeue) from [<bf002f20>]
> (composite_dev_cleanup+0xd0/0x11c [libcomposite])
>  r5:eeb341c0 r4:eeb34180
> [<bf002e50>] (composite_dev_cleanup [libcomposite]) from [<bf002fec>]
> (__composite_unbind+0x80/0xa0 [libcomposite])
>  r7:eeb341b8 r6:ee1b1800 r5:00000001 r4:eeb34180
> [<bf002f6c>] (__composite_unbind [libcomposite]) from [<bf003028>]
> (composite_unbind+0x1c/0x20 [libcomposite])
>  r7:00000081 r6:bed3ff0d r5:bf01d624 r4:ee181600
> [<bf00300c>] (composite_unbind [libcomposite]) from [<c036fb88>]
> (usb_gadget_remove_driver+0x60/0x88)
> [<c036fb28>] (usb_gadget_remove_driver) from [<c036fc10>]
> (usb_gadget_unregister_driver+0x60/0xa0)
>  r5:bf01d624 r4:ee181600
> [<c036fbb0>] (usb_gadget_unregister_driver) from [<bf0013d0>]
> (usb_composite_unregister+0x1c/0x20 [libcomposite])
>  r5:7f64cd30 r4:bf01d694
> [<bf0013b4>] (usb_composite_unregister [libcomposite]) from [<bf01d26c>]
> (msg_cleanup+0x30/0x3c [g_mass_storage])
> [<bf01d23c>] (msg_cleanup [g_mass_storage]) from [<c0090bd8>]
> (SyS_delete_module+0x108/0x1c4)
> [<c0090ad0>] (SyS_delete_module) from [<c0010040>]
> (ret_fast_syscall+0x0/0x3c)
>  r5:7f64cd00 r4:00000001
> Code: e52de004 e8bd4000 e1a05000 e1a04001 (e5907004)
> ---[ end trace 8152e492b3f02f66 ]---
> 
> 
> I don't know why this kernel panic happened after I applied this patch.
> But, if I added the following code in the mod_gadget.c, this issue disappeared:
> 
> @@ -682,7 +684,8 @@ static int usbhsg_ep_dequeue(struct usb_ep *ep, struct
> usb_r
>         struct usbhsg_request *ureq = usbhsg_req_to_ureq(req);
>         struct usbhs_pipe *pipe = usbhsg_uep_to_pipe(uep);
> 
> -       usbhs_pkt_pop(pipe, usbhsg_ureq_to_pkt(ureq));
> +       if (pipe)
> +               usbhs_pkt_pop(pipe, usbhsg_ureq_to_pkt(ureq));
>         usbhsg_queue_pop(uep, ureq, -ECONNRESET);
> 
>         return 0;

That's odd, I can't see how my patch causes this problem. Do you think
that there has always been a race problem here and my changes make this
happen?

> Best regards,
> Yoshihiro Shimoda
> 

Best regards
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/