Received: by 2002:a05:7412:2a8c:b0:e2:908c:2ebd with SMTP id u12csp2205424rdh; Tue, 26 Sep 2023 16:27:25 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGf2Bko1Zw6S9u9C7GIftapafz4wnEQnhkl4GBLpaDmMK0ly7lFMEs/vKvNthRafC+AhI+J X-Received: by 2002:a17:903:1cc:b0:1b8:8ff5:2cee with SMTP id e12-20020a17090301cc00b001b88ff52ceemr156006plh.64.1695770845454; Tue, 26 Sep 2023 16:27:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695770845; cv=none; d=google.com; s=arc-20160816; b=OscmwVozM8aXdbklPYZbXUaIvpwzr72FSZsKgx86StNODb/Ua0ZVbEAlkdU2rrQcqw 2FJJ2MisnGAdHDPLSwX2rm2RJj6RBXK8zyMNNdZ857iclUDKFeVFs1tSyl8i9ypZF2A7 CpXnUl2YJHrXm78nXJjBZIJ3TvgVKfy1fLRa7Z1qnyo+GeDcsFuq844+n493XPLoVyB3 q92gW7OHy1K9A8yAoFPm1F01qJ0mqaULdPK0yQ2XFfx+HlLetcAhj7Qbn3MsSgatIavk HtjE4ZhbcE9tZJOX5lRNMqXOqiALGBTR4S+zaHLxKZth0ae4SUo0K3fmEL8d7xGojB2Y ZupQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=2WUJ712wkNz+iv71Iz9j4s1oQhhL8WtXCprWWFa2iiQ=; fh=5yVmm0fdeqq/fviVmHVhhRovjPyYq/LepqaXNsR7uFY=; b=A49T8y+TBHvGv+9DFN2Dg5tMBvPOnVYrDf6LA02BkAMJznM7lJHJ9qSIFUQGr/KoF3 9c6nyF67GE01eKDO1hF0wpDob//hLPRRg6/GIXc+8HhfXDu1FnmfOjSoQEP6xrsHh5Kb 4ycU4z3G0FQr0iPXUYcv8F+5V35CmrCPOt2rJNKS/SkCKjWSfLaxzy7IQIlvZqrfqHl+ IUwVux6ZyAvoym+c+t6eIzWGitKoBT85QRkmg8NeLFLjctIQVpov//5+DpoyaVodEVl2 exog1TH/zjKbmpvOpzoVc3ax73OnY4nAN3Pw49CiVkDmkkZ1omGi+zbHh2e8Vlxgtd+9 rCvw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=harvard.edu Return-Path: Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id o4-20020a170902d4c400b001bf2931ccdesi10836412plg.232.2023.09.26.16.27.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Sep 2023 16:27:25 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=harvard.edu Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 99F9B81410FC; Tue, 26 Sep 2023 16:24:07 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230508AbjIZXYD (ORCPT + 99 others); Tue, 26 Sep 2023 19:24:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232648AbjIZXWC (ORCPT ); Tue, 26 Sep 2023 19:22:02 -0400 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by lindbergh.monkeyblade.net (Postfix) with SMTP id A119BA27A for ; Tue, 26 Sep 2023 15:24:21 -0700 (PDT) Received: (qmail 1476453 invoked by uid 1000); 26 Sep 2023 17:24:19 -0400 Date: Tue, 26 Sep 2023 17:24:19 -0400 From: Alan Stern To: Krishna Kurapati PSSNV Cc: Greg Kroah-Hartman , Francesco Dolcini , Badhri Jagan Sridharan , Michael Grzeschik , Ivan Orlov , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, quic_ppratap@quicinc.com, quic_wcheng@quicinc.com, quic_jackp@quicinc.com Subject: Re: [PATCH v4] usb: gadget: udc: Handle gadget_connect failure during bind operation Message-ID: <9384ac6a-f877-4835-b1ec-0e620a5ba8ba@rowland.harvard.edu> References: <20230926193708.22405-1-quic_kriskura@quicinc.com> <2178cf29-5e5c-4ed6-8d1c-916bc7036589@rowland.harvard.edu> <62083b55-0b78-4ebc-ab78-1c1d99f92507@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_BLOCKED,SPF_HELO_PASS, SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 26 Sep 2023 16:24:07 -0700 (PDT) On Wed, Sep 27, 2023 at 01:54:34AM +0530, Krishna Kurapati PSSNV wrote: > > > On 9/27/2023 1:36 AM, Krishna Kurapati PSSNV wrote: > > > > ? drivers/usb/gadget/udc/core.c | 23 +++++++++++++++++++---- > > > > ? 1 file changed, 19 insertions(+), 4 deletions(-) > > > > > > > > ? static void vbus_event_work(struct work_struct *work) > > > > @@ -1604,12 +1608,23 @@ static int gadget_bind_driver(struct > > > > device *dev) > > > > ????? } > > > > ????? usb_gadget_enable_async_callbacks(udc); > > > > ????? udc->allow_connect = true; > > > > -??? usb_udc_connect_control_locked(udc); > > > > +??? ret = usb_udc_connect_control_locked(udc); > > > > +??? if (ret) { > > > > +??????? mutex_unlock(&udc->connect_lock); > > > > +??????? goto err_connect_control; > > > > +??? } > > > > + > > > > ????? mutex_unlock(&udc->connect_lock); > > > > ????? kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); > > > > ????? return 0; > > > > + err_connect_control: > > > > +??? usb_gadget_disable_async_callbacks(udc); > > > > +??? if (gadget->irq) > > > > +??????? synchronize_irq(gadget->irq); > > > > +??? usb_gadget_udc_stop_locked(udc); > > > > > > Not good -- usb_gadget_udc_stop_locked() expects you to be holding > > > udc->connect_lock, but you just dropped the lock!? Also, you never set > > > udc->allow_connect back to false. > > > > > > You should move the mutex_unlock() call from inside the "if" statement > > > to down here, and add a line for udc->allow_connect. > > > > > > > Hi Alan, > > > > ?Thanks for the review. Will push v5 addressing the changes. > > > > > Hi Alan, > > I tried out the following diff: > > - usb_udc_connect_control_locked(udc); > + ret = usb_udc_connect_control_locked(udc); > + if (ret) > + goto err_connect_control; > + > mutex_unlock(&udc->connect_lock); > > kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); > return 0; > > + err_connect_control: > + udc->allow_connect = false; > + usb_gadget_disable_async_callbacks(udc); > + if (gadget->irq) > + synchronize_irq(gadget->irq); > + usb_gadget_udc_stop_locked(udc); > + mutex_unlock(&udc->connect_lock); > + > > If I clear UDC and fail dwc3 soft reset on purpose, I see UDC_store failing: > > #echo a600000.usb > /sys/kernel/config/usb_gadget/g1/UDC > [ 127.394087] dwc3 a600000.usb: request 000000003f43f907 was not queued to > ep0out > [ 127.401637] udc a600000.usb: failed to start g1: -110 > [ 127.406841] configfs-gadget.g1: probe of gadget.0 failed with error -110 > [ 127.413809] UDC core: g1: couldn't find an available UDC or it's busy > > The same output came when I tested v4 as well. Every time soft_reset would > fail when I try to write to UDC, UDC_store fails and above log will come up. Isn't that what you want? I thought the whole purpose of this patch was to make it so that configfs would realize when usb_udc_connect_control_locked() had failed. So you should be happy that the log shows a failure occurred. > Can you help confirm if the diff above is proper as I don't see any diff in > the logs in v4 and about to push v5. "Diff in the logs in v4"? What does that mean? A diff is a comparison between two text files (often between before-and-after versions of a source code file). Why would you expect a diff to show up in the logs? This revised patch looks okay to me. Alan Stern