[PATCH] power:power_supply_syfs : Treat PROP_TYPE as a regular attribute first

[PATCH] power:power_supply_syfs : Treat PROP_TYPE as a regular attribute first

[Philippe De Swert]

These days we often have USB powered devices. This means that often the
type is variable. Common examples are smartphones which can be charged
through a normal USB port on a PC/laptop, a dedicated charger, etc...
Often those chips also have support for other charger types like a
mains/DC charger. This means that often there are several psy structures
in the driver and makes it impossible to stick to just one type.

Userspace sometimes needs to behave differently based on the type of charger
connected to such devices. This goes from loading gadget drivers to get
enumerated on a PC/laptop to draw more power, to enabling special charging
functions for a dedicated charger. Or to just enable userspace to discriminate
between the two for charging indications/USB cable detection.

Moreover TYPE seems to be the right attribute to use as it has definitions
already for all the different USB charger types.This patch allows for using the 
regular get_property function with the TYPE property without breaking current 
functionality/way of working.

Signed-off-by: Philippe De Swert <philippe.deswert@jollamobile.com>
---
 drivers/power/power_supply_sysfs.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index 29178f7..a839c91 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -73,19 +73,20 @@ static ssize_t power_supply_show_property(struct device *dev,
 	const ptrdiff_t off = attr - power_supply_attrs;
 	union power_supply_propval value;
 
-	if (off == POWER_SUPPLY_PROP_TYPE)
-		value.intval = psy->type;
-	else
-		ret = psy->get_property(psy, off, &value);
+	ret = psy->get_property(psy, off, &value);
 
 	if (ret < 0) {
-		if (ret == -ENODATA)
-			dev_dbg(dev, "driver has no data for `%s' property\n",
-				attr->attr.name);
-		else if (ret != -ENODEV)
-			dev_err(dev, "driver failed to report `%s' property: %zd\n",
-				attr->attr.name, ret);
-		return ret;
+		if (off == POWER_SUPPLY_PROP_TYPE)
+			value.intval = psy->type;
+		else {
+			if (ret == -ENODATA)
+				dev_dbg(dev, "driver has no data for `%s' property\n",
+					attr->attr.name);
+			else if (ret != -ENODEV)
+				dev_err(dev, "driver failed to report `%s' property: %zd\n",
+					attr->attr.name, ret);
+			return ret;
+		}
 	}
 
 	if (off == POWER_SUPPLY_PROP_STATUS)
-- 
1.8.1.2

Re: [PATCH] power:power_supply_syfs : Treat PROP_TYPE as a regular attribute first

[Anton Vorontsov]

On Thu, Aug 15, 2013 at 02:09:58AM +0300, Philippe De Swert wrote:
> These days we often have USB powered devices. This means that often the
> type is variable. Common examples are smartphones which can be charged
> through a normal USB port on a PC/laptop, a dedicated charger, etc...
> Often those chips also have support for other charger types like a
> mains/DC charger. This means that often there are several psy structures
> in the driver and makes it impossible to stick to just one type.

I would guess that a lot of userland code assumes that the type is fixed.
We can't break this assumption. Plus I don't think we really need the
changing types.

> Userspace sometimes needs to behave differently based on the type of charger
> connected to such devices.

A system with several charger should either:

1. Register all of them (mains, usb, etc.) as a separate power supply
device, and then use PROP_ONLINE to specify whether something is connected
to the port or not.

2. Register only the charger type that is currently connected to the
system.

This is how we've been doing things from the very start.

Thanks,

Anton

Re: [PATCH] power:power_supply_syfs : Treat PROP_TYPE as a regular attribute first

[Philippe De Swert]

Hi,

On 26/10/13 01:15, Anton Vorontsov wrote:
> On Thu, Aug 15, 2013 at 02:09:58AM +0300, Philippe De Swert wrote:
>> These days we often have USB powered devices. This means that often the
>> type is variable. Common examples are smartphones which can be charged
>> through a normal USB port on a PC/laptop, a dedicated charger, etc...
 >
> I would guess that a lot of userland code assumes that the type is fixed.
> We can't break this assumption. Plus I don't think we really need the
> changing types.

That is a fair assumption, but that does not work anymore with 
smartphones, GPS devices, tablets etc.. Moreover why is it the only 
property that would be fixed? Also we have the power supply name to 
identify it, and I expect most userland code uses that.

>> Userspace sometimes needs to behave differently based on the type of charger
>> connected to such devices.
>
> A system with several charger should either:
>
> 1. Register all of them (mains, usb, etc.) as a separate power supply
> device, and then use PROP_ONLINE to specify whether something is connected
> to the port or not.
> 2. Register only the charger type that is currently connected to the
> system.

I think I did not make myself very clear. Well there are actually no 
different chargers here. It is one USB charger input, however its 
properties are different depending on the actual connected "charger 
device/type". It is the same charging controller however when a usb 
cable is connected and power comes from a PC it is very different
from when it is a dedicated USB charger.

Constantly register/unregister the current "charger type" makes it very 
difficult to monitor. Having a separate power_supply device per possible 
connected charger type is pretty tedious. We are talking USB_DCP, 
USB_ACA, USB_CDP, USB already, and new ones probably in the future. Not 
to mention that at some point we would have to deal with OTG also were 
we might actually supply power. In any case a lot harder than having a 
changing type property.

One of the main reasons to monitor this is to have the device react 
differently depending on what gets plugged in. Pure charging when 
charger gets connected or mass-storage gadget mode, mtp, ... when a 
cable is connected. Or eventually switch to a totally different device 
mode to support things like terminal mode (in cars).

> This is how we've been doing things from the very start.

Well a number of upstream charging drivers and non-upstreamed android 
drivers already change the charger type  like for example the isp1704. 
The patch I proposed just changes the inconsistency with the charger 
type property as it is the only one that gets treated differently, to
avoid confusion with it.

But I am sure open to better ideas for handling this.

Thanks,

Philippe

Re: [PATCH] power:power_supply_syfs : Treat PROP_TYPE as a regular attribute first

[Anton Vorontsov]

On Sat, Oct 26, 2013 at 02:01:22AM +0300, Philippe De Swert wrote:
...
> I think I did not make myself very clear. Well there are actually no
> different chargers here. It is one USB charger input, however its
> properties are different depending on the actual connected "charger
> device/type". It is the same charging controller however when a usb
> cable is connected and power comes from a PC it is very different
> from when it is a dedicated USB charger.

See this discussion http://lkml.org/lkml/2013/10/28/39, there I am trying
to make a point that we need a separate subsystem for chargers. And in
power supply subsystem we want to only report the AC/USB supply state.

Thanks,

Anton