Re: [PATCH 2/2] OMAP3: rx51: specify phy_power for usb tranceiver

[Heikki Krogerus <heikki.krogerus@nokia.com>]

Hi,

On Tue, Mar 22, 2011 at 12:24:27PM +0200, Jokiniemi Kalle (Nokia-MS/Tampere) wrote:
>  > > +static int rx51_xceiv_power(struct device *dev, int iD, int on)
>  > > +{
>  > > +	unsigned long	timeout;
>  > > +
>  > > +	if (!on) {
>  > > +		/* Let musb go stdby before powering down the transceiver */
>  > > +		timeout = jiffies + msecs_to_jiffies(100);
>  > > +		while (!time_after(jiffies, timeout))
>  > > +			if (omap2_cm_read_mod_reg(CORE_MOD,
>  > CM_IDLEST1)
>  > > +				&
>  > OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK)
>  > > +				break;
>  > > +		if (!(omap2_cm_read_mod_reg(CORE_MOD, CM_IDLEST1)
>  > > +			& OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK))
>  > > +			WARN(1, "could not put musb to sleep\n");
>  > > +	}
>  > > +	gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
>  > > +
>  > > +	return 0;
>  > > +}
>  > 
>  > The busy loop is not needed, and not what we want. We need to be able
>  > to toggle the CHIP_SEL even if the USB block is not IDLE or STDBY.
> 
> I basically just took what was in the maemo kernel. Was there some reason
> originally to include the busyloop? Do I get it now correctly that the ISP is
> only needed active when we are charging?

OK, my comment was incorrect. The gpio is clearly set regardless of the
outcome of the loop. To answer your question, we only need the ISP to
be active when _detecting_ the charger and obviously when USB is in
use.

We had problems hitting off-mode if we switched the transceiver off
before the controller had entered STDBY. I think this needs to be
tested again. The code in drivers/usb/musb/omap2430.c is quite
different then what we had in maemo kernel. It could be that there is
no need for the loop anymore. If you want to play it safe, fix it and
leave it there.

-- 
heikki