Re: [PATCH] power: bq24261_charger: Add support for TI BQ24261 charger

[Sebastian Reichel <sre@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2321 bytes --]

Hi,

On Fri, Sep 11, 2015 at 09:58:40AM +0900, Krzysztof Kozlowski wrote:
> On 11.09.2015 01:42, Andrew F. Davis wrote:
> > On 09/09/2015 06:47 PM, Krzysztof Kozlowski wrote:
> >>>>> +- ti,enable-user-write: boolean, if present driver will allow the
> >>>>> user space
> >>>>> +    to control the charging current and voltage through sysfs;
> >>>>
> >>>> This is not DT property. It does not describe hardware.
> >>> We needed a mechanism to enable the sysfs writes on certain properties.
> >>> If DT is not the place where should it go?
> >>
> >> DT is not the place. As I discussed later with Andreas, if you really
> >> need this and if mainline is a place for that then probably this should
> >> be compile option (a Kconfig symbol).
> >>
> > 
> > I think this would actually be a good use for module parameters, this way
> > it could still be set at boot without re-compiling.
> > 
> > I think compile-time disabling sysfs properties because they are
> > "dangerous" is
> > a little bit too artificially restricting and controlling, you can set
> > permissions
> > so only root can change them already. The kernel should not be
> > restricting root,
> > I understand the fear of someone rooting a machine and remotely over
> > charging
> > a LiPo[1], but these physical limits are hardware descriptions and can
> > and should
> > be set by DT, beyond this root should have full control over their machine.
> 
> 
> Indeed module parameters could be used for enabling/disabling debug
> options... but as fair as I understand these are for purely development
> purposes. That is why they got into DT initially, right? To allow the
> developer to play with it on the development board?
> 
> This is why I am really not convinced that this should go to mainline.
> 
> Anyway if it goes, then maybe compiling it out is the safest choice?
> What's the purpose of having it in kernel all the time? If this was a
> debug option, than some experienced user could turn it on and report to
> LKML with extended debug data. But it's not a debug but development option?

Changing the current limit is useful for "expert" users with custom
usb power supplies, that are not correctly detected by extcon. I
also think a module parameter would be the best option here.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]