Hi, On Monday, December 12, 2016 05:14:45 PM Venkat Reddy Talla wrote: > Adding support to avoid regmap bulk write for the > devices which are not supported register bulk write.
What about register bulk reads done by the driver? Do they also need a fixup? > Max77620 RTC device does not support register bulk write > so disabling regmap bulk write for max77620 rtc device > and enabling only for max77683. > > Signed-off-by: Venkat Reddy Talla <vreddyta...@nvidia.com> > --- > drivers/rtc/rtc-max77686.c | 39 ++++++++++++++++++++++++++++++++++----- > 1 file changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c > index 182fdd0..401ab25 100644 > --- a/drivers/rtc/rtc-max77686.c > +++ b/drivers/rtc/rtc-max77686.c > @@ -84,6 +84,7 @@ struct max77686_rtc_driver_data { > int alarm_pending_status_reg; > /* RTC IRQ CHIP for regmap */ > const struct regmap_irq_chip *rtc_irq_chip; > + bool avoid_rtc_bulk_write; It should be grouped with other bool fields of this struct. Reversing the logic would make it simpler and would make the naming (rtc_bulk_write) consistent with other bool fields in the struct. > }; > > struct max77686_rtc_info { > @@ -197,6 +198,7 @@ static const struct max77686_rtc_driver_data > max77686_drv_data = { > .alarm_pending_status_reg = MAX77686_REG_STATUS2, > .rtc_i2c_addr = MAX77686_I2C_ADDR_RTC, > .rtc_irq_chip = &max77686_rtc_irq_chip, > + .avoid_rtc_bulk_write = false, > }; > > static const struct max77686_rtc_driver_data max77620_drv_data = { > @@ -208,6 +210,7 @@ static const struct max77686_rtc_driver_data > max77620_drv_data = { > .alarm_pending_status_reg = MAX77686_INVALID_REG, > .rtc_i2c_addr = MAX77620_I2C_ADDR_RTC, > .rtc_irq_chip = &max77686_rtc_irq_chip, > + .avoid_rtc_bulk_write = true, > }; > > static const unsigned int max77802_map[REG_RTC_END] = { > @@ -259,6 +262,32 @@ static const struct max77686_rtc_driver_data > max77802_drv_data = { > .rtc_irq_chip = &max77802_rtc_irq_chip, > }; > > +static inline int _regmap_bulk_write(struct max77686_rtc_info *info, rtc_regmap_bulk_write? > + unsigned int reg, void *val, int len) > +{ Please keep arguments (except "info" one) in sync with regmap_bulk_write(): int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val, size_t val_count) > + int ret = 0; > + > + if (!info->drv_data->avoid_rtc_bulk_write) { > + /* RTC registers support sequential writing */ > + ret = regmap_bulk_write(info->rtc_regmap, reg, val, len); > + } else { > + /* Power registers support register-data pair writing */ Hmn, maybe this can be handled be regmap_bulk_write() with proper regmap setting (map->bus == NULL?), can anyone with more regmap expertise comment on this? > + u8 *src = (u8 *)val; > + int i; > + > + for (i = 0; i < len; i++) { > + ret = regmap_write(info->rtc_regmap, reg, *src++); > + if (ret < 0) > + break; > + reg++; > + } > + } > + if (ret < 0) > + dev_err(info->dev, "%s() failed, e %d\n", __func__, ret); Not needed, upper layers already check ret < 0 cases. > + return ret; > +} > + > static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm, > struct max77686_rtc_info *info) > { > @@ -383,7 +412,7 @@ static int max77686_rtc_set_time(struct device *dev, > struct rtc_time *tm) > > mutex_lock(&info->lock); > > - ret = regmap_bulk_write(info->rtc_regmap, > + ret = _regmap_bulk_write(info, > info->drv_data->map[REG_RTC_SEC], > data, ARRAY_SIZE(data)); > if (ret < 0) { > @@ -506,7 +535,7 @@ static int max77686_rtc_stop_alarm(struct > max77686_rtc_info *info) > for (i = 0; i < ARRAY_SIZE(data); i++) > data[i] &= ~ALARM_ENABLE_MASK; > > - ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC], > + ret = _regmap_bulk_write(info, map[REG_ALARM1_SEC], > data, ARRAY_SIZE(data)); > } > > @@ -558,7 +587,7 @@ static int max77686_rtc_start_alarm(struct > max77686_rtc_info *info) > if (data[RTC_DATE] & 0x1f) > data[RTC_DATE] |= (1 << ALARM_ENABLE_SHIFT); > > - ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC], > + ret = _regmap_bulk_write(info, map[REG_ALARM1_SEC], > data, ARRAY_SIZE(data)); > } > > @@ -588,7 +617,7 @@ static int max77686_rtc_set_alarm(struct device *dev, > struct rtc_wkalrm *alrm) > if (ret < 0) > goto out; > > - ret = regmap_bulk_write(info->rtc_regmap, > + ret = _regmap_bulk_write(info, > info->drv_data->map[REG_ALARM1_SEC], > data, ARRAY_SIZE(data)); > > @@ -654,7 +683,7 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info > *info) > > info->rtc_24hr_mode = 1; > > - ret = regmap_bulk_write(info->rtc_regmap, > + ret = _regmap_bulk_write(info, > info->drv_data->map[REG_RTC_CONTROLM], > data, ARRAY_SIZE(data)); > if (ret < 0) { Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics