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