lkml.org 
[lkml]   [2015]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] power: bq24261_charger: Add support for TI BQ24261 charger
Date
> 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.

Ok. I will resubmit the patches along with fixing the other comments.

Thanks,
Ram


\
 
 \ /
  Last update: 2015-09-22 18:01    [W:0.088 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site