Re: Two remain problems at chipidea driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 20, 2013 at 01:04:33PM +0200, Alexander Shishkin wrote:
> Peter Chen <peter.chen@xxxxxxxxxxxxx> writes:
> 
> > On Fri, Mar 15, 2013 at 05:17:08PM +0200, Alexander Shishkin wrote:
> >> 
> >> > Eg, for tablet or phone, the dr_mode may be "gadget", but the
> >> > otg_capable = 1.
> >> 
> >> No, because dr_mode indicates controller's capability, and not the
> >> "current" mode of operation. Why would anyone want to put *that* in a
> >> DT?
> >> 
> >
> > OK, now I totally understand your mind of this problem. In fact, dr_mode
> > is NOT controller's capability, even at its original place:
> > (Documentation/devicetree/bindings/usb/fsl-usb.txt or nvidia, tegra20-ehci.txt)
> > dr_mode is the usb working mode.
> >
> > When we design USB system, the requirements are differ from products
> > to products. 
> > The phone/tablet device may only wants itself as gadget
> > device, it needs to be no-response when there is a usb device plug in
> > (eg usb keyboard with Micro B-to-A cable). 
> >
> > The car entertainment system or other Standard-A port system do not want
> > to be enumerated when it plugs to notebook using Standard A-to-A cable.
> 
> Bah. Of course, you're right. We're stuck with dr_mode till people learn
> to design middleware stacks that can handle being both host and
> peripheral.
> 
> > So, currently, even most of controllers are otg-capable, still most
> > of designs are one working mode designed. The reason why we design 
> > the dr_mode is that we want controller working mode to be decided 
> > by DT without re-compile the kernel by build out the host/gadget driver.
> 
> Ok, so then how about introducing *one* more parameter, something like
> "dr_cap", which
> 1) when specified, supersedes DCCPARAMS, so no need to read that
> register any more;
> 2) when unspecified, use DCCPARAMS;
> 3) can be one of "host", "peripheral", "otg", "dual_role":
>    - host, peripheral: initialize one role only, stick to that, no otg;
>    - dual_role: initialize both roles, no otg;
>    - otg: both roles, ci->is_otg == true.

> 
> Another question now is, do we need "dual_role" variant for the dr_mode
> parameter?
> 

"dr_cap" is a good idea to indicate controller's capability, but when
it combines with "dr_mode", things will be more complicated.
Eg, how we initialize roles, depends on dr_cap or dr_mode? Besides, we
need to some judgements if dr_mode and dr_cap are conflict or not.
Since we already have DCCPARAMS check at each role's init, I suggest
we only need "otg_cap", it only indicates if otg is capable or not.
We can do things like below:

dr_mode: "host", "peripheral", "otg"
otg_cap; false, true

1) For dr_mode usage, it is used like sascha's  patch.
2) Then we decide ci->is_otg:
	if (otg_cap is existed)
		ci->is_otg = otg_cap;
	else
		read DDCPARMAS;
		if (both DC and HC are 1) 
			ci->is_otg = 1;
		else 
			ci->is_otg = 0;

3) if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET] && ci->is_otg)
		do_otg_init; /* Eg, enable ID interrupt at otgsc */
4) At ci_irq:
	if (ci->is_otg)
		otgsc = hw_read(ci, OP_OTGSC, ~0);

	/* Since id is only enabled at both roles are enabled, 
	 * if dr_mode = peripheral and ci_is_otg = true, code will
	 * not handle id change.
	 */
	if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
		handle id interrupt;
	}

	/*
	 * Handle vbus change interrupt, it indicates device connection
	 * and disconnection events.
	 */
	if (ci->is_otg && (otgsc & OTGSC_BSVIE)
			&& (otgsc & OTGSC_BSVIS)) {
		handle vbus interrupt;
	}

Besides, at my patch, I always build otg.c, I don't think
we need to give otg.c a config, this just like we don't
need to have a config for otg capable. I will
move all otgsc access to under the condition of ci->is_otg.

-- 

Best Regards,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux