[PATCH] jz battery driver cleanup

Lars-Peter Clausen lars at metafoo.de
Mon Sep 21 18:14:29 EDT 2009


Hi

I've fixed the compile error of the driver and did some other smaller
cleanups.
But there are still some issues with the driver. It would be nice if you
could send a follow up patch which addresses these.

- Lars

> This patch is cleanup more about gpio and platform data.
>
> Signed-off-by: Jiejing.Zhang <kzjeef at gmail.com>
> ---
>  
> diff --git a/target/linux/xburst/files-2.6.31/drivers/power/jz_battery.c
> b/target/linux/xburst/files-2.6.31/drivers/power/jz_battery.c
> index 80b3747..2d74d75 100755
> --- a/target/linux/xburst/files-2.6.31/drivers/power/jz_battery.c
> +++ b/target/linux/xburst/files-2.6.31/drivers/power/jz_battery.c
> @@ -6,6 +6,7 @@
>   * based on tosa_battery.c
>   *
>   * Copyright (C) 2008 Marek Vasut <marek.vasut at gmail.com>
> + * Copyright (C) 2009 Jiejing Zhang <kzjeef at gmail.com>
>   *
>   * 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
> @@ -19,28 +20,19 @@
>  #include <linux/spinlock.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> +#include <linux/jz4740_batt.h>
>
>  #include <asm/jzsoc.h>
>
> -#ifdef CONFIG_POWER_SUPPLY_DEBUG
> -#define dprintk(x...) printk(x)
> -#else
> -#define dprintk(x...) while(0){}
> -#endif
> -
> -#define JZ_BAT_MAX_VOLTAGE 4200000 // uV
> -#define JZ_BAT_MIN_VOLTAGE 3600000
> -
> -static DEFINE_MUTEX(bat_lock);
>  struct workqueue_struct *monitor_wqueue;
>  struct delayed_work bat_work;
>  struct mutex work_lock;
>
> -int bat_status = POWER_SUPPLY_STATUS_DISCHARGING;
> +static int bat_status = POWER_SUPPLY_STATUS_DISCHARGING;
> +static struct jz_batt_info *pdata = 0;
All these global variables should be put into a driver data struct.
>
>  extern unsigned int jz_read_battery(void);
>
> -
>  /*********************************************************************
>   *		Power
>   *********************************************************************/
> @@ -52,9 +44,9 @@ static int jz_get_power_prop(struct power_supply *psy,
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_ONLINE:
>  		if (psy->type == POWER_SUPPLY_TYPE_MAINS)
> -			val->intval = !__gpio_get_pin(GPIO_DC_DETE_N);
> +			val->intval = !gpio_get_value(pdata->dc_dect_gpio);
You use the gpio even though it might not have been valid. If the gpio
is not valid the power supply should not be registered.
>  		else
> -			val->intval = __gpio_get_pin(GPIO_USB_DETE);
> +			val->intval = !!gpio_get_value(pdata->usb_dect_gpio);
dito
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -92,13 +84,26 @@ static unsigned long jz_read_bat(struct
> power_supply *bat_ps)
>  {
>  	unsigned long val;
>  	if (CFG_PBAT_DIV == 1)
> -		val = (((unsigned long long)jz_read_battery() * 7500000)) >> 12;
> +		val = (((unsigned long long)jz_read_battery() * 7500000L) >> 12) + 33000L;
>  	else
> -		val = (((unsigned long long)jz_read_battery() * CFG_PBAT_DIV *
> 2500000)) >> 12;
> -	dprintk("--raw_batter_vol=%d uV\n", val);
> +		val = ((unsigned long long)jz_read_battery() * CFG_PBAT_DIV *
> 2500000L) >> 12;
> +	dev_dbg(bat_ps->dev, "%s: raw_batter_vol = %d uV\n",__func__,val);
>  	return val;
>  }
>
> +static int jz_bat_get_capacity(struct power_supply *bat_ps)
> +{
> +	int ret;
> +	ret = (jz_read_bat(bat_ps) - pdata->min_voltag) * 100
> +		/ (pdata->max_voltag - pdata->min_voltag);
> +	if (ret > 100) {
> +		dev_warn(bat_ps->dev, "%s: capacity=%d which exceeds 100,"
> +			 "set to 100\n", __func__, ret);
> +		ret = 100;
> +	}
> +	return ret;
> +}
> +
>  static int jz_bat_get_property(struct power_supply *bat_ps,
>  			    enum power_supply_property psp,
>  			    union power_supply_propval *val)
> @@ -108,40 +113,37 @@ static int jz_bat_get_property(struct
> power_supply *bat_ps,
>  		val->intval = bat_status;
>  		break;
>  	case POWER_SUPPLY_PROP_TECHNOLOGY:
> -		val->intval = POWER_SUPPLY_TECHNOLOGY_LIPO;
> +		val->intval = pdata->batt_tech;
>  		break;
>  	case POWER_SUPPLY_PROP_HEALTH:
> -		if(jz_read_bat(bat_ps) < 3600000) {
> -			dprintk("--battery dead\n");
> +		if(jz_read_bat(bat_ps) < JZ_BAT_MIN_VOLTAGE) {
> +			dev_dbg(bat_ps->dev, "%s: battery is dead,"
> +				"voltage too low!\n", __func__);
>  			val->intval = POWER_SUPPLY_HEALTH_DEAD;
>  		} else {
> -			dprintk("--battery good\n");
> +			dev_dbg(bat_ps->dev, "%s: battery is good,"
> +				"voltage normal.\n", __func__);
>  			val->intval = POWER_SUPPLY_HEALTH_GOOD;
>  		}
>  		break;
>  	case POWER_SUPPLY_PROP_CAPACITY:
> -		val->intval = (jz_read_bat(bat_ps) - 3600000) * 100 / (4200000 - 3600000);
> -		if (val->intval > 100)
> -			val->intval = 100;
> -		dprintk("--battery_capacity=%d\%\n",val->intval);
> +		val->intval = jz_bat_get_capacity(bat_ps);
> +		dev_dbg(bat_ps->dev, "%s: battery_capacity = %d\%\n",
> +			__func__, val->intval);
>  		break;
>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>  		val->intval = jz_read_bat(bat_ps);
>  		break;
>  	case POWER_SUPPLY_PROP_VOLTAGE_MAX:
>  	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> -		val->intval = JZ_BAT_MAX_VOLTAGE;
> +		val->intval = pdata->max_voltag;
>  		break;
>  	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> -		val->intval = JZ_BAT_MIN_VOLTAGE;
> +		val->intval = pdata->min_voltag;
>  		break;
>  	case POWER_SUPPLY_PROP_PRESENT:
>  		val->intval = 1;
>  		break;
> -	case POWER_SUPPLY_PROP_TEMP:
> -	case POWER_SUPPLY_PROP_VOL:
> -		val->intval = 0; // reading TEMP and VOL aren't supported
> -		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -168,18 +170,21 @@ static void jz_bat_update(struct power_supply *bat_ps)
>  	unsigned long batt_vol = jz_read_bat(bat_ps);
>  	mutex_lock(&work_lock);
>
> -	if(!__gpio_get_pin(GPIO_CHARG_STAT_N))
> +	if(!gpio_get_value(pdata->charg_stat_pgio))
>  		bat_status = POWER_SUPPLY_STATUS_CHARGING;
> -	else {
> +	else
>  		bat_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> -	}
Again, don't use the gpio if it's not valid.
>
> -	dprintk("--battery status=%s\n", status_text[bat_status]);
> +	dev_dbg(bat_ps->dev, "%s: battery status=%s\n",
> +		__func__, status_text[bat_status]);
> +
>  	if ((old_status != bat_status) ||
>  	    (old_batt_vol - batt_vol > 50000)) {
> -		pr_debug("%s %s -> %s\n", bat_ps->name,
> -				status_text[old_status],
> -				status_text[bat_status]);
> +		dev_dbg(bat_ps->dev, "%s %s -> %s\n",
> +			 bat_ps->name,
> +			 status_text[old_status],
> +			 status_text[bat_status]);
> +
>  		power_supply_changed(bat_ps);
>  	}
>
> @@ -196,8 +201,6 @@ static enum power_supply_property jz_bat_main_props[] = {
>  	POWER_SUPPLY_PROP_VOLTAGE_MAX,
>  	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
>  	POWER_SUPPLY_PROP_PRESENT,
> -	POWER_SUPPLY_PROP_TEMP,
> -	POWER_SUPPLY_PROP_VOL,
>  };
>
>  struct power_supply bat_ps = {
> @@ -212,7 +215,8 @@ struct power_supply bat_ps = {
>
>  static void jz_bat_work(struct work_struct *work)
>  {
> -	const int interval = HZ * 6;
> +	/* query interval too small will increase system workload*/
> +	const int interval = HZ * 30;
>
>  	jz_bat_update(&bat_ps);
>  	queue_delayed_work(monitor_wqueue, &bat_work, interval);
> @@ -242,17 +246,81 @@ static int jz_bat_resume(struct platform_device *dev)
>  static int __devinit jz_bat_probe(struct platform_device *dev)
>  {
>  	int ret = 0;
> +
>  	printk("JZ battery init.\n");
>  	mutex_init(&work_lock);
> -
>  	INIT_DELAYED_WORK(&bat_work, jz_bat_work);
>
> -	__gpio_disable_pull(GPIO_USB_DETE);
> +	if (!dev->dev->platform_data) {
> +		dev_error(&dev->dev, "Please set battery info\n");
> +		return -EINVAL;
> +	}
> +
> +	pdata = dev->dev->platform_data;
>
> -	power_supply_register(&dev->dev, &jz_ac);
> -	power_supply_register(&dev->dev, &jz_usb);
> +	if (gpio_is_valid(pdata->dc_dect_gpio)) {
> +		ret = gpio_request(pdata->dc_dect_gpio, "AC/DC DECT");
> +		if (ret) {
> +			dev_err(dev->dev, "ac/dc dect gpio request failed.\n");
> +
> +			goto err_dc_gpio_request;
> +		}
> +		ret = gpio_direction_input(pdata->dc_dect_gpio);
> +		if (ret) {
> +			dev_err(dev->dev, "ac/dc dect gpio direction failed.\n");
> +
> +			goto err_dc_gpio_direction;
> +		}
> +	}
> +
> +	if (gpio_is_valid(pdata->usb_dect_gpio)) {
> +		ret = gpio_request(pdata->usb_dect_gpio, "USB DECT");
> +		if (ret) {
> +			dev_err(dev->dev, "usb dect gpio request failed.\n");
> +
> +			goto err_usb_gpio_request;
> +		}
> +		ret = gpio_direction_input(pdata->usb_dect_gpio);
> +		if (ret) {
> +			dev_err(dev->dev, "usb dect gpio set direction failed.\n");
> +			goto err_usb_gpio_direction;
> +		}
> +
> +		jz_gpio_disable_pullup(pdata->usb_dect_gpio);
> +		/* TODO: Use generic gpio is better */
> +	}
> +
> +	if (gpio_is_valid(pdata->charg_stat_gpio)) {
> +		ret = gpio_request(pdata->charg_stat_gpio, "CHARG STAT");
> +		if (ret) {
> +			dev_err(dev->dev, "charger state gpio request failed.\n");
> +			goto err_charg_gpio_request;
> +		}
> +		ret = gpio_direction_input(pdata->charg_stat_pgio);
> +		if (ret) {
> +			dev_err(dev->dev, "charger state gpio set direction failed.\n");
> +			goto err_charg_gpio_direction;
> +		}
> +	}
> +
> +	ret = power_supply_register(&dev->dev, &jz_ac);
> +	if (ret) {
> +		dev_err(dev->dev, "power supply ac/dc register failed.\n");
> +		goto err_power_register_ac;
> +	}
> +	
> +	ret = power_supply_register(&dev->dev, &jz_usb);
> +	if (ret) {
> +		dev_err(dev->dev, "power supply usb register failed.\n");
> +		goto err_power_register_usb;
> +	}
>
>  	ret = power_supply_register(&dev->dev, &bat_ps);
> +	if (ret) {
> +		dev_err(dev->dev, "power supply battery register failed.\n");
> +		goto err_power_register_bat;
> +	}
> +	
>  	if (!ret) {
>  		monitor_wqueue = create_singlethread_workqueue("jz_battery");
>  		if (!monitor_wqueue) {
> @@ -262,11 +330,38 @@ static int __devinit jz_bat_probe(struct
> platform_device *dev)
>  	}
>
>  	return ret;
> +
> +err_power_register_bat:
> +	power_supply_unregister(&jz_usb);
> +err_power_register_usb:
> +	power_supply_unregister(&jz_ac);
> +err_power_register_ac:
> +err_charg_gpio_direction:
> +	gpio_free(pdata->charg_stat_pgio);
> +err_charg_gpio_request:
> +err_usb_gpio_direction:
> +	gpio_free(pdata->usb_dect_gpio);
> +err_usb_gpio_request:
> +err_dc_gpio_direction:
> +	gpio_free(pdata->dc_dect_gpio);
> +err_err_dc_gpio_request:
> +	return ret;
>  }
>
>  static int __devexit jz_bat_remove(struct platform_device *dev)
>  {
> +	if (pdata) {
> +	    if (gpio_is_valid(pdata->dc_dct_gpio))
> +		    gpio_free(pdata->dc_dect_gpio);
> +	    if (gpio_is_valid(pdata->usb_dect_gpio))
> +		    gpio_free(pdata->usb_dect_pgio);
> +	    if (gpio_is_valid(pdata->charg_stat_gpio))
> +		    gpio_free(pdata->charg_stat_gpio);
> +	}
> +
>  	power_supply_unregister(&bat_ps);
> +	power_supply_unregister(&jz_ac);
> +	power_supply_unregister(&jz_usb);
>  	return 0;
>  }
>
>   





More information about the discussion mailing list


interactive