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

[Mike Rapoport <mike@compulab.co.il>]

Eric Miao wrote:
> On Wed, Dec 17, 2008 at 3:56 PM, Mike Rapoport <mike@compulab.co.il> 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@compulab.co.il>
>>
>>  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@compulab.co.il>

 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.