From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754597AbcEQHix (ORCPT ); Tue, 17 May 2016 03:38:53 -0400 Received: from mail-db3on0090.outbound.protection.outlook.com ([157.55.234.90]:18720 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751791AbcEQHiu convert rfc822-to-8bit (ORCPT ); Tue, 17 May 2016 03:38:50 -0400 From: Jun Li To: Roger Quadros , Peter Chen CC: "peter.chen@freescale.com" , "balbi@kernel.org" , "tony@atomide.com" , "gregkh@linuxfoundation.org" , "dan.j.williams@intel.com" , "mathias.nyman@linux.intel.com" , "Joao.Pinto@synopsys.com" , "sergei.shtylyov@cogentembedded.com" , "jun.li@freescale.com" , "grygorii.strashko@ti.com" , "yoshihiro.shimoda.uh@renesas.com" , "robh@kernel.org" , "nsekhar@ti.com" , "b-liu@ti.com" , "linux-usb@vger.kernel.org" , "linux-omap@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" Subject: RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Thread-Topic: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Thread-Index: AQHRrP7fiGOaT66xRk2Vwa/MP6wYS5+7J+uAgAAXgoCAAA/EgIAAB/eAgAFqGKA= Date: Tue, 17 May 2016 07:38:46 +0000 Message-ID: References: <1463133808-10630-1-git-send-email-rogerq@ti.com> <1463133808-10630-14-git-send-email-rogerq@ti.com> <20160516070249.GB24609@shlinux2.ap.freescale.net> <57398451.2060103@ti.com> <20160516092323.GD24609@shlinux2.ap.freescale.net> <57399839.90706@ti.com> In-Reply-To: <57399839.90706@ti.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: ti.com; dkim=none (message not signed) header.d=none;ti.com; dmarc=none action=none header.from=nxp.com; x-originating-ip: [123.151.195.51] x-ms-office365-filtering-correlation-id: 447301c3-a978-4d33-2f01-08d37e264799 x-microsoft-exchange-diagnostics: 1;AM4PR04MB2131;5:c3VcwsuXRBWoBeRg7sPp7FBobIrz4L0t4gnX9DKhHoGfromZMHhr5xR8ekVjps1PTSCHexta//5LEUR842OkYBNuiwZwxONqBB7iFDzsX37DOjVtDwyoMehIkGwsXB8p/gLyg9fyHMlwWH0dOsUaWA==;24:qqcAdT95rtstAKaocQ7YcmG9h2TXM8xgJ8mf1ouC5UZdv4ixroLs16rRRmw3g8ms/8QDqB+A+vU7yoa7NsnD67VY/2RdEE1WtjQY7mhEmHM=;7:r55Gw23adnNabr73vKs8fU2EZ5KMU9uFMbECSv774kSqh5xfnG0WEMzgQIRMBS9fQ16gfmyMJ65IRBqhCs6CBtvzQJd6b9SHLEFrt1GsgTOy/JNi+jAXb94pWtMMelXqWkulAzKqe0ztrJFosRkIRTISv1HIzxXvTC2dvdhcygjU8zTJ/0FJJ7nYYOvnzcYS x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2131; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:AM4PR04MB2131;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2131; x-forefront-prvs: 0945B0CC72 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(13464003)(24454002)(377454003)(5008740100001)(5004730100002)(81166006)(8936002)(3280700002)(5002640100001)(86362001)(106116001)(122556002)(11100500001)(3660700001)(93886004)(74316001)(66066001)(8676002)(92566002)(6116002)(4326007)(77096005)(1220700001)(5003600100002)(102836003)(586003)(3846002)(2906002)(2950100001)(9686002)(87936001)(76176999)(10400500002)(189998001)(54356999)(2900100001)(19580405001)(33656002)(76576001)(5001770100001)(50986999)(19580395003)(8666002)(7059030);DIR:OUT;SFP:1101;SCL:1;SRVR:AM4PR04MB2131;H:AM4PR04MB2130.eurprd04.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 17 May 2016 07:38:46.2508 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR04MB2131 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi > -----Original Message----- > From: Roger Quadros [mailto:rogerq@ti.com] > Sent: Monday, May 16, 2016 5:52 PM > To: Peter Chen > Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com; > gregkh@linuxfoundation.org; dan.j.williams@intel.com; > mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com; > sergei.shtylyov@cogentembedded.com; jun.li@freescale.com; > grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com; > robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org; > linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core > > On 16/05/16 12:23, Peter Chen wrote: > > On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote: > >> Hi, > >> > >> On 16/05/16 10:02, Peter Chen wrote: > >>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote: > >>>> + > >>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget, > >>>> +bool connect) { > >>>> + struct usb_udc *udc; > >>>> + > >>>> + mutex_lock(&udc_lock); > >>>> + udc = usb_gadget_to_udc(gadget); > >>>> + if (!udc) { > >>>> + dev_err(gadget->dev.parent, "%s: gadget not > registered.\n", > >>>> + __func__); > >>>> + mutex_unlock(&udc_lock); > >>>> + return -EINVAL; > >>>> + } > >>>> + > >>>> + if (connect) { > >>>> + if (!gadget->connected) > >>>> + usb_gadget_connect(udc->gadget); > >>>> + } else { > >>>> + if (gadget->connected) { > >>>> + usb_gadget_disconnect(udc->gadget); > >>>> + udc->driver->disconnect(udc->gadget); > >>>> + } > >>>> + } > >>>> + > >>>> + mutex_unlock(&udc_lock); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>> > >>> Since this is called for vbus interrupt, why not using > >>> usb_udc_vbus_handler directly, and call udc->driver->disconnect at > >>> usb_gadget_stop. > >> > >> We can't assume that this is always called for vbus interrupt so I > >> decided not to call usb_udc_vbus_handler. > >> > >> udc->vbus is really pointless for us. We keep vbus states in our > >> state machine and leave udc->vbus as ture always. > >> > >> Why do you want to move udc->driver->disconnect() to stop? > >> If USB controller disconnected from bus then the gadget driver must > >> be notified about the disconnect immediately. The controller may or > >> may not be stopped by the core. > >> > > > > Then, would you give some comments when this API will be used? > > I was assumed it is only used for drd state machine. > > drd_state machine didn't even need this API in the first place :). > You guys wanted me to separate out start/stop and connect/disconnect for > full OTG case. > Won't full OTG state machine want to use this API? If not what would it > use? Instead create those new interfaces/symbol here and there just aim to address build problems in diff configures, Could we only allow meaningful combination of those 3 drivers configures? Hcd=y, gadget=y, otg=y or Hcd=m, gadget=m, otg=m Li Jun > > cheers, > -roger > > > > >>> > >>>> return 0; > >>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct > device *dev, > >>>> return -EOPNOTSUPP; > >>>> } > >>>> > >>>> + /* In OTG mode we don't support softconnect, but b_bus_req */ > >>>> + if (udc->gadget->otg_dev) { > >>>> + dev_err(dev, "soft-connect not supported in OTG mode\n"); > >>>> + return -EOPNOTSUPP; > >>>> + } > >>>> + > >>> > >>> The soft-connect can be supported at dual-role mode currently, we > >>> can use b_bus_req entry once it is implemented later. > >> > >> Soft-connect should be done via sysfs handling within the OTG core. > >> This can be added later. I don't want anything outside the OTG core > >> to handle soft-connect behaviour as it will be hard to keep things in > >> sync. > >> > >> I can update the comment to something like this. > >> > >> /* In OTG/dual-role mode, soft-connect should be handled by OTG core > >> */ > > > > Ok, let's Felipe decide it. > > > >> > >>> > >>>> if (sysfs_streq(buf, "connect")) { > >>>> usb_gadget_udc_start(udc); > >>>> - usb_gadget_connect(udc->gadget); > >>>> + usb_udc_connect_control(udc); > >>> > >>> This line seems to be not related with this patch. > >>> > >> Right. I'll remove it. > >> > >> cheers, > >> -roger > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jun Li Subject: RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Date: Tue, 17 May 2016 07:38:46 +0000 Message-ID: References: <1463133808-10630-1-git-send-email-rogerq@ti.com> <1463133808-10630-14-git-send-email-rogerq@ti.com> <20160516070249.GB24609@shlinux2.ap.freescale.net> <57398451.2060103@ti.com> <20160516092323.GD24609@shlinux2.ap.freescale.net> <57399839.90706@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <57399839.90706@ti.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Roger Quadros , Peter Chen Cc: "peter.chen@freescale.com" , "balbi@kernel.org" , "tony@atomide.com" , "gregkh@linuxfoundation.org" , "dan.j.williams@intel.com" , "mathias.nyman@linux.intel.com" , "Joao.Pinto@synopsys.com" , "sergei.shtylyov@cogentembedded.com" , "jun.li@freescale.com" , "grygorii.strashko@ti.com" , "yoshihiro.shimoda.uh@renesas.com" , "robh@kernel.org" , "nsekhar@ti.com" , "b-liu@ti.com" , "linux-usb@vger.kernel.org" , "linux-omap@vger.kernel.org" , "linux-kernel@vger.kernel.org" List-Id: devicetree@vger.kernel.org Hi > -----Original Message----- > From: Roger Quadros [mailto:rogerq@ti.com] > Sent: Monday, May 16, 2016 5:52 PM > To: Peter Chen > Cc: peter.chen@freescale.com; balbi@kernel.org; tony@atomide.com; > gregkh@linuxfoundation.org; dan.j.williams@intel.com; > mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com; > sergei.shtylyov@cogentembedded.com; jun.li@freescale.com; > grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com; > robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org; > linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core > > On 16/05/16 12:23, Peter Chen wrote: > > On Mon, May 16, 2016 at 11:26:57AM +0300, Roger Quadros wrote: > >> Hi, > >> > >> On 16/05/16 10:02, Peter Chen wrote: > >>> On Fri, May 13, 2016 at 01:03:27PM +0300, Roger Quadros wrote: > >>>> + > >>>> +static int usb_gadget_connect_control(struct usb_gadget *gadget, > >>>> +bool connect) { > >>>> + struct usb_udc *udc; > >>>> + > >>>> + mutex_lock(&udc_lock); > >>>> + udc = usb_gadget_to_udc(gadget); > >>>> + if (!udc) { > >>>> + dev_err(gadget->dev.parent, "%s: gadget not > registered.\n", > >>>> + __func__); > >>>> + mutex_unlock(&udc_lock); > >>>> + return -EINVAL; > >>>> + } > >>>> + > >>>> + if (connect) { > >>>> + if (!gadget->connected) > >>>> + usb_gadget_connect(udc->gadget); > >>>> + } else { > >>>> + if (gadget->connected) { > >>>> + usb_gadget_disconnect(udc->gadget); > >>>> + udc->driver->disconnect(udc->gadget); > >>>> + } > >>>> + } > >>>> + > >>>> + mutex_unlock(&udc_lock); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>> > >>> Since this is called for vbus interrupt, why not using > >>> usb_udc_vbus_handler directly, and call udc->driver->disconnect at > >>> usb_gadget_stop. > >> > >> We can't assume that this is always called for vbus interrupt so I > >> decided not to call usb_udc_vbus_handler. > >> > >> udc->vbus is really pointless for us. We keep vbus states in our > >> state machine and leave udc->vbus as ture always. > >> > >> Why do you want to move udc->driver->disconnect() to stop? > >> If USB controller disconnected from bus then the gadget driver must > >> be notified about the disconnect immediately. The controller may or > >> may not be stopped by the core. > >> > > > > Then, would you give some comments when this API will be used? > > I was assumed it is only used for drd state machine. > > drd_state machine didn't even need this API in the first place :). > You guys wanted me to separate out start/stop and connect/disconnect for > full OTG case. > Won't full OTG state machine want to use this API? If not what would it > use? Instead create those new interfaces/symbol here and there just aim to address build problems in diff configures, Could we only allow meaningful combination of those 3 drivers configures? Hcd=y, gadget=y, otg=y or Hcd=m, gadget=m, otg=m Li Jun > > cheers, > -roger > > > > >>> > >>>> return 0; > >>>> @@ -660,9 +830,15 @@ static ssize_t usb_udc_softconn_store(struct > device *dev, > >>>> return -EOPNOTSUPP; > >>>> } > >>>> > >>>> + /* In OTG mode we don't support softconnect, but b_bus_req */ > >>>> + if (udc->gadget->otg_dev) { > >>>> + dev_err(dev, "soft-connect not supported in OTG mode\n"); > >>>> + return -EOPNOTSUPP; > >>>> + } > >>>> + > >>> > >>> The soft-connect can be supported at dual-role mode currently, we > >>> can use b_bus_req entry once it is implemented later. > >> > >> Soft-connect should be done via sysfs handling within the OTG core. > >> This can be added later. I don't want anything outside the OTG core > >> to handle soft-connect behaviour as it will be hard to keep things in > >> sync. > >> > >> I can update the comment to something like this. > >> > >> /* In OTG/dual-role mode, soft-connect should be handled by OTG core > >> */ > > > > Ok, let's Felipe decide it. > > > >> > >>> > >>>> if (sysfs_streq(buf, "connect")) { > >>>> usb_gadget_udc_start(udc); > >>>> - usb_gadget_connect(udc->gadget); > >>>> + usb_udc_connect_control(udc); > >>> > >>> This line seems to be not related with this patch. > >>> > >> Right. I'll remove it. > >> > >> cheers, > >> -roger > >