RE: [PATCH v8 15/20] usb: chipidea: add host role

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

 



Chen Peter-B29397 <B29397@xxxxxxxxxxxxx> writes:

>> -----Original Message-----
>> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-
>> owner@xxxxxxxxxxxxxxx] On Behalf Of Alexander Shishkin
>> Sent: Friday, May 11, 2012 8:06 AM
>> To: Greg Kroah-Hartman
>> Cc: linux-usb@xxxxxxxxxxxxxxx; Felipe Balbi; Alexander Shishkin; Alan
>> Stern
>> Subject: [PATCH v8 15/20] usb: chipidea: add host role
>> 
>> This adds EHCI host support to the chipidea driver. We want it to be
>> part of the hdrc driver and not a standalone (sub-)driver module, as
>> the structure of ehci-hcd.c suggests, so for chipidea controller we
>> hack it to not provide platform-related code, but only the ehci hcd.
>> 
>> The ehci-platform driver won't work for us here too, because the
>> controller uses the same registers for both device and host mode and
>> also otg-related bits, so it's not really possible to put ehci registers
>> into a separate resource.
>> 
>> This is not a pretty solution, but the alternative is exporting symbols
>> from the chipidea driver to a ehci-chipidea driver and doing all the
>> module refcounting.
>> 
> Hi Alexander,

Hi,

> First, it is really great work, and I hope we can all switch to it 
> in future.

Thanks for your interest and sorry for not replying earlier. First, let
me point out that my goal with this patchset was to lay the groundwork
so that we could make it into something that's usable for everybody. I
didn't try to implement every single corner case, because it would have
turned into a much longer and even less maintainable patchset.

> I can't find summery for this patchset, so I have added some
> of my questions here, some of them I asked yesterday, but does
> not get answers. Some of Freescale i.MX users would like to use
> your structure, so I go through your code, and would like to 
> know your comments for below questions:
>
> - How to support OTG, you will give up struct usb_otg or not?

I'm not sure what you mean by giving up usb_otg. What we were discussing
with Heikki and Felipe is reworking the otg framework so that, for
example, we have a common otg state machine instead of reimplementing it
in every otg driver. We planned for this work to be based upon this
chipidea driver. Maybe this topic should be brought up again.

In any case, my plan is that the otg part of ci_hdrc should be based on
those otg framework changes, which should come first. Heikki, do you
have anything to add?

> - When ID switch occurs, do udc_start/udc_stop is enough, seems no registers
> operation?

The ID switch will cause the stop() for current role and the start() for
the new role, which at the very least resets the device and sets
USBMODE_CM_{D,H}C. Also, when the gadget is stopped, UDC is
deinitialized and queue heads are flushed. Does this answer your
question?

> - I have not found vbus interrupt code which is used to stand for
> connect and disconnect at device mode.

It's not there yet. I think it should be in the otg part of the
driver. What do you think?

> - Seems hcd will register interrupt 0 irq handler, if the system
> has really interrupt 0 irq handler, it may mess up.

The hcd core regards 0 irq line as "no irq" and doesn't set its own
interrupt handler. But, since chipidea's core.c already has an irq
handler on this line, I decided to reuse that and call usb_hcd_irq() to
deliver interrupts to the ehci driver. Core irq handler will be
installed on whatever line number is provided by the platform code as
long as it's not negative (in which case the whole initialization will
fail). So theoretically, it will work even if chipidea's irq is 0 and
irq 0 is valid.

> - Current, there is no PHY Layer code (I will begin to do it soon),
> so your PHY operation is at udc code, once the PHY Layer code is ready,
> do you will move it to core.c?

I think of phy code as platform code, it doesn't directly concern the
chipidea driver, except for the bits where chipidea might need explicit
configuration of the phy type. But I think that is still separate from
the actual phy code. My understanding is, usb controllers should be
really decoupled from phy code and shoud only talk to one another
through certain api. Again, Heikki might have more to say on this.

> - What's your plan for low power mode and wakeup support?

It needs to be implemented. As many other things on the original
driver's todo list.

> - For host driver, what's plan to add controller callback (override 
> struct hc_driver) for vendor special, like you have done for udc.

I haven't really given it much thought yet and I don't have the whole
picture in my mind. I will return to it shortly, when I have gone
through the other thread. But certainly, we need to provide for enough
flexibility for platforms to work.

> Seems you have moved msm udc to chipidea folder, but have not changed
> host and otg part, how msm user goes on using usb driver?

Well, both ehci-msm.c and msm-otg.c are separate drivers, and nothing
has changed from their prospective. With my patchset, they should still
work as they did before it.

I hope I did answer at least some of your questions.

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


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

  Powered by Linux