[PATCH v2 0/2] isp1704_charger: fix powering for N900

[PATCH v2 0/2] isp1704_charger: fix powering for N900

[Kalle Jokiniemi]

This patch set enables powering down the isp1704 usb
tranceiver when not in use.

Tested on RX-51 and MeeGo.

v2: fixed a boot issue in the 1/2 patch.

Kalle Jokiniemi (2):
  isp1704_charger: allow board specific powering routine
  RX-51: Enable isp1704 power on/off

 arch/arm/mach-omap2/board-rx51-peripherals.c |   27 ++++++++++++++++++++++-
 drivers/power/isp1704_charger.c              |   25 +++++++++++++++++++++-
 include/linux/power/isp1704_charger.h        |   29 ++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/power/isp1704_charger.h

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[Kalle Jokiniemi]

The ISP1704/1707 chip can be put to full power down
state by asserting the CHIP_SEL line. This patch enables
platform or board specific hooks to put the device into
power down mode in case not needed.

These patches are preparatio for enabling this powering
routine in n900 (rx-51) devices.

Thanks to Heikki Krogerus for helping out with the patch.

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
Cc: Heikki Krogerus <heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
---
 drivers/power/isp1704_charger.c       |   26 ++++++++++++++++++++++++++
 include/linux/power/isp1704_charger.h |   29 +++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/power/isp1704_charger.h

diff --git a/drivers/power/isp1704_charger.c b/drivers/power/isp1704_charger.c
index 2ad9b14..c796b9f 100644
--- a/drivers/power/isp1704_charger.c
+++ b/drivers/power/isp1704_charger.c
@@ -33,6 +33,7 @@
 #include <linux/usb/ulpi.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/power/isp1704_charger.h>
 
 /* Vendor specific Power Control register */
 #define ISP1704_PWR_CTRL		0x3d
@@ -63,6 +64,7 @@ struct isp1704_charger {
 	char			model[8];
 	unsigned		present:1;
 	unsigned		online:1;
+	unsigned		init_done;
 	unsigned		current_max;
 
 	/* temp storage variables */
@@ -71,6 +73,18 @@ struct isp1704_charger {
 };
 
 /*
+ * Disable/enable the power from the isp1704 if a function for it
+ * has been provided with platform data.
+ */
+static void isp1704_charger_set_power(struct isp1704_charger *isp, bool on)
+{
+	struct isp1704_charger_data	*board = isp->dev->platform_data;
+
+	if (board->set_power)
+		board->set_power(on);
+}
+
+/*
  * Determine is the charging port DCP (dedicated charger) or CDP (Host/HUB
  * chargers).
  *
@@ -222,6 +236,9 @@ static void isp1704_charger_work(struct work_struct *data)
 
 	mutex_lock(&lock);
 
+	if (event != USB_EVENT_NONE)
+		isp1704_charger_set_power(isp, 1);
+
 	switch (event) {
 	case USB_EVENT_VBUS:
 		isp->online = true;
@@ -269,6 +286,9 @@ static void isp1704_charger_work(struct work_struct *data)
 		 */
 		if (isp->otg->gadget)
 			usb_gadget_disconnect(isp->otg->gadget);
+		/* If we're initialized, we can power down the isp */
+		if (isp->init_done)
+			isp1704_charger_set_power(isp, 0);
 		break;
 	case USB_EVENT_ENUMERATED:
 		if (isp->present)
@@ -394,6 +414,8 @@ static int __devinit isp1704_charger_probe(struct platform_device *pdev)
 	isp->dev = &pdev->dev;
 	platform_set_drvdata(pdev, isp);
 
+	isp1704_charger_set_power(isp, 1);
+
 	ret = isp1704_test_ulpi(isp);
 	if (ret < 0)
 		goto fail1;
@@ -437,8 +459,11 @@ static int __devinit isp1704_charger_probe(struct platform_device *pdev)
 	if ((ret & ULPI_INT_VBUS_VALID) && !isp->otg->default_a) {
 		isp->event = USB_EVENT_VBUS;
 		schedule_work(&isp->work);
+	} else {
+		isp1704_charger_set_power(isp, 0);
 	}
 
+	isp->init_done = 1;
 	return 0;
 fail2:
 	power_supply_unregister(&isp->psy);
@@ -459,6 +484,7 @@ static int __devexit isp1704_charger_remove(struct platform_device *pdev)
 	otg_unregister_notifier(isp->otg, &isp->nb);
 	power_supply_unregister(&isp->psy);
 	otg_put_transceiver(isp->otg);
+	isp1704_charger_set_power(isp, 0);
 	kfree(isp);
 
 	return 0;
diff --git a/include/linux/power/isp1704_charger.h b/include/linux/power/isp1704_charger.h
new file mode 100644
index 0000000..68096a6
--- /dev/null
+++ b/include/linux/power/isp1704_charger.h
@@ -0,0 +1,29 @@
+/*
+ * ISP1704 USB Charger Detection driver
+ *
+ * Copyright (C) 2011 Nokia Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+
+#ifndef __ISP1704_CHARGER_H
+#define __ISP1704_CHARGER_H
+
+struct isp1704_charger_data {
+	void		(*set_power)(bool on);
+};
+
+#endif
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[Kalle Jokiniemi]

The isp1704 usb tranceiver is used for charging and can be
disabled when not in use. Provide the powering routine to
the driver via platform data.

Loosely based on earlier patches from Heikki Krogerus in
Nokia N900 maemo kernel.

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
Cc: Heikki Krogerus <heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
---
 arch/arm/mach-omap2/board-rx51-peripherals.c |   27 ++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
index e75e240..8584dd8 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -23,6 +23,7 @@
 #include <linux/gpio.h>
 #include <linux/gpio_keys.h>
 #include <linux/mmc/host.h>
+#include <linux/power/isp1704_charger.h>
 
 #include <plat/mcspi.h>
 #include <plat/board.h>
@@ -48,6 +49,8 @@
 #define RX51_WL1251_POWER_GPIO		87
 #define RX51_WL1251_IRQ_GPIO		42
 
+#define RX51_USB_TRANSCEIVER_RST_GPIO	67
+
 /* list all spi devices here */
 enum {
 	RX51_SPI_WL1251,
@@ -106,10 +109,30 @@ static struct spi_board_info rx51_peripherals_spi_board_info[] __initdata = {
 	},
 };
 
+static void rx51_charger_set_power(bool on)
+{
+	gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
+}
+
+static struct isp1704_charger_data rx51_charger_data = {
+	.set_power	= rx51_charger_set_power,
+};
+
 static struct platform_device rx51_charger_device = {
-	.name = "isp1704_charger",
+	.name	= "isp1704_charger",
+	.dev	= {
+		.platform_data = &rx51_charger_data,
+	},
 };
 
+static void __init rx51_charger_init(void)
+{
+	BUG_ON(gpio_request_one(RX51_USB_TRANSCEIVER_RST_GPIO,
+		GPIOF_OUT_INIT_LOW, "isp1704_reset"));
+
+	platform_device_register(&rx51_charger_device);
+}
+
 #if defined(CONFIG_KEYBOARD_GPIO) || defined(CONFIG_KEYBOARD_GPIO_MODULE)
 
 #define RX51_GPIO_CAMERA_LENS_COVER	110
@@ -928,6 +951,6 @@ void __init rx51_peripherals_init(void)
 	if (partition)
 		omap2_hsmmc_init(mmc);
 
-	platform_device_register(&rx51_charger_device);
+	rx51_charger_init();
 }
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[Keshava Munegowda]

> +
>  static struct platform_device rx51_charger_device = {
> -	.name = "isp1704_charger",
> +	.name	= "isp1704_charger",

Both are same; you don’t need this change!



> +	.dev	= {
> +		.platform_data = &rx51_charger_data,
> +	},
>  };
>
> +static void __init rx51_charger_init(void)
> +{
> +	BUG_ON(gpio_request_one(RX51_USB_TRANSCEIVER_RST_GPIO,
> +		GPIOF_OUT_INIT_LOW, "isp1704_reset"));
> +
> +	platform_device_register(&rx51_charger_device);
> +}
> +
>  #if defined(CONFIG_KEYBOARD_GPIO) ||
defined(CONFIG_KEYBOARD_GPIO_MODULE)
>
>  #define RX51_GPIO_CAMERA_LENS_COVER	110
> @@ -928,6 +951,6 @@ void __init rx51_peripherals_init(void)
>  	if (partition)
>  		omap2_hsmmc_init(mmc);
>
> -	platform_device_register(&rx51_charger_device);
> +	rx51_charger_init();
>  }
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi,

 > -----Original Message-----
 > From: ext Keshava Munegowda [mailto:keshava_mgowda-l0cyMroinI0@public.gmane.org]
 > Sent: 28. maaliskuuta 2011 10:19
 > To: Jokiniemi Kalle (Nokia-MS/Tampere); linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
 > omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Felipe Balbi
 > Cc: Krogerus Heikki (Nokia-MS/Helsinki); jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Kevin Hilman
 > Subject: RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > 
 > > +
 > >  static struct platform_device rx51_charger_device = {
 > > -	.name = "isp1704_charger",
 > > +	.name	= "isp1704_charger",
 > 
 > Both are same; you don't need this change!

No, actually they are not same. I added tab instead of space to match indentation with the .dev member. This is the correct way IMHO.

- Kalle

 > 
 > 
 > 
 > > +	.dev	= {
 > > +		.platform_data = &rx51_charger_data,
 > > +	},
 > >  };
 > >
 > > +static void __init rx51_charger_init(void)
 > > +{
 > > +	BUG_ON(gpio_request_one(RX51_USB_TRANSCEIVER_RST_GPIO,
 > > +		GPIOF_OUT_INIT_LOW, "isp1704_reset"));
 > > +
 > > +	platform_device_register(&rx51_charger_device);
 > > +}
 > > +
 > >  #if defined(CONFIG_KEYBOARD_GPIO) ||
 > defined(CONFIG_KEYBOARD_GPIO_MODULE)
 > >
 > >  #define RX51_GPIO_CAMERA_LENS_COVER	110
 > > @@ -928,6 +951,6 @@ void __init rx51_peripherals_init(void)
 > >  	if (partition)
 > >  		omap2_hsmmc_init(mmc);
 > >
 > > -	platform_device_register(&rx51_charger_device);
 > > +	rx51_charger_init();
 > >  }
 > >
 > > --
 > > 1.7.1
 > >
 > > --
 > > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
 > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[Sergei Shtylyov]

Hello.

On 28-03-2011 10:51, Kalle Jokiniemi wrote:

> The ISP1704/1707 chip can be put to full power down
> state by asserting the CHIP_SEL line. This patch enables
> platform or board specific hooks to put the device into
> power down mode in case not needed.

> These patches are preparatio for enabling this powering

    Preparation.

> routine in n900 (rx-51) devices.

> Thanks to Heikki Krogerus for helping out with the patch.

> Signed-off-by: Kalle Jokiniemi<kalle.jokiniemi@nokia.com>
> Cc: Heikki Krogerus<heikki.krogerus@nokia.com>
[...]

> diff --git a/include/linux/power/isp1704_charger.h b/include/linux/power/isp1704_charger.h
> new file mode 100644
> index 0000000..68096a6
> --- /dev/null
> +++ b/include/linux/power/isp1704_charger.h
> @@ -0,0 +1,29 @@
> +/*
> + * ISP1704 USB Charger Detection driver
> + *
> + * Copyright (C) 2011 Nokia Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +
> +#ifndef __ISP1704_CHARGER_H
> +#define __ISP1704_CHARGER_H
> +
> +struct isp1704_charger_data {
> +	void		(*set_power)(bool on);
> +};
> +
> +#endif

    There should be include/linux/platform_data/ directory now, specifically 
for such headers...

WBR, Sergei

Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[Sergei Shtylyov]

On 28-03-2011 10:51, Kalle Jokiniemi wrote:

> The isp1704 usb tranceiver is used for charging and can be
> disabled when not in use. Provide the powering routine to
> the driver via platform data.

> Loosely based on earlier patches from Heikki Krogerus in
> Nokia N900 maemo kernel.

> Signed-off-by: Kalle Jokiniemi<kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: Heikki Krogerus<heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> ---
>   arch/arm/mach-omap2/board-rx51-peripherals.c |   27 ++++++++++++++++++++++++-
>   1 files changed, 25 insertions(+), 2 deletions(-)

> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
> index e75e240..8584dd8 100644
> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
[...]
> @@ -106,10 +109,30 @@ static struct spi_board_info rx51_peripherals_spi_board_info[] __initdata = {
>   	},
>   };
>
> +static void rx51_charger_set_power(bool on)
> +{
> +	gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
> +}
> +
> +static struct isp1704_charger_data rx51_charger_data = {
> +	.set_power	= rx51_charger_set_power,
> +};
> +
>   static struct platform_device rx51_charger_device = {
> -	.name = "isp1704_charger",
> +	.name	= "isp1704_charger",
> +	.dev	= {
> +		.platform_data =&rx51_charger_data,
> +	},
>   };
>
> +static void __init rx51_charger_init(void)
> +{
> +	BUG_ON(gpio_request_one(RX51_USB_TRANSCEIVER_RST_GPIO,
> +		GPIOF_OUT_INIT_LOW, "isp1704_reset"));

    I'm not sure we should kill the machine if this fails...

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[Felipe Balbi]

Hi,

(break your lines at 80-chars, I asked you to do this before)

On Mon, Mar 28, 2011 at 08:06:32AM +0000, kalle.jokiniemi@nokia.com wrote:
> Hi,
> 
>  > -----Original Message-----
>  > From: ext Keshava Munegowda [mailto:keshava_mgowda@ti.com]
>  > Sent: 28. maaliskuuta 2011 10:19
>  > To: Jokiniemi Kalle (Nokia-MS/Tampere); linux-usb@vger.kernel.org; linux-
>  > omap@vger.kernel.org; Felipe Balbi
>  > Cc: Krogerus Heikki (Nokia-MS/Helsinki); jhnikula@gmail.com; Kevin Hilman
>  > Subject: RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
>  > 
>  > > +
>  > >  static struct platform_device rx51_charger_device = {
>  > > -	.name = "isp1704_charger",
>  > > +	.name	= "isp1704_charger",
>  > 
>  > Both are same; you don't need this change!
> 
> No, actually they are not same. I added tab instead of space to match
> indentation with the .dev member. This is the correct way IMHO.

Still, changing that indentation isn't part of $SUBJECT. You need to put
that in another patch.

-- 
balbi

Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[Felipe Balbi]

On Mon, Mar 28, 2011 at 12:20:02PM +0400, Sergei Shtylyov wrote:
> >+static void __init rx51_charger_init(void)
> >+{
> >+	BUG_ON(gpio_request_one(RX51_USB_TRANSCEIVER_RST_GPIO,
> >+		GPIOF_OUT_INIT_LOW, "isp1704_reset"));
> 
>    I'm not sure we should kill the machine if this fails...

agreed.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi,

 > -----Original Message-----
 > From: ext Felipe Balbi [mailto:balbi-l0cyMroinI0@public.gmane.org]
 > Sent: 28. maaliskuuta 2011 11:39
 > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > Cc: keshava_mgowda-l0cyMroinI0@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
 > omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; balbi-l0cyMroinI0@public.gmane.org; Krogerus Heikki (Nokia-MS/Helsinki);
 > jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; khilman-l0cyMroinI0@public.gmane.org
 > Subject: Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > 
 > Hi,
 > 
 > (break your lines at 80-chars, I asked you to do this before)

Sorry, using outlook :( ... Will do that.

 > 
 > On Mon, Mar 28, 2011 at 08:06:32AM +0000, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org wrote:
 > > Hi,
 > >
 > >  > -----Original Message-----
 > >  > From: ext Keshava Munegowda [mailto:keshava_mgowda-l0cyMroinI0@public.gmane.org]
 > >  > Sent: 28. maaliskuuta 2011 10:19
 > >  > To: Jokiniemi Kalle (Nokia-MS/Tampere); linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
 > >  > omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Felipe Balbi
 > >  > Cc: Krogerus Heikki (Nokia-MS/Helsinki); jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Kevin Hilman
 > >  > Subject: RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > >  >
 > >  > > +
 > >  > >  static struct platform_device rx51_charger_device = {
 > >  > > -	.name = "isp1704_charger",
 > >  > > +	.name	= "isp1704_charger",
 > >  >
 > >  > Both are same; you don't need this change!
 > >
 > > No, actually they are not same. I added tab instead of space to match
 > > indentation with the .dev member. This is the correct way IMHO.
 > 
 > Still, changing that indentation isn't part of $SUBJECT. You need to put
 > that in another patch.

Are you sure? It'll be patch that changes indentation that need not be
changed... Only the additional member definition makes the indentation
necessary, hence I see it as natural part of this patch.

But if that's a problem, I can do a separate patch, no problem.

- Kalle

 > 
 > --
 > balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[Felipe Balbi]

On Mon, Mar 28, 2011 at 08:52:23AM +0000, kalle.jokiniemi@nokia.com wrote:
> Hi,
> 
>  > -----Original Message-----
>  > From: ext Felipe Balbi [mailto:balbi@ti.com]
>  > Sent: 28. maaliskuuta 2011 11:39
>  > To: Jokiniemi Kalle (Nokia-MS/Tampere)
>  > Cc: keshava_mgowda@ti.com; linux-usb@vger.kernel.org; linux-
>  > omap@vger.kernel.org; balbi@ti.com; Krogerus Heikki (Nokia-MS/Helsinki);
>  > jhnikula@gmail.com; khilman@ti.com
>  > Subject: Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
>  > 
>  > Hi,
>  > 
>  > (break your lines at 80-chars, I asked you to do this before)
> 
> Sorry, using outlook :( ... Will do that.
> 
>  > 
>  > On Mon, Mar 28, 2011 at 08:06:32AM +0000, kalle.jokiniemi@nokia.com wrote:
>  > > Hi,
>  > >
>  > >  > -----Original Message-----
>  > >  > From: ext Keshava Munegowda [mailto:keshava_mgowda@ti.com]
>  > >  > Sent: 28. maaliskuuta 2011 10:19
>  > >  > To: Jokiniemi Kalle (Nokia-MS/Tampere); linux-usb@vger.kernel.org; linux-
>  > >  > omap@vger.kernel.org; Felipe Balbi
>  > >  > Cc: Krogerus Heikki (Nokia-MS/Helsinki); jhnikula@gmail.com; Kevin Hilman
>  > >  > Subject: RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
>  > >  >
>  > >  > > +
>  > >  > >  static struct platform_device rx51_charger_device = {
>  > >  > > -	.name = "isp1704_charger",
>  > >  > > +	.name	= "isp1704_charger",
>  > >  >
>  > >  > Both are same; you don't need this change!
>  > >
>  > > No, actually they are not same. I added tab instead of space to match
>  > > indentation with the .dev member. This is the correct way IMHO.
>  > 
>  > Still, changing that indentation isn't part of $SUBJECT. You need to put
>  > that in another patch.
> 
> Are you sure? It'll be patch that changes indentation that need not be
> changed... Only the additional member definition makes the indentation
> necessary, hence I see it as natural part of this patch.
> 
> But if that's a problem, I can do a separate patch, no problem.

you need to at least add notes on the commit log explicit saying that
you're also fixing indentation of one member because it's not worth
having a separate patch and blablabla...

-- 
balbi

RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi,

 > -----Original Message-----
 > From: ext Felipe Balbi [mailto:balbi-l0cyMroinI0@public.gmane.org]
 > Sent: 28. maaliskuuta 2011 11:55
 > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > Cc: balbi-l0cyMroinI0@public.gmane.org; keshava_mgowda-l0cyMroinI0@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
 > omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Krogerus Heikki (Nokia-MS/Helsinki);
 > jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; khilman-l0cyMroinI0@public.gmane.org
 > Subject: Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > 
 > On Mon, Mar 28, 2011 at 08:52:23AM +0000, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org wrote:
 > > Hi,
 > >
 > >  > -----Original Message-----
 > >  > From: ext Felipe Balbi [mailto:balbi-l0cyMroinI0@public.gmane.org]
 > >  > Sent: 28. maaliskuuta 2011 11:39
 > >  > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > >  > Cc: keshava_mgowda-l0cyMroinI0@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
 > >  > omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; balbi-l0cyMroinI0@public.gmane.org; Krogerus Heikki (Nokia-
 > MS/Helsinki);
 > >  > jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; khilman-l0cyMroinI0@public.gmane.org
 > >  > Subject: Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > >  >
 > >  > Hi,
 > >  >
 > >  > (break your lines at 80-chars, I asked you to do this before)
 > >
 > > Sorry, using outlook :( ... Will do that.
 > >
 > >  >
 > >  > On Mon, Mar 28, 2011 at 08:06:32AM +0000, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org
 > wrote:
 > >  > > Hi,
 > >  > >
 > >  > >  > -----Original Message-----
 > >  > >  > From: ext Keshava Munegowda [mailto:keshava_mgowda-l0cyMroinI0@public.gmane.org]
 > >  > >  > Sent: 28. maaliskuuta 2011 10:19
 > >  > >  > To: Jokiniemi Kalle (Nokia-MS/Tampere); linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
 > linux-
 > >  > >  > omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Felipe Balbi
 > >  > >  > Cc: Krogerus Heikki (Nokia-MS/Helsinki); jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Kevin
 > Hilman
 > >  > >  > Subject: RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > >  > >  >
 > >  > >  > > +
 > >  > >  > >  static struct platform_device rx51_charger_device = {
 > >  > >  > > -	.name = "isp1704_charger",
 > >  > >  > > +	.name	= "isp1704_charger",
 > >  > >  >
 > >  > >  > Both are same; you don't need this change!
 > >  > >
 > >  > > No, actually they are not same. I added tab instead of space to match
 > >  > > indentation with the .dev member. This is the correct way IMHO.
 > >  >
 > >  > Still, changing that indentation isn't part of $SUBJECT. You need to put
 > >  > that in another patch.
 > >
 > > Are you sure? It'll be patch that changes indentation that need not be
 > > changed... Only the additional member definition makes the indentation
 > > necessary, hence I see it as natural part of this patch.
 > >
 > > But if that's a problem, I can do a separate patch, no problem.
 > 
 > you need to at least add notes on the commit log explicit saying that
 > you're also fixing indentation of one member because it's not worth
 > having a separate patch and blablabla...

Sounds fair, I'll do that.

- Kalle


 > 
 > --
 > balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[Heikki Krogerus]

Hi,

Add Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> to your v3. This will
need ack from him, or this needs to go to him. In this case I guess we
are only dealing with RX51 stuff, so maybe this should go to Tony.

On Mon, Mar 28, 2011 at 09:51:38AM +0300, Kalle Jokiniemi wrote:
> The ISP1704/1707 chip can be put to full power down
> state by asserting the CHIP_SEL line. This patch enables
> platform or board specific hooks to put the device into
> power down mode in case not needed.
> 
> These patches are preparatio for enabling this powering
> routine in n900 (rx-51) devices.
> 
> Thanks to Heikki Krogerus for helping out with the patch.
> 
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: Heikki Krogerus <heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/power/isp1704_charger.c       |   26 ++++++++++++++++++++++++++
>  include/linux/power/isp1704_charger.h |   29 +++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/power/isp1704_charger.h
> 
> diff --git a/drivers/power/isp1704_charger.c b/drivers/power/isp1704_charger.c
> index 2ad9b14..c796b9f 100644
> --- a/drivers/power/isp1704_charger.c
> +++ b/drivers/power/isp1704_charger.c
> @@ -33,6 +33,7 @@
>  #include <linux/usb/ulpi.h>
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> +#include <linux/power/isp1704_charger.h>
>  
>  /* Vendor specific Power Control register */
>  #define ISP1704_PWR_CTRL		0x3d
> @@ -63,6 +64,7 @@ struct isp1704_charger {
>  	char			model[8];
>  	unsigned		present:1;
>  	unsigned		online:1;
> +	unsigned		init_done;

Do we need this?

>  	unsigned		current_max;
>  
>  	/* temp storage variables */
> @@ -71,6 +73,18 @@ struct isp1704_charger {
>  };
>  
>  /*
> + * Disable/enable the power from the isp1704 if a function for it
> + * has been provided with platform data.
> + */
> +static void isp1704_charger_set_power(struct isp1704_charger *isp, bool on)
> +{
> +	struct isp1704_charger_data	*board = isp->dev->platform_data;
> +
> +	if (board->set_power)
> +		board->set_power(on);
> +}
> +
> +/*
>   * Determine is the charging port DCP (dedicated charger) or CDP (Host/HUB
>   * chargers).
>   *
> @@ -222,6 +236,9 @@ static void isp1704_charger_work(struct work_struct *data)
>  
>  	mutex_lock(&lock);
>  
> +	if (event != USB_EVENT_NONE)
> +		isp1704_charger_set_power(isp, 1);
> +
>  	switch (event) {
>  	case USB_EVENT_VBUS:
>  		isp->online = true;
> @@ -269,6 +286,9 @@ static void isp1704_charger_work(struct work_struct *data)
>  		 */
>  		if (isp->otg->gadget)
>  			usb_gadget_disconnect(isp->otg->gadget);
> +		/* If we're initialized, we can power down the isp */
> +		if (isp->init_done)
> +			isp1704_charger_set_power(isp, 0);
>  		break;
>  	case USB_EVENT_ENUMERATED:
>  		if (isp->present)
> @@ -394,6 +414,8 @@ static int __devinit isp1704_charger_probe(struct platform_device *pdev)
>  	isp->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, isp);
>  
> +	isp1704_charger_set_power(isp, 1);
> +
>  	ret = isp1704_test_ulpi(isp);
>  	if (ret < 0)
>  		goto fail1;
> @@ -437,8 +459,11 @@ static int __devinit isp1704_charger_probe(struct platform_device *pdev)
>  	if ((ret & ULPI_INT_VBUS_VALID) && !isp->otg->default_a) {
>  		isp->event = USB_EVENT_VBUS;
>  		schedule_work(&isp->work);
> +	} else {
> +		isp1704_charger_set_power(isp, 0);
>  	}

I think the transceiver can be powered down even if we just scheduled
the work. This will basically cause hw reset on the transceiver which
is only a good thing IMO. Drop the else condition.

> +	isp->init_done = 1;
>  	return 0;
>  fail2:
>  	power_supply_unregister(&isp->psy);
> @@ -459,6 +484,7 @@ static int __devexit isp1704_charger_remove(struct platform_device *pdev)
>  	otg_unregister_notifier(isp->otg, &isp->nb);
>  	power_supply_unregister(&isp->psy);
>  	otg_put_transceiver(isp->otg);
> +	isp1704_charger_set_power(isp, 0);
>  	kfree(isp);
>  
>  	return 0;

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[kalle.jokiniemi]

Hello,

 > -----Original Message-----
 > From: Heikki Krogerus [mailto:heikki.krogerus@nokia.com]
 > Sent: 28. maaliskuuta 2011 12:13
 > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; balbi@ti.com;
 > jhnikula@gmail.com; khilman@ti.com
 > Subject: Re: [PATCH v2 1/2] isp1704_charger: allow board specific powering
 > routine
 > 
 > Hi,
 > 
 > Add Anton Vorontsov <cbouatmailru@gmail.com> to your v3. This will
 > need ack from him, or this needs to go to him. In this case I guess we
 > are only dealing with RX51 stuff, so maybe this should go to Tony.

Thanks, I'll add Anton to cc.

 > 
 > On Mon, Mar 28, 2011 at 09:51:38AM +0300, Kalle Jokiniemi wrote:
 > > The ISP1704/1707 chip can be put to full power down
 > > state by asserting the CHIP_SEL line. This patch enables
 > > platform or board specific hooks to put the device into
 > > power down mode in case not needed.
 > >
 > > These patches are preparatio for enabling this powering
 > > routine in n900 (rx-51) devices.
 > >
 > > Thanks to Heikki Krogerus for helping out with the patch.
 > >
 > > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@nokia.com>
 > > Cc: Heikki Krogerus <heikki.krogerus@nokia.com>
 > > ---
 > >  drivers/power/isp1704_charger.c       |   26 ++++++++++++++++++++++++++
 > >  include/linux/power/isp1704_charger.h |   29
 > +++++++++++++++++++++++++++++
 > >  2 files changed, 55 insertions(+), 0 deletions(-)
 > >  create mode 100644 include/linux/power/isp1704_charger.h
 > >
 > > diff --git a/drivers/power/isp1704_charger.c
 > b/drivers/power/isp1704_charger.c
 > > index 2ad9b14..c796b9f 100644
 > > --- a/drivers/power/isp1704_charger.c
 > > +++ b/drivers/power/isp1704_charger.c
 > > @@ -33,6 +33,7 @@
 > >  #include <linux/usb/ulpi.h>
 > >  #include <linux/usb/ch9.h>
 > >  #include <linux/usb/gadget.h>
 > > +#include <linux/power/isp1704_charger.h>
 > >
 > >  /* Vendor specific Power Control register */
 > >  #define ISP1704_PWR_CTRL		0x3d
 > > @@ -63,6 +64,7 @@ struct isp1704_charger {
 > >  	char			model[8];
 > >  	unsigned		present:1;
 > >  	unsigned		online:1;
 > > +	unsigned		init_done;
 > 
 > Do we need this?

Good point. I needed this for 2.6.37 meego kernel, because it calls
otg_get_last_event in middle of the probing, which causes the
isp1704_charger_work to be run with USB_EVENT_NONE, which
then caused the power to be shut down from the isp during probe.

But it seems the get_last_notifier is not called in upstream code,
So I probably can drop that init_done variable.

 > 
 > >  	unsigned		current_max;
 > >
 > >  	/* temp storage variables */
 > > @@ -71,6 +73,18 @@ struct isp1704_charger {
 > >  };
 > >
 > >  /*
 > > + * Disable/enable the power from the isp1704 if a function for it
 > > + * has been provided with platform data.
 > > + */
 > > +static void isp1704_charger_set_power(struct isp1704_charger *isp, bool
 > on)
 > > +{
 > > +	struct isp1704_charger_data	*board = isp->dev->platform_data;
 > > +
 > > +	if (board->set_power)
 > > +		board->set_power(on);
 > > +}
 > > +
 > > +/*
 > >   * Determine is the charging port DCP (dedicated charger) or CDP (Host/HUB
 > >   * chargers).
 > >   *
 > > @@ -222,6 +236,9 @@ static void isp1704_charger_work(struct work_struct
 > *data)
 > >
 > >  	mutex_lock(&lock);
 > >
 > > +	if (event != USB_EVENT_NONE)
 > > +		isp1704_charger_set_power(isp, 1);
 > > +
 > >  	switch (event) {
 > >  	case USB_EVENT_VBUS:
 > >  		isp->online = true;
 > > @@ -269,6 +286,9 @@ static void isp1704_charger_work(struct work_struct
 > *data)
 > >  		 */
 > >  		if (isp->otg->gadget)
 > >  			usb_gadget_disconnect(isp->otg->gadget);
 > > +		/* If we're initialized, we can power down the isp */
 > > +		if (isp->init_done)
 > > +			isp1704_charger_set_power(isp, 0);
 > >  		break;
 > >  	case USB_EVENT_ENUMERATED:
 > >  		if (isp->present)
 > > @@ -394,6 +414,8 @@ static int __devinit isp1704_charger_probe(struct
 > platform_device *pdev)
 > >  	isp->dev = &pdev->dev;
 > >  	platform_set_drvdata(pdev, isp);
 > >
 > > +	isp1704_charger_set_power(isp, 1);
 > > +
 > >  	ret = isp1704_test_ulpi(isp);
 > >  	if (ret < 0)
 > >  		goto fail1;
 > > @@ -437,8 +459,11 @@ static int __devinit isp1704_charger_probe(struct
 > platform_device *pdev)
 > >  	if ((ret & ULPI_INT_VBUS_VALID) && !isp->otg->default_a) {
 > >  		isp->event = USB_EVENT_VBUS;
 > >  		schedule_work(&isp->work);
 > > +	} else {
 > > +		isp1704_charger_set_power(isp, 0);
 > >  	}
 > 
 > I think the transceiver can be powered down even if we just scheduled
 > the work. This will basically cause hw reset on the transceiver which
 > is only a good thing IMO. Drop the else condition.

Ok, I'll test that out and include it if it works w/o problems.

Thanks for the excellent comments.

- Kalle

 > 
 > > +	isp->init_done = 1;
 > >  	return 0;
 > >  fail2:
 > >  	power_supply_unregister(&isp->psy);
 > > @@ -459,6 +484,7 @@ static int __devexit isp1704_charger_remove(struct
 > platform_device *pdev)
 > >  	otg_unregister_notifier(isp->otg, &isp->nb);
 > >  	power_supply_unregister(&isp->psy);
 > >  	otg_put_transceiver(isp->otg);
 > > +	isp1704_charger_set_power(isp, 0);
 > >  	kfree(isp);
 > >
 > >  	return 0;
 > 
 > --
 > heikki

RE: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi,

 > -----Original Message-----
 > From: ext Sergei Shtylyov [mailto:sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org]
 > Sent: 28. maaliskuuta 2011 11:20
 > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; balbi-l0cyMroinI0@public.gmane.org;
 > Krogerus Heikki (Nokia-MS/Helsinki); jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; khilman-l0cyMroinI0@public.gmane.org
 > Subject: Re: [PATCH v2 2/2] RX-51: Enable isp1704 power on/off
 > 
 > On 28-03-2011 10:51, Kalle Jokiniemi wrote:
 > 
 > > The isp1704 usb tranceiver is used for charging and can be
 > > disabled when not in use. Provide the powering routine to
 > > the driver via platform data.
 > 
 > > Loosely based on earlier patches from Heikki Krogerus in
 > > Nokia N900 maemo kernel.
 > 
 > > Signed-off-by: Kalle Jokiniemi<kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
 > > Cc: Heikki Krogerus<heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
 > > ---
 > >   arch/arm/mach-omap2/board-rx51-peripherals.c |   27
 > ++++++++++++++++++++++++-
 > >   1 files changed, 25 insertions(+), 2 deletions(-)
 > 
 > > diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
 > b/arch/arm/mach-omap2/board-rx51-peripherals.c
 > > index e75e240..8584dd8 100644
 > > --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
 > > +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
 > [...]
 > > @@ -106,10 +109,30 @@ static struct spi_board_info
 > rx51_peripherals_spi_board_info[] __initdata = {
 > >   	},
 > >   };
 > >
 > > +static void rx51_charger_set_power(bool on)
 > > +{
 > > +	gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
 > > +}
 > > +
 > > +static struct isp1704_charger_data rx51_charger_data = {
 > > +	.set_power	= rx51_charger_set_power,
 > > +};
 > > +
 > >   static struct platform_device rx51_charger_device = {
 > > -	.name = "isp1704_charger",
 > > +	.name	= "isp1704_charger",
 > > +	.dev	= {
 > > +		.platform_data =&rx51_charger_data,
 > > +	},
 > >   };
 > >
 > > +static void __init rx51_charger_init(void)
 > > +{
 > > +	BUG_ON(gpio_request_one(RX51_USB_TRANSCEIVER_RST_GPIO,
 > > +		GPIOF_OUT_INIT_LOW, "isp1704_reset"));
 > 
 >     I'm not sure we should kill the machine if this fails...

True, WARN_ON is probably sufficient.

- Kalle

 > 
 > WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[kalle.jokiniemi]

Hi,

 > -----Original Message-----
 > From: ext Sergei Shtylyov [mailto:sshtylyov@mvista.com]
 > Sent: 28. maaliskuuta 2011 11:18
 > To: Jokiniemi Kalle (Nokia-MS/Tampere)
 > Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; balbi@ti.com;
 > Krogerus Heikki (Nokia-MS/Helsinki); jhnikula@gmail.com; khilman@ti.com
 > Subject: Re: [PATCH v2 1/2] isp1704_charger: allow board specific powering
 > routine
 > 
 > Hello.
 > 
 > On 28-03-2011 10:51, Kalle Jokiniemi wrote:
 > 
 > > The ISP1704/1707 chip can be put to full power down
 > > state by asserting the CHIP_SEL line. This patch enables
 > > platform or board specific hooks to put the device into
 > > power down mode in case not needed.
 > 
 > > These patches are preparatio for enabling this powering
 > 
 >     Preparation.
 > 
 > > routine in n900 (rx-51) devices.
 > 
 > > Thanks to Heikki Krogerus for helping out with the patch.
 > 
 > > Signed-off-by: Kalle Jokiniemi<kalle.jokiniemi@nokia.com>
 > > Cc: Heikki Krogerus<heikki.krogerus@nokia.com>
 > [...]
 > 
 > > diff --git a/include/linux/power/isp1704_charger.h
 > b/include/linux/power/isp1704_charger.h
 > > new file mode 100644
 > > index 0000000..68096a6
 > > --- /dev/null
 > > +++ b/include/linux/power/isp1704_charger.h
 > > @@ -0,0 +1,29 @@
 > > +/*
 > > + * ISP1704 USB Charger Detection driver
 > > + *
 > > + * Copyright (C) 2011 Nokia Corporation
 > > + *
 > > + * This program is free software; you can redistribute it and/or modify
 > > + * it under the terms of the GNU General Public License as published by
 > > + * the Free Software Foundation; either version 2 of the License, or
 > > + * (at your option) any later version.
 > > + *
 > > + * This program is distributed in the hope that it will be useful,
 > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 > > + * GNU General Public License for more details.
 > > + *
 > > + * You should have received a copy of the GNU General Public License
 > > + * along with this program; if not, write to the Free Software
 > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 > > + */
 > > +
 > > +
 > > +#ifndef __ISP1704_CHARGER_H
 > > +#define __ISP1704_CHARGER_H
 > > +
 > > +struct isp1704_charger_data {
 > > +	void		(*set_power)(bool on);
 > > +};
 > > +
 > > +#endif
 > 
 >     There should be include/linux/platform_data/ directory now, specifically
 > for such headers...

Thanks for the pointer, I'll put it there.

- Kalle

 > 
 > WBR, Sergei

RE: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi,

 > -----Original Message-----
 > From: Jokiniemi Kalle (Nokia-MS/Tampere)

<snip>

>  > > diff --git a/include/linux/power/isp1704_charger.h
 >  > b/include/linux/power/isp1704_charger.h
 >  > > new file mode 100644
 >  > > index 0000000..68096a6
 >  > > --- /dev/null
 >  > > +++ b/include/linux/power/isp1704_charger.h
 >  > > @@ -0,0 +1,29 @@
 >  > > +/*
 >  > > + * ISP1704 USB Charger Detection driver
 >  > > + *
 >  > > + * Copyright (C) 2011 Nokia Corporation
 >  > > + *
 >  > > + * This program is free software; you can redistribute it and/or modify
 >  > > + * it under the terms of the GNU General Public License as published by
 >  > > + * the Free Software Foundation; either version 2 of the License, or
 >  > > + * (at your option) any later version.
 >  > > + *
 >  > > + * This program is distributed in the hope that it will be useful,
 >  > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 >  > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 >  > > + * GNU General Public License for more details.
 >  > > + *
 >  > > + * You should have received a copy of the GNU General Public License
 >  > > + * along with this program; if not, write to the Free Software
 >  > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 > USA
 >  > > + */
 >  > > +
 >  > > +
 >  > > +#ifndef __ISP1704_CHARGER_H
 >  > > +#define __ISP1704_CHARGER_H
 >  > > +
 >  > > +struct isp1704_charger_data {
 >  > > +	void		(*set_power)(bool on);
 >  > > +};
 >  > > +
 >  > > +#endif
 >  >
 >  >     There should be include/linux/platform_data/ directory now, specifically
 >  > for such headers...
 > 
 > Thanks for the pointer, I'll put it there.

There are things like msm-serial.h and tegra-usb.h there, but I'm not so convinced
that this isp1704_charger.h should be there. The isp1707 component on n900 is
a discrete chip, it's not part of the SoC, so in theory some other device or platform
could also use it as a charger. It's not even made by TI, so it's not related in that
sense to OMAP platform...

What do you think about keeping it still in /include/linux/power ?

- Kalle

 > 
 > - Kalle
 > 
 >  >
 >  > WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[Sergei Shtylyov]

Hello.

On 29.03.2011 9:52, kalle.jokiniemi@nokia.com wrote:

>>   >  >  diff --git a/include/linux/power/isp1704_charger.h
>   >   >  b/include/linux/power/isp1704_charger.h
>   >   >  >  new file mode 100644
>   >   >  >  index 0000000..68096a6
>   >   >  >  --- /dev/null
>   >   >  >  +++ b/include/linux/power/isp1704_charger.h
>   >   >  >  @@ -0,0 +1,29 @@
>   >   >  >  +/*
>   >   >  >  + * ISP1704 USB Charger Detection driver
>   >   >  >  + *
>   >   >  >  + * Copyright (C) 2011 Nokia Corporation
>   >   >  >  + *
>   >   >  >  + * This program is free software; you can redistribute it and/or modify
>   >   >  >  + * it under the terms of the GNU General Public License as published by
>   >   >  >  + * the Free Software Foundation; either version 2 of the License, or
>   >   >  >  + * (at your option) any later version.
>   >   >  >  + *
>   >   >  >  + * This program is distributed in the hope that it will be useful,
>   >   >  >  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>   >   >  >  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>   >   >  >  + * GNU General Public License for more details.
>   >   >  >  + *
>   >   >  >  + * You should have received a copy of the GNU General Public License
>   >   >  >  + * along with this program; if not, write to the Free Software
>   >   >  >  + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
>   >  USA
>   >   >  >  + */
>   >   >  >  +
>   >   >  >  +
>   >   >  >  +#ifndef __ISP1704_CHARGER_H
>   >   >  >  +#define __ISP1704_CHARGER_H
>   >   >  >  +
>   >   >  >  +struct isp1704_charger_data {
>   >   >  >  +	void		(*set_power)(bool on);
>   >   >  >  +};
>   >   >  >  +
>   >   >  >  +#endif

>   >   >      There should be include/linux/platform_data/ directory now, specifically
>   >   >  for such headers...

>   >  Thanks for the pointer, I'll put it there.

> There are things like msm-serial.h and tegra-usb.h there, but I'm not so convinced
> that this isp1704_charger.h should be there. The isp1707 component on n900 is
> a discrete chip, it's not part of the SoC, so in theory some other device or platform
> could also use it as a charger. It's not even made by TI, so it's not related in that
> sense to OMAP platform...

    This directory is very new, so there's not much there yet. I don't think 
it's dedicated to SoC devices. Frankly speaking, I didn't get your 
argumentation here...

> What do you think about keeping it still in /include/linux/power ?

    Well, I wouldn't object. It seemed to me initially that you're going to 
put it into include/linux/, so I've pointed to a better place...

> - Kalle

WBR, Sergei