From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932536AbcEROq6 (ORCPT ); Wed, 18 May 2016 10:46:58 -0400 Received: from mail-am1on0065.outbound.protection.outlook.com ([157.56.112.65]:20448 "EHLO emea01-am1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932390AbcEROqy convert rfc822-to-8bit (ORCPT ); Wed, 18 May 2016 10:46:54 -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/eAgAFqGKCAAAt5AIAAAifQgAHcsYCAAAOXcIAADYOAgAAE45A= Date: Wed, 18 May 2016 14:46:50 +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> <573AD198.6030307@ti.com> <573C6347.6080705@ti.com> <573C719F.5010107@ti.com> In-Reply-To: <573C719F.5010107@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: [192.158.241.86] x-ms-office365-filtering-correlation-id: 3a57cfc1-4b81-4b5e-1f9e-08d37f2b3ecc x-microsoft-exchange-diagnostics: 1;AM4PR04MB2129;5:/PXEE3ej5maSK9TfTG/9eMe7fCKOWaKj2Z9urupzf9ZyjT4LO9mNsP+bkqXZCZ69qfxWMwmT5Bf5212Qe0yRD9XsBgeFHBFipblGgbxgYq4OEox8PSQUkiE/513zcbhd4xAFxtM5ejolr3b4f8nHIA==;24:+fAB+wdmAI4urv4zPjjIRy8dFJ2Z3USN/hoLaeXav/gfSrS8Ff1CKkgGXwqPJBhW11leRKaaEd1ta/kEj7kzGVv1ziw937o5iYLVhqxCjOk=;7:doHNqBZCjnftJqGQit0SJbDj61cW3iLOprGtiXNyExVSq8wYulBFDCKym7KoxknrbssHMqMjqSCvkGstS11+yieANrncx4E360HVHgyWpvBpQ3zaCe3XbH6zihN5liCzc+znzTJJ3AJ446tSM4tcXFnd2p4aDeKtnqqq1v9Cs4eVZj6MsKLhLhF8CQSGyEYK x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2129; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:AM4PR04MB2129;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2129; x-forefront-prvs: 0946DC87A1 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(5003600100002)(81166006)(189998001)(50986999)(102836003)(5008740100001)(93886004)(586003)(6116002)(33656002)(10400500002)(8936002)(5001770100001)(1220700001)(5002640100001)(86362001)(74316001)(5004730100002)(122556002)(106116001)(8666002)(8676002)(11100500001)(66066001)(9686002)(4326007)(76176999)(2900100001)(54356999)(2950100001)(3280700002)(2906002)(3660700001)(77096005)(87936001)(76576001)(92566002)(7059030);DIR:OUT;SFP:1101;SCL:1;SRVR:AM4PR04MB2129;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: 18 May 2016 14:46:50.2267 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR04MB2129 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > >> > >> I didn't want to have complex Kconfig so decided to have otg as > >> built-in only. > >> What do you want me to change in existing code? and why? > > > > Remove those stuff which only for pass diff driver config Like every > > controller driver need a duplicated > > > > static struct otg_hcd_ops ci_hcd_ops = { > > ... > > } > > This is an exception only. Every controller driver doesn't need to > implement hcd_ops. It is implemented in the hcd core. > > > > > And here is another example, for gadget connect, otg driver can > > directly call to usb_udc_vbus_handler() in drd state machine, but you > > create another interface: > > > > .connect_control = usb_gadget_connect_control, > > > > If the symbol is defined in one driver which is 'm', another driver > > reference it should be 'm' as well, then there is no this kind of > > problem as my understanding. > > That is fine as long as all are 'm'. but how do you solve the case when > Gadget is built in and host is 'm'? OTG has to be built-in and you will > need an hcd to gadget interface. Hcd to gadget interface? Or you want to say otg to host interface? I think hcd and gadget are independent each other, now Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol) If you directly call to usb_udc_vbus_handler() in drd state machine Then: Hcd --> otg; and gadget <--> otg, (gadget and otg will refer to symbol of each other) Li Jun > > Do you have any ideas to solve that case? > > 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: Wed, 18 May 2016 14:46:50 +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> <573AD198.6030307@ti.com> <573C6347.6080705@ti.com> <573C719F.5010107@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <573C719F.5010107@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 > >> > >> I didn't want to have complex Kconfig so decided to have otg as > >> built-in only. > >> What do you want me to change in existing code? and why? > > > > Remove those stuff which only for pass diff driver config Like every > > controller driver need a duplicated > > > > static struct otg_hcd_ops ci_hcd_ops = { > > ... > > } > > This is an exception only. Every controller driver doesn't need to > implement hcd_ops. It is implemented in the hcd core. > > > > > And here is another example, for gadget connect, otg driver can > > directly call to usb_udc_vbus_handler() in drd state machine, but you > > create another interface: > > > > .connect_control = usb_gadget_connect_control, > > > > If the symbol is defined in one driver which is 'm', another driver > > reference it should be 'm' as well, then there is no this kind of > > problem as my understanding. > > That is fine as long as all are 'm'. but how do you solve the case when > Gadget is built in and host is 'm'? OTG has to be built-in and you will > need an hcd to gadget interface. Hcd to gadget interface? Or you want to say otg to host interface? I think hcd and gadget are independent each other, now Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol) If you directly call to usb_udc_vbus_handler() in drd state machine Then: Hcd --> otg; and gadget <--> otg, (gadget and otg will refer to symbol of each other) Li Jun > > Do you have any ideas to solve that case? > > 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 > >>>>>>>