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

[Pallala, Ramakrishna]

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/