Skip to contentSkip to conversation

s3c-adc-battery: Add naive POWER_SUPPLY_PROP_PRESENT implementation

PATCH In review Heiko Stübner 3 participants 6 messages Nov 10, 2011 – Nov 11, 2011

lore
Share & tools
lore

Heiko Stübner

author
#0 lore raw

PATCH 1/1 s3c-adc-battery: Add naive POWER_SUPPLY_PROP_PRESENT implementation

Userspace programs like upower seem to depend on the presence of the POWER_SUPPLY_PROP_PRESENT property.

The real presence of the battery cannot be sensed and all other routines of s3c-adc-battery simply expect it to be present.

Therefore we simply return 1, unless the battery struct is not present at all.

Signed-off-by Heiko Stuebner <[email protected]>
---
 drivers/power/s3c_adc_battery.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/power/s3c_adc_battery.c +18 -0
index d32d0d7..42d1f92 100644
@@ -51,6 +51,7 @@ static enum power_supply_property s3c_adc_backup_bat_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_PRESENT,
 };
 
 static int s3c_adc_backup_bat_get_property(struct power_supply *psy,

@@ -60,6 +61,11 @@ static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
 	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
 
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "%s: no battery infos ?!\n", __func__);
 		return -EINVAL;
 	}

@@ -83,6 +89,9 @@ static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
 		val->intval = bat->pdata->backup_volt_max;
 		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
 	default:
 		return -EINVAL;
 	}

@@ -106,6 +115,7 @@ static enum power_supply_property s3c_adc_main_bat_props[] = {
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_PRESENT,
 };
 
 static int calc_full_volt(int volt_val, int cur_val, int impedance)

@@ -132,6 +142,11 @@ static int s3c_adc_bat_get_property(struct power_supply *psy,
 	unsigned int lut_size = bat->pdata->lut_noac_cnt;
 
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "no battery infos ?!\n");
 		return -EINVAL;
 	}

@@ -205,6 +220,9 @@ static int s3c_adc_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		val->intval = bat->cur_value;
 		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
 	default:
 		return -EINVAL;
 	}
Signature · 1 line
1.6.1
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines · Heiko Stübner

Userspace programs like upower seem to depend on the presence of the POWER_SUPPLY_PROP_PRESENT property.

The real presence of the battery cannot be sensed and all other routines of s3c-adc-battery simply expect it to be present.

Therefore we simply return 1, unless the battery struct is not present at all.

Looks OK for me,

Quoted 1 line
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Vasily Khoruzhick <[email protected]>
Quoted 59 lines
---
 drivers/power/s3c_adc_battery.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/power/s3c_adc_battery.c +15 -0
index d32d0d7..42d1f92 100644
@@ -51,6 +51,7 @@ static enum power_supply_property s3c_adc_backup_bat_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_PRESENT,
 };
 
 static int s3c_adc_backup_bat_get_property(struct power_supply *psy,

@@ -60,6 +61,11 @@ static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
 	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
 
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "%s: no battery infos ?!\n", __func__);
 		return -EINVAL;
 	}

@@ -83,6 +89,9 @@ static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
 		val->intval = bat->pdata->backup_volt_max;
 		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
 	default:
 		return -EINVAL;
 	}

@@ -106,6 +115,7 @@ static enum power_supply_property s3c_adc_main_bat_props[] = {
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_PRESENT,
 };
 
 static int calc_full_volt(int volt_val, int cur_val, int impedance)

@@ -132,6 +142,11 @@ static int s3c_adc_bat_get_property(struct power_supply *psy,
 	unsigned int lut_size = bat->pdata->lut_noac_cnt;
 
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "no battery infos ?!\n");
 		return -EINVAL;
 	}

drivers/power/s3c_adc_battery.c static int s3c_adc_bat_get_property(struct power_supply *psy, · line 228

Quoted 9 lines
drivers/power/s3c_adc_battery.c +3 -0
@@ -205,6 +220,9 @@ static int s3c_adc_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		val->intval = bat->cur_value;
 		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
 	default:
 		return -EINVAL;
 	}

_______________________________________________
linux-pm mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

Acked-by on v1 1/1
#2 lore raw
In reply to Heiko Stübner
Quoted 7 lines · Heiko Stübner

Userspace programs like upower seem to depend on the presence of the POWER_SUPPLY_PROP_PRESENT property.

The real presence of the battery cannot be sensed and all other routines of s3c-adc-battery simply expect it to be present.

Therefore we simply return 1, unless the battery struct is not present at all.

Looks OK for me,

Quoted 1 line
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Vasily Khoruzhick <[email protected]>
Quoted 68 lines
---
 drivers/power/s3c_adc_battery.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/power/s3c_adc_battery.c +18 -0
index d32d0d7..42d1f92 100644
@@ -51,6 +51,7 @@ static enum power_supply_property s3c_adc_backup_bat_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_PRESENT,
 };
 
 static int s3c_adc_backup_bat_get_property(struct power_supply *psy,

@@ -60,6 +61,11 @@ static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
 	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
 
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "%s: no battery infos ?!\n", __func__);
 		return -EINVAL;
 	}

@@ -83,6 +89,9 @@ static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
 		val->intval = bat->pdata->backup_volt_max;
 		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
 	default:
 		return -EINVAL;
 	}

@@ -106,6 +115,7 @@ static enum power_supply_property s3c_adc_main_bat_props[] = {
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_PRESENT,
 };
 
 static int calc_full_volt(int volt_val, int cur_val, int impedance)

@@ -132,6 +142,11 @@ static int s3c_adc_bat_get_property(struct power_supply *psy,
 	unsigned int lut_size = bat->pdata->lut_noac_cnt;
 
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "no battery infos ?!\n");
 		return -EINVAL;
 	}

@@ -205,6 +220,9 @@ static int s3c_adc_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		val->intval = bat->cur_value;
 		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
 	default:
 		return -EINVAL;
 	}
#3 lore raw
Quoted 7 lines · Vasily Khoruzhick
Quoted 6 lines · Heiko Stübner

Userspace programs like upower seem to depend on the presence of the POWER_SUPPLY_PROP_PRESENT property.

The real presence of the battery cannot be sensed and all other routines of s3c-adc-battery simply expect it to be present.

Quoted 6 lines
Quoted 1 line

Therefore we simply return 1, unless the battery struct is not present at all.

Looks OK for me,

Quoted 1 line
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Vasily Khoruzhick <[email protected]>

I disagree. If the driver behaves like the hardware does not support hot-pluggable cells, then present property is not necessary.

Upowerd should look at the 'present' only if it is there; and if it is not, cells assumed to be always present.

[..]

Quoted 7 lines

drivers/power/s3c_adc_battery.c static int s3c_adc_backup_bat_get_property(struct power_supply *psy, · line 69

Quoted 7 lines
drivers/power/s3c_adc_battery.c +5 -0
 	if (!bat) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = 0;
+			return 0;
+		}
+
 		dev_err(psy->dev, "%s: no battery infos ?!\n", __func__);

And in this case that dev_err() tells that either hardware does not support hot-pluggable cells, or driver does not handle this case; which is actually the same from the userspace point of view.

So, the problem is in upowerd, not this driver.

Thanks,

Signature · 6 lines
Anton Vorontsov
Email: [email protected]
_______________________________________________
linux-pm mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
author
#4 lore raw

Am Donnerstag, 10. November 2011, 14:30:52 schrieb Anton Vorontsov:

Quoted 22 lines · Vasily Khoruzhick
Quoted 7 lines · Heiko Stübner
Quoted 6 lines

Userspace programs like upower seem to depend on the presence of the POWER_SUPPLY_PROP_PRESENT property.

The real presence of the battery cannot be sensed and all other routines of s3c-adc-battery simply expect it to be present.

Quoted 8 lines
Quoted 2 lines

Therefore we simply return 1, unless the battery struct is not present at all.

Looks OK for me,

Quoted 1 line
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Vasily Khoruzhick <[email protected]>

I disagree. If the driver behaves like the hardware does not support hot-pluggable cells, then present property is not necessary.

Upowerd should look at the 'present' only if it is there; and if it is not, cells assumed to be always present.

[..]

Quoted 1 line

So, the problem is in upowerd, not this driver.

Thanks for your comment. I will therefore take this matter up with the upower- guys.

Thanks
Heiko
_______________________________________________
linux-pm mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

author
#5 lore raw
In reply to Anton Vorontsov

Am Donnerstag, 10. November 2011, 14:30:52 schrieb Anton Vorontsov:

Quoted 22 lines · Vasily Khoruzhick
Quoted 7 lines · Heiko Stübner
Quoted 6 lines

Userspace programs like upower seem to depend on the presence of the POWER_SUPPLY_PROP_PRESENT property.

The real presence of the battery cannot be sensed and all other routines of s3c-adc-battery simply expect it to be present.

Quoted 8 lines
Quoted 2 lines

Therefore we simply return 1, unless the battery struct is not present at all.

Looks OK for me,

Quoted 1 line
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Vasily Khoruzhick <[email protected]>

I disagree. If the driver behaves like the hardware does not support hot-pluggable cells, then present property is not necessary.

Upowerd should look at the 'present' only if it is there; and if it is not, cells assumed to be always present.

[..]

Quoted 1 line

So, the problem is in upowerd, not this driver.

Thanks for your comment. I will therefore take this matter up with the upower- guys.

Thanks
Heiko