Messages in this thread | | | Date | Wed, 17 Dec 2008 17:15:01 +0800 | From | "Eric Miao" <> | Subject | Re: [RFC PATCH 1/2] Add Dialog DA9030 battery charger driver |
| |
On Wed, Dec 17, 2008 at 5:12 PM, Mike Rapoport <mike@compulab.co.il> wrote: > > > 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>
Looks OK.
Acked-by: Eric Miao <eric.miao@marvell.com>
| |