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 17:37:25 +0200
From:	Sebastian Reichel <sre@...nel.org>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc:	"Andrew F. Davis" <afd@...com>,
	"Pallala, Ramakrishna" <ramakrishna.pallala@...el.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.

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)