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=-6.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 DD98BC433F5 for ; Thu, 9 Sep 2021 09:48:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BB1A5611C1 for ; Thu, 9 Sep 2021 09:48:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233150AbhIIJtR (ORCPT ); Thu, 9 Sep 2021 05:49:17 -0400 Received: from mail.kernel.org ([198.145.29.99]:50648 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233353AbhIIJtN (ORCPT ); Thu, 9 Sep 2021 05:49:13 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id ACBEA611BD; Thu, 9 Sep 2021 09:48:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1631180884; bh=OdJ9hF8aF+EiFbO4nYWFpj9ePWRPK7plwXk/kGVvHMs=; h=References:From:To:Cc:Subject:Date:In-reply-to:From; b=dTxXkeil9JfzGywlLvd2UoTIuIfw3FLQQoBC/zeMP3wrTrtwrwseDPKkXMmLRRHm/ u6klNYX6SXr4bACzE6up2ogUs08lvZWIfLvVAR6ndtm8t+mXOITbYBfBK2HBTbSrwc 8AHUkc3T1bCgjeZ5lwpiGY7vyDIm+FnhSwsC7Jvs1+ak2wKeQHoHDkzVoewdmWVWRG qY3D294zmE/obSB7U5glHju8bu3t8D6hjKEuwe40OS+mFrKtKfs4lBkhzC6aPVqmwP fH3vPHVhBY6Jg8plDzekFivH2mD8j4qmdxoBOMg/MooWaNlpuc3vzpFIO0exUOBvvG //BSz2BCU8sJA== References: <1631166347-25021-1-git-send-email-quic_linyyuan@quicinc.com> <87bl52b0i9.fsf@kernel.org> 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 12:44:59 +0300 In-reply-to: Message-ID: <877dfqaxm6.fsf@kernel.org> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Hi, "Linyu Yuan (QUIC)" writes: >> >> > > 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. > Hi Felipe, > > I update a v2 patch, no, I didn't use this pointer for role switch, > just used to check role switch status, make sure it complete. exactly, you're using it as a flag for role switch. But gadget pointer being invalid is not very indication of that :-) It could be that right now these two things correlate, but there's no guarantee. You end up writing code that has possibility of failing in the future ;-) > when do role switch from device to host mode, > first disable device mode, currently there is no better way to > confirm when gadget was complete removed. if ->udc_stop() finished running, you know for a fact that gadget was completely removed ;-) If you want to be doubly sure, you can somehow notify the UDC from the gadget_release() function, but that's a little more invasive change > change device release function to clear gadget pointer, > this will confirm device stop complete when it is NULL. And instead of checking for the pointer (which can be racy in some occasions), perhaps tell the UDC about it with a callback or something like that? -- balbi