| Message ID | 1378630239-10006-5-git-send-email-pali.rohar@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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.
* 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
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
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
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
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
* 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
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)
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
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
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[] = {
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(-)