From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH v1 1/1] power: bq24190_charger: Deprecate battery class and replicate its features in charger Date: Tue, 18 Apr 2017 13:49:03 +0200 Message-ID: References: <20170416230626.30022-1-liam@networkimprov.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:35622 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752384AbdDRLtG (ORCPT ); Tue, 18 Apr 2017 07:49:06 -0400 In-Reply-To: Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Liam Breck Cc: Sebastian Reichel , linux-pm@vger.kernel.org, Tony Lindgren , Liam Breck Hi, On 18-04-17 12:27, Liam Breck wrote: > On Tue, Apr 18, 2017 at 3:05 AM, Hans de Goede wrote: >> Hi, >> >> On 18-04-17 11:42, Liam Breck wrote: >>> >>> Hi Hans, >>> >>> On Tue, Apr 18, 2017 at 1:05 AM, Hans de Goede >>> wrote: >>>> >>>> Hi Liam, >>>> >>>> On 17-04-17 01:06, Liam Breck wrote: >>>>> >>>>> >>>>> From: Liam Breck >>>>> >>>>> The driver was registering two classes, bq24190-battery & -charger. >>>>> Because the power supply framework cannot surface features from multiple >>>>> drivers in a single class, a fuel gauge driver would create a third >>>>> class, >>>>> which some power management utilities cannot see. >>>>> >>>>> Deprecate the -battery class for future removal and replicate its >>>>> features >>>>> in -charger. Set /sys/class...-charger/online = pg_stat && >>>>> !batfet_disable. >>>>> If device_property "omit-battery-class" is set, don't register -battery. >>>>> >>>>> Cc: Tony Lindgren >>>>> Cc: Hans de Goede >>>>> Signed-off-by: Liam Breck >>>> >>>> >>>> >>>> Thank you for writing this patch. >>> >>> >>> Glad to help! >>> >>>> The patch looks good to me: >>> >>> >>> Darn, I was hoping you'd see *something* amiss in all that clutter :-) >> >> >> Wel, I did not find the forward declaration of bq24190_battery_set_online >> and friends pretty, but I can understand why you did this and we can >> clean that up when we remove the battery interface altogether later. > > Blame the C standards crew for that mess :-) > >>> Do you think charger-powered and battery-connected is the right >>> definition of power_supply_prop_online? >> >> >> Now that you specifically ask I've been thinking a bit more this, >> but I still can't find anything wrong with that as definition :) > > The power_supply_class docs really should tell us how to define this. > >>> Sebastian had suggested surfacing the batfet as a regulator, but altho >>> user settable, it's a binary widget. >> >> >> Turning off the batfet typically will turn off the entire system, >> I believe it is more of an emergency break style switch which gets >> thrown by the charger under certain (very bad) conditions then >> something which we would actually ever want to use / control from >> within the kernel. > > It's supposed to be disabled at the factory prior to shipping to avoid > depleting the battery before product reaches buyer. The same should > also be done for storage. So it can be an end-user switch. True. Regards, Hans