Hi Liam,
On 29/03/2017 08:55, Liam Breck wrote:
> 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?
>
Sorry, completely missed your first mail.
Constant charge current is actually what I set in the PMIC. The maximum
constant charge current is completely PMIC-agnostic in my driver as it
is used to tell the user that (s)he shouldn't really have a constant
charge current over the maximum limit as the battery might not be able
to handle such high current.
What I suggest is to have both properties in DT. I'll set the maximum as
I do today and we can give the user the ability to set a "default"
constant charge current (below maximum constant charge current) in the
DT. Maybe we do not want to recharge your battery with the maximum value
by default?
> 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.
>
I guess that adding the two properties make sense, don't you think?
Also, could you Cc me on your next version of your patches? So I know
when to rebase and slightly rework my patches once your patch series is
merged.
Thanks,
Quentin
>> 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
--
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
