diff mbox series

[v2,3/7] power: supply: bq25890: Clean up POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE

Message ID 20221014172427.128512-3-marex@denx.de (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series [v2,1/7] power: supply: bq25890: Document POWER_SUPPLY_PROP_CURRENT_NOW | expand

Commit Message

Marek Vasut Oct. 14, 2022, 5:24 p.m. UTC
Clean up misuse of POWER_SUPPLY_PROP_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX
and POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE
and document what exactly each value means.

The POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE content is newly read
back from hardware, while POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX
is reported as the maximum value set in DT.

The POWER_SUPPLY_PROP_VOLTAGE is newly used to report immediate value
of battery voltage V_BAT, which is what this property was intended to
report and which has been thus far misused to report the charger chip
output voltage V_SYS.

The V_SYS is no longer reported as there is currently no suitable
property to report V_SYS. V_SYS reporting will be reinstated in
subsequent patch.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
To: linux-pm@vger.kernel.org
---
V2: Drop the state online check from new POWER_SUPPLY_PROP_VOLTAGE_NOW
    since the VBAT can be measured even when discharging
---
 drivers/power/supply/bq25890_charger.c | 72 ++++++++++++++++----------
 1 file changed, 45 insertions(+), 27 deletions(-)

Comments

Hans de Goede Oct. 15, 2022, 2:19 p.m. UTC | #1
Hi,

On 10/14/22 19:24, Marek Vasut wrote:
> Clean up misuse of POWER_SUPPLY_PROP_VOLTAGE,
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX
> and POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE
> and document what exactly each value means.
> 
> The POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE content is newly read
> back from hardware, while POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX
> is reported as the maximum value set in DT.
> 
> The POWER_SUPPLY_PROP_VOLTAGE is newly used to report immediate value
> of battery voltage V_BAT, which is what this property was intended to
> report and which has been thus far misused to report the charger chip
> output voltage V_SYS.
> 
> The V_SYS is no longer reported as there is currently no suitable
> property to report V_SYS. V_SYS reporting will be reinstated in
> subsequent patch.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Cc: Sebastian Reichel <sebastian.reichel@collabora.com>
> To: linux-pm@vger.kernel.org
> ---
> V2: Drop the state online check from new POWER_SUPPLY_PROP_VOLTAGE_NOW
>     since the VBAT can be measured even when discharging

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/power/supply/bq25890_charger.c | 72 ++++++++++++++++----------
>  1 file changed, 45 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 5924b036b1588..050eef2571e8e 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -529,24 +529,6 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
>  		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_find_val(bq->init_data.vreg, TBL_VREG);
> -		break;
> -
>  	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
>  		val->intval = bq25890_find_val(bq->init_data.iprechg, TBL_ITERM);
>  		break;
> @@ -563,15 +545,6 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  		val->intval = bq25890_find_val(ret, TBL_IINLIM);
>  		break;
>  
> -	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> -		ret = bq25890_field_read(bq, F_SYSV); /* 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_CURRENT_NOW:	/* I_BAT now */
>  		/*
>  		 * This is ADC-sampled immediate charge current supplied
> @@ -628,6 +601,51 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  		val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG);
>  		break;
>  
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:	/* V_BAT now */
> +		/*
> +		 * This is ADC-sampled immediate charge voltage supplied
> +		 * from charger to battery. The property name is confusing,
> +		 * for clarification refer to:
> +		 * Documentation/ABI/testing/sysfs-class-power
> +		 * /sys/class/power_supply/<supply_name>/voltage_now
> +		 */
> +		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:	/* V_BAT user limit */
> +		/*
> +		 * This is user-configured constant charge voltage supplied
> +		 * from charger to battery in second phase of charging, when
> +		 * battery voltage reached constant charge voltage.
> +		 *
> +		 * This value reflects the current hardware setting.
> +		 *
> +		 * The POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX is the
> +		 * maximum value of this property.
> +		 */
> +		ret = bq25890_field_read(bq, F_VREG);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = bq25890_find_val(ret, TBL_VREG);
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:	/* V_BAT max */
> +		/*
> +		 * This is maximum allowed constant charge voltage supplied
> +		 * from charger to battery in second phase of charging, when
> +		 * battery voltage reached constant charge voltage.
> +		 *
> +		 * This value is constant for each battery and set from DT.
> +		 */
> +		val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG);
> +		break;
> +
>  	case POWER_SUPPLY_PROP_TEMP:
>  		ret = bq25890_field_read(bq, F_TSPCT);
>  		if (ret < 0)
diff mbox series

Patch

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 5924b036b1588..050eef2571e8e 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -529,24 +529,6 @@  static int bq25890_power_supply_get_property(struct power_supply *psy,
 			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
 		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_find_val(bq->init_data.vreg, TBL_VREG);
-		break;
-
 	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
 		val->intval = bq25890_find_val(bq->init_data.iprechg, TBL_ITERM);
 		break;
@@ -563,15 +545,6 @@  static int bq25890_power_supply_get_property(struct power_supply *psy,
 		val->intval = bq25890_find_val(ret, TBL_IINLIM);
 		break;
 
-	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		ret = bq25890_field_read(bq, F_SYSV); /* 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_CURRENT_NOW:	/* I_BAT now */
 		/*
 		 * This is ADC-sampled immediate charge current supplied
@@ -628,6 +601,51 @@  static int bq25890_power_supply_get_property(struct power_supply *psy,
 		val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG);
 		break;
 
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:	/* V_BAT now */
+		/*
+		 * This is ADC-sampled immediate charge voltage supplied
+		 * from charger to battery. The property name is confusing,
+		 * for clarification refer to:
+		 * Documentation/ABI/testing/sysfs-class-power
+		 * /sys/class/power_supply/<supply_name>/voltage_now
+		 */
+		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:	/* V_BAT user limit */
+		/*
+		 * This is user-configured constant charge voltage supplied
+		 * from charger to battery in second phase of charging, when
+		 * battery voltage reached constant charge voltage.
+		 *
+		 * This value reflects the current hardware setting.
+		 *
+		 * The POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX is the
+		 * maximum value of this property.
+		 */
+		ret = bq25890_field_read(bq, F_VREG);
+		if (ret < 0)
+			return ret;
+
+		val->intval = bq25890_find_val(ret, TBL_VREG);
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:	/* V_BAT max */
+		/*
+		 * This is maximum allowed constant charge voltage supplied
+		 * from charger to battery in second phase of charging, when
+		 * battery voltage reached constant charge voltage.
+		 *
+		 * This value is constant for each battery and set from DT.
+		 */
+		val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG);
+		break;
+
 	case POWER_SUPPLY_PROP_TEMP:
 		ret = bq25890_field_read(bq, F_TSPCT);
 		if (ret < 0)