Peter Chen <peter.chen@xxxxxxxxxxxxx> writes:
> 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.
I'm thinking now that what Felipe suggests is probably the best at least
for the time being. That is, if we don't want host, for example, in the
product build, we just compile it out. And then we expect the dr_mode to
be "otg" if we want to be able to access otgsc, but no role changes will
happen, since only peripheral is compiled in.
Regards,
--
Alex
--
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
