linux-kernel - RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG 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: <AM4PR04MB2130926F9AE8EBFC0D19447689490@AM4PR04MB2130.eurprd04.prod.outlook.com>
Date:	Wed, 18 May 2016 14:46:50 +0000
From:	Jun Li <jun.li@....com>
To:	Roger Quadros <rogerq@...com>, Peter Chen <hzpeterchen@...il.com>
CC:	"peter.chen@...escale.com" <peter.chen@...escale.com>,
	"balbi@...nel.org" <balbi@...nel.org>,
	"tony@...mide.com" <tony@...mide.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"mathias.nyman@...ux.intel.com" <mathias.nyman@...ux.intel.com>,
	"Joao.Pinto@...opsys.com" <Joao.Pinto@...opsys.com>,
	"sergei.shtylyov@...entembedded.com" 
	<sergei.shtylyov@...entembedded.com>,
	"jun.li@...escale.com" <jun.li@...escale.com>,
	"grygorii.strashko@...com" <grygorii.strashko@...com>,
	"yoshihiro.shimoda.uh@...esas.com" <yoshihiro.shimoda.uh@...esas.com>,
	"robh@...nel.org" <robh@...nel.org>,
	"nsekhar@...com" <nsekhar@...com>, "b-liu@...com" <b-liu@...com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core



> >>
> >> 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
> >>>>>>>