From patchwork Fri Aug 9 12:16:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Chen X-Patchwork-Id: 2841933 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5F9349F295 for ; Fri, 9 Aug 2013 14:10:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 04CC2201FD for ; Fri, 9 Aug 2013 14:10:56 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2EBB9201F9 for ; Fri, 9 Aug 2013 14:10:54 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1V7nOz-0007Yg-LQ; Fri, 09 Aug 2013 14:10:49 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1V7nOx-0006WI-HQ; Fri, 09 Aug 2013 14:10:47 +0000 Received: from mail-db9lp0248.outbound.messaging.microsoft.com ([213.199.154.248] helo=db9outboundpool.messaging.microsoft.com) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1V7nOu-0006Vr-80 for linux-arm-kernel@lists.infradead.org; Fri, 09 Aug 2013 14:10:45 +0000 Received: from mail32-db9-R.bigfish.com (10.174.16.239) by DB9EHSOBE012.bigfish.com (10.174.14.75) with Microsoft SMTP Server id 14.1.225.22; Fri, 9 Aug 2013 14:10:22 +0000 Received: from mail32-db9 (localhost [127.0.0.1]) by mail32-db9-R.bigfish.com (Postfix) with ESMTP id EC55C26017A; Fri, 9 Aug 2013 14:10:21 +0000 (UTC) X-Forefront-Antispam-Report: CIP:70.37.183.190; KIP:(null); UIP:(null); IPV:NLI; H:mail.freescale.net; RD:none; EFVD:NLI X-SpamScore: -2 X-BigFish: VS-2(zz98dI1432Izz1f42h208ch1ee6h1de0h1fdah2073h1202h1e76h1d1ah1d2ah1fc6hzz1de098h8275bh1de097hz2dh2a8h668h839h944hd25hf0ah1220h1288h12a5h12a9h12bdh137ah13b6h1441h1504h1537h153bh162dh1631h1758h18e1h1946h19b5h1ad9h1b0ah1b2fh1fb3h1d0ch1d2eh1d3fh1dfeh1dffh1fe8h1155h) Received: from mail32-db9 (localhost.localdomain [127.0.0.1]) by mail32-db9 (MessageSwitch) id 1376057419835903_8179; Fri, 9 Aug 2013 14:10:19 +0000 (UTC) Received: from DB9EHSMHS008.bigfish.com (unknown [10.174.16.239]) by mail32-db9.bigfish.com (Postfix) with ESMTP id BB681460047; Fri, 9 Aug 2013 14:10:19 +0000 (UTC) Received: from mail.freescale.net (70.37.183.190) by DB9EHSMHS008.bigfish.com (10.174.14.18) with Microsoft SMTP Server (TLS) id 14.16.227.3; Fri, 9 Aug 2013 14:10:13 +0000 Received: from tx30smr01.am.freescale.net (10.81.153.31) by 039-SN1MMR1-001.039d.mgd.msft.net (10.84.1.13) with Microsoft SMTP Server (TLS) id 14.3.136.1; Fri, 9 Aug 2013 14:10:11 +0000 Received: from shlinux1.ap.freescale.net (shlinux1.ap.freescale.net [10.192.225.216]) by tx30smr01.am.freescale.net (8.14.3/8.14.0) with ESMTP id r79EA4Ls031865; Fri, 9 Aug 2013 07:10:06 -0700 Date: Fri, 9 Aug 2013 20:16:01 +0800 From: Peter Chen To: Alexander Shishkin Subject: Re: [PATCH v14 11/12] usb: chipidea: udc: misuse flag CI_HDRC_REGS_SHARED and CI_HDRC_PULLUP_ON_VBUS Message-ID: <20130809121600.GA26158@shlinux1.ap.freescale.net> References: <1375432445-31918-1-git-send-email-peter.chen@freescale.com> <1375432445-31918-12-git-send-email-peter.chen@freescale.com> <874nazathc.fsf@ashishki-desk.ger.corp.intel.com> <20130809114559.GC24763@shlinux1.ap.freescale.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130809114559.GC24763@shlinux1.ap.freescale.net> User-Agent: Mutt/1.5.20 (2009-06-14) X-OriginatorOrg: freescale.com X-FOPE-CONNECTOR: Id%0$Dn%*$RO%0$TLS%0$FQDN%$TlsDn% X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130809_101044_485686_CA4E756D X-CRM114-Status: GOOD ( 26.51 ) X-Spam-Score: -0.1 (/) Cc: marex@denx.de, m.grzeschik@pengutronix.de, frank.li@freescale.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, mkl@pengutronix.de, kernel@pengutronix.de, maxime.ripard@free-electrons.com, shawn.guo@linaro.org, festevam@gmail.com, linux-arm-kernel@lists.infradead.org, LW@KARO-electronics.de X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,KHOP_BIG_TO_CC, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Aug 09, 2013 at 07:46:00PM +0800, Peter Chen wrote: > On Fri, Aug 09, 2013 at 04:23:11PM +0300, Alexander Shishkin wrote: > > Peter Chen writes: > > > > > CI_HDRC_REGS_SHARED stands for the controller registers is shared > > > with other USB drivers, if all USB drivers are at chipidea/, it doesn't > > > needed to set. > > > > We still have the msm driver that uses REGS_SHARED. > > > > Yes, I have considered it. At udc interrupt handler, the REGS_SHARED > is still used. The msm set both CI_HDRC_REGS_SHARED and CI_HDRC_PULLUP_ON_VBUS. > > > > CI_HDRC_PULLUP_ON_VBUS stands for pullup dp when the vbus is on. This > > > flag doesn't need to set if the vbus is always on for gadget > > > since dp has always pulled up after the gadget has initialized. > > > > Didn't we agree at some point to get rid of this flag altogether once we > > have proper VBUS detection? > > Yes, we can delete it now, the reason why I haven't remove it is: > I met some use cases that the vbus is always on recently, > no connection/disconnection. Eg, the USB audio device connects > to Apple Sound machine, the vbus is the power of the device system. > > I checked the code just now again, we can cover such kind of case. I will have a patch to delete CI_HDRC_PULLUP_ON_VBUS. If we squash two patches, the change like below: diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index bd1fe25..ab3e74a 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -91,7 +91,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) .name = "ci_hdrc_imx", .capoffset = DEF_CAPOFFSET, .flags = CI_HDRC_REQUIRE_TRANSCEIVER | - CI_HDRC_PULLUP_ON_VBUS | CI_HDRC_DISABLE_STREAMING, }; int ret; diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index fb657ef..2d51d85 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -49,7 +49,6 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = { .name = "ci_hdrc_msm", .flags = CI_HDRC_REGS_SHARED | CI_HDRC_REQUIRE_TRANSCEIVER | - CI_HDRC_PULLUP_ON_VBUS | CI_HDRC_DISABLE_STREAMING, .notify_event = ci_hdrc_msm_notify_event, diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index c70ce38..f77c904 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1635,14 +1635,11 @@ static int ci_udc_start(struct usb_gadget *gadget, ci->driver = driver; pm_runtime_get_sync(&ci->gadget.dev); - if (ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS) { - if (ci->vbus_active) { - if (ci->platdata->flags & CI_HDRC_REGS_SHARED) - hw_device_reset(ci, USBMODE_CM_DC); - } else { - pm_runtime_put_sync(&ci->gadget.dev); - goto done; - } + if (ci->vbus_active) { + hw_device_reset(ci, USBMODE_CM_DC); + } else { + pm_runtime_put_sync(&ci->gadget.dev); + goto done; } retval = hw_device_state(ci, ci->ep0out->qh.dma); @@ -1665,8 +1662,7 @@ static int ci_udc_stop(struct usb_gadget *gadget, spin_lock_irqsave(&ci->lock, flags); - if (!(ci->platdata->flags & CI_HDRC_PULLUP_ON_VBUS) || - ci->vbus_active) { + if (ci->vbus_active) { hw_device_state(ci, 0); if (ci->platdata->notify_event) ci->platdata->notify_event(ci, @@ -1801,12 +1797,6 @@ static int udc_start(struct ci_hdrc *ci) } } - if (!(ci->platdata->flags & CI_HDRC_REGS_SHARED)) { - retval = hw_device_reset(ci, USBMODE_CM_DC); - if (retval) - goto put_transceiver; - } - if (ci->transceiver) { retval = otg_set_peripheral(ci->transceiver->otg, &ci->gadget); diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h index 10a607c..7c4ba37 100644 --- a/include/linux/usb/chipidea.h +++ b/include/linux/usb/chipidea.h @@ -18,13 +18,12 @@ struct ci_hdrc_platform_data { unsigned long flags; #define CI_HDRC_REGS_SHARED BIT(0) #define CI_HDRC_REQUIRE_TRANSCEIVER BIT(1) -#define CI_HDRC_PULLUP_ON_VBUS BIT(2) -#define CI_HDRC_DISABLE_STREAMING BIT(3) +#define CI_HDRC_DISABLE_STREAMING BIT(2) /* * Only set it when DCCPARAMS.DC==1 and DCCPARAMS.HC==1, * but otg is not supported (no register otgsc). */ -#define CI_HDRC_DUAL_ROLE_NOT_OTG BIT(4) +#define CI_HDRC_DUAL_ROLE_NOT_OTG BIT(3) enum usb_dr_mode dr_mode; #define CI_HDRC_CONTROLLER_RESET_EVENT 0 #define CI_HDRC_CONTROLLER_STOPPED_EVENT 1