On Mon, Jun 17, 2013 at 9:59 AM, Peter Chen <hzpeterchen@xxxxxxxxx> wrote: > On Thu, Mar 21, 2013 at 11:06 AM, Peter Chen <peter.chen@xxxxxxxxxxxxx> wrote: >> 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. >> > > Alex, I would like to post dual-role/vbus patch recently. Do you have > further suggestion for how to access otgsc? If not, I will > use the way I showed above. > ping.... Alex, any comments? Peter -- 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