linux-kernel - Re: [PATCHv2] power_supply: Add support for Richtek rt9455 battery 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] [day] [month] [year] [list]

Date:	Tue, 5 May 2015 14:35:16 +0900
From:	Krzysztof Kozłowski <k.kozlowski.k@...il.com>
To:	Laurentiu Palcu <laurentiu.palcu@...el.com>
Cc:	"Nicolae, Anda-maria" <anda-maria.nicolae@...el.com>,
	"sre@...nel.org" <sre@...nel.org>,
	"dbaryshkov@...il.com" <dbaryshkov@...il.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"pawel.moll@....com" <pawel.moll@....com>,
	"mark.rutland@....com" <mark.rutland@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"galak@...eaurora.org" <galak@...eaurora.org>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCHv2] power_supply: Add support for Richtek rt9455 battery charger

2015-05-05 0:22 GMT+09:00 Laurentiu Palcu <laurentiu.palcu@...el.com>:
> Hi Anda,
>
> Can you please use a decent mail client to reply to emails? It was
> really hard for me to figure out which were my comments and which were
> yours... And I tested 3 different clients: mutt, evolution and
> outlook(the web app).
>
> More comments inline.
>
> laurentiu
>
> On Mon, May 04, 2015 at 05:12:07PM +0300, Nicolae, Anda-maria wrote:

(...)

>> > +
>> (...)
>> > +
>> > +static int rt9455_charger_set_voltage_max(struct rt9455_info *info,
>> > +                                       const union power_supply_propval *val)
>> > +{
>> > +     return rt9455_set_field_val(info, F_VMREG,
>> > +                                 rt9455_vmreg_values,
>> > +                                 ARRAY_SIZE(rt9455_vmreg_values),
>> > +                                 val->intval);
>> > +}
>> > +
>> > +static int rt9455_charger_set_property(struct power_supply *psy,
>> > +                                    enum power_supply_property psp,
>> > +                                    const union power_supply_propval *val)
>> > +{
>> > +     struct rt9455_info *info = power_supply_get_drvdata(psy);
>> > +
>> > +     switch (psp) {
>> > +     case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
>> > +             return rt9455_charger_set_current(info, val);
>> > +     case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>> > +             return rt9455_charger_set_voltage(info, val);
>> Personally, I'm against having these properties writable. A user might
>> play with the charging voltage or current and then complain that the
>> battery does not charge at all, or it takes ages to charge. I'd leave
>> these to be set by manufacturer through DT/ACPI. But, it's just an
>> opinion.
>>
>> If the user really wants to damage his/her battery, he/she can charge
>> the values from DT/ACPI.

No, the end user does not change the DT/ACPI. It just runs some Linux
distro or Android phone. The userspace does not change DT either. So
allowing userspace to set this through regular sysfs API is rather
exceptional.

>> Also, numerous charger drivers expose these
>> properties as writable.  I prefer leaving them as they are.

I found only one mainline driver which exposes these properties as
writeable: bq24190_charger.c. Others do not. Many other drivers uses
power supply class only as a reading interface. The charger is
actually operated through respective regulator driver.

Best regards,
Krzysztof
--
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/