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 for Android: free password hash cracker in your pocket

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

Message-ID: <5710C9DE.2050106@ti.com>
Date:	Fri, 15 Apr 2016 14:00:46 +0300
From:	Roger Quadros <rogerq@...com>
To:	Peter Chen <hzpeterchen@...il.com>
CC:	<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 15/04/16 12:25, Peter Chen wrote:
> On Tue, Apr 05, 2016 at 05:05:12PM +0300, Roger Quadros wrote:
>> + * usb_otg_register() - Register the OTG/dual-role device to OTG core
>> + * @dev: OTG/dual-role controller device.
>> + * @config: OTG configuration.
>> + *
>> + * Registers the OTG/dual-role controller device with the USB OTG core.
>> + *
>> + * Return: struct usb_otg * if success, ERR_PTR() if error.
>> + */
>> +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");
> 
> What does above mean? Customized otg_work item may be dual-role,
> may be full otg.

I'm checking for !config->otg_work so we're sure of using the
default dual-role only work function.

> 
>> +
>> +	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;
>> +	}
> 
> I found a bug that caused by non-freezable workqueue, change it
> as freezable please.
> 
> https://marc.ttias.be/linux-stable/2016-03/msg00723.php

OK.

> 
> I will review the whole set from next week.

Thanks.

--
cheers,
-roger