All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: anish kumar <anish198519851985@gmail.com>
Cc: lars@metafoo.de, jic23@cam.ac.uk, cbou@mail.ru,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org
Subject: Re: [PATCH] power: battery: Generic battery driver using IIO
Date: Thu, 13 Sep 2012 21:39:25 +0100	[thread overview]
Message-ID: <5052447D.8010907@kernel.org> (raw)
In-Reply-To: <1347552992-13836-1-git-send-email-anish198519851985@gmail.com>

On 09/13/2012 05:16 PM, anish kumar wrote:
> From: anish kumar <anish198519851985@gmail.com>
>
> In this version:
> Addressed concerns raised by lars:
> a. made the adc_bat per device.
> b. get the IIO channel using hardcoded channel names.
Couple of bits related to this inline..

> c. Minor issues related to gpio_is_valid and some code
>    refactoring.
>
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
>  drivers/power/generic-adc-battery.c       |  460 +++++++++++++++++++++++++++++
>  include/linux/power/generic-adc-battery.h |   33 ++
>  2 files changed, 493 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/power/generic-adc-battery.c
>  create mode 100644 include/linux/power/generic-adc-battery.h
>
> diff --git a/drivers/power/generic-adc-battery.c b/drivers/power/generic-adc-battery.c
> new file mode 100644
> index 0000000..003e0e1
> --- /dev/null
> +++ b/drivers/power/generic-adc-battery.c
> @@ -0,0 +1,460 @@
> +/*
> + * Generic battery driver code using IIO
> + * Copyright (C) 2012, Anish Kumar <anish198519851985@gmail.com>
> + * based on jz4740-battery.c
> + * based on s3c_adc_battery.c
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file COPYING in the main directory of this archive for
> + * more details.
> + *
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/gpio.h>
> +#include <linux/err.h>
> +#include <linux/timer.h>
> +#include <linux/jiffies.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/types.h>
> +
> +#include <linux/power/generic-adc-battery.h>
> +
> +#define to_generic_bat(ptr)	\
> +	container_of(ptr, struct generic_adc_bat, ptr)
> +
> +enum chan_type {
> +	VOLTAGE = 0,
> +	CURRENT,
> +	POWER,
> +	MAX_CHAN_TYPE
> +};
> +
> +#define CHAN_MAX_NAME 30
> +/*
> + * channel_name suggests the standard channel names for commonly used
> + * channel types.
> + */
const
> +char channel_name[][CHAN_MAX_NAME + 1] = {
> +	[VOLTAGE]	= "voltage",
> +	[CURRENT]	= "current",
> +	[POWER]		= "power",
> +};
> +
> +struct generic_adc_bat {
> +	struct power_supply	psy;
> +	struct iio_channel	**channel;
> +	struct iio_battery_platform_data	*pdata;
> +	struct delayed_work bat_work;
> +	int		was_plugged;
> +	int		volt_value;
> +	int		cur_value;
> +	int		level;
> +	int		status;
> +	int		cable_plugged:1;
> +};
> +
> +static void generic_adc_bat_ext_power_changed(struct power_supply *psy)
> +{
> +	struct generic_adc_bat *adc_bat;
> +	adc_bat = to_generic_bat(psy);
> +
> +	schedule_delayed_work(&adc_bat->bat_work,
> +			msecs_to_jiffies(adc_bat->pdata->jitter_delay));
> +}
> +
> +static enum power_supply_property bat_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_TECHNOLOGY,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> +	POWER_SUPPLY_PROP_MODEL_NAME,
> +};
> +
> +/*
> + * This properties are set based on the received platform data and this
> + * should correspond one-to-one with enum chan_type.
> + */
> +static enum power_supply_property dyn_props[] = {
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_POWER_NOW,
> +};
> +
> +static int charge_finished(struct generic_adc_bat *adc_bat)
> +{
> +	return adc_bat->pdata->gpio_inverted ?
> +		!gpio_get_value(adc_bat->pdata->gpio_charge_finished) :
> +		gpio_get_value(adc_bat->pdata->gpio_charge_finished);
> +}
> +
> +long read_scale_channel(struct generic_adc_bat *adc_bat,
> +			enum power_supply_property psp)
> +{
> +	int scaleint, scalepart, iio_val, ret = 0;
> +	int result = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_POWER_NOW:
> +		ret = iio_read_channel_raw(adc_bat->channel[POWER],
> +				&iio_val);
> +		ret = iio_read_channel_scale(adc_bat->channel[POWER],
> +				&scaleint, &scalepart);
> +		if (ret < 0)
> +			return ret;
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = iio_read_channel_raw(adc_bat->channel[VOLTAGE],
> +				&iio_val);
> +		ret = iio_read_channel_scale(adc_bat->channel[VOLTAGE],
> +				&scaleint, &scalepart);
> +		if (ret < 0)
> +			return ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = iio_read_channel_raw(adc_bat->channel[CURRENT],
> +				&iio_val);
> +		ret = iio_read_channel_scale(adc_bat->channel[CURRENT],
> +				&scaleint, &scalepart);
> +		if (ret < 0)
> +			return ret;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	switch (ret) {
> +	case IIO_VAL_INT:
> +		result = iio_val * scaleint;
> +		break;
> +	case IIO_VAL_INT_PLUS_MICRO:
> +		result = (s64)iio_val * (s64)scaleint +
> +			div_s64((s64)iio_val * (s64)scalepart, 1000000LL);
> +		break;
> +	case IIO_VAL_INT_PLUS_NANO:
> +		result = (s64)iio_val * (s64)scaleint +
> +			div_s64((s64)iio_val * (s64)scalepart, 1000000000LL);
> +		break;
> +	}
> +	return result;
> +}
> +
> +static int generic_adc_bat_get_property(struct power_supply *psy,
> +		enum power_supply_property psp,
> +		union power_supply_propval *val)
> +{
> +	struct generic_adc_bat *adc_bat;
> +	int ret = 0;
> +	long result;
> +
> +	adc_bat = to_generic_bat(psy);
> +	if (!adc_bat) {
> +		dev_err(psy->dev, "no battery infos ?!\n");
> +		return -EINVAL;
> +	}
> +
> +	result = read_scale_channel(adc_bat, psp);
> +	if (result < 0) {
> +		dev_err(psy->dev, "read channel error\n");
> +		ret =  -EINVAL;
> +		goto err;
> +	}
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		if (gpio_is_valid(adc_bat->pdata->gpio_charge_finished)) {
> +			if (adc_bat->pdata->gpio_charge_finished < 0)
> +				val->intval = adc_bat->level == 100000 ?
> +					POWER_SUPPLY_STATUS_FULL :
> +					adc_bat->status;
> +			else
> +				val->intval = adc_bat->status;
> +		}
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN:
> +		val->intval = 0;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
> +		val->intval = adc_bat->pdata->cal_charge(result);
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		val->intval = result;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		val->intval = result;
> +		break;
> +	case POWER_SUPPLY_PROP_POWER_NOW:
> +		val->intval = result;
> +		break;
> +	case POWER_SUPPLY_PROP_TECHNOLOGY:
> +		val->intval = adc_bat->pdata->battery_info.technology;
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> +		val->intval = adc_bat->pdata->battery_info.voltage_min_design;
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> +		val->intval = adc_bat->pdata->battery_info.voltage_max_design;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		val->intval = adc_bat->pdata->battery_info.charge_full_design;
> +		break;
> +	case POWER_SUPPLY_PROP_MODEL_NAME:
> +		val->strval = adc_bat->pdata->battery_info.name;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +err:
> +	return ret;
> +}
> +
> +static void generic_adc_bat_work(struct work_struct *work)
> +{
> +	struct generic_adc_bat *adc_bat;
> +	struct delayed_work *delayed_work;
> +	int is_charged;
> +	int is_plugged;
> +
> +	delayed_work = container_of(work,
> +				struct delayed_work, work);
> +	adc_bat = container_of(delayed_work,
> +				struct generic_adc_bat, bat_work);
> +
> +	is_plugged = power_supply_am_i_supplied(&adc_bat->psy);
> +	adc_bat->cable_plugged = is_plugged;
> +	if (is_plugged != adc_bat->was_plugged) {
> +		adc_bat->was_plugged = is_plugged;
> +		if (is_plugged)
> +			adc_bat->status = POWER_SUPPLY_STATUS_CHARGING;
> +		else
> +			adc_bat->status = POWER_SUPPLY_STATUS_DISCHARGING;
> +	} else {
> +		if (gpio_is_valid(adc_bat->pdata->gpio_charge_finished)) {
> +			if ((adc_bat->pdata->gpio_charge_finished >= 0) &&
> +								is_plugged) {
> +				is_charged = charge_finished(adc_bat);
> +				if (is_charged)
> +					adc_bat->status =
> +						POWER_SUPPLY_STATUS_FULL;
> +				else
> +					adc_bat->status =
> +						POWER_SUPPLY_STATUS_CHARGING;
> +			}
> +		}
> +	}
> +
> +	power_supply_changed(&adc_bat->psy);
> +}
> +
> +static irqreturn_t generic_adc_bat_charged(int irq, void *dev_id)
> +{
> +	struct generic_adc_bat *adc_bat = dev_id;
> +	adc_bat = container_of(&adc_bat->bat_work,
> +				struct generic_adc_bat, bat_work);
> +	schedule_delayed_work(&adc_bat->bat_work,
> +			msecs_to_jiffies(adc_bat->pdata->jitter_delay));
> +	return IRQ_HANDLED;
> +}
> +
> +static int __devinit generic_adc_bat_probe(struct platform_device *pdev)
> +{
> +	struct generic_adc_bat *adc_bat;
> +	struct iio_battery_platform_data *pdata = pdev->dev.platform_data;
> +	int ret, chan_index, fail = 0;
> +
> +	adc_bat = devm_kzalloc(&pdev->dev, sizeof(*adc_bat), GFP_KERNEL);
> +	if (!adc_bat) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	/* copying the battery name from platform data */
> +	adc_bat->psy.name = pdata->battery_name;
> +
> +	/* bootup default values for the battery */
> +	adc_bat->volt_value = -1;
> +	adc_bat->cur_value = -1;
> +	adc_bat->cable_plugged = 0;
> +	adc_bat->status = POWER_SUPPLY_STATUS_DISCHARGING;
> +	adc_bat->psy.type = POWER_SUPPLY_TYPE_BATTERY;
> +	adc_bat->psy.get_property =
> +		generic_adc_bat_get_property;
> +	adc_bat->psy.external_power_changed =
> +		generic_adc_bat_ext_power_changed;
> +
> +	adc_bat->pdata = pdata;
> +
> +	/* calculate the total number of channels */
> +	for (chan_index = 0; channel_name[chan_index]; chan_index++)
> +		;
> +
> +	if (!chan_index) {
> +		pr_err("atleast provide one channel\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * copying the static properties and allocating extra memory for holding
> +	 * the extra configurable properties received from platform data.
> +	*/
> +	adc_bat->psy.properties = kzalloc(sizeof(bat_props)
> +					+ sizeof(int)*chan_index, GFP_KERNEL);
> +	if (!adc_bat->psy.properties) {
> +		ret = -ENOMEM;
> +		goto first_mem_fail;
> +	}
> +	memcpy(adc_bat->psy.properties, bat_props, sizeof(bat_props));
> +
> +	/* allocating memory for iio channels */
> +	adc_bat->channel = kzalloc(chan_index * sizeof(struct iio_channel),
> +					GFP_KERNEL);
> +	if (!adc_bat->channel) {
> +		ret = -ENOMEM;
> +		goto second_mem_fail;
> +	}
> +
> +	/*
> +	 * getting channel from iio and copying the battery properties
> +	 * based on the channel supported by consumer device.
> +	 */
> +	for (chan_index = 0; channel_name[chan_index]; chan_index++) {
> +		adc_bat->channel[chan_index] =
> +		iio_channel_get(dev_name(&pdev->dev),
> +			channel_name[chan_index]);
> +		/* we skip for channels which fail */
> +		if (IS_ERR(adc_bat->channel[chan_index]))
> +			fail++;
> +		else {
> +			static int index;
> +			memcpy(adc_bat->psy.properties +
> +				sizeof(bat_props) + sizeof(int)*index,
> +				&dyn_props[chan_index],
> +				sizeof(dyn_props[chan_index]));
> +			index++;
> +		}
> +	}
> +
> +	/* none of the channels are supported so let's bail out */
> +	if (fail == chan_index) {
> +		ret = PTR_ERR(adc_bat->channel[chan_index]);
> +		goto second_mem_fail;
> +	}
> +
> +	ret = power_supply_register(&pdev->dev, &adc_bat->psy);
> +	if (ret)
> +		goto err_reg_fail;
> +
> +	INIT_DELAYED_WORK(&adc_bat->bat_work, generic_adc_bat_work);
> +
> +	if (gpio_is_valid(pdata->gpio_charge_finished)) {
> +		ret = gpio_request(pdata->gpio_charge_finished, "charged");
> +		if (ret)
> +			goto err_gpio;
> +
> +		ret = request_any_context_irq(gpio_to_irq
> +				(pdata->gpio_charge_finished),
> +				generic_adc_bat_charged,
> +				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +				"battery charged", &adc_bat);
> +		if (ret)
> +			goto err_gpio;
> +	} else
> +		goto err_gpio; /* let's bail out */
> +
> +	platform_set_drvdata(pdev, &adc_bat);
> +
> +	/* Schedule timer to check current status */
> +	schedule_delayed_work(&adc_bat->bat_work,
> +			msecs_to_jiffies(0));
> +	return 0;
> +
> +err_gpio:
> +	power_supply_unregister(&adc_bat->psy);
> +err_reg_fail:
> +	for (chan_index = 0; channel_name[chan_index]; chan_index++)
> +		iio_channel_release(adc_bat->channel[chan_index]);
> +	kfree(adc_bat->channel);
> +second_mem_fail:
> +	kfree(adc_bat->psy.properties);
> +first_mem_fail:
> +	return ret;
> +}
> +
> +static int generic_adc_bat_remove(struct platform_device *pdev)
> +{
> +	int chan_index;
> +	struct generic_adc_bat *adc_bat = platform_get_drvdata(pdev);
> +
> +	power_supply_unregister(&adc_bat->psy);
> +
> +	if (gpio_is_valid(adc_bat->pdata->gpio_charge_finished)) {
> +		if (adc_bat->pdata->gpio_charge_finished >= 0) {
> +			free_irq(gpio_to_irq(
> +				adc_bat->pdata->gpio_charge_finished),
> +								NULL);
> +			gpio_free(adc_bat->pdata->gpio_charge_finished);
> +		}
> +	}
> +
> +	for (chan_index = 0; channel_name[chan_index]; chan_index++)
> +		iio_channel_release(adc_bat->channel[chan_index]);
> +	cancel_delayed_work(&adc_bat->bat_work);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int generic_adc_bat_suspend(struct device *dev)
> +{
> +	struct generic_adc_bat *adc_bat = dev_get_drvdata(dev);
> +
> +	cancel_delayed_work_sync(&adc_bat->bat_work);
> +	adc_bat->status = POWER_SUPPLY_STATUS_UNKNOWN;
> +	return 0;
> +}
> +
> +static int generic_adc_bat_resume(struct device *dev)
> +{
> +	struct generic_adc_bat *adc_bat = dev_get_drvdata(dev);
> +
> +	/* Schedule timer to check current status */
> +	schedule_delayed_work(&adc_bat->bat_work,
> +			msecs_to_jiffies(adc_bat->pdata->jitter_delay));
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops generic_adc_battery_pm_ops = {
> +	.suspend        = generic_adc_bat_suspend,
> +	.resume         = generic_adc_bat_resume,
> +};
> +
> +#define GENERIC_ADC_BATTERY_PM_OPS       (&generic_adc_battery_pm_ops)
> +#else
> +#define GENERIC_ADC_BATTERY_PM_OPS       (NULL)
> +#endif
> +
> +static struct platform_driver generic_adc_bat_driver = {
> +	.driver		= {
> +		.name	= "generic-adc-battery",
> +		.owner	= THIS_MODULE,
> +		.pm	= GENERIC_ADC_BATTERY_PM_OPS
> +	},
> +	.probe		= generic_adc_bat_probe,
> +	.remove		= generic_adc_bat_remove,
> +};
> +
> +module_platform_driver(generic_adc_bat_driver);
> +
> +MODULE_AUTHOR("anish kumar <anish198519851985@gmail.com>");
> +MODULE_DESCRIPTION("generic battery driver using IIO");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/power/generic-adc-battery.h b/include/linux/power/generic-adc-battery.h
> new file mode 100644
> index 0000000..d7c6d20
> --- /dev/null
> +++ b/include/linux/power/generic-adc-battery.h
> @@ -0,0 +1,33 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef GENERIC_ADC_BATTERY_H
> +#define GENERIC_ADC_BATTERY_H
> +
> +/* should be enough for the channel names */
> +#define BAT_MAX_NAME	30
> +
> +enum chan_type {
> +	VOLTAGE = 0,
> +	CURRENT,
> +	POWER,
> +	MAX_CHAN_TYPE
> +};
> +
why this definition?
> +extern char channel_name[][BAT_MAX_NAME + 1];
> +
> +struct iio_battery_platform_data {
> +	struct power_supply_info battery_info;

Should this still be here?
> +	char	**channel_name;
> +	char	*battery_name;
> +	int	(*cal_charge)(long);
> +	int	gpio_charge_finished;
> +	int	gpio_inverted;

unused..
> +	int	bat_poll_interval;
> +	int	jitter_delay;
> +};
> +
> +#endif /* GENERIC_ADC_BATTERY_H */
>

  reply	other threads:[~2012-09-13 20:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-13 16:16 [PATCH] power: battery: Generic battery driver using IIO anish kumar
2012-09-13 20:39 ` Jonathan Cameron [this message]
2012-09-13 23:46 ` Anton Vorontsov
2012-09-14  5:21   ` anish singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5052447D.8010907@kernel.org \
    --to=jic23@kernel.org \
    --cc=anish198519851985@gmail.com \
    --cc=cbou@mail.ru \
    --cc=jic23@cam.ac.uk \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.