* [PATCH] rtc: max77683: avoid regmap bulk write for max77620
@ 2016-12-12 11:44 Venkat Reddy Talla
2016-12-12 13:28 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 3+ messages in thread
From: Venkat Reddy Talla @ 2016-12-12 11:44 UTC (permalink / raw)
To: cw00.choi, krzk, b.zolnierkie, a.zummo, alexandre.belloni,
linux-kernel, rtc-linux
Cc: ldewangan, vreddytalla
Adding support to avoid regmap bulk write for the
devices which are not supported register bulk write.
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 <vreddytalla@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;
};
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,
+ unsigned int reg, void *val, int len)
+{
+ 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 */
+ 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);
+
+ 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) {
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] rtc: max77683: avoid regmap bulk write for max77620
2016-12-12 11:44 [PATCH] rtc: max77683: avoid regmap bulk write for max77620 Venkat Reddy Talla
@ 2016-12-12 13:28 ` Bartlomiej Zolnierkiewicz
2016-12-12 17:59 ` [rtc-linux] " Krzysztof Kozlowski
0 siblings, 1 reply; 3+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2016-12-12 13:28 UTC (permalink / raw)
To: Venkat Reddy Talla
Cc: cw00.choi, krzk, a.zummo, alexandre.belloni, linux-kernel,
rtc-linux, ldewangan
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 <vreddytalla@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [rtc-linux] Re: [PATCH] rtc: max77683: avoid regmap bulk write for max77620
2016-12-12 13:28 ` Bartlomiej Zolnierkiewicz
@ 2016-12-12 17:59 ` Krzysztof Kozlowski
0 siblings, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-12 17:59 UTC (permalink / raw)
To: rtc-linux
Cc: Venkat Reddy Talla, cw00.choi, krzk, a.zummo, alexandre.belloni,
linux-kernel, ldewangan
On Mon, Dec 12, 2016 at 02:28:11PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > @@ -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?
I think max77686_regmap_bulk_write would be nice. This might still pop
somewhere in the backtrace so the prefix is useful.
>
> > + 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?
Good catch. This does not look like a property of this device driver but
of the bus it is attached to.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-12 17:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-12 11:44 [PATCH] rtc: max77683: avoid regmap bulk write for max77620 Venkat Reddy Talla
2016-12-12 13:28 ` Bartlomiej Zolnierkiewicz
2016-12-12 17:59 ` [rtc-linux] " Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).