Hi Quentin, On Thu, Mar 16, 2017 at 12:42 AM, Liam Breck <liam@xxxxxxxxxxxxxxxxx> wrote: > On Thu, Mar 16, 2017 at 12:03 AM, Quentin Schulz > <quentin.schulz@xxxxxxxxxxxxxxxxxx> wrote: >> Hi Liam, >> >> On 16/03/2017 07:27, Liam Breck wrote: >>> Hi Rob, >>> >>> On Wed, Mar 15, 2017 at 6:10 AM, Quentin Schulz >>> <quentin.schulz@xxxxxxxxxxxxxxxxxx> wrote: >>>> Hi Liam, >>>> >>>> On 15/03/2017 13:08, Liam Breck wrote: >>>>> I dropped most of the CCs, pls re-add anyone essential. Use Cc lines >>>>> in patch description to direct a patch to interested parties and >>>>> relevant lists. I don't want to see all 19 patches in this series. >>>>> >>>>> On Wed, Mar 15, 2017 at 3:55 AM, Quentin Schulz >>>>> <quentin.schulz@xxxxxxxxxxxxxxxxxx> wrote: >>>>>> This adds the constant-charge-current property to the list of optional >>>>>> properties of the battery. >>>>>> >>>>>> The constant charge current is critical for batteries as they can't >>>>>> handle all charge currents. >>>>>> >>>>>> Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxx> >>>>>> Acked-by: Rob Herring <robh@xxxxxxxxxx> >>>>>> --- >>>>>> >>>>>> v4: >>>>>> - switch from constant-charge-current-microamp to constant-charge-microamp, >>> >>> Sebastian is on record supporting name alignment between DT:battery >>> properties and enum power_supply_property. >>> >>> Could you OK a change back to constant-charge-current-microamp in this patch? >>> >>> Note that the following could appear in DT:battery in future: >>> >>> constant-charge-current-microamp >>> constant-charge-current-max-microamp >> >> OK. I am actually setting max constant charge current and constant >> current charge with the same DT property so I might need to adapt my >> patches. > > I suspect you should set battery:*-current-max with DT, as -current is > "programmed by charger" so seems to be a stat, not a fixed > characteristic. Did you resolve this issue? Rob & Sebastian want me to merge all items for bindings/power/supply/battery.txt. If I do that I'll also roll up their counterparts in power_supply_core.c and submit a patchset for those two files. > You may program your charger's -current with the battery's > -current-max, of course. > > Note that your charger also has a -current-max characteristic, likely > different than your battery. You may not need that in DT tho. > >>> constant-charge-voltage-microvolt >>> constant-charge-voltage-max-microvolt >>> >>> Thanks! >>> >>>>> Must be constant-charge-current-microamp for the reasons discussed in >>>>> battery.txt - consistency with sysfs names. >>>>> >>>> >>>> Hum. Just nitpicking, but I disagree with the use of 'must'. IIRC there >>>> is nothing in the code that would require the property to be named after >>>> a property from the enum power_supply_property. >>>> >>>> I would say that you _want_ it to be named like that because it >>>> underlines the relation between the DT property and the actual impacted >>>> property in the power supply subsystem. I'm fine with this reason but in >>>> the end, the maintainer's opinion prevails (if (s)he does not want it, >>>> (s)he will not take it). So, basically, I actually don't mind either >>>> option and I see arguments on each side. >>>> >>>> So, just waiting for maintainer's opinion to make the final version of >>>> this patch. >>>> >>>> Thanks, >>>> Quentin >>>> >>>>> Also applies to power_supply_core.c patch. >>>>> >>>>> Rob: enum power_supply_property also lists constant-charge-voltage, >>>>> hence the -current >>>>> >>>>> >>>>>> added in v3 >>>>>> >>>>>> Documentation/devicetree/bindings/power/supply/battery.txt | 2 ++ >>>>>> 1 file changed, 2 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt >>>>>> index 0278617..9594e1e 100644 >>>>>> --- a/Documentation/devicetree/bindings/power/supply/battery.txt >>>>>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt >>>>>> @@ -7,6 +7,7 @@ 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 >>>>>> + - constant-charge-microamp: battery constant charge current >>>>>> >>>>>> Because drivers surface properties in sysfs using names derived >>>>>> from enum power_supply_property, e.g. >>>>>> @@ -30,6 +31,7 @@ Example: >>>>>> voltage-min-design-microvolt = <3200000>; >>>>>> energy-full-design-microwatt-hours = <5290000>; >>>>>> charge-full-design-microamp-hours = <1430000>; >>>>>> + constant-charge-microamp = <300000>; >>>>>> }; >>>>>> >>>>>> charger: charger@11 { >>>>>> -- >>>>>> 2.9.3 >>>>>> >>>> >>>> -- >>>> Quentin Schulz, Free Electrons >>>> Embedded Linux and Kernel engineering >>>> http://free-electrons.com >> >> -- >> Quentin Schulz, Free Electrons >> Embedded Linux and Kernel engineering >> http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html