Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

Bin Liu
2016-05-15T04:42:12Z

Hi,

On Fri, May 13, 2016 at 03:25:17PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160513 15:04]:
> >
> > But what would be in musb_default_set_mode()? Currently only am35x,
> > da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
> > have any in common. Only omap2430 sets session bit in _set_mode(), no
> > one else does so.
>
> Well how about the following if no glue specific configuration of the
> ID pin is possible?

ID pin configuration is required, either in sw or hw, for the controller
otg state transition.

In my understanding, if ID pin is grounded, setting session bit make the
controller transition to host mode; or if ID pin is float, setting
session bit triggers srp request.

I think I don't understand what you are trying to solve by adding
default set_mode() in core.

Regards,
-Bin.

>
> Tony
>
> 8< -----------------------
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -416,6 +416,26 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src)
> return hw_ep->musb->io.write_fifo(hw_ep, len, src);
> }
>
> +static int musb_try_set_mode(struct musb *musb, u8 mode)
> +{
> + if (musb->ops->set_mode)
> + return musb->ops->set_mode(musb, mode);
> +
> + if (mode == MUSB_HOST) {
> + u8 devctl;
> +
> + devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> + devctl |= MUSB_DEVCTL_SESSION;
> + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> + } else {
> + dev_warn(musb->controller,
> + "platform specific set_mode not implemnted: %i\n",
> + mode);
> + }
> +
> + return 0;
> +}
> +
> /*-------------------------------------------------------------------------*/
>
> /* for high speed test mode; see USB 2.0 spec 7.1.20 */
> @@ -1723,11 +1743,11 @@ musb_mode_store(struct device *dev, struct device_attribute *attr,
>
> spin_lock_irqsave(&musb->lock, flags);
> if (sysfs_streq(buf, "host"))
> - status = musb_platform_set_mode(musb, MUSB_HOST);
> + status = musb_try_set_mode(musb, MUSB_HOST);
> else if (sysfs_streq(buf, "peripheral"))
> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> + status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
> else if (sysfs_streq(buf, "otg"))
> - status = musb_platform_set_mode(musb, MUSB_OTG);
> + status = musb_try_set_mode(musb, MUSB_OTG);
> else
> status = -EINVAL;
> spin_unlock_irqrestore(&musb->lock, flags);
> @@ -2185,13 +2205,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> status = musb_host_setup(musb, plat->power);
> if (status < 0)
> goto fail3;
> - status = musb_platform_set_mode(musb, MUSB_HOST);
> + status = musb_try_set_mode(musb, MUSB_HOST);
> break;
> case MUSB_PORT_MODE_GADGET:
> status = musb_gadget_setup(musb);
> if (status < 0)
> goto fail3;
> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> + status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
> break;
> case MUSB_PORT_MODE_DUAL_ROLE:
> status = musb_host_setup(musb, plat->power);
> @@ -2202,7 +2222,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> musb_host_cleanup(musb);
> goto fail3;
> }
> - status = musb_platform_set_mode(musb, MUSB_OTG);
> + status = musb_try_set_mode(musb, MUSB_OTG);
> break;
> default:
> dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -556,14 +556,6 @@ static inline void musb_platform_disable(struct musb *musb)
> musb->ops->disable(musb);
> }
>
> -static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> -{
> - if (!musb->ops->set_mode)
> - return 0;
> -
> - return musb->ops->set_mode(musb, mode);
> -}
> -
> static inline void musb_platform_try_idle(struct musb *musb,
> unsigned long timeout)
> {