lkml.org 
[lkml]   [2015]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] power_supply: Add support for tps65217-charger.
Hi,

On Tue, Sep 08, 2015 at 10:09:38AM +0200, Enric Balletbo i Serra wrote:
> This patch adds support for the tps65217 charger driver. This driver is
> responsible for controlling the charger aspect of the tps65217 mfd.
> Currently, this mainly consists of turning on and off the charger, but
> some other features of the charger can be supported through this driver.

Driver looks mostly fine. I have two comments though:

> [...]
>
> +static int tps65217_ac_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct tps65217_charger *charger = power_supply_get_drvdata(psy);
> +
> + if (psp == POWER_SUPPLY_PROP_ONLINE) {
> + val->intval = charger->ac_online;
> + charger->prev_ac_online = charger->ac_online;

I think prev_ac_online should be set at the beginning of
tps65217_charger_irq().

> [...]
>
> +static int tps65217_charger_probe(struct platform_device *pdev)
> +{
>
> [...]
>
> + charger->ac = power_supply_register(&pdev->dev,
> + &tps65217_charger_desc, NULL);
> + if (IS_ERR(charger->ac)) {
> + dev_err(&pdev->dev, "failed: power supply register\n");
> + return PTR_ERR(charger->ac);
> + }

You can use devm_power_supply_register(...) to simplify the driver
a bit more.

-- Sebastian
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2015-09-22 16:01    [W:0.305 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site