Patchwork [4/4] RX-51: Add platform function and data for bq24150a charger

login
register
mail settings
Submitter Pali Rohár
Date Sept. 8, 2013, 8:50 a.m.
Message ID <1378630239-10006-5-git-send-email-pali.rohar@gmail.com>
Download mbox | patch
Permalink /patch/273427/
State New
Headers show

Comments

Pali Rohár - Sept. 8, 2013, 8:50 a.m.
This patch will register bq24150a charger in RX-51 board data.
Patch also adding platform function between isp1704 and bq2415x
drivers for detecting charger type.

So finally charging battery on Nokia N900 (RX-51) working
automatically without any proprietary Nokia bits in userspace.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 arch/arm/mach-omap2/board-rx51-peripherals.c |   56 +++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)
Sebastian Reichel - Sept. 9, 2013, 1:39 p.m.
Hi Pali,

On Sun, Sep 08, 2013 at 10:50:39AM +0200, Pali Rohár wrote:
> This patch will register bq24150a charger in RX-51 board data.
> Patch also adding platform function between isp1704 and bq2415x
> drivers for detecting charger type.
> 
> So finally charging battery on Nokia N900 (RX-51) working
> automatically without any proprietary Nokia bits in userspace.

cool :)

> index 9c2dd10..a993ffe 100644
> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c

AFAIK platform code for omap3 based boards is supposed to be removed
in the near future [0]. Device Tree does not support the glue layer, so
probably a small rx51 specific driver is needed.

[0] https://lkml.org/lkml/2013/8/12/70

-- Sebastian
Pali Rohár - Sept. 20, 2013, 7:22 p.m.
On Sunday 08 September 2013 10:50:39 Pali Rohár wrote:
> This patch will register bq24150a charger in RX-51 board data.
> Patch also adding platform function between isp1704 and
> bq2415x drivers for detecting charger type.
> 
> So finally charging battery on Nokia N900 (RX-51) working
> automatically without any proprietary Nokia bits in userspace.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  arch/arm/mach-omap2/board-rx51-peripherals.c |   56
> +++++++++++++++++++++++++- 1 file changed, 55 insertions(+),
> 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
> b/arch/arm/mach-omap2/board-rx51-peripherals.c index
> 9c2dd10..a993ffe 100644
> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> @@ -25,6 +25,7 @@
>  #include <linux/gpio_keys.h>
>  #include <linux/mmc/host.h>
>  #include <linux/power/isp1704_charger.h>
> +#include <linux/power/bq2415x_charger.h>
>  #include <linux/platform_data/spi-omap2-mcspi.h>
>  #include <linux/platform_data/mtd-onenand-omap2.h>
> 
> @@ -270,6 +271,44 @@ static struct platform_device
> rx51_battery_device = { .id	= -1,
>  };
> 
> +static enum bq2415x_mode rx51_charger_mode =
> BQ2415X_MODE_OFF; +static void *rx51_charger_hook_data;
> +static void (*rx51_charger_hook)(enum bq2415x_mode mode, void
> *data); +
> +static int rx51_charger_set_hook(
> +		void (*hook)(enum bq2415x_mode mode, void *data), void
> *data) +{
> +	rx51_charger_hook = hook;
> +	rx51_charger_hook_data = data;
> +	if (rx51_charger_hook)
> +		rx51_charger_hook(rx51_charger_mode,
> rx51_charger_hook_data); +	return 1;
> +}
> +
> +static void rx51_charger_set_current(int mA)
> +{
> +	enum bq2415x_mode mode;
> +
> +	pr_info("RX-51: Charger current limit is %d mA\n", mA);
> +
> +	if (mA == 0)
> +		mode = BQ2415X_MODE_OFF;
> +	else if (mA < 500)
> +		mode = BQ2415X_MODE_NONE;
> +	else if (mA < 1800)
> +		mode = BQ2415X_MODE_HOST_CHARGER;
> +	else
> +		mode = BQ2415X_MODE_DEDICATED_CHARGER;
> +
> +	if (rx51_charger_mode == mode)
> +		return;
> +
> +	rx51_charger_mode = mode;
> +
> +	if (rx51_charger_hook)
> +		rx51_charger_hook(rx51_charger_mode,
> rx51_charger_hook_data); +}
> +
>  static void rx51_charger_set_power(bool on)
>  {
>  	gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
> @@ -277,6 +316,7 @@ static void rx51_charger_set_power(bool
> on)
> 
>  static struct isp1704_charger_data rx51_charger_data = {
>  	.set_power	= rx51_charger_set_power,
> +	.set_current	= rx51_charger_set_current,
>  };
> 
>  static struct platform_device rx51_charger_device = {
> @@ -1017,6 +1057,16 @@ static struct aic3x_pdata
> rx51_aic3x_data2 = { .gpio_reset = 60,
>  };
> 
> +static struct bq2415x_platform_data
> rx51_bq24150a_platform_data = { +	.current_limit = 100,			
/*
> mA */
> +	.weak_battery_voltage = 3400,		/* mV */
> +	.battery_regulation_voltage = 4200,	/* mV */
> +	.charge_current = 650,			/* mA */
> +	.termination_current = 100,		/* mA */
> +	.resistor_sense = 68,			/* m ohm */
> +	.set_mode_hook = &rx51_charger_set_hook,
> +};
> +
>  static struct i2c_board_info __initdata
> rx51_peripherals_i2c_board_info_2[] = { {
>  		I2C_BOARD_INFO("tlv320aic3x", 0x18),
> @@ -1044,7 +1094,11 @@ static struct i2c_board_info __initdata
> rx51_peripherals_i2c_board_info_2[] = { {
>  		I2C_BOARD_INFO("tpa6130a2", 0x60),
>  		.platform_data = &rx51_tpa6130a2_data,
> -	}
> +	},
> +	{
> +		I2C_BOARD_INFO("bq24150a", 0x6b),
> +		.platform_data = &rx51_bq24150a_platform_data,
> +	},
>  };
> 
>  static struct i2c_board_info __initdata
> rx51_peripherals_i2c_board_info_3[] = {

Tony, can you look and review this board patch?

I think that this patch series it the most important for Nokia 
N900, because it finally bringing charging support. And without 
charging battery it very hard to use phone which has power supply 
only from battery.
Tony Lindgren - Sept. 23, 2013, 6:03 p.m.
* Pali Rohár <pali.rohar@gmail.com> [130920 12:29]:
> On Sunday 08 September 2013 10:50:39 Pali Rohár wrote:
> > This patch will register bq24150a charger in RX-51 board data.
> > Patch also adding platform function between isp1704 and
> > bq2415x drivers for detecting charger type.
> > 
> > So finally charging battery on Nokia N900 (RX-51) working
> > automatically without any proprietary Nokia bits in userspace.
...
 
> > @@ -277,6 +316,7 @@ static void rx51_charger_set_power(bool
> > on)
> > 
> >  static struct isp1704_charger_data rx51_charger_data = {
> >  	.set_power	= rx51_charger_set_power,
> > +	.set_current	= rx51_charger_set_current,
> >  };

We want to get rid of the platform data callbacks here,
there no longer any need to keep these under arch/arm.

> Tony, can you look and review this board patch?

Yes, looks like this can all be done in the driver nowadays.
You can use drivers/reset for the set_power. Or if it's really
controlling the regulator, then the regulator framework. The
info can be passed in a .dts file for those.

The .set_current you can do in the driver based on the
compatible flag.
 
> I think that this patch series it the most important for Nokia 
> N900, because it finally bringing charging support. And without 
> charging battery it very hard to use phone which has power supply 
> only from battery.

Right, let's get this driver updated to use the device tree
based init and that way this file is no longer needed.
I would like to $ grep -i grandmom ~/.phonebook | call too :)

I forgot how this charger is wired up, but maybe take a
look at commit d7bf353f (bq24190_charger: Add support for TI
BQ24190 Battery Charger) for the DT parts.

Regards,

Tony
Pali Rohár - Sept. 23, 2013, 7:16 p.m.
Hello,

On Monday 23 September 2013 20:03:00 you wrote:
> * Pali Rohár <pali.rohar@gmail.com> [130920 12:29]:
> > On Sunday 08 September 2013 10:50:39 Pali Rohár wrote:
> > > This patch will register bq24150a charger in RX-51 board
> > > data. Patch also adding platform function between isp1704
> > > and bq2415x drivers for detecting charger type.
> > > 
> > > So finally charging battery on Nokia N900 (RX-51) working
> > > automatically without any proprietary Nokia bits in
> > > userspace.
> 
> ...
> 
> > > @@ -277,6 +316,7 @@ static void
> > > rx51_charger_set_power(bool on)
> > > 
> > >  static struct isp1704_charger_data rx51_charger_data = {
> > >  
> > >  	.set_power	= rx51_charger_set_power,
> > > 
> > > +	.set_current	= rx51_charger_set_current,
> > > 
> > >  };
> 
> We want to get rid of the platform data callbacks here,
> there no longer any need to keep these under arch/arm.
> 

Where to put rx51 board specified functions?
It cannot go to DT, because DT does not support C/ASM code.

> > Tony, can you look and review this board patch?
> 
> Yes, looks like this can all be done in the driver nowadays.
> You can use drivers/reset for the set_power. Or if it's really
> controlling the regulator, then the regulator framework. The
> info can be passed in a .dts file for those.
> 
> The .set_current you can do in the driver based on the
> compatible flag.
> 

It is not as simple as it looks. This is reason why I submited 
this patch long time after I submited bq2415x driver.

Problem is that for rx51 is needed specific function which connect 
to two drivers (bq2415x and isp1704) plus it call specific rx51 
board functions.

Something which cannot be in DT (unless DT support C/ASM code).

> > I think that this patch series it the most important for
> > Nokia N900, because it finally bringing charging support.
> > And without charging battery it very hard to use phone
> > which has power supply only from battery.
> 
> Right, let's get this driver updated to use the device tree
> based init and that way this file is no longer needed.
> I would like to $ grep -i grandmom ~/.phonebook | call too :)
> 

Patches for modem support are being prepared for upstreaming :-) 
so after that it is up to you to create "call" script as you want

> I forgot how this charger is wired up, but maybe take a
> look at commit d7bf353f (bq24190_charger: Add support for TI
> BQ24190 Battery Charger) for the DT parts.
> 
> Regards,
> 
> Tony
Sebastian Reichel - Sept. 23, 2013, 8 p.m.
On Mon, Sep 23, 2013 at 09:16:18PM +0200, Pali Rohár wrote:
> It is not as simple as it looks. This is reason why I submited 
> this patch long time after I submited bq2415x driver.
> 
> Problem is that for rx51 is needed specific function which connect 
> to two drivers (bq2415x and isp1704) plus it call specific rx51 
> board functions.
> 
> Something which cannot be in DT (unless DT support C/ASM code).

mh could isp1704 driver expose the data via the regulator framework?
Then the bq2415x chip can just use the regulator framework. This
should make converting to DT easy (by giving the bq2415x chip the
isp1704 as regulator) and uses existing standard interfaces.

> Patches for modem support are being prepared for upstreaming :-) 
> so after that it is up to you to create "call" script as you want

I'm on it. RFC round will be sent out this week. It seems
hwmod data is already finished, since i didn't get feedback for
that patch :)

-- Sebastian
Pali Rohár - Sept. 23, 2013, 8:06 p.m.
On Monday 23 September 2013 22:00:09 Sebastian Reichel wrote:
> On Mon, Sep 23, 2013 at 09:16:18PM +0200, Pali Rohár wrote:
> > It is not as simple as it looks. This is reason why I
> > submited this patch long time after I submited bq2415x
> > driver.
> > 
> > Problem is that for rx51 is needed specific function which
> > connect to two drivers (bq2415x and isp1704) plus it call
> > specific rx51 board functions.
> > 
> > Something which cannot be in DT (unless DT support C/ASM
> > code).
> 
> mh could isp1704 driver expose the data via the regulator
> framework?

No, isp1704 is power supply driver and export data via power 
supply (sysfs) interface. It is not regulator but charger driver.

> Then the bq2415x chip can just use the regulator
> framework. This should make converting to DT easy (by giving
> the bq2415x chip the isp1704 as regulator) and uses existing
> standard interfaces.
> 
> > Patches for modem support are being prepared for upstreaming
> > :-) so after that it is up to you to create "call" script
> > as you want
> 
> I'm on it. RFC round will be sent out this week. It seems
> hwmod data is already finished, since i didn't get feedback
> for that patch :)
> 
> -- Sebastian
Sebastian Reichel - Sept. 23, 2013, 8:47 p.m.
On Mon, Sep 23, 2013 at 10:06:46PM +0200, Pali Rohár wrote:
> On Monday 23 September 2013 22:00:09 Sebastian Reichel wrote:
> > On Mon, Sep 23, 2013 at 09:16:18PM +0200, Pali Rohár wrote:
> > > It is not as simple as it looks. This is reason why I
> > > submited this patch long time after I submited bq2415x
> > > driver.
> > > 
> > > Problem is that for rx51 is needed specific function which
> > > connect to two drivers (bq2415x and isp1704) plus it call
> > > specific rx51 board functions.
> > > 
> > > Something which cannot be in DT (unless DT support C/ASM
> > > code).
> > 
> > mh could isp1704 driver expose the data via the regulator
> > framework?
> 
> No, isp1704 is power supply driver and export data via power 
> supply (sysfs) interface. It is not regulator but charger driver.

well it does not charge the battery directly, but just provides a
power line with 5 Volt and a specified amount of current to the
system, doesn't it?

From my POV this is a candiate for the regulator framework:

https://www.kernel.org/doc/Documentation/power/regulator/overview.txt

-- Sebastian
Tony Lindgren - Sept. 23, 2013, 11:11 p.m.
* Sebastian Reichel <sre@ring0.de> [130923 13:55]:
> On Mon, Sep 23, 2013 at 10:06:46PM +0200, Pali Rohár wrote:
> > On Monday 23 September 2013 22:00:09 Sebastian Reichel wrote:
> > > On Mon, Sep 23, 2013 at 09:16:18PM +0200, Pali Rohár wrote:
> > > > It is not as simple as it looks. This is reason why I
> > > > submited this patch long time after I submited bq2415x
> > > > driver.
> > > > 
> > > > Problem is that for rx51 is needed specific function which
> > > > connect to two drivers (bq2415x and isp1704) plus it call
> > > > specific rx51 board functions.
> > > > 
> > > > Something which cannot be in DT (unless DT support C/ASM
> > > > code).
> > > 
> > > mh could isp1704 driver expose the data via the regulator
> > > framework?
> > 
> > No, isp1704 is power supply driver and export data via power 
> > supply (sysfs) interface. It is not regulator but charger driver.
> 
> well it does not charge the battery directly, but just provides a
> power line with 5 Volt and a specified amount of current to the
> system, doesn't it?
> 
> From my POV this is a candiate for the regulator framework:
> 
> https://www.kernel.org/doc/Documentation/power/regulator/overview.txt

Yes I think we should be able handle that rail with the
regulator framework.

Regards,

Tony
Pavel Machek - Sept. 24, 2013, 12:05 a.m.
Hi!

> > No, isp1704 is power supply driver and export data via power 
> > supply (sysfs) interface. It is not regulator but charger driver.
> 
> well it does not charge the battery directly, but just provides a
> power line with 5 Volt and a specified amount of current to the
> system, doesn't it?

I don't want to talk about isp1704, but...

LiION charger is just something that provides 4.2V power line with
C/10 current limitation...

(IOW line between regulators and chargers is very very thin, if it
even exists).

									Pavel

            (who did use laboratory power supply as emergency battery charger)
Pali Rohár - Sept. 24, 2013, 5:05 p.m.
On Monday 23 September 2013 22:47:15 Sebastian Reichel wrote:
> On Mon, Sep 23, 2013 at 10:06:46PM +0200, Pali Rohár wrote:
> > On Monday 23 September 2013 22:00:09 Sebastian Reichel wrote:
> > > On Mon, Sep 23, 2013 at 09:16:18PM +0200, Pali Rohár wrote:
> > > > It is not as simple as it looks. This is reason why I
> > > > submited this patch long time after I submited bq2415x
> > > > driver.
> > > > 
> > > > Problem is that for rx51 is needed specific function
> > > > which connect to two drivers (bq2415x and isp1704) plus
> > > > it call specific rx51 board functions.
> > > > 
> > > > Something which cannot be in DT (unless DT support C/ASM
> > > > code).
> > > 
> > > mh could isp1704 driver expose the data via the regulator
> > > framework?
> > 
> > No, isp1704 is power supply driver and export data via power
> > supply (sysfs) interface. It is not regulator but charger
> > driver.
> 
> well it does not charge the battery directly, but just
> provides a power line with 5 Volt and a specified amount of
> current to the system, doesn't it?
> 

No, isp1704 driver is doing fastcharger detection (and then 
export charger type via sysfs power supply) based on musb usb 
events.

Real charging (enabling/disabling and setting properties) is done 
by bq24150a chip which has own power driver bq2415x_charger.

As already wrote this is not simple and this is reason why I sent 
board data and functions only now...

> From my POV this is a candiate for the regulator framework:
> 
> https://www.kernel.org/doc/Documentation/power/regulator/overv
> iew.txt
> 
> -- Sebastian
Sebastian Reichel - Sept. 24, 2013, 8:50 p.m.
Hi,

On Tue, Sep 24, 2013 at 07:05:47PM +0200, Pali Rohár wrote:
> No, isp1704 driver is doing fastcharger detection (and then 
> export charger type via sysfs power supply) based on musb usb 
> events.
> 
> Real charging (enabling/disabling and setting properties) is done 
> by bq24150a chip which has own power driver bq2415x_charger.
> 
> As already wrote this is not simple and this is reason why I sent 
> board data and functions only now...

Yes, I'm aware of this. Technically the isp1704 does not provide the
5 volt line, but for the system as a whole that fact does not really
matter.

The isp1704 can provide its functionality via the regulator API
as a simple regulator device. It provides information about the
power "it can supply".

The bq24150a on the other hand can just use the regulator via the
consumer interface.

The regulator framework provides capabilities for events:
"Regulators can notify consumers of external events"

-- Sebastian

Patch

diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 9c2dd10..a993ffe 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -25,6 +25,7 @@ 
 #include <linux/gpio_keys.h>
 #include <linux/mmc/host.h>
 #include <linux/power/isp1704_charger.h>
+#include <linux/power/bq2415x_charger.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
 #include <linux/platform_data/mtd-onenand-omap2.h>
 
@@ -270,6 +271,44 @@  static struct platform_device rx51_battery_device = {
 	.id	= -1,
 };
 
+static enum bq2415x_mode rx51_charger_mode = BQ2415X_MODE_OFF;
+static void *rx51_charger_hook_data;
+static void (*rx51_charger_hook)(enum bq2415x_mode mode, void *data);
+
+static int rx51_charger_set_hook(
+		void (*hook)(enum bq2415x_mode mode, void *data), void *data)
+{
+	rx51_charger_hook = hook;
+	rx51_charger_hook_data = data;
+	if (rx51_charger_hook)
+		rx51_charger_hook(rx51_charger_mode, rx51_charger_hook_data);
+	return 1;
+}
+
+static void rx51_charger_set_current(int mA)
+{
+	enum bq2415x_mode mode;
+
+	pr_info("RX-51: Charger current limit is %d mA\n", mA);
+
+	if (mA == 0)
+		mode = BQ2415X_MODE_OFF;
+	else if (mA < 500)
+		mode = BQ2415X_MODE_NONE;
+	else if (mA < 1800)
+		mode = BQ2415X_MODE_HOST_CHARGER;
+	else
+		mode = BQ2415X_MODE_DEDICATED_CHARGER;
+
+	if (rx51_charger_mode == mode)
+		return;
+
+	rx51_charger_mode = mode;
+
+	if (rx51_charger_hook)
+		rx51_charger_hook(rx51_charger_mode, rx51_charger_hook_data);
+}
+
 static void rx51_charger_set_power(bool on)
 {
 	gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
@@ -277,6 +316,7 @@  static void rx51_charger_set_power(bool on)
 
 static struct isp1704_charger_data rx51_charger_data = {
 	.set_power	= rx51_charger_set_power,
+	.set_current	= rx51_charger_set_current,
 };
 
 static struct platform_device rx51_charger_device = {
@@ -1017,6 +1057,16 @@  static struct aic3x_pdata rx51_aic3x_data2 = {
 	.gpio_reset = 60,
 };
 
+static struct bq2415x_platform_data rx51_bq24150a_platform_data = {
+	.current_limit = 100,			/* mA */
+	.weak_battery_voltage = 3400,		/* mV */
+	.battery_regulation_voltage = 4200,	/* mV */
+	.charge_current = 650,			/* mA */
+	.termination_current = 100,		/* mA */
+	.resistor_sense = 68,			/* m ohm */
+	.set_mode_hook = &rx51_charger_set_hook,
+};
+
 static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
 	{
 		I2C_BOARD_INFO("tlv320aic3x", 0x18),
@@ -1044,7 +1094,11 @@  static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
 	{
 		I2C_BOARD_INFO("tpa6130a2", 0x60),
 		.platform_data = &rx51_tpa6130a2_data,
-	}
+	},
+	{
+		I2C_BOARD_INFO("bq24150a", 0x6b),
+		.platform_data = &rx51_bq24150a_platform_data,
+	},
 };
 
 static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_3[] = {