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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 22 Sep 2015 15:43:13 +0000
From:	"Pallala, Ramakrishna" <ramakrishna.pallala@...el.com>
To:	Sebastian Reichel <sre@...nel.org>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>
CC:	"Andrew F. Davis" <afd@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"Tc, Jenny" <jenny.tc@...el.com>,
	"Andreas Dannenberg" <dannenberg@...com>
Subject: RE: [PATCH] power: bq24261_charger: Add support for TI BQ24261
 charger

> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/