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

Groups > linux.kernel > #1575562 > unrolled thread

Re: [PATCH] power: supply: Add driver for TI BQ2416X battery charger

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] power: supply: Add driver for TI BQ2416X battery charger Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-07 12:10 +0100

#1575562 — Re: [PATCH] power: supply: Add driver for TI BQ2416X battery charger

On Tue, Feb 7, 2017 at 3:09 AM, Wojciech Ziemba <wo.ziemba@gmail.com> wrote:
> There is interest in adding a Linux driver for TI BQ2416X battery
> charger. The driver supports BQ24160 chip, thus can be easily extended
> for other BQ2416X family chargers.
> Device exposes 'POWER_SUPPLY_PROP_*' properties and a number of knobs
> for controlling the charging process as well as sends power supply change
> notification via power-supply subsystem.

Some style related comments

0. Less lines of code -> better! (but not be too eager)
1. Use GENMASK()
2.
int ret = -EINVAL;

if (a)
 ret = x;
else if (b)
 ret = y;

>> else
>> ret = -EINVAL;

Those are redundant.

if (ret)
 return ret;

3. #ifdef:s are ugly.
For CONFIG_PM_SLEEP functions just use
__maybe_unused attribute.

4. Try to avoid #ifdef CONFIG_OF (this will limit driver for OF case
when it might be used elsewhere, e.g. ACPI case)

5. Check headers and library for existing helpers. I believe some of
your bit operations and such already have nice helpers in kernel.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [standalone]

Back to top | Article view | linux.kernel

csiph-web