Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1230339

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

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

RE: [PATCH] power: bq24261_charger: Add support for TI BQ24261  charger "Pallala, Ramakrishna" <ramakrishna.pallala@intel.com> - 2015-09-09 20:20 +0200
  Re: [PATCH] power: bq24261_charger: Add support for TI BQ24261 charger Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-10 01:50 +0200
    Re: [PATCH] power: bq24261_charger: Add support for TI BQ24261  charger "Andrew F. Davis" <afd@ti.com> - 2015-09-10 18:50 +0200
      Re: [PATCH] power: bq24261_charger: Add support for TI BQ24261 charger Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-09-11 03:00 +0200
        Re: [PATCH] power: bq24261_charger: Add support for TI BQ24261  charger Sebastian Reichel <sre@kernel.org> - 2015-09-22 17:40 +0200
          RE: [PATCH] power: bq24261_charger: Add support for TI BQ24261  charger "Pallala, Ramakrishna" <ramakrishna.pallala@intel.com> - 2015-09-22 17:50 +0200

csiph-web