[PATCH V3 0/5] rtc: max77686: make max77686 rtc driver as IP driver

[PATCH V3 0/5] rtc: max77686: make max77686 rtc driver as IP driver

[Laxman Dewangan]

Based on discussion on patch series of MAX77620 when adding separate
driver for max77620 RTC, it is discussed to reuse the max77686 driver
for all CHips MAX77802, MAX77686 and MAX77620. For this, the rtc-max77686
need to make as IP driver independent of their MFD parent driver.

This series makes the rtc-max77686 as independent driver from its
parent. Required information is passed through the device parent
which is generic and does not depends on any max77686 specific
header ifnromation.

CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>

Changes from V1:
- Added reviewed/tested by tag which we got from V1.
- Remove changes from Kconfig.
- Maintain all register definition in max77686 private header and remove
  the movement to rtc driver.
- Taken care of all comments on V1 from Krzysztof and Javier.

Changes from V2:
- Fix the issue of crash in unbind.

Laxman Dewangan (5):
  rtc: max77686: fix checkpatch error
  rtc: max77686: use rtc regmap to access RTC registers
  rtc: max77686: avoid reference of parent device info multiple places
  mfd: max77686: do not set i2c client data for rtc i2c client
  rtc: max77686: move initialisation of rtc regmap, irq chip locally

 drivers/mfd/max77686.c               |  86 +-------------
 drivers/rtc/rtc-max77686.c           | 214 ++++++++++++++++++++++++++---------
 include/linux/mfd/max77686-private.h |   3 -
 3 files changed, 161 insertions(+), 142 deletions(-)

-- 
2.1.4

[rtc-linux] [PATCH V3 0/5] rtc: max77686: make max77686 rtc driver as IP driver

[Laxman Dewangan]

Based on discussion on patch series of MAX77620 when adding separate
driver for max77620 RTC, it is discussed to reuse the max77686 driver
for all CHips MAX77802, MAX77686 and MAX77620. For this, the rtc-max77686
need to make as IP driver independent of their MFD parent driver.

This series makes the rtc-max77686 as independent driver from its
parent. Required information is passed through the device parent
which is generic and does not depends on any max77686 specific
header ifnromation.

CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>

Changes from V1:
- Added reviewed/tested by tag which we got from V1.
- Remove changes from Kconfig.
- Maintain all register definition in max77686 private header and remove
  the movement to rtc driver.
- Taken care of all comments on V1 from Krzysztof and Javier.

Changes from V2:
- Fix the issue of crash in unbind.

Laxman Dewangan (5):
  rtc: max77686: fix checkpatch error
  rtc: max77686: use rtc regmap to access RTC registers
  rtc: max77686: avoid reference of parent device info multiple places
  mfd: max77686: do not set i2c client data for rtc i2c client
  rtc: max77686: move initialisation of rtc regmap, irq chip locally

 drivers/mfd/max77686.c               |  86 +-------------
 drivers/rtc/rtc-max77686.c           | 214 ++++++++++++++++++++++++++---------
 include/linux/mfd/max77686-private.h |   3 -
 3 files changed, 161 insertions(+), 142 deletions(-)

-- 
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.

[PATCH V3 1/5] rtc: max77686: fix checkpatch error

[Laxman Dewangan]

Fix following check patch error in rtc-max77686 driver:
- Alignment should match open parenthesis.
- braces {} should be used on all arms of this statement.
- Prefer using the BIT macro

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
- None, added reviewed/tested by.

Changes from V1:
- None

 drivers/rtc/rtc-max77686.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 188593d..253cf12 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -24,20 +24,20 @@
 
 /* RTC Control Register */
 #define BCD_EN_SHIFT			0
-#define BCD_EN_MASK			(1 << BCD_EN_SHIFT)
+#define BCD_EN_MASK			BIT(BCD_EN_SHIFT)
 #define MODEL24_SHIFT			1
-#define MODEL24_MASK			(1 << MODEL24_SHIFT)
+#define MODEL24_MASK			BIT(MODEL24_SHIFT)
 /* RTC Update Register1 */
 #define RTC_UDR_SHIFT			0
-#define RTC_UDR_MASK			(1 << RTC_UDR_SHIFT)
+#define RTC_UDR_MASK			BIT(RTC_UDR_SHIFT)
 #define RTC_RBUDR_SHIFT			4
-#define RTC_RBUDR_MASK			(1 << RTC_RBUDR_SHIFT)
+#define RTC_RBUDR_MASK			BIT(RTC_RBUDR_SHIFT)
 /* RTC Hour register */
 #define HOUR_PM_SHIFT			6
-#define HOUR_PM_MASK			(1 << HOUR_PM_SHIFT)
+#define HOUR_PM_MASK			BIT(HOUR_PM_SHIFT)
 /* RTC Alarm Enable */
 #define ALARM_ENABLE_SHIFT		7
-#define ALARM_ENABLE_MASK		(1 << ALARM_ENABLE_SHIFT)
+#define ALARM_ENABLE_MASK		BIT(ALARM_ENABLE_SHIFT)
 
 #define REG_RTC_NONE			0xdeadbeef
 
@@ -205,9 +205,9 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
 
 	tm->tm_sec = data[RTC_SEC] & mask;
 	tm->tm_min = data[RTC_MIN] & mask;
-	if (info->rtc_24hr_mode)
+	if (info->rtc_24hr_mode) {
 		tm->tm_hour = data[RTC_HOUR] & 0x1f;
-	else {
+	} else {
 		tm->tm_hour = data[RTC_HOUR] & 0x0f;
 		if (data[RTC_HOUR] & HOUR_PM_MASK)
 			tm->tm_hour += 12;
@@ -256,7 +256,7 @@ static int max77686_rtc_tm_to_data(struct rtc_time *tm, u8 *data,
 }
 
 static int max77686_rtc_update(struct max77686_rtc_info *info,
-	enum MAX77686_RTC_OP op)
+			       enum MAX77686_RTC_OP op)
 {
 	int ret;
 	unsigned int data;
@@ -547,7 +547,7 @@ out:
 }
 
 static int max77686_rtc_alarm_irq_enable(struct device *dev,
-					unsigned int enabled)
+					 unsigned int enabled)
 {
 	struct max77686_rtc_info *info = dev_get_drvdata(dev);
 	int ret;
@@ -612,7 +612,7 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 	int ret;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(struct max77686_rtc_info),
-				GFP_KERNEL);
+			    GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -662,7 +662,8 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, info->virq, NULL,
-				max77686_rtc_alarm_irq, 0, "rtc-alarm1", info);
+					max77686_rtc_alarm_irq, 0,
+					"rtc-alarm1", info);
 	if (ret < 0)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
-- 
2.1.4

[rtc-linux] [PATCH V3 1/5] rtc: max77686: fix checkpatch error

[Laxman Dewangan]

Fix following check patch error in rtc-max77686 driver:
- Alignment should match open parenthesis.
- braces {} should be used on all arms of this statement.
- Prefer using the BIT macro

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
- None, added reviewed/tested by.

Changes from V1:
- None

 drivers/rtc/rtc-max77686.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 188593d..253cf12 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -24,20 +24,20 @@
 
 /* RTC Control Register */
 #define BCD_EN_SHIFT			0
-#define BCD_EN_MASK			(1 << BCD_EN_SHIFT)
+#define BCD_EN_MASK			BIT(BCD_EN_SHIFT)
 #define MODEL24_SHIFT			1
-#define MODEL24_MASK			(1 << MODEL24_SHIFT)
+#define MODEL24_MASK			BIT(MODEL24_SHIFT)
 /* RTC Update Register1 */
 #define RTC_UDR_SHIFT			0
-#define RTC_UDR_MASK			(1 << RTC_UDR_SHIFT)
+#define RTC_UDR_MASK			BIT(RTC_UDR_SHIFT)
 #define RTC_RBUDR_SHIFT			4
-#define RTC_RBUDR_MASK			(1 << RTC_RBUDR_SHIFT)
+#define RTC_RBUDR_MASK			BIT(RTC_RBUDR_SHIFT)
 /* RTC Hour register */
 #define HOUR_PM_SHIFT			6
-#define HOUR_PM_MASK			(1 << HOUR_PM_SHIFT)
+#define HOUR_PM_MASK			BIT(HOUR_PM_SHIFT)
 /* RTC Alarm Enable */
 #define ALARM_ENABLE_SHIFT		7
-#define ALARM_ENABLE_MASK		(1 << ALARM_ENABLE_SHIFT)
+#define ALARM_ENABLE_MASK		BIT(ALARM_ENABLE_SHIFT)
 
 #define REG_RTC_NONE			0xdeadbeef
 
@@ -205,9 +205,9 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
 
 	tm->tm_sec = data[RTC_SEC] & mask;
 	tm->tm_min = data[RTC_MIN] & mask;
-	if (info->rtc_24hr_mode)
+	if (info->rtc_24hr_mode) {
 		tm->tm_hour = data[RTC_HOUR] & 0x1f;
-	else {
+	} else {
 		tm->tm_hour = data[RTC_HOUR] & 0x0f;
 		if (data[RTC_HOUR] & HOUR_PM_MASK)
 			tm->tm_hour += 12;
@@ -256,7 +256,7 @@ static int max77686_rtc_tm_to_data(struct rtc_time *tm, u8 *data,
 }
 
 static int max77686_rtc_update(struct max77686_rtc_info *info,
-	enum MAX77686_RTC_OP op)
+			       enum MAX77686_RTC_OP op)
 {
 	int ret;
 	unsigned int data;
@@ -547,7 +547,7 @@ out:
 }
 
 static int max77686_rtc_alarm_irq_enable(struct device *dev,
-					unsigned int enabled)
+					 unsigned int enabled)
 {
 	struct max77686_rtc_info *info = dev_get_drvdata(dev);
 	int ret;
@@ -612,7 +612,7 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 	int ret;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(struct max77686_rtc_info),
-				GFP_KERNEL);
+			    GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -662,7 +662,8 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, info->virq, NULL,
-				max77686_rtc_alarm_irq, 0, "rtc-alarm1", info);
+					max77686_rtc_alarm_irq, 0,
+					"rtc-alarm1", info);
 	if (ret < 0)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
-- 
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.

[PATCH V3 2/5] rtc: max77686: use rtc regmap to access RTC registers

[Laxman Dewangan]

rtc_regmap should be used to access all RTC registers instead
of parent regmap regardless of what chip or property have it.

This makes the register access uniform and extendible for other
chips.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
None, added reviewed/tested by.

Changes from V1:
None

 drivers/rtc/rtc-max77686.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 253cf12..11f74ed 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -370,7 +370,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			goto out;
 		}
 
-		ret = regmap_read(info->max77686->regmap,
+		ret = regmap_read(info->max77686->rtc_regmap,
 				  map[REG_RTC_AE1], &val);
 		if (ret < 0) {
 			dev_err(info->dev,
@@ -426,7 +426,8 @@ static int max77686_rtc_stop_alarm(struct max77686_rtc_info *info)
 			goto out;
 		}
 
-		ret = regmap_write(info->max77686->regmap, map[REG_RTC_AE1], 0);
+		ret = regmap_write(info->max77686->rtc_regmap,
+				   map[REG_RTC_AE1], 0);
 	} else {
 		ret = regmap_bulk_read(info->max77686->rtc_regmap,
 				       map[REG_ALARM1_SEC], data,
@@ -471,7 +472,7 @@ static int max77686_rtc_start_alarm(struct max77686_rtc_info *info)
 		goto out;
 
 	if (info->drv_data->alarm_enable_reg) {
-		ret = regmap_write(info->max77686->regmap, map[REG_RTC_AE1],
+		ret = regmap_write(info->max77686->rtc_regmap, map[REG_RTC_AE1],
 				   MAX77802_ALARM_ENABLE_VALUE);
 	} else {
 		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-- 
2.1.4

[rtc-linux] [PATCH V3 2/5] rtc: max77686: use rtc regmap to access RTC registers

[Laxman Dewangan]

rtc_regmap should be used to access all RTC registers instead
of parent regmap regardless of what chip or property have it.

This makes the register access uniform and extendible for other
chips.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
None, added reviewed/tested by.

Changes from V1:
None

 drivers/rtc/rtc-max77686.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 253cf12..11f74ed 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -370,7 +370,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			goto out;
 		}
 
-		ret = regmap_read(info->max77686->regmap,
+		ret = regmap_read(info->max77686->rtc_regmap,
 				  map[REG_RTC_AE1], &val);
 		if (ret < 0) {
 			dev_err(info->dev,
@@ -426,7 +426,8 @@ static int max77686_rtc_stop_alarm(struct max77686_rtc_info *info)
 			goto out;
 		}
 
-		ret = regmap_write(info->max77686->regmap, map[REG_RTC_AE1], 0);
+		ret = regmap_write(info->max77686->rtc_regmap,
+				   map[REG_RTC_AE1], 0);
 	} else {
 		ret = regmap_bulk_read(info->max77686->rtc_regmap,
 				       map[REG_ALARM1_SEC], data,
@@ -471,7 +472,7 @@ static int max77686_rtc_start_alarm(struct max77686_rtc_info *info)
 		goto out;
 
 	if (info->drv_data->alarm_enable_reg) {
-		ret = regmap_write(info->max77686->regmap, map[REG_RTC_AE1],
+		ret = regmap_write(info->max77686->rtc_regmap, map[REG_RTC_AE1],
 				   MAX77802_ALARM_ENABLE_VALUE);
 	} else {
 		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-- 
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.

[PATCH V3 3/5] rtc: max77686: avoid reference of parent device info multiple places

[Laxman Dewangan]

Get rid of referring parent device info for register access
all the places by making regmap as part of max77686 rtc
device info. This will also remove the need of storing parent
device info in max77686 rtc device info as this is no more required.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
None, added reviewed/tested by.

Changes form V2:
- Fix typo in description.

 drivers/rtc/rtc-max77686.c | 52 ++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 11f74ed..ab1f2cd 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -74,12 +74,12 @@ struct max77686_rtc_driver_data {
 
 struct max77686_rtc_info {
 	struct device		*dev;
-	struct max77686_dev	*max77686;
 	struct i2c_client	*rtc;
 	struct rtc_device	*rtc_dev;
 	struct mutex		lock;
 
 	struct regmap		*regmap;
+	struct regmap		*rtc_regmap;
 
 	const struct max77686_rtc_driver_data *drv_data;
 
@@ -267,7 +267,7 @@ static int max77686_rtc_update(struct max77686_rtc_info *info,
 	else
 		data = 1 << RTC_RBUDR_SHIFT;
 
-	ret = regmap_update_bits(info->max77686->rtc_regmap,
+	ret = regmap_update_bits(info->rtc_regmap,
 				 info->drv_data->map[REG_RTC_UPDATE0],
 				 data, data);
 	if (ret < 0)
@@ -293,7 +293,7 @@ static int max77686_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_bulk_read(info->max77686->rtc_regmap,
+	ret = regmap_bulk_read(info->rtc_regmap,
 			       info->drv_data->map[REG_RTC_SEC],
 			       data, ARRAY_SIZE(data));
 	if (ret < 0) {
@@ -322,7 +322,7 @@ static int max77686_rtc_set_time(struct device *dev, struct rtc_time *tm)
 
 	mutex_lock(&info->lock);
 
-	ret = regmap_bulk_write(info->max77686->rtc_regmap,
+	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_RTC_SEC],
 				data, ARRAY_SIZE(data));
 	if (ret < 0) {
@@ -351,8 +351,8 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_bulk_read(info->max77686->rtc_regmap,
-			       map[REG_ALARM1_SEC], data, ARRAY_SIZE(data));
+	ret = regmap_bulk_read(info->rtc_regmap, map[REG_ALARM1_SEC],
+			       data, ARRAY_SIZE(data));
 	if (ret < 0) {
 		dev_err(info->dev, "Fail to read alarm reg(%d)\n", ret);
 		goto out;
@@ -370,8 +370,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			goto out;
 		}
 
-		ret = regmap_read(info->max77686->rtc_regmap,
-				  map[REG_RTC_AE1], &val);
+		ret = regmap_read(info->rtc_regmap, map[REG_RTC_AE1], &val);
 		if (ret < 0) {
 			dev_err(info->dev,
 				"fail to read alarm enable(%d)\n", ret);
@@ -390,7 +389,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	}
 
 	alrm->pending = 0;
-	ret = regmap_read(info->max77686->regmap, MAX77686_REG_STATUS2, &val);
+	ret = regmap_read(info->regmap, MAX77686_REG_STATUS2, &val);
 	if (ret < 0) {
 		dev_err(info->dev, "Fail to read status2 reg(%d)\n", ret);
 		goto out;
@@ -426,12 +425,10 @@ static int max77686_rtc_stop_alarm(struct max77686_rtc_info *info)
 			goto out;
 		}
 
-		ret = regmap_write(info->max77686->rtc_regmap,
-				   map[REG_RTC_AE1], 0);
+		ret = regmap_write(info->rtc_regmap, map[REG_RTC_AE1], 0);
 	} else {
-		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-				       map[REG_ALARM1_SEC], data,
-				       ARRAY_SIZE(data));
+		ret = regmap_bulk_read(info->rtc_regmap, map[REG_ALARM1_SEC],
+				       data, ARRAY_SIZE(data));
 		if (ret < 0) {
 			dev_err(info->dev, "Fail to read alarm reg(%d)\n", ret);
 			goto out;
@@ -442,9 +439,8 @@ 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->max77686->rtc_regmap,
-					map[REG_ALARM1_SEC], data,
-					ARRAY_SIZE(data));
+		ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC],
+					data, ARRAY_SIZE(data));
 	}
 
 	if (ret < 0) {
@@ -472,12 +468,11 @@ static int max77686_rtc_start_alarm(struct max77686_rtc_info *info)
 		goto out;
 
 	if (info->drv_data->alarm_enable_reg) {
-		ret = regmap_write(info->max77686->rtc_regmap, map[REG_RTC_AE1],
+		ret = regmap_write(info->rtc_regmap, map[REG_RTC_AE1],
 				   MAX77802_ALARM_ENABLE_VALUE);
 	} else {
-		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-				       map[REG_ALARM1_SEC], data,
-				       ARRAY_SIZE(data));
+		ret = regmap_bulk_read(info->rtc_regmap, map[REG_ALARM1_SEC],
+				       data, ARRAY_SIZE(data));
 		if (ret < 0) {
 			dev_err(info->dev, "Fail to read alarm reg(%d)\n", ret);
 			goto out;
@@ -496,9 +491,8 @@ 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->max77686->rtc_regmap,
-					map[REG_ALARM1_SEC], data,
-					ARRAY_SIZE(data));
+		ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC],
+					data, ARRAY_SIZE(data));
 	}
 
 	if (ret < 0) {
@@ -527,7 +521,7 @@ static int max77686_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_bulk_write(info->max77686->rtc_regmap,
+	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_ALARM1_SEC],
 				data, ARRAY_SIZE(data));
 
@@ -593,7 +587,7 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
 
 	info->rtc_24hr_mode = 1;
 
-	ret = regmap_bulk_write(info->max77686->rtc_regmap,
+	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_RTC_CONTROLM],
 				data, ARRAY_SIZE(data));
 	if (ret < 0) {
@@ -619,13 +613,13 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 
 	mutex_init(&info->lock);
 	info->dev = &pdev->dev;
-	info->max77686 = max77686;
 	info->rtc = max77686->rtc;
 	info->drv_data = (const struct max77686_rtc_driver_data *)
 		id->driver_data;
 
-	if (!info->drv_data->separate_i2c_addr)
-		info->max77686->rtc_regmap = info->max77686->regmap;
+	info->regmap = max77686->regmap;
+	info->rtc_regmap = (info->drv_data->separate_i2c_addr) ?
+			    max77686->rtc_regmap : info->regmap;
 
 	platform_set_drvdata(pdev, info);
 
-- 
2.1.4

[rtc-linux] [PATCH V3 3/5] rtc: max77686: avoid reference of parent device info multiple places

[Laxman Dewangan]

Get rid of referring parent device info for register access
all the places by making regmap as part of max77686 rtc
device info. This will also remove the need of storing parent
device info in max77686 rtc device info as this is no more required.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
None, added reviewed/tested by.

Changes form V2:
- Fix typo in description.

 drivers/rtc/rtc-max77686.c | 52 ++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 11f74ed..ab1f2cd 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -74,12 +74,12 @@ struct max77686_rtc_driver_data {
 
 struct max77686_rtc_info {
 	struct device		*dev;
-	struct max77686_dev	*max77686;
 	struct i2c_client	*rtc;
 	struct rtc_device	*rtc_dev;
 	struct mutex		lock;
 
 	struct regmap		*regmap;
+	struct regmap		*rtc_regmap;
 
 	const struct max77686_rtc_driver_data *drv_data;
 
@@ -267,7 +267,7 @@ static int max77686_rtc_update(struct max77686_rtc_info *info,
 	else
 		data = 1 << RTC_RBUDR_SHIFT;
 
-	ret = regmap_update_bits(info->max77686->rtc_regmap,
+	ret = regmap_update_bits(info->rtc_regmap,
 				 info->drv_data->map[REG_RTC_UPDATE0],
 				 data, data);
 	if (ret < 0)
@@ -293,7 +293,7 @@ static int max77686_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_bulk_read(info->max77686->rtc_regmap,
+	ret = regmap_bulk_read(info->rtc_regmap,
 			       info->drv_data->map[REG_RTC_SEC],
 			       data, ARRAY_SIZE(data));
 	if (ret < 0) {
@@ -322,7 +322,7 @@ static int max77686_rtc_set_time(struct device *dev, struct rtc_time *tm)
 
 	mutex_lock(&info->lock);
 
-	ret = regmap_bulk_write(info->max77686->rtc_regmap,
+	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_RTC_SEC],
 				data, ARRAY_SIZE(data));
 	if (ret < 0) {
@@ -351,8 +351,8 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_bulk_read(info->max77686->rtc_regmap,
-			       map[REG_ALARM1_SEC], data, ARRAY_SIZE(data));
+	ret = regmap_bulk_read(info->rtc_regmap, map[REG_ALARM1_SEC],
+			       data, ARRAY_SIZE(data));
 	if (ret < 0) {
 		dev_err(info->dev, "Fail to read alarm reg(%d)\n", ret);
 		goto out;
@@ -370,8 +370,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			goto out;
 		}
 
-		ret = regmap_read(info->max77686->rtc_regmap,
-				  map[REG_RTC_AE1], &val);
+		ret = regmap_read(info->rtc_regmap, map[REG_RTC_AE1], &val);
 		if (ret < 0) {
 			dev_err(info->dev,
 				"fail to read alarm enable(%d)\n", ret);
@@ -390,7 +389,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	}
 
 	alrm->pending = 0;
-	ret = regmap_read(info->max77686->regmap, MAX77686_REG_STATUS2, &val);
+	ret = regmap_read(info->regmap, MAX77686_REG_STATUS2, &val);
 	if (ret < 0) {
 		dev_err(info->dev, "Fail to read status2 reg(%d)\n", ret);
 		goto out;
@@ -426,12 +425,10 @@ static int max77686_rtc_stop_alarm(struct max77686_rtc_info *info)
 			goto out;
 		}
 
-		ret = regmap_write(info->max77686->rtc_regmap,
-				   map[REG_RTC_AE1], 0);
+		ret = regmap_write(info->rtc_regmap, map[REG_RTC_AE1], 0);
 	} else {
-		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-				       map[REG_ALARM1_SEC], data,
-				       ARRAY_SIZE(data));
+		ret = regmap_bulk_read(info->rtc_regmap, map[REG_ALARM1_SEC],
+				       data, ARRAY_SIZE(data));
 		if (ret < 0) {
 			dev_err(info->dev, "Fail to read alarm reg(%d)\n", ret);
 			goto out;
@@ -442,9 +439,8 @@ 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->max77686->rtc_regmap,
-					map[REG_ALARM1_SEC], data,
-					ARRAY_SIZE(data));
+		ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC],
+					data, ARRAY_SIZE(data));
 	}
 
 	if (ret < 0) {
@@ -472,12 +468,11 @@ static int max77686_rtc_start_alarm(struct max77686_rtc_info *info)
 		goto out;
 
 	if (info->drv_data->alarm_enable_reg) {
-		ret = regmap_write(info->max77686->rtc_regmap, map[REG_RTC_AE1],
+		ret = regmap_write(info->rtc_regmap, map[REG_RTC_AE1],
 				   MAX77802_ALARM_ENABLE_VALUE);
 	} else {
-		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-				       map[REG_ALARM1_SEC], data,
-				       ARRAY_SIZE(data));
+		ret = regmap_bulk_read(info->rtc_regmap, map[REG_ALARM1_SEC],
+				       data, ARRAY_SIZE(data));
 		if (ret < 0) {
 			dev_err(info->dev, "Fail to read alarm reg(%d)\n", ret);
 			goto out;
@@ -496,9 +491,8 @@ 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->max77686->rtc_regmap,
-					map[REG_ALARM1_SEC], data,
-					ARRAY_SIZE(data));
+		ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC],
+					data, ARRAY_SIZE(data));
 	}
 
 	if (ret < 0) {
@@ -527,7 +521,7 @@ static int max77686_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_bulk_write(info->max77686->rtc_regmap,
+	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_ALARM1_SEC],
 				data, ARRAY_SIZE(data));
 
@@ -593,7 +587,7 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
 
 	info->rtc_24hr_mode = 1;
 
-	ret = regmap_bulk_write(info->max77686->rtc_regmap,
+	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_RTC_CONTROLM],
 				data, ARRAY_SIZE(data));
 	if (ret < 0) {
@@ -619,13 +613,13 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 
 	mutex_init(&info->lock);
 	info->dev = &pdev->dev;
-	info->max77686 = max77686;
 	info->rtc = max77686->rtc;
 	info->drv_data = (const struct max77686_rtc_driver_data *)
 		id->driver_data;
 
-	if (!info->drv_data->separate_i2c_addr)
-		info->max77686->rtc_regmap = info->max77686->regmap;
+	info->regmap = max77686->regmap;
+	info->rtc_regmap = (info->drv_data->separate_i2c_addr) ?
+			    max77686->rtc_regmap : info->regmap;
 
 	platform_set_drvdata(pdev, info);
 
-- 
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.

[PATCH V3 4/5] mfd: max77686: do not set i2c client data for rtc i2c client

[Laxman Dewangan]

There is different RTC I2C address for RTC block in MAX77686.
Driver is creating dummy i2c client for this address to access
the register of this IP block.

As there is no call to i2c_get_clientdata() for rtc_i2c client,
there is no need to store pointer and hence removing the call
to set client data for rtc i2c client.

Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
This is new in this series based on review comment from V1.

Changes from V2:
- Rephrase description.
- Added reviewed by.

 drivers/mfd/max77686.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index d959ebb..bda6fd7 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -277,7 +277,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 				"Failed to allocate I2C device for RTC\n");
 			return -ENODEV;
 		}
-		i2c_set_clientdata(max77686->rtc, max77686);
 
 		max77686->rtc_regmap =
 			devm_regmap_init_i2c(max77686->rtc,
-- 
2.1.4

[rtc-linux] [PATCH V3 4/5] mfd: max77686: do not set i2c client data for rtc i2c client

[Laxman Dewangan]

There is different RTC I2C address for RTC block in MAX77686.
Driver is creating dummy i2c client for this address to access
the register of this IP block.

As there is no call to i2c_get_clientdata() for rtc_i2c client,
there is no need to store pointer and hence removing the call
to set client data for rtc i2c client.

Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
This is new in this series based on review comment from V1.

Changes from V2:
- Rephrase description.
- Added reviewed by.

 drivers/mfd/max77686.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index d959ebb..bda6fd7 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -277,7 +277,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 				"Failed to allocate I2C device for RTC\n");
 			return -ENODEV;
 		}
-		i2c_set_clientdata(max77686->rtc, max77686);
 
 		max77686->rtc_regmap =
 			devm_regmap_init_i2c(max77686->rtc,
-- 
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.

[PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

To make RTC block of MAX77686/MAX77802 as independent driver,
move the registration of i2c device, regmap for register access
and irq_chip for interrupt support inside the RTC driver.
Removed the same initialisation from MFD driver.

Having this change will allow to reuse this driver for different
PMIC/devices from Maxim Semiconductor if they kept same RTC IP on
different PMIC. Some of examples as PMIC MAX77620, MAX20024 where
same RTC IP used and hence driver for these chips will use this
driver only for RTC support.

Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
- Remove changes from Kconfig.
- Maintain all register definition in max77686 private header and remove
  the movement to rtc driver.
- Taken care of all comments on V1 from Krzysztof and Javier.

Changes from V2:
- Taken care of missed sequence for removing the resource.
- Fix the crash when doing unbind by using requested_threaded_irq()
  instead of demv_requested_threaded_irq().

 drivers/mfd/max77686.c               |  85 +-------------------
 drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
 include/linux/mfd/max77686-private.h |   3 -
 3 files changed, 130 insertions(+), 106 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index bda6fd7..98ecd13 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -35,8 +35,6 @@
 #include <linux/err.h>
 #include <linux/of.h>
 
-#define I2C_ADDR_RTC	(0x0C >> 1)
-
 static const struct mfd_cell max77686_devs[] = {
 	{ .name = "max77686-pmic", },
 	{ .name = "max77686-rtc", },
@@ -116,11 +114,6 @@ static const struct regmap_config max77686_regmap_config = {
 	.val_bits = 8,
 };
 
-static const struct regmap_config max77686_rtc_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
-};
-
 static const struct regmap_config max77802_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -156,25 +149,6 @@ static const struct regmap_irq_chip max77686_irq_chip = {
 	.num_irqs		= ARRAY_SIZE(max77686_irqs),
 };
 
-static const struct regmap_irq max77686_rtc_irqs[] = {
-	/* RTC interrupts */
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC60S_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA1_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA2_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_SMPL_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC1S_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_WTSR_MSK, },
-};
-
-static const struct regmap_irq_chip max77686_rtc_irq_chip = {
-	.name			= "max77686-rtc",
-	.status_base		= MAX77686_RTC_INT,
-	.mask_base		= MAX77686_RTC_INTM,
-	.num_regs		= 1,
-	.irqs			= max77686_rtc_irqs,
-	.num_irqs		= ARRAY_SIZE(max77686_rtc_irqs),
-};
-
 static const struct regmap_irq_chip max77802_irq_chip = {
 	.name			= "max77802-pmic",
 	.status_base		= MAX77802_REG_INT1,
@@ -184,15 +158,6 @@ static const struct regmap_irq_chip max77802_irq_chip = {
 	.num_irqs		= ARRAY_SIZE(max77686_irqs),
 };
 
-static const struct regmap_irq_chip max77802_rtc_irq_chip = {
-	.name			= "max77802-rtc",
-	.status_base		= MAX77802_RTC_INT,
-	.mask_base		= MAX77802_RTC_INTM,
-	.num_regs		= 1,
-	.irqs			= max77686_rtc_irqs, /* same masks as 77686 */
-	.num_irqs		= ARRAY_SIZE(max77686_rtc_irqs),
-};
-
 static const struct of_device_id max77686_pmic_dt_match[] = {
 	{
 		.compatible = "maxim,max77686",
@@ -214,8 +179,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	int ret = 0;
 	const struct regmap_config *config;
 	const struct regmap_irq_chip *irq_chip;
-	const struct regmap_irq_chip *rtc_irq_chip;
-	struct regmap **rtc_regmap;
 	const struct mfd_cell *cells;
 	int n_devs;
 
@@ -242,15 +205,11 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	if (max77686->type == TYPE_MAX77686) {
 		config = &max77686_regmap_config;
 		irq_chip = &max77686_irq_chip;
-		rtc_irq_chip = &max77686_rtc_irq_chip;
-		rtc_regmap = &max77686->rtc_regmap;
 		cells =  max77686_devs;
 		n_devs = ARRAY_SIZE(max77686_devs);
 	} else {
 		config = &max77802_regmap_config;
 		irq_chip = &max77802_irq_chip;
-		rtc_irq_chip = &max77802_rtc_irq_chip;
-		rtc_regmap = &max77686->regmap;
 		cells =  max77802_devs;
 		n_devs = ARRAY_SIZE(max77802_devs);
 	}
@@ -270,59 +229,25 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 		return -ENODEV;
 	}
 
-	if (max77686->type == TYPE_MAX77686) {
-		max77686->rtc = i2c_new_dummy(i2c->adapter, I2C_ADDR_RTC);
-		if (!max77686->rtc) {
-			dev_err(max77686->dev,
-				"Failed to allocate I2C device for RTC\n");
-			return -ENODEV;
-		}
-
-		max77686->rtc_regmap =
-			devm_regmap_init_i2c(max77686->rtc,
-					     &max77686_rtc_regmap_config);
-		if (IS_ERR(max77686->rtc_regmap)) {
-			ret = PTR_ERR(max77686->rtc_regmap);
-			dev_err(max77686->dev,
-				"failed to allocate RTC regmap: %d\n",
-				ret);
-			goto err_unregister_i2c;
-		}
-	}
-
 	ret = regmap_add_irq_chip(max77686->regmap, max77686->irq,
 				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
 				  IRQF_SHARED, 0, irq_chip,
 				  &max77686->irq_data);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&i2c->dev, "failed to add PMIC irq chip: %d\n", ret);
-		goto err_unregister_i2c;
-	}
-
-	ret = regmap_add_irq_chip(*rtc_regmap, max77686->irq,
-				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
-				  IRQF_SHARED, 0, rtc_irq_chip,
-				  &max77686->rtc_irq_data);
-	if (ret) {
-		dev_err(&i2c->dev, "failed to add RTC irq chip: %d\n", ret);
-		goto err_del_irqc;
+		return ret;
 	}
 
 	ret = mfd_add_devices(max77686->dev, -1, cells, n_devs, NULL, 0, NULL);
 	if (ret < 0) {
 		dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
-		goto err_del_rtc_irqc;
+		goto err_del_irqc;
 	}
 
 	return 0;
 
-err_del_rtc_irqc:
-	regmap_del_irq_chip(max77686->irq, max77686->rtc_irq_data);
 err_del_irqc:
 	regmap_del_irq_chip(max77686->irq, max77686->irq_data);
-err_unregister_i2c:
-	if (max77686->type == TYPE_MAX77686)
-		i2c_unregister_device(max77686->rtc);
 
 	return ret;
 }
@@ -333,12 +258,8 @@ static int max77686_i2c_remove(struct i2c_client *i2c)
 
 	mfd_remove_devices(max77686->dev);
 
-	regmap_del_irq_chip(max77686->irq, max77686->rtc_irq_data);
 	regmap_del_irq_chip(max77686->irq, max77686->irq_data);
 
-	if (max77686->type == TYPE_MAX77686)
-		i2c_unregister_device(max77686->rtc);
-
 	return 0;
 }
 
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index ab1f2cd..45d6396 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/rtc.h>
 #include <linux/delay.h>
@@ -22,6 +23,9 @@
 #include <linux/irqdomain.h>
 #include <linux/regmap.h>
 
+#define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
+#define MAX77686_INVALID_I2C_ADDR	(-1)
+
 /* RTC Control Register */
 #define BCD_EN_SHIFT			0
 #define BCD_EN_MASK			BIT(BCD_EN_SHIFT)
@@ -68,8 +72,10 @@ struct max77686_rtc_driver_data {
 	const unsigned int	*map;
 	/* Has a separate alarm enable register? */
 	bool			alarm_enable_reg;
-	/* Has a separate I2C regmap for the RTC? */
-	bool			separate_i2c_addr;
+	/* I2C address for RTC block */
+	int			rtc_i2c_addr;
+	/* RTC IRQ CHIP for regmap */
+	const struct regmap_irq_chip *rtc_irq_chip;
 };
 
 struct max77686_rtc_info {
@@ -82,7 +88,9 @@ struct max77686_rtc_info {
 	struct regmap		*rtc_regmap;
 
 	const struct max77686_rtc_driver_data *drv_data;
+	struct regmap_irq_chip_data *rtc_irq_data;
 
+	int rtc_irq;
 	int virq;
 	int rtc_24hr_mode;
 };
@@ -153,12 +161,32 @@ static const unsigned int max77686_map[REG_RTC_END] = {
 	[REG_RTC_AE1]	     = REG_RTC_NONE,
 };
 
+static const struct regmap_irq max77686_rtc_irqs[] = {
+	/* RTC interrupts */
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC60S_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA1_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA2_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_SMPL_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC1S_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_WTSR_MSK, },
+};
+
+static const struct regmap_irq_chip max77686_rtc_irq_chip = {
+	.name		= "max77686-rtc",
+	.status_base	= MAX77686_RTC_INT,
+	.mask_base	= MAX77686_RTC_INTM,
+	.num_regs	= 1,
+	.irqs		= max77686_rtc_irqs,
+	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs),
+};
+
 static const struct max77686_rtc_driver_data max77686_drv_data = {
 	.delay = 16000,
 	.mask  = 0x7f,
 	.map   = max77686_map,
 	.alarm_enable_reg  = false,
-	.separate_i2c_addr = true,
+	.rtc_i2c_addr = MAX77686_I2C_ADDR_RTC,
+	.rtc_irq_chip = &max77686_rtc_irq_chip,
 };
 
 static const unsigned int max77802_map[REG_RTC_END] = {
@@ -190,12 +218,22 @@ static const unsigned int max77802_map[REG_RTC_END] = {
 	[REG_RTC_AE1]	     = MAX77802_RTC_AE1,
 };
 
+static const struct regmap_irq_chip max77802_rtc_irq_chip = {
+	.name		= "max77802-rtc",
+	.status_base	= MAX77802_RTC_INT,
+	.mask_base	= MAX77802_RTC_INTM,
+	.num_regs	= 1,
+	.irqs		= max77686_rtc_irqs, /* same masks as 77686 */
+	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs),
+};
+
 static const struct max77686_rtc_driver_data max77802_drv_data = {
 	.delay = 200,
 	.mask  = 0xff,
 	.map   = max77802_map,
 	.alarm_enable_reg  = true,
-	.separate_i2c_addr = false,
+	.rtc_i2c_addr = MAX77686_INVALID_I2C_ADDR,
+	.rtc_irq_chip = &max77802_rtc_irq_chip,
 };
 
 static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
@@ -599,9 +637,65 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
 	return ret;
 }
 
+static const struct regmap_config max77686_rtc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
+{
+	struct device *parent = info->dev->parent;
+	struct i2c_client *parent_i2c = to_i2c_client(parent);
+	int ret;
+
+	info->rtc_irq = parent_i2c->irq;
+
+	info->regmap = dev_get_regmap(parent, NULL);
+	if (!info->regmap) {
+		dev_err(info->dev, "Failed to get rtc regmap\n");
+		return -ENODEV;
+	}
+
+	if (info->drv_data->rtc_i2c_addr == MAX77686_INVALID_I2C_ADDR) {
+		info->rtc_regmap = info->regmap;
+		goto add_rtc_irq;
+	}
+
+	info->rtc = i2c_new_dummy(parent_i2c->adapter,
+				  info->drv_data->rtc_i2c_addr);
+	if (!info->rtc) {
+		dev_err(info->dev, "Failed to allocate I2C device for RTC\n");
+		return -ENODEV;
+	}
+
+	info->rtc_regmap = devm_regmap_init_i2c(info->rtc,
+						&max77686_rtc_regmap_config);
+	if (IS_ERR(info->rtc_regmap)) {
+		ret = PTR_ERR(info->rtc_regmap);
+		dev_err(info->dev, "Failed to allocate RTC regmap: %d\n", ret);
+		goto err_unregister_i2c;
+	}
+
+add_rtc_irq:
+	ret = regmap_add_irq_chip(info->rtc_regmap, info->rtc_irq,
+				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
+				  IRQF_SHARED, 0, info->drv_data->rtc_irq_chip,
+				  &info->rtc_irq_data);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to add RTC irq chip: %d\n", ret);
+		goto err_unregister_i2c;
+	}
+
+	return 0;
+
+err_unregister_i2c:
+	if (info->rtc)
+		i2c_unregister_device(info->rtc);
+	return ret;
+}
+
 static int max77686_rtc_probe(struct platform_device *pdev)
 {
-	struct max77686_dev *max77686 = dev_get_drvdata(pdev->dev.parent);
 	struct max77686_rtc_info *info;
 	const struct platform_device_id *id = platform_get_device_id(pdev);
 	int ret;
@@ -613,18 +707,16 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 
 	mutex_init(&info->lock);
 	info->dev = &pdev->dev;
-	info->rtc = max77686->rtc;
 	info->drv_data = (const struct max77686_rtc_driver_data *)
 		id->driver_data;
 
-	info->regmap = max77686->regmap;
-	info->rtc_regmap = (info->drv_data->separate_i2c_addr) ?
-			    max77686->rtc_regmap : info->regmap;
+	ret = max77686_init_rtc_regmap(info);
+	if (ret < 0)
+		return ret;
 
 	platform_set_drvdata(pdev, info);
 
 	ret = max77686_rtc_init_reg(info);
-
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to initialize RTC reg:%d\n", ret);
 		goto err_rtc;
@@ -643,30 +735,43 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
-	if (!max77686->rtc_irq_data) {
-		ret = -EINVAL;
-		dev_err(&pdev->dev, "No RTC regmap IRQ chip\n");
-		goto err_rtc;
-	}
-
-	info->virq = regmap_irq_get_virq(max77686->rtc_irq_data,
+	info->virq = regmap_irq_get_virq(info->rtc_irq_data,
 					 MAX77686_RTCIRQ_RTCA1);
 	if (!info->virq) {
 		ret = -ENXIO;
 		goto err_rtc;
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, info->virq, NULL,
-					max77686_rtc_alarm_irq, 0,
-					"rtc-alarm1", info);
-	if (ret < 0)
+	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq, 0,
+				   "rtc-alarm1", info);
+	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
+		goto err_rtc;
+	}
+
+	return 0;
 
 err_rtc:
+	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
+	if (info->rtc)
+		i2c_unregister_device(info->rtc);
+
 	return ret;
 }
 
+static int max77686_rtc_remove(struct platform_device *pdev)
+{
+	struct max77686_rtc_info *info = platform_get_drvdata(pdev);
+
+	free_irq(info->virq, info);
+	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
+	if (info->rtc)
+		i2c_unregister_device(info->rtc);
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int max77686_rtc_suspend(struct device *dev)
 {
@@ -707,6 +812,7 @@ static struct platform_driver max77686_rtc_driver = {
 		.pm	= &max77686_rtc_pm_ops,
 	},
 	.probe		= max77686_rtc_probe,
+	.remove		= max77686_rtc_remove,
 	.id_table	= rtc_id,
 };
 
diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
index f504349..643dae7 100644
--- a/include/linux/mfd/max77686-private.h
+++ b/include/linux/mfd/max77686-private.h
@@ -437,14 +437,11 @@ enum max77686_irq {
 struct max77686_dev {
 	struct device *dev;
 	struct i2c_client *i2c; /* 0xcc / PMIC, Battery Control, and FLASH */
-	struct i2c_client *rtc; /* slave addr 0x0c */
 
 	unsigned long type;
 
 	struct regmap *regmap;		/* regmap for mfd */
-	struct regmap *rtc_regmap;	/* regmap for rtc */
 	struct regmap_irq_chip_data *irq_data;
-	struct regmap_irq_chip_data *rtc_irq_data;
 
 	int irq;
 	struct mutex irqlock;
-- 
2.1.4

[rtc-linux] [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

To make RTC block of MAX77686/MAX77802 as independent driver,
move the registration of i2c device, regmap for register access
and irq_chip for interrupt support inside the RTC driver.
Removed the same initialisation from MFD driver.

Having this change will allow to reuse this driver for different
PMIC/devices from Maxim Semiconductor if they kept same RTC IP on
different PMIC. Some of examples as PMIC MAX77620, MAX20024 where
same RTC IP used and hence driver for these chips will use this
driver only for RTC support.

Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
- Remove changes from Kconfig.
- Maintain all register definition in max77686 private header and remove
  the movement to rtc driver.
- Taken care of all comments on V1 from Krzysztof and Javier.

Changes from V2:
- Taken care of missed sequence for removing the resource.
- Fix the crash when doing unbind by using requested_threaded_irq()
  instead of demv_requested_threaded_irq().

 drivers/mfd/max77686.c               |  85 +-------------------
 drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
 include/linux/mfd/max77686-private.h |   3 -
 3 files changed, 130 insertions(+), 106 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index bda6fd7..98ecd13 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -35,8 +35,6 @@
 #include <linux/err.h>
 #include <linux/of.h>
 
-#define I2C_ADDR_RTC	(0x0C >> 1)
-
 static const struct mfd_cell max77686_devs[] = {
 	{ .name = "max77686-pmic", },
 	{ .name = "max77686-rtc", },
@@ -116,11 +114,6 @@ static const struct regmap_config max77686_regmap_config = {
 	.val_bits = 8,
 };
 
-static const struct regmap_config max77686_rtc_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
-};
-
 static const struct regmap_config max77802_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -156,25 +149,6 @@ static const struct regmap_irq_chip max77686_irq_chip = {
 	.num_irqs		= ARRAY_SIZE(max77686_irqs),
 };
 
-static const struct regmap_irq max77686_rtc_irqs[] = {
-	/* RTC interrupts */
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC60S_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA1_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA2_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_SMPL_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC1S_MSK, },
-	{ .reg_offset = 0, .mask = MAX77686_RTCINT_WTSR_MSK, },
-};
-
-static const struct regmap_irq_chip max77686_rtc_irq_chip = {
-	.name			= "max77686-rtc",
-	.status_base		= MAX77686_RTC_INT,
-	.mask_base		= MAX77686_RTC_INTM,
-	.num_regs		= 1,
-	.irqs			= max77686_rtc_irqs,
-	.num_irqs		= ARRAY_SIZE(max77686_rtc_irqs),
-};
-
 static const struct regmap_irq_chip max77802_irq_chip = {
 	.name			= "max77802-pmic",
 	.status_base		= MAX77802_REG_INT1,
@@ -184,15 +158,6 @@ static const struct regmap_irq_chip max77802_irq_chip = {
 	.num_irqs		= ARRAY_SIZE(max77686_irqs),
 };
 
-static const struct regmap_irq_chip max77802_rtc_irq_chip = {
-	.name			= "max77802-rtc",
-	.status_base		= MAX77802_RTC_INT,
-	.mask_base		= MAX77802_RTC_INTM,
-	.num_regs		= 1,
-	.irqs			= max77686_rtc_irqs, /* same masks as 77686 */
-	.num_irqs		= ARRAY_SIZE(max77686_rtc_irqs),
-};
-
 static const struct of_device_id max77686_pmic_dt_match[] = {
 	{
 		.compatible = "maxim,max77686",
@@ -214,8 +179,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	int ret = 0;
 	const struct regmap_config *config;
 	const struct regmap_irq_chip *irq_chip;
-	const struct regmap_irq_chip *rtc_irq_chip;
-	struct regmap **rtc_regmap;
 	const struct mfd_cell *cells;
 	int n_devs;
 
@@ -242,15 +205,11 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	if (max77686->type == TYPE_MAX77686) {
 		config = &max77686_regmap_config;
 		irq_chip = &max77686_irq_chip;
-		rtc_irq_chip = &max77686_rtc_irq_chip;
-		rtc_regmap = &max77686->rtc_regmap;
 		cells =  max77686_devs;
 		n_devs = ARRAY_SIZE(max77686_devs);
 	} else {
 		config = &max77802_regmap_config;
 		irq_chip = &max77802_irq_chip;
-		rtc_irq_chip = &max77802_rtc_irq_chip;
-		rtc_regmap = &max77686->regmap;
 		cells =  max77802_devs;
 		n_devs = ARRAY_SIZE(max77802_devs);
 	}
@@ -270,59 +229,25 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 		return -ENODEV;
 	}
 
-	if (max77686->type == TYPE_MAX77686) {
-		max77686->rtc = i2c_new_dummy(i2c->adapter, I2C_ADDR_RTC);
-		if (!max77686->rtc) {
-			dev_err(max77686->dev,
-				"Failed to allocate I2C device for RTC\n");
-			return -ENODEV;
-		}
-
-		max77686->rtc_regmap =
-			devm_regmap_init_i2c(max77686->rtc,
-					     &max77686_rtc_regmap_config);
-		if (IS_ERR(max77686->rtc_regmap)) {
-			ret = PTR_ERR(max77686->rtc_regmap);
-			dev_err(max77686->dev,
-				"failed to allocate RTC regmap: %d\n",
-				ret);
-			goto err_unregister_i2c;
-		}
-	}
-
 	ret = regmap_add_irq_chip(max77686->regmap, max77686->irq,
 				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
 				  IRQF_SHARED, 0, irq_chip,
 				  &max77686->irq_data);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&i2c->dev, "failed to add PMIC irq chip: %d\n", ret);
-		goto err_unregister_i2c;
-	}
-
-	ret = regmap_add_irq_chip(*rtc_regmap, max77686->irq,
-				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
-				  IRQF_SHARED, 0, rtc_irq_chip,
-				  &max77686->rtc_irq_data);
-	if (ret) {
-		dev_err(&i2c->dev, "failed to add RTC irq chip: %d\n", ret);
-		goto err_del_irqc;
+		return ret;
 	}
 
 	ret = mfd_add_devices(max77686->dev, -1, cells, n_devs, NULL, 0, NULL);
 	if (ret < 0) {
 		dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
-		goto err_del_rtc_irqc;
+		goto err_del_irqc;
 	}
 
 	return 0;
 
-err_del_rtc_irqc:
-	regmap_del_irq_chip(max77686->irq, max77686->rtc_irq_data);
 err_del_irqc:
 	regmap_del_irq_chip(max77686->irq, max77686->irq_data);
-err_unregister_i2c:
-	if (max77686->type == TYPE_MAX77686)
-		i2c_unregister_device(max77686->rtc);
 
 	return ret;
 }
@@ -333,12 +258,8 @@ static int max77686_i2c_remove(struct i2c_client *i2c)
 
 	mfd_remove_devices(max77686->dev);
 
-	regmap_del_irq_chip(max77686->irq, max77686->rtc_irq_data);
 	regmap_del_irq_chip(max77686->irq, max77686->irq_data);
 
-	if (max77686->type == TYPE_MAX77686)
-		i2c_unregister_device(max77686->rtc);
-
 	return 0;
 }
 
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index ab1f2cd..45d6396 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/rtc.h>
 #include <linux/delay.h>
@@ -22,6 +23,9 @@
 #include <linux/irqdomain.h>
 #include <linux/regmap.h>
 
+#define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
+#define MAX77686_INVALID_I2C_ADDR	(-1)
+
 /* RTC Control Register */
 #define BCD_EN_SHIFT			0
 #define BCD_EN_MASK			BIT(BCD_EN_SHIFT)
@@ -68,8 +72,10 @@ struct max77686_rtc_driver_data {
 	const unsigned int	*map;
 	/* Has a separate alarm enable register? */
 	bool			alarm_enable_reg;
-	/* Has a separate I2C regmap for the RTC? */
-	bool			separate_i2c_addr;
+	/* I2C address for RTC block */
+	int			rtc_i2c_addr;
+	/* RTC IRQ CHIP for regmap */
+	const struct regmap_irq_chip *rtc_irq_chip;
 };
 
 struct max77686_rtc_info {
@@ -82,7 +88,9 @@ struct max77686_rtc_info {
 	struct regmap		*rtc_regmap;
 
 	const struct max77686_rtc_driver_data *drv_data;
+	struct regmap_irq_chip_data *rtc_irq_data;
 
+	int rtc_irq;
 	int virq;
 	int rtc_24hr_mode;
 };
@@ -153,12 +161,32 @@ static const unsigned int max77686_map[REG_RTC_END] = {
 	[REG_RTC_AE1]	     = REG_RTC_NONE,
 };
 
+static const struct regmap_irq max77686_rtc_irqs[] = {
+	/* RTC interrupts */
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC60S_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA1_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTCA2_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_SMPL_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_RTC1S_MSK, },
+	{ .reg_offset = 0, .mask = MAX77686_RTCINT_WTSR_MSK, },
+};
+
+static const struct regmap_irq_chip max77686_rtc_irq_chip = {
+	.name		= "max77686-rtc",
+	.status_base	= MAX77686_RTC_INT,
+	.mask_base	= MAX77686_RTC_INTM,
+	.num_regs	= 1,
+	.irqs		= max77686_rtc_irqs,
+	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs),
+};
+
 static const struct max77686_rtc_driver_data max77686_drv_data = {
 	.delay = 16000,
 	.mask  = 0x7f,
 	.map   = max77686_map,
 	.alarm_enable_reg  = false,
-	.separate_i2c_addr = true,
+	.rtc_i2c_addr = MAX77686_I2C_ADDR_RTC,
+	.rtc_irq_chip = &max77686_rtc_irq_chip,
 };
 
 static const unsigned int max77802_map[REG_RTC_END] = {
@@ -190,12 +218,22 @@ static const unsigned int max77802_map[REG_RTC_END] = {
 	[REG_RTC_AE1]	     = MAX77802_RTC_AE1,
 };
 
+static const struct regmap_irq_chip max77802_rtc_irq_chip = {
+	.name		= "max77802-rtc",
+	.status_base	= MAX77802_RTC_INT,
+	.mask_base	= MAX77802_RTC_INTM,
+	.num_regs	= 1,
+	.irqs		= max77686_rtc_irqs, /* same masks as 77686 */
+	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs),
+};
+
 static const struct max77686_rtc_driver_data max77802_drv_data = {
 	.delay = 200,
 	.mask  = 0xff,
 	.map   = max77802_map,
 	.alarm_enable_reg  = true,
-	.separate_i2c_addr = false,
+	.rtc_i2c_addr = MAX77686_INVALID_I2C_ADDR,
+	.rtc_irq_chip = &max77802_rtc_irq_chip,
 };
 
 static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
@@ -599,9 +637,65 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
 	return ret;
 }
 
+static const struct regmap_config max77686_rtc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
+{
+	struct device *parent = info->dev->parent;
+	struct i2c_client *parent_i2c = to_i2c_client(parent);
+	int ret;
+
+	info->rtc_irq = parent_i2c->irq;
+
+	info->regmap = dev_get_regmap(parent, NULL);
+	if (!info->regmap) {
+		dev_err(info->dev, "Failed to get rtc regmap\n");
+		return -ENODEV;
+	}
+
+	if (info->drv_data->rtc_i2c_addr == MAX77686_INVALID_I2C_ADDR) {
+		info->rtc_regmap = info->regmap;
+		goto add_rtc_irq;
+	}
+
+	info->rtc = i2c_new_dummy(parent_i2c->adapter,
+				  info->drv_data->rtc_i2c_addr);
+	if (!info->rtc) {
+		dev_err(info->dev, "Failed to allocate I2C device for RTC\n");
+		return -ENODEV;
+	}
+
+	info->rtc_regmap = devm_regmap_init_i2c(info->rtc,
+						&max77686_rtc_regmap_config);
+	if (IS_ERR(info->rtc_regmap)) {
+		ret = PTR_ERR(info->rtc_regmap);
+		dev_err(info->dev, "Failed to allocate RTC regmap: %d\n", ret);
+		goto err_unregister_i2c;
+	}
+
+add_rtc_irq:
+	ret = regmap_add_irq_chip(info->rtc_regmap, info->rtc_irq,
+				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
+				  IRQF_SHARED, 0, info->drv_data->rtc_irq_chip,
+				  &info->rtc_irq_data);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to add RTC irq chip: %d\n", ret);
+		goto err_unregister_i2c;
+	}
+
+	return 0;
+
+err_unregister_i2c:
+	if (info->rtc)
+		i2c_unregister_device(info->rtc);
+	return ret;
+}
+
 static int max77686_rtc_probe(struct platform_device *pdev)
 {
-	struct max77686_dev *max77686 = dev_get_drvdata(pdev->dev.parent);
 	struct max77686_rtc_info *info;
 	const struct platform_device_id *id = platform_get_device_id(pdev);
 	int ret;
@@ -613,18 +707,16 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 
 	mutex_init(&info->lock);
 	info->dev = &pdev->dev;
-	info->rtc = max77686->rtc;
 	info->drv_data = (const struct max77686_rtc_driver_data *)
 		id->driver_data;
 
-	info->regmap = max77686->regmap;
-	info->rtc_regmap = (info->drv_data->separate_i2c_addr) ?
-			    max77686->rtc_regmap : info->regmap;
+	ret = max77686_init_rtc_regmap(info);
+	if (ret < 0)
+		return ret;
 
 	platform_set_drvdata(pdev, info);
 
 	ret = max77686_rtc_init_reg(info);
-
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to initialize RTC reg:%d\n", ret);
 		goto err_rtc;
@@ -643,30 +735,43 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
-	if (!max77686->rtc_irq_data) {
-		ret = -EINVAL;
-		dev_err(&pdev->dev, "No RTC regmap IRQ chip\n");
-		goto err_rtc;
-	}
-
-	info->virq = regmap_irq_get_virq(max77686->rtc_irq_data,
+	info->virq = regmap_irq_get_virq(info->rtc_irq_data,
 					 MAX77686_RTCIRQ_RTCA1);
 	if (!info->virq) {
 		ret = -ENXIO;
 		goto err_rtc;
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, info->virq, NULL,
-					max77686_rtc_alarm_irq, 0,
-					"rtc-alarm1", info);
-	if (ret < 0)
+	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq, 0,
+				   "rtc-alarm1", info);
+	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
+		goto err_rtc;
+	}
+
+	return 0;
 
 err_rtc:
+	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
+	if (info->rtc)
+		i2c_unregister_device(info->rtc);
+
 	return ret;
 }
 
+static int max77686_rtc_remove(struct platform_device *pdev)
+{
+	struct max77686_rtc_info *info = platform_get_drvdata(pdev);
+
+	free_irq(info->virq, info);
+	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
+	if (info->rtc)
+		i2c_unregister_device(info->rtc);
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int max77686_rtc_suspend(struct device *dev)
 {
@@ -707,6 +812,7 @@ static struct platform_driver max77686_rtc_driver = {
 		.pm	= &max77686_rtc_pm_ops,
 	},
 	.probe		= max77686_rtc_probe,
+	.remove		= max77686_rtc_remove,
 	.id_table	= rtc_id,
 };
 
diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
index f504349..643dae7 100644
--- a/include/linux/mfd/max77686-private.h
+++ b/include/linux/mfd/max77686-private.h
@@ -437,14 +437,11 @@ enum max77686_irq {
 struct max77686_dev {
 	struct device *dev;
 	struct i2c_client *i2c; /* 0xcc / PMIC, Battery Control, and FLASH */
-	struct i2c_client *rtc; /* slave addr 0x0c */
 
 	unsigned long type;
 
 	struct regmap *regmap;		/* regmap for mfd */
-	struct regmap *rtc_regmap;	/* regmap for rtc */
 	struct regmap_irq_chip_data *irq_data;
-	struct regmap_irq_chip_data *rtc_irq_data;
 
 	int irq;
 	struct mutex irqlock;
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Krzysztof Kozlowski]

On 03.02.2016 22:47, Laxman Dewangan wrote:
> To make RTC block of MAX77686/MAX77802 as independent driver,
> move the registration of i2c device, regmap for register access
> and irq_chip for interrupt support inside the RTC driver.
> Removed the same initialisation from MFD driver.
> 
> Having this change will allow to reuse this driver for different
> PMIC/devices from Maxim Semiconductor if they kept same RTC IP on
> different PMIC. Some of examples as PMIC MAX77620, MAX20024 where
> same RTC IP used and hence driver for these chips will use this
> driver only for RTC support.
> 
> Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@osg.samsung.com>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> Changes from V1:
> - Remove changes from Kconfig.
> - Maintain all register definition in max77686 private header and remove
>   the movement to rtc driver.
> - Taken care of all comments on V1 from Krzysztof and Javier.
> 
> Changes from V2:
> - Taken care of missed sequence for removing the resource.
> - Fix the crash when doing unbind by using requested_threaded_irq()
>   instead of demv_requested_threaded_irq().
> 
>  drivers/mfd/max77686.c               |  85 +-------------------
>  drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
>  include/linux/mfd/max77686-private.h |   3 -
>  3 files changed, 130 insertions(+), 106 deletions(-)
> 

Looks good and works properly (RTC, unbind/rebind, wakealarm again)
but... after applying this patch the device cannot suspend to RAM after
unbind/rebind.

Reproduce path, after booting:
$ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
$ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
$ rtcwake -d rtc2 -m mem -s 5 -v
(or instead of rtcwake: echo mem > /sys/power/state)

Proper behaviour:
##############################
[   28.549244] PM: late suspend of devices complete after 2.239 msecs
[   28.556051] PM: noirq suspend of devices complete after 2.078 msecs
[   28.560952] Disabling non-boot CPUs ...
[   28.588392] CPU1: shutdown
[   28.623283] CPU2: shutdown
[   28.653242] CPU3: shutdown

Power usage drops. Device wakes up (either from RTC or from manual, e.g.
key press)
##############################


Behaviour with patch 5/5:
[   67.068586] PM: Syncing filesystems ... done.
[   67.121673] Freezing user space processes ... (elapsed 0.004 seconds)
done.
[   67.123457] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[   67.182024] wake enabled for irq 125
[   67.184315] wake enabled for irq 127
[   67.187829] wake enabled for irq 147
[   67.191238] wake enabled for irq 148
[   67.195178] dwc2 12480000.hsotg: suspending usb gadget g_ether
[   67.200647] dwc2 12480000.hsotg: new device is full-speed
[   67.206036] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   67.212523] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   67.221742] PM: suspend of devices complete after 93.132 msecs
[   67.226113] VALIVE_1.0V_AP: No configuration
[   67.230399] VCC_1.8V_AP: No configuration
[   67.234385] VCC_2.8V_AP: No configuration
[   67.238373] VCC_1.8V_IO: No configuration
[   67.243130] CAM_ISP_MIPI_1.2V: No configuration
[   67.249142] NFC_AVDD_1.8V: No configuration
[   67.252661] CAM_SENSOR_CORE_1.2V: No configuration
[   67.256648] CAM_ISP_SEN_IO_1.8V: No configuration
[   67.261311] VT_CAM_1.8V: No configuration
[   67.265323] VDDQ_PRE_1.8V: No configuration
[   67.269488] VTF_2.8V: No configuration
[   67.273219] VMEM_VDD_2.8V: No configuration
[   67.277386] TSP_AVDD_3.3V: No configuration
[   67.281553] TSP_VDD_1.8V: No configuration
[   67.285615] LCD_VCC_3.3V: No configuration
[   67.289712] MOTOR_VCC_3.0V: No configuration
[   67.296266] VMEM_1.2V_AP: No configuration
[   67.298902] VCC_SUB_1.35V: No configuration
[   67.303064] VCC_SUB_2.0V: No configuration
[   67.307142] VMEM_VDDF_3.0V: No configuration
[   67.311377] CAM_ISP_CORE_1.2V: No configuration
[   67.318181] PM: late suspend of devices complete after 2.267 msecs
[   67.325046] PM: noirq suspend of devices complete after 2.137 msecs
[   67.329857] Disabling non-boot CPUs ...

No CPU shutdown messages. Power usage not decreased, device does not
wake up.

I don't know yet what is the cause... At first glance it looks okay...

Best regards,
Krzysztof

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Krzysztof Kozlowski]

On 03.02.2016 22:47, Laxman Dewangan wrote:
> To make RTC block of MAX77686/MAX77802 as independent driver,
> move the registration of i2c device, regmap for register access
> and irq_chip for interrupt support inside the RTC driver.
> Removed the same initialisation from MFD driver.
> 
> Having this change will allow to reuse this driver for different
> PMIC/devices from Maxim Semiconductor if they kept same RTC IP on
> different PMIC. Some of examples as PMIC MAX77620, MAX20024 where
> same RTC IP used and hence driver for these chips will use this
> driver only for RTC support.
> 
> Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> CC: Javier Martinez Canillas <javier@osg.samsung.com>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> Changes from V1:
> - Remove changes from Kconfig.
> - Maintain all register definition in max77686 private header and remove
>   the movement to rtc driver.
> - Taken care of all comments on V1 from Krzysztof and Javier.
> 
> Changes from V2:
> - Taken care of missed sequence for removing the resource.
> - Fix the crash when doing unbind by using requested_threaded_irq()
>   instead of demv_requested_threaded_irq().
> 
>  drivers/mfd/max77686.c               |  85 +-------------------
>  drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
>  include/linux/mfd/max77686-private.h |   3 -
>  3 files changed, 130 insertions(+), 106 deletions(-)
> 

Looks good and works properly (RTC, unbind/rebind, wakealarm again)
but... after applying this patch the device cannot suspend to RAM after
unbind/rebind.

Reproduce path, after booting:
$ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
$ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
$ rtcwake -d rtc2 -m mem -s 5 -v
(or instead of rtcwake: echo mem > /sys/power/state)

Proper behaviour:
##############################
[   28.549244] PM: late suspend of devices complete after 2.239 msecs
[   28.556051] PM: noirq suspend of devices complete after 2.078 msecs
[   28.560952] Disabling non-boot CPUs ...
[   28.588392] CPU1: shutdown
[   28.623283] CPU2: shutdown
[   28.653242] CPU3: shutdown

Power usage drops. Device wakes up (either from RTC or from manual, e.g.
key press)
##############################


Behaviour with patch 5/5:
[   67.068586] PM: Syncing filesystems ... done.
[   67.121673] Freezing user space processes ... (elapsed 0.004 seconds)
done.
[   67.123457] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[   67.182024] wake enabled for irq 125
[   67.184315] wake enabled for irq 127
[   67.187829] wake enabled for irq 147
[   67.191238] wake enabled for irq 148
[   67.195178] dwc2 12480000.hsotg: suspending usb gadget g_ether
[   67.200647] dwc2 12480000.hsotg: new device is full-speed
[   67.206036] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   67.212523] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   67.221742] PM: suspend of devices complete after 93.132 msecs
[   67.226113] VALIVE_1.0V_AP: No configuration
[   67.230399] VCC_1.8V_AP: No configuration
[   67.234385] VCC_2.8V_AP: No configuration
[   67.238373] VCC_1.8V_IO: No configuration
[   67.243130] CAM_ISP_MIPI_1.2V: No configuration
[   67.249142] NFC_AVDD_1.8V: No configuration
[   67.252661] CAM_SENSOR_CORE_1.2V: No configuration
[   67.256648] CAM_ISP_SEN_IO_1.8V: No configuration
[   67.261311] VT_CAM_1.8V: No configuration
[   67.265323] VDDQ_PRE_1.8V: No configuration
[   67.269488] VTF_2.8V: No configuration
[   67.273219] VMEM_VDD_2.8V: No configuration
[   67.277386] TSP_AVDD_3.3V: No configuration
[   67.281553] TSP_VDD_1.8V: No configuration
[   67.285615] LCD_VCC_3.3V: No configuration
[   67.289712] MOTOR_VCC_3.0V: No configuration
[   67.296266] VMEM_1.2V_AP: No configuration
[   67.298902] VCC_SUB_1.35V: No configuration
[   67.303064] VCC_SUB_2.0V: No configuration
[   67.307142] VMEM_VDDF_3.0V: No configuration
[   67.311377] CAM_ISP_CORE_1.2V: No configuration
[   67.318181] PM: late suspend of devices complete after 2.267 msecs
[   67.325046] PM: noirq suspend of devices complete after 2.137 msecs
[   67.329857] Disabling non-boot CPUs ...

No CPU shutdown messages. Power usage not decreased, device does not
wake up.

I don't know yet what is the cause... At first glance it looks okay...

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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>
>> Changes from V2:
>> - Taken care of missed sequence for removing the resource.
>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>    instead of demv_requested_threaded_irq().
>>
>>   drivers/mfd/max77686.c               |  85 +-------------------
>>   drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
>>   include/linux/mfd/max77686-private.h |   3 -
>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>
> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
> but... after applying this patch the device cannot suspend to RAM after
> unbind/rebind.
>
> Reproduce path, after booting:
> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
> $ rtcwake -d rtc2 -m mem -s 5 -v
> (or instead of rtcwake: echo mem > /sys/power/state)


My device does not support suspend and hence I can nto test this part 
and need your help to debug it.

Few question to narrow down:
1. Have you try to bind/unbind mfd driver to make sure that there is no 
inherent issue in the framework driver?

2. Does suspend entry and wake alarm work if you dont do unbind and just 
bind it and enter into suspend?

3. Can you try locally to not use devm_regmap_init_i2c() and just use 
the regmap_init_i2c() and proper removal of this from error path and 
remove callback?



Three things we are doing new on this movement, i2c client creating, 
regmap_init and adding irq chip.
And probably some issue on this calls?
Let's try with not using devm_ * for this calls?

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>
>> Changes from V2:
>> - Taken care of missed sequence for removing the resource.
>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>    instead of demv_requested_threaded_irq().
>>
>>   drivers/mfd/max77686.c               |  85 +-------------------
>>   drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
>>   include/linux/mfd/max77686-private.h |   3 -
>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>
> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
> but... after applying this patch the device cannot suspend to RAM after
> unbind/rebind.
>
> Reproduce path, after booting:
> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
> $ rtcwake -d rtc2 -m mem -s 5 -v
> (or instead of rtcwake: echo mem > /sys/power/state)


My device does not support suspend and hence I can nto test this part 
and need your help to debug it.

Few question to narrow down:
1. Have you try to bind/unbind mfd driver to make sure that there is no 
inherent issue in the framework driver?

2. Does suspend entry and wake alarm work if you dont do unbind and just 
bind it and enter into suspend?

3. Can you try locally to not use devm_regmap_init_i2c() and just use 
the regmap_init_i2c() and proper removal of this from error path and 
remove callback?



Three things we are doing new on this movement, i2c client creating, 
regmap_init and adding irq chip.
And probably some issue on this calls?
Let's try with not using devm_ * for this calls?


-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

Hi Javier,

On Thursday 04 February 2016 07:33 AM, Laxman Dewangan wrote:
>
> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>
>>> Changes from V2:
>>> - Taken care of missed sequence for removing the resource.
>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>    instead of demv_requested_threaded_irq().
>>>
>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>   drivers/rtc/rtc-max77686.c           | 148 
>>> ++++++++++++++++++++++++++++++-----
>>>   include/linux/mfd/max77686-private.h |   3 -
>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>
>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>> but... after applying this patch the device cannot suspend to RAM after
>> unbind/rebind.
>>
>> Reproduce path, after booting:
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>> $ rtcwake -d rtc2 -m mem -s 5 -v
>> (or instead of rtcwake: echo mem > /sys/power/state)
>

Can you please also try to test in your setup which have MAX77802?
On this, we do not create i2c client and do regmap_init_i2c. So may be 
it helps on narrow down the issue.

>
> My device does not support suspend and hence I can nto test this part 
> and need your help to debug it.
>
> Few question to narrow down:
> 1. Have you try to bind/unbind mfd driver to make sure that there is 
> no inherent issue in the framework driver?
>
> 2. Does suspend entry and wake alarm work if you dont do unbind and 
> just bind it and enter into suspend?
>
> 3. Can you try locally to not use devm_regmap_init_i2c() and just use 
> the regmap_init_i2c() and proper removal of this from error path and 
> remove callback?
>
>
>
> Three things we are doing new on this movement, i2c client creating, 
> regmap_init and adding irq chip.
> And probably some issue on this calls?
> Let's try with not using devm_ * for this calls?
>
>

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

Hi Javier,

On Thursday 04 February 2016 07:33 AM, Laxman Dewangan wrote:
>
> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>
>>> Changes from V2:
>>> - Taken care of missed sequence for removing the resource.
>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>    instead of demv_requested_threaded_irq().
>>>
>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>   drivers/rtc/rtc-max77686.c           | 148 
>>> ++++++++++++++++++++++++++++++-----
>>>   include/linux/mfd/max77686-private.h |   3 -
>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>
>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>> but... after applying this patch the device cannot suspend to RAM after
>> unbind/rebind.
>>
>> Reproduce path, after booting:
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>> $ rtcwake -d rtc2 -m mem -s 5 -v
>> (or instead of rtcwake: echo mem > /sys/power/state)
>

Can you please also try to test in your setup which have MAX77802?
On this, we do not create i2c client and do regmap_init_i2c. So may be 
it helps on narrow down the issue.

>
> My device does not support suspend and hence I can nto test this part 
> and need your help to debug it.
>
> Few question to narrow down:
> 1. Have you try to bind/unbind mfd driver to make sure that there is 
> no inherent issue in the framework driver?
>
> 2. Does suspend entry and wake alarm work if you dont do unbind and 
> just bind it and enter into suspend?
>
> 3. Can you try locally to not use devm_regmap_init_i2c() and just use 
> the regmap_init_i2c() and proper removal of this from error path and 
> remove callback?
>
>
>
> Three things we are doing new on this movement, i2c client creating, 
> regmap_init and adding irq chip.
> And probably some issue on this calls?
> Let's try with not using devm_ * for this calls?
>
>

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/03/2016 11:50 PM, Laxman Dewangan wrote:
> Hi Javier,
>
> On Thursday 04 February 2016 07:33 AM, Laxman Dewangan wrote:
>>
>> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>>
>>>> Changes from V2:
>>>> - Taken care of missed sequence for removing the resource.
>>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>>    instead of demv_requested_threaded_irq().
>>>>
>>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>>   drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
>>>>   include/linux/mfd/max77686-private.h |   3 -
>>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>>
>>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>>> but... after applying this patch the device cannot suspend to RAM after
>>> unbind/rebind.
>>>
>>> Reproduce path, after booting:
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>>> $ rtcwake -d rtc2 -m mem -s 5 -v
>>> (or instead of rtcwake: echo mem > /sys/power/state)
>>
>
> Can you please also try to test in your setup which have MAX77802?
> On this, we do not create i2c client and do regmap_init_i2c. So may be it helps on narrow down the issue.
>

Of course, I planned to test it today but ran out of time and I don't have
access to my Chromebook now. I'll test it at first time tomorrow morning.
  
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/03/2016 11:50 PM, Laxman Dewangan wrote:
> Hi Javier,
>
> On Thursday 04 February 2016 07:33 AM, Laxman Dewangan wrote:
>>
>> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>>
>>>> Changes from V2:
>>>> - Taken care of missed sequence for removing the resource.
>>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>>    instead of demv_requested_threaded_irq().
>>>>
>>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>>   drivers/rtc/rtc-max77686.c           | 148 ++++++++++++++++++++++++++++++-----
>>>>   include/linux/mfd/max77686-private.h |   3 -
>>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>>
>>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>>> but... after applying this patch the device cannot suspend to RAM after
>>> unbind/rebind.
>>>
>>> Reproduce path, after booting:
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>>> $ rtcwake -d rtc2 -m mem -s 5 -v
>>> (or instead of rtcwake: echo mem > /sys/power/state)
>>
>
> Can you please also try to test in your setup which have MAX77802?
> On this, we do not create i2c client and do regmap_init_i2c. So may be it helps on narrow down the issue.
>

Of course, I planned to test it today but ran out of time and I don't have
access to my Chromebook now. I'll test it at first time tomorrow morning.
  
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Krzysztof Kozlowski]

On 04.02.2016 11:03, Laxman Dewangan wrote:
> 
> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>
>>> Changes from V2:
>>> - Taken care of missed sequence for removing the resource.
>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>    instead of demv_requested_threaded_irq().
>>>
>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>   drivers/rtc/rtc-max77686.c           | 148
>>> ++++++++++++++++++++++++++++++-----
>>>   include/linux/mfd/max77686-private.h |   3 -
>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>
>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>> but... after applying this patch the device cannot suspend to RAM after
>> unbind/rebind.
>>
>> Reproduce path, after booting:
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>> $ rtcwake -d rtc2 -m mem -s 5 -v
>> (or instead of rtcwake: echo mem > /sys/power/state)
> 
> 
> My device does not support suspend and hence I can nto test this part
> and need your help to debug it.
> 
> Few question to narrow down:
> 1. Have you try to bind/unbind mfd driver to make sure that there is no
> inherent issue in the framework driver?

That might be tricky... there are also regulator and clock drivers.  I
would have to disable them... It might take me some time to prepare an
environment for this.

> 
> 2. Does suspend entry and wake alarm work if you dont do unbind and just
> bind it and enter into suspend?
If by "just bind" you mean bind during system boot, then answer is yes.
Without the unbind+bind, the system suspends correctly.

Without your patch, system suspends correctly always (no rebind, rebind
etc.).

Actually I don't have to re-bind the RTC. Just:
$ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
$ echo mem > /sys/power/state
triggers the issue.

> 
> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
> the regmap_init_i2c() and proper removal of this from error path and
> remove callback?

I'll try to find some time for that. Maybe tomorrow.

BR,
Krzysztof

> Three things we are doing new on this movement, i2c client creating,
> regmap_init and adding irq chip.
> And probably some issue on this calls?
> Let's try with not using devm_ * for this calls?
> 
> 
> 
> 

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Krzysztof Kozlowski]

On 04.02.2016 11:03, Laxman Dewangan wrote:
> 
> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>
>>> Changes from V2:
>>> - Taken care of missed sequence for removing the resource.
>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>    instead of demv_requested_threaded_irq().
>>>
>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>   drivers/rtc/rtc-max77686.c           | 148
>>> ++++++++++++++++++++++++++++++-----
>>>   include/linux/mfd/max77686-private.h |   3 -
>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>
>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>> but... after applying this patch the device cannot suspend to RAM after
>> unbind/rebind.
>>
>> Reproduce path, after booting:
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>> $ rtcwake -d rtc2 -m mem -s 5 -v
>> (or instead of rtcwake: echo mem > /sys/power/state)
> 
> 
> My device does not support suspend and hence I can nto test this part
> and need your help to debug it.
> 
> Few question to narrow down:
> 1. Have you try to bind/unbind mfd driver to make sure that there is no
> inherent issue in the framework driver?

That might be tricky... there are also regulator and clock drivers.  I
would have to disable them... It might take me some time to prepare an
environment for this.

> 
> 2. Does suspend entry and wake alarm work if you dont do unbind and just
> bind it and enter into suspend?
If by "just bind" you mean bind during system boot, then answer is yes.
Without the unbind+bind, the system suspends correctly.

Without your patch, system suspends correctly always (no rebind, rebind
etc.).

Actually I don't have to re-bind the RTC. Just:
$ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
$ echo mem > /sys/power/state
triggers the issue.

> 
> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
> the regmap_init_i2c() and proper removal of this from error path and
> remove callback?

I'll try to find some time for that. Maybe tomorrow.

BR,
Krzysztof

> Three things we are doing new on this movement, i2c client creating,
> regmap_init and adding irq chip.
> And probably some issue on this calls?
> Let's try with not using devm_ * for this calls?
> 
> 
> 
> 

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Krzysztof Kozlowski]

On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
> On 04.02.2016 11:03, Laxman Dewangan wrote:
>>
>> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>>
>>>> Changes from V2:
>>>> - Taken care of missed sequence for removing the resource.
>>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>>    instead of demv_requested_threaded_irq().
>>>>
>>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>>   drivers/rtc/rtc-max77686.c           | 148
>>>> ++++++++++++++++++++++++++++++-----
>>>>   include/linux/mfd/max77686-private.h |   3 -
>>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>>
>>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>>> but... after applying this patch the device cannot suspend to RAM after
>>> unbind/rebind.
>>>
>>> Reproduce path, after booting:
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>>> $ rtcwake -d rtc2 -m mem -s 5 -v
>>> (or instead of rtcwake: echo mem > /sys/power/state)
>>
>>
>> My device does not support suspend and hence I can nto test this part
>> and need your help to debug it.
>>
>> Few question to narrow down:
>> 1. Have you try to bind/unbind mfd driver to make sure that there is no
>> inherent issue in the framework driver?
> 
> That might be tricky... there are also regulator and clock drivers.  I
> would have to disable them... It might take me some time to prepare an
> environment for this.
> 
>>
>> 2. Does suspend entry and wake alarm work if you dont do unbind and just
>> bind it and enter into suspend?
> If by "just bind" you mean bind during system boot, then answer is yes.
> Without the unbind+bind, the system suspends correctly.
> 
> Without your patch, system suspends correctly always (no rebind, rebind
> etc.).
> 
> Actually I don't have to re-bind the RTC. Just:
> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
> $ echo mem > /sys/power/state
> triggers the issue.
> 
>>
>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>> the regmap_init_i2c() and proper removal of this from error path and
>> remove callback?
> 
> I'll try to find some time for that. Maybe tomorrow.

regmap_init_i2c does not help. However helps commenting out the:
	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
from remove() callback.

BR,
Krzysztof

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Krzysztof Kozlowski]

On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
> On 04.02.2016 11:03, Laxman Dewangan wrote:
>>
>> On Thursday 04 February 2016 05:40 AM, Krzysztof Kozlowski wrote:
>>> On 03.02.2016 22:47, Laxman Dewangan wrote:
>>>>
>>>> Changes from V2:
>>>> - Taken care of missed sequence for removing the resource.
>>>> - Fix the crash when doing unbind by using requested_threaded_irq()
>>>>    instead of demv_requested_threaded_irq().
>>>>
>>>>   drivers/mfd/max77686.c               |  85 +-------------------
>>>>   drivers/rtc/rtc-max77686.c           | 148
>>>> ++++++++++++++++++++++++++++++-----
>>>>   include/linux/mfd/max77686-private.h |   3 -
>>>>   3 files changed, 130 insertions(+), 106 deletions(-)
>>>>
>>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>>> but... after applying this patch the device cannot suspend to RAM after
>>> unbind/rebind.
>>>
>>> Reproduce path, after booting:
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>>> $ rtcwake -d rtc2 -m mem -s 5 -v
>>> (or instead of rtcwake: echo mem > /sys/power/state)
>>
>>
>> My device does not support suspend and hence I can nto test this part
>> and need your help to debug it.
>>
>> Few question to narrow down:
>> 1. Have you try to bind/unbind mfd driver to make sure that there is no
>> inherent issue in the framework driver?
> 
> That might be tricky... there are also regulator and clock drivers.  I
> would have to disable them... It might take me some time to prepare an
> environment for this.
> 
>>
>> 2. Does suspend entry and wake alarm work if you dont do unbind and just
>> bind it and enter into suspend?
> If by "just bind" you mean bind during system boot, then answer is yes.
> Without the unbind+bind, the system suspends correctly.
> 
> Without your patch, system suspends correctly always (no rebind, rebind
> etc.).
> 
> Actually I don't have to re-bind the RTC. Just:
> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
> $ echo mem > /sys/power/state
> triggers the issue.
> 
>>
>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>> the regmap_init_i2c() and proper removal of this from error path and
>> remove callback?
> 
> I'll try to find some time for that. Maybe tomorrow.

regmap_init_i2c does not help. However helps commenting out the:
	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
from remove() callback.

BR,
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>> On 04.02.2016 11:03, Laxman Dewangan wrote:
>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>> the regmap_init_i2c() and proper removal of this from error path and
>>> remove callback?
>> I'll try to find some time for that. Maybe tomorrow.
> regmap_init_i2c does not help. However helps commenting out the:
> 	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
> from remove() callback.
>
>

Thanks for debugging.  I do not see anything on regmap_del_irq_chip() 
which can be suspected. Is this because of sharing interrupt between mfd 
and rtc driver?

What is your suggestion here?  Should we avoid this inside RTC driver 
(creating this in mfd) and pass this information to the rtc driver using 
customized platform driver?

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>> On 04.02.2016 11:03, Laxman Dewangan wrote:
>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>> the regmap_init_i2c() and proper removal of this from error path and
>>> remove callback?
>> I'll try to find some time for that. Maybe tomorrow.
> regmap_init_i2c does not help. However helps commenting out the:
> 	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
> from remove() callback.
>
>

Thanks for debugging.  I do not see anything on regmap_del_irq_chip() 
which can be suspected. Is this because of sharing interrupt between mfd 
and rtc driver?

What is your suggestion here?  Should we avoid this inside RTC driver 
(creating this in mfd) and pass this information to the rtc driver using 
customized platform driver?

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/03/2016 11:50 PM, Laxman Dewangan wrote:

[snip]

>>>>
>>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>>> but... after applying this patch the device cannot suspend to RAM after
>>> unbind/rebind.
>>>
>>> Reproduce path, after booting:
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>>> $ rtcwake -d rtc2 -m mem -s 5 -v
>>> (or instead of rtcwake: echo mem > /sys/power/state)
>>
>
> Can you please also try to test in your setup which have MAX77802?
> On this, we do not create i2c client and do regmap_init_i2c. So may be it helps on narrow down the issue.
>

Sorry for the delay but I was busy with others tasks. I've tested it
on an Exynos5800 Peach Pi (which has a max77802) and I've the same
issue. The system doesn't suspend after the max77802-rc unbind.

As Krzysztof narrowed down, commenting the regmap_del_irq_chip() call
in the .remove callback makes it to not fail again.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/03/2016 11:50 PM, Laxman Dewangan wrote:

[snip]

>>>>
>>> Looks good and works properly (RTC, unbind/rebind, wakealarm again)
>>> but... after applying this patch the device cannot suspend to RAM after
>>> unbind/rebind.
>>>
>>> Reproduce path, after booting:
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/unbind
>>> $ echo max77686-rtc > /sys/bus/platform/drivers/max77686-rtc/bind
>>> $ rtcwake -d rtc2 -m mem -s 5 -v
>>> (or instead of rtcwake: echo mem > /sys/power/state)
>>
>
> Can you please also try to test in your setup which have MAX77802?
> On this, we do not create i2c client and do regmap_init_i2c. So may be it helps on narrow down the issue.
>

Sorry for the delay but I was busy with others tasks. I've tested it
on an Exynos5800 Peach Pi (which has a max77802) and I've the same
issue. The system doesn't suspend after the max77802-rc unbind.

As Krzysztof narrowed down, commenting the regmap_del_irq_chip() call
in the .remove callback makes it to not fail again.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/04/2016 07:13 AM, Laxman Dewangan wrote:
>
> On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
>> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>>> On 04.02.2016 11:03, Laxman Dewangan wrote:
>>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>>> the regmap_init_i2c() and proper removal of this from error path and
>>>> remove callback?
>>> I'll try to find some time for that. Maybe tomorrow.
>> regmap_init_i2c does not help. However helps commenting out the:
>>     regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
>> from remove() callback.
>>
>>
>
> Thanks for debugging.  I do not see anything on regmap_del_irq_chip() which can be suspected. Is this because of sharing interrupt between mfd and rtc driver?

I don't think so, as long as all the callers pass the IRQF_SHARED flag,
there shouldn't be issues if different drivers call request / free irq.

AFAIK the only thing to take into account is that all callers should
pass a non-NULL cookie (the void * parameter in request and free irq)
so the kernel has a way to differentiate at request / free time.

And this is the case for regmap irq since the regmap_irq_chip_data *
that is passed to regmap_{add,del}_irq_chip() is used as the cookie.

>
> What is your suggestion here?  Should we avoid this inside RTC driver (creating this in mfd) and pass this information to the rtc driver using customized platform driver?
>

That wouldn't be the right approach IMHO, that would just mask the real
issue by making the call to regmap_del_irq_chip() to never happen since
the MFD device is not usually unbound due regulators being too critical.

I think we should find the real cause why this is happening and fix it.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/04/2016 07:13 AM, Laxman Dewangan wrote:
>
> On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
>> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>>> On 04.02.2016 11:03, Laxman Dewangan wrote:
>>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>>> the regmap_init_i2c() and proper removal of this from error path and
>>>> remove callback?
>>> I'll try to find some time for that. Maybe tomorrow.
>> regmap_init_i2c does not help. However helps commenting out the:
>>     regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
>> from remove() callback.
>>
>>
>
> Thanks for debugging.  I do not see anything on regmap_del_irq_chip() which can be suspected. Is this because of sharing interrupt between mfd and rtc driver?

I don't think so, as long as all the callers pass the IRQF_SHARED flag,
there shouldn't be issues if different drivers call request / free irq.

AFAIK the only thing to take into account is that all callers should
pass a non-NULL cookie (the void * parameter in request and free irq)
so the kernel has a way to differentiate at request / free time.

And this is the case for regmap irq since the regmap_irq_chip_data *
that is passed to regmap_{add,del}_irq_chip() is used as the cookie.

>
> What is your suggestion here?  Should we avoid this inside RTC driver (creating this in mfd) and pass this information to the rtc driver using customized platform driver?
>

That wouldn't be the right approach IMHO, that would just mask the real
issue by making the call to regmap_del_irq_chip() to never happen since
the MFD device is not usually unbound due regulators being too critical.

I think we should find the real cause why this is happening and fix it.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

Hi Krzysztof, Javier,

On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>> the regmap_init_i2c() and proper removal of this from error path and
>>> remove callback?
>> I'll try to find some time for that. Maybe tomorrow.
> regmap_init_i2c does not help. However helps commenting out the:
> 	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
> from remove() callback.
>
>

I am trying to reproduce this in my system but I am ending up with 
different issue as it need to enable the suspend.
can you please help on following experiment:
1. In probe/init, do
    regmap_add_irq_chip()
   regmap_del_irq_chip() and then
   regmap_add_irq_chip() and
then without unbind()/bind(), working or not?

   This is to make sure that it is universal issue rather than calling 
from remove callback only.

2. Do regmap_add_irq_chip() but dont do any interrupt registration i.e. 
comment regmap_irq_get_virq() and request_threaded_irq() and hence 
free_irq().
Then do unbind/bind and then suspend.
   To make sure that this happen when only we have client registered 
interrupt.

3. Extension of 2
Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating irq 
mapping but dont do any interrupt registration i.e. comment 
request_threaded_irq() and hence free_irq().
Then do unbind/bind and then suspend.
   To make sure that this happen when only we have client registered 
interrupt or with mapping also.


Thanks for help in advance.

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

Hi Krzysztof, Javier,

On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>> the regmap_init_i2c() and proper removal of this from error path and
>>> remove callback?
>> I'll try to find some time for that. Maybe tomorrow.
> regmap_init_i2c does not help. However helps commenting out the:
> 	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
> from remove() callback.
>
>

I am trying to reproduce this in my system but I am ending up with 
different issue as it need to enable the suspend.
can you please help on following experiment:
1. In probe/init, do
    regmap_add_irq_chip()
   regmap_del_irq_chip() and then
   regmap_add_irq_chip() and
then without unbind()/bind(), working or not?

   This is to make sure that it is universal issue rather than calling 
from remove callback only.

2. Do regmap_add_irq_chip() but dont do any interrupt registration i.e. 
comment regmap_irq_get_virq() and request_threaded_irq() and hence 
free_irq().
Then do unbind/bind and then suspend.
   To make sure that this happen when only we have client registered 
interrupt.

3. Extension of 2
Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating irq 
mapping but dont do any interrupt registration i.e. comment 
request_threaded_irq() and hence free_irq().
Then do unbind/bind and then suspend.
   To make sure that this happen when only we have client registered 
interrupt or with mapping also.


Thanks for help in advance.

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

Sorry for not doing this before but today was a busy one.

On 02/05/2016 11:37 AM, Laxman Dewangan wrote:
> Hi Krzysztof, Javier,
>
> On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
>> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>>> the regmap_init_i2c() and proper removal of this from error path and
>>>> remove callback?
>>> I'll try to find some time for that. Maybe tomorrow.
>> regmap_init_i2c does not help. However helps commenting out the:
>>     regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
>> from remove() callback.
>>
>>
>
> I am trying to reproduce this in my system but I am ending up with different issue as it need to enable the suspend.

Ok, no worries. I can of course help testing on my system.

> can you please help on following experiment:
> 1. In probe/init, do
>     regmap_add_irq_chip()
>    regmap_del_irq_chip() and then
>    regmap_add_irq_chip() and
> then without unbind()/bind(), working or not?
>
>    This is to make sure that it is universal issue rather than calling from remove callback only.
>

This works, the system can S2R successfully.
  
> 2. Do regmap_add_irq_chip() but dont do any interrupt registration i.e. comment regmap_irq_get_virq() and request_threaded_irq() and hence free_irq().
> Then do unbind/bind and then suspend.
>    To make sure that this happen when only we have client registered interrupt.
>

This works as well.
  
> 3. Extension of 2
> Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating irq mapping but dont do any interrupt registration i.e. comment request_threaded_irq() and hence free_irq().
> Then do unbind/bind and then suspend.
>    To make sure that this happen when only we have client registered interrupt or with mapping also.
>

This fails, so the problem seems to be with the mapping.

So I tried another scenario:

4. Call regmap_del_irq_chip() just after regmap_irq_get_virq() and try to S2R
    without doing any unbind before.

    To test if this is a general issue with regmap_del_irq_chip() after doing
    the IRQ mapping and not something specific to the remove callback.

The machine failed to boot. So now at least we have narrowed down the issue.

I've looked at both regmap_irq_get_virq() and regmap_del_irq_chip() but I
couldn't find any obvious cause for the issue we are seeing. But it's late
Friday so probably I should just stop here and take a fresh look on Monday.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

Sorry for not doing this before but today was a busy one.

On 02/05/2016 11:37 AM, Laxman Dewangan wrote:
> Hi Krzysztof, Javier,
>
> On Thursday 04 February 2016 02:38 PM, Krzysztof Kozlowski wrote:
>> On 04.02.2016 15:58, Krzysztof Kozlowski wrote:
>>>> 3. Can you try locally to not use devm_regmap_init_i2c() and just use
>>>> the regmap_init_i2c() and proper removal of this from error path and
>>>> remove callback?
>>> I'll try to find some time for that. Maybe tomorrow.
>> regmap_init_i2c does not help. However helps commenting out the:
>>     regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
>> from remove() callback.
>>
>>
>
> I am trying to reproduce this in my system but I am ending up with different issue as it need to enable the suspend.

Ok, no worries. I can of course help testing on my system.

> can you please help on following experiment:
> 1. In probe/init, do
>     regmap_add_irq_chip()
>    regmap_del_irq_chip() and then
>    regmap_add_irq_chip() and
> then without unbind()/bind(), working or not?
>
>    This is to make sure that it is universal issue rather than calling from remove callback only.
>

This works, the system can S2R successfully.
  
> 2. Do regmap_add_irq_chip() but dont do any interrupt registration i.e. comment regmap_irq_get_virq() and request_threaded_irq() and hence free_irq().
> Then do unbind/bind and then suspend.
>    To make sure that this happen when only we have client registered interrupt.
>

This works as well.
  
> 3. Extension of 2
> Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating irq mapping but dont do any interrupt registration i.e. comment request_threaded_irq() and hence free_irq().
> Then do unbind/bind and then suspend.
>    To make sure that this happen when only we have client registered interrupt or with mapping also.
>

This fails, so the problem seems to be with the mapping.

So I tried another scenario:

4. Call regmap_del_irq_chip() just after regmap_irq_get_virq() and try to S2R
    without doing any unbind before.

    To test if this is a general issue with regmap_del_irq_chip() after doing
    the IRQ mapping and not something specific to the remove callback.

The machine failed to boot. So now at least we have narrowed down the issue.

I've looked at both regmap_irq_get_virq() and regmap_del_irq_chip() but I
couldn't find any obvious cause for the issue we are seeing. But it's late
Friday so probably I should just stop here and take a fresh look on Monday.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

Hi Javier,
'
On Saturday 06 February 2016 11:00 AM, Javier Martinez Canillas wrote:
> Hello Laxman,
>
> Sorry for not doing this before but today was a busy one.
>

Thanks for testing.


> On 02/05/2016 11:37 AM, Laxman Dewangan wrote:
>> Hi Krzysztof, Javier,
>>
>
>> 3. Extension of 2
>> Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating 
>> irq mapping but dont do any interrupt registration i.e. comment 
>> request_threaded_irq() and hence free_irq().
>> Then do unbind/bind and then suspend.
>>    To make sure that this happen when only we have client registered 
>> interrupt or with mapping also.
>>
>
> This fails, so the problem seems to be with the mapping.
>
> So I tried another scenario:
>
> 4. Call regmap_del_irq_chip() just after regmap_irq_get_virq() and try 
> to S2R
>    without doing any unbind before.
>
>    To test if this is a general issue with regmap_del_irq_chip() after 
> doing
>    the IRQ mapping and not something specific to the remove callback.
>
> The machine failed to boot. So now at least we have narrowed down the 
> issue.
>
> I've looked at both regmap_irq_get_virq() and regmap_del_irq_chip() but I
> couldn't find any obvious cause for the issue we are seeing. But it's 
> late
> Friday so probably I should just stop here and take a fresh look on 
> Monday.
>

So the issue is that when we create mapping, we can not delete the irq_chip.

I saw one function from irq framework irq_dispose_mapping(unsigned int 
virq).

So we need to dispose the mapping before deleting irq chip.


Becasue it is reproduced in normal boot also if we do create mapping and 
delete the irq chip data, I will also be able to validate if I get some 
time on weekend.

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

Hi Javier,
'
On Saturday 06 February 2016 11:00 AM, Javier Martinez Canillas wrote:
> Hello Laxman,
>
> Sorry for not doing this before but today was a busy one.
>

Thanks for testing.


> On 02/05/2016 11:37 AM, Laxman Dewangan wrote:
>> Hi Krzysztof, Javier,
>>
>
>> 3. Extension of 2
>> Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating 
>> irq mapping but dont do any interrupt registration i.e. comment 
>> request_threaded_irq() and hence free_irq().
>> Then do unbind/bind and then suspend.
>>    To make sure that this happen when only we have client registered 
>> interrupt or with mapping also.
>>
>
> This fails, so the problem seems to be with the mapping.
>
> So I tried another scenario:
>
> 4. Call regmap_del_irq_chip() just after regmap_irq_get_virq() and try 
> to S2R
>    without doing any unbind before.
>
>    To test if this is a general issue with regmap_del_irq_chip() after 
> doing
>    the IRQ mapping and not something specific to the remove callback.
>
> The machine failed to boot. So now at least we have narrowed down the 
> issue.
>
> I've looked at both regmap_irq_get_virq() and regmap_del_irq_chip() but I
> couldn't find any obvious cause for the issue we are seeing. But it's 
> late
> Friday so probably I should just stop here and take a fresh look on 
> Monday.
>

So the issue is that when we create mapping, we can not delete the irq_chip.

I saw one function from irq framework irq_dispose_mapping(unsigned int 
virq).

So we need to dispose the mapping before deleting irq chip.


Becasue it is reproduced in normal boot also if we do create mapping and 
delete the irq chip data, I will also be able to validate if I get some 
time on weekend.

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/06/2016 03:29 AM, Laxman Dewangan wrote:
> Hi Javier,
> '
> On Saturday 06 February 2016 11:00 AM, Javier Martinez Canillas wrote:
>> Hello Laxman,
>>
>> Sorry for not doing this before but today was a busy one.
>>
>
> Thanks for testing.
>
>
>> On 02/05/2016 11:37 AM, Laxman Dewangan wrote:
>>> Hi Krzysztof, Javier,
>>>
>>
>>> 3. Extension of 2
>>> Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating irq mapping but dont do any interrupt registration i.e. comment request_threaded_irq() and hence free_irq().
>>> Then do unbind/bind and then suspend.
>>>    To make sure that this happen when only we have client registered interrupt or with mapping also.
>>>
>>
>> This fails, so the problem seems to be with the mapping.
>>
>> So I tried another scenario:
>>
>> 4. Call regmap_del_irq_chip() just after regmap_irq_get_virq() and try to S2R
>>    without doing any unbind before.
>>
>>    To test if this is a general issue with regmap_del_irq_chip() after doing
>>    the IRQ mapping and not something specific to the remove callback.
>>
>> The machine failed to boot. So now at least we have narrowed down the issue.
>>
>> I've looked at both regmap_irq_get_virq() and regmap_del_irq_chip() but I
>> couldn't find any obvious cause for the issue we are seeing. But it's late
>> Friday so probably I should just stop here and take a fresh look on Monday.
>>
>
> So the issue is that when we create mapping, we can not delete the irq_chip.
>
> I saw one function from irq framework irq_dispose_mapping(unsigned int virq).
>
> So we need to dispose the mapping before deleting irq chip.
>
>
> Becasue it is reproduced in normal boot also if we do create mapping and delete the irq chip data, I will also be able to validate if I get some time on weekend.
>

Thanks, that was it! I was searching yesterday for the regmap_irq_get_virq()
counterpart and did not find it, but I was looking at include/linux/regmap.h
and didn't think about looking at include/linux/irqdomain.h.

Maybe we should add a regmap_irq_put_virq() that's just a wrapper around
irq_dispose_mapping() to make the API consistent and avoid this confusion?
I see that most drivers don't dispose the mapping on their error/remove path.

Anyways, with this patch on top of yours 5/5 my system is able to suspend
after an unbind again:

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 45d639674c0b..8ea6d601482a 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -765,6 +765,7 @@ static int max77686_rtc_remove(struct platform_device *pdev)
  	struct max77686_rtc_info *info = platform_get_drvdata(pdev);
  
  	free_irq(info->virq, info);
+	irq_dispose_mapping(info->virq);
  	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
  	if (info->rtc)
  		i2c_unregister_device(info->rtc);

This is a bug in the current MFD driver though so I would include it as
a separate patch prior to your patch that moves the regmap and i2c init
to the RTC driver.

When posting your new revsion feel free to add my {Reviewed,Tested}-by
tags for $SUBJECT and the new bugfix patch.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Javier Martinez Canillas]

Hello Laxman,

On 02/06/2016 03:29 AM, Laxman Dewangan wrote:
> Hi Javier,
> '
> On Saturday 06 February 2016 11:00 AM, Javier Martinez Canillas wrote:
>> Hello Laxman,
>>
>> Sorry for not doing this before but today was a busy one.
>>
>
> Thanks for testing.
>
>
>> On 02/05/2016 11:37 AM, Laxman Dewangan wrote:
>>> Hi Krzysztof, Javier,
>>>
>>
>>> 3. Extension of 2
>>> Do regmap_add_irq_chip(), call  regmap_irq_get_virq() for creating irq mapping but dont do any interrupt registration i.e. comment request_threaded_irq() and hence free_irq().
>>> Then do unbind/bind and then suspend.
>>>    To make sure that this happen when only we have client registered interrupt or with mapping also.
>>>
>>
>> This fails, so the problem seems to be with the mapping.
>>
>> So I tried another scenario:
>>
>> 4. Call regmap_del_irq_chip() just after regmap_irq_get_virq() and try to S2R
>>    without doing any unbind before.
>>
>>    To test if this is a general issue with regmap_del_irq_chip() after doing
>>    the IRQ mapping and not something specific to the remove callback.
>>
>> The machine failed to boot. So now at least we have narrowed down the issue.
>>
>> I've looked at both regmap_irq_get_virq() and regmap_del_irq_chip() but I
>> couldn't find any obvious cause for the issue we are seeing. But it's late
>> Friday so probably I should just stop here and take a fresh look on Monday.
>>
>
> So the issue is that when we create mapping, we can not delete the irq_chip.
>
> I saw one function from irq framework irq_dispose_mapping(unsigned int virq).
>
> So we need to dispose the mapping before deleting irq chip.
>
>
> Becasue it is reproduced in normal boot also if we do create mapping and delete the irq chip data, I will also be able to validate if I get some time on weekend.
>

Thanks, that was it! I was searching yesterday for the regmap_irq_get_virq()
counterpart and did not find it, but I was looking at include/linux/regmap.h
and didn't think about looking at include/linux/irqdomain.h.

Maybe we should add a regmap_irq_put_virq() that's just a wrapper around
irq_dispose_mapping() to make the API consistent and avoid this confusion?
I see that most drivers don't dispose the mapping on their error/remove path.

Anyways, with this patch on top of yours 5/5 my system is able to suspend
after an unbind again:

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 45d639674c0b..8ea6d601482a 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -765,6 +765,7 @@ static int max77686_rtc_remove(struct platform_device *pdev)
  	struct max77686_rtc_info *info = platform_get_drvdata(pdev);
  
  	free_irq(info->virq, info);
+	irq_dispose_mapping(info->virq);
  	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
  	if (info->rtc)
  		i2c_unregister_device(info->rtc);

This is a bug in the current MFD driver though so I would include it as
a separate patch prior to your patch that moves the regmap and i2c init
to the RTC driver.

When posting your new revsion feel free to add my {Reviewed,Tested}-by
tags for $SUBJECT and the new bugfix patch.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

-- 
-- 
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.

Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Saturday 06 February 2016 06:31 PM, Javier Martinez Canillas wrote:
> Hello Laxman,
>
> On 02/06/2016 03:29 AM, Laxman Dewangan wrote:
>>
>> So the issue is that when we create mapping, we can not delete the 
>> irq_chip.
>>
>> I saw one function from irq framework irq_dispose_mapping(unsigned 
>> int virq).
>>
>> So we need to dispose the mapping before deleting irq chip.
>>
>>
>> Becasue it is reproduced in normal boot also if we do create mapping 
>> and delete the irq chip data, I will also be able to validate if I 
>> get some time on weekend.
>>
>
> Thanks, that was it! I was searching yesterday for the 
> regmap_irq_get_virq()
> counterpart and did not find it, but I was looking at 
> include/linux/regmap.h
> and didn't think about looking at include/linux/irqdomain.h.
>
> Maybe we should add a regmap_irq_put_virq() that's just a wrapper around
> irq_dispose_mapping() to make the API consistent and avoid this 
> confusion?
> I see that most drivers don't dispose the mapping on their 
> error/remove path.
>
> Anyways, with this patch on top of yours 5/5 my system is able to suspend
> after an unbind again:
>
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 45d639674c0b..8ea6d601482a 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -765,6 +765,7 @@ static int max77686_rtc_remove(struct 
> platform_device *pdev)
>      struct max77686_rtc_info *info = platform_get_drvdata(pdev);
>
>      free_irq(info->virq, info);
> +    irq_dispose_mapping(info->virq);
>      regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
>      if (info->rtc)
>          i2c_unregister_device(info->rtc);
>
> This is a bug in the current MFD driver though so I would include it as
> a separate patch prior to your patch that moves the regmap and i2c init
> to the RTC driver.
>
> When posting your new revsion feel free to add my {Reviewed,Tested}-by
> tags for $SUBJECT and the new bugfix patch.


Thank you very much for testing/help.
Now we have all the fixes on this series. I am going to add the "put" 
funntion on regmap and then first fix it in the rtc driver and then 
apply my series on top of that. Now 7 patches..

[rtc-linux] Re: [PATCH V3 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Saturday 06 February 2016 06:31 PM, Javier Martinez Canillas wrote:
> Hello Laxman,
>
> On 02/06/2016 03:29 AM, Laxman Dewangan wrote:
>>
>> So the issue is that when we create mapping, we can not delete the 
>> irq_chip.
>>
>> I saw one function from irq framework irq_dispose_mapping(unsigned 
>> int virq).
>>
>> So we need to dispose the mapping before deleting irq chip.
>>
>>
>> Becasue it is reproduced in normal boot also if we do create mapping 
>> and delete the irq chip data, I will also be able to validate if I 
>> get some time on weekend.
>>
>
> Thanks, that was it! I was searching yesterday for the 
> regmap_irq_get_virq()
> counterpart and did not find it, but I was looking at 
> include/linux/regmap.h
> and didn't think about looking at include/linux/irqdomain.h.
>
> Maybe we should add a regmap_irq_put_virq() that's just a wrapper around
> irq_dispose_mapping() to make the API consistent and avoid this 
> confusion?
> I see that most drivers don't dispose the mapping on their 
> error/remove path.
>
> Anyways, with this patch on top of yours 5/5 my system is able to suspend
> after an unbind again:
>
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 45d639674c0b..8ea6d601482a 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -765,6 +765,7 @@ static int max77686_rtc_remove(struct 
> platform_device *pdev)
>      struct max77686_rtc_info *info = platform_get_drvdata(pdev);
>
>      free_irq(info->virq, info);
> +    irq_dispose_mapping(info->virq);
>      regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
>      if (info->rtc)
>          i2c_unregister_device(info->rtc);
>
> This is a bug in the current MFD driver though so I would include it as
> a separate patch prior to your patch that moves the regmap and i2c init
> to the RTC driver.
>
> When posting your new revsion feel free to add my {Reviewed,Tested}-by
> tags for $SUBJECT and the new bugfix patch.


Thank you very much for testing/help.
Now we have all the fixes on this series. I am going to add the "put" 
funntion on regmap and then first fix it in the rtc driver and then 
apply my series on top of that. Now 7 patches..


-- 
-- 
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.