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 <[email protected]>
---
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
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
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
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