Re: [PATCH 01/13] power: Make power_supply_am_i_supplied return -ENODEV if there are no suppliers

[Krzysztof Kozlowski <krzk@kernel.org>]

On Fri, Apr 14, 2017 at 04:07:22PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 14-04-17 15:56, Krzysztof Kozlowski wrote:
> > On Fri, Apr 14, 2017 at 02:59:07PM +0200, Hans de Goede wrote:
> > > Allow a caller of power_supply_am_i_supplied to differentiate between
> > > there not being any suppliers, vs no suppliers being online by returning
> > > -ENODEV if there are no suppliers matching supplied_to / supplied_from.
> > > 
> > 
> > This is missing important piece of information - why you need to
> > differentiate that? What is the difference for you between no supplies
> > at all and not-being-supplied?
> 
> Thank you for the quick response.
> 
> I think it is sensible to assume that the hardware actually always has a
> way of charging the battery so where you say "no supplies at all" in
> reality what would be the case is : "no power_supply-drivers
> registered / bound for any supplies at all". At which point we can not
> determine in many fuel-gauge drivers (which are the only user of
> power_supply_am_i_supplied) if we're charging or discharging.
> 
> Here is the code for the STATUS property I'm adding to the max17042
> driver in a later commit in the set:
> 
> static int max17042_get_status(struct max17042_chip *chip, int *status)
> {
>         int ret, charge_full, charge_now;
> 
>         ret = power_supply_am_i_supplied(chip->battery);
>         if (ret < 0) {
>                 *status = POWER_SUPPLY_STATUS_UNKNOWN;
>                 return 0;
>         }
>         if (ret == 0) {
>                 *status = POWER_SUPPLY_STATUS_DISCHARGING;
>                 return 0;
>         }
> 
> 	...
> }

The explanation makes sense. It should be put in the commit msg as
rationale for this patch.


Best regards,
Krzysztof

> 
> This is where the -ENODEV comes in to make it properly return
> POWER_SUPPLY_STATUS_UNKNOWN instead of always returning
> POWER_SUPPLY_STATUS_DISCHARGING even if that may be untrue.
> 
> Note that I've since found out that the only in tree user of the
> max17042 driver is arch/arm/boot/dts/exynos4412-trats2.dts and
> the last patch in my makes the max77693 charger driver properly
> set supplied_to so that power_supply_am_i_supplied() will work
> correctly for the max17047 / max77693 combo on that board, which
> sort of renders this patch unnecessary. I still think this patch
> is a good idea, but it can be dropped if other people disagree.
> 
> Regards,
> 
> Hans
>