linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-05-15 14:06 ` [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip Laurentiu Palcu
@ 2015-05-02 14:59   ` Pavel Machek
  2015-06-24  7:32     ` Laurentiu Palcu
  2015-05-19  8:40   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 12+ messages in thread
From: Pavel Machek @ 2015-05-02 14:59 UTC (permalink / raw)
  To: Laurentiu Palcu
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse, devicetree, linux-kernel, linux-pm


Should this link....

> More details about the chip can be found here:
> http://www.ti.com/product/bq25890

> @@ -0,0 +1,998 @@
> +/*
> + * TI BQ25890 charger driver
> + *
> + * Copyright (C) 2015 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

> + *
> + */

Go here?
									Pavel

> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/usb/phy.h>
> +
> +#include <linux/acpi.h>
> +#include <linux/of.h>
> +
> +#define BQ25890_MANUFACTURER		"Texas Instruments"
> +#define BQ25890_IRQ_PIN			"bq25890_irq"
> +
> +#define BQ25890_ID			3
> +
> +enum bq25890_fields {
> +	F_EN_HIZ, F_EN_ILIM, F_IILIM,				     /* Reg00 */
> +	F_BHOT, F_BCOLD, F_VINDPM_OFS,				     /* Reg01 */
> +	F_CONV_START, F_CONV_RATE, F_BOOSTF, F_ICO_EN,
> +	F_HVDCP_EN, F_MAXC_EN, F_FORCE_DPM, F_AUTO_DPDM_EN,	     /* Reg02 */
> +	F_BAT_LOAD_EN, F_WD_RST, F_OTG_CFG, F_CHG_CFG, F_SYSVMIN,    /* Reg03 */
> +	F_PUMPX_EN, F_ICHG,					     /* Reg04 */
> +	F_IPRECHG, F_ITERM,					     /* Reg05 */
> +	F_VREG, F_BATLOWV, F_VRECHG,				     /* Reg06 */
> +	F_TERM_EN, F_STAT_DIS, F_WD, F_TMR_EN, F_CHG_TMR,
> +	F_JEITA_ISET,						     /* Reg07 */
> +	F_BATCMP, F_VCLAMP, F_TREG,				     /* Reg08 */
> +	F_FORCE_ICO, F_TMR2X_EN, F_BATFET_DIS, F_JEITA_VSET,
> +	F_BATFET_DLY, F_BATFET_RST_EN, F_PUMPX_UP, F_PUMPX_DN,	     /* Reg09 */
> +	F_BOOSTV, F_BOOSTI,					     /* Reg0A */
> +	F_VBUS_STAT, F_CHG_STAT, F_PG_STAT, F_SDP_STAT, F_VSYS_STAT, /* Reg0B */
> +	F_WD_FAULT, F_BOOST_FAULT, F_CHG_FAULT, F_BAT_FAULT,
> +	F_NTC_FAULT,						     /* Reg0C */
> +	F_FORCE_VINDPM, F_VINDPM,				     /* Reg0D */
> +	F_THERM_STAT, F_BATV,					     /* Reg0E */
> +	F_SYSV,							     /* Reg0F */
> +	F_TSPCT,						     /* Reg10 */
> +	F_VBUS_GD, F_VBUSV,					     /* Reg11 */
> +	F_ICHGR,						     /* Reg12 */
> +	F_VDPM_STAT, F_IDPM_STAT, F_IDPM_LIM,			     /* Reg13 */
> +	F_REG_RST, F_ICO_OPTIMIZED, F_PN, F_TS_PROFILE, F_DEV_REV,   /* Reg14 */
> +
> +	F_MAX_FIELDS
> +};
> +
> +/* initial field values, converted to register values */
> +struct bq25890_init_data {
> +	u8 ichg;	/* charge current		*/
> +	u8 vreg;	/* regulation voltage		*/
> +	u8 iterm;	/* termination current		*/
> +	u8 iprechg;	/* precharge current		*/
> +	u8 sysvmin;	/* minimum system voltage limit */
> +	u8 boostv;	/* boost regulation voltage	*/
> +	u8 boosti;	/* boost current limit		*/
> +	u8 boostf;	/* boost frequency		*/
> +	u8 ilim_en;	/* enable ILIM pin		*/
> +	u8 treg;	/* thermal regulation threshold */
> +};
> +
> +struct bq25890_state {
> +	u8 online;
> +	u8 chrg_status;
> +	u8 chrg_fault;
> +	u8 vsys_status;
> +	u8 boost_fault;
> +	u8 bat_fault;
> +};
> +
> +struct bq25890_device {
> +	struct i2c_client *client;
> +	struct device *dev;
> +	struct power_supply *charger;
> +
> +	struct usb_phy *usb_phy;
> +	struct notifier_block usb_nb;
> +	struct work_struct usb_work;
> +	unsigned long usb_event;
> +
> +	struct regmap *rmap;
> +	struct regmap_field *rmap_fields[F_MAX_FIELDS];
> +
> +	u8 chip_id;
> +	struct bq25890_init_data init_data;
> +	struct bq25890_state state;
> +
> +	struct mutex lock; /* protect state data */
> +};
> +
> +static const struct regmap_range bq25890_readonly_reg_ranges[] = {
> +	regmap_reg_range(0x0b, 0x0c),
> +	regmap_reg_range(0x0e, 0x13),
> +};
> +
> +static const struct regmap_access_table bq25890_writeable_regs = {
> +	.no_ranges = bq25890_readonly_reg_ranges,
> +	.n_no_ranges = ARRAY_SIZE(bq25890_readonly_reg_ranges),
> +};
> +
> +static const struct regmap_range bq25890_volatile_reg_ranges[] = {
> +	regmap_reg_range(0x00, 0x00),
> +	regmap_reg_range(0x09, 0x09),
> +	regmap_reg_range(0x0b, 0x0c),
> +	regmap_reg_range(0x0e, 0x14),
> +};
> +
> +static const struct regmap_access_table bq25890_volatile_regs = {
> +	.yes_ranges = bq25890_volatile_reg_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(bq25890_volatile_reg_ranges),
> +};
> +
> +static const struct regmap_config bq25890_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.max_register = 0x14,
> +	.cache_type = REGCACHE_RBTREE,
> +
> +	.wr_table = &bq25890_writeable_regs,
> +	.volatile_table = &bq25890_volatile_regs,
> +};
> +
> +static const struct reg_field bq25890_reg_fields[] = {
> +	/* REG00 */
> +	[F_EN_HIZ]		= REG_FIELD(0x00, 7, 7),
> +	[F_EN_ILIM]		= REG_FIELD(0x00, 6, 6),
> +	[F_IILIM]		= REG_FIELD(0x00, 0, 5),
> +	/* REG01 */
> +	[F_BHOT]		= REG_FIELD(0x01, 6, 7),
> +	[F_BCOLD]		= REG_FIELD(0x01, 5, 5),
> +	[F_VINDPM_OFS]		= REG_FIELD(0x01, 0, 4),
> +	/* REG02 */
> +	[F_CONV_START]		= REG_FIELD(0x02, 7, 7),
> +	[F_CONV_RATE]		= REG_FIELD(0x02, 6, 6),
> +	[F_BOOSTF]		= REG_FIELD(0x02, 5, 5),
> +	[F_ICO_EN]		= REG_FIELD(0x02, 4, 4),
> +	[F_HVDCP_EN]		= REG_FIELD(0x02, 3, 3),
> +	[F_MAXC_EN]		= REG_FIELD(0x02, 2, 2),
> +	[F_FORCE_DPM]		= REG_FIELD(0x02, 1, 1),
> +	[F_AUTO_DPDM_EN]	= REG_FIELD(0x02, 0, 0),
> +	/* REG03 */
> +	[F_BAT_LOAD_EN]		= REG_FIELD(0x03, 7, 7),
> +	[F_WD_RST]		= REG_FIELD(0x03, 6, 6),
> +	[F_OTG_CFG]		= REG_FIELD(0x03, 5, 5),
> +	[F_CHG_CFG]		= REG_FIELD(0x03, 4, 4),
> +	[F_SYSVMIN]		= REG_FIELD(0x03, 1, 3),
> +	/* REG04 */
> +	[F_PUMPX_EN]		= REG_FIELD(0x04, 7, 7),
> +	[F_ICHG]		= REG_FIELD(0x04, 0, 6),
> +	/* REG05 */
> +	[F_IPRECHG]		= REG_FIELD(0x05, 4, 7),
> +	[F_ITERM]		= REG_FIELD(0x05, 0, 3),
> +	/* REG06 */
> +	[F_VREG]		= REG_FIELD(0x06, 2, 7),
> +	[F_BATLOWV]		= REG_FIELD(0x06, 1, 1),
> +	[F_VRECHG]		= REG_FIELD(0x06, 0, 0),
> +	/* REG07 */
> +	[F_TERM_EN]		= REG_FIELD(0x07, 7, 7),
> +	[F_STAT_DIS]		= REG_FIELD(0x07, 6, 6),
> +	[F_WD]			= REG_FIELD(0x07, 4, 5),
> +	[F_TMR_EN]		= REG_FIELD(0x07, 3, 3),
> +	[F_CHG_TMR]		= REG_FIELD(0x07, 1, 2),
> +	[F_JEITA_ISET]		= REG_FIELD(0x07, 0, 0),
> +	/* REG08 */
> +	[F_BATCMP]		= REG_FIELD(0x08, 6, 7),
> +	[F_VCLAMP]		= REG_FIELD(0x08, 2, 4),
> +	[F_TREG]		= REG_FIELD(0x08, 0, 1),
> +	/* REG09 */
> +	[F_FORCE_ICO]		= REG_FIELD(0x09, 7, 7),
> +	[F_TMR2X_EN]		= REG_FIELD(0x09, 6, 6),
> +	[F_BATFET_DIS]		= REG_FIELD(0x09, 5, 5),
> +	[F_JEITA_VSET]		= REG_FIELD(0x09, 4, 4),
> +	[F_BATFET_DLY]		= REG_FIELD(0x09, 3, 3),
> +	[F_BATFET_RST_EN]	= REG_FIELD(0x09, 2, 2),
> +	[F_PUMPX_UP]		= REG_FIELD(0x09, 1, 1),
> +	[F_PUMPX_DN]		= REG_FIELD(0x09, 0, 0),
> +	/* REG0A */
> +	[F_BOOSTV]		= REG_FIELD(0x0A, 4, 7),
> +	[F_BOOSTI]		= REG_FIELD(0x0A, 0, 2),
> +	/* REG0B */
> +	[F_VBUS_STAT]		= REG_FIELD(0x0B, 5, 7),
> +	[F_CHG_STAT]		= REG_FIELD(0x0B, 3, 4),
> +	[F_PG_STAT]		= REG_FIELD(0x0B, 2, 2),
> +	[F_SDP_STAT]		= REG_FIELD(0x0B, 1, 1),
> +	[F_VSYS_STAT]		= REG_FIELD(0x0B, 0, 0),
> +	/* REG0C */
> +	[F_WD_FAULT]		= REG_FIELD(0x0C, 7, 7),
> +	[F_BOOST_FAULT]		= REG_FIELD(0x0C, 6, 6),
> +	[F_CHG_FAULT]		= REG_FIELD(0x0C, 4, 5),
> +	[F_BAT_FAULT]		= REG_FIELD(0x0C, 3, 3),
> +	[F_NTC_FAULT]		= REG_FIELD(0x0C, 0, 2),
> +	/* REG0D */
> +	[F_FORCE_VINDPM]	= REG_FIELD(0x0D, 7, 7),
> +	[F_VINDPM]		= REG_FIELD(0x0D, 0, 6),
> +	/* REG0E */
> +	[F_THERM_STAT]		= REG_FIELD(0x0E, 7, 7),
> +	[F_BATV]		= REG_FIELD(0x0E, 0, 6),
> +	/* REG0F */
> +	[F_SYSV]		= REG_FIELD(0x0F, 0, 6),
> +	/* REG10 */
> +	[F_TSPCT]		= REG_FIELD(0x10, 0, 6),
> +	/* REG11 */
> +	[F_VBUS_GD]		= REG_FIELD(0x11, 7, 7),
> +	[F_VBUSV]		= REG_FIELD(0x11, 0, 6),
> +	/* REG12 */
> +	[F_ICHGR]		= REG_FIELD(0x12, 0, 6),
> +	/* REG13 */
> +	[F_VDPM_STAT]		= REG_FIELD(0x13, 7, 7),
> +	[F_IDPM_STAT]		= REG_FIELD(0x13, 6, 6),
> +	[F_IDPM_LIM]		= REG_FIELD(0x13, 0, 5),
> +	/* REG14 */
> +	[F_REG_RST]		= REG_FIELD(0x14, 7, 7),
> +	[F_ICO_OPTIMIZED]	= REG_FIELD(0x14, 6, 6),
> +	[F_PN]			= REG_FIELD(0x14, 3, 5),
> +	[F_TS_PROFILE]		= REG_FIELD(0x14, 2, 2),
> +	[F_DEV_REV]		= REG_FIELD(0x14, 0, 1)
> +};
> +
> +/*
> + * Most of the val -> idx conversions can be computed, given the minimum,
> + * maximum and the step between values. For the rest of conversions, we use
> + * lookup tables.
> + */
> +enum bq25890_table_ids {
> +	/* range tables */
> +	TBL_ICHG,
> +	TBL_ITERM,
> +	TBL_IPRECHG,
> +	TBL_VREG,
> +	TBL_BATCMP,
> +	TBL_VCLAMP,
> +	TBL_BOOSTV,
> +	TBL_SYSVMIN,
> +
> +	/* lookup tables */
> +	TBL_TREG,
> +	TBL_BOOSTI,
> +};
> +
> +/* Thermal Regulation Threshold lookup table, in degrees Celsius */
> +static const u32 bq25890_treg_tbl[] = { 60, 80, 100, 120 };
> +
> +#define BQ25890_TREG_TBL_SIZE		ARRAY_SIZE(bq25890_treg_tbl)
> +
> +/* Boost mode current limit lookup table, in uA */
> +static const u32 bq25890_boosti_tbl[] = {
> +	500000, 700000, 1100000, 1300000, 1600000, 1800000, 2100000, 2400000
> +};
> +
> +#define BQ25890_BOOSTI_TBL_SIZE		ARRAY_SIZE(bq25890_boosti_tbl)
> +
> +struct bq25890_range {
> +	u32 min;
> +	u32 max;
> +	u32 step;
> +};
> +
> +struct bq25890_lookup {
> +	const u32 *tbl;
> +	u32 size;
> +};
> +
> +static const union {
> +	struct bq25890_range  rt;
> +	struct bq25890_lookup lt;
> +} bq25890_tables[] = {
> +	/* range tables */
> +	[TBL_ICHG] =	{ .rt = {0,	  5056000, 64000} },	 /* uA */
> +	[TBL_ITERM] =	{ .rt = {64000,   1024000, 64000} },	 /* uA */
> +	[TBL_VREG] =	{ .rt = {3840000, 4608000, 16000} },	 /* uV */
> +	[TBL_BATCMP] =	{ .rt = {0,	  140,     20} },	 /* mOhm */
> +	[TBL_VCLAMP] =	{ .rt = {0,	  224000,  32000} },	 /* uV */
> +	[TBL_BOOSTV] =	{ .rt = {4550000, 5510000, 64000} },	 /* uV */
> +	[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} },	 /* uV */
> +
> +	/* lookup tables */
> +	[TBL_TREG] =	{ .lt = {bq25890_treg_tbl, BQ25890_TREG_TBL_SIZE} },
> +	[TBL_BOOSTI] =	{ .lt = {bq25890_boosti_tbl, BQ25890_BOOSTI_TBL_SIZE} }
> +};
> +
> +static int bq25890_field_read(struct bq25890_device *bq,
> +			      enum bq25890_fields field_id)
> +{
> +	int ret;
> +	int val;
> +
> +	ret = regmap_field_read(bq->rmap_fields[field_id], &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return val;
> +}
> +
> +static int bq25890_field_write(struct bq25890_device *bq,
> +			       enum bq25890_fields field_id, u8 val)
> +{
> +	return regmap_field_write(bq->rmap_fields[field_id], val);
> +}
> +
> +static u8 bq25890_find_idx(u32 value, enum bq25890_table_ids id)
> +{
> +	u8 idx;
> +
> +	if (id >= TBL_TREG) {
> +		const u32 *tbl = bq25890_tables[id].lt.tbl;
> +		u32 tbl_size = bq25890_tables[id].lt.size;
> +
> +		for (idx = 1; idx < tbl_size && tbl[idx] <= value; idx++)
> +			;
> +	} else {
> +		const struct bq25890_range *rtbl = &bq25890_tables[id].rt;
> +		u8 rtbl_size;
> +
> +		rtbl_size = (rtbl->max - rtbl->min) / rtbl->step + 1;
> +
> +		for (idx = 1;
> +		     idx < rtbl_size && idx * rtbl->step + rtbl->min <= value;
> +		     idx++)
> +			;
> +	}
> +
> +	return idx - 1;
> +}
> +
> +static u32 bq25890_find_val(u8 idx, enum bq25890_table_ids id)
> +{
> +	const struct bq25890_range *rtbl;
> +
> +	/* lookup table? */
> +	if (id >= TBL_TREG)
> +		return bq25890_tables[id].lt.tbl[idx];
> +
> +	/* range table */
> +	rtbl = &bq25890_tables[id].rt;
> +
> +	return(rtbl->min + idx * rtbl->step);
> +}
> +
> +enum bq25890_status {
> +	STATUS_NOT_CHARGING,
> +	STATUS_PRE_CHARGING,
> +	STATUS_FAST_CHARGING,
> +	STATUS_TERMINATION_DONE,
> +};
> +
> +enum bq25890_chrg_fault {
> +	CHRG_FAULT_NORMAL,
> +	CHRG_FAULT_INPUT,
> +	CHRG_FAULT_THERMAL_SHUTDOWN,
> +	CHRG_FAULT_TIMER_EXPIRED,
> +};
> +
> +static int bq25890_power_supply_get_property(struct power_supply *psy,
> +					     enum power_supply_property psp,
> +					     union power_supply_propval *val)
> +{
> +	int ret;
> +	struct bq25890_device *bq = power_supply_get_drvdata(psy);
> +	struct bq25890_state state;
> +
> +	mutex_lock(&bq->lock);
> +	state = bq->state;
> +	mutex_unlock(&bq->lock);
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		if (!state.online)
> +			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> +		else if (state.chrg_status == STATUS_NOT_CHARGING)
> +			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +		else if (state.chrg_status == STATUS_PRE_CHARGING ||
> +			 state.chrg_status == STATUS_FAST_CHARGING)
> +			val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +		else if (state.chrg_status == STATUS_TERMINATION_DONE)
> +			val->intval = POWER_SUPPLY_STATUS_FULL;
> +		else
> +			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> +
> +		break;
> +
> +	case POWER_SUPPLY_PROP_MANUFACTURER:
> +		val->strval = BQ25890_MANUFACTURER;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval = state.online;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		if (!state.chrg_fault && !state.bat_fault && !state.boost_fault)
> +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		else if (state.bat_fault)
> +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +		else if (state.chrg_fault == CHRG_FAULT_TIMER_EXPIRED)
> +			val->intval = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> +		else if (state.chrg_fault == CHRG_FAULT_THERMAL_SHUTDOWN)
> +			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
> +		else
> +			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> +		ret = bq25890_field_read(bq, F_ICHGR); /* read measured value */
> +		if (ret < 0)
> +			return ret;
> +
> +		/* converted_val = ADC_val * 50mA (table 10.3.19) */
> +		val->intval = ret * 50000;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +		val->intval = bq25890_tables[TBL_ICHG].rt.max;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> +		if (!state.online) {
> +			val->intval = 0;
> +			break;
> +		}
> +
> +		ret = bq25890_field_read(bq, F_BATV); /* read measured value */
> +		if (ret < 0)
> +			return ret;
> +
> +		/* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
> +		val->intval = 2304000 + ret * 20000;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +		val->intval = bq25890_tables[TBL_VREG].rt.max;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
> +		val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bq25890_get_chip_state(struct bq25890_device *bq,
> +				  struct bq25890_state *state)
> +{
> +	int i, ret;
> +
> +	struct {
> +		enum bq25890_fields id;
> +		u8 *data;
> +	} state_fields[] = {
> +		{F_CHG_STAT,	&state->chrg_status},
> +		{F_PG_STAT,	&state->online},
> +		{F_VSYS_STAT,	&state->vsys_status},
> +		{F_BOOST_FAULT, &state->boost_fault},
> +		{F_BAT_FAULT,	&state->bat_fault},
> +		{F_CHG_FAULT,	&state->chrg_fault}
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(state_fields); i++) {
> +		ret = bq25890_field_read(bq, state_fields[i].id);
> +		if (ret < 0)
> +			return ret;
> +
> +		*state_fields[i].data = ret;
> +	}
> +
> +	dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT=%d/%d/%d\n",
> +		state->chrg_status, state->online, state->vsys_status,
> +		state->chrg_fault, state->boost_fault, state->bat_fault);
> +
> +	return 0;
> +}
> +
> +static bool bq25890_state_changed(struct bq25890_device *bq,
> +				  struct bq25890_state *new_state)
> +{
> +	struct bq25890_state old_state;
> +
> +	mutex_lock(&bq->lock);
> +	old_state = bq->state;
> +	mutex_unlock(&bq->lock);
> +
> +	return (old_state.chrg_status != new_state->chrg_status ||
> +		old_state.chrg_fault != new_state->chrg_fault	||
> +		old_state.online != new_state->online		||
> +		old_state.bat_fault != new_state->bat_fault	||
> +		old_state.boost_fault != new_state->boost_fault ||
> +		old_state.vsys_status != new_state->vsys_status);
> +}
> +
> +static void bq25890_handle_state_change(struct bq25890_device *bq,
> +					struct bq25890_state *new_state)
> +{
> +	int ret;
> +	struct bq25890_state old_state;
> +
> +	mutex_lock(&bq->lock);
> +	old_state = bq->state;
> +	mutex_unlock(&bq->lock);
> +
> +	if (!new_state->online) {			     /* power removed */
> +		/* disable ADC */
> +		ret = bq25890_field_write(bq, F_CONV_START, 0);
> +		if (ret < 0)
> +			goto error;
> +	} else if (!old_state.online) {			    /* power inserted */
> +		/* enable ADC, to have control of charge current/voltage */
> +		ret = bq25890_field_write(bq, F_CONV_START, 1);
> +		if (ret < 0)
> +			goto error;
> +	}
> +
> +	return;
> +
> +error:
> +	dev_err(bq->dev, "%s: Error communicating with the chip.\n", __func__);
> +}
> +
> +static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
> +{
> +	struct bq25890_device *bq = private;
> +	int ret;
> +	struct bq25890_state state;
> +
> +	ret = bq25890_get_chip_state(bq, &state);
> +	if (ret < 0)
> +		goto handled;
> +
> +	if (!bq25890_state_changed(bq, &state))
> +		goto handled;
> +
> +	bq25890_handle_state_change(bq, &state);
> +
> +	mutex_lock(&bq->lock);
> +	bq->state = state;
> +	mutex_unlock(&bq->lock);
> +
> +	power_supply_changed(bq->charger);
> +
> +handled:
> +	return IRQ_HANDLED;
> +}
> +
> +static int bq25890_chip_reset(struct bq25890_device *bq)
> +{
> +	int ret;
> +
> +	ret = bq25890_field_write(bq, F_REG_RST, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	do {
> +		ret = bq25890_field_read(bq, F_REG_RST);
> +		if (ret < 0)
> +			return ret;
> +
> +		usleep_range(5, 10);
> +	} while (ret == 1);
> +
> +	return 0;
> +}
> +
> +static int bq25890_hw_init(struct bq25890_device *bq)
> +{
> +	int ret;
> +	int i;
> +	struct bq25890_state state;
> +
> +	const struct {
> +		enum bq25890_fields id;
> +		u32 value;
> +	} init_data[] = {
> +		{F_ICHG,	 bq->init_data.ichg},
> +		{F_VREG,	 bq->init_data.vreg},
> +		{F_ITERM,	 bq->init_data.iterm},
> +		{F_IPRECHG,	 bq->init_data.iprechg},
> +		{F_SYSVMIN,	 bq->init_data.sysvmin},
> +		{F_BOOSTV,	 bq->init_data.boostv},
> +		{F_BOOSTI,	 bq->init_data.boosti},
> +		{F_BOOSTF,	 bq->init_data.boostf},
> +		{F_EN_ILIM,	 bq->init_data.ilim_en},
> +		{F_TREG,	 bq->init_data.treg}
> +	};
> +
> +	ret = bq25890_chip_reset(bq);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* disable watchdog */
> +	ret = bq25890_field_write(bq, F_WD, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* initialize currents/voltages and other parameters */
> +	for (i = 0; i < ARRAY_SIZE(init_data); i++) {
> +		ret = bq25890_field_write(bq, init_data[i].id,
> +					  init_data[i].value);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* Configure ADC for continuous conversions. This does not enable it. */
> +	ret = bq25890_field_write(bq, F_CONV_RATE, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = bq25890_get_chip_state(bq, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	mutex_lock(&bq->lock);
> +	bq->state = state;
> +	mutex_unlock(&bq->lock);
> +
> +	return 0;
> +}
> +
> +static enum power_supply_property bq25890_power_supply_props[] = {
> +	POWER_SUPPLY_PROP_MANUFACTURER,
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> +};
> +
> +static char *bq25890_charger_supplied_to[] = {
> +	"main-battery",
> +};
> +
> +static const struct power_supply_desc bq25890_power_supply_desc = {
> +	.name = "bq25890-charger",
> +	.type = POWER_SUPPLY_TYPE_USB,
> +	.properties = bq25890_power_supply_props,
> +	.num_properties = ARRAY_SIZE(bq25890_power_supply_props),
> +	.get_property = bq25890_power_supply_get_property,
> +};
> +
> +static int bq25890_power_supply_init(struct bq25890_device *bq)
> +{
> +	struct power_supply_config psy_cfg = { .drv_data = bq, };
> +
> +	psy_cfg.supplied_to = bq25890_charger_supplied_to;
> +	psy_cfg.num_supplicants = ARRAY_SIZE(bq25890_charger_supplied_to);
> +
> +	bq->charger = power_supply_register(bq->dev, &bq25890_power_supply_desc,
> +					    &psy_cfg);
> +	if (IS_ERR(bq->charger))
> +		return PTR_ERR(bq->charger);
> +
> +	return 0;
> +}
> +
> +static void bq25890_usb_work(struct work_struct *data)
> +{
> +	int ret;
> +	struct bq25890_device *bq =
> +			container_of(data, struct bq25890_device, usb_work);
> +
> +	switch (bq->usb_event) {
> +	case USB_EVENT_ID:
> +		/* Enable boost mode */
> +		ret = bq25890_field_write(bq, F_OTG_CFG, 1);
> +		if (ret < 0)
> +			goto error;
> +		break;
> +
> +	case USB_EVENT_NONE:
> +		/* Disable boost mode */
> +		ret = bq25890_field_write(bq, F_OTG_CFG, 0);
> +		if (ret < 0)
> +			goto error;
> +
> +		power_supply_changed(bq->charger);
> +		break;
> +	}
> +
> +	return;
> +
> +error:
> +	dev_err(bq->dev, "%s - Error switching to boost/charger mode.\n",
> +		__func__);
> +}
> +
> +static int bq25890_usb_notifier(struct notifier_block *nb, unsigned long val,
> +				void *priv)
> +{
> +	struct bq25890_device *bq =
> +			container_of(nb, struct bq25890_device, usb_nb);
> +
> +	bq->usb_event = val;
> +	schedule_work(&bq->usb_work);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int bq25890_irq_probe(struct bq25890_device *bq)
> +{
> +	int ret;
> +	struct gpio_desc *irq;
> +
> +	irq = devm_gpiod_get_index(bq->dev, BQ25890_IRQ_PIN, 0);
> +	if (IS_ERR(irq)) {
> +		dev_err(bq->dev, "could not probe irq pin\n");
> +		return PTR_ERR(irq);
> +	}
> +
> +	ret = gpiod_direction_input(irq);
> +	if (ret < 0)
> +		return ret;
> +
> +	return gpiod_to_irq(irq);
> +}
> +
> +static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
> +{
> +	int ret;
> +	u32 property;
> +	int i;
> +	struct bq25890_init_data *init = &bq->init_data;
> +	struct {
> +		char *name;
> +		bool optional;
> +		enum bq25890_table_ids tbl_id;
> +		u8 *conv_data; /* holds converted value from given property */
> +	} props[] = {
> +		/* required properties */
> +		{"ti,charge-current", false, TBL_ICHG, &init->ichg},
> +		{"ti,battery-regulation-voltage", false, TBL_VREG, &init->vreg},
> +		{"ti,termination-current", false, TBL_ITERM, &init->iterm},
> +		{"ti,precharge-current", false, TBL_ITERM, &init->iprechg},
> +		{"ti,minimum-sys-voltage", false, TBL_SYSVMIN, &init->sysvmin},
> +		{"ti,boost-voltage", false, TBL_BOOSTV, &init->boostv},
> +		{"ti,boost-max-current", false, TBL_BOOSTI, &init->boosti},
> +
> +		/* optional properties */
> +		{"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg}
> +	};
> +
> +	/* initialize data for optional properties */
> +	init->treg = 3; /* 120 degrees Celsius */
> +
> +	for (i = 0; i < ARRAY_SIZE(props); i++) {
> +		ret = device_property_read_u32(bq->dev, props[i].name,
> +					       &property);
> +		if (ret < 0) {
> +			if (props[i].optional)
> +				continue;
> +
> +			return ret;
> +		}
> +
> +		*props[i].conv_data = bq25890_find_idx(property,
> +						       props[i].tbl_id);
> +	}
> +
> +	return 0;
> +}
> +
> +static int bq25890_fw_probe(struct bq25890_device *bq)
> +{
> +	int ret;
> +	struct bq25890_init_data *init = &bq->init_data;
> +
> +	ret = bq25890_fw_read_u32_props(bq);
> +	if (ret < 0)
> +		return ret;
> +
> +	init->ilim_en = device_property_read_bool(bq->dev, "ti,use-ilim-pin");
> +	init->boostf = device_property_read_bool(bq->dev, "ti,boost-low-freq");
> +
> +	return 0;
> +}
> +
> +static int bq25890_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +	struct device *dev = &client->dev;
> +	struct bq25890_device *bq;
> +	int ret;
> +	int i;
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> +		dev_err(dev, "No support for SMBUS_BYTE_DATA\n");
> +		return -ENODEV;
> +	}
> +
> +	bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
> +	if (!bq)
> +		return -ENOMEM;
> +
> +	bq->client = client;
> +	bq->dev = dev;
> +
> +	mutex_init(&bq->lock);
> +
> +	bq->rmap = devm_regmap_init_i2c(client, &bq25890_regmap_config);
> +	if (IS_ERR(bq->rmap)) {
> +		dev_err(dev, "failed to allocate register map\n");
> +		return PTR_ERR(bq->rmap);
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(bq25890_reg_fields); i++) {
> +		const struct reg_field *reg_fields = bq25890_reg_fields;
> +
> +		bq->rmap_fields[i] = devm_regmap_field_alloc(dev, bq->rmap,
> +							     reg_fields[i]);
> +		if (IS_ERR(bq->rmap_fields[i])) {
> +			dev_err(dev, "cannot allocate regmap field\n");
> +			return PTR_ERR(bq->rmap_fields[i]);
> +		}
> +	}
> +
> +	i2c_set_clientdata(client, bq);
> +
> +	bq->chip_id = bq25890_field_read(bq, F_PN);
> +	if (bq->chip_id < 0) {
> +		dev_err(dev, "Cannot read chip ID.\n");
> +		return ret;
> +	}
> +
> +	if (bq->chip_id != BQ25890_ID) {
> +		dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
> +		return -ENODEV;
> +	}
> +
> +	if (!dev->platform_data) {
> +		ret = bq25890_fw_probe(bq);
> +		if (ret < 0) {
> +			dev_err(dev, "Cannot read device properties.\n");
> +			return ret;
> +		}
> +	} else {
> +		return -ENODEV;
> +	}
> +
> +	ret = bq25890_hw_init(bq);
> +	if (ret < 0) {
> +		dev_err(dev, "Cannot initialize the chip.\n");
> +		return ret;
> +	}
> +
> +	if (client->irq <= 0)
> +		client->irq = bq25890_irq_probe(bq);
> +
> +	if (client->irq < 0) {
> +		dev_err(dev, "no irq resource found\n");
> +		return client->irq;
> +	}
> +
> +	/* OTG reporting */
> +	bq->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> +	if (!IS_ERR_OR_NULL(bq->usb_phy)) {
> +		INIT_WORK(&bq->usb_work, bq25890_usb_work);
> +		bq->usb_nb.notifier_call = bq25890_usb_notifier;
> +		usb_register_notifier(bq->usb_phy, &bq->usb_nb);
> +	}
> +
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +					bq25890_irq_handler_thread,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					BQ25890_IRQ_PIN, bq);
> +	if (ret)
> +		goto irq_fail;
> +
> +	ret = bq25890_power_supply_init(bq);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to register power supply\n");
> +		goto irq_fail;
> +	}
> +
> +	return 0;
> +
> +irq_fail:
> +	if (!IS_ERR_OR_NULL(bq->usb_phy))
> +		usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
> +
> +	return ret;
> +}
> +
> +static int bq25890_remove(struct i2c_client *client)
> +{
> +	struct bq25890_device *bq = i2c_get_clientdata(client);
> +
> +	if (!IS_ERR_OR_NULL(bq->usb_phy))
> +		usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
> +
> +	power_supply_unregister(bq->charger);
> +
> +	/* reset all registers to default values */
> +	bq25890_chip_reset(bq);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int bq25890_suspend(struct device *dev)
> +{
> +	struct bq25890_device *bq = dev_get_drvdata(dev);
> +
> +	/*
> +	 * If charger is removed, while in suspend, make sure ADC is diabled
> +	 * since it consumes slightly more power.
> +	 */
> +	return bq25890_field_write(bq, F_CONV_START, 0);
> +}
> +
> +static int bq25890_resume(struct device *dev)
> +{
> +	int ret;
> +	struct bq25890_state state;
> +	struct bq25890_device *bq = dev_get_drvdata(dev);
> +
> +	ret = bq25890_get_chip_state(bq, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	mutex_lock(&bq->lock);
> +	bq->state = state;
> +	mutex_unlock(&bq->lock);
> +
> +	/* Re-enable ADC only if charger is plugged in. */
> +	if (bq->state.online) {
> +		ret = bq25890_field_write(bq, F_CONV_START, 1);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* signal userspace, maybe state changed while suspended */
> +	power_supply_changed(bq->charger);
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops bq25890_pm = {
> +	SET_SYSTEM_SLEEP_PM_OPS(bq25890_suspend, bq25890_resume)
> +};
> +
> +static const struct i2c_device_id bq25890_i2c_ids[] = {
> +	{ "bq25890", 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, bq25890_i2c_ids);
> +
> +static const struct of_device_id bq25890_of_match[] = {
> +	{ .compatible = "ti,bq25890", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, bq25890_of_match);
> +
> +static const struct acpi_device_id bq25890_acpi_match[] = {
> +	{"BQ258900", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, bq25890_acpi_match);
> +
> +static struct i2c_driver bq25890_driver = {
> +	.driver = {
> +		.name = "bq25890-charger",
> +		.of_match_table = of_match_ptr(bq25890_of_match),
> +		.acpi_match_table = ACPI_PTR(bq25890_acpi_match),
> +		.pm = &bq25890_pm,
> +	},
> +	.probe = bq25890_probe,
> +	.remove = bq25890_remove,
> +	.id_table = bq25890_i2c_ids,
> +};
> +module_i2c_driver(bq25890_driver);
> +
> +MODULE_AUTHOR("Laurentiu Palcu <laurentiu.palcu@intel.com>");
> +MODULE_DESCRIPTION("bq25890 charger driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/2] Add support for BQ25890 charger chip
@ 2015-05-15 14:06 Laurentiu Palcu
  2015-05-15 14:06 ` [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings Laurentiu Palcu
  2015-05-15 14:06 ` [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip Laurentiu Palcu
  0 siblings, 2 replies; 12+ messages in thread
From: Laurentiu Palcu @ 2015-05-15 14:06 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse
  Cc: Laurentiu Palcu, devicetree, linux-kernel, linux-pm

Hi,

This series adds support for BQ25890 USB charger chip. Information about
the chip can be found here:

http://www.ti.com/product/bq25890

Thanks,
laurentiu

Laurentiu Palcu (2):
  Documentation: devicetree: Add TI BQ25890 bindings
  power_supply: Add support for TI BQ25890 charger chip

 .../devicetree/bindings/power/bq25890.txt          |  45 +
 drivers/power/Kconfig                              |   7 +
 drivers/power/Makefile                             |   1 +
 drivers/power/bq25890_charger.c                    | 998 +++++++++++++++++++++
 4 files changed, 1051 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/bq25890.txt
 create mode 100644 drivers/power/bq25890_charger.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings
  2015-05-15 14:06 [PATCH 0/2] Add support for BQ25890 charger chip Laurentiu Palcu
@ 2015-05-15 14:06 ` Laurentiu Palcu
  2015-05-16 11:53   ` Krzysztof Kozlowski
  2015-05-15 14:06 ` [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip Laurentiu Palcu
  1 sibling, 1 reply; 12+ messages in thread
From: Laurentiu Palcu @ 2015-05-15 14:06 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse
  Cc: Laurentiu Palcu, devicetree, linux-kernel, linux-pm

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 .../devicetree/bindings/power/bq25890.txt          | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/bq25890.txt

diff --git a/Documentation/devicetree/bindings/power/bq25890.txt b/Documentation/devicetree/bindings/power/bq25890.txt
new file mode 100644
index 0000000..44f50a7
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/bq25890.txt
@@ -0,0 +1,45 @@
+Binding for TI bq25890 Li-Ion Charger
+
+Required properties:
+- compatible: Should contain one of the following:
+    * "ti,bq25890"
+- reg: integer, i2c address of the device.
+- 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,precharge-current: integer, maximum charge current during precharge
+    phase (in uA);
+- ti,minimum-sys-voltage: integer, when battery is charging and it is below
+    minimum system voltage, the system will be regulated above
+    minimum-sys-voltage setting (in uV);
+- ti,boost-voltage: integer, VBUS voltage level in boost mode (in uV);
+- ti,boost-max-current: integer, maximum allowed current draw in boost mode
+    (in uA).
+
+Optional properties:
+- ti,boost-low-freq: boolean, if present boost mode frequency will be 500kHz,
+    otherwise 1.5MHz;
+- ti,use-ilim-pin: boolean, if present the ILIM resistor will be used and the
+    input current will be the lower between the resistor setting and the IINLIM
+    register setting;
+- ti,thermal-regulation-threshold: integer, temperature above which the charge
+    current is lowered, to avoid overheating (in degrees Celsius);
+
+Example:
+
+bq25890 {
+        compatible = "ti,bq25890";
+        reg = <0x6a>;
+
+        ti,battery-regulation-voltage = <4200000>;
+        ti,charge-current = <1000000>;
+        ti,termination-current = <50000>;
+        ti,precharge-current = <128000>;
+        ti,minimum-sys-voltage = <3600000>;
+        ti,boost-voltage = <5000000>;
+        ti,boost-max-current = <1000000>;
+
+        ti,use-ilim-pin;
+        ti,thermal-regulation-threshold = <120>;
+};
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-05-15 14:06 [PATCH 0/2] Add support for BQ25890 charger chip Laurentiu Palcu
  2015-05-15 14:06 ` [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings Laurentiu Palcu
@ 2015-05-15 14:06 ` Laurentiu Palcu
  2015-05-02 14:59   ` Pavel Machek
  2015-05-19  8:40   ` Krzysztof Kozlowski
  1 sibling, 2 replies; 12+ messages in thread
From: Laurentiu Palcu @ 2015-05-15 14:06 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse
  Cc: Laurentiu Palcu, devicetree, linux-kernel, linux-pm

More details about the chip can be found here:
http://www.ti.com/product/bq25890

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 drivers/power/Kconfig           |   7 +
 drivers/power/Makefile          |   1 +
 drivers/power/bq25890_charger.c | 998 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1006 insertions(+)
 create mode 100644 drivers/power/bq25890_charger.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 4091fb0..3fbfb55 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -388,6 +388,13 @@ config CHARGER_BQ24190
 	help
 	  Say Y to enable support for the TI BQ24190 battery charger.
 
+config CHARGER_BQ25890
+	tristate "TI BQ25890 battery charger driver"
+	depends on I2C && GPIOLIB
+	select REGMAP_I2C
+	help
+	  Say Y to enable support for the TI BQ25890 battery charger.
+
 config CHARGER_BQ24735
 	tristate "TI BQ24735 battery charger support"
 	depends on I2C && GPIOLIB
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b7b0181..bb7ee7b 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
 obj-$(CONFIG_CHARGER_MAX8998)	+= max8998_charger.o
 obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
 obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
+obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
 obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
 obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
diff --git a/drivers/power/bq25890_charger.c b/drivers/power/bq25890_charger.c
new file mode 100644
index 0000000..9ac99ad
--- /dev/null
+++ b/drivers/power/bq25890_charger.c
@@ -0,0 +1,998 @@
+/*
+ * TI BQ25890 charger driver
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/usb/phy.h>
+
+#include <linux/acpi.h>
+#include <linux/of.h>
+
+#define BQ25890_MANUFACTURER		"Texas Instruments"
+#define BQ25890_IRQ_PIN			"bq25890_irq"
+
+#define BQ25890_ID			3
+
+enum bq25890_fields {
+	F_EN_HIZ, F_EN_ILIM, F_IILIM,				     /* Reg00 */
+	F_BHOT, F_BCOLD, F_VINDPM_OFS,				     /* Reg01 */
+	F_CONV_START, F_CONV_RATE, F_BOOSTF, F_ICO_EN,
+	F_HVDCP_EN, F_MAXC_EN, F_FORCE_DPM, F_AUTO_DPDM_EN,	     /* Reg02 */
+	F_BAT_LOAD_EN, F_WD_RST, F_OTG_CFG, F_CHG_CFG, F_SYSVMIN,    /* Reg03 */
+	F_PUMPX_EN, F_ICHG,					     /* Reg04 */
+	F_IPRECHG, F_ITERM,					     /* Reg05 */
+	F_VREG, F_BATLOWV, F_VRECHG,				     /* Reg06 */
+	F_TERM_EN, F_STAT_DIS, F_WD, F_TMR_EN, F_CHG_TMR,
+	F_JEITA_ISET,						     /* Reg07 */
+	F_BATCMP, F_VCLAMP, F_TREG,				     /* Reg08 */
+	F_FORCE_ICO, F_TMR2X_EN, F_BATFET_DIS, F_JEITA_VSET,
+	F_BATFET_DLY, F_BATFET_RST_EN, F_PUMPX_UP, F_PUMPX_DN,	     /* Reg09 */
+	F_BOOSTV, F_BOOSTI,					     /* Reg0A */
+	F_VBUS_STAT, F_CHG_STAT, F_PG_STAT, F_SDP_STAT, F_VSYS_STAT, /* Reg0B */
+	F_WD_FAULT, F_BOOST_FAULT, F_CHG_FAULT, F_BAT_FAULT,
+	F_NTC_FAULT,						     /* Reg0C */
+	F_FORCE_VINDPM, F_VINDPM,				     /* Reg0D */
+	F_THERM_STAT, F_BATV,					     /* Reg0E */
+	F_SYSV,							     /* Reg0F */
+	F_TSPCT,						     /* Reg10 */
+	F_VBUS_GD, F_VBUSV,					     /* Reg11 */
+	F_ICHGR,						     /* Reg12 */
+	F_VDPM_STAT, F_IDPM_STAT, F_IDPM_LIM,			     /* Reg13 */
+	F_REG_RST, F_ICO_OPTIMIZED, F_PN, F_TS_PROFILE, F_DEV_REV,   /* Reg14 */
+
+	F_MAX_FIELDS
+};
+
+/* initial field values, converted to register values */
+struct bq25890_init_data {
+	u8 ichg;	/* charge current		*/
+	u8 vreg;	/* regulation voltage		*/
+	u8 iterm;	/* termination current		*/
+	u8 iprechg;	/* precharge current		*/
+	u8 sysvmin;	/* minimum system voltage limit */
+	u8 boostv;	/* boost regulation voltage	*/
+	u8 boosti;	/* boost current limit		*/
+	u8 boostf;	/* boost frequency		*/
+	u8 ilim_en;	/* enable ILIM pin		*/
+	u8 treg;	/* thermal regulation threshold */
+};
+
+struct bq25890_state {
+	u8 online;
+	u8 chrg_status;
+	u8 chrg_fault;
+	u8 vsys_status;
+	u8 boost_fault;
+	u8 bat_fault;
+};
+
+struct bq25890_device {
+	struct i2c_client *client;
+	struct device *dev;
+	struct power_supply *charger;
+
+	struct usb_phy *usb_phy;
+	struct notifier_block usb_nb;
+	struct work_struct usb_work;
+	unsigned long usb_event;
+
+	struct regmap *rmap;
+	struct regmap_field *rmap_fields[F_MAX_FIELDS];
+
+	u8 chip_id;
+	struct bq25890_init_data init_data;
+	struct bq25890_state state;
+
+	struct mutex lock; /* protect state data */
+};
+
+static const struct regmap_range bq25890_readonly_reg_ranges[] = {
+	regmap_reg_range(0x0b, 0x0c),
+	regmap_reg_range(0x0e, 0x13),
+};
+
+static const struct regmap_access_table bq25890_writeable_regs = {
+	.no_ranges = bq25890_readonly_reg_ranges,
+	.n_no_ranges = ARRAY_SIZE(bq25890_readonly_reg_ranges),
+};
+
+static const struct regmap_range bq25890_volatile_reg_ranges[] = {
+	regmap_reg_range(0x00, 0x00),
+	regmap_reg_range(0x09, 0x09),
+	regmap_reg_range(0x0b, 0x0c),
+	regmap_reg_range(0x0e, 0x14),
+};
+
+static const struct regmap_access_table bq25890_volatile_regs = {
+	.yes_ranges = bq25890_volatile_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(bq25890_volatile_reg_ranges),
+};
+
+static const struct regmap_config bq25890_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = 0x14,
+	.cache_type = REGCACHE_RBTREE,
+
+	.wr_table = &bq25890_writeable_regs,
+	.volatile_table = &bq25890_volatile_regs,
+};
+
+static const struct reg_field bq25890_reg_fields[] = {
+	/* REG00 */
+	[F_EN_HIZ]		= REG_FIELD(0x00, 7, 7),
+	[F_EN_ILIM]		= REG_FIELD(0x00, 6, 6),
+	[F_IILIM]		= REG_FIELD(0x00, 0, 5),
+	/* REG01 */
+	[F_BHOT]		= REG_FIELD(0x01, 6, 7),
+	[F_BCOLD]		= REG_FIELD(0x01, 5, 5),
+	[F_VINDPM_OFS]		= REG_FIELD(0x01, 0, 4),
+	/* REG02 */
+	[F_CONV_START]		= REG_FIELD(0x02, 7, 7),
+	[F_CONV_RATE]		= REG_FIELD(0x02, 6, 6),
+	[F_BOOSTF]		= REG_FIELD(0x02, 5, 5),
+	[F_ICO_EN]		= REG_FIELD(0x02, 4, 4),
+	[F_HVDCP_EN]		= REG_FIELD(0x02, 3, 3),
+	[F_MAXC_EN]		= REG_FIELD(0x02, 2, 2),
+	[F_FORCE_DPM]		= REG_FIELD(0x02, 1, 1),
+	[F_AUTO_DPDM_EN]	= REG_FIELD(0x02, 0, 0),
+	/* REG03 */
+	[F_BAT_LOAD_EN]		= REG_FIELD(0x03, 7, 7),
+	[F_WD_RST]		= REG_FIELD(0x03, 6, 6),
+	[F_OTG_CFG]		= REG_FIELD(0x03, 5, 5),
+	[F_CHG_CFG]		= REG_FIELD(0x03, 4, 4),
+	[F_SYSVMIN]		= REG_FIELD(0x03, 1, 3),
+	/* REG04 */
+	[F_PUMPX_EN]		= REG_FIELD(0x04, 7, 7),
+	[F_ICHG]		= REG_FIELD(0x04, 0, 6),
+	/* REG05 */
+	[F_IPRECHG]		= REG_FIELD(0x05, 4, 7),
+	[F_ITERM]		= REG_FIELD(0x05, 0, 3),
+	/* REG06 */
+	[F_VREG]		= REG_FIELD(0x06, 2, 7),
+	[F_BATLOWV]		= REG_FIELD(0x06, 1, 1),
+	[F_VRECHG]		= REG_FIELD(0x06, 0, 0),
+	/* REG07 */
+	[F_TERM_EN]		= REG_FIELD(0x07, 7, 7),
+	[F_STAT_DIS]		= REG_FIELD(0x07, 6, 6),
+	[F_WD]			= REG_FIELD(0x07, 4, 5),
+	[F_TMR_EN]		= REG_FIELD(0x07, 3, 3),
+	[F_CHG_TMR]		= REG_FIELD(0x07, 1, 2),
+	[F_JEITA_ISET]		= REG_FIELD(0x07, 0, 0),
+	/* REG08 */
+	[F_BATCMP]		= REG_FIELD(0x08, 6, 7),
+	[F_VCLAMP]		= REG_FIELD(0x08, 2, 4),
+	[F_TREG]		= REG_FIELD(0x08, 0, 1),
+	/* REG09 */
+	[F_FORCE_ICO]		= REG_FIELD(0x09, 7, 7),
+	[F_TMR2X_EN]		= REG_FIELD(0x09, 6, 6),
+	[F_BATFET_DIS]		= REG_FIELD(0x09, 5, 5),
+	[F_JEITA_VSET]		= REG_FIELD(0x09, 4, 4),
+	[F_BATFET_DLY]		= REG_FIELD(0x09, 3, 3),
+	[F_BATFET_RST_EN]	= REG_FIELD(0x09, 2, 2),
+	[F_PUMPX_UP]		= REG_FIELD(0x09, 1, 1),
+	[F_PUMPX_DN]		= REG_FIELD(0x09, 0, 0),
+	/* REG0A */
+	[F_BOOSTV]		= REG_FIELD(0x0A, 4, 7),
+	[F_BOOSTI]		= REG_FIELD(0x0A, 0, 2),
+	/* REG0B */
+	[F_VBUS_STAT]		= REG_FIELD(0x0B, 5, 7),
+	[F_CHG_STAT]		= REG_FIELD(0x0B, 3, 4),
+	[F_PG_STAT]		= REG_FIELD(0x0B, 2, 2),
+	[F_SDP_STAT]		= REG_FIELD(0x0B, 1, 1),
+	[F_VSYS_STAT]		= REG_FIELD(0x0B, 0, 0),
+	/* REG0C */
+	[F_WD_FAULT]		= REG_FIELD(0x0C, 7, 7),
+	[F_BOOST_FAULT]		= REG_FIELD(0x0C, 6, 6),
+	[F_CHG_FAULT]		= REG_FIELD(0x0C, 4, 5),
+	[F_BAT_FAULT]		= REG_FIELD(0x0C, 3, 3),
+	[F_NTC_FAULT]		= REG_FIELD(0x0C, 0, 2),
+	/* REG0D */
+	[F_FORCE_VINDPM]	= REG_FIELD(0x0D, 7, 7),
+	[F_VINDPM]		= REG_FIELD(0x0D, 0, 6),
+	/* REG0E */
+	[F_THERM_STAT]		= REG_FIELD(0x0E, 7, 7),
+	[F_BATV]		= REG_FIELD(0x0E, 0, 6),
+	/* REG0F */
+	[F_SYSV]		= REG_FIELD(0x0F, 0, 6),
+	/* REG10 */
+	[F_TSPCT]		= REG_FIELD(0x10, 0, 6),
+	/* REG11 */
+	[F_VBUS_GD]		= REG_FIELD(0x11, 7, 7),
+	[F_VBUSV]		= REG_FIELD(0x11, 0, 6),
+	/* REG12 */
+	[F_ICHGR]		= REG_FIELD(0x12, 0, 6),
+	/* REG13 */
+	[F_VDPM_STAT]		= REG_FIELD(0x13, 7, 7),
+	[F_IDPM_STAT]		= REG_FIELD(0x13, 6, 6),
+	[F_IDPM_LIM]		= REG_FIELD(0x13, 0, 5),
+	/* REG14 */
+	[F_REG_RST]		= REG_FIELD(0x14, 7, 7),
+	[F_ICO_OPTIMIZED]	= REG_FIELD(0x14, 6, 6),
+	[F_PN]			= REG_FIELD(0x14, 3, 5),
+	[F_TS_PROFILE]		= REG_FIELD(0x14, 2, 2),
+	[F_DEV_REV]		= REG_FIELD(0x14, 0, 1)
+};
+
+/*
+ * Most of the val -> idx conversions can be computed, given the minimum,
+ * maximum and the step between values. For the rest of conversions, we use
+ * lookup tables.
+ */
+enum bq25890_table_ids {
+	/* range tables */
+	TBL_ICHG,
+	TBL_ITERM,
+	TBL_IPRECHG,
+	TBL_VREG,
+	TBL_BATCMP,
+	TBL_VCLAMP,
+	TBL_BOOSTV,
+	TBL_SYSVMIN,
+
+	/* lookup tables */
+	TBL_TREG,
+	TBL_BOOSTI,
+};
+
+/* Thermal Regulation Threshold lookup table, in degrees Celsius */
+static const u32 bq25890_treg_tbl[] = { 60, 80, 100, 120 };
+
+#define BQ25890_TREG_TBL_SIZE		ARRAY_SIZE(bq25890_treg_tbl)
+
+/* Boost mode current limit lookup table, in uA */
+static const u32 bq25890_boosti_tbl[] = {
+	500000, 700000, 1100000, 1300000, 1600000, 1800000, 2100000, 2400000
+};
+
+#define BQ25890_BOOSTI_TBL_SIZE		ARRAY_SIZE(bq25890_boosti_tbl)
+
+struct bq25890_range {
+	u32 min;
+	u32 max;
+	u32 step;
+};
+
+struct bq25890_lookup {
+	const u32 *tbl;
+	u32 size;
+};
+
+static const union {
+	struct bq25890_range  rt;
+	struct bq25890_lookup lt;
+} bq25890_tables[] = {
+	/* range tables */
+	[TBL_ICHG] =	{ .rt = {0,	  5056000, 64000} },	 /* uA */
+	[TBL_ITERM] =	{ .rt = {64000,   1024000, 64000} },	 /* uA */
+	[TBL_VREG] =	{ .rt = {3840000, 4608000, 16000} },	 /* uV */
+	[TBL_BATCMP] =	{ .rt = {0,	  140,     20} },	 /* mOhm */
+	[TBL_VCLAMP] =	{ .rt = {0,	  224000,  32000} },	 /* uV */
+	[TBL_BOOSTV] =	{ .rt = {4550000, 5510000, 64000} },	 /* uV */
+	[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} },	 /* uV */
+
+	/* lookup tables */
+	[TBL_TREG] =	{ .lt = {bq25890_treg_tbl, BQ25890_TREG_TBL_SIZE} },
+	[TBL_BOOSTI] =	{ .lt = {bq25890_boosti_tbl, BQ25890_BOOSTI_TBL_SIZE} }
+};
+
+static int bq25890_field_read(struct bq25890_device *bq,
+			      enum bq25890_fields field_id)
+{
+	int ret;
+	int val;
+
+	ret = regmap_field_read(bq->rmap_fields[field_id], &val);
+	if (ret < 0)
+		return ret;
+
+	return val;
+}
+
+static int bq25890_field_write(struct bq25890_device *bq,
+			       enum bq25890_fields field_id, u8 val)
+{
+	return regmap_field_write(bq->rmap_fields[field_id], val);
+}
+
+static u8 bq25890_find_idx(u32 value, enum bq25890_table_ids id)
+{
+	u8 idx;
+
+	if (id >= TBL_TREG) {
+		const u32 *tbl = bq25890_tables[id].lt.tbl;
+		u32 tbl_size = bq25890_tables[id].lt.size;
+
+		for (idx = 1; idx < tbl_size && tbl[idx] <= value; idx++)
+			;
+	} else {
+		const struct bq25890_range *rtbl = &bq25890_tables[id].rt;
+		u8 rtbl_size;
+
+		rtbl_size = (rtbl->max - rtbl->min) / rtbl->step + 1;
+
+		for (idx = 1;
+		     idx < rtbl_size && idx * rtbl->step + rtbl->min <= value;
+		     idx++)
+			;
+	}
+
+	return idx - 1;
+}
+
+static u32 bq25890_find_val(u8 idx, enum bq25890_table_ids id)
+{
+	const struct bq25890_range *rtbl;
+
+	/* lookup table? */
+	if (id >= TBL_TREG)
+		return bq25890_tables[id].lt.tbl[idx];
+
+	/* range table */
+	rtbl = &bq25890_tables[id].rt;
+
+	return(rtbl->min + idx * rtbl->step);
+}
+
+enum bq25890_status {
+	STATUS_NOT_CHARGING,
+	STATUS_PRE_CHARGING,
+	STATUS_FAST_CHARGING,
+	STATUS_TERMINATION_DONE,
+};
+
+enum bq25890_chrg_fault {
+	CHRG_FAULT_NORMAL,
+	CHRG_FAULT_INPUT,
+	CHRG_FAULT_THERMAL_SHUTDOWN,
+	CHRG_FAULT_TIMER_EXPIRED,
+};
+
+static int bq25890_power_supply_get_property(struct power_supply *psy,
+					     enum power_supply_property psp,
+					     union power_supply_propval *val)
+{
+	int ret;
+	struct bq25890_device *bq = power_supply_get_drvdata(psy);
+	struct bq25890_state state;
+
+	mutex_lock(&bq->lock);
+	state = bq->state;
+	mutex_unlock(&bq->lock);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		if (!state.online)
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		else if (state.chrg_status == STATUS_NOT_CHARGING)
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		else if (state.chrg_status == STATUS_PRE_CHARGING ||
+			 state.chrg_status == STATUS_FAST_CHARGING)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else if (state.chrg_status == STATUS_TERMINATION_DONE)
+			val->intval = POWER_SUPPLY_STATUS_FULL;
+		else
+			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+
+		break;
+
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		val->strval = BQ25890_MANUFACTURER;
+		break;
+
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = state.online;
+		break;
+
+	case POWER_SUPPLY_PROP_HEALTH:
+		if (!state.chrg_fault && !state.bat_fault && !state.boost_fault)
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		else if (state.bat_fault)
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		else if (state.chrg_fault == CHRG_FAULT_TIMER_EXPIRED)
+			val->intval = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
+		else if (state.chrg_fault == CHRG_FAULT_THERMAL_SHUTDOWN)
+			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+		else
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		ret = bq25890_field_read(bq, F_ICHGR); /* read measured value */
+		if (ret < 0)
+			return ret;
+
+		/* converted_val = ADC_val * 50mA (table 10.3.19) */
+		val->intval = ret * 50000;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		val->intval = bq25890_tables[TBL_ICHG].rt.max;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		if (!state.online) {
+			val->intval = 0;
+			break;
+		}
+
+		ret = bq25890_field_read(bq, F_BATV); /* read measured value */
+		if (ret < 0)
+			return ret;
+
+		/* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
+		val->intval = 2304000 + ret * 20000;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+		val->intval = bq25890_tables[TBL_VREG].rt.max;
+		break;
+
+	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
+		val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bq25890_get_chip_state(struct bq25890_device *bq,
+				  struct bq25890_state *state)
+{
+	int i, ret;
+
+	struct {
+		enum bq25890_fields id;
+		u8 *data;
+	} state_fields[] = {
+		{F_CHG_STAT,	&state->chrg_status},
+		{F_PG_STAT,	&state->online},
+		{F_VSYS_STAT,	&state->vsys_status},
+		{F_BOOST_FAULT, &state->boost_fault},
+		{F_BAT_FAULT,	&state->bat_fault},
+		{F_CHG_FAULT,	&state->chrg_fault}
+	};
+
+	for (i = 0; i < ARRAY_SIZE(state_fields); i++) {
+		ret = bq25890_field_read(bq, state_fields[i].id);
+		if (ret < 0)
+			return ret;
+
+		*state_fields[i].data = ret;
+	}
+
+	dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT=%d/%d/%d\n",
+		state->chrg_status, state->online, state->vsys_status,
+		state->chrg_fault, state->boost_fault, state->bat_fault);
+
+	return 0;
+}
+
+static bool bq25890_state_changed(struct bq25890_device *bq,
+				  struct bq25890_state *new_state)
+{
+	struct bq25890_state old_state;
+
+	mutex_lock(&bq->lock);
+	old_state = bq->state;
+	mutex_unlock(&bq->lock);
+
+	return (old_state.chrg_status != new_state->chrg_status ||
+		old_state.chrg_fault != new_state->chrg_fault	||
+		old_state.online != new_state->online		||
+		old_state.bat_fault != new_state->bat_fault	||
+		old_state.boost_fault != new_state->boost_fault ||
+		old_state.vsys_status != new_state->vsys_status);
+}
+
+static void bq25890_handle_state_change(struct bq25890_device *bq,
+					struct bq25890_state *new_state)
+{
+	int ret;
+	struct bq25890_state old_state;
+
+	mutex_lock(&bq->lock);
+	old_state = bq->state;
+	mutex_unlock(&bq->lock);
+
+	if (!new_state->online) {			     /* power removed */
+		/* disable ADC */
+		ret = bq25890_field_write(bq, F_CONV_START, 0);
+		if (ret < 0)
+			goto error;
+	} else if (!old_state.online) {			    /* power inserted */
+		/* enable ADC, to have control of charge current/voltage */
+		ret = bq25890_field_write(bq, F_CONV_START, 1);
+		if (ret < 0)
+			goto error;
+	}
+
+	return;
+
+error:
+	dev_err(bq->dev, "%s: Error communicating with the chip.\n", __func__);
+}
+
+static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
+{
+	struct bq25890_device *bq = private;
+	int ret;
+	struct bq25890_state state;
+
+	ret = bq25890_get_chip_state(bq, &state);
+	if (ret < 0)
+		goto handled;
+
+	if (!bq25890_state_changed(bq, &state))
+		goto handled;
+
+	bq25890_handle_state_change(bq, &state);
+
+	mutex_lock(&bq->lock);
+	bq->state = state;
+	mutex_unlock(&bq->lock);
+
+	power_supply_changed(bq->charger);
+
+handled:
+	return IRQ_HANDLED;
+}
+
+static int bq25890_chip_reset(struct bq25890_device *bq)
+{
+	int ret;
+
+	ret = bq25890_field_write(bq, F_REG_RST, 1);
+	if (ret < 0)
+		return ret;
+
+	do {
+		ret = bq25890_field_read(bq, F_REG_RST);
+		if (ret < 0)
+			return ret;
+
+		usleep_range(5, 10);
+	} while (ret == 1);
+
+	return 0;
+}
+
+static int bq25890_hw_init(struct bq25890_device *bq)
+{
+	int ret;
+	int i;
+	struct bq25890_state state;
+
+	const struct {
+		enum bq25890_fields id;
+		u32 value;
+	} init_data[] = {
+		{F_ICHG,	 bq->init_data.ichg},
+		{F_VREG,	 bq->init_data.vreg},
+		{F_ITERM,	 bq->init_data.iterm},
+		{F_IPRECHG,	 bq->init_data.iprechg},
+		{F_SYSVMIN,	 bq->init_data.sysvmin},
+		{F_BOOSTV,	 bq->init_data.boostv},
+		{F_BOOSTI,	 bq->init_data.boosti},
+		{F_BOOSTF,	 bq->init_data.boostf},
+		{F_EN_ILIM,	 bq->init_data.ilim_en},
+		{F_TREG,	 bq->init_data.treg}
+	};
+
+	ret = bq25890_chip_reset(bq);
+	if (ret < 0)
+		return ret;
+
+	/* disable watchdog */
+	ret = bq25890_field_write(bq, F_WD, 0);
+	if (ret < 0)
+		return ret;
+
+	/* initialize currents/voltages and other parameters */
+	for (i = 0; i < ARRAY_SIZE(init_data); i++) {
+		ret = bq25890_field_write(bq, init_data[i].id,
+					  init_data[i].value);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Configure ADC for continuous conversions. This does not enable it. */
+	ret = bq25890_field_write(bq, F_CONV_RATE, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = bq25890_get_chip_state(bq, &state);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&bq->lock);
+	bq->state = state;
+	mutex_unlock(&bq->lock);
+
+	return 0;
+}
+
+static enum power_supply_property bq25890_power_supply_props[] = {
+	POWER_SUPPLY_PROP_MANUFACTURER,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+};
+
+static char *bq25890_charger_supplied_to[] = {
+	"main-battery",
+};
+
+static const struct power_supply_desc bq25890_power_supply_desc = {
+	.name = "bq25890-charger",
+	.type = POWER_SUPPLY_TYPE_USB,
+	.properties = bq25890_power_supply_props,
+	.num_properties = ARRAY_SIZE(bq25890_power_supply_props),
+	.get_property = bq25890_power_supply_get_property,
+};
+
+static int bq25890_power_supply_init(struct bq25890_device *bq)
+{
+	struct power_supply_config psy_cfg = { .drv_data = bq, };
+
+	psy_cfg.supplied_to = bq25890_charger_supplied_to;
+	psy_cfg.num_supplicants = ARRAY_SIZE(bq25890_charger_supplied_to);
+
+	bq->charger = power_supply_register(bq->dev, &bq25890_power_supply_desc,
+					    &psy_cfg);
+	if (IS_ERR(bq->charger))
+		return PTR_ERR(bq->charger);
+
+	return 0;
+}
+
+static void bq25890_usb_work(struct work_struct *data)
+{
+	int ret;
+	struct bq25890_device *bq =
+			container_of(data, struct bq25890_device, usb_work);
+
+	switch (bq->usb_event) {
+	case USB_EVENT_ID:
+		/* Enable boost mode */
+		ret = bq25890_field_write(bq, F_OTG_CFG, 1);
+		if (ret < 0)
+			goto error;
+		break;
+
+	case USB_EVENT_NONE:
+		/* Disable boost mode */
+		ret = bq25890_field_write(bq, F_OTG_CFG, 0);
+		if (ret < 0)
+			goto error;
+
+		power_supply_changed(bq->charger);
+		break;
+	}
+
+	return;
+
+error:
+	dev_err(bq->dev, "%s - Error switching to boost/charger mode.\n",
+		__func__);
+}
+
+static int bq25890_usb_notifier(struct notifier_block *nb, unsigned long val,
+				void *priv)
+{
+	struct bq25890_device *bq =
+			container_of(nb, struct bq25890_device, usb_nb);
+
+	bq->usb_event = val;
+	schedule_work(&bq->usb_work);
+
+	return NOTIFY_OK;
+}
+
+static int bq25890_irq_probe(struct bq25890_device *bq)
+{
+	int ret;
+	struct gpio_desc *irq;
+
+	irq = devm_gpiod_get_index(bq->dev, BQ25890_IRQ_PIN, 0);
+	if (IS_ERR(irq)) {
+		dev_err(bq->dev, "could not probe irq pin\n");
+		return PTR_ERR(irq);
+	}
+
+	ret = gpiod_direction_input(irq);
+	if (ret < 0)
+		return ret;
+
+	return gpiod_to_irq(irq);
+}
+
+static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
+{
+	int ret;
+	u32 property;
+	int i;
+	struct bq25890_init_data *init = &bq->init_data;
+	struct {
+		char *name;
+		bool optional;
+		enum bq25890_table_ids tbl_id;
+		u8 *conv_data; /* holds converted value from given property */
+	} props[] = {
+		/* required properties */
+		{"ti,charge-current", false, TBL_ICHG, &init->ichg},
+		{"ti,battery-regulation-voltage", false, TBL_VREG, &init->vreg},
+		{"ti,termination-current", false, TBL_ITERM, &init->iterm},
+		{"ti,precharge-current", false, TBL_ITERM, &init->iprechg},
+		{"ti,minimum-sys-voltage", false, TBL_SYSVMIN, &init->sysvmin},
+		{"ti,boost-voltage", false, TBL_BOOSTV, &init->boostv},
+		{"ti,boost-max-current", false, TBL_BOOSTI, &init->boosti},
+
+		/* optional properties */
+		{"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg}
+	};
+
+	/* initialize data for optional properties */
+	init->treg = 3; /* 120 degrees Celsius */
+
+	for (i = 0; i < ARRAY_SIZE(props); i++) {
+		ret = device_property_read_u32(bq->dev, props[i].name,
+					       &property);
+		if (ret < 0) {
+			if (props[i].optional)
+				continue;
+
+			return ret;
+		}
+
+		*props[i].conv_data = bq25890_find_idx(property,
+						       props[i].tbl_id);
+	}
+
+	return 0;
+}
+
+static int bq25890_fw_probe(struct bq25890_device *bq)
+{
+	int ret;
+	struct bq25890_init_data *init = &bq->init_data;
+
+	ret = bq25890_fw_read_u32_props(bq);
+	if (ret < 0)
+		return ret;
+
+	init->ilim_en = device_property_read_bool(bq->dev, "ti,use-ilim-pin");
+	init->boostf = device_property_read_bool(bq->dev, "ti,boost-low-freq");
+
+	return 0;
+}
+
+static int bq25890_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct device *dev = &client->dev;
+	struct bq25890_device *bq;
+	int ret;
+	int i;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(dev, "No support for SMBUS_BYTE_DATA\n");
+		return -ENODEV;
+	}
+
+	bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
+	if (!bq)
+		return -ENOMEM;
+
+	bq->client = client;
+	bq->dev = dev;
+
+	mutex_init(&bq->lock);
+
+	bq->rmap = devm_regmap_init_i2c(client, &bq25890_regmap_config);
+	if (IS_ERR(bq->rmap)) {
+		dev_err(dev, "failed to allocate register map\n");
+		return PTR_ERR(bq->rmap);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(bq25890_reg_fields); i++) {
+		const struct reg_field *reg_fields = bq25890_reg_fields;
+
+		bq->rmap_fields[i] = devm_regmap_field_alloc(dev, bq->rmap,
+							     reg_fields[i]);
+		if (IS_ERR(bq->rmap_fields[i])) {
+			dev_err(dev, "cannot allocate regmap field\n");
+			return PTR_ERR(bq->rmap_fields[i]);
+		}
+	}
+
+	i2c_set_clientdata(client, bq);
+
+	bq->chip_id = bq25890_field_read(bq, F_PN);
+	if (bq->chip_id < 0) {
+		dev_err(dev, "Cannot read chip ID.\n");
+		return ret;
+	}
+
+	if (bq->chip_id != BQ25890_ID) {
+		dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
+		return -ENODEV;
+	}
+
+	if (!dev->platform_data) {
+		ret = bq25890_fw_probe(bq);
+		if (ret < 0) {
+			dev_err(dev, "Cannot read device properties.\n");
+			return ret;
+		}
+	} else {
+		return -ENODEV;
+	}
+
+	ret = bq25890_hw_init(bq);
+	if (ret < 0) {
+		dev_err(dev, "Cannot initialize the chip.\n");
+		return ret;
+	}
+
+	if (client->irq <= 0)
+		client->irq = bq25890_irq_probe(bq);
+
+	if (client->irq < 0) {
+		dev_err(dev, "no irq resource found\n");
+		return client->irq;
+	}
+
+	/* OTG reporting */
+	bq->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+	if (!IS_ERR_OR_NULL(bq->usb_phy)) {
+		INIT_WORK(&bq->usb_work, bq25890_usb_work);
+		bq->usb_nb.notifier_call = bq25890_usb_notifier;
+		usb_register_notifier(bq->usb_phy, &bq->usb_nb);
+	}
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+					bq25890_irq_handler_thread,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					BQ25890_IRQ_PIN, bq);
+	if (ret)
+		goto irq_fail;
+
+	ret = bq25890_power_supply_init(bq);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register power supply\n");
+		goto irq_fail;
+	}
+
+	return 0;
+
+irq_fail:
+	if (!IS_ERR_OR_NULL(bq->usb_phy))
+		usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
+
+	return ret;
+}
+
+static int bq25890_remove(struct i2c_client *client)
+{
+	struct bq25890_device *bq = i2c_get_clientdata(client);
+
+	if (!IS_ERR_OR_NULL(bq->usb_phy))
+		usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
+
+	power_supply_unregister(bq->charger);
+
+	/* reset all registers to default values */
+	bq25890_chip_reset(bq);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bq25890_suspend(struct device *dev)
+{
+	struct bq25890_device *bq = dev_get_drvdata(dev);
+
+	/*
+	 * If charger is removed, while in suspend, make sure ADC is diabled
+	 * since it consumes slightly more power.
+	 */
+	return bq25890_field_write(bq, F_CONV_START, 0);
+}
+
+static int bq25890_resume(struct device *dev)
+{
+	int ret;
+	struct bq25890_state state;
+	struct bq25890_device *bq = dev_get_drvdata(dev);
+
+	ret = bq25890_get_chip_state(bq, &state);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&bq->lock);
+	bq->state = state;
+	mutex_unlock(&bq->lock);
+
+	/* Re-enable ADC only if charger is plugged in. */
+	if (bq->state.online) {
+		ret = bq25890_field_write(bq, F_CONV_START, 1);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* signal userspace, maybe state changed while suspended */
+	power_supply_changed(bq->charger);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops bq25890_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(bq25890_suspend, bq25890_resume)
+};
+
+static const struct i2c_device_id bq25890_i2c_ids[] = {
+	{ "bq25890", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, bq25890_i2c_ids);
+
+static const struct of_device_id bq25890_of_match[] = {
+	{ .compatible = "ti,bq25890", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bq25890_of_match);
+
+static const struct acpi_device_id bq25890_acpi_match[] = {
+	{"BQ258900", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, bq25890_acpi_match);
+
+static struct i2c_driver bq25890_driver = {
+	.driver = {
+		.name = "bq25890-charger",
+		.of_match_table = of_match_ptr(bq25890_of_match),
+		.acpi_match_table = ACPI_PTR(bq25890_acpi_match),
+		.pm = &bq25890_pm,
+	},
+	.probe = bq25890_probe,
+	.remove = bq25890_remove,
+	.id_table = bq25890_i2c_ids,
+};
+module_i2c_driver(bq25890_driver);
+
+MODULE_AUTHOR("Laurentiu Palcu <laurentiu.palcu@intel.com>");
+MODULE_DESCRIPTION("bq25890 charger driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings
  2015-05-15 14:06 ` [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings Laurentiu Palcu
@ 2015-05-16 11:53   ` Krzysztof Kozlowski
  2015-05-18  6:54     ` Laurentiu Palcu
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2015-05-16 11:53 UTC (permalink / raw)
  To: Laurentiu Palcu
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse, devicetree, linux-kernel, linux-pm

2015-05-15 23:06 GMT+09:00 Laurentiu Palcu <laurentiu.palcu@intel.com>:
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
> ---
>  .../devicetree/bindings/power/bq25890.txt          | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/bq25890.txt
>
> diff --git a/Documentation/devicetree/bindings/power/bq25890.txt b/Documentation/devicetree/bindings/power/bq25890.txt
> new file mode 100644
> index 0000000..44f50a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/bq25890.txt
> @@ -0,0 +1,45 @@
> +Binding for TI bq25890 Li-Ion Charger
> +
> +Required properties:
> +- compatible: Should contain one of the following:
> +    * "ti,bq25890"
> +- reg: integer, i2c address of the device.
> +- 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,precharge-current: integer, maximum charge current during precharge
> +    phase (in uA);
> +- ti,minimum-sys-voltage: integer, when battery is charging and it is below
> +    minimum system voltage, the system will be regulated above
> +    minimum-sys-voltage setting (in uV);
> +- ti,boost-voltage: integer, VBUS voltage level in boost mode (in uV);
> +- ti,boost-max-current: integer, maximum allowed current draw in boost mode
> +    (in uA).
> +
> +Optional properties:
> +- ti,boost-low-freq: boolean, if present boost mode frequency will be 500kHz,
> +    otherwise 1.5MHz;
> +- ti,use-ilim-pin: boolean, if present the ILIM resistor will be used and the
> +    input current will be the lower between the resistor setting and the IINLIM
> +    register setting;
> +- ti,thermal-regulation-threshold: integer, temperature above which the charge
> +    current is lowered, to avoid overheating (in degrees Celsius);

What if property is absent? The temperature will be ignored? If so,
maybe consider adding such short statement about the default state?

Rest looks good, but I am still digging through the driver and I want
to give reviewed-tag after that. This will take me some time :) .

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings
  2015-05-16 11:53   ` Krzysztof Kozlowski
@ 2015-05-18  6:54     ` Laurentiu Palcu
  0 siblings, 0 replies; 12+ messages in thread
From: Laurentiu Palcu @ 2015-05-18  6:54 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	devicetree, linux-kernel, linux-pm

Hi Krzysztof,

On Sat, May 16, 2015 at 08:53:42PM +0900, Krzysztof Kozlowski wrote:
> 2015-05-15 23:06 GMT+09:00 Laurentiu Palcu <laurentiu.palcu@intel.com>:
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
> > ---
> >  .../devicetree/bindings/power/bq25890.txt          | 45 ++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/bq25890.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/bq25890.txt b/Documentation/devicetree/bindings/power/bq25890.txt
> > new file mode 100644
> > index 0000000..44f50a7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/bq25890.txt
> > @@ -0,0 +1,45 @@
> > +Binding for TI bq25890 Li-Ion Charger
> > +
> > +Required properties:
> > +- compatible: Should contain one of the following:
> > +    * "ti,bq25890"
> > +- reg: integer, i2c address of the device.
> > +- 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,precharge-current: integer, maximum charge current during precharge
> > +    phase (in uA);
> > +- ti,minimum-sys-voltage: integer, when battery is charging and it is below
> > +    minimum system voltage, the system will be regulated above
> > +    minimum-sys-voltage setting (in uV);
> > +- ti,boost-voltage: integer, VBUS voltage level in boost mode (in uV);
> > +- ti,boost-max-current: integer, maximum allowed current draw in boost mode
> > +    (in uA).
> > +
> > +Optional properties:
> > +- ti,boost-low-freq: boolean, if present boost mode frequency will be 500kHz,
> > +    otherwise 1.5MHz;
> > +- ti,use-ilim-pin: boolean, if present the ILIM resistor will be used and the
> > +    input current will be the lower between the resistor setting and the IINLIM
> > +    register setting;
> > +- ti,thermal-regulation-threshold: integer, temperature above which the charge
> > +    current is lowered, to avoid overheating (in degrees Celsius);
> 
> What if property is absent? The temperature will be ignored? If so,
> maybe consider adding such short statement about the default state?
You're right, I should've mentioned it here. If it's missing, the
default value will be used (120 degrees Celsius). I'll add a comment
about it in v2.

Thanks for reviewing,
laurentiu


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-05-15 14:06 ` [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip Laurentiu Palcu
  2015-05-02 14:59   ` Pavel Machek
@ 2015-05-19  8:40   ` Krzysztof Kozlowski
  2015-05-19  9:14     ` Laurentiu Palcu
  1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2015-05-19  8:40 UTC (permalink / raw)
  To: Laurentiu Palcu
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse, devicetree, linux-kernel, linux-pm

2015-05-15 23:06 GMT+09:00 Laurentiu Palcu <laurentiu.palcu@intel.com>:
> More details about the chip can be found here:
> http://www.ti.com/product/bq25890
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
> ---
>  drivers/power/Kconfig           |   7 +
>  drivers/power/Makefile          |   1 +
>  drivers/power/bq25890_charger.c | 998 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1006 insertions(+)
>  create mode 100644 drivers/power/bq25890_charger.c

Hi,

Some comments (nothing serious) inline.

(...)

> +
> +static int bq25890_field_write(struct bq25890_device *bq,
> +                              enum bq25890_fields field_id, u8 val)
> +{
> +       return regmap_field_write(bq->rmap_fields[field_id], val);
> +}
> +
> +static u8 bq25890_find_idx(u32 value, enum bq25890_table_ids id)
> +{
> +       u8 idx;
> +
> +       if (id >= TBL_TREG) {
> +               const u32 *tbl = bq25890_tables[id].lt.tbl;
> +               u32 tbl_size = bq25890_tables[id].lt.size;
> +
> +               for (idx = 1; idx < tbl_size && tbl[idx] <= value; idx++)
> +                       ;
> +       } else {
> +               const struct bq25890_range *rtbl = &bq25890_tables[id].rt;
> +               u8 rtbl_size;
> +
> +               rtbl_size = (rtbl->max - rtbl->min) / rtbl->step + 1;
> +
> +               for (idx = 1;
> +                    idx < rtbl_size && idx * rtbl->step + rtbl->min <= value;

Could you add parentheses around part of this expression? It is non
obvious to find the comparison statements.

> +                    idx++)
> +                       ;
> +       }
> +
> +       return idx - 1;
> +}
> +
> +static u32 bq25890_find_val(u8 idx, enum bq25890_table_ids id)
> +{
> +       const struct bq25890_range *rtbl;
> +
> +       /* lookup table? */
> +       if (id >= TBL_TREG)
> +               return bq25890_tables[id].lt.tbl[idx];
> +
> +       /* range table */
> +       rtbl = &bq25890_tables[id].rt;
> +
> +       return(rtbl->min + idx * rtbl->step);

A nit: space between return and parenthesis would be nice.

> +}
> +
> +enum bq25890_status {
> +       STATUS_NOT_CHARGING,
> +       STATUS_PRE_CHARGING,
> +       STATUS_FAST_CHARGING,
> +       STATUS_TERMINATION_DONE,
> +};
> +
> +enum bq25890_chrg_fault {
> +       CHRG_FAULT_NORMAL,
> +       CHRG_FAULT_INPUT,
> +       CHRG_FAULT_THERMAL_SHUTDOWN,
> +       CHRG_FAULT_TIMER_EXPIRED,
> +};
> +
> +static int bq25890_power_supply_get_property(struct power_supply *psy,
> +                                            enum power_supply_property psp,
> +                                            union power_supply_propval *val)
> +{
> +       int ret;
> +       struct bq25890_device *bq = power_supply_get_drvdata(psy);
> +       struct bq25890_state state;
> +
> +       mutex_lock(&bq->lock);
> +       state = bq->state;
> +       mutex_unlock(&bq->lock);
> +
> +       switch (psp) {
> +       case POWER_SUPPLY_PROP_STATUS:
> +               if (!state.online)
> +                       val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> +               else if (state.chrg_status == STATUS_NOT_CHARGING)
> +                       val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +               else if (state.chrg_status == STATUS_PRE_CHARGING ||
> +                        state.chrg_status == STATUS_FAST_CHARGING)
> +                       val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +               else if (state.chrg_status == STATUS_TERMINATION_DONE)
> +                       val->intval = POWER_SUPPLY_STATUS_FULL;
> +               else
> +                       val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> +
> +               break;
> +
> +       case POWER_SUPPLY_PROP_MANUFACTURER:
> +               val->strval = BQ25890_MANUFACTURER;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_ONLINE:
> +               val->intval = state.online;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_HEALTH:
> +               if (!state.chrg_fault && !state.bat_fault && !state.boost_fault)
> +                       val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +               else if (state.bat_fault)
> +                       val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +               else if (state.chrg_fault == CHRG_FAULT_TIMER_EXPIRED)
> +                       val->intval = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> +               else if (state.chrg_fault == CHRG_FAULT_THERMAL_SHUTDOWN)
> +                       val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
> +               else
> +                       val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> +               ret = bq25890_field_read(bq, F_ICHGR); /* read measured value */
> +               if (ret < 0)
> +                       return ret;
> +
> +               /* converted_val = ADC_val * 50mA (table 10.3.19) */
> +               val->intval = ret * 50000;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +               val->intval = bq25890_tables[TBL_ICHG].rt.max;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> +               if (!state.online) {
> +                       val->intval = 0;
> +                       break;
> +               }
> +
> +               ret = bq25890_field_read(bq, F_BATV); /* read measured value */
> +               if (ret < 0)
> +                       return ret;
> +
> +               /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
> +               val->intval = 2304000 + ret * 20000;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +               val->intval = bq25890_tables[TBL_VREG].rt.max;
> +               break;
> +
> +       case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
> +               val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int bq25890_get_chip_state(struct bq25890_device *bq,
> +                                 struct bq25890_state *state)
> +{
> +       int i, ret;
> +
> +       struct {
> +               enum bq25890_fields id;
> +               u8 *data;
> +       } state_fields[] = {
> +               {F_CHG_STAT,    &state->chrg_status},
> +               {F_PG_STAT,     &state->online},
> +               {F_VSYS_STAT,   &state->vsys_status},
> +               {F_BOOST_FAULT, &state->boost_fault},
> +               {F_BAT_FAULT,   &state->bat_fault},
> +               {F_CHG_FAULT,   &state->chrg_fault}
> +       };
> +
> +       for (i = 0; i < ARRAY_SIZE(state_fields); i++) {
> +               ret = bq25890_field_read(bq, state_fields[i].id);
> +               if (ret < 0)
> +                       return ret;
> +
> +               *state_fields[i].data = ret;
> +       }
> +
> +       dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT=%d/%d/%d\n",
> +               state->chrg_status, state->online, state->vsys_status,
> +               state->chrg_fault, state->boost_fault, state->bat_fault);
> +
> +       return 0;
> +}
> +
> +static bool bq25890_state_changed(struct bq25890_device *bq,
> +                                 struct bq25890_state *new_state)
> +{
> +       struct bq25890_state old_state;
> +
> +       mutex_lock(&bq->lock);
> +       old_state = bq->state;
> +       mutex_unlock(&bq->lock);
> +
> +       return (old_state.chrg_status != new_state->chrg_status ||
> +               old_state.chrg_fault != new_state->chrg_fault   ||
> +               old_state.online != new_state->online           ||
> +               old_state.bat_fault != new_state->bat_fault     ||
> +               old_state.boost_fault != new_state->boost_fault ||
> +               old_state.vsys_status != new_state->vsys_status);
> +}
> +
> +static void bq25890_handle_state_change(struct bq25890_device *bq,
> +                                       struct bq25890_state *new_state)
> +{
> +       int ret;
> +       struct bq25890_state old_state;
> +
> +       mutex_lock(&bq->lock);
> +       old_state = bq->state;
> +       mutex_unlock(&bq->lock);
> +
> +       if (!new_state->online) {                            /* power removed */
> +               /* disable ADC */
> +               ret = bq25890_field_write(bq, F_CONV_START, 0);
> +               if (ret < 0)
> +                       goto error;
> +       } else if (!old_state.online) {                     /* power inserted */
> +               /* enable ADC, to have control of charge current/voltage */
> +               ret = bq25890_field_write(bq, F_CONV_START, 1);
> +               if (ret < 0)
> +                       goto error;
> +       }
> +
> +       return;
> +
> +error:
> +       dev_err(bq->dev, "%s: Error communicating with the chip.\n", __func__);

The __func__ is not needed here, there is only one kind of such message.

> +}
> +
> +static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
> +{
> +       struct bq25890_device *bq = private;
> +       int ret;
> +       struct bq25890_state state;
> +
> +       ret = bq25890_get_chip_state(bq, &state);
> +       if (ret < 0)
> +               goto handled;
> +
> +       if (!bq25890_state_changed(bq, &state))
> +               goto handled;
> +
> +       bq25890_handle_state_change(bq, &state);
> +
> +       mutex_lock(&bq->lock);
> +       bq->state = state;
> +       mutex_unlock(&bq->lock);
> +
> +       power_supply_changed(bq->charger);
> +
> +handled:
> +       return IRQ_HANDLED;
> +}
> +
> +static int bq25890_chip_reset(struct bq25890_device *bq)
> +{
> +       int ret;
> +
> +       ret = bq25890_field_write(bq, F_REG_RST, 1);
> +       if (ret < 0)
> +               return ret;
> +
> +       do {
> +               ret = bq25890_field_read(bq, F_REG_RST);
> +               if (ret < 0)
> +                       return ret;
> +
> +               usleep_range(5, 10);
> +       } while (ret == 1);

Is it possible to loop here indefinetely?

> +
> +       return 0;
> +}
> +
> +static int bq25890_hw_init(struct bq25890_device *bq)
> +{
> +       int ret;
> +       int i;
> +       struct bq25890_state state;
> +
> +       const struct {
> +               enum bq25890_fields id;
> +               u32 value;
> +       } init_data[] = {
> +               {F_ICHG,         bq->init_data.ichg},
> +               {F_VREG,         bq->init_data.vreg},
> +               {F_ITERM,        bq->init_data.iterm},
> +               {F_IPRECHG,      bq->init_data.iprechg},
> +               {F_SYSVMIN,      bq->init_data.sysvmin},
> +               {F_BOOSTV,       bq->init_data.boostv},
> +               {F_BOOSTI,       bq->init_data.boosti},
> +               {F_BOOSTF,       bq->init_data.boostf},
> +               {F_EN_ILIM,      bq->init_data.ilim_en},
> +               {F_TREG,         bq->init_data.treg}
> +       };
> +
> +       ret = bq25890_chip_reset(bq);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* disable watchdog */
> +       ret = bq25890_field_write(bq, F_WD, 0);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* initialize currents/voltages and other parameters */
> +       for (i = 0; i < ARRAY_SIZE(init_data); i++) {
> +               ret = bq25890_field_write(bq, init_data[i].id,
> +                                         init_data[i].value);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       /* Configure ADC for continuous conversions. This does not enable it. */
> +       ret = bq25890_field_write(bq, F_CONV_RATE, 1);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = bq25890_get_chip_state(bq, &state);
> +       if (ret < 0)
> +               return ret;
> +
> +       mutex_lock(&bq->lock);
> +       bq->state = state;
> +       mutex_unlock(&bq->lock);
> +
> +       return 0;
> +}
> +
> +static enum power_supply_property bq25890_power_supply_props[] = {
> +       POWER_SUPPLY_PROP_MANUFACTURER,
> +       POWER_SUPPLY_PROP_STATUS,
> +       POWER_SUPPLY_PROP_ONLINE,
> +       POWER_SUPPLY_PROP_HEALTH,
> +       POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> +       POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
> +       POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> +       POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> +       POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> +};
> +
> +static char *bq25890_charger_supplied_to[] = {
> +       "main-battery",
> +};
> +
> +static const struct power_supply_desc bq25890_power_supply_desc = {
> +       .name = "bq25890-charger",
> +       .type = POWER_SUPPLY_TYPE_USB,
> +       .properties = bq25890_power_supply_props,
> +       .num_properties = ARRAY_SIZE(bq25890_power_supply_props),
> +       .get_property = bq25890_power_supply_get_property,
> +};
> +
> +static int bq25890_power_supply_init(struct bq25890_device *bq)
> +{
> +       struct power_supply_config psy_cfg = { .drv_data = bq, };
> +
> +       psy_cfg.supplied_to = bq25890_charger_supplied_to;
> +       psy_cfg.num_supplicants = ARRAY_SIZE(bq25890_charger_supplied_to);
> +
> +       bq->charger = power_supply_register(bq->dev, &bq25890_power_supply_desc,
> +                                           &psy_cfg);
> +       if (IS_ERR(bq->charger))
> +               return PTR_ERR(bq->charger);
> +
> +       return 0;

return PTR_ERR_OR_ZERO

> +}
> +
> +static void bq25890_usb_work(struct work_struct *data)
> +{
> +       int ret;
> +       struct bq25890_device *bq =
> +                       container_of(data, struct bq25890_device, usb_work);
> +
> +       switch (bq->usb_event) {
> +       case USB_EVENT_ID:
> +               /* Enable boost mode */
> +               ret = bq25890_field_write(bq, F_OTG_CFG, 1);
> +               if (ret < 0)
> +                       goto error;
> +               break;
> +
> +       case USB_EVENT_NONE:
> +               /* Disable boost mode */
> +               ret = bq25890_field_write(bq, F_OTG_CFG, 0);
> +               if (ret < 0)
> +                       goto error;
> +
> +               power_supply_changed(bq->charger);
> +               break;
> +       }
> +
> +       return;
> +
> +error:
> +       dev_err(bq->dev, "%s - Error switching to boost/charger mode.\n",
> +               __func__);

Again the __func__. It shouldn't be here.

> +}
> +
> +static int bq25890_usb_notifier(struct notifier_block *nb, unsigned long val,
> +                               void *priv)
> +{
> +       struct bq25890_device *bq =
> +                       container_of(nb, struct bq25890_device, usb_nb);
> +
> +       bq->usb_event = val;
> +       schedule_work(&bq->usb_work);

If this shouldn't be executed on exactly this CPU then usage of
system_power_efficient_wq is encouraged.

> +
> +       return NOTIFY_OK;
> +}
> +
> +static int bq25890_irq_probe(struct bq25890_device *bq)
> +{
> +       int ret;
> +       struct gpio_desc *irq;
> +
> +       irq = devm_gpiod_get_index(bq->dev, BQ25890_IRQ_PIN, 0);
> +       if (IS_ERR(irq)) {
> +               dev_err(bq->dev, "could not probe irq pin\n");
> +               return PTR_ERR(irq);
> +       }
> +
> +       ret = gpiod_direction_input(irq);
> +       if (ret < 0)
> +               return ret;
> +
> +       return gpiod_to_irq(irq);
> +}
> +
> +static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
> +{
> +       int ret;
> +       u32 property;
> +       int i;
> +       struct bq25890_init_data *init = &bq->init_data;
> +       struct {
> +               char *name;
> +               bool optional;
> +               enum bq25890_table_ids tbl_id;
> +               u8 *conv_data; /* holds converted value from given property */
> +       } props[] = {
> +               /* required properties */
> +               {"ti,charge-current", false, TBL_ICHG, &init->ichg},
> +               {"ti,battery-regulation-voltage", false, TBL_VREG, &init->vreg},
> +               {"ti,termination-current", false, TBL_ITERM, &init->iterm},
> +               {"ti,precharge-current", false, TBL_ITERM, &init->iprechg},
> +               {"ti,minimum-sys-voltage", false, TBL_SYSVMIN, &init->sysvmin},
> +               {"ti,boost-voltage", false, TBL_BOOSTV, &init->boostv},
> +               {"ti,boost-max-current", false, TBL_BOOSTI, &init->boosti},
> +
> +               /* optional properties */
> +               {"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg}
> +       };
> +
> +       /* initialize data for optional properties */
> +       init->treg = 3; /* 120 degrees Celsius */
> +
> +       for (i = 0; i < ARRAY_SIZE(props); i++) {
> +               ret = device_property_read_u32(bq->dev, props[i].name,
> +                                              &property);
> +               if (ret < 0) {
> +                       if (props[i].optional)
> +                               continue;
> +
> +                       return ret;
> +               }
> +
> +               *props[i].conv_data = bq25890_find_idx(property,
> +                                                      props[i].tbl_id);
> +       }
> +
> +       return 0;
> +}
> +
> +static int bq25890_fw_probe(struct bq25890_device *bq)
> +{
> +       int ret;
> +       struct bq25890_init_data *init = &bq->init_data;
> +
> +       ret = bq25890_fw_read_u32_props(bq);
> +       if (ret < 0)
> +               return ret;
> +
> +       init->ilim_en = device_property_read_bool(bq->dev, "ti,use-ilim-pin");
> +       init->boostf = device_property_read_bool(bq->dev, "ti,boost-low-freq");
> +
> +       return 0;
> +}
> +
> +static int bq25890_probe(struct i2c_client *client,
> +                        const struct i2c_device_id *id)
> +{
> +       struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +       struct device *dev = &client->dev;
> +       struct bq25890_device *bq;
> +       int ret;
> +       int i;
> +
> +       if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> +               dev_err(dev, "No support for SMBUS_BYTE_DATA\n");
> +               return -ENODEV;
> +       }
> +
> +       bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
> +       if (!bq)
> +               return -ENOMEM;
> +
> +       bq->client = client;
> +       bq->dev = dev;
> +
> +       mutex_init(&bq->lock);
> +
> +       bq->rmap = devm_regmap_init_i2c(client, &bq25890_regmap_config);
> +       if (IS_ERR(bq->rmap)) {
> +               dev_err(dev, "failed to allocate register map\n");
> +               return PTR_ERR(bq->rmap);
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(bq25890_reg_fields); i++) {
> +               const struct reg_field *reg_fields = bq25890_reg_fields;
> +
> +               bq->rmap_fields[i] = devm_regmap_field_alloc(dev, bq->rmap,
> +                                                            reg_fields[i]);
> +               if (IS_ERR(bq->rmap_fields[i])) {
> +                       dev_err(dev, "cannot allocate regmap field\n");
> +                       return PTR_ERR(bq->rmap_fields[i]);
> +               }
> +       }
> +
> +       i2c_set_clientdata(client, bq);
> +
> +       bq->chip_id = bq25890_field_read(bq, F_PN);
> +       if (bq->chip_id < 0) {
> +               dev_err(dev, "Cannot read chip ID.\n");
> +               return ret;
> +       }
> +
> +       if (bq->chip_id != BQ25890_ID) {
> +               dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
> +               return -ENODEV;
> +       }
> +
> +       if (!dev->platform_data) {
> +               ret = bq25890_fw_probe(bq);
> +               if (ret < 0) {
> +                       dev_err(dev, "Cannot read device properties.\n");
> +                       return ret;
> +               }
> +       } else {
> +               return -ENODEV;
> +       }
> +
> +       ret = bq25890_hw_init(bq);
> +       if (ret < 0) {
> +               dev_err(dev, "Cannot initialize the chip.\n");
> +               return ret;
> +       }
> +
> +       if (client->irq <= 0)
> +               client->irq = bq25890_irq_probe(bq);
> +
> +       if (client->irq < 0) {
> +               dev_err(dev, "no irq resource found\n");

A nit: stick to one convention - capitilize first letter of error message.

> +               return client->irq;
> +       }
> +
> +       /* OTG reporting */
> +       bq->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> +       if (!IS_ERR_OR_NULL(bq->usb_phy)) {
> +               INIT_WORK(&bq->usb_work, bq25890_usb_work);
> +               bq->usb_nb.notifier_call = bq25890_usb_notifier;
> +               usb_register_notifier(bq->usb_phy, &bq->usb_nb);
> +       }
> +
> +       ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +                                       bq25890_irq_handler_thread,
> +                                       IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +                                       BQ25890_IRQ_PIN, bq);
> +       if (ret)
> +               goto irq_fail;
> +
> +       ret = bq25890_power_supply_init(bq);
> +       if (ret < 0) {
> +               dev_err(dev, "Failed to register power supply\n");
> +               goto irq_fail;
> +       }
> +
> +       return 0;
> +
> +irq_fail:
> +       if (!IS_ERR_OR_NULL(bq->usb_phy))
> +               usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
> +
> +       return ret;
> +}
> +
> +static int bq25890_remove(struct i2c_client *client)
> +{
> +       struct bq25890_device *bq = i2c_get_clientdata(client);
> +
> +       if (!IS_ERR_OR_NULL(bq->usb_phy))
> +               usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
> +
> +       power_supply_unregister(bq->charger);

I would prefer cleaning in reversed order of probe, so first would be
power_supply_unregister(). I think this is expected in usual case.
Often when I encounter such code I wonder - is this non-standard order
of cleanup is important?

> +
> +       /* reset all registers to default values */
> +       bq25890_chip_reset(bq);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int bq25890_suspend(struct device *dev)
> +{
> +       struct bq25890_device *bq = dev_get_drvdata(dev);
> +
> +       /*
> +        * If charger is removed, while in suspend, make sure ADC is diabled
> +        * since it consumes slightly more power.
> +        */
> +       return bq25890_field_write(bq, F_CONV_START, 0);
> +}
> +
> +static int bq25890_resume(struct device *dev)
> +{
> +       int ret;
> +       struct bq25890_state state;
> +       struct bq25890_device *bq = dev_get_drvdata(dev);
> +
> +       ret = bq25890_get_chip_state(bq, &state);
> +       if (ret < 0)
> +               return ret;
> +
> +       mutex_lock(&bq->lock);
> +       bq->state = state;
> +       mutex_unlock(&bq->lock);
> +
> +       /* Re-enable ADC only if charger is plugged in. */
> +       if (bq->state.online) {

Shouldn't you be here accessing local variable state?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-05-19  8:40   ` Krzysztof Kozlowski
@ 2015-05-19  9:14     ` Laurentiu Palcu
  2015-05-19 10:03       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 12+ messages in thread
From: Laurentiu Palcu @ 2015-05-19  9:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	devicetree, linux-kernel, linux-pm

Hi Krzysztof,

On Tue, May 19, 2015 at 05:40:25PM +0900, Krzysztof Kozlowski wrote:
> > +
> > +static int bq25890_chip_reset(struct bq25890_device *bq)
> > +{
> > +       int ret;
> > +
> > +       ret = bq25890_field_write(bq, F_REG_RST, 1);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       do {
> > +               ret = bq25890_field_read(bq, F_REG_RST);
> > +               if (ret < 0)
> > +                       return ret;
> > +
> > +               usleep_range(5, 10);
> > +       } while (ret == 1);
> 
> Is it possible to loop here indefinetely?
According to specifications, this field is "Reset to 0 after register
reset is completed", so I'm trusting the chip will behave as advertised!
:) We could implement a safety mechanism to avoid looping in case the
chip misbehaves but I don't think it's worth it. What do you think?

I'll address the other comments in v2.

Thanks for reviewing,
laurentiu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-05-19  9:14     ` Laurentiu Palcu
@ 2015-05-19 10:03       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2015-05-19 10:03 UTC (permalink / raw)
  To: Laurentiu Palcu
  Cc: Krzysztof Kozlowski, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Sebastian Reichel,
	Dmitry Eremin-Solenikov, David Woodhouse, devicetree,
	linux-kernel, linux-pm

2015-05-19 18:14 GMT+09:00 Laurentiu Palcu <laurentiu.palcu@intel.com>:
> Hi Krzysztof,
>
> On Tue, May 19, 2015 at 05:40:25PM +0900, Krzysztof Kozlowski wrote:
>> > +
>> > +static int bq25890_chip_reset(struct bq25890_device *bq)
>> > +{
>> > +       int ret;
>> > +
>> > +       ret = bq25890_field_write(bq, F_REG_RST, 1);
>> > +       if (ret < 0)
>> > +               return ret;
>> > +
>> > +       do {
>> > +               ret = bq25890_field_read(bq, F_REG_RST);
>> > +               if (ret < 0)
>> > +                       return ret;
>> > +
>> > +               usleep_range(5, 10);
>> > +       } while (ret == 1);
>>
>> Is it possible to loop here indefinetely?
> According to specifications, this field is "Reset to 0 after register
> reset is completed", so I'm trusting the chip will behave as advertised!
> :) We could implement a safety mechanism to avoid looping in case the
> chip misbehaves but I don't think it's worth it. What do you think?

I just prefer to use some retry counter because it would be better to
fail the reset instead of being stuck here. The chip may behave
correctly but still a bug could exist in the driver. The retry counter
won't be much complicated here also. But I do not insist so if you
really think it is not worth it then I am fine.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-05-02 14:59   ` Pavel Machek
@ 2015-06-24  7:32     ` Laurentiu Palcu
  2015-06-24  8:30       ` Pavel Machek
  0 siblings, 1 reply; 12+ messages in thread
From: Laurentiu Palcu @ 2015-06-24  7:32 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse, devicetree, linux-kernel, linux-pm

On Sat, May 02, 2015 at 04:59:34PM +0200, Pavel Machek wrote:
> 
> Should this link....
> 
> > More details about the chip can be found here:
> > http://www.ti.com/product/bq25890
> 
> > @@ -0,0 +1,998 @@
> > +/*
> > + * TI BQ25890 charger driver
> > + *
> > + * Copyright (C) 2015 Intel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> 
> > + *
> > + */
> 
> Go here?
Where exactly? Was 'here' supposed to be a hyperlink that, for some
reason, is missing? Anyway, if you're asking why the link I provided
doesn't go directly to the documentation, but to the product page,
here's the answer: the documentation for this chip was not released
publicly, yet, by TI. I was told they intend to do so in the near
future. So, for the time being, this link will do. I'll update it when
they'll release the docs.

laurentiu

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-06-24  7:32     ` Laurentiu Palcu
@ 2015-06-24  8:30       ` Pavel Machek
  2015-06-24  9:20         ` Laurentiu Palcu
  0 siblings, 1 reply; 12+ messages in thread
From: Pavel Machek @ 2015-06-24  8:30 UTC (permalink / raw)
  To: Laurentiu Palcu
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse, devicetree, linux-kernel, linux-pm

On Wed 2015-06-24 10:32:54, Laurentiu Palcu wrote:
> On Sat, May 02, 2015 at 04:59:34PM +0200, Pavel Machek wrote:
> > 
> > Should this link....
> > 
> > > More details about the chip can be found here:
> > > http://www.ti.com/product/bq25890
> > 
> > > @@ -0,0 +1,998 @@
> > > +/*
> > > + * TI BQ25890 charger driver
> > > + *
> > > + * Copyright (C) 2015 Intel Corporation
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > 
> > > + *
> > > + */
> > 
> > Go here?
> Where exactly? Was 'here' supposed to be a hyperlink that, for some
> reason, is missing? Anyway, if you're asking why the link I provided


Put the link to the comment at top of the driver, not into the
changelog.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip
  2015-06-24  8:30       ` Pavel Machek
@ 2015-06-24  9:20         ` Laurentiu Palcu
  0 siblings, 0 replies; 12+ messages in thread
From: Laurentiu Palcu @ 2015-06-24  9:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Sebastian Reichel, Krzysztof Kozlowski, Dmitry Eremin-Solenikov,
	David Woodhouse, devicetree, linux-kernel, linux-pm

On Wed, Jun 24, 2015 at 10:30:58AM +0200, Pavel Machek wrote:
> On Wed 2015-06-24 10:32:54, Laurentiu Palcu wrote:
> > On Sat, May 02, 2015 at 04:59:34PM +0200, Pavel Machek wrote:
> > > 
> > > Should this link....
> > > 
> > > > More details about the chip can be found here:
> > > > http://www.ti.com/product/bq25890
> > > 
> > > > @@ -0,0 +1,998 @@
> > > > +/*
> > > > + * TI BQ25890 charger driver
> > > > + *
> > > > + * Copyright (C) 2015 Intel Corporation
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published by
> > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > + * (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > + * GNU General Public License for more details.
> > > 
> > > > + *
> > > > + */
> > > 
> > > Go here?
> > Where exactly? Was 'here' supposed to be a hyperlink that, for some
> > reason, is missing? Anyway, if you're asking why the link I provided
> 
> 
> Put the link to the comment at top of the driver, not into the
> changelog.
I see now what you meant. :) I'll do that once the documentation is
public.

laurentiu


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-06-24  9:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-15 14:06 [PATCH 0/2] Add support for BQ25890 charger chip Laurentiu Palcu
2015-05-15 14:06 ` [PATCH 1/2] Documentation: devicetree: Add TI BQ25890 bindings Laurentiu Palcu
2015-05-16 11:53   ` Krzysztof Kozlowski
2015-05-18  6:54     ` Laurentiu Palcu
2015-05-15 14:06 ` [PATCH 2/2] power_supply: Add support for TI BQ25890 charger chip Laurentiu Palcu
2015-05-02 14:59   ` Pavel Machek
2015-06-24  7:32     ` Laurentiu Palcu
2015-06-24  8:30       ` Pavel Machek
2015-06-24  9:20         ` Laurentiu Palcu
2015-05-19  8:40   ` Krzysztof Kozlowski
2015-05-19  9:14     ` Laurentiu Palcu
2015-05-19 10:03       ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).