Re: [PATCHv2 2/2] power_supply: Add support for tps65217-charger.

2015-09-24 Thread Enric Balletbo Serra 
Hi,

2015-09-24 17:45 GMT+02:00 Sebastian Reichel :
> Hi,
>
> On Thu, Sep 24, 2015 at 09:23:25AM +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.
>>
>> [...]
>>
>> +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 this is no longer needed at this place.
>
Right, as I moved ...

>> + return 0;
>> + }
>> + return -EINVAL;
>> +}
>>
>> [...]
>>
>> +static int tps65217_charger_remove(struct platform_device *pdev)
>> +{
>> + struct tps65217_charger *charger = platform_get_drvdata(pdev);
>> +
>> + kthread_stop(charger->poll_task);
>> +
>> + power_supply_unregister(charger->ac);
>
> This is not needed with devm_power_supply_register() and actually
> it's wrong, since it would be called again by managed resources
> infrastructure.
>

Yes I missed this unregister. I'll send v3

Thanks,
Enric

>> + return 0;
>> +}
>
> -- Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/2] power_supply: Add support for tps65217-charger.

2015-09-24 Thread Sebastian Reichel 
Hi,

On Thu, Sep 24, 2015 at 09:23:25AM +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.
>
> [...]
>
> +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 this is no longer needed at this place.

> + return 0;
> + }
> + return -EINVAL;
> +}
> 
> [...]
>
> +static int tps65217_charger_remove(struct platform_device *pdev)
> +{
> + struct tps65217_charger *charger = platform_get_drvdata(pdev);
> +
> + kthread_stop(charger->poll_task);
> +
> + power_supply_unregister(charger->ac);

This is not needed with devm_power_supply_register() and actually
it's wrong, since it would be called again by managed resources
infrastructure.

> + return 0;
> +}

-- Sebastian


signature.asc
Description: Digital signature

Re: [PATCHv2 2/2] power_supply: Add support for tps65217-charger.

2015-09-24 Thread Sebastian Reichel 
Hi,

On Thu, Sep 24, 2015 at 09:23:25AM +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.
>
> [...]
>
> +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 this is no longer needed at this place.

> + return 0;
> + }
> + return -EINVAL;
> +}
> 
> [...]
>
> +static int tps65217_charger_remove(struct platform_device *pdev)
> +{
> + struct tps65217_charger *charger = platform_get_drvdata(pdev);
> +
> + kthread_stop(charger->poll_task);
> +
> + power_supply_unregister(charger->ac);

This is not needed with devm_power_supply_register() and actually
it's wrong, since it would be called again by managed resources
infrastructure.

> + return 0;
> +}

-- Sebastian


signature.asc
Description: Digital signature

Re: [PATCHv2 2/2] power_supply: Add support for tps65217-charger.

2015-09-24 Thread Enric Balletbo Serra 
Hi,

2015-09-24 17:45 GMT+02:00 Sebastian Reichel :
> Hi,
>
> On Thu, Sep 24, 2015 at 09:23:25AM +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.
>>
>> [...]
>>
>> +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 this is no longer needed at this place.
>
Right, as I moved ...

>> + return 0;
>> + }
>> + return -EINVAL;
>> +}
>>
>> [...]
>>
>> +static int tps65217_charger_remove(struct platform_device *pdev)
>> +{
>> + struct tps65217_charger *charger = platform_get_drvdata(pdev);
>> +
>> + kthread_stop(charger->poll_task);
>> +
>> + power_supply_unregister(charger->ac);
>
> This is not needed with devm_power_supply_register() and actually
> it's wrong, since it would be called again by managed resources
> infrastructure.
>

Yes I missed this unregister. I'll send v3

Thanks,
Enric

>> + return 0;
>> +}
>
> -- Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/