Re: [PATCH v4 01/18] dt-bindings: power: battery: add constant-charge-current property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux