Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751599AbdFAKcU (ORCPT ); Thu, 1 Jun 2017 06:32:20 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:32974 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751180AbdFAKcS (ORCPT ); Thu, 1 Jun 2017 06:32:18 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170526110413.24836-1-enric.balletbo@collabora.com> From: Enric Balletbo Serra Date: Thu, 1 Jun 2017 12:32:16 +0200 Message-ID: Subject: Re: [PATCH v3 1/4] dt-bindings: power: supply: add battery charge voltage/current To: Liam Breck Cc: Enric Balletbo i Serra , Sebastian Reichel , linux-pm@vger.kernel.org, "devicetree@vger.kernel.org" , linux-kernel , Liam Breck Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4567 Lines: 119 2017-06-01 12:25 GMT+02:00 Liam Breck : > Hi Enric, > > > > On Thu, Jun 1, 2017 at 1:18 AM, Enric Balletbo Serra > wrote: >> Hi Liam, >> >> 2017-06-01 9:01 GMT+02:00 Liam Breck : >>> Hi Enric, >>> >>> On Sat, May 27, 2017 at 1:11 PM, Enric Balletbo Serra >>> wrote: >>>> Hi Liam, >>>> >>>> 2017-05-26 23:20 GMT+02:00 Liam Breck : >>>>> Hi Enric, >>>>> >>>>> On Fri, May 26, 2017 at 4:04 AM, Enric Balletbo i Serra >>>>> wrote: >>>>>> Add charging voltage and current characteristics to the battery DT for >>>>>> proper handling of the battery by fuel-gauge and charger chips. >>>>>> >>>>>> Signed-off-by: Enric Balletbo i Serra >>>>>> --- >>>>>> Changes since v2: >>>>>> - Requested by Sebastian Reichel >>>>>> - Move to its own patch and apply to simple-battery framework. >>>>>> Changes since v1: >>>>>> - Requested by Rob Herring >>>>>> - Rename ti,charge-* to charge-* to be standard properties. >>>>>> - Use unit suffixes as per bindings/property-units.txt >>>>>> >>>>>> Documentation/devicetree/bindings/power/supply/battery.txt | 4 ++++ >>>>>> 1 file changed, 4 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt >>>>>> index 63a7028..c87a439 100644 >>>>>> --- a/Documentation/devicetree/bindings/power/supply/battery.txt >>>>>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt >>>>>> @@ -12,6 +12,8 @@ Optional Properties: >>>>>> - voltage-min-design-microvolt: drained battery voltage >>>>>> - energy-full-design-microwatt-hours: battery design energy >>>>>> - charge-full-design-microamp-hours: battery design capacity >>>>>> + - charge-voltage-microvolt: battery charging voltage >>>>>> + - charge-current-microamp: battery charging current >>>>> >>>>> I think you mean constant-charge-* which is how you surface these >>>>> properties in your tps65217_charger patch. >>>>> >>>> >>>> Yes, to be strict this is constant-charge-* >>> >>> The DT battery node should carry static battery characteristics. So on >>> reflection, I think you want >>> >>> constant-charge-current-max-microamp >>> constant-charge-voltage-max-microvolt >>> >>> The charger or the user could then safely apply any value <= those. >>> >>> Thoughts? > > I'm curious to hear how your hw config requires constant-charge settings? > >> Hmm I see your point and I'm thinking now that actually this is not >> what I wanted to do. What I wanted is set the charger voltage and the >> current hence my first patchset was setting these properties in the >> charger node not the battery. > > You could certainly support constant-charge-* params in the charger > node. And it's easy to enable userspace to set these via sysfs. > >> Said that, I think that you have reason and what we want in battery >> node is the current/voltage max values but we also need to implement a >> mechanism to set the charging voltage/current from userspace or from >> the DT for the charger. The charger should be able to set these values >> and fail if, based in the battery specs, is not supported. > > The charger could default constant-charge-* to the battery node's max values. > > See also this discussion about similar issues. > https://patchwork.kernel.org/patch/9625331/ > Oh I missed that thread, please keep me in the loop for next versions of the patchset :) Best regards, Enric > >> More thoughts? >> >>>>> I'll add these to v14 of my patchset which adds simple-battery >>>>> support. Rob requested a single patch for this file. >>>>> >>>> >>>> Ok, I'll send the tps charger series without this patch, so please, >>>> include this patch in your series. >>>> >>>>> I've been waiting for feedback on v13.2 from Sebastian. If I don't >>>>> hear from him within a few days, I'll post v14. >>>>> >>>>>> Batteries must be referenced by chargers and/or fuel-gauges >>>>>> using a phandle. The phandle's property should be named >>>>>> @@ -24,6 +26,8 @@ Example: >>>>>> voltage-min-design-microvolt = <3200000>; >>>>>> energy-full-design-microwatt-hours = <5290000>; >>>>>> charge-full-design-microamp-hours = <1430000>; >>>>>> + charge-voltage-microvolt = <4100000>; >>>>>> + charge-current-microamp = <300000>; >>>>>> }; >>>>>> >>>>>> charger: charger@11 { >>>>>> -- >>>>>> 2.9.3 >>>>>>