Re: [RFC PATCH 1/2] Add Dialog DA9030 battery charger driver

From: Mike Rapoport
Date: Wed Dec 17 2008 - 04:12:32 EST




Eric Miao wrote:
> On Wed, Dec 17, 2008 at 3:56 PM, Mike Rapoport <mike@xxxxxxxxxxxxxx> wrote:
>> This patch amends DA903x MFD driver with definitions and methods needed for
>> battery charger driver.
>>
>
> Patch looks generally good, see some of my concerns below:
>
>> Signed-off-by: Mike Rapoport <mike@xxxxxxxxxxxxxx>
>>
>> drivers/mfd/da903x.c | 16 +++++++++++++++-
>> include/linux/mfd/da903x.h | 44 ++++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
>> index 0b5bd85..2e04b44 100644
>> --- a/drivers/mfd/da903x.c
>> +++ b/drivers/mfd/da903x.c
>> @@ -151,12 +151,24 @@ int da903x_write(struct device *dev, int reg, uint8_t val)
>> }
>> EXPORT_SYMBOL_GPL(da903x_write);
>>
>> +int da903x_writes(struct device *dev, int reg, int len, uint8_t *val)
>> +{
>> + return __da903x_writes(to_i2c_client(dev), reg, len, val);
>> +}
>> +EXPORT_SYMBOL_GPL(da903x_writes);
>> +
>> int da903x_read(struct device *dev, int reg, uint8_t *val)
>> {
>> return __da903x_read(to_i2c_client(dev), reg, val);
>> }
>> EXPORT_SYMBOL_GPL(da903x_read);
>>
>> +int da903x_reads(struct device *dev, int reg, int len, uint8_t *val)
>> +{
>> + return __da903x_reads(to_i2c_client(dev), reg, len, val);
>> +}
>> +EXPORT_SYMBOL_GPL(da903x_reads);
>> +
>> int da903x_set_bits(struct device *dev, int reg, uint8_t bit_mask)
>> {
>> struct da903x_chip *chip = dev_get_drvdata(dev);
>> @@ -254,7 +266,7 @@ static int da9030_unmask_events(struct da903x_chip *chip, unsigned int events)
>> {
>> uint8_t v[3];
>>
>> - chip->events_mask &= ~events;
>> + chip->events_mask |= events;
>>
>
> I wonder if this is true, it may also impact the da9030_mask_events(),
> which is a pair.

This is not true. Thanks for pointing out.

>> v[0] = (chip->events_mask & 0xff);
>> v[1] = (chip->events_mask >> 8) & 0xff;
>> @@ -391,6 +403,8 @@ static void da903x_irq_work(struct work_struct *work)
>> if (chip->ops->read_events(chip, &events))
>> break;
>>
>> +/* pr_info("%s: events=%x events_mask=%x\n", */
>> +/* __func__, events, chip->events_mask); */
>
> I'd prefer to remove this if it is annoying, sorry for this initial garbage
> I sent.

It's rather my own garbage, I'll remove it :)

>> events &= ~chip->events_mask;
>> if (events == 0)
>> break;
>

Signed-off-by: Mike Rapoport <mike@xxxxxxxxxxxxxx>

drivers/mfd/da903x.c | 12 ++++++++++++
include/linux/mfd/da903x.h | 44 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
index 0b5bd85..fcaf1f6 100644
--- a/drivers/mfd/da903x.c
+++ b/drivers/mfd/da903x.c
@@ -151,12 +151,24 @@ int da903x_write(struct device *dev, int reg, uint8_t val)
}
EXPORT_SYMBOL_GPL(da903x_write);

+int da903x_writes(struct device *dev, int reg, int len, uint8_t *val)
+{
+ return __da903x_writes(to_i2c_client(dev), reg, len, val);
+}
+EXPORT_SYMBOL_GPL(da903x_writes);
+
int da903x_read(struct device *dev, int reg, uint8_t *val)
{
return __da903x_read(to_i2c_client(dev), reg, val);
}
EXPORT_SYMBOL_GPL(da903x_read);

+int da903x_reads(struct device *dev, int reg, int len, uint8_t *val)
+{
+ return __da903x_reads(to_i2c_client(dev), reg, len, val);
+}
+EXPORT_SYMBOL_GPL(da903x_reads);
+
int da903x_set_bits(struct device *dev, int reg, uint8_t bit_mask)
{
struct da903x_chip *chip = dev_get_drvdata(dev);
diff --git a/include/linux/mfd/da903x.h b/include/linux/mfd/da903x.h
index cad314c..115dbe9 100644
--- a/include/linux/mfd/da903x.h
+++ b/include/linux/mfd/da903x.h
@@ -32,6 +32,7 @@ enum {
DA9030_ID_LDO18,
DA9030_ID_LDO19,
DA9030_ID_LDO_INT, /* LDO Internal */
+ DA9030_ID_BAT, /* battery charger */

DA9034_ID_LED_1,
DA9034_ID_LED_2,
@@ -93,6 +94,43 @@ struct da9034_touch_pdata {
int y_inverted;
};

+/* DA9030 battery charger data */
+struct power_supply_info;
+
+struct da9030_battery_info {
+ /* battery parameters */
+ struct power_supply_info *battery_info;
+
+ /* current and voltage to use for battery charging */
+ unsigned int charge_milliamp;
+ unsigned int charge_millivolt;
+
+ /* voltage thresholds (in millivolts) */
+ int vbat_low;
+ int vbat_crit;
+ int vbat_charge_start;
+ int vbat_charge_stop;
+ int vbat_charge_restart;
+
+ /* battery nominal minimal and maximal voltages in millivolts */
+ int vcharge_min;
+ int vcharge_max;
+
+ /* Temperature thresholds. These are DA9030 register values
+ "as is" and should be measured for each battery type */
+ int tbat_low;
+ int tbat_high;
+ int tbat_restart;
+
+
+ /* battery monitor interval (seconds) */
+ unsigned int batmon_interval;
+
+ /* platform callbacks for battery low and critical events */
+ void (*battery_low)(void);
+ void (*battery_critical)(void);
+};
+
struct da903x_subdev_info {
int id;
const char *name;
@@ -190,11 +228,13 @@ extern int da903x_unregister_notifier(struct device *dev,
extern int da903x_query_status(struct device *dev, unsigned int status);


-/* NOTE: the two functions below are not intended for use outside
- * of the DA9034 sub-device drivers
+/* NOTE: the functions below are not intended for use outside
+ * of the DA903x sub-device drivers
*/
extern int da903x_write(struct device *dev, int reg, uint8_t val);
+extern int da903x_writes(struct device *dev, int reg, int len, uint8_t *val);
extern int da903x_read(struct device *dev, int reg, uint8_t *val);
+extern int da903x_reads(struct device *dev, int reg, int len, uint8_t *val);
extern int da903x_update(struct device *dev, int reg, uint8_t val, uint8_t mask);
extern int da903x_set_bits(struct device *dev, int reg, uint8_t bit_mask);
extern int da903x_clr_bits(struct device *dev, int reg, uint8_t bit_mask);


--
Sincerely yours,
Mike.

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