All of lore.kernel.org
 help / color / mirror / Atom feed
* [rtc-linux] [PATCH] rtc: max77683: avoid regmap bulk write for max77620
@ 2016-12-12 11:44 ` Venkat Reddy Talla
  0 siblings, 0 replies; 6+ 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

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] rtc: max77683: avoid regmap bulk write for max77620
@ 2016-12-12 11:44 ` Venkat Reddy Talla
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

* [rtc-linux] Re: [PATCH] rtc: max77683: avoid regmap bulk write for max77620
  2016-12-12 11:44 ` Venkat Reddy Talla
@ 2016-12-12 13:28   ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 6+ 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

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtc: max77683: avoid regmap bulk write for max77620
@ 2016-12-12 13:28   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 6+ 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] 6+ 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
  -1 siblings, 0 replies; 6+ 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

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [rtc-linux] Re: [PATCH] rtc: max77683: avoid regmap bulk write for max77620
@ 2016-12-12 17:59     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

end of thread, other threads:[~2016-12-12 17:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-12 11:44 [rtc-linux] [PATCH] rtc: max77683: avoid regmap bulk write for max77620 Venkat Reddy Talla
2016-12-12 11:44 ` Venkat Reddy Talla
2016-12-12 13:28 ` [rtc-linux] " Bartlomiej Zolnierkiewicz
2016-12-12 13:28   ` Bartlomiej Zolnierkiewicz
2016-12-12 17:59   ` [rtc-linux] " Krzysztof Kozlowski
2016-12-12 17:59     ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.