* [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251
@ 2015-09-11 20:26 Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices Andreas Dannenberg
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
This patch series extends the driver to also support bq24250/bq24251.
The bq24250/251/257 devices have a very similar feature set and are
virtually identical from a control register point of view so it made
sense to extend the existing driver rather than submitting a new driver.
In addition to the new device support the driver is also extended to
allow access to some device features previously hidden. Basic and
potentially dangerous charger config parameters affecting the actual
charging of the Li-Ion battery are only configurable through firmware
rather than sysfs properties. However some newly introduced properties
are exposed through sysfs properties as access to them may be desired
from userspace. For example, it is now possible to manually configure
the maximum current drawn from the input source to accommodate different
chargers (0.5A, 1.5A, 2.0A and so on) based on system knowledge a
userspace application may have rather than rely on the auto-detection
mechanism that may not work in all possible scenarios.
Note that most patches have dependencies on other patches in the series.
v3:
- Dropped the driver/symbol rename patch from v2 due to anticipated
issues with upcoming bq2425x family additions
- Reverted additional mutex coverage for I2C access due to regmap
built-in mutex protection being sufficient
- Removed support for trickle charging due to being a rare/uncommon
use case
- Fixed most checkpatch.pl --strict alignment issues (except where
the line length would exceed 80 chars)
- Fixed an issue with how the return value of gpio_to_desc() was
handled
- Fixed an issue with the definition of bq24257_of_match[]
- Reordered the patch series to put the DT doc changes to the
beginning
v2:
- Aligned DT bindings better with existing "ti,*" charger bindings
- Dropped patch that improperly reported a missing battery as a dead
battery
- Fixed (hopefully, that is -- still waiting for my test platform)
issue with how the private ACPI driver_data used to identify which
bq2425x device to use
- Removed boolean DT/ACPI properties mostly by replacing them with more
intelligent handling in the driver
- Rework/clarification of DT bindings doc
- Renamed/refactored filenames/symbols from bq24257 to bq2425x to
better reflect that multiple devices are covered. Despite initial
hesitation I feel this is a good opportunity for some clean-up as
the driver is still very new in the Kernel so the change should be
low risk. This also addresses one of Andrew Davis' feedback items.
Plus, it makes for a nice alignment with the existing bq2415x_charger
driver.
v1:
- Initial submission
Andreas Dannenberg (10):
dt: power: bq24257-charger: Cover additional devices
power: bq24257: Add basic support for bq24250/bq24251
power: bq24257: Add bit definition for temp sense enable
power: bq24257: Allow manual setting of input current limit
power: bq24257: Add SW-based approach for Power Good determination
power: bq24257: Add over voltage protection setting support
power: bq24257: Add input DPM voltage threshold setting support
power: bq24257: Allow input current limit sysfs access
power: bq24257: Add various device-specific sysfs properties
power: bq24257: Add platform data based initialization
.../devicetree/bindings/power/bq24257.txt | 59 ++-
drivers/power/Kconfig | 5 +-
drivers/power/bq24257_charger.c | 562 +++++++++++++++++++--
include/linux/power/bq24257_charger.h | 30 ++
4 files changed, 606 insertions(+), 50 deletions(-)
create mode 100644 include/linux/power/bq24257_charger.h
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-14 6:11 ` Krzysztof Kozlowski
2015-09-11 20:26 ` [PATCH v3 03/10] power: bq24257: Add bit definition for temp sense enable Andreas Dannenberg
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
Extend the bq24257 charger's device tree documentation to cover the
bq24250 and bq24251 devices as well feature additions.
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
.../devicetree/bindings/power/bq24257.txt | 59 ++++++++++++++++++++--
1 file changed, 54 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
index 5c9d394..4a88c96 100644
--- a/Documentation/devicetree/bindings/power/bq24257.txt
+++ b/Documentation/devicetree/bindings/power/bq24257.txt
@@ -1,21 +1,70 @@
-Binding for TI bq24257 Li-Ion Charger
+Binding for TI bq24250/bq24251/bq24257 Li-Ion Charger
Required properties:
- compatible: Should contain one of the following:
+ * "ti,bq24250"
+ * "ti,bq24251"
* "ti,bq24257"
-- reg: integer, i2c address of the device.
+- reg: integer, i2c address of the device.
+- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can
+ also be defined through the standard interrupt definition properties (see
+ optional properties section below). Only use one method.
- ti,battery-regulation-voltage: integer, maximum charging voltage in uV.
-- ti,charge-current: integer, maximum charging current in uA.
-- ti,termination-current: integer, charge will be terminated when current in
- constant-voltage phase drops below this value (in uA).
+- ti,charge-current: integer, maximum charging current in uA.
+- ti,termination-current: integer, charge will be terminated when current in
+ constant-voltage phase drops below this value (in uA).
+
+Optional properties:
+- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin.
+ This pin is not available on all devices however it should be used if
+ possible as this is the recommended way to obtain the charger's input PG
+ state. If this pin is not specified a software-based approach for PG
+ detection is used.
+- ti,current-limit: The maximum current to be drawn from the charger's input
+ (in uA). If this property is not specified a USB D+/D- signal based charger
+ type detection is used (if available) and the input limit current is set
+ accordingly. If the D+/D- based detection is not available on a given device
+ a default of 500,000 is used (=500mA).
+- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If
+ not specified a default of 6,5000,000 (=6.5V) is used.
+- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic
+ power path management (in uV). If not specified a default of 4,360,000
+ (=4.36V) is used.
+- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety
+ timer is extended (slowed down) by a factor of two. Setting this property
+ to 0 or not providing it will leave the safety timer at its default
+ setting.
+- interrupt-parent: Should be the phandle for the interrupt controller. Use in
+ conjunction with "interrupts" and only in case "stat-gpios" is not used.
+- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in
+ conjunction with "interrupt-parent" and only in case "stat-gpios" is not
+ used.
Example:
bq24257 {
compatible = "ti,bq24257";
reg = <0x6a>;
+ stat-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>;
+ pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
ti,battery-regulation-voltage = <4200000>;
ti,charge-current = <1000000>;
ti,termination-current = <50000>;
};
+
+Example:
+
+bq24250 {
+ compatible = "ti,bq24250";
+ reg = <0x6a>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <16 IRQ_TYPE_EDGE_BOTH>;
+
+ ti,battery-regulation-voltage = <4200000>;
+ ti,charge-current = <500000>;
+ ti,termination-current = <50000>;
+ ti,current-limit = <900000>;
+ ti,ovp-voltage = <9500000>;
+ ti,in-dpm-voltage = <4440000>;
+};
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 02/10] power: bq24257: Add basic support for bq24250/bq24251
[not found] ` <1442003202-1430-1-git-send-email-dannenberg-l0cyMroinI0@public.gmane.org>
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 06/10] power: bq24257: Add over voltage protection setting support Andreas Dannenberg
1 sibling, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andreas Dannenberg
This patch adds basic support for bq24250 and bq24251 which are very
similar to the bq24257 the driver was originally written for. Basic
support means the ability to select a device through Kconfig, DT and
ACPI, an instance variable allowing to check which chip is active, and
the reporting back of the selected device through the
POWER_SUPPLY_PROP_MODEL_NAME power supply sysfs property.
This patch by itself is not sufficient to actually use those two added
devices in a real-world setting due to some feature differences which
are addressed by other patches in this series.
Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
drivers/power/Kconfig | 5 +++--
drivers/power/bq24257_charger.c | 46 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index f8758d6..7ecd9b6 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -397,12 +397,13 @@ config CHARGER_BQ24190
Say Y to enable support for the TI BQ24190 battery charger.
config CHARGER_BQ24257
- tristate "TI BQ24257 battery charger driver"
+ tristate "TI BQ24250/24251/24257 battery charger driver"
depends on I2C
depends on GPIOLIB || COMPILE_TEST
depends on REGMAP_I2C
help
- Say Y to enable support for the TI BQ24257 battery charger.
+ Say Y to enable support for the TI BQ24250, BQ24251, and BQ24257 battery
+ chargers.
config CHARGER_BQ24735
tristate "TI BQ24735 battery charger support"
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 5859bc7..1b3ddfb 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -13,6 +13,10 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
+ * Datasheets:
+ * http://www.ti.com/product/bq24250
+ * http://www.ti.com/product/bq24251
+ * http://www.ti.com/product/bq24257
*/
#include <linux/module.h>
@@ -41,6 +45,18 @@
#define BQ24257_ILIM_SET_DELAY 1000 /* msec */
+enum bq2425x_chip {
+ BQ24250,
+ BQ24251,
+ BQ24257,
+};
+
+static const char *const bq2425x_chip_name[] = {
+ "bq24250",
+ "bq24251",
+ "bq24257",
+};
+
enum bq24257_fields {
F_WD_FAULT, F_WD_EN, F_STAT, F_FAULT, /* REG 1 */
F_RESET, F_IILIMIT, F_EN_STAT, F_EN_TERM, F_CE, F_HZ_MODE, /* REG 2 */
@@ -71,6 +87,8 @@ struct bq24257_device {
struct device *dev;
struct power_supply *charger;
+ enum bq2425x_chip chip;
+
struct regmap *rmap;
struct regmap_field *rmap_fields[F_MAX_FIELDS];
@@ -249,6 +267,10 @@ static int bq24257_power_supply_get_property(struct power_supply *psy,
val->strval = BQ24257_MANUFACTURER;
break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = bq2425x_chip_name[bq->chip];
+ break;
+
case POWER_SUPPLY_PROP_ONLINE:
val->intval = state.power_good;
break;
@@ -569,6 +591,7 @@ static int bq24257_hw_init(struct bq24257_device *bq)
static enum power_supply_property bq24257_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_HEALTH,
@@ -666,6 +689,7 @@ static int bq24257_probe(struct i2c_client *client,
{
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
struct device *dev = &client->dev;
+ const struct acpi_device_id *acpi_id;
struct bq24257_device *bq;
int ret;
int i;
@@ -682,6 +706,18 @@ static int bq24257_probe(struct i2c_client *client,
bq->client = client;
bq->dev = dev;
+ if (ACPI_HANDLE(dev)) {
+ acpi_id = acpi_match_device(dev->driver->acpi_match_table,
+ &client->dev);
+ if (!acpi_id) {
+ dev_err(dev, "Failed to match ACPI device\n");
+ return -ENODEV;
+ }
+ bq->chip = (enum bq2425x_chip)acpi_id->driver_data;
+ } else {
+ bq->chip = (enum bq2425x_chip)id->driver_data;
+ }
+
mutex_init(&bq->lock);
bq->rmap = devm_regmap_init_i2c(client, &bq24257_regmap_config);
@@ -823,19 +859,25 @@ static const struct dev_pm_ops bq24257_pm = {
};
static const struct i2c_device_id bq24257_i2c_ids[] = {
- { "bq24257", 0 },
+ { "bq24250", BQ24250 },
+ { "bq24251", BQ24251 },
+ { "bq24257", BQ24257 },
{},
};
MODULE_DEVICE_TABLE(i2c, bq24257_i2c_ids);
static const struct of_device_id bq24257_of_match[] = {
+ { .compatible = "ti,bq24250", },
+ { .compatible = "ti,bq24251", },
{ .compatible = "ti,bq24257", },
{ },
};
MODULE_DEVICE_TABLE(of, bq24257_of_match);
static const struct acpi_device_id bq24257_acpi_match[] = {
- {"BQ242570", 0},
+ { "BQ242500", BQ24250 },
+ { "BQ242510", BQ24251 },
+ { "BQ242570", BQ24257 },
{},
};
MODULE_DEVICE_TABLE(acpi, bq24257_acpi_match);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 03/10] power: bq24257: Add bit definition for temp sense enable
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 04/10] power: bq24257: Allow manual setting of input current limit Andreas Dannenberg
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
Adding a missing bit definition for the sake of consistency device model
vs. bit field representation. No change in functionality.
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 1b3ddfb..fdfe855 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -63,7 +63,7 @@ enum bq24257_fields {
F_VBAT, F_USB_DET, /* REG 3 */
F_ICHG, F_ITERM, /* REG 4 */
F_LOOP_STATUS, F_LOW_CHG, F_DPDM_EN, F_CE_STATUS, F_VINDPM, /* REG 5 */
- F_X2_TMR_EN, F_TMR, F_SYSOFF, F_TS_STAT, /* REG 6 */
+ F_X2_TMR_EN, F_TMR, F_SYSOFF, F_TS_EN, F_TS_STAT, /* REG 6 */
F_VOVP, F_CLR_VDP, F_FORCE_BATDET, F_FORCE_PTM, /* REG 7 */
F_MAX_FIELDS
@@ -153,6 +153,7 @@ static const struct reg_field bq24257_reg_fields[] = {
[F_X2_TMR_EN] = REG_FIELD(BQ24257_REG_6, 7, 7),
[F_TMR] = REG_FIELD(BQ24257_REG_6, 5, 6),
[F_SYSOFF] = REG_FIELD(BQ24257_REG_6, 4, 4),
+ [F_TS_EN] = REG_FIELD(BQ24257_REG_6, 3, 3),
[F_TS_STAT] = REG_FIELD(BQ24257_REG_6, 0, 2),
/* REG 7 */
[F_VOVP] = REG_FIELD(BQ24257_REG_7, 5, 7),
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 04/10] power: bq24257: Allow manual setting of input current limit
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 03/10] power: bq24257: Add bit definition for temp sense enable Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 05/10] power: bq24257: Add SW-based approach for Power Good determination Andreas Dannenberg
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
A new optional device property called "ti,current-limit" is introduced
to allow disabling the D+/D- USB signal-based charger type auto-
detection algorithm used to set the input current limit and instead to
use a fixed input current limit.
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 102 +++++++++++++++++++++++++++++++---------
1 file changed, 81 insertions(+), 21 deletions(-)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index fdfe855..55e4ee4 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -74,6 +74,7 @@ struct bq24257_init_data {
u8 ichg; /* charge current */
u8 vbat; /* regulation voltage */
u8 iterm; /* termination current */
+ u8 in_ilimit; /* input current limit */
};
struct bq24257_state {
@@ -100,6 +101,8 @@ struct bq24257_device {
struct bq24257_state state;
struct mutex lock; /* protect state data */
+
+ bool in_ilimit_autoset_disable;
};
static bool bq24257_is_volatile_reg(struct device *dev, unsigned int reg)
@@ -188,6 +191,12 @@ static const u32 bq24257_iterm_map[] = {
#define BQ24257_ITERM_MAP_SIZE ARRAY_SIZE(bq24257_iterm_map)
+static const u32 bq24257_iilimit_map[] = {
+ 100000, 150000, 500000, 900000, 1500000, 2000000
+};
+
+#define BQ24257_IILIMIT_MAP_SIZE ARRAY_SIZE(bq24257_iilimit_map)
+
static int bq24257_field_read(struct bq24257_device *bq,
enum bq24257_fields field_id)
{
@@ -479,24 +488,35 @@ static void bq24257_handle_state_change(struct bq24257_device *bq,
old_state = bq->state;
mutex_unlock(&bq->lock);
- if (!new_state->power_good) { /* power removed */
- cancel_delayed_work_sync(&bq->iilimit_setup_work);
-
- /* activate D+/D- port detection algorithm */
- ret = bq24257_field_write(bq, F_DPDM_EN, 1);
- if (ret < 0)
- goto error;
+ /*
+ * Handle BQ2425x state changes observing whether the D+/D- based input
+ * current limit autoset functionality is enabled.
+ */
+ if (!new_state->power_good) {
+ dev_dbg(bq->dev, "Power removed\n");
+ if (!bq->in_ilimit_autoset_disable) {
+ cancel_delayed_work_sync(&bq->iilimit_setup_work);
- reset_iilimit = true;
- } else if (!old_state.power_good) { /* power inserted */
- config_iilimit = true;
- } else if (new_state->fault == FAULT_NO_BAT) { /* battery removed */
- cancel_delayed_work_sync(&bq->iilimit_setup_work);
+ /* activate D+/D- port detection algorithm */
+ ret = bq24257_field_write(bq, F_DPDM_EN, 1);
+ if (ret < 0)
+ goto error;
- reset_iilimit = true;
- } else if (old_state.fault == FAULT_NO_BAT) { /* battery connected */
- config_iilimit = true;
- } else if (new_state->fault == FAULT_TIMER) { /* safety timer expired */
+ reset_iilimit = true;
+ }
+ } else if (!old_state.power_good) {
+ dev_dbg(bq->dev, "Power inserted\n");
+ config_iilimit = !bq->in_ilimit_autoset_disable;
+ } else if (new_state->fault == FAULT_NO_BAT) {
+ dev_warn(bq->dev, "Battery removed\n");
+ if (!bq->in_ilimit_autoset_disable) {
+ cancel_delayed_work_sync(&bq->iilimit_setup_work);
+ reset_iilimit = true;
+ }
+ } else if (old_state.fault == FAULT_NO_BAT) {
+ dev_warn(bq->dev, "Battery connected\n");
+ config_iilimit = !bq->in_ilimit_autoset_disable;
+ } else if (new_state->fault == FAULT_TIMER) {
dev_err(bq->dev, "Safety timer expired! Battery dead?\n");
}
@@ -581,7 +601,16 @@ static int bq24257_hw_init(struct bq24257_device *bq)
bq->state = state;
mutex_unlock(&bq->lock);
- if (!state.power_good)
+ if (bq->in_ilimit_autoset_disable) {
+ dev_dbg(bq->dev, "manually setting iilimit = %d\n",
+ bq->init_data.in_ilimit);
+
+ /* program fixed input current limit */
+ ret = bq24257_field_write(bq, F_IILIMIT,
+ bq->init_data.in_ilimit);
+ if (ret < 0)
+ return ret;
+ } else if (!state.power_good)
/* activate D+/D- detection algorithm */
ret = bq24257_field_write(bq, F_DPDM_EN, 1);
else if (state.fault != FAULT_NO_BAT)
@@ -659,6 +688,7 @@ static int bq24257_fw_probe(struct bq24257_device *bq)
int ret;
u32 property;
+ /* Required properties */
ret = device_property_read_u32(bq->dev, "ti,charge-current", &property);
if (ret < 0)
return ret;
@@ -682,6 +712,24 @@ static int bq24257_fw_probe(struct bq24257_device *bq)
bq->init_data.iterm = bq24257_find_idx(property, bq24257_iterm_map,
BQ24257_ITERM_MAP_SIZE);
+ /* Optional properties. If not provided use reasonable default. */
+ ret = device_property_read_u32(bq->dev, "ti,current-limit",
+ &property);
+ if (ret < 0)
+ /*
+ * Explicitly set a default value which will be needed for
+ * devices that don't support the automatic setting of the input
+ * current limit through the charger type detection mechanism.
+ */
+ bq->init_data.in_ilimit = IILIMIT_500;
+ else {
+ bq->in_ilimit_autoset_disable = true;
+ bq->init_data.in_ilimit =
+ bq24257_find_idx(property,
+ bq24257_iilimit_map,
+ BQ24257_IILIMIT_MAP_SIZE);
+ }
+
return 0;
}
@@ -740,8 +788,6 @@ static int bq24257_probe(struct i2c_client *client,
i2c_set_clientdata(client, bq);
- INIT_DELAYED_WORK(&bq->iilimit_setup_work, bq24257_iilimit_setup_work);
-
if (!dev->platform_data) {
ret = bq24257_fw_probe(bq);
if (ret < 0) {
@@ -752,6 +798,18 @@ static int bq24257_probe(struct i2c_client *client,
return -ENODEV;
}
+ /*
+ * The BQ24250 doesn't support the D+/D- based charger type detection
+ * used for the automatic setting of the input current limit setting so
+ * explicitly disable that feature.
+ */
+ if (bq->chip == BQ24250)
+ bq->in_ilimit_autoset_disable = true;
+
+ if (!bq->in_ilimit_autoset_disable)
+ INIT_DELAYED_WORK(&bq->iilimit_setup_work,
+ bq24257_iilimit_setup_work);
+
/* we can only check Power Good status by probing the PG pin */
ret = bq24257_pg_gpio_probe(bq);
if (ret < 0)
@@ -804,7 +862,8 @@ static int bq24257_remove(struct i2c_client *client)
{
struct bq24257_device *bq = i2c_get_clientdata(client);
- cancel_delayed_work_sync(&bq->iilimit_setup_work);
+ if (!bq->in_ilimit_autoset_disable)
+ cancel_delayed_work_sync(&bq->iilimit_setup_work);
power_supply_unregister(bq->charger);
@@ -819,7 +878,8 @@ static int bq24257_suspend(struct device *dev)
struct bq24257_device *bq = dev_get_drvdata(dev);
int ret = 0;
- cancel_delayed_work_sync(&bq->iilimit_setup_work);
+ if (!bq->in_ilimit_autoset_disable)
+ cancel_delayed_work_sync(&bq->iilimit_setup_work);
/* reset all registers to default (and activate standalone mode) */
ret = bq24257_field_write(bq, F_RESET, 1);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 05/10] power: bq24257: Add SW-based approach for Power Good determination
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
` (2 preceding siblings ...)
2015-09-11 20:26 ` [PATCH v3 04/10] power: bq24257: Allow manual setting of input current limit Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
[not found] ` <1442003202-1430-1-git-send-email-dannenberg-l0cyMroinI0@public.gmane.org>
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
A software-based approach for determining the charger's input voltage
"Power Good" state is introduced for devices like the bq24250 which
don't have a dedicated hardware pin for that purpose. This SW-based
approach is also used for other devices (with dedicated PG pin) as a
fall back solution if that pin is not configured to be used through
"pg-gpios".
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 49 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 55e4ee4..d7488cf 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -103,6 +103,7 @@ struct bq24257_device {
struct mutex lock; /* protect state data */
bool in_ilimit_autoset_disable;
+ bool pg_gpio_disable;
};
static bool bq24257_is_volatile_reg(struct device *dev, unsigned int reg)
@@ -356,7 +357,26 @@ static int bq24257_get_chip_state(struct bq24257_device *bq,
state->fault = ret;
- state->power_good = !gpiod_get_value_cansleep(bq->pg);
+ if (bq->pg_gpio_disable)
+ /*
+ * If we have a chip without a dedicated power-good GPIO or
+ * some other explicit bit that would provide this information
+ * assume the power is good if there is no supply related
+ * fault - and not good otherwise. There is a possibility for
+ * other errors to mask that power in fact is not good but this
+ * is probably the best we can do here.
+ */
+ switch (state->fault) {
+ case FAULT_INPUT_OVP:
+ case FAULT_INPUT_UVLO:
+ case FAULT_INPUT_LDO_LOW:
+ state->power_good = false;
+ break;
+ default:
+ state->power_good = true;
+ }
+ else
+ state->power_good = !gpiod_get_value_cansleep(bq->pg);
return 0;
}
@@ -676,7 +696,7 @@ static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
{
bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
if (IS_ERR(bq->pg)) {
- dev_err(bq->dev, "could not probe PG pin\n");
+ dev_info(bq->dev, "could not probe PG pin\n");
return PTR_ERR(bq->pg);
}
@@ -810,10 +830,27 @@ static int bq24257_probe(struct i2c_client *client,
INIT_DELAYED_WORK(&bq->iilimit_setup_work,
bq24257_iilimit_setup_work);
- /* we can only check Power Good status by probing the PG pin */
- ret = bq24257_pg_gpio_probe(bq);
- if (ret < 0)
- return ret;
+ /*
+ * The BQ24250 doesn't have a dedicated Power Good (PG) pin so we
+ * explicitly disable this feature for this device and instead use
+ * a SW-based approach to determine the PG state.
+ */
+ if (bq->chip == BQ24250)
+ bq->pg_gpio_disable = true;
+
+ /*
+ * For devices that do have a dedicated PG pin go ahead and probe it,
+ * using the SW-based approach as a fall back solution. Note that the
+ * use of the dedicated pin is preferred.
+ */
+ if (!bq->pg_gpio_disable) {
+ ret = bq24257_pg_gpio_probe(bq);
+ if (ret < 0) {
+ dev_info(bq->dev,
+ "using SW-based power-good detection\n");
+ bq->pg_gpio_disable = true;
+ }
+ }
/* reset all registers to defaults */
ret = bq24257_field_write(bq, F_RESET, 1);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 06/10] power: bq24257: Add over voltage protection setting support
[not found] ` <1442003202-1430-1-git-send-email-dannenberg-l0cyMroinI0@public.gmane.org>
2015-09-11 20:26 ` [PATCH v3 02/10] power: bq24257: Add basic support for bq24250/bq24251 Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
1 sibling, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andreas Dannenberg
A new optional device property called "ti,ovp-voltage" is introduced to
allow configuring the input over voltage protection setting.
This commit also adds the basic sysfs support for custom properties
which is being used to allow userspace to read the current ovp-voltage
setting.
Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
drivers/power/bq24257_charger.c | 67 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index d7488cf..47af858 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -75,6 +75,7 @@ struct bq24257_init_data {
u8 vbat; /* regulation voltage */
u8 iterm; /* termination current */
u8 in_ilimit; /* input current limit */
+ u8 vovp; /* over voltage protection voltage */
};
struct bq24257_state {
@@ -198,6 +199,13 @@ static const u32 bq24257_iilimit_map[] = {
#define BQ24257_IILIMIT_MAP_SIZE ARRAY_SIZE(bq24257_iilimit_map)
+static const u32 bq24257_vovp_map[] = {
+ 6000000, 6500000, 7000000, 8000000, 9000000, 9500000, 10000000,
+ 10500000
+};
+
+#define BQ24257_VOVP_MAP_SIZE ARRAY_SIZE(bq24257_vovp_map)
+
static int bq24257_field_read(struct bq24257_device *bq,
enum bq24257_fields field_id)
{
@@ -413,6 +421,17 @@ enum bq24257_in_ilimit {
IILIMIT_NONE,
};
+enum bq24257_vovp {
+ VOVP_6000,
+ VOVP_6500,
+ VOVP_7000,
+ VOVP_8000,
+ VOVP_9000,
+ VOVP_9500,
+ VOVP_10000,
+ VOVP_10500
+};
+
enum bq24257_port_type {
PORT_TYPE_DCP, /* Dedicated Charging Port */
PORT_TYPE_CDP, /* Charging Downstream Port */
@@ -594,7 +613,8 @@ static int bq24257_hw_init(struct bq24257_device *bq)
} init_data[] = {
{F_ICHG, bq->init_data.ichg},
{F_VBAT, bq->init_data.vbat},
- {F_ITERM, bq->init_data.iterm}
+ {F_ITERM, bq->init_data.iterm},
+ {F_VOVP, bq->init_data.vovp},
};
/*
@@ -664,6 +684,28 @@ static const struct power_supply_desc bq24257_power_supply_desc = {
.get_property = bq24257_power_supply_get_property,
};
+static ssize_t bq24257_show_ovp_voltage(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct power_supply *psy = dev_get_drvdata(dev);
+ struct bq24257_device *bq = power_supply_get_drvdata(psy);
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
+ bq24257_vovp_map[bq->init_data.vovp]);
+}
+
+static DEVICE_ATTR(ovp_voltage, S_IRUGO, bq24257_show_ovp_voltage, NULL);
+
+static struct attribute *bq24257_charger_attr[] = {
+ &dev_attr_ovp_voltage.attr,
+ NULL,
+};
+
+static const struct attribute_group bq24257_attr_group = {
+ .attrs = bq24257_charger_attr,
+};
+
static int bq24257_power_supply_init(struct bq24257_device *bq)
{
struct power_supply_config psy_cfg = { .drv_data = bq, };
@@ -750,6 +792,15 @@ static int bq24257_fw_probe(struct bq24257_device *bq)
BQ24257_IILIMIT_MAP_SIZE);
}
+ ret = device_property_read_u32(bq->dev, "ti,ovp-voltage",
+ &property);
+ if (ret < 0)
+ bq->init_data.vovp = VOVP_6500;
+ else
+ bq->init_data.vovp = bq24257_find_idx(property,
+ bq24257_vovp_map,
+ BQ24257_VOVP_MAP_SIZE);
+
return 0;
}
@@ -889,10 +940,18 @@ static int bq24257_probe(struct i2c_client *client,
return ret;
ret = bq24257_power_supply_init(bq);
- if (ret < 0)
+ if (ret < 0) {
dev_err(dev, "Failed to register power supply\n");
+ return ret;
+ }
- return ret;
+ ret = sysfs_create_group(&bq->charger->dev.kobj, &bq24257_attr_group);
+ if (ret < 0) {
+ dev_err(dev, "Can't create sysfs entries\n");
+ return ret;
+ }
+
+ return 0;
}
static int bq24257_remove(struct i2c_client *client)
@@ -902,6 +961,8 @@ static int bq24257_remove(struct i2c_client *client)
if (!bq->in_ilimit_autoset_disable)
cancel_delayed_work_sync(&bq->iilimit_setup_work);
+ sysfs_remove_group(&bq->charger->dev.kobj, &bq24257_attr_group);
+
power_supply_unregister(bq->charger);
bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 07/10] power: bq24257: Add input DPM voltage threshold setting support
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
` (4 preceding siblings ...)
[not found] ` <1442003202-1430-1-git-send-email-dannenberg-l0cyMroinI0@public.gmane.org>
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 08/10] power: bq24257: Allow input current limit sysfs access Andreas Dannenberg
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
A new optional device property called "ti,in-dpm-voltage" is introduced
to allow configuring the input voltage threshold for the devices'
dynamic power path management (DPM) feature. In short, it can be used to
prevent the input voltage from dropping below a certain value as current
is drawn to charge the battery or supply the system.
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 44 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 47af858..84a8945 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -76,6 +76,7 @@ struct bq24257_init_data {
u8 iterm; /* termination current */
u8 in_ilimit; /* input current limit */
u8 vovp; /* over voltage protection voltage */
+ u8 vindpm; /* VDMP input threshold voltage */
};
struct bq24257_state {
@@ -206,6 +207,13 @@ static const u32 bq24257_vovp_map[] = {
#define BQ24257_VOVP_MAP_SIZE ARRAY_SIZE(bq24257_vovp_map)
+static const u32 bq24257_vindpm_map[] = {
+ 4200000, 4280000, 4360000, 4440000, 4520000, 4600000, 4680000,
+ 4760000
+};
+
+#define BQ24257_VINDPM_MAP_SIZE ARRAY_SIZE(bq24257_vindpm_map)
+
static int bq24257_field_read(struct bq24257_device *bq,
enum bq24257_fields field_id)
{
@@ -432,6 +440,17 @@ enum bq24257_vovp {
VOVP_10500
};
+enum bq24257_vindpm {
+ VINDPM_4200,
+ VINDPM_4280,
+ VINDPM_4360,
+ VINDPM_4440,
+ VINDPM_4520,
+ VINDPM_4600,
+ VINDPM_4680,
+ VINDPM_4760
+};
+
enum bq24257_port_type {
PORT_TYPE_DCP, /* Dedicated Charging Port */
PORT_TYPE_CDP, /* Charging Downstream Port */
@@ -615,6 +634,7 @@ static int bq24257_hw_init(struct bq24257_device *bq)
{F_VBAT, bq->init_data.vbat},
{F_ITERM, bq->init_data.iterm},
{F_VOVP, bq->init_data.vovp},
+ {F_VINDPM, bq->init_data.vindpm},
};
/*
@@ -648,6 +668,7 @@ static int bq24257_hw_init(struct bq24257_device *bq)
/* program fixed input current limit */
ret = bq24257_field_write(bq, F_IILIMIT,
bq->init_data.in_ilimit);
+
if (ret < 0)
return ret;
} else if (!state.power_good)
@@ -695,10 +716,23 @@ static ssize_t bq24257_show_ovp_voltage(struct device *dev,
bq24257_vovp_map[bq->init_data.vovp]);
}
+static ssize_t bq24257_show_in_dpm_voltage(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct power_supply *psy = dev_get_drvdata(dev);
+ struct bq24257_device *bq = power_supply_get_drvdata(psy);
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
+ bq24257_vindpm_map[bq->init_data.vindpm]);
+}
+
static DEVICE_ATTR(ovp_voltage, S_IRUGO, bq24257_show_ovp_voltage, NULL);
+static DEVICE_ATTR(in_dpm_voltage, S_IRUGO, bq24257_show_in_dpm_voltage, NULL);
static struct attribute *bq24257_charger_attr[] = {
&dev_attr_ovp_voltage.attr,
+ &dev_attr_in_dpm_voltage.attr,
NULL,
};
@@ -801,6 +835,16 @@ static int bq24257_fw_probe(struct bq24257_device *bq)
bq24257_vovp_map,
BQ24257_VOVP_MAP_SIZE);
+ ret = device_property_read_u32(bq->dev, "ti,in-dpm-voltage",
+ &property);
+ if (ret < 0)
+ bq->init_data.vindpm = VINDPM_4360;
+ else
+ bq->init_data.vindpm =
+ bq24257_find_idx(property,
+ bq24257_vindpm_map,
+ BQ24257_VINDPM_MAP_SIZE);
+
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 08/10] power: bq24257: Allow input current limit sysfs access
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
` (5 preceding siblings ...)
2015-09-11 20:26 ` [PATCH v3 07/10] power: bq24257: Add input DPM voltage threshold " Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 09/10] power: bq24257: Add various device-specific sysfs properties Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 10/10] power: bq24257: Add platform data based initialization Andreas Dannenberg
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
This patch allows reading (and writing, if the D+/D- USB signal-based
charger type detection is disabled) of the input current limit through
the POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT sysfs property. This allows
userspace to see what charger was detected and to re-configure the
maximum current drawn from the external supply at runtime based on
system-level knowledge or user input.
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 67 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 84a8945..517a522 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -265,6 +265,42 @@ enum bq24257_fault {
FAULT_INPUT_LDO_LOW,
};
+static int bq24257_get_input_current_limit(struct bq24257_device *bq,
+ union power_supply_propval *val)
+{
+ int ret;
+
+ ret = bq24257_field_read(bq, F_IILIMIT);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * The "External ILIM" and "Production & Test" modes are not exposed
+ * through this driver and not being covered by the lookup table.
+ * Should such a mode have become active let's return an error rather
+ * than exceeding the bounds of the lookup table and returning
+ * garbage.
+ */
+ if (ret >= BQ24257_IILIMIT_MAP_SIZE)
+ return -ENODATA;
+
+ val->intval = bq24257_iilimit_map[ret];
+
+ return 0;
+}
+
+static int bq24257_set_input_current_limit(struct bq24257_device *bq,
+ const union power_supply_propval *val)
+{
+ if (!bq->in_ilimit_autoset_disable)
+ return -EPERM;
+
+ return bq24257_field_write(bq, F_IILIMIT,
+ bq24257_find_idx(val->intval,
+ bq24257_iilimit_map,
+ BQ24257_IILIMIT_MAP_SIZE));
+}
+
static int bq24257_power_supply_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -349,6 +385,9 @@ static int bq24257_power_supply_get_property(struct power_supply *psy,
val->intval = bq24257_iterm_map[bq->init_data.iterm];
break;
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return bq24257_get_input_current_limit(bq, val);
+
default:
return -EINVAL;
}
@@ -356,6 +395,31 @@ static int bq24257_power_supply_get_property(struct power_supply *psy,
return 0;
}
+static int bq24257_power_supply_set_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ const union power_supply_propval *val)
+{
+ struct bq24257_device *bq = power_supply_get_drvdata(psy);
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return bq24257_set_input_current_limit(bq, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int bq24257_power_supply_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return true;
+ default:
+ return false;
+ }
+}
+
static int bq24257_get_chip_state(struct bq24257_device *bq,
struct bq24257_state *state)
{
@@ -691,6 +755,7 @@ static enum power_supply_property bq24257_power_supply_props[] = {
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
};
static char *bq24257_charger_supplied_to[] = {
@@ -703,6 +768,8 @@ static const struct power_supply_desc bq24257_power_supply_desc = {
.properties = bq24257_power_supply_props,
.num_properties = ARRAY_SIZE(bq24257_power_supply_props),
.get_property = bq24257_power_supply_get_property,
+ .set_property = bq24257_power_supply_set_property,
+ .property_is_writeable = bq24257_power_supply_property_is_writeable,
};
static ssize_t bq24257_show_ovp_voltage(struct device *dev,
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 09/10] power: bq24257: Add various device-specific sysfs properties
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
` (6 preceding siblings ...)
2015-09-11 20:26 ` [PATCH v3 08/10] power: bq24257: Allow input current limit sysfs access Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 10/10] power: bq24257: Add platform data based initialization Andreas Dannenberg
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
This patch adds support to enable/disable some device specific features
through device properties at boot time or sysfs properties at runtime
depending on whether they should be accessible during runtime or not.
The use of those features is be optional however they might be helpful
in certain scenarios:
* High-impedance mode enable/disable (through sysfs r/w)
* Sysoff enable/disable (through sysfs r/w)
* 2X Timer enable/disable (through "ti,safety-timer-2x-enable" dev
property)
If not explicitly modified those settings are left at their device-
specific power-on defaults. Refer to the respectice device datasheets
for more information:
http://www.ti.com/product/bq24250
http://www.ti.com/product/bq24251
http://www.ti.com/product/bq24257
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 67 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 517a522..6d73736 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -77,6 +77,7 @@ struct bq24257_init_data {
u8 in_ilimit; /* input current limit */
u8 vovp; /* over voltage protection voltage */
u8 vindpm; /* VDMP input threshold voltage */
+ bool timer2x_enable; /* slow down safety timer by 2x */
};
struct bq24257_state {
@@ -699,6 +700,7 @@ static int bq24257_hw_init(struct bq24257_device *bq)
{F_ITERM, bq->init_data.iterm},
{F_VOVP, bq->init_data.vovp},
{F_VINDPM, bq->init_data.vindpm},
+ {F_X2_TMR_EN, bq->init_data.timer2x_enable},
};
/*
@@ -794,12 +796,70 @@ static ssize_t bq24257_show_in_dpm_voltage(struct device *dev,
bq24257_vindpm_map[bq->init_data.vindpm]);
}
+static ssize_t bq24257_sysfs_show_enable(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct power_supply *psy = dev_get_drvdata(dev);
+ struct bq24257_device *bq = power_supply_get_drvdata(psy);
+ int ret;
+
+ if (strcmp(attr->attr.name, "high_impedance_enable") == 0)
+ ret = bq24257_field_read(bq, F_HZ_MODE);
+ else if (strcmp(attr->attr.name, "sysoff_enable") == 0)
+ ret = bq24257_field_read(bq, F_SYSOFF);
+ else if (strcmp(attr->attr.name, "timer2x_enable") == 0)
+ ret = bq->init_data.timer2x_enable;
+ else
+ return -EINVAL;
+
+ if (ret < 0)
+ return ret;
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
+}
+
+static ssize_t bq24257_sysfs_set_enable(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct power_supply *psy = dev_get_drvdata(dev);
+ struct bq24257_device *bq = power_supply_get_drvdata(psy);
+ long val;
+ int ret;
+
+ if (kstrtol(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ if (strcmp(attr->attr.name, "high_impedance_enable") == 0)
+ ret = bq24257_field_write(bq, F_HZ_MODE, val);
+ else if (strcmp(attr->attr.name, "sysoff_enable") == 0)
+ ret = bq24257_field_write(bq, F_SYSOFF, val);
+ else
+ return -EINVAL;
+
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
static DEVICE_ATTR(ovp_voltage, S_IRUGO, bq24257_show_ovp_voltage, NULL);
static DEVICE_ATTR(in_dpm_voltage, S_IRUGO, bq24257_show_in_dpm_voltage, NULL);
+static DEVICE_ATTR(high_impedance_enable, S_IWUSR | S_IRUGO,
+ bq24257_sysfs_show_enable, bq24257_sysfs_set_enable);
+static DEVICE_ATTR(sysoff_enable, S_IWUSR | S_IRUGO,
+ bq24257_sysfs_show_enable, bq24257_sysfs_set_enable);
+static DEVICE_ATTR(timer2x_enable, S_IRUGO,
+ bq24257_sysfs_show_enable, NULL);
static struct attribute *bq24257_charger_attr[] = {
&dev_attr_ovp_voltage.attr,
&dev_attr_in_dpm_voltage.attr,
+ &dev_attr_high_impedance_enable.attr,
+ &dev_attr_sysoff_enable.attr,
+ &dev_attr_timer2x_enable.attr,
NULL,
};
@@ -912,6 +972,13 @@ static int bq24257_fw_probe(struct bq24257_device *bq)
bq24257_vindpm_map,
BQ24257_VINDPM_MAP_SIZE);
+ ret = device_property_read_u32(bq->dev, "ti,safety-timer-2x-enable",
+ &property);
+ if (ret < 0)
+ bq->init_data.timer2x_enable = false;
+ else
+ bq->init_data.timer2x_enable = property;
+
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 10/10] power: bq24257: Add platform data based initialization
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
` (7 preceding siblings ...)
2015-09-11 20:26 ` [PATCH v3 09/10] power: bq24257: Add various device-specific sysfs properties Andreas Dannenberg
@ 2015-09-11 20:26 ` Andreas Dannenberg
8 siblings, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-11 20:26 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Krzysztof Kozlowski
Cc: Ramakrishna Pallala, linux-pm, devicetree, Andreas Dannenberg
The patch adds a way to setup and initialize the device through the use
of platform data with configuration options equivalent to when using
device firmware (DT or ACPI) for systems where this is not available.
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
drivers/power/bq24257_charger.c | 119 ++++++++++++++++++++++++++++++----
include/linux/power/bq24257_charger.h | 30 +++++++++
2 files changed, 138 insertions(+), 11 deletions(-)
create mode 100644 include/linux/power/bq24257_charger.h
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 6d73736..6d16d60 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -27,10 +27,13 @@
#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
+#include <linux/gpio.h>
#include <linux/acpi.h>
#include <linux/of.h>
+#include <linux/power/bq24257_charger.h>
+
#define BQ24257_REG_1 0x00
#define BQ24257_REG_2 0x01
#define BQ24257_REG_3 0x02
@@ -884,28 +887,114 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
static int bq24257_irq_probe(struct bq24257_device *bq)
{
+ struct bq24257_platform_data *pdata = bq->client->dev.platform_data;
struct gpio_desc *stat_irq;
+ int ret;
+
+ if (!pdata) {
+ stat_irq = devm_gpiod_get_index(bq->dev, BQ24257_STAT_IRQ, 0,
+ GPIOD_IN);
+ if (IS_ERR(stat_irq)) {
+ dev_err(bq->dev, "could not probe stat_irq pin\n");
+ return PTR_ERR(stat_irq);
+ }
+ } else {
+ if (!gpio_is_valid(pdata->stat_gpio)) {
+ dev_err(bq->dev, "invalid stat_irq pin\n");
+ return -EINVAL;
+ }
- stat_irq = devm_gpiod_get_index(bq->dev, BQ24257_STAT_IRQ, 0, GPIOD_IN);
- if (IS_ERR(stat_irq)) {
- dev_err(bq->dev, "could not probe stat_irq pin\n");
- return PTR_ERR(stat_irq);
+ ret = devm_gpio_request_one(bq->dev, pdata->stat_gpio,
+ GPIOD_IN, BQ24257_STAT_IRQ);
+ if (ret) {
+ dev_err(bq->dev, "stat_irq pin request failed\n");
+ return ret;
+ }
+
+ stat_irq = gpio_to_desc(pdata->stat_gpio);
+ if (!stat_irq) {
+ dev_err(bq->dev,
+ "could not get stat_irq pin descriptor\n");
+ return -EINVAL;
+ }
}
+ dev_dbg(bq->dev, "probed stat_irq pin = %d", desc_to_gpio(stat_irq));
+
return gpiod_to_irq(stat_irq);
}
static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
{
- bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
- if (IS_ERR(bq->pg)) {
- dev_info(bq->dev, "could not probe PG pin\n");
- return PTR_ERR(bq->pg);
+ struct bq24257_platform_data *pdata = bq->client->dev.platform_data;
+ int ret;
+
+ if (!pdata) {
+ bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0,
+ GPIOD_IN);
+ if (IS_ERR(bq->pg)) {
+ dev_info(bq->dev, "could not probe PG pin\n");
+ return PTR_ERR(bq->pg);
+ }
+ } else {
+ if (!gpio_is_valid(pdata->pg_gpio)) {
+ dev_info(bq->dev, "invalid PG pin\n");
+ return -EINVAL;
+ }
+
+ ret = devm_gpio_request_one(bq->dev, pdata->pg_gpio,
+ GPIOD_IN, BQ24257_PG_GPIO);
+ if (ret) {
+ dev_info(bq->dev, "PG pin request failed\n");
+ return ret;
+ }
+
+ bq->pg = gpio_to_desc(pdata->pg_gpio);
+ if (!bq->pg) {
+ dev_info(bq->dev, "could not get PG pin descriptor\n");
+ return -EINVAL;
+ }
}
+ dev_dbg(bq->dev, "probed PG pin = %d", desc_to_gpio(bq->pg));
+
return 0;
}
+static void bq24257_pdata_probe(struct bq24257_device *bq,
+ struct bq24257_platform_data *pdata)
+{
+ bq->init_data.ichg = bq24257_find_idx(pdata->ichg,
+ bq24257_ichg_map,
+ BQ24257_ICHG_MAP_SIZE);
+
+ bq->init_data.vbat = bq24257_find_idx(pdata->vbat,
+ bq24257_vbat_map,
+ BQ24257_VBAT_MAP_SIZE);
+
+ bq->init_data.iterm = bq24257_find_idx(pdata->iterm,
+ bq24257_iterm_map,
+ BQ24257_ITERM_MAP_SIZE);
+
+ bq->init_data.in_ilimit = bq24257_find_idx(pdata->in_ilimit,
+ bq24257_iilimit_map,
+ BQ24257_IILIMIT_MAP_SIZE);
+
+ bq->init_data.vovp = bq24257_find_idx(pdata->vovp,
+ bq24257_vovp_map,
+ BQ24257_VOVP_MAP_SIZE);
+
+ bq->init_data.vindpm = bq24257_find_idx(pdata->vindpm,
+ bq24257_vindpm_map,
+ BQ24257_VINDPM_MAP_SIZE);
+
+ bq->init_data.timer2x_enable = pdata->timer2x_enable;
+
+ bq->in_ilimit_autoset_disable = pdata->in_ilimit_autoset_disable;
+
+ bq->pg_gpio_disable = pdata->pg_gpio_disable;
+}
+
static int bq24257_fw_probe(struct bq24257_device *bq)
{
int ret;
@@ -988,6 +1077,7 @@ static int bq24257_probe(struct i2c_client *client,
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
struct device *dev = &client->dev;
const struct acpi_device_id *acpi_id;
+ struct bq24257_platform_data *pdata = client->dev.platform_data;
struct bq24257_device *bq;
int ret;
int i;
@@ -1037,14 +1127,15 @@ static int bq24257_probe(struct i2c_client *client,
i2c_set_clientdata(client, bq);
- if (!dev->platform_data) {
+ if (!pdata) {
ret = bq24257_fw_probe(bq);
if (ret < 0) {
dev_err(dev, "Cannot read device properties.\n");
return ret;
}
} else {
- return -ENODEV;
+ dev_dbg(dev, "init using platform data\n");
+ bq24257_pdata_probe(bq, pdata);
}
/*
@@ -1101,7 +1192,13 @@ static int bq24257_probe(struct i2c_client *client,
return ret;
}
- if (client->irq <= 0)
+ /*
+ * When using device firmware we either take the IRQ specified directly
+ * or probe for a pin named BQ24257_STAT_IRQ. In case of using platform
+ * data always run the IRQ pin probe function to finish IRQ and GPIO
+ * setup based on the stat pin specified in the platform data.
+ */
+ if (client->irq <= 0 || pdata)
client->irq = bq24257_irq_probe(bq);
if (client->irq < 0) {
diff --git a/include/linux/power/bq24257_charger.h b/include/linux/power/bq24257_charger.h
new file mode 100644
index 0000000..7fc505b
--- /dev/null
+++ b/include/linux/power/bq24257_charger.h
@@ -0,0 +1,30 @@
+/*
+ * Platform data for the TI bq24257 battery charger driver.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _BQ24257_CHARGER_H_
+#define _BQ24257_CHARGER_H_
+
+#include <asm/types.h>
+#include <linux/types.h>
+
+struct bq24257_platform_data {
+ u32 ichg; /* charge current (uA) */
+ u32 vbat; /* regulation voltage (uV) */
+ u32 iterm; /* termination current (uA) */
+ u32 in_ilimit; /* input current limit (uA) */
+ u32 vovp; /* over voltage protection voltage (uV) */
+ u32 vindpm; /* VDMP input threshold voltage (uV) */
+ bool in_ilimit_autoset_disable; /* auto-detect of input current limit */
+ bool pg_gpio_disable; /* use of a dedicated pin for Power Good */
+ bool timer2x_enable; /* slow down safety timer by 2x */
+
+ int stat_gpio; /* stat_int (interrupt) pin */
+ int pg_gpio; /* power good pin */
+};
+
+#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-11 20:26 ` [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices Andreas Dannenberg
@ 2015-09-14 6:11 ` Krzysztof Kozlowski
2015-09-14 13:54 ` Andreas Dannenberg
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-14 6:11 UTC (permalink / raw)
To: Andreas Dannenberg, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, Laurentiu Palcu
Cc: Ramakrishna Pallala, linux-pm, devicetree
On 12.09.2015 05:26, Andreas Dannenberg wrote:
> Extend the bq24257 charger's device tree documentation to cover the
> bq24250 and bq24251 devices as well feature additions.
>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> ---
> .../devicetree/bindings/power/bq24257.txt | 59 ++++++++++++++++++++--
> 1 file changed, 54 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
> index 5c9d394..4a88c96 100644
> --- a/Documentation/devicetree/bindings/power/bq24257.txt
> +++ b/Documentation/devicetree/bindings/power/bq24257.txt
> @@ -1,21 +1,70 @@
> -Binding for TI bq24257 Li-Ion Charger
> +Binding for TI bq24250/bq24251/bq24257 Li-Ion Charger
>
> Required properties:
> - compatible: Should contain one of the following:
> + * "ti,bq24250"
> + * "ti,bq24251"
> * "ti,bq24257"
> -- reg: integer, i2c address of the device.
> +- reg: integer, i2c address of the device.
> +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can
> + also be defined through the standard interrupt definition properties (see
> + optional properties section below). Only use one method.
> - ti,battery-regulation-voltage: integer, maximum charging voltage in uV.
> -- ti,charge-current: integer, maximum charging current in uA.
> -- ti,termination-current: integer, charge will be terminated when current in
> - constant-voltage phase drops below this value (in uA).
> +- ti,charge-current: integer, maximum charging current in uA.
> +- ti,termination-current: integer, charge will be terminated when current in
> + constant-voltage phase drops below this value (in uA).
> +
> +Optional properties:
> +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin.
> + This pin is not available on all devices however it should be used if
> + possible as this is the recommended way to obtain the charger's input PG
> + state. If this pin is not specified a software-based approach for PG
> + detection is used.
> +- ti,current-limit: The maximum current to be drawn from the charger's input
> + (in uA). If this property is not specified a USB D+/D- signal based charger
> + type detection is used (if available) and the input limit current is set
> + accordingly. If the D+/D- based detection is not available on a given device
> + a default of 500,000 is used (=500mA).
> +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If
> + not specified a default of 6,5000,000 (=6.5V) is used.
> +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic
> + power path management (in uV). If not specified a default of 4,360,000
> + (=4.36V) is used.
> +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety
> + timer is extended (slowed down) by a factor of two. Setting this property
> + to 0 or not providing it will leave the safety timer at its default
> + setting.
Now I spotted the difference in this binding from previous emails.
Previously you made it as a boolean property. Now it is a integer
property for a boolean value. It is unusual. If you expect a need (or a
possibility of need) of:
1. extending,
2. overriding,
3. using similar (extended) property in different drivers,
then it should be a real value, like:
1. ti,safety-timer: integer, in seconds
2. ti,safety-timer-ratio/multiplier: integer, supported values are: 0
(use default) or 2 (slow down by factor of 2)
It is unusual and not obvious to use a integer value for boolean strict
property. With current solution you can actually only override it.
Extending is possible but would look really weird, like:
ti,safety-timer-2x-enable = <4>; /* Heh? 4x or (4*2)x ??? */
To add even more confusion: why this property has to be configured in
Device Tree? How it is related to the hardware? It rather looks like a
job for sysfs.
Best regards,
Krzysztof
> +- interrupt-parent: Should be the phandle for the interrupt controller. Use in
> + conjunction with "interrupts" and only in case "stat-gpios" is not used.
> +- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in
> + conjunction with "interrupt-parent" and only in case "stat-gpios" is not
> + used.
>
> Example:
>
> bq24257 {
> compatible = "ti,bq24257";
> reg = <0x6a>;
> + stat-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>;
> + pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>
> ti,battery-regulation-voltage = <4200000>;
> ti,charge-current = <1000000>;
> ti,termination-current = <50000>;
> };
> +
> +Example:
> +
> +bq24250 {
> + compatible = "ti,bq24250";
> + reg = <0x6a>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <16 IRQ_TYPE_EDGE_BOTH>;
> +
> + ti,battery-regulation-voltage = <4200000>;
> + ti,charge-current = <500000>;
> + ti,termination-current = <50000>;
> + ti,current-limit = <900000>;
> + ti,ovp-voltage = <9500000>;
> + ti,in-dpm-voltage = <4440000>;
> +};
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-14 6:11 ` Krzysztof Kozlowski
@ 2015-09-14 13:54 ` Andreas Dannenberg
2015-09-15 7:56 ` Krzysztof Kozlowski
0 siblings, 1 reply; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-14 13:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Ramakrishna Pallala, linux-pm, devicetree
On Mon, Sep 14, 2015 at 03:11:32PM +0900, Krzysztof Kozlowski wrote:
> On 12.09.2015 05:26, Andreas Dannenberg wrote:
> > Extend the bq24257 charger's device tree documentation to cover the
> > bq24250 and bq24251 devices as well feature additions.
> >
> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> > ---
> > .../devicetree/bindings/power/bq24257.txt | 59 ++++++++++++++++++++--
> > 1 file changed, 54 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
> > index 5c9d394..4a88c96 100644
> > --- a/Documentation/devicetree/bindings/power/bq24257.txt
> > +++ b/Documentation/devicetree/bindings/power/bq24257.txt
> > @@ -1,21 +1,70 @@
> > -Binding for TI bq24257 Li-Ion Charger
> > +Binding for TI bq24250/bq24251/bq24257 Li-Ion Charger
> >
> > Required properties:
> > - compatible: Should contain one of the following:
> > + * "ti,bq24250"
> > + * "ti,bq24251"
> > * "ti,bq24257"
> > -- reg: integer, i2c address of the device.
> > +- reg: integer, i2c address of the device.
> > +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can
> > + also be defined through the standard interrupt definition properties (see
> > + optional properties section below). Only use one method.
> > - ti,battery-regulation-voltage: integer, maximum charging voltage in uV.
> > -- ti,charge-current: integer, maximum charging current in uA.
> > -- ti,termination-current: integer, charge will be terminated when current in
> > - constant-voltage phase drops below this value (in uA).
> > +- ti,charge-current: integer, maximum charging current in uA.
> > +- ti,termination-current: integer, charge will be terminated when current in
> > + constant-voltage phase drops below this value (in uA).
> > +
> > +Optional properties:
> > +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin.
> > + This pin is not available on all devices however it should be used if
> > + possible as this is the recommended way to obtain the charger's input PG
> > + state. If this pin is not specified a software-based approach for PG
> > + detection is used.
> > +- ti,current-limit: The maximum current to be drawn from the charger's input
> > + (in uA). If this property is not specified a USB D+/D- signal based charger
> > + type detection is used (if available) and the input limit current is set
> > + accordingly. If the D+/D- based detection is not available on a given device
> > + a default of 500,000 is used (=500mA).
> > +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If
> > + not specified a default of 6,5000,000 (=6.5V) is used.
> > +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic
> > + power path management (in uV). If not specified a default of 4,360,000
> > + (=4.36V) is used.
> > +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety
> > + timer is extended (slowed down) by a factor of two. Setting this property
> > + to 0 or not providing it will leave the safety timer at its default
> > + setting.
>
> Now I spotted the difference in this binding from previous emails.
> Previously you made it as a boolean property. Now it is a integer
> property for a boolean value. It is unusual. If you expect a need (or a
> possibility of need) of:
> 1. extending,
> 2. overriding,
> 3. using similar (extended) property in different drivers,
> then it should be a real value, like:
>
> 1. ti,safety-timer: integer, in seconds
Hi Krzysztof,
I thought about this as well because the device actually allows
configuring different safety timer intervals (currently not exposed) and
it appears that the "2x enable" could be factored in there however it's
not as easy as it seems. The "2x enable" is a dedicated HW feature
that's only being used in some states of the devices and with that does
not generally extend the safety timer under all conditions.
> 2. ti,safety-timer-ratio/multiplier: integer, supported values are: 0
> (use default) or 2 (slow down by factor of 2)
>
> It is unusual and not obvious to use a integer value for boolean strict
> property. With current solution you can actually only override it.
Yes that was the sole reason behind using integer instead of boolean.
The boolean "character" of this property was left intact because of the
direct mapping to a device HW feature (see below). But I can certainly
change this to ti,safety-timer-multiplier and use 0/2. This avoids the
inaccuracy as explained above with just using ti,safety-timer (the
corresponding underlying device property should be separate and could be
exposed if ever needed) and still leaves room for this property to grow
which may come in handy later.
> Extending is possible but would look really weird, like:
> ti,safety-timer-2x-enable = <4>; /* Heh? 4x or (4*2)x ??? */
>
> To add even more confusion: why this property has to be configured in
> Device Tree? How it is related to the hardware? It rather looks like a
> job for sysfs.
This property maps directly to a corresponding bit in the device
register map so yes it is related to the HW and IMHO should be
applicable to DT. The reason for not providing write access to that bit
via sysfs is the same as this driver doesn't have write access to other
"critical" properties such as charge current/voltages.
Regards,
--
Andreas Dannenberg
Texas Instruments Inc
>
> Best regards,
> Krzysztof
>
> > +- interrupt-parent: Should be the phandle for the interrupt controller. Use in
> > + conjunction with "interrupts" and only in case "stat-gpios" is not used.
> > +- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in
> > + conjunction with "interrupt-parent" and only in case "stat-gpios" is not
> > + used.
> >
> > Example:
> >
> > bq24257 {
> > compatible = "ti,bq24257";
> > reg = <0x6a>;
> > + stat-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>;
> > + pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> >
> > ti,battery-regulation-voltage = <4200000>;
> > ti,charge-current = <1000000>;
> > ti,termination-current = <50000>;
> > };
> > +
> > +Example:
> > +
> > +bq24250 {
> > + compatible = "ti,bq24250";
> > + reg = <0x6a>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <16 IRQ_TYPE_EDGE_BOTH>;
> > +
> > + ti,battery-regulation-voltage = <4200000>;
> > + ti,charge-current = <500000>;
> > + ti,termination-current = <50000>;
> > + ti,current-limit = <900000>;
> > + ti,ovp-voltage = <9500000>;
> > + ti,in-dpm-voltage = <4440000>;
> > +};
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-14 13:54 ` Andreas Dannenberg
@ 2015-09-15 7:56 ` Krzysztof Kozlowski
2015-09-15 9:05 ` Laurentiu Palcu
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-15 7:56 UTC (permalink / raw)
To: Andreas Dannenberg
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Laurentiu Palcu, Ramakrishna Pallala, linux-pm, devicetree
On 14.09.2015 22:54, Andreas Dannenberg wrote:
> On Mon, Sep 14, 2015 at 03:11:32PM +0900, Krzysztof Kozlowski wrote:
>> On 12.09.2015 05:26, Andreas Dannenberg wrote:
>>> Extend the bq24257 charger's device tree documentation to cover the
>>> bq24250 and bq24251 devices as well feature additions.
>>>
>>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
>>> ---
>>> .../devicetree/bindings/power/bq24257.txt | 59 ++++++++++++++++++++--
>>> 1 file changed, 54 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
>>> index 5c9d394..4a88c96 100644
>>> --- a/Documentation/devicetree/bindings/power/bq24257.txt
>>> +++ b/Documentation/devicetree/bindings/power/bq24257.txt
>>> @@ -1,21 +1,70 @@
>>> -Binding for TI bq24257 Li-Ion Charger
>>> +Binding for TI bq24250/bq24251/bq24257 Li-Ion Charger
>>>
>>> Required properties:
>>> - compatible: Should contain one of the following:
>>> + * "ti,bq24250"
>>> + * "ti,bq24251"
>>> * "ti,bq24257"
>>> -- reg: integer, i2c address of the device.
>>> +- reg: integer, i2c address of the device.
>>> +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can
>>> + also be defined through the standard interrupt definition properties (see
>>> + optional properties section below). Only use one method.
>>> - ti,battery-regulation-voltage: integer, maximum charging voltage in uV.
>>> -- ti,charge-current: integer, maximum charging current in uA.
>>> -- ti,termination-current: integer, charge will be terminated when current in
>>> - constant-voltage phase drops below this value (in uA).
>>> +- ti,charge-current: integer, maximum charging current in uA.
>>> +- ti,termination-current: integer, charge will be terminated when current in
>>> + constant-voltage phase drops below this value (in uA).
>>> +
>>> +Optional properties:
>>> +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin.
>>> + This pin is not available on all devices however it should be used if
>>> + possible as this is the recommended way to obtain the charger's input PG
>>> + state. If this pin is not specified a software-based approach for PG
>>> + detection is used.
>>> +- ti,current-limit: The maximum current to be drawn from the charger's input
>>> + (in uA). If this property is not specified a USB D+/D- signal based charger
>>> + type detection is used (if available) and the input limit current is set
>>> + accordingly. If the D+/D- based detection is not available on a given device
>>> + a default of 500,000 is used (=500mA).
>>> +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If
>>> + not specified a default of 6,5000,000 (=6.5V) is used.
>>> +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic
>>> + power path management (in uV). If not specified a default of 4,360,000
>>> + (=4.36V) is used.
>>> +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety
>>> + timer is extended (slowed down) by a factor of two. Setting this property
>>> + to 0 or not providing it will leave the safety timer at its default
>>> + setting.
>>
>> Now I spotted the difference in this binding from previous emails.
>> Previously you made it as a boolean property. Now it is a integer
>> property for a boolean value. It is unusual. If you expect a need (or a
>> possibility of need) of:
>> 1. extending,
>> 2. overriding,
>> 3. using similar (extended) property in different drivers,
>> then it should be a real value, like:
>>
>> 1. ti,safety-timer: integer, in seconds
>
> Hi Krzysztof,
>
> I thought about this as well because the device actually allows
> configuring different safety timer intervals (currently not exposed) and
> it appears that the "2x enable" could be factored in there however it's
> not as easy as it seems. The "2x enable" is a dedicated HW feature
> that's only being used in some states of the devices and with that does
> not generally extend the safety timer under all conditions.
Right, I read also the datasheet which explains it.
>
>> 2. ti,safety-timer-ratio/multiplier: integer, supported values are: 0
>> (use default) or 2 (slow down by factor of 2)
>>
>> It is unusual and not obvious to use a integer value for boolean strict
>> property. With current solution you can actually only override it.
>
> Yes that was the sole reason behind using integer instead of boolean.
> The boolean "character" of this property was left intact because of the
> direct mapping to a device HW feature (see below).
DT serves a purpose of providing information allowing the kernel to use
and configure the hardware. DT binding does not have to map device
property. In the same time having a device property does not mean you
create a DT binding.
If such requirement of mapping property to a HW feature was valid then
each driver would expose each register in DT. Sometimes platform data
did this... but DT is not pdata.
You're using such argument also below but this argument is not sufficient.
Sufficient argument would be: this property is necessary to configure
the hardware in different board configurations (so on different boards
it will have different values).
Example of device features not matching to DeviceTree:
- configuration changing over time:
/sys/class/power/ds2760-battery.*/charge_full
- various timers:
/sys/class/power_supply/max14577-charger/device/fast_charge_timer
- various thresholds:
/sys/class/power_supply/max77693-charger/device/top_off_threshold_current
These are strictly HW features (they map to a bit in register) but they
are not related to configuring device for specific board. They serve for
user-space to use the devices in different ways.
In this case - the 2XTMR_EN - I assume that for certain batteries or
boards you want to extend the time of charging in case of special
conditions?
> But I can certainly
> change this to ti,safety-timer-multiplier and use 0/2. This avoids the
> inaccuracy as explained above with just using ti,safety-timer (the
> corresponding underlying device property should be separate and could be
> exposed if ever needed) and still leaves room for this property to grow
> which may come in handy later.
Hmmmm, the "2x" part of timer suggests to use a numerical value but
actually the datasheet does the opposite. I would not expect different
values... so your original idea makes sense (if DT is the place for it).
Overriding could be achieved with /delete-property/ (I forgot about it...).
>
>> Extending is possible but would look really weird, like:
>> ti,safety-timer-2x-enable = <4>; /* Heh? 4x or (4*2)x ??? */
>
>>
>> To add even more confusion: why this property has to be configured in
>> Device Tree? How it is related to the hardware? It rather looks like a
>> job for sysfs.
>
> This property maps directly to a corresponding bit in the device
> register map so yes it is related to the HW and IMHO should be
> applicable to DT. The reason for not providing write access to that bit
> via sysfs is the same as this driver doesn't have write access to other
> "critical" properties such as charge current/voltages.
Responded to this above.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-15 7:56 ` Krzysztof Kozlowski
@ 2015-09-15 9:05 ` Laurentiu Palcu
2015-09-15 12:51 ` Krzysztof Kozlowski
2015-09-15 13:57 ` Andreas Dannenberg
0 siblings, 2 replies; 17+ messages in thread
From: Laurentiu Palcu @ 2015-09-15 9:05 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Dannenberg, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, Ramakrishna Pallala, linux-pm, devicetree
On Tue, Sep 15, 2015 at 04:56:52PM +0900, Krzysztof Kozlowski wrote:
> On 14.09.2015 22:54, Andreas Dannenberg wrote:
> > On Mon, Sep 14, 2015 at 03:11:32PM +0900, Krzysztof Kozlowski wrote:
> >> On 12.09.2015 05:26, Andreas Dannenberg wrote:
> >>> Extend the bq24257 charger's device tree documentation to cover the
> >>> bq24250 and bq24251 devices as well feature additions.
> >>>
> >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> >>> ---
> >>> .../devicetree/bindings/power/bq24257.txt | 59 ++++++++++++++++++++--
> >>> 1 file changed, 54 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
> >>> index 5c9d394..4a88c96 100644
> >>> --- a/Documentation/devicetree/bindings/power/bq24257.txt
> >>> +++ b/Documentation/devicetree/bindings/power/bq24257.txt
> >>> @@ -1,21 +1,70 @@
> >>> -Binding for TI bq24257 Li-Ion Charger
> >>> +Binding for TI bq24250/bq24251/bq24257 Li-Ion Charger
> >>>
> >>> Required properties:
> >>> - compatible: Should contain one of the following:
> >>> + * "ti,bq24250"
> >>> + * "ti,bq24251"
> >>> * "ti,bq24257"
> >>> -- reg: integer, i2c address of the device.
> >>> +- reg: integer, i2c address of the device.
> >>> +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can
> >>> + also be defined through the standard interrupt definition properties (see
> >>> + optional properties section below). Only use one method.
> >>> - ti,battery-regulation-voltage: integer, maximum charging voltage in uV.
> >>> -- ti,charge-current: integer, maximum charging current in uA.
> >>> -- ti,termination-current: integer, charge will be terminated when current in
> >>> - constant-voltage phase drops below this value (in uA).
> >>> +- ti,charge-current: integer, maximum charging current in uA.
> >>> +- ti,termination-current: integer, charge will be terminated when current in
> >>> + constant-voltage phase drops below this value (in uA).
> >>> +
> >>> +Optional properties:
> >>> +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin.
> >>> + This pin is not available on all devices however it should be used if
> >>> + possible as this is the recommended way to obtain the charger's input PG
> >>> + state. If this pin is not specified a software-based approach for PG
> >>> + detection is used.
> >>> +- ti,current-limit: The maximum current to be drawn from the charger's input
> >>> + (in uA). If this property is not specified a USB D+/D- signal based charger
> >>> + type detection is used (if available) and the input limit current is set
> >>> + accordingly. If the D+/D- based detection is not available on a given device
> >>> + a default of 500,000 is used (=500mA).
> >>> +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If
> >>> + not specified a default of 6,5000,000 (=6.5V) is used.
> >>> +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic
> >>> + power path management (in uV). If not specified a default of 4,360,000
> >>> + (=4.36V) is used.
> >>> +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety
> >>> + timer is extended (slowed down) by a factor of two. Setting this property
> >>> + to 0 or not providing it will leave the safety timer at its default
> >>> + setting.
> >>
> >> Now I spotted the difference in this binding from previous emails.
> >> Previously you made it as a boolean property. Now it is a integer
> >> property for a boolean value. It is unusual. If you expect a need (or a
> >> possibility of need) of:
> >> 1. extending,
> >> 2. overriding,
> >> 3. using similar (extended) property in different drivers,
> >> then it should be a real value, like:
> >>
> >> 1. ti,safety-timer: integer, in seconds
> >
> > Hi Krzysztof,
> >
> > I thought about this as well because the device actually allows
> > configuring different safety timer intervals (currently not exposed) and
> > it appears that the "2x enable" could be factored in there however it's
> > not as easy as it seems. The "2x enable" is a dedicated HW feature
> > that's only being used in some states of the devices and with that does
> > not generally extend the safety timer under all conditions.
>
> Right, I read also the datasheet which explains it.
>
> >
> >> 2. ti,safety-timer-ratio/multiplier: integer, supported values are: 0
> >> (use default) or 2 (slow down by factor of 2)
> >>
> >> It is unusual and not obvious to use a integer value for boolean strict
> >> property. With current solution you can actually only override it.
> >
> > Yes that was the sole reason behind using integer instead of boolean.
> > The boolean "character" of this property was left intact because of the
> > direct mapping to a device HW feature (see below).
>
> DT serves a purpose of providing information allowing the kernel to use
> and configure the hardware. DT binding does not have to map device
> property. In the same time having a device property does not mean you
> create a DT binding.
>
> If such requirement of mapping property to a HW feature was valid then
> each driver would expose each register in DT. Sometimes platform data
> did this... but DT is not pdata.
> You're using such argument also below but this argument is not sufficient.
>
> Sufficient argument would be: this property is necessary to configure
> the hardware in different board configurations (so on different boards
> it will have different values).
>
> Example of device features not matching to DeviceTree:
> - configuration changing over time:
> /sys/class/power/ds2760-battery.*/charge_full
>
> - various timers:
> /sys/class/power_supply/max14577-charger/device/fast_charge_timer
>
> - various thresholds:
> /sys/class/power_supply/max77693-charger/device/top_off_threshold_current
>
> These are strictly HW features (they map to a bit in register) but they
> are not related to configuring device for specific board. They serve for
> user-space to use the devices in different ways.
>
> In this case - the 2XTMR_EN - I assume that for certain batteries or
> boards you want to extend the time of charging in case of special
> conditions?
FWIW, I don't see any real benefit in manually changing the 2X timer at
all. The default setting is decent enough. The chip will automatically
switch to 2X timer in certain conditions: for example, when the
temperature threshold is exceeded and the charge current is
automatically lowered. This makes sense since it'll take the battery
more time to charge at a lower current.
It would probably make more sense to add a DT property to change the
'Safety Timer Time Limit' device property (which can be 0.75hrs, 6hrs,
9hrs or totally disabled) than the 2XTMR.
I omitted both when I originally added support for this chip since the
6hrs default safety timer seemed more than enough to charge a nowadays
battery, even at 500mA (provided by a standard USB port). But, future
batteries might (and probably will) have higher capacities and will take
longer to charge.
laurentiu
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-15 9:05 ` Laurentiu Palcu
@ 2015-09-15 12:51 ` Krzysztof Kozlowski
2015-09-15 13:57 ` Andreas Dannenberg
1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2015-09-15 12:51 UTC (permalink / raw)
To: Laurentiu Palcu
Cc: Krzysztof Kozlowski, Andreas Dannenberg, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Ramakrishna Pallala,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
2015-09-15 18:05 GMT+09:00 Laurentiu Palcu <laurentiu.palcu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
> On Tue, Sep 15, 2015 at 04:56:52PM +0900, Krzysztof Kozlowski wrote:
>> On 14.09.2015 22:54, Andreas Dannenberg wrote:
>> > On Mon, Sep 14, 2015 at 03:11:32PM +0900, Krzysztof Kozlowski wrote:
>> >>
>> >> Now I spotted the difference in this binding from previous emails.
>> >> Previously you made it as a boolean property. Now it is a integer
>> >> property for a boolean value. It is unusual. If you expect a need (or a
>> >> possibility of need) of:
>> >> 1. extending,
>> >> 2. overriding,
>> >> 3. using similar (extended) property in different drivers,
>> >> then it should be a real value, like:
>> >>
>> >> 1. ti,safety-timer: integer, in seconds
>> >
>> > Hi Krzysztof,
>> >
>> > I thought about this as well because the device actually allows
>> > configuring different safety timer intervals (currently not exposed) and
>> > it appears that the "2x enable" could be factored in there however it's
>> > not as easy as it seems. The "2x enable" is a dedicated HW feature
>> > that's only being used in some states of the devices and with that does
>> > not generally extend the safety timer under all conditions.
>>
>> Right, I read also the datasheet which explains it.
>>
>> >
>> >> 2. ti,safety-timer-ratio/multiplier: integer, supported values are: 0
>> >> (use default) or 2 (slow down by factor of 2)
>> >>
>> >> It is unusual and not obvious to use a integer value for boolean strict
>> >> property. With current solution you can actually only override it.
>> >
>> > Yes that was the sole reason behind using integer instead of boolean.
>> > The boolean "character" of this property was left intact because of the
>> > direct mapping to a device HW feature (see below).
>>
>> DT serves a purpose of providing information allowing the kernel to use
>> and configure the hardware. DT binding does not have to map device
>> property. In the same time having a device property does not mean you
>> create a DT binding.
>>
>> If such requirement of mapping property to a HW feature was valid then
>> each driver would expose each register in DT. Sometimes platform data
>> did this... but DT is not pdata.
>> You're using such argument also below but this argument is not sufficient.
>>
>> Sufficient argument would be: this property is necessary to configure
>> the hardware in different board configurations (so on different boards
>> it will have different values).
>>
>> Example of device features not matching to DeviceTree:
>> - configuration changing over time:
>> /sys/class/power/ds2760-battery.*/charge_full
>>
>> - various timers:
>> /sys/class/power_supply/max14577-charger/device/fast_charge_timer
>>
>> - various thresholds:
>> /sys/class/power_supply/max77693-charger/device/top_off_threshold_current
>>
>> These are strictly HW features (they map to a bit in register) but they
>> are not related to configuring device for specific board. They serve for
>> user-space to use the devices in different ways.
>>
>> In this case - the 2XTMR_EN - I assume that for certain batteries or
>> boards you want to extend the time of charging in case of special
>> conditions?
>
> FWIW, I don't see any real benefit in manually changing the 2X timer at
> all. The default setting is decent enough. The chip will automatically
> switch to 2X timer in certain conditions: for example, when the
> temperature threshold is exceeded and the charge current is
> automatically lowered. This makes sense since it'll take the battery
> more time to charge at a lower current.
>
> It would probably make more sense to add a DT property to change the
> 'Safety Timer Time Limit' device property (which can be 0.75hrs, 6hrs,
> 9hrs or totally disabled) than the 2XTMR.
>
> I omitted both when I originally added support for this chip since the
> 6hrs default safety timer seemed more than enough to charge a nowadays
> battery, even at 500mA (provided by a standard USB port). But, future
> batteries might (and probably will) have higher capacities and will take
> longer to charge.
>From my past mistakes I found that if binding is not necessary and it
is not used (e.g. every DTS have the same value) then it will be
easier not to add it. Bindings mean ABI so if you make a mistake then
you will have to leave with it. Usually it is easier to add new
bindings when they are needed.
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices
2015-09-15 9:05 ` Laurentiu Palcu
2015-09-15 12:51 ` Krzysztof Kozlowski
@ 2015-09-15 13:57 ` Andreas Dannenberg
1 sibling, 0 replies; 17+ messages in thread
From: Andreas Dannenberg @ 2015-09-15 13:57 UTC (permalink / raw)
To: Laurentiu Palcu
Cc: Krzysztof Kozlowski, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, Ramakrishna Pallala, linux-pm, devicetree
On Tue, Sep 15, 2015 at 12:05:37PM +0300, Laurentiu Palcu wrote:
> On Tue, Sep 15, 2015 at 04:56:52PM +0900, Krzysztof Kozlowski wrote:
[...]
> > In this case - the 2XTMR_EN - I assume that for certain batteries or
> > boards you want to extend the time of charging in case of special
> > conditions?
>
> FWIW, I don't see any real benefit in manually changing the 2X timer at
> all. The default setting is decent enough. The chip will automatically
> switch to 2X timer in certain conditions: for example, when the
> temperature threshold is exceeded and the charge current is
> automatically lowered. This makes sense since it'll take the battery
> more time to charge at a lower current.
Laurentiu,
your comment just made me go double-check the applicable datasheets and
realize that the timer 2x extension feature is by default enabled (not
disabled as I originally thought) which is great. With that, we do have
good coverage for the "special" charging conditions and don't
need to expose access to this feature at this time. Will remove it.
> It would probably make more sense to add a DT property to change the
> 'Safety Timer Time Limit' device property (which can be 0.75hrs, 6hrs,
> 9hrs or totally disabled) than the 2XTMR.
>
> I omitted both when I originally added support for this chip since the
> 6hrs default safety timer seemed more than enough to charge a nowadays
> battery, even at 500mA (provided by a standard USB port). But, future
> batteries might (and probably will) have higher capacities and will take
> longer to charge.
Agreed, 6hrs should cover most use cases. And it'll be easy enough to
add on this property when needed.
Regards,
--
Andreas Dannenberg
Texas Instruments Inc
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-09-15 13:57 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-11 20:26 [PATCH v3 00/10] power: bq24257: Add support for bq24250/bq24251 Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 01/10] dt: power: bq24257-charger: Cover additional devices Andreas Dannenberg
2015-09-14 6:11 ` Krzysztof Kozlowski
2015-09-14 13:54 ` Andreas Dannenberg
2015-09-15 7:56 ` Krzysztof Kozlowski
2015-09-15 9:05 ` Laurentiu Palcu
2015-09-15 12:51 ` Krzysztof Kozlowski
2015-09-15 13:57 ` Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 03/10] power: bq24257: Add bit definition for temp sense enable Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 04/10] power: bq24257: Allow manual setting of input current limit Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 05/10] power: bq24257: Add SW-based approach for Power Good determination Andreas Dannenberg
[not found] ` <1442003202-1430-1-git-send-email-dannenberg-l0cyMroinI0@public.gmane.org>
2015-09-11 20:26 ` [PATCH v3 02/10] power: bq24257: Add basic support for bq24250/bq24251 Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 06/10] power: bq24257: Add over voltage protection setting support Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 07/10] power: bq24257: Add input DPM voltage threshold " Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 08/10] power: bq24257: Allow input current limit sysfs access Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 09/10] power: bq24257: Add various device-specific sysfs properties Andreas Dannenberg
2015-09-11 20:26 ` [PATCH v3 10/10] power: bq24257: Add platform data based initialization Andreas Dannenberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.