linux-kernel - RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM4PR04MB2130124412E09799A2CB01BF89630@AM4PR04MB2130.eurprd04.prod.outlook.com>
Date:	Tue, 26 Apr 2016 05:11:36 +0000
From:	Jun Li <jun.li@....com>
To:	Peter Chen <hzpeterchen@...il.com>
CC:	Roger Quadros <rogerq@...com>,
	"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
	"balbi@...nel.org" <balbi@...nel.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"peter.chen@...escale.com" <peter.chen@...escale.com>,
	"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"jun.li@...escale.com" <jun.li@...escale.com>,
	"mathias.nyman@...ux.intel.com" <mathias.nyman@...ux.intel.com>,
	"tony@...mide.com" <tony@...mide.com>,
	"Joao.Pinto@...opsys.com" <Joao.Pinto@...opsys.com>,
	"abrestic@...omium.org" <abrestic@...omium.org>,
	"r.baldyga@...sung.com" <r.baldyga@...sung.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>
Subject: RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core

Hi

> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen@...il.com]
> Sent: Tuesday, April 26, 2016 11:47 AM
> To: Jun Li <jun.li@....com>
> Cc: Roger Quadros <rogerq@...com>; stern@...land.harvard.edu;
> balbi@...nel.org; gregkh@...uxfoundation.org; peter.chen@...escale.com;
> dan.j.williams@...el.com; jun.li@...escale.com;
> mathias.nyman@...ux.intel.com; tony@...mide.com; Joao.Pinto@...opsys.com;
> abrestic@...omium.org; r.baldyga@...sung.com; linux-usb@...r.kernel.org;
> linux-kernel@...r.kernel.org; linux-omap@...r.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.

Li Jun
 
> 
> Peter
> >
> > > +
> > > +	if (config->otg_work)	/* custom otg_work ? */
> > > +		INIT_WORK(&otg->work, config->otg_work);
> > > +	else
> > > +		INIT_WORK(&otg->work, usb_otg_work);
> > > +
> > > +	otg->wq = create_singlethread_workqueue("usb_otg");
> > > +	if (!otg->wq) {
> > > +		dev_err(dev, "otg: %s: can't create workqueue\n",
> > > +			__func__);
> > > +		ret = -ENOMEM;
> > > +		goto err_wq;
> > > +	}
> > > +
> > > +	/* set otg ops */
> > > +	otg->fsm.ops = config->fsm_ops;
> > > +
> > > +	mutex_init(&otg->fsm.lock);
> > > +
> > > +	list_add_tail(&otg->list, &otg_list);
> > > +	mutex_unlock(&otg_list_mutex);
> > > +
> > > +	/* were we in wait list? */
> > > +	mutex_lock(&wait_list_mutex);
> > > +	wait = usb_otg_get_wait(dev);
> > > +	mutex_unlock(&wait_list_mutex);
> > > +	if (wait) {
> > > +		/* register pending host/gadget and flush from list */
> > > +		usb_otg_flush_wait(dev);
> > > +	}
> > > +
> > > +	return otg;
> > > +
> > > +err_wq:
> > > +	kfree(otg);
> > > +unlock:
> > > +	mutex_unlock(&otg_list_mutex);
> > > +	return ERR_PTR(ret);
> > > +}
> > > +EXPORT_SYMBOL_GPL(usb_otg_register);
> > > +
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb"
> > in the body of a message to majordomo@...r.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Best Regards,
> Peter Chen