[PATCH v7 00/15] power: bq24257: Add support for bq24250/bq24251

[PATCH v7 00/15] power: bq24257: Add support for bq24250/bq24251

[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.

v7:
- Fixed an issue that would have impacted running bisect on the series
  and double checked the entire series builds one by one by running
  a git rebase -i --exec="make ..." ... on the Kernel tree
- Minor simplification of a return from a function

v6:
- Removed the ability to configure the IRQ through "stat-gpios"
- Allow re-configuring the input_current_limit via sysfs also when
  charger auto-detection is enabled
- Use gpiod interface for initialization as a platform device (nice
  GPIO setup simplification)
- Document all newly introduced sysfs nodes
- Streamlined/fixed/simplified charger state handling as it relates
  to battery removal/insertion and the associated handling of the
  input current limit setting
- Minor simplifications/cleanup
- Rebased onto git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-
  power-supply.git (branch "next") and completely retested

v5:
- Added patch to use the managed version of power supply register which
  also fixes a code flow issue that was introduced
- Minor fixes / consistency cleanup (Krzysztof's feedback)

v4:
- Removed configurability of the safety timer multiplier through DT

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 (15):
  dt: power: bq24257-charger: Cover additional devices
  power: bq24257: Remove IRQ config through stat-gpios
  power: bq24257: Streamline input current limit setup
  power: bq24257: Use managed power supply register
  power: bq24257: Simplify bq24257_power_supply_init()
  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
  Documentation: power: bq24257: Document exported sysfs entries

 Documentation/ABI/testing/sysfs-class-power        |  58 +++
 .../devicetree/bindings/power/bq24257.txt          |  53 ++-
 drivers/power/Kconfig                              |   5 +-
 drivers/power/bq24257_charger.c                    | 518 ++++++++++++++++++---
 include/linux/power/bq24257_charger.h              |  25 +
 5 files changed, 577 insertions(+), 82 deletions(-)
 create mode 100644 include/linux/power/bq24257_charger.h

-- 
1.9.1

[PATCH v7 06/15] power: bq24257: Add basic support for bq24250/bq24251

[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 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@ti.com>
---
 drivers/power/Kconfig           |  5 ++--
 drivers/power/bq24257_charger.c | 52 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index eeb5776..9e68853 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -409,12 +409,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 060f754..b0c8533 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>
@@ -40,6 +44,22 @@
 
 #define BQ24257_ILIM_SET_DELAY		1000	/* msec */
 
+/*
+ * When adding support for new devices make sure that enum bq2425x_chip and
+ * bq2425x_chip_name[] always stay in sync!
+ */
+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 */
@@ -70,6 +90,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];
 
@@ -248,6 +270,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;
@@ -561,6 +587,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,
@@ -644,6 +671,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;
@@ -660,6 +688,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);
@@ -722,7 +762,7 @@ static int bq24257_probe(struct i2c_client *client,
 					bq24257_irq_handler_thread,
 					IRQF_TRIGGER_FALLING |
 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
-					"bq24257", bq);
+					bq2425x_chip_name[bq->chip], bq);
 	if (ret) {
 		dev_err(dev, "Failed to request IRQ #%d\n", client->irq);
 		return ret;
@@ -793,19 +833,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

Re: [PATCH v7 06/15] power: bq24257: Add basic support for bq24250/bq24251

[Krzysztof Kozlowski]

W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> 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 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>

For the second time (after 1st patch changing DT documentation) I am
confused - my reviewed-by was not added.

I assume it got lost due to misunderstanding or something else but
anyway I won't insist. If you want it, just let me know.

Best regards,
Krzysztof

Re: [PATCH v7 06/15] power: bq24257: Add basic support for bq24250/bq24251

[Andreas Dannenberg]

On Mon, Sep 28, 2015 at 10:14:56PM +0900, Krzysztof Kozlowski wrote:
> W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> > 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 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@ti.com>
> 
> For the second time (after 1st patch changing DT documentation) I am
> confused - my reviewed-by was not added.
> 
> I assume it got lost due to misunderstanding or something else but
> anyway I won't insist. If you want it, just let me know.

Hi Krzysztof, thanks for the reviewed-by's. Yes there was a
misunderstanding. Will make sure to carry them forward from now on.

Regards,

--
Andreas Dannenberg
Texas Instruments Inc

[PATCH v7 07/15] power: bq24257: Add bit definition for temp sense enable

[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 b0c8533..93f7582 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -66,7 +66,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
@@ -156,6 +156,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

[PATCH v7 01/15] dt: power: bq24257-charger: Cover additional devices

[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-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/power/bq24257.txt          | 53 ++++++++++++++++++++--
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
index 5c9d394..6c376ef 100644
--- a/Documentation/devicetree/bindings/power/bq24257.txt
+++ b/Documentation/devicetree/bindings/power/bq24257.txt
@@ -1,21 +1,64 @@
-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.
+- interrupt-parent: Should be the phandle for the interrupt controller. Use in
+    conjunction with "interrupts".
+- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in
+    conjunction with "interrupt-parent".
 - 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.
 
 Example:
 
 bq24257 {
 	compatible = "ti,bq24257";
 	reg = <0x6a>;
+	interrupt-parent = <&gpio1>;
+	interrupts = <16 IRQ_TYPE_EDGE_BOTH>;
+
+	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

Re: [PATCH v7 01/15] dt: power: bq24257-charger: Cover additional devices

[Krzysztof Kozlowski]

W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> 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-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/power/bq24257.txt          | 53 ++++++++++++++++++++--
>  1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt
> index 5c9d394..6c376ef 100644
> --- a/Documentation/devicetree/bindings/power/bq24257.txt
> +++ b/Documentation/devicetree/bindings/power/bq24257.txt
> @@ -1,21 +1,64 @@
> -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.
> +- interrupt-parent: Should be the phandle for the interrupt controller. Use in
> +    conjunction with "interrupts".
> +- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in
> +    conjunction with "interrupt-parent".
>  - 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.
>  
>  Example:
>  
>  bq24257 {
>  	compatible = "ti,bq24257";
>  	reg = <0x6a>;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <16 IRQ_TYPE_EDGE_BOTH>;
> +
> +	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>;
> +};

Once again:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Best regards,
Krzysztof

Re: [PATCH v7 01/15] dt: power: bq24257-charger: Cover additional devices

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:06AM -0500, Andreas Dannenberg wrote:
> Extend the bq24257 charger's device tree documentation to cover the
> bq24250 and bq24251 devices as well feature additions.
>
> [...]
>
> +- 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).

I think this wording is a bit better:

- ti,current-limit: The maximum current to be drawn from the charger's input
    (in uA). If this property is not specified, the input limit current is
    set automatically using USB D+/D- signal based charger type detection.
    If the hardware does not support the D+/D- based detection, a default
    of 500,000 is used (=500mA) instead.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v7 02/15] power: bq24257: Remove IRQ config through stat-gpios

[Andreas Dannenberg]

At the time the driver was written GpioInt resources in ACPI were not
passed to the driver in client->irq, as opposed to DT enumeration. To
accommodate this use case, a "stat-gpios" property was introduced to
allow configuring the IRQ.

However this issue with ACPI was fixed in commit "845c877 i2c / ACPI:
Assign IRQ for devices that have GpioInt automatically" and makes this
workaround no longer necessary, hence we can remove the support for the
"stat-gpios" property and the associated code from the bq24257 driver.

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 5859bc7..69b53d0 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -36,7 +36,6 @@
 #define BQ24257_REG_7			0x06
 
 #define BQ24257_MANUFACTURER		"Texas Instruments"
-#define BQ24257_STAT_IRQ		"stat"
 #define BQ24257_PG_GPIO			"pg"
 
 #define BQ24257_ILIM_SET_DELAY		1000	/* msec */
@@ -606,19 +605,6 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
 	return 0;
 }
 
-static int bq24257_irq_probe(struct bq24257_device *bq)
-{
-	struct gpio_desc *stat_irq;
-
-	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);
-	}
-
-	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);
@@ -740,21 +726,15 @@ static int bq24257_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	if (client->irq <= 0)
-		client->irq = bq24257_irq_probe(bq);
-
-	if (client->irq < 0) {
-		dev_err(dev, "no irq resource found\n");
-		return client->irq;
-	}
-
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
 					bq24257_irq_handler_thread,
 					IRQF_TRIGGER_FALLING |
 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
-					BQ24257_STAT_IRQ, bq);
-	if (ret)
+					"bq24257", bq);
+	if (ret) {
+		dev_err(dev, "Failed to request IRQ #%d\n", client->irq);
 		return ret;
+	}
 
 	ret = bq24257_power_supply_init(bq);
 	if (ret < 0)
-- 
1.9.1

Re: [PATCH v7 02/15] power: bq24257: Remove IRQ config through stat-gpios

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:07AM -0500, Andreas Dannenberg wrote:
> At the time the driver was written GpioInt resources in ACPI were not
> passed to the driver in client->irq, as opposed to DT enumeration. To
> accommodate this use case, a "stat-gpios" property was introduced to
> allow configuring the IRQ.
> 
> However this issue with ACPI was fixed in commit "845c877 i2c / ACPI:
> Assign IRQ for devices that have GpioInt automatically" and makes this
> workaround no longer necessary, hence we can remove the support for the
> "stat-gpios" property and the associated code from the bq24257 driver.

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v7 03/15] power: bq24257: Streamline input current limit setup

[Andreas Dannenberg]

The maximum amount of input current the charger should draw is dependent
on the power supply and should only be (re-)configured when the power
supply gets connected and disconnected. However the driver was also
lowering the bq24257's input current limit setting to 500mA when the
battery was removed and restored the previous setting according to the
power supply capabilities when the battery was reconnected although
these events are not impacting the amount of power that can be drawn
from the supply. Furthermore, a re-configuration of the input current
limit to 500mA when the battery gets disconnected is actually dangerous
if the limit was set higher previously and the system draws more than
500mA in which case the system voltage would be reduced in order to
maintain 500mA which could result in the system getting too low of a
supply to maintain operation. Last but not least the mechanism itself
used for battery re-connection detection did not work in corner cases
such as when the device's input current loop becomes active and the
bq24257 device clears its battery fault error resulting in incorrectly
reporting that the battery got reconnected.

This patches removes the impact the battery removal/insertion has on the
input current limit configured for the bq24257 and simplifies the
associated handler routine.

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 69b53d0..d2d077c 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -448,14 +448,13 @@ static void bq24257_handle_state_change(struct bq24257_device *bq,
 {
 	int ret;
 	struct bq24257_state old_state;
-	bool reset_iilimit = false;
-	bool config_iilimit = false;
 
 	mutex_lock(&bq->lock);
 	old_state = bq->state;
 	mutex_unlock(&bq->lock);
 
-	if (!new_state->power_good) {			     /* power removed */
+	if (!new_state->power_good) {
+		dev_dbg(bq->dev, "Power removed\n");
 		cancel_delayed_work_sync(&bq->iilimit_setup_work);
 
 		/* activate D+/D- port detection algorithm */
@@ -463,26 +462,20 @@ static void bq24257_handle_state_change(struct bq24257_device *bq,
 		if (ret < 0)
 			goto error;
 
-		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);
-
-		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 */
-		dev_err(bq->dev, "Safety timer expired! Battery dead?\n");
-	}
-
-	if (reset_iilimit) {
+		/* reset input current limit */
 		ret = bq24257_field_write(bq, F_IILIMIT, IILIMIT_500);
 		if (ret < 0)
 			goto error;
-	} else if (config_iilimit) {
+	} else if (!old_state.power_good) {
+		dev_dbg(bq->dev, "Power inserted\n");
+
+		/* configure input current limit */
 		schedule_delayed_work(&bq->iilimit_setup_work,
 				      msecs_to_jiffies(BQ24257_ILIM_SET_DELAY));
+	} else if (new_state->fault == FAULT_NO_BAT) {
+		dev_warn(bq->dev, "Battery removed\n");
+	} else if (new_state->fault == FAULT_TIMER) {
+		dev_err(bq->dev, "Safety timer expired! Battery dead?\n");
 	}
 
 	return;
-- 
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

Re: [PATCH v7 03/15] power: bq24257: Streamline input current limit setup

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:08AM -0500, Andreas Dannenberg wrote:
> The maximum amount of input current the charger should draw is dependent
> on the power supply and should only be (re-)configured when the power
> supply gets connected and disconnected. However the driver was also
> lowering the bq24257's input current limit setting to 500mA when the
> battery was removed and restored the previous setting according to the
> power supply capabilities when the battery was reconnected although
> these events are not impacting the amount of power that can be drawn
> from the supply. Furthermore, a re-configuration of the input current
> limit to 500mA when the battery gets disconnected is actually dangerous
> if the limit was set higher previously and the system draws more than
> 500mA in which case the system voltage would be reduced in order to
> maintain 500mA which could result in the system getting too low of a
> supply to maintain operation. Last but not least the mechanism itself
> used for battery re-connection detection did not work in corner cases
> such as when the device's input current loop becomes active and the
> bq24257 device clears its battery fault error resulting in incorrectly
> reporting that the battery got reconnected.
> 
> This patches removes the impact the battery removal/insertion has on the
> input current limit configured for the bq24257 and simplifies the
> associated handler routine.
> 
> Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v7 04/15] power: bq24257: Use managed power supply register

[Andreas Dannenberg]

Use the devm_* managed version of the function to register the power
supply and remove the associated unregister function.

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index d2d077c..6757b41 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -590,8 +590,10 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
 	psy_cfg.supplied_to = bq24257_charger_supplied_to;
 	psy_cfg.num_supplicants = ARRAY_SIZE(bq24257_charger_supplied_to);
 
-	bq->charger = power_supply_register(bq->dev, &bq24257_power_supply_desc,
-					    &psy_cfg);
+	bq->charger = devm_power_supply_register(bq->dev,
+						 &bq24257_power_supply_desc,
+						 &psy_cfg);
+
 	if (IS_ERR(bq->charger))
 		return PTR_ERR(bq->charger);
 
@@ -742,8 +744,6 @@ static int bq24257_remove(struct i2c_client *client)
 
 	cancel_delayed_work_sync(&bq->iilimit_setup_work);
 
-	power_supply_unregister(bq->charger);
-
 	bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */
 
 	return 0;
-- 
1.9.1

Re: [PATCH v7 04/15] power: bq24257: Use managed power supply register

[Krzysztof Kozlowski]

W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> Use the devm_* managed version of the function to register the power
> supply and remove the associated unregister function.

Add explanation for "why" you want to use devm method.

After that:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Best regards,
Krzysztof

> 
> Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/power/bq24257_charger.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
> index d2d077c..6757b41 100644
> --- a/drivers/power/bq24257_charger.c
> +++ b/drivers/power/bq24257_charger.c
> @@ -590,8 +590,10 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
>  	psy_cfg.supplied_to = bq24257_charger_supplied_to;
>  	psy_cfg.num_supplicants = ARRAY_SIZE(bq24257_charger_supplied_to);
>  
> -	bq->charger = power_supply_register(bq->dev, &bq24257_power_supply_desc,
> -					    &psy_cfg);
> +	bq->charger = devm_power_supply_register(bq->dev,
> +						 &bq24257_power_supply_desc,
> +						 &psy_cfg);
> +
>  	if (IS_ERR(bq->charger))
>  		return PTR_ERR(bq->charger);
>  
> @@ -742,8 +744,6 @@ static int bq24257_remove(struct i2c_client *client)
>  
>  	cancel_delayed_work_sync(&bq->iilimit_setup_work);
>  
> -	power_supply_unregister(bq->charger);
> -
>  	bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */
>  
>  	return 0;
> 

--
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

Re: [PATCH v7 04/15] power: bq24257: Use managed power supply register

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:09AM -0500, Andreas Dannenberg wrote:
> Use the devm_* managed version of the function to register the power
> supply and remove the associated unregister function.
> 
> Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/power/bq24257_charger.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
> index d2d077c..6757b41 100644
> --- a/drivers/power/bq24257_charger.c
> +++ b/drivers/power/bq24257_charger.c
> @@ -590,8 +590,10 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
>  	psy_cfg.supplied_to = bq24257_charger_supplied_to;
>  	psy_cfg.num_supplicants = ARRAY_SIZE(bq24257_charger_supplied_to);
>  
> -	bq->charger = power_supply_register(bq->dev, &bq24257_power_supply_desc,
> -					    &psy_cfg);
> +	bq->charger = devm_power_supply_register(bq->dev,
> +						 &bq24257_power_supply_desc,
> +						 &psy_cfg);
> +
>  	if (IS_ERR(bq->charger))
>  		return PTR_ERR(bq->charger);
>  
> @@ -742,8 +744,6 @@ static int bq24257_remove(struct i2c_client *client)
>  
>  	cancel_delayed_work_sync(&bq->iilimit_setup_work);
>  
> -	power_supply_unregister(bq->charger);
> -
>  	bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */


This effectively reordering bq24257_field_write() before
power_supply_unregister(), so you may want to loose a few words in
the patch description, that this is a known/unproblematic
side-effect.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v7 05/15] power: bq24257: Simplify bq24257_power_supply_init()

[Andreas Dannenberg]

Eliminate a few lines of code by using the PTR_ERR_OR_ZERO() macro.

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 6757b41..060f754 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -594,10 +594,7 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
 						 &bq24257_power_supply_desc,
 						 &psy_cfg);
 
-	if (IS_ERR(bq->charger))
-		return PTR_ERR(bq->charger);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(bq->charger);
 }
 
 static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
-- 
1.9.1

Re: [PATCH v7 05/15] power: bq24257: Simplify bq24257_power_supply_init()

[Krzysztof Kozlowski]

W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> Eliminate a few lines of code by using the PTR_ERR_OR_ZERO() macro.
> 
> Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/power/bq24257_charger.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Best regards,
Krzysztof

[PATCH v7 08/15] power: bq24257: Allow manual setting of input current limit

[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-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 93 +++++++++++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 18 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 93f7582..7bbff80 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -77,6 +77,7 @@ struct bq24257_init_data {
 	u8 ichg;	/* charge current      */
 	u8 vbat;	/* regulation voltage  */
 	u8 iterm;	/* termination current */
+	u8 iilimit;	/* input current limit */
 };
 
 struct bq24257_state {
@@ -103,6 +104,8 @@ struct bq24257_device {
 	struct bq24257_state state;
 
 	struct mutex lock; /* protect state data */
+
+	bool iilimit_autoset_enable;
 };
 
 static bool bq24257_is_volatile_reg(struct device *dev, unsigned int reg)
@@ -191,6 +194,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)
 {
@@ -480,24 +489,32 @@ static void bq24257_handle_state_change(struct bq24257_device *bq,
 	old_state = bq->state;
 	mutex_unlock(&bq->lock);
 
+	/*
+	 * 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");
-		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 input current limit */
-		ret = bq24257_field_write(bq, F_IILIMIT, IILIMIT_500);
-		if (ret < 0)
-			goto error;
+		if (bq->iilimit_autoset_enable) {
+			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 input current limit */
+			ret = bq24257_field_write(bq, F_IILIMIT,
+						  bq->init_data.iilimit);
+			if (ret < 0)
+				goto error;
+		}
 	} else if (!old_state.power_good) {
 		dev_dbg(bq->dev, "Power inserted\n");
 
-		/* configure input current limit */
-		schedule_delayed_work(&bq->iilimit_setup_work,
+		if (bq->iilimit_autoset_enable)
+			/* configure input current limit */
+			schedule_delayed_work(&bq->iilimit_setup_work,
 				      msecs_to_jiffies(BQ24257_ILIM_SET_DELAY));
 	} else if (new_state->fault == FAULT_NO_BAT) {
 		dev_warn(bq->dev, "Battery removed\n");
@@ -577,7 +594,16 @@ static int bq24257_hw_init(struct bq24257_device *bq)
 	bq->state = state;
 	mutex_unlock(&bq->lock);
 
-	if (!state.power_good)
+	if (!bq->iilimit_autoset_enable) {
+		dev_dbg(bq->dev, "manually setting iilimit = %u\n",
+			bq->init_data.iilimit);
+
+		/* program fixed input current limit */
+		ret = bq24257_field_write(bq, F_IILIMIT,
+					  bq->init_data.iilimit);
+		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)
@@ -641,6 +667,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;
@@ -664,6 +691,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) {
+		bq->iilimit_autoset_enable = true;
+
+		/*
+		 * 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.iilimit = IILIMIT_500;
+	} else
+		bq->init_data.iilimit =
+				bq24257_find_idx(property,
+						 bq24257_iilimit_map,
+						 BQ24257_IILIMIT_MAP_SIZE);
+
 	return 0;
 }
 
@@ -722,8 +767,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) {
@@ -734,6 +777,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->iilimit_autoset_enable = false;
+
+	if (bq->iilimit_autoset_enable)
+		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)
@@ -780,7 +835,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->iilimit_autoset_enable)
+		cancel_delayed_work_sync(&bq->iilimit_setup_work);
 
 	bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */
 
@@ -793,7 +849,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->iilimit_autoset_enable)
+		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

--
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

Re: [PATCH v7 08/15] power: bq24257: Allow manual setting of input current limit

[Krzysztof Kozlowski]

W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> 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 | 93 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 75 insertions(+), 18 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH v7 09/15] power: bq24257: Add SW-based approach for Power Good determination

[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-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 49 ++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 7bbff80..5800697 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -359,7 +359,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)
+		state->power_good = !gpiod_get_value_cansleep(bq->pg);
+	else
+		/*
+		 * 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;
+		}
 
 	return 0;
 }
@@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
 	return PTR_ERR_OR_ZERO(bq->charger);
 }
 
-static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
+static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
 {
-	bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
+	bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN);
+
 	if (IS_ERR(bq->pg)) {
-		dev_err(bq->dev, "could not probe PG pin\n");
-		return PTR_ERR(bq->pg);
+		dev_err(bq->dev, "error probing PG pin\n");
+		bq->pg = NULL;
+		return;
 	}
 
-	return 0;
+	if (bq->pg)
+		dev_dbg(bq->dev, "probed PG pin = %d\n", desc_to_gpio(bq->pg));
 }
 
 static int bq24257_fw_probe(struct bq24257_device *bq)
@@ -789,10 +811,17 @@ 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 let's
+	 * not probe for it and instead use a SW-based approach to determine
+	 * the PG state. We also use a SW-based approach for all other devices
+	 * if the PG pin is either not defined or can't be probed.
+	 */
+	if (bq->chip != BQ24250)
+		bq24257_pg_gpio_probe(bq);
+
+	if (!bq->pg)
+		dev_info(bq->dev, "using SW-based power-good detection\n");
 
 	/* reset all registers to defaults */
 	ret = bq24257_field_write(bq, F_RESET, 1);
-- 
1.9.1

Re: [PATCH v7 09/15] power: bq24257: Add SW-based approach for Power Good determination

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote:
> @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
>  	return PTR_ERR_OR_ZERO(bq->charger);
>  }
>  
> -static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
> +static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
>  {
> -	bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
> +	bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN);
> +
>  	if (IS_ERR(bq->pg)) {
> -		dev_err(bq->dev, "could not probe PG pin\n");
> -		return PTR_ERR(bq->pg);
> +		dev_err(bq->dev, "error probing PG pin\n");
> +		bq->pg = NULL;
> +		return;
>  	}

You should handle -EPROBE_DEFER here.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v7 09/15] power: bq24257: Add SW-based approach for Power Good determination

[Andreas Dannenberg]

On Sun, Sep 27, 2015 at 10:20:26PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote:
> > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
> >  	return PTR_ERR_OR_ZERO(bq->charger);
> >  }
> >  
> > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
> > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
> >  {
> > -	bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
> > +	bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN);
> > +
> >  	if (IS_ERR(bq->pg)) {
> > -		dev_err(bq->dev, "could not probe PG pin\n");
> > -		return PTR_ERR(bq->pg);
> > +		dev_err(bq->dev, "error probing PG pin\n");
> > +		bq->pg = NULL;
> > +		return;
> >  	}
> 
> You should handle -EPROBE_DEFER here.

Hi Sebastian. From having a quick look at the Kernel tree it seems like
this error shouldn't get exposed to the user (which it doesn't since it
would get zeroe'd out) and the error itself suggests that I should
"retry" which none of the things I looked at in the Kernel through "git
grep" seemed to do. So before making any assumptions I wanted to see if
I'm missing something or you could give me any pointers how to proceed.

Thanks,

--
Andreas Dannenberg
Texas Instruments Inc

Re: [PATCH v7 09/15] power: bq24257: Add SW-based approach for Power Good determination

[Andreas Dannenberg]

On Mon, Sep 28, 2015 at 07:04:12AM -0500, Andreas Dannenberg wrote:
> On Sun, Sep 27, 2015 at 10:20:26PM +0200, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote:
> > > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
> > >  	return PTR_ERR_OR_ZERO(bq->charger);
> > >  }
> > >  
> > > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
> > > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
> > >  {
> > > -	bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
> > > +	bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN);
> > > +
> > >  	if (IS_ERR(bq->pg)) {
> > > -		dev_err(bq->dev, "could not probe PG pin\n");
> > > -		return PTR_ERR(bq->pg);
> > > +		dev_err(bq->dev, "error probing PG pin\n");
> > > +		bq->pg = NULL;
> > > +		return;
> > >  	}
> > 
> > You should handle -EPROBE_DEFER here.
> 
> Hi Sebastian. From having a quick look at the Kernel tree it seems like
> this error shouldn't get exposed to the user (which it doesn't since it
> would get zeroe'd out) and the error itself suggests that I should
> "retry" which none of the things I looked at in the Kernel through "git
> grep" seemed to do. So before making any assumptions I wanted to see if
> I'm missing something or you could give me any pointers how to proceed.

Ok after checking drivers/base/dd.c it seems like what I should be doing
here is to simply exit the probe function and pass on -EPROBE_DEFER as
the return value of the probe function, and the system would then
automatically take care of re-trying the probe later, correct? Please
let know, I want to make sure this patch series is converging into an
acceptable state.

Regards,

--
Andreas Dannenberg
Texas Instruments Inc

Re: [PATCH v7 09/15] power: bq24257: Add SW-based approach for Power Good determination

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

Hi,

On Mon, Sep 28, 2015 at 07:04:12AM -0500, Andreas Dannenberg wrote:
> On Sun, Sep 27, 2015 at 10:20:26PM +0200, Sebastian Reichel wrote:
> > On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote:
> > > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
> > >  	return PTR_ERR_OR_ZERO(bq->charger);
> > >  }
> > >  
> > > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq)
> > > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
> > >  {
> > > -	bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN);
> > > +	bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN);
> > > +
> > >  	if (IS_ERR(bq->pg)) {
> > > -		dev_err(bq->dev, "could not probe PG pin\n");
> > > -		return PTR_ERR(bq->pg);
> > > +		dev_err(bq->dev, "error probing PG pin\n");
> > > +		bq->pg = NULL;
> > > +		return;
> > >  	}
> > 
> > You should handle -EPROBE_DEFER here.
> 
> Hi Sebastian. From having a quick look at the Kernel tree it seems like
> this error shouldn't get exposed to the user (which it doesn't since it
> would get zeroe'd out) and the error itself suggests that I should
> "retry" which none of the things I looked at in the Kernel through "git
> grep" seemed to do. So before making any assumptions I wanted to see if
> I'm missing something or you could give me any pointers how to proceed.

If you return -EPROBE_DEFER from your driver's probe function, the
driver probe will fail. In opposit to other error codes it will be
marked for retry at a later point, though. You will get
EPROBE_DEFER, if you try to access a GPIO from a not-yet initialized
GPIO controller (e.g. if you load the charger driver before the
gpio driver).

Correct handling is simple: Just forward it, making sure, that all
non-devm probed resources are correctly free'd.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v7 10/15] power: bq24257: Add over voltage protection setting support

[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 5800697..fd4d2da 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -78,6 +78,7 @@ struct bq24257_init_data {
 	u8 vbat;	/* regulation voltage  */
 	u8 iterm;	/* termination current */
 	u8 iilimit;	/* input current limit */
+	u8 vovp;	/* over voltage protection voltage */
 };
 
 struct bq24257_state {
@@ -200,6 +201,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)
 {
@@ -415,6 +423,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 */
@@ -586,7 +605,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},
 	};
 
 	/*
@@ -656,6 +676,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, "%u\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, };
@@ -731,6 +773,15 @@ static int bq24257_fw_probe(struct bq24257_device *bq)
 						 bq24257_iilimit_map,
 						 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;
 }
 
@@ -854,10 +905,18 @@ static int bq24257_probe(struct i2c_client *client,
 	}
 
 	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)
@@ -867,6 +926,8 @@ static int bq24257_remove(struct i2c_client *client)
 	if (bq->iilimit_autoset_enable)
 		cancel_delayed_work_sync(&bq->iilimit_setup_work);
 
+	sysfs_remove_group(&bq->charger->dev.kobj, &bq24257_attr_group);
+
 	bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */
 
 	return 0;
-- 
1.9.1

[PATCH v7 11/15] power: bq24257: Add input DPM voltage threshold setting support

[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-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 43 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index fd4d2da..2a76009 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -79,6 +79,7 @@ struct bq24257_init_data {
 	u8 iterm;	/* termination current */
 	u8 iilimit;	/* input current limit */
 	u8 vovp;	/* over voltage protection voltage */
+	u8 vindpm;	/* VDMP input threshold voltage */
 };
 
 struct bq24257_state {
@@ -208,6 +209,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)
 {
@@ -434,6 +442,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 */
@@ -607,6 +626,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},
 	};
 
 	/*
@@ -687,10 +707,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, "%u\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,
 };
 
@@ -782,6 +815,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

[PATCH v7 12/15] power: bq24257: Allow input current limit sysfs access

[Andreas Dannenberg]

This patch allows reading and writing of the input current limit through
the power supply's input_current_limit sysfs property. This allows
userspace to see what charger was detected (if the D+/D- USB signal-
based charger type detection is enabled) and to re-configure the maximum
current drawn from the external supply at runtime based on system-level
knowledge or user input. Note that upon charger disconnection and
re-connection the limit configured through firmware becomes active again
(or the D+/D- USB signal-based charger detection will be run again).

Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 85 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 79 insertions(+), 6 deletions(-)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 2a76009..73fcf89 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -267,6 +267,47 @@ 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)
+{
+	/*
+	 * Address the case where the user manually sets an input current limit
+	 * while the charger auto-detection mechanism is is active. In this
+	 * case we want to abort and go straight to the user-specified value.
+	 */
+	if (bq->iilimit_autoset_enable)
+		cancel_delayed_work_sync(&bq->iilimit_setup_work);
+
+	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)
@@ -351,6 +392,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;
 	}
@@ -358,6 +402,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)
 {
@@ -559,13 +628,14 @@ static void bq24257_handle_state_change(struct bq24257_device *bq,
 			ret = bq24257_field_write(bq, F_DPDM_EN, 1);
 			if (ret < 0)
 				goto error;
-
-			/* reset input current limit */
-			ret = bq24257_field_write(bq, F_IILIMIT,
-						  bq->init_data.iilimit);
-			if (ret < 0)
-				goto error;
 		}
+		/*
+		 * When power is removed always return to the default input
+		 * current limit as configured during probe.
+		 */
+		ret = bq24257_field_write(bq, F_IILIMIT, bq->init_data.iilimit);
+		if (ret < 0)
+			goto error;
 	} else if (!old_state.power_good) {
 		dev_dbg(bq->dev, "Power inserted\n");
 
@@ -682,6 +752,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[] = {
@@ -694,6 +765,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

[PATCH v7 13/15] power: bq24257: Add various device-specific sysfs properties

[Andreas Dannenberg]

This patch adds support for enabling/disabling optional device specific
features through sysfs properties at runtime.

* High-impedance mode enable/disable
* Sysoff enable/disable

Refer to the respective 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-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c | 53 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
index 73fcf89..19a0962 100644
--- a/drivers/power/bq24257_charger.c
+++ b/drivers/power/bq24257_charger.c
@@ -791,12 +791,65 @@ 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
+		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, (bool)val);
+	else if (strcmp(attr->attr.name, "sysoff_enable") == 0)
+		ret = bq24257_field_write(bq, F_SYSOFF, (bool)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 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,
 	NULL,
 };
 
-- 
1.9.1

[PATCH v7 14/15] power: bq24257: Add platform data based initialization

[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-l0cyMroinI0@public.gmane.org>
---
 drivers/power/bq24257_charger.c       | 39 +++++++++++++++++++++++++++++++++--
 include/linux/power/bq24257_charger.h | 25 ++++++++++++++++++++++
 2 files changed, 62 insertions(+), 2 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 19a0962..3b1a3b6 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
@@ -885,6 +888,36 @@ static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
 		dev_dbg(bq->dev, "probed PG pin = %d\n", desc_to_gpio(bq->pg));
 }
 
+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.iilimit = bq24257_find_idx(pdata->iilimit,
+						 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->iilimit_autoset_enable = pdata->iilimit_autoset_enable;
+}
+
 static int bq24257_fw_probe(struct bq24257_device *bq)
 {
 	int ret;
@@ -960,6 +993,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;
@@ -1009,14 +1043,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);
 	}
 
 	/*
diff --git a/include/linux/power/bq24257_charger.h b/include/linux/power/bq24257_charger.h
new file mode 100644
index 0000000..c131c5d
--- /dev/null
+++ b/include/linux/power/bq24257_charger.h
@@ -0,0 +1,25 @@
+/*
+ * 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 iilimit;				  /* input current limit (uA) */
+	u32 vovp;		      /* over voltage protection voltage (uV) */
+	u32 vindpm;			 /* VDMP input threshold voltage (uV) */
+	bool iilimit_autoset_enable;	/* auto-detect of input current limit */
+};
+
+#endif /* _BQ24257_CHARGER_H_ */
-- 
1.9.1

Re: [PATCH v7 14/15] power: bq24257: Add platform data based initialization

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:19AM -0500, Andreas Dannenberg wrote:
> 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.

Which board is this btw? Looks like most arm board code is going
away in the next few kernel releases, so I wonder if this patch
will just increase maintainence without any proper user.

> Signed-off-by: Andreas Dannenberg <dannenberg-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/power/bq24257_charger.c       | 39 +++++++++++++++++++++++++++++++++--
>  include/linux/power/bq24257_charger.h | 25 ++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 2 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 19a0962..3b1a3b6 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>

You can drop this include. It's not needed with gpiod_*().

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v7 14/15] power: bq24257: Add platform data based initialization

[Andreas Dannenberg]

On Mon, Sep 28, 2015 at 07:58:12PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Sep 25, 2015 at 10:54:19AM -0500, Andreas Dannenberg wrote:
> > 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.
> 
> Which board is this btw? Looks like most arm board code is going
> away in the next few kernel releases, so I wonder if this patch
> will just increase maintainence without any proper user.

There is no particular board I'm targeting this for however platform
data support has been tested with a board file that I created for one of
the TI devices (don't worry I won't try to upstream that one :)

>From a silicon vendor POV I want to make sure to make it easy to use
these devices for the largest audience possible hence the added pdata
support as there are still archs out there that don't support DT/ACPI.

With the changes based on your suggestion to use the gpiod interface
also for pdata this patch has become almost trivial and so should be the
effort removing this support should it ever become necessary.

Regards,

--
Andreas Dannenberg
Texas Instruments Inc

Re: [PATCH v7 00/15] power: bq24257: Add support for bq24250/bq24251

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

Hi,

On Fri, Sep 25, 2015 at 10:54:05AM -0500, Andreas Dannenberg wrote:
> 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.

I reviewed all patches, those without comments look fine to me. I
did not queue them because of the mentioned dependencies on previous
patches, which got comments from me.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v7 15/15] Documentation: power: bq24257: Document exported sysfs entries

[Andreas Dannenberg]

Document the settings exported by bq24257 charger driver through sysfs
entries:
- ovp_voltage
- in_dpm_voltage
- high_impedance_enable
- sysoff_enable

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
---
 Documentation/ABI/testing/sysfs-class-power | 58 +++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 369d2a2..d96b5fc 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -74,3 +74,61 @@ Description:
 
 		Valid values:
 		- 0 - 70 (minutes), step by 10 (rounded down)
+
+What:		/sys/class/power_supply/bq24257-charger/ovp_voltage
+Date:		September 2015
+KernelVersion: 4.3.0
+Contact:	Andreas Dannenberg <dannenberg@ti.com>
+Description:
+		This entry configures the overvoltage protection feature of bq24257-
+		type charger devices. This feature protects the device and other
+		components against damage from overvoltage on the input supply. See
+		device datasheet for details.
+
+		Valid values:
+		- 6000000, 6500000, 7000000, 8000000, 9000000, 9500000, 10000000,
+		  10500000 (all uV)
+
+What:		/sys/class/power_supply/bq24257-charger/in_dpm_voltage
+Date:		September 2015
+KernelVersion: 4.3.0
+Contact:	Andreas Dannenberg <dannenberg@ti.com>
+Description:
+		This entry configures the input dynamic power path management voltage of
+		bq24257-type charger devices. Once the supply drops to the configured
+		voltage, the input current limit is reduced down to prevent the further
+		drop of the supply. When the IC enters this mode, the charge current is
+		lower than the set value. See device datasheet for details.
+
+		Valid values:
+		- 4200000, 4280000, 4360000, 4440000, 4520000, 4600000, 4680000,
+		  4760000 (all uV)
+
+What:		/sys/class/power_supply/bq24257-charger/high_impedance_enable
+Date:		September 2015
+KernelVersion: 4.3.0
+Contact:	Andreas Dannenberg <dannenberg@ti.com>
+Description:
+		This entry allows enabling the high-impedance mode of bq24257-type
+		charger devices. If enabled, it places the charger IC into low power
+		standby mode with the switch mode controller disabled. When disabled,
+		the charger operates normally. See device datasheet for details.
+
+		Valid values:
+		- 1: enabled
+		- 0: disabled
+
+What:		/sys/class/power_supply/bq24257-charger/sysoff_enable
+Date:		September 2015
+KernelVersion: 4.3.0
+Contact:	Andreas Dannenberg <dannenberg@ti.com>
+Description:
+		This entry allows enabling the sysoff mode of bq24257-type charger
+		devices. If enabled and the input is removed, the internal battery FET
+		is turned off in order to reduce the leakage from the BAT pin to less
+		than 1uA. Note that on some devices/systems this disconnects the battery
+		from the system. See device datasheet for details.
+
+		Valid values:
+		- 1: enabled
+		- 0: disabled
-- 
1.9.1

Re: [PATCH v7 00/15] power: bq24257: Add support for bq24250/bq24251

[Krzysztof Kozlowski]

W dniu 26.09.2015 o 00:54, Andreas Dannenberg pisze:
> 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.
> 
> v7:
> - Fixed an issue that would have impacted running bisect on the series
>   and double checked the entire series builds one by one by running
>   a git rebase -i --exec="make ..." ... on the Kernel tree
> - Minor simplification of a return from a function
> 
> v6:
> - Removed the ability to configure the IRQ through "stat-gpios"
> - Allow re-configuring the input_current_limit via sysfs also when
>   charger auto-detection is enabled
> - Use gpiod interface for initialization as a platform device (nice
>   GPIO setup simplification)
> - Document all newly introduced sysfs nodes
> - Streamlined/fixed/simplified charger state handling as it relates
>   to battery removal/insertion and the associated handling of the
>   input current limit setting
> - Minor simplifications/cleanup
> - Rebased onto git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-
>   power-supply.git (branch "next") and completely retested

It would be useful if you would mark which patches have changed. I
reviewed already the first patch but you did not add my reviewed-by tag.
I suppose something changed... but I can't find the exact reason for
missing review.

Best regards,
Krzysztof