From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [PATCH 01/13] power: Make power_supply_am_i_supplied return -ENODEV if there are no suppliers Date: Fri, 14 Apr 2017 16:31:04 +0200 Message-ID: <20170414143104.afao53fxo5hbwqgu@kozik-lap> References: <20170414125919.25771-1-hdegoede@redhat.com> <20170414135638.pbcjxvvf6a7gfizw@kozik-lap> <67216acf-89da-033c-8214-7166e858a306@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Received: from mail-wr0-f194.google.com ([209.85.128.194]:35317 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752688AbdDNObJ (ORCPT ); Fri, 14 Apr 2017 10:31:09 -0400 Received: by mail-wr0-f194.google.com with SMTP id l44so12644404wrc.2 for ; Fri, 14 Apr 2017 07:31:08 -0700 (PDT) Content-Disposition: inline In-Reply-To: <67216acf-89da-033c-8214-7166e858a306@redhat.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Hans de Goede Cc: Sebastian Reichel , Bartlomiej Zolnierkiewicz , linux-pm@vger.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 >