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]

Date:	Wed, 20 Apr 2016 17:26:50 +0800
From:	Peter Chen <hzpeterchen@...il.com>
To:	Roger Quadros <rogerq@...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 Wed, Apr 20, 2016 at 09:54:49AM +0300, Roger Quadros wrote:
> On 18/04/16 05:09, Peter Chen wrote:
> > On Fri, Apr 15, 2016 at 02:00:46PM +0300, Roger Quadros wrote:
> >> 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.
> >>
> > 
> > I see. But whether it is dual-role or fully otg should depend on
> > otg->caps, the work item may be different according to design.
> > Besides, your code seems to depend on one of the otg capabilities
> > for dual-role.
> > 
> Which capability? Id/vbus status?
> 

if (otg->caps->hnp_support || otg->caps->srp_support || otg->caps->adp_support)
	it is fully otg;
else
	it is dual role;

Do you think so?

-- 

Best Regards,
Peter Chen