Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752321AbcDZG2Y (ORCPT ); Tue, 26 Apr 2016 02:28:24 -0400 Received: from mail-pa0-f68.google.com ([209.85.220.68]:34784 "EHLO mail-pa0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751146AbcDZG2W (ORCPT ); Tue, 26 Apr 2016 02:28:22 -0400 Date: Tue, 26 Apr 2016 14:28:09 +0800 From: Peter Chen To: Jun Li Cc: Roger Quadros , "stern@rowland.harvard.edu" , "balbi@kernel.org" , "gregkh@linuxfoundation.org" , "peter.chen@freescale.com" , "dan.j.williams@intel.com" , "jun.li@freescale.com" , "mathias.nyman@linux.intel.com" , "tony@atomide.com" , "Joao.Pinto@synopsys.com" , "abrestic@chromium.org" , "r.baldyga@samsung.com" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-omap@vger.kernel.org" Subject: Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Message-ID: <20160426062809.GD2978@peterchendt> References: <1459865117-7032-1-git-send-email-rogerq@ti.com> <1459865117-7032-8-git-send-email-rogerq@ti.com> <20160426034711.GC2978@peterchendt> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2520 Lines: 67 On Tue, Apr 26, 2016 at 05:11:36AM +0000, Jun Li wrote: > Hi > > > -----Original Message----- > > From: Peter Chen [mailto:hzpeterchen@gmail.com] > > Sent: Tuesday, April 26, 2016 11:47 AM > > To: Jun Li > > Cc: Roger Quadros ; stern@rowland.harvard.edu; > > balbi@kernel.org; gregkh@linuxfoundation.org; peter.chen@freescale.com; > > dan.j.williams@intel.com; jun.li@freescale.com; > > mathias.nyman@linux.intel.com; tony@atomide.com; Joao.Pinto@synopsys.com; > > abrestic@chromium.org; r.baldyga@samsung.com; linux-usb@vger.kernel.org; > > linux-kernel@vger.kernel.org; linux-omap@vger.kernel.org > > Subject: Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core > > > > On Tue, Apr 26, 2016 at 02:07:56AM +0000, Jun Li wrote: > > > > +struct usb_otg *usb_otg_register(struct device *dev, > > > > + struct usb_otg_config *config) { > > > > + struct usb_otg *otg; > > > > + struct otg_wait_data *wait; > > > > + int ret = 0; > > > > + > > > > + if (!dev || !config || !config->fsm_ops) > > > > + return ERR_PTR(-EINVAL); > > > > + > > > > + /* already in list? */ > > > > + mutex_lock(&otg_list_mutex); > > > > + if (usb_otg_get_data(dev)) { > > > > + dev_err(dev, "otg: %s: device already in otg list\n", > > > > + __func__); > > > > + ret = -EINVAL; > > > > + goto unlock; > > > > + } > > > > + > > > > + /* allocate and add to list */ > > > > + otg = kzalloc(sizeof(*otg), GFP_KERNEL); > > > > + if (!otg) { > > > > + ret = -ENOMEM; > > > > + goto unlock; > > > > + } > > > > + > > > > + otg->dev = dev; > > > > + otg->caps = config->otg_caps; > > > > + > > > > + if ((otg->caps->hnp_support || otg->caps->srp_support || > > > > + otg->caps->adp_support) && !config->otg_work) > > > > + dev_info(dev, "otg: limiting to dual-role\n"); > > > > > > dev_err, this should be an error. > > > > The condition may be wrong, but it is an information to show that current > > OTG is dual-role. > > This should not happen in any correct design, I even doubt if we > should try to continue by "downgrade" it to be duel role, currently > the only example user is dual role, so doing like this can't be > tested by real case, this downgrade is not so easy like we image, > at least for chipidea otg driver, simply replace a queue worker may > not work, as we have much more difference between the 2 configs. > Would you show more why chipidea can't work just replace the work item, and see if anything we still can improve for this framework? -- Best Regards, Peter Chen