Hi,
On Fri, Apr 05, 2013 at 02:40:45PM +0200, Daniel Mack wrote:
> >> Hi all,
> >>
> >> here are some patches to separate the HCD and gadget part of the musb
> >> driver so they can be deselected in Kconfig. They also make the driver
> >> keep track of the configured port mode that is set from DT, so the
> >> actual runtime configuration can be selected dynamically.
> >>
> >> One thing that is still broken is that once pm_suspend() was called on
> >> a musb device on a USB disconnect, the port won't wake up again when a
> >> device is plugged back in. I doubt this is related to my patches, but I
> >> might be wrong. If that effect rings a bell to anyone, please let me
> >> know.
> >>
> >>
> >> Thanks,
> >> Daniel
> >>
> >>
> >> Daniel Mack (7):
> >> usb: gadget: drop unused USB_GADGET_MUSB_HDRC
> >> usb: musb: move function declarations to musb_{host,gadget}.h
> >> usb: musb: factor out hcd initalization
> >> usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes
> >> usb: musb: add musb_host_setup() and musb_host_cleanup()
> >> usb: musb: re-introduce musb->port_mode
> >> usb: musb: eliminate musb_to_hcd
> >
> > pretty good, getting there. Here's what I'd to see though:
> >
> > introduce musb_gadget_init() and musb_host_init(). Those two should
> > contain everything necessary to initialize each side of the API.
> >
> > Then we can simply:
> >
> > switch(mode) {
> > case HOST:
> > musb_host_init();
> > break;
> > case PERIPHERAL:
> > musb_gadget_init();
> > break;
> > case OTG:
> > /* it's important to initialize gadget side first */
> > musb_gadget_init();
> > musb_host_init();
> > break;
> > }
>
> Sorry, I read over the code again and I want to be sure I understand
> what you mean here. The _init() functions you mention are called
> _setup() in my case, because there was already a musb_gadget_setup(), so
> I called the host side accordingly.
>
> Including the check of the return value, I ended up with these lines:
>
> if (musb->port_mode == MUSB_PORT_MODE_HOST ||
> musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) {
> status = musb_host_setup(musb, plat->power);
> if (status < 0)
> goto fail3;
> }
>
> if (musb->port_mode == MUSB_PORT_MODE_GADGET ||
> musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) {
> status = musb_gadget_setup(musb);
> if (status < 0)
> goto fail3;
> }
>
> ... and that's part of the result of my patch set. What exactly do you
> want me to change? Is it that musb_gadget_setup() is be called before
> musb_host_setup()? Or is it about their names?
>
> > code will look cleaner and initialization will be straight forward and a
> > lot easier to understand.
>
> Compared to what exactly?
>
> > This means that the call to usb_add_hcd() from musb_gadget_start()
> > should be removed.
>
> As stated in a previous mail, this is already done.
I replied to each patch what needs to be changed.
> Any idea about the host port not waking up after the device was
> unplugged once?
Bad PM ?
--
balbi
Attachment:
signature.asc
Description: Digital signature
