Re: [PATCH v2 13/13] dt: power: bq2425x-charger: Cover additional devices

[Krzysztof Kozlowski <k.kozlowski@samsung.com>]

On 11.09.2015 05:57, Andreas Dannenberg wrote:
> On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote:

(...)

> 
>>>> bq2415x.txt - ti,current-limit  - maximum current to be drawn in mA
>>>>
>>>> New bindings:
>>>> ti,charge-current (bq24261) - default charge current in mA
>>>> ti,max-charge-current (bq24261) - maximum charging current in mA
>>>
>>> This ti,max-charge-current is actually an interesting one. It's not a
>>> device setting as it does not impact any of the device registers at all.
>>> Instead, it's an artificial limit that can be set through DT that
>>> prevents somebody from going into sysfs and configuring a charge current
>>> higher than ti,max-charge-current. In other drivers I have seen that the
>>> sysfs property reflecting that max charge current is just read-only and
>>> gives you the maximum the HW is capable of. From a device point of view
>>> there is nothing configurable about this property.
>>
>> Hmmm, so now I wonder whether this should be a DT binding. The purpose
>> of DT isn't the control of the driver like enable some stuff, set some
>> value used by the driver. The DT provides information about hardware so
>> the driver could properly configure the device and work with it.
>>
>> Reason to put this into DT would be:
>> For example one device configuration (device, board, connected battery)
>> could handle one maximum value and the other configuration would require
>> lower one. The device and its capabilities (bq24257 for example) are the
>> same but configuration changes.
> 
> Yes this could be useful in this case but this property only makes sense
> if userspace is allowed to change the actual charge current through
> sysfs as some drivers do including Ram's proposed bq24261 driver. But if we
> get rid of the general sysfs configurability of the charge current
> (which I say we should) we don't need that ti,max-charge-current property.
> Also see...
> 
> http://marc.info/?l=linux-pm&m=143080413218161&w=2

Right, that's my previous reply. :) I must admit that here and for
bq24261 I looked mostly at bindings so I did not initially about the
sysfs interface.

Anyway I am not convinced to having such sysfs interfaces and definitely
they should not mess with DT. I would rather expect these to be totally
orthogonal.

Summarizing all these bq24261 properties:
 - ti,enable-user-write
 - ti,max-charge-current
 - ti,max-charge-voltage
are related to that sysfs interface, not to hardware configuration. If
that's true, then all should be gone.

> 
>> To prevent future issues in naming and bindings consistency maybe there
>> should be a dedicated maintainer for bq/TI charger devices? Pali Rohar
>> recently took care for devices related to Nokia N900 but maybe someone
>> from TI should also take care of rest or all of them (if TI is
>> interested in this)?
> 
> I'd be happy to step in here. Let me know how I can help and contribute.

I don't know Sebastian's opinion on that idea but I think usually a new
maintainer and reviewer of particular drivers is welcomed by community.
I see also Intel's interest and his contributions in these chargers.
Laurentiu seems to do a thorough review as well. There can be both
official reviewers.

It's up to you people. Just make a first step and we'll see how other
people react (and what Sebastian thinks about it :) ).

Best regards,
Krzysztof