| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 17 Dec 2008 22:33:57 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Mike Rapoport <mike@...pulab.co.il>
Cc: LKML <linux-kernel@...r.kernel.org>, sameo@...nedhand.com,
eric miao <eric.miao@...vell.com>, cbou@...l.ru,
David Woodhouse <dwmw2@...radead.org>,
Jonathan Cameron <jic23@....ac.uk>
Subject: Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
On Wed, 17 Dec 2008 09:56:10 +0200 Mike Rapoport <mike@...pulab.co.il> wrote:
> Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC
>
>
>
> Signed-off-by: Mike Rapoport <mike@...pulab.co.il>
>
> drivers/power/Kconfig | 7 +
> drivers/power/Makefile | 1 +
> drivers/power/da9030_battery.c | 592 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 600 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8e0c2b4..db8145c 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -68,4 +68,11 @@ config BATTERY_BQ27x00
> help
> Say Y here to enable support for batteries with BQ27200(I2C) chip.
>
> +config BATTERY_DA903X
> + tristate "DA903x battery driver"
> + depends on PMIC_DA903X
> + help
> + Say Y here to enable support for batteries charger integrated into
> + DA9030/DA9030 PMICs.
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index e8f1ece..58d1b46 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC) += olpc_battery.o
> obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
> obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o
> obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
> +obj-$(CONFIG_BATTERY_DA903X) += da903x_battery.o
> diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
> new file mode 100644
> index 0000000..2dd9fef
> --- /dev/null
> +++ b/drivers/power/da9030_battery.c
> @@ -0,0 +1,592 @@
> +/*
> + * Battery charger driver for Dialog Semiconductor DA9030
> + *
> + * Copyright (C) 2008 Compulab, Ltd.
> + * Mike Rapoport <mike@...pulab.co.il>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/mfd/da903x.h>
> +
> +#ifdef CONFIG_DEBUG_FS
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +#endif
It's not a good ideas to put these includes inside an ifdef. What
happens is that later someones adds some seq_file stuff outside
CONFIG_DEBUG_FS and it all works nicely so it gets merged. Then
someone else disables debugfs and boom.
> +#define DA9030_STATUS_CHDET (1 << 3)
> +
> +#define DA9030_FAULT_LOG 0x0a
> +#define DA9030_FAULT_LOG_OVER_TEMP (1 << 7)
> +#define DA9030_FAULT_LOG_VBAT_OVER (1 << 4)
> +
> +#define DA9030_CHARGE_CONTROL 0x28
> +#define DA9030_CHRG_CHARGER_ENABLE (1 << 7)
> +
> +#define DA9030_ADC_MAN_CONTROL 0x30
> +#define DA9030_ADC_TBATREF_ENABLE (1 << 5)
> +#define DA9030_ADC_LDO_INT_ENABLE (1 << 4)
> +
> +#define DA9030_ADC_AUTO_CONTROL 0x31
> +#define DA9030_ADC_TBAT_ENABLE (1 << 5)
> +#define DA9030_ADC_VBAT_IN_TXON (1 << 4)
> +#define DA9030_ADC_VCH_ENABLE (1 << 3)
> +#define DA9030_ADC_ICH_ENABLE (1 << 2)
> +#define DA9030_ADC_VBAT_ENABLE (1 << 1)
> +#define DA9030_ADC_AUTO_SLEEP_ENABLE (1 << 0)
> +
> +#define DA9030_VBATMON 0x32
> +#define DA9030_VBATMONTXON 0x33
> +#define DA9030_TBATHIGHP 0x34
> +#define DA9030_TBATHIGHN 0x35
> +#define DA9030_TBATLOW 0x36
> +
> +#define DA9030_VBAT_RES 0x41
> +#define DA9030_VBATMIN_RES 0x42
> +#define DA9030_VBATMINTXON_RES 0x43
> +#define DA9030_ICHMAX_RES 0x44
> +#define DA9030_ICHMIN_RES 0x45
> +#define DA9030_ICHAVERAGE_RES 0x46
> +#define DA9030_VCHMAX_RES 0x47
> +#define DA9030_VCHMIN_RES 0x48
> +#define DA9030_TBAT_RES 0x49
> +
> +struct da9030_adc_res {
> + uint8_t vbat_res;
> + uint8_t vbatmin_res;
> + uint8_t vbatmintxon;
> + uint8_t ichmax_res;
> + uint8_t ichmin_res;
> + uint8_t ichaverage_res;
> + uint8_t vchmax_res;
> + uint8_t vchmin_res;
> + uint8_t tbat_res;
> + uint8_t adc_in4_res;
> + uint8_t adc_in5_res;
> +};
hm. Are all the above fields sufficiently obvious as to not need any
description?
> +struct da9030_battery_thresholds {
> + int tbat_low;
> + int tbat_high;
> + int tbat_restart;
> +
> + int vbat_low;
> + int vbat_crit;
> + int vbat_charge_start;
> + int vbat_charge_stop;
> + int vbat_charge_restart;
> +
> + int vcharge_min;
> + int vcharge_max;
> +};
> +
>
> ...
>
> +static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
> +{
> + charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
> + &bat_debug_fops);
> + return charger->debug_file;
> +}
> +
> +static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
> +{
> + debugfs_remove(charger->debug_file);
> +}
> +#else
> +#define da9030_bat_create_debugfs(x) NULL
> +#define da9030_bat_remove_debugfs(x) do {} while (0)
It would be better to make these stubs regular C functions, please.
It's more symmetrical, more pleasing to the eye and provides
typechecking when CONFIG_DEBUG_FS=n.
> +#endif
> +
>
> ...
>
> +MODULE_DESCRIPTION("DA9030 battery charger driver");
> +MODULE_AUTHOR("Mike Rapoport, CompuLab");
> +MODULE_LICENSE("GPL");
A neat looking driver. You deprive me of nits to pick.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists