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 Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E7BFAC46467 for ; Tue, 10 Jan 2023 13:55:42 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3312285265; Tue, 10 Jan 2023 14:55:40 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20210112.gappssmtp.com header.i=@baylibre-com.20210112.gappssmtp.com header.b="xMJpo2IX"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8F66285265; Tue, 10 Jan 2023 14:55:37 +0100 (CET) Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 8408185499 for ; Tue, 10 Jan 2023 14:55:30 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mkorpershoek@baylibre.com Received: by mail-wr1-x42f.google.com with SMTP id t5so7415678wrq.1 for ; Tue, 10 Jan 2023 05:55:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=tUkgnz6GeUySXSQ4WKC0pxbJJNak9LhEO+1FQDeijrc=; b=xMJpo2IXpqTf+5qxvMVf3x1yGsXhMRH6UwdyOwHHlH42GR0xi+mHDeCPwYlt0xe7JX tG8lA83EEeRaDlpBuFU7e46PTmd7fcewGnqgO/fAKdBgVitkrM1FHHueSMn2xAzXTwGe fnY5WNxfCigAC7c9NIIY8qHI+76An1mVU+jtpbiXjiy1fZ/dxDkhW80u6d957nLhwKEf WrvSg5v4l0MawupdP+URvAg/7VFiqoCCJi6f6yJUZdk5x2kNEhk3OOlKfodqDMtPQfM8 mbbOssPflg+REgjkgcaEZ/tTrOYSQwE6gBYb7j3550aebR/MZTW52AIrtONmJQFe6J05 ORhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tUkgnz6GeUySXSQ4WKC0pxbJJNak9LhEO+1FQDeijrc=; b=I1sslxTLPc9lKYcziAzutRPYfSFPSy3efkQevbBtvVOwnqOoNnxNegV0m2w8BLmA0j W7bxr2qjAAmjptR7mY/eM7ORL11YpbHqCtsQimKTBZfDV8dGBw94OLsDtcAzotT+YP2+ eEKYTMhcSqKY4rVHtv6j7fJn9Kc8vmbJL+0kM2d1vtZgGadcOxY0KyUz056kAcdSP4sW tY7XqFh/PK66TizFBtUF8Vh0qCaOoZEfB0JrqVkDOA0qyt0/7bVvhMvMgXQmPZ3E0d4o DkLP5Xr8vaa5A70sWY58QAj43nBqQXSWqzM/bWaAKeNfiWLqvvMo7AlJyzop843WAdRM zfdQ== X-Gm-Message-State: AFqh2kqlkRT/g+IyIKfWOK7Xy4HKbPUy5G1GS+yym1jcPhoatezEx17K Nyn5zGWouuJEb5F5Uead5BCEfQ== X-Google-Smtp-Source: AMrXdXs3ntL9+AE0NAFhP/4W2st5/Jxi+8RX0xyPa3sy2Izp1qFVHWE6ObDWA7DZZbxrP3cQsJhaYw== X-Received: by 2002:a5d:560a:0:b0:27b:45ba:3b47 with SMTP id l10-20020a5d560a000000b0027b45ba3b47mr35901531wrv.57.1673358929788; Tue, 10 Jan 2023 05:55:29 -0800 (PST) Received: from localhost ([2a01:cb19:85e6:1900:2bf7:7388:731d:c4e1]) by smtp.gmail.com with ESMTPSA id g1-20020a056000118100b002755e301eeasm11043389wrx.100.2023.01.10.05.55.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Jan 2023 05:55:29 -0800 (PST) From: Mattijs Korpershoek To: Marek Vasut , Harald Seiler , Lukasz Majewski Cc: Dario Binacchi , Michael Trimarchi , u-boot@lists.denx.de Subject: Re: [PATCH] usb: gadget: dwc2_udc_otg: implement pullup() In-Reply-To: <9c725f25-5976-28e0-b5b0-08f30c885dce@denx.de> References: <20230110-dwc2-pullup-v1-1-2bf9743e59e5@baylibre.com> <45c496c31b29671a1f3d38c95362db0d238c70a6.camel@denx.de> <87eds28q5a.fsf@baylibre.com> <9c725f25-5976-28e0-b5b0-08f30c885dce@denx.de> Date: Tue, 10 Jan 2023 14:55:28 +0100 Message-ID: <87358i8pf3.fsf@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Tue, Jan 10, 2023 at 14:45, Marek Vasut wrote: > On 1/10/23 14:39, Mattijs Korpershoek wrote: >> Hi Harald, >> >> Thank you for your review. >> >> On Tue, Jan 10, 2023 at 14:30, Harald Seiler wrote: >> >>> Hi, >>> >>> On Tue, 2023-01-10 at 10:11 +0100, Mattijs Korpershoek wrote: >>>> Pullup is used by the usb framework in order to do software-controlled >>>> usb_gadget_connect() and usb_gadget_disconnect(). >>>> >>>> Implement pullup() for dwc2 using the SOFT_DISCONNECT bit in the dctl >>>> register. >>>> >>>> This is especially useful when a gadget disconnection is initiated but >>>> no board_usb_cleanup() is called. >>>> >>>> Signed-off-by: Mattijs Korpershoek >>>> --- >>>> On some boards using the dwc2 controller, like the Khadas VIM3L, whenever >>>> usb_gadget_release() is called, the D+ and D- lines are in an unknown state. >>>> >>>> Because of that, the host can't detect usb disconnection. >>>> >>>> It was attempted to be be fixed with [1] but ended up doing the gadget disconnection >>>> too early, creating issues on NXP-based boards which use uuu [2]. >>>> >>>> By implementing pullup() in the controller driver, we ensure that the disconnection will >>>> only be done when the framework calls usb_gadget_disconnect(). >>>> >>>> [1] https://lore.kernel.org/all/20220728-reset-usb-controller-v2-1-ef7657ce78b1@baylibre.com/ >>>> [2] https://lore.kernel.org/all/20230107164807.3597020-1-dario.binacchi@amarulasolutions.com/ >>>> --- >>>> drivers/usb/gadget/dwc2_udc_otg.c | 16 ++++++++++++++++ >>>> 1 file changed, 16 insertions(+) >>>> >>>> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c >>>> index 77988f78ab30..93ed9712d18a 100644 >>>> --- a/drivers/usb/gadget/dwc2_udc_otg.c >>>> +++ b/drivers/usb/gadget/dwc2_udc_otg.c >>>> @@ -236,6 +236,21 @@ static int udc_enable(struct dwc2_udc *dev) >>>> return 0; >>>> } >>>> >>>> +static int dwc2_gadget_pullup(struct usb_gadget *g, int is_on) >>>> +{ >>>> + unsigned int uTemp; >>> >>> Just some minor points about style... >>> >>> I think the `u32` type should be used for 32-bit registers instead. >>> More explicit and no room for accidentally choosing the wrong size. >>> >>> Also, U-Boot and Linux don't use hungarian notation for variable names, >>> just call it `tmp` or even better `dctl` to be explicit. >> >> I agree with this. I actually picked up some of the code from >> reconfig_usbd() where hungarian nototation is used. That's also where >> the unsigned int comes from. >> >> Sorry I did not convert it to some more Linux/U-Boot oriented style >> before submitting. >> >>> >>>> + >>>> + is_on = !!is_on; >>> >>> This is superfluous, the if condition works either way. >> >> Ack. >> >> The maintainer (Marek) already picked this up in his usb tree: >> https://lore.kernel.org/all/1f2c5d74-faae-42f8-5d4d-dfc08cb093aa@denx.de/ >> >> Should I send a clean-up/follow-up patch for this? > > The maintainer already dropped it and waits for V2 :) Thank you, will send a V2 shortly.