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

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

 



Hi,

 > -----Original Message-----
 > From: Heikki Krogerus [mailto:heikki.krogerus@xxxxxxxxx]
 > Sent: 22. maaliskuuta 2011 11:13
 > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > Cc: linux-usb@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; balbi@xxxxxx;
 > tony@xxxxxxxxxxx; jhnikula@xxxxxxxxx; Koskinen Ilkka (Nokia-MS/Tampere)
 > Subject: Re: [PATCH 2/2] OMAP3: rx51: specify phy_power for usb tranceiver
 > 
 > Hi,
 > 
 > On Mon, Mar 21, 2011 at 03:50:20PM +0200, Kalle Jokiniemi wrote:
 > > This patch allows the ISP1707 USB tranceiver on Nokia
 > > N900 to be disabled when usb cable is disconnected.
 > > This saves approximately 14mA of battery current.
 > >
 > > Patch based on work done by Heikki Krogerus on N900
 > > maemo kernel.
 > >
 > > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx>
 > > Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxx>
 > > ---
 > >  arch/arm/mach-omap2/board-rx51-peripherals.c |   32
 > ++++++++++++++++++++++++++
 > >  1 files changed, 32 insertions(+), 0 deletions(-)
 > 
 > <snip>
 > 
 > > +static void __init rx51_xceiv_init(void)
 > > +{
 > > +	if (gpio_request(RX51_USB_TRANSCEIVER_RST_GPIO, NULL) < 0)
 > > +		BUG();
 > > +	gpio_direction_output(RX51_USB_TRANSCEIVER_RST_GPIO, 1);
 > > +}
 > > +
 > > +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?

 > 
 > The guys have already pointed out some issues with this code, so
 > please rework these functions.

Will do.

 > 
 > >  static struct twl4030_usb_data rx51_usb_data = {
 > >  	.usb_mode		= T2_USB_MODE_ULPI,
 > > +	.phy_power		= rx51_xceiv_power,
 > >  };
 > 
 > This is not the right place for this. The gpio controls isp1707, not
 > the twl4030_usb. You have the rx51_charger_device platform device for
 > isp1707. Add them there.

Will fix.

- Kalle


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