From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71B98C433EF for ; Thu, 9 Sep 2021 08:45:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 44EA16113A for ; Thu, 9 Sep 2021 08:45:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231682AbhIIIqq (ORCPT ); Thu, 9 Sep 2021 04:46:46 -0400 Received: from mail.kernel.org ([198.145.29.99]:52932 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231234AbhIIIqq (ORCPT ); Thu, 9 Sep 2021 04:46:46 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 3EC7261139; Thu, 9 Sep 2021 08:45:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1631177137; bh=9ul2mQXB/wAf38NMXb8TFZ/Ew7YNPUC8eNwV/nt+xJQ=; h=References:From:To:Cc:Subject:Date:In-reply-to:From; b=YUPz8Jy2AoOuVGKdm/cDHknbW2N1GmrM3dj2mvdNvkomPYb6sViytJzDQ1OnpSqAk ATzFyZAjiUU6Fwjtuz3vsTbEKdP1brN5MIeAK5v0Oqb/6m/vkJZ1lRbCurfScnnUw1 YDJnooytSlIvP5WcsIWTOqJ4WlhZM/XYoga0ZBf96f+v+PMIOPeC1Mm23YBzH/rCGO NLOTVeUr/4RLP8eDZ5hKL47iGi/WI3I14JxYMSkvcBV1XwMnOQ3VOhjfmQq4hNUO+W HmbkUXruxkoVwx7Fpe6QgaH1LUq/PzGGnEM51tdOG4R/4xzpdB/RIgfZVDo5QbKAbE pHWCLW3JjCa6g== References: <1631166347-25021-1-git-send-email-quic_linyyuan@quicinc.com> User-agent: mu4e 1.6.5; emacs 27.2 From: Felipe Balbi To: "Linyu Yuan (QUIC)" Cc: Greg Kroah-Hartman , "linux-usb@vger.kernel.org" Subject: Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit Date: Thu, 09 Sep 2021 11:45:14 +0300 In-reply-to: Message-ID: <87bl52b0i9.fsf@kernel.org> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org "Linyu Yuan (QUIC)" writes: >> From: Greg Kroah-Hartman >> Sent: Thursday, September 9, 2021 3:38 PM >> To: Linyu Yuan (QUIC) >> Cc: Felipe Balbi ; linux-usb@vger.kernel.org >> Subject: Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit >> >> On Thu, Sep 09, 2021 at 07:17:57AM +0000, Linyu Yuan (QUIC) wrote: >> > >> > >> > > From: Greg Kroah-Hartman >> > > Sent: Thursday, September 9, 2021 2:11 PM >> > > To: Linyu Yuan (QUIC) >> > > Cc: Felipe Balbi ; linux-usb@vger.kernel.org >> > > Subject: Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit >> > > >> > > On Thu, Sep 09, 2021 at 01:45:47PM +0800, Linyu Yuan wrote: >> > > > change device release function to clear gadget pointer. >> > > >> > > That does not properly describe what and why this change is needed. >> > > >> > > > >> > > > Signed-off-by: Linyu Yuan >> > > > --- >> > > > drivers/usb/dwc3/gadget.c | 5 +++-- >> > > > 1 file changed, 3 insertions(+), 2 deletions(-) >> > > > >> > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> > > > index 804b505..e2ab5f6 100644 >> > > > --- a/drivers/usb/dwc3/gadget.c >> > > > +++ b/drivers/usb/dwc3/gadget.c >> > > > @@ -4188,9 +4188,10 @@ static int dwc3_gadget_get_irq(struct dwc3 >> > > *dwc) >> > > > >> > > > static void dwc_gadget_release(struct device *dev) >> > > > { >> > > > - struct usb_gadget *gadget = container_of(dev, struct usb_gadget, >> > > dev); >> > > > + struct dwc3 *dwc = dev_get_platdata(dev); >> > > >> > > Are you sure this is the same? >> > Yes, in dwc3_gadget_init() >> > usb_initialize_gadget(dwc->dev, dwc->gadget, dwc_gadget_release); >> > dev = &dwc->gadget->dev; >> > dev->platform_data = dwc; >> > >> > here original code use follow container_of, it use same dev, >> > container_of(dev, struct usb_gadget, dev); >> > > >> > > > >> > > > - kfree(gadget); >> > > > + kfree(dwc->gadget); >> > > > + dwc->gadget = NULL; >> > > >> > > Why set this to NULL? Who cares about this now? What changed to >> make >> > > it required? >> > It better to set to NULL for better understanding. >> >> Understanding of what? What issue does this fix? You need a reason to >> make this, or any, kernel change. > > Sorry, let explain, for example, when do role switch, we can check it > value to make sure it switch complete, > > If we do not set to NULL, it will be invalid. Using this pointer as a role switch flag seems fragile, though. -- balbi