From patchwork Tue Sep 2 17:31:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 36501 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qc0-f198.google.com (mail-qc0-f198.google.com [209.85.216.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 753022032B for ; Tue, 2 Sep 2014 17:32:11 +0000 (UTC) Received: by mail-qc0-f198.google.com with SMTP id r5sf23249496qcx.1 for ; Tue, 02 Sep 2014 10:32:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :reply-to:references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=ZmbumnvYAOb6vMrsYg/VEUKEnO+bn7Pdc9ICCUKmFFs=; b=Vr5jXA5EUqtfl3933j7UW3J3ljqcR+uPDmZhMxt+PCzW98ca3t+os9IhOtx79XCrYz U8N6TO9n8B7u6xtELVWE4BXkHHFEQNOW9ABLl9Kd5Z0GW9bs0raxG54cbNf+WTarypjK DcMwJdwfT9ya6FtlyY+grW5SK0g5gVJVyLfK+MW9zSdFpTkbLBG7BAaIsj1FTOugbhUw uDDkqZf6nY3WLocaj60V/Ot0PItwNO0PrPd+6IzMfRAP4Lq1kuBVEI8iK+Mx4qt8e6fQ 1UrNVKYJsCxQyX/fI86DRx5g5xMH+FLcV/toFKT6ATT/DfkcfjHXLgGnqW5ty8Rmn4ow nB4A== X-Gm-Message-State: ALoCoQmuQhJ/KahgK49tMKYjgSITX411Xvw+XlnuBYwnZbBuIi8lUyHr0VMsxOnaLp3haWWcWHko X-Received: by 10.236.148.2 with SMTP id u2mr1163873yhj.55.1409679131332; Tue, 02 Sep 2014 10:32:11 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.108.38 with SMTP id i35ls2493578qgf.76.gmail; Tue, 02 Sep 2014 10:32:11 -0700 (PDT) X-Received: by 10.220.59.138 with SMTP id l10mr1012440vch.59.1409679131202; Tue, 02 Sep 2014 10:32:11 -0700 (PDT) Received: from mail-vc0-f173.google.com (mail-vc0-f173.google.com [209.85.220.173]) by mx.google.com with ESMTPS id rx9si2605945vdc.64.2014.09.02.10.32.11 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 02 Sep 2014 10:32:11 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.173 as permitted sender) client-ip=209.85.220.173; Received: by mail-vc0-f173.google.com with SMTP id im17so7352804vcb.4 for ; Tue, 02 Sep 2014 10:32:11 -0700 (PDT) X-Received: by 10.52.249.66 with SMTP id ys2mr1429010vdc.41.1409679131123; Tue, 02 Sep 2014 10:32:11 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.45.67 with SMTP id uj3csp562714vcb; Tue, 2 Sep 2014 10:32:10 -0700 (PDT) X-Received: by 10.66.229.8 with SMTP id sm8mr50102815pac.66.1409679129734; Tue, 02 Sep 2014 10:32:09 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ff10si6991000pdb.137.2014.09.02.10.32.09 for ; Tue, 02 Sep 2014 10:32:09 -0700 (PDT) Received-SPF: none (google.com: linux-usb-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754877AbaIBRcG (ORCPT + 2 others); Tue, 2 Sep 2014 13:32:06 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:60173 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754183AbaIBRcE (ORCPT ); Tue, 2 Sep 2014 13:32:04 -0400 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id s82HVuOC018160; Tue, 2 Sep 2014 12:31:56 -0500 Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s82HVuP5009000; Tue, 2 Sep 2014 12:31:56 -0500 Received: from dflp32.itg.ti.com (10.64.6.15) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.174.1; Tue, 2 Sep 2014 12:31:56 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s82HVtZZ000636; Tue, 2 Sep 2014 12:31:56 -0500 Date: Tue, 2 Sep 2014 12:31:49 -0500 From: Felipe Balbi To: Alan Stern CC: Felipe Balbi , Peter Chen , USB list , Subject: Re: [RFC PATCH 0/7] usb: gadget: add reset API at usb_gadget_driver Message-ID: <20140902173149.GM16872@saruman.home> Reply-To: References: <20140902160903.GJ16872@saruman.home> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-usb-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: balbi@ti.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.173 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi, On Tue, Sep 02, 2014 at 01:15:18PM -0400, Alan Stern wrote: > On Tue, 2 Sep 2014, Felipe Balbi wrote: > > > > That needn't be a problem. If Peter updates the four gadget drivers, > > > adding reset callbacks, then we can remove the parts of our patches > > > that invoke the disconnect callback if there is no reset callback. In > > > other words, we can make reset callbacks mandatory for gadget drivers. > > > > alright, but we need to do this in steps to avoid regressions or a > > non-bisectable tree. So maybe we add ->reset as an optional method, > > implement support for it to all UDC drivers, patch all gadget drivers to > > implement reset, make reset mandatory. Does that work ? > > It would be okay, but doing things in a different order would be a > little better: Add the reset callback to the usb_gadget_driver > structure, implement it in all four gadget drivers, and then implement > it in the UDC drivers. That way we don't have to make a second round it can't be only in these four gadget drivers. It needs to be implemented in all of them even if: otherwise ->reset() will be optional and that means UDC will have to: if (gadget_driver->reset) gadget_driver->reset(g); else if (gadget_driver->disconnect && g.speed != UNKNOWN) gadget_driver->disconnect(g); and then we end up with a possibility of disconnecting from the host when we don't want to. Bottom line, we _must_ tell the gadget driver about a reset IRQ, so it can reset its internal state. > of modifications to the UDC drivers, removing the fallback calls to > ->disconnect for when ->reset is missing. ->reset() can't be missing if it were to be mandatory, right ? > The kerneldoc could state that some UDC drivers may call ->disconnect > when a reset occurs, instead of calling ->reset. Then we won't have to > fix up all the UDC drivers at once. we won't be able to do that because the discussion on this thread is that ->disconnect() should call usb_gadget_disconnect(). > If you want, you could add a remark to the kerneldoc saying that a > disconnect handler generally should do everything that a reset handler > does plus perhaps a few additional things. yeah, that additional thing is usb_gadget_disconnect() and we don't want to call that from a simple USB reset. diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c index 986fc51..2210289 100644 --- a/drivers/usb/gadget/legacy/dbgp.c +++ b/drivers/usb/gadget/legacy/dbgp.c @@ -409,6 +409,7 @@ static __refdata struct usb_gadget_driver dbgp_driver = { .unbind = dbgp_unbind, .setup = dbgp_setup, .disconnect = dbgp_disconnect, + .reset = dbgp_disconnect, .driver = { .owner = THIS_MODULE, .name = "dbgp"