About the DRD mode implementation of DWC3 driver]

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

 



Hi Balbi,

At first, thank you to help give the response in patience.

> Hi,
>
> On Wed, Apr 09, 2014 at 02:08:32PM +0800, Wang, Yu wrote:
> > Glad to see the OTG mode is prepare to support in your
> > dwc3-role-switch branch. But it is not fit for intel
> > Merrfield/Moorfield platforms. :(
>
> that's not what I heard when I asked some friends to test that branch
> on different platforms. It's certainly not perfect yet and that's why
> the code isn't merged in mainline, but claiming that it's not fit for
> Merrifield/Moorfield is just a lie.
>

Can I get your friends' email address if they are in Intel? I would like to
contact them to make the branch work on my Merrifield/Moorefield board too.
Currently I can't see any link change event with the branch.

> > The reason is we implemented DRD mode instead of OTG mode. So the
> > GCTL.PortCapDir will be set as 01 for host mode, and 10 for device mode.
>
> from a SW perspective there is *no* difference. The only reason for
> using PortCapDir is to cope with platforms which want to switch roles
> but have screwed up ID pin reporting. And since most of the platforms
> actually have tha problem, it's just easier to implement it that way.
>

Ok. Just confirm with you that you think it's not a matter to use
GCTL.PortCapDir or OCTL.PeriMode to switch role, right?

> > And the most important thing is we implemented two low power states
> > for
> > dwc3 controller. D0i3hot and D0i3cold.
>
> what does that have to do with role switching ? Power management and
> role switching are two completely different problems. You can have
> full DRD without PM the same way you can have full PM without DRD.
>

Agree. They can be discussed separately.

> > If no cable connected, we will trigger D0i3cold which will power
> > gate every clock/power used by dwc3 controller.
>
> that's not in mainline though, why should I care ? Show me code adding
> support for D0i3cold for dwc3 then we can talk. Until then, I'll
> continue assuming there's no such PM state and continue with happy hacking
> sessions.
>
> > And entering D0i3hot with hibernation mode when acting as host mode
> > (micro A cable connected), also during device mode(micro B cable
> > connected to PC host).
>
> Current driver also doesn't support any runtime PM, so why should I
> care about your out-of-tree changes ? If you have any code which is
> not in mainline and I change something in mainline you have to deal to with it,
> not me.

Get it. So we can treat this solution is working w/o PM implementation
so far. And maybe we need to re-design DRD/OTG when we consider to
support PM, right?

>
> I just cannot spend time imagining all twisted scenarios Vendors
> (including my employer, for that matter) want to support with whatever
> hacked up version of mainline drivers they might have.
>
> My plan is to *not* add a lot of PM support until I know all other
> features are functional. When I'm happy with those features and know
> they have been thoroughly tested on *all* users of dwc3 then we can
> all get together in a conference (maybe have a DWC3 mini-summit or
> whatever) and discuss how we're gonna implement PM *together*.
>
> Since the driver is used by many different vendors, it would be unfair
> for me to treat Intel (or any vendor, really) differently just because
> someone dropped me an email commenting about all the out-of-tree changes
> they have.
>

Understand. What are the other features you mean that need thorough testing
before adding PM support?

> > For ID/VBus detection, we are using PMIC to do detect. So we will
> > also power gate the USB PHY for D0i3cold.
>
> yeah, everybody does that. Everybody I've seen so far have moved the
> analog comparators for VBUS and ID out of the PHY and into the PMIC.
>
> We even have a very nice framework to cope with that (see
> drivers/extcon/extcon-palmas.c for an example).
>

Thanks for pointing it out. We will use it too.

> > When we plug in micro A/B cable, the PMIC will report the ID/VBus
> > change event, then driver will force controller resume to D0 from
> > D0i3cold. Due to we haven't do any backup before entering D0i3cold,
> > so we have to re-initialize all host/device portion registers with
> > setting GCTL.PortCapDir to 01 or 10.
>
> yeah, but this is just how *you* decided to implement this for your
> employer and, guess what, all of that is out-of-tree and, by all
> practical means wrt mainline kernel, it's all non-existent. This means
> I'm free to implement all of this any way I feel it's best considering the diverse
> user-base we have on this driver.
>
> Quite frankly, resetting the IP is only necessary iff all power rails
> are cut, in that case, sure, we will reset the IP and start all over,
> but we don't want to cut all power while we're attached to host, which
> means we don't need to reset the IP, a simple context restore is enough in
> most cases.
>
> Also, when we implement hibernation (dwc3's hibernation, that is) we
> won't need to reset the IP even if we go all the way down to D0i3cold
> because the IP keeps all context in a scratch buffer we allocate during probe.

Yes. I know what is your mean. Just I don't know how to implement
hibernation mode for DRD mode. The synopsys DWC3 spec haven't mentioned
it.

And also for OTG hibernation mode. If we power gate the USB PHY during
OTG hibernation mode, and using PMIC to do ID/VBUS detection. I don't
know if there have any issues.

>
> > So with this PM design and DRD mode, we can't use your OTG mode. :(
>
> that's not my problem though, is it ? Since *your* PM design isn't
> available in mainline anyway. Not to mention that *my* DRD mode wasn't
> merged either, even though I have good reports from some friends
> stating it's working in 90% of the cases.
>

Maybe I should add more explanation on the purpose of this email. I was not
complaining you on your codes. Just wanted to discuss with you on our solution
in order to improve our codes acceptable in community.

> > We add DRD support based on your otg.c or create new file drd.c?
>
> Well, since I've already been working on it (well, George Cherian -
> now in Cc - was the originator) I rather continue on it together with
> George. Thank you
>

Do you have a first version that I can take a look? Since I'm working on it too, I
would like to avoid conflict at the beginning. Thanks.

Yu

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