linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/6] Add Maxim 77802 RTC support
@ 2014-09-19 10:26 Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 1/6] rtc: max77686: Allow the max77686 rtc to wakeup the system Javier Martinez Canillas
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

Hello Andrew,

This series add support for the Real Time clock present in
the Maxim 77802 Power Managment IC. The version number is
quite high because it previously was part of a bigger series
[0] that aimed to add support for all the devices in the
max77802 PMIC. But now that the max77802 dependencies were
already merged for 3.17, the series were split but I kept
the version numbering.

While working on the max77802 rtc support a lot of feedback
was given and the issues pointed out also apply to a driver
for a similar PMIC RTC (max77686). So patches 01/06 to 05/06
in the series are cleanups for the max77686 driver and patch
06/06 adds the support for the max77802 RTC.

This version address the issues pointed out on the previous
version [1] and changelog are present on each patch when is
applicable.

The series were tested on an Exynos5250 Snow (max77686) and
Exynos5420 Peach Pit (max77802) machines and applies cleanly
to both 3.17-rc1 and today's linux-next (20140919).

NOTE: The patches from the previous version [1] were already
present in the -mm tree [2] so I didn't know if I should had
sent this as a delta or as a new revision. I decided to do
the later but please let me know if you expected the former.

Doug Anderson (1):
  rtc: max77686: Allow the max77686 rtc to wakeup the system

Javier Martinez Canillas (5):
  rtc: max77686: Remove dead code for SMPL and WTSR.
  rtc: max77686: Fail to probe if no RTC regmap irqchip is set
  rtc: max77686: Remove unneded info log
  rtc: max77686: Use ffs() to calculate tm_wday
  rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock

 drivers/rtc/Kconfig        |  10 +
 drivers/rtc/Makefile       |   1 +
 drivers/rtc/rtc-max77686.c | 139 +++----------
 drivers/rtc/rtc-max77802.c | 501 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 542 insertions(+), 109 deletions(-)
 create mode 100644 drivers/rtc/rtc-max77802.c

[0]: http://lwn.net/Articles/605350/
[1]: https://lkml.org/lkml/2014/9/12/87
[2]: http://ozlabs.org/~akpm/mmots/series

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

* [PATCH v10 1/6] rtc: max77686: Allow the max77686 rtc to wakeup the system
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
@ 2014-09-19 10:26 ` Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 2/6] rtc: max77686: Remove dead code for SMPL and WTSR Javier Martinez Canillas
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

From: Doug Anderson <dianders@chromium.org>

The max77686 includes an RTC that keeps power during suspend.  It's
convenient to be able to use it as a wakeup source.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v9:
 - Remove note from Doug that is not applicable anymore due changes in the
   i2c driver.
 - Add s-o-b since I'm in the patch delivery path. Suggested by Andrew Morton.

Changes since v8: None

Changes since v7: None

Changes since v6: None

Changes since v5:
 - Fix $SUBJECT since the patch does not actually touch the mfd subsys.
   Suggested by Lee Jones.

Changes since v4: None

Changes since v3:
 - Keep the note that this patch needs another change due wakeup
   ordering problems.

 drivers/rtc/rtc-max77686.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index d20a7f0..c1c6055 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -583,6 +583,33 @@ static void max77686_rtc_shutdown(struct platform_device *pdev)
 #endif /* MAX77686_RTC_WTSR_SMPL */
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int max77686_rtc_suspend(struct device *dev)
+{
+	if (device_may_wakeup(dev)) {
+		struct max77686_rtc_info *info = dev_get_drvdata(dev);
+
+		return enable_irq_wake(info->virq);
+	}
+
+	return 0;
+}
+
+static int max77686_rtc_resume(struct device *dev)
+{
+	if (device_may_wakeup(dev)) {
+		struct max77686_rtc_info *info = dev_get_drvdata(dev);
+
+		return disable_irq_wake(info->virq);
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max77686_rtc_pm_ops,
+			 max77686_rtc_suspend, max77686_rtc_resume);
+
 static const struct platform_device_id rtc_id[] = {
 	{ "max77686-rtc", 0 },
 	{},
@@ -592,6 +619,7 @@ static struct platform_driver max77686_rtc_driver = {
 	.driver		= {
 		.name	= "max77686-rtc",
 		.owner	= THIS_MODULE,
+		.pm	= &max77686_rtc_pm_ops,
 	},
 	.probe		= max77686_rtc_probe,
 	.shutdown	= max77686_rtc_shutdown,
-- 
2.1.0


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

* [PATCH v10 2/6] rtc: max77686: Remove dead code for SMPL and WTSR.
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 1/6] rtc: max77686: Allow the max77686 rtc to wakeup the system Javier Martinez Canillas
@ 2014-09-19 10:26 ` Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 3/6] rtc: max77686: Fail to probe if no RTC regmap irqchip is set Javier Martinez Canillas
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

The MAX77686 RTC chip has two features called SMPL (Sudden Momentary
Power Loss) and WTSR (Watchdog Timeout and Software Resets).
Support for these features seems to be implemented in the driver but
compilation is disabled using a C pre-processor conditional.

This code has been disabled since the driver was original merged in
commit fca1dd03 ("rtc: max77686: add Maxim 77686 driver").

So, since this code has never been built, let's just remove it.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/rtc/rtc-max77686.c | 101 ---------------------------------------------
 1 file changed, 101 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index c1c6055..7bb5433 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -32,15 +32,6 @@
 #define RTC_UDR_MASK			(1 << RTC_UDR_SHIFT)
 #define RTC_RBUDR_SHIFT			4
 #define RTC_RBUDR_MASK			(1 << RTC_RBUDR_SHIFT)
-/* WTSR and SMPL Register */
-#define WTSRT_SHIFT			0
-#define SMPLT_SHIFT			2
-#define WTSR_EN_SHIFT			6
-#define SMPL_EN_SHIFT			7
-#define WTSRT_MASK			(3 << WTSRT_SHIFT)
-#define SMPLT_MASK			(3 << SMPLT_SHIFT)
-#define WTSR_EN_MASK			(1 << WTSR_EN_SHIFT)
-#define SMPL_EN_MASK			(1 << SMPL_EN_SHIFT)
 /* RTC Hour register */
 #define HOUR_PM_SHIFT			6
 #define HOUR_PM_MASK			(1 << HOUR_PM_SHIFT)
@@ -49,7 +40,6 @@
 #define ALARM_ENABLE_MASK		(1 << ALARM_ENABLE_SHIFT)
 
 #define MAX77686_RTC_UPDATE_DELAY	16
-#undef MAX77686_RTC_WTSR_SMPL
 
 enum {
 	RTC_SEC = 0,
@@ -412,64 +402,6 @@ static const struct rtc_class_ops max77686_rtc_ops = {
 	.alarm_irq_enable = max77686_rtc_alarm_irq_enable,
 };
 
-#ifdef MAX77686_RTC_WTSR_SMPL
-static void max77686_rtc_enable_wtsr(struct max77686_rtc_info *info, bool enable)
-{
-	int ret;
-	unsigned int val, mask;
-
-	if (enable)
-		val = (1 << WTSR_EN_SHIFT) | (3 << WTSRT_SHIFT);
-	else
-		val = 0;
-
-	mask = WTSR_EN_MASK | WTSRT_MASK;
-
-	dev_info(info->dev, "%s: %s WTSR\n", __func__,
-			enable ? "enable" : "disable");
-
-	ret = regmap_update_bits(info->max77686->rtc_regmap,
-				 MAX77686_WTSR_SMPL_CNTL, mask, val);
-	if (ret < 0) {
-		dev_err(info->dev, "%s: fail to update WTSR reg(%d)\n",
-				__func__, ret);
-		return;
-	}
-
-	max77686_rtc_update(info, MAX77686_RTC_WRITE);
-}
-
-static void max77686_rtc_enable_smpl(struct max77686_rtc_info *info, bool enable)
-{
-	int ret;
-	unsigned int val, mask;
-
-	if (enable)
-		val = (1 << SMPL_EN_SHIFT) | (0 << SMPLT_SHIFT);
-	else
-		val = 0;
-
-	mask = SMPL_EN_MASK | SMPLT_MASK;
-
-	dev_info(info->dev, "%s: %s SMPL\n", __func__,
-			enable ? "enable" : "disable");
-
-	ret = regmap_update_bits(info->max77686->rtc_regmap,
-				 MAX77686_WTSR_SMPL_CNTL, mask, val);
-	if (ret < 0) {
-		dev_err(info->dev, "%s: fail to update SMPL reg(%d)\n",
-				__func__, ret);
-		return;
-	}
-
-	max77686_rtc_update(info, MAX77686_RTC_WRITE);
-
-	val = 0;
-	regmap_read(info->max77686->rtc_regmap, MAX77686_WTSR_SMPL_CNTL, &val);
-	dev_info(info->dev, "%s: WTSR_SMPL(0x%02x)\n", __func__, val);
-}
-#endif /* MAX77686_RTC_WTSR_SMPL */
-
 static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
 {
 	u8 data[2];
@@ -519,11 +451,6 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
-#ifdef MAX77686_RTC_WTSR_SMPL
-	max77686_rtc_enable_wtsr(info, true);
-	max77686_rtc_enable_smpl(info, true);
-#endif
-
 	device_init_wakeup(&pdev->dev, 1);
 
 	info->rtc_dev = devm_rtc_device_register(&pdev->dev, "max77686-rtc",
@@ -556,33 +483,6 @@ err_rtc:
 	return ret;
 }
 
-static void max77686_rtc_shutdown(struct platform_device *pdev)
-{
-#ifdef MAX77686_RTC_WTSR_SMPL
-	struct max77686_rtc_info *info = platform_get_drvdata(pdev);
-	int i;
-	u8 val = 0;
-
-	for (i = 0; i < 3; i++) {
-		max77686_rtc_enable_wtsr(info, false);
-		regmap_read(info->max77686->rtc_regmap, MAX77686_WTSR_SMPL_CNTL, &val);
-		dev_info(info->dev, "%s: WTSR_SMPL reg(0x%02x)\n", __func__,
-				val);
-		if (val & WTSR_EN_MASK) {
-			dev_emerg(info->dev, "%s: fail to disable WTSR\n",
-					__func__);
-		} else {
-			dev_info(info->dev, "%s: success to disable WTSR\n",
-					__func__);
-			break;
-		}
-	}
-
-	/* Disable SMPL when power off */
-	max77686_rtc_enable_smpl(info, false);
-#endif /* MAX77686_RTC_WTSR_SMPL */
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int max77686_rtc_suspend(struct device *dev)
 {
@@ -622,7 +522,6 @@ static struct platform_driver max77686_rtc_driver = {
 		.pm	= &max77686_rtc_pm_ops,
 	},
 	.probe		= max77686_rtc_probe,
-	.shutdown	= max77686_rtc_shutdown,
 	.id_table	= rtc_id,
 };
 
-- 
2.1.0


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

* [PATCH v10 3/6] rtc: max77686: Fail to probe if no RTC regmap irqchip is set
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 1/6] rtc: max77686: Allow the max77686 rtc to wakeup the system Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 2/6] rtc: max77686: Remove dead code for SMPL and WTSR Javier Martinez Canillas
@ 2014-09-19 10:26 ` Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 4/6] rtc: max77686: Remove unneded info log Javier Martinez Canillas
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

The max77686 mfd driver adds a regmap IRQ chip which creates an
IRQ domain that is used to map the virtual RTC alarm1 interrupt.

The RTC driver assumes that this will always be true since the
PMIC IRQ is a required property according to the max77686 DT
binding doc. If an "interrupts" property is not defined for a
max77686 PMIC, then the mfd probe function will fail and the
RTC platform driver will never be probed. But even when it is
not possible to probe the rtc-max77686 driver without a regmap
IRQ chip, it's better to explicitly check if the IRQ chip data
is not NULL and gracefully fail instead of getting an OOPS.

Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---

Fixes the issue reported by Krzystof in: https://lkml.org/lkml/2014/8/8/121

 drivers/rtc/rtc-max77686.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 7bb5433..55396bb 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -466,6 +466,12 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
+	if (!max77686->rtc_irq_data) {
+		ret = -EINVAL;
+		dev_err(&pdev->dev, "%s: no RTC regmap IRQ chip\n", __func__);
+		goto err_rtc;
+	}
+
 	info->virq = regmap_irq_get_virq(max77686->rtc_irq_data,
 					 MAX77686_RTCIRQ_RTCA1);
 	if (!info->virq) {
-- 
2.1.0


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

* [PATCH v10 4/6] rtc: max77686: Remove unneded info log
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
                   ` (2 preceding siblings ...)
  2014-09-19 10:26 ` [PATCH v10 3/6] rtc: max77686: Fail to probe if no RTC regmap irqchip is set Javier Martinez Canillas
@ 2014-09-19 10:26 ` Javier Martinez Canillas
  2014-09-19 10:26 ` [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday Javier Martinez Canillas
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

If devm_rtc_device_register() fails a dev_err() is already
reported so there is no need to do an additional dev_info().

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/rtc/rtc-max77686.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 55396bb..b177ba1 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -457,8 +457,6 @@ static int max77686_rtc_probe(struct platform_device *pdev)
 					&max77686_rtc_ops, THIS_MODULE);
 
 	if (IS_ERR(info->rtc_dev)) {
-		dev_info(&pdev->dev, "%s: fail\n", __func__);
-
 		ret = PTR_ERR(info->rtc_dev);
 		dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);
 		if (ret == 0)
-- 
2.1.0


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

* [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
                   ` (3 preceding siblings ...)
  2014-09-19 10:26 ` [PATCH v10 4/6] rtc: max77686: Remove unneded info log Javier Martinez Canillas
@ 2014-09-19 10:26 ` Javier Martinez Canillas
  2014-09-19 14:39   ` Joe Perches
  2014-09-19 10:26 ` [PATCH v10 6/6] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock Javier Martinez Canillas
  2014-09-20  0:14 ` [PATCH v10 0/6] Add Maxim 77802 RTC support Doug Anderson
  6 siblings, 1 reply; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

The function max77686_rtc_calculate_wday() is used to
calculate the day of the week to be filled in struct
rtc_time but that function only calculates the number
of bits shifted. So the ffs() function can be used to
find the first bit set instead of a special function.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/rtc/rtc-max77686.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index b177ba1..2659bd3 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -70,16 +70,6 @@ enum MAX77686_RTC_OP {
 	MAX77686_RTC_READ,
 };
 
-static inline int max77686_rtc_calculate_wday(u8 shifted)
-{
-	int counter = -1;
-	while (shifted) {
-		shifted >>= 1;
-		counter++;
-	}
-	return counter;
-}
-
 static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
 				   int rtc_24hr_mode)
 {
@@ -93,7 +83,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
 			tm->tm_hour += 12;
 	}
 
-	tm->tm_wday = max77686_rtc_calculate_wday(data[RTC_WEEKDAY] & 0x7f);
+	tm->tm_wday = ffs(data[RTC_WEEKDAY] & 0x7f) - 1;
 	tm->tm_mday = data[RTC_DATE] & 0x1f;
 	tm->tm_mon = (data[RTC_MONTH] & 0x0f) - 1;
 	tm->tm_year = (data[RTC_YEAR] & 0x7f) + 100;
-- 
2.1.0


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

* [PATCH v10 6/6] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
                   ` (4 preceding siblings ...)
  2014-09-19 10:26 ` [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday Javier Martinez Canillas
@ 2014-09-19 10:26 ` Javier Martinez Canillas
  2014-09-20  0:14 ` [PATCH v10 0/6] Add Maxim 77802 RTC support Doug Anderson
  6 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 10:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Alessandro Zummo, Doug Anderson, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel,
	Javier Martinez Canillas

The MAX7802 PMIC has a Real-Time-Clock (RTC) with two alarms.
This patch adds support for the RTC and is based on a driver
added by Simon Glass to the Chrome OS kernel 3.8 tree.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---

Changes since v9:
 - Use ffs() to calculate tm_wday instead of a custom function.
   Suggested by Andrew Morton.

Changes since v8: None

Changes since v7: None

Changes since v6:
 - Remove unused code for SMPL and WTSR. Suggested by Krzysztof Kozlowski.
 - Don't spam the kernel log with unnecessarily info and just print for debug.
   Suggested by Krzysztof Kozlowski.
 - Use ARRAY_SIZE() instead of constant value. Suggested by Krzysztof Kozlowski.
 - Remove duplicated register setup. Suggested by Krzysztof Kozlowski.
 - Don't free/unregister managed allocated resources.
   Suggested by Krzysztof Kozlowski.

 drivers/rtc/Kconfig        |  10 +
 drivers/rtc/Makefile       |   1 +
 drivers/rtc/rtc-max77802.c | 501 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 512 insertions(+)
 create mode 100644 drivers/rtc/rtc-max77802.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index fae9464..241b8be 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -288,6 +288,16 @@ config RTC_DRV_MAX77686
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max77686.
 
+config RTC_DRV_MAX77802
+	tristate "Maxim 77802 RTC"
+	depends on MFD_MAX77686
+	help
+	  If you say yes here you will get support for the
+	  RTC of Maxim MAX77802 PMIC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-max77802.
+
 config RTC_DRV_RS5C372
 	tristate "Ricoh R2025S/D, RS5C372A/B, RV5C386, RV5C387A"
 	help
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 56f061c..d55ebe2 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_RTC_DRV_MAX8998)	+= rtc-max8998.o
 obj-$(CONFIG_RTC_DRV_MAX8997)	+= rtc-max8997.o
 obj-$(CONFIG_RTC_DRV_MAX6902)	+= rtc-max6902.o
 obj-$(CONFIG_RTC_DRV_MAX77686)	+= rtc-max77686.o
+obj-$(CONFIG_RTC_DRV_MAX77802)  += rtc-max77802.o
 obj-$(CONFIG_RTC_DRV_MC13XXX)	+= rtc-mc13xxx.o
 obj-$(CONFIG_RTC_DRV_MCP795)	+= rtc-mcp795.o
 obj-$(CONFIG_RTC_DRV_MSM6242)	+= rtc-msm6242.o
diff --git a/drivers/rtc/rtc-max77802.c b/drivers/rtc/rtc-max77802.c
new file mode 100644
index 0000000..3a00ec2
--- /dev/null
+++ b/drivers/rtc/rtc-max77802.c
@@ -0,0 +1,501 @@
+/*
+ * RTC driver for Maxim MAX77802
+ *
+ * Copyright (C) 2013 Google, Inc
+ *
+ * Copyright (C) 2012 Samsung Electronics Co.Ltd
+ *
+ *  based on rtc-max8997.c
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/rtc.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/max77686-private.h>
+#include <linux/irqdomain.h>
+#include <linux/regmap.h>
+
+/* RTC Control Register */
+#define BCD_EN_SHIFT			0
+#define BCD_EN_MASK			(1 << BCD_EN_SHIFT)
+#define MODEL24_SHIFT			1
+#define MODEL24_MASK			(1 << MODEL24_SHIFT)
+/* RTC Update Register1 */
+#define RTC_UDR_SHIFT			0
+#define RTC_UDR_MASK			(1 << RTC_UDR_SHIFT)
+#define RTC_RBUDR_SHIFT			4
+#define RTC_RBUDR_MASK			(1 << RTC_RBUDR_SHIFT)
+/* RTC Hour register */
+#define HOUR_PM_SHIFT			6
+#define HOUR_PM_MASK			(1 << HOUR_PM_SHIFT)
+/* RTC Alarm Enable */
+#define ALARM_ENABLE_SHIFT		7
+#define ALARM_ENABLE_MASK		(1 << ALARM_ENABLE_SHIFT)
+
+/* For the RTCAE1 register, we write this value to enable the alarm */
+#define ALARM_ENABLE_VALUE		0x77
+
+#define MAX77802_RTC_UPDATE_DELAY_US	200
+
+enum {
+	RTC_SEC = 0,
+	RTC_MIN,
+	RTC_HOUR,
+	RTC_WEEKDAY,
+	RTC_MONTH,
+	RTC_YEAR,
+	RTC_DATE,
+	RTC_NR_TIME
+};
+
+struct max77802_rtc_info {
+	struct device		*dev;
+	struct max77686_dev	*max77802;
+	struct i2c_client	*rtc;
+	struct rtc_device	*rtc_dev;
+	struct mutex		lock;
+
+	struct regmap		*regmap;
+
+	int virq;
+	int rtc_24hr_mode;
+};
+
+enum MAX77802_RTC_OP {
+	MAX77802_RTC_WRITE,
+	MAX77802_RTC_READ,
+};
+
+static void max77802_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
+				   int rtc_24hr_mode)
+{
+	tm->tm_sec = data[RTC_SEC] & 0xff;
+	tm->tm_min = data[RTC_MIN] & 0xff;
+	if (rtc_24hr_mode)
+		tm->tm_hour = data[RTC_HOUR] & 0x1f;
+	else {
+		tm->tm_hour = data[RTC_HOUR] & 0x0f;
+		if (data[RTC_HOUR] & HOUR_PM_MASK)
+			tm->tm_hour += 12;
+	}
+
+	tm->tm_wday = ffs(data[RTC_WEEKDAY] & 0xff) - 1;
+	tm->tm_mday = data[RTC_DATE] & 0x1f;
+	tm->tm_mon = (data[RTC_MONTH] & 0x0f) - 1;
+
+	tm->tm_year = data[RTC_YEAR] & 0xff;
+	tm->tm_yday = 0;
+	tm->tm_isdst = 0;
+}
+
+static int max77802_rtc_tm_to_data(struct rtc_time *tm, u8 *data)
+{
+	data[RTC_SEC] = tm->tm_sec;
+	data[RTC_MIN] = tm->tm_min;
+	data[RTC_HOUR] = tm->tm_hour;
+	data[RTC_WEEKDAY] = 1 << tm->tm_wday;
+	data[RTC_DATE] = tm->tm_mday;
+	data[RTC_MONTH] = tm->tm_mon + 1;
+	data[RTC_YEAR] = tm->tm_year;
+
+	return 0;
+}
+
+static int max77802_rtc_update(struct max77802_rtc_info *info,
+	enum MAX77802_RTC_OP op)
+{
+	int ret;
+	unsigned int data;
+
+	if (op == MAX77802_RTC_WRITE)
+		data = 1 << RTC_UDR_SHIFT;
+	else
+		data = 1 << RTC_RBUDR_SHIFT;
+
+	ret = regmap_update_bits(info->max77802->regmap,
+				 MAX77802_RTC_UPDATE0, data, data);
+	if (ret < 0)
+		dev_err(info->dev, "%s: fail to write update reg(ret=%d, data=0x%x)\n",
+				__func__, ret, data);
+	else {
+		/* Minimum delay required before RTC update. */
+		usleep_range(MAX77802_RTC_UPDATE_DELAY_US,
+			     MAX77802_RTC_UPDATE_DELAY_US * 2);
+	}
+
+	return ret;
+}
+
+static int max77802_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct max77802_rtc_info *info = dev_get_drvdata(dev);
+	u8 data[RTC_NR_TIME];
+	int ret;
+
+	mutex_lock(&info->lock);
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_READ);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_bulk_read(info->max77802->regmap,
+				MAX77802_RTC_SEC, data, RTC_NR_TIME);
+	if (ret < 0) {
+		dev_err(info->dev, "%s: fail to read time reg(%d)\n", __func__,
+			ret);
+		goto out;
+	}
+
+	max77802_rtc_data_to_tm(data, tm, info->rtc_24hr_mode);
+
+	ret = rtc_valid_tm(tm);
+
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int max77802_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct max77802_rtc_info *info = dev_get_drvdata(dev);
+	u8 data[RTC_NR_TIME];
+	int ret;
+
+	ret = max77802_rtc_tm_to_data(tm, data);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&info->lock);
+
+	ret = regmap_bulk_write(info->max77802->regmap,
+				 MAX77802_RTC_SEC, data, RTC_NR_TIME);
+	if (ret < 0) {
+		dev_err(info->dev, "%s: fail to write time reg(%d)\n", __func__,
+			ret);
+		goto out;
+	}
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_WRITE);
+
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int max77802_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct max77802_rtc_info *info = dev_get_drvdata(dev);
+	u8 data[RTC_NR_TIME];
+	unsigned int val;
+	int ret;
+
+	mutex_lock(&info->lock);
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_READ);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_bulk_read(info->max77802->regmap,
+				 MAX77802_ALARM1_SEC, data, RTC_NR_TIME);
+	if (ret < 0) {
+		dev_err(info->dev, "%s:%d fail to read alarm reg(%d)\n",
+				__func__, __LINE__, ret);
+		goto out;
+	}
+
+	max77802_rtc_data_to_tm(data, &alrm->time, info->rtc_24hr_mode);
+
+	alrm->enabled = 0;
+	ret = regmap_read(info->max77802->regmap,
+			  MAX77802_RTC_AE1, &val);
+	if (ret < 0) {
+		dev_err(info->dev, "%s:%d fail to read alarm enable(%d)\n",
+			__func__, __LINE__, ret);
+		goto out;
+	}
+	if (val)
+		alrm->enabled = 1;
+
+	alrm->pending = 0;
+	ret = regmap_read(info->max77802->regmap, MAX77802_REG_STATUS2, &val);
+	if (ret < 0) {
+		dev_err(info->dev, "%s:%d fail to read status2 reg(%d)\n",
+				__func__, __LINE__, ret);
+		goto out;
+	}
+
+	if (val & (1 << 2)) /* RTCA1 */
+		alrm->pending = 1;
+
+out:
+	mutex_unlock(&info->lock);
+	return 0;
+}
+
+static int max77802_rtc_stop_alarm(struct max77802_rtc_info *info)
+{
+	int ret;
+
+	if (!mutex_is_locked(&info->lock))
+		dev_warn(info->dev, "%s: should have mutex locked\n", __func__);
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_READ);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_write(info->max77802->regmap,
+			   MAX77802_RTC_AE1, 0);
+	if (ret < 0) {
+		dev_err(info->dev, "%s: fail to write alarm reg(%d)\n",
+			__func__, ret);
+		goto out;
+	}
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_WRITE);
+out:
+	return ret;
+}
+
+static int max77802_rtc_start_alarm(struct max77802_rtc_info *info)
+{
+	int ret;
+
+	if (!mutex_is_locked(&info->lock))
+		dev_warn(info->dev, "%s: should have mutex locked\n",
+			 __func__);
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_READ);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_write(info->max77802->regmap,
+				   MAX77802_RTC_AE1,
+				   ALARM_ENABLE_VALUE);
+
+	if (ret < 0) {
+		dev_err(info->dev, "%s: fail to read alarm reg(%d)\n",
+				__func__, ret);
+		goto out;
+	}
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_WRITE);
+out:
+	return ret;
+}
+
+static int max77802_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct max77802_rtc_info *info = dev_get_drvdata(dev);
+	u8 data[RTC_NR_TIME];
+	int ret;
+
+	ret = max77802_rtc_tm_to_data(&alrm->time, data);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&info->lock);
+
+	ret = max77802_rtc_stop_alarm(info);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_bulk_write(info->max77802->regmap,
+				 MAX77802_ALARM1_SEC, data, RTC_NR_TIME);
+
+	if (ret < 0) {
+		dev_err(info->dev, "%s: fail to write alarm reg(%d)\n",
+				__func__, ret);
+		goto out;
+	}
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_WRITE);
+	if (ret < 0)
+		goto out;
+
+	if (alrm->enabled)
+		ret = max77802_rtc_start_alarm(info);
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int max77802_rtc_alarm_irq_enable(struct device *dev,
+					 unsigned int enabled)
+{
+	struct max77802_rtc_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	mutex_lock(&info->lock);
+	if (enabled)
+		ret = max77802_rtc_start_alarm(info);
+	else
+		ret = max77802_rtc_stop_alarm(info);
+	mutex_unlock(&info->lock);
+
+	return ret;
+}
+
+static irqreturn_t max77802_rtc_alarm_irq(int irq, void *data)
+{
+	struct max77802_rtc_info *info = data;
+
+	dev_dbg(info->dev, "%s:irq(%d)\n", __func__, irq);
+
+	rtc_update_irq(info->rtc_dev, 1, RTC_IRQF | RTC_AF);
+
+	return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops max77802_rtc_ops = {
+	.read_time = max77802_rtc_read_time,
+	.set_time = max77802_rtc_set_time,
+	.read_alarm = max77802_rtc_read_alarm,
+	.set_alarm = max77802_rtc_set_alarm,
+	.alarm_irq_enable = max77802_rtc_alarm_irq_enable,
+};
+
+static int max77802_rtc_init_reg(struct max77802_rtc_info *info)
+{
+	u8 data[2];
+	int ret;
+
+	max77802_rtc_update(info, MAX77802_RTC_READ);
+
+	/* Set RTC control register : Binary mode, 24hour mdoe */
+	data[0] = (1 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
+	data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
+
+	info->rtc_24hr_mode = 1;
+
+	ret = regmap_bulk_write(info->max77802->regmap,
+				MAX77802_RTC_CONTROLM, data, ARRAY_SIZE(data));
+	if (ret < 0) {
+		dev_err(info->dev, "%s: fail to write controlm reg(%d)\n",
+				__func__, ret);
+		return ret;
+	}
+
+	ret = max77802_rtc_update(info, MAX77802_RTC_WRITE);
+	return ret;
+}
+
+static int max77802_rtc_probe(struct platform_device *pdev)
+{
+	struct max77686_dev *max77802 = dev_get_drvdata(pdev->dev.parent);
+	struct max77802_rtc_info *info;
+	int ret;
+
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	info = devm_kzalloc(&pdev->dev, sizeof(struct max77802_rtc_info),
+			    GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	mutex_init(&info->lock);
+	info->dev = &pdev->dev;
+	info->max77802 = max77802;
+	info->rtc = max77802->i2c;
+
+	platform_set_drvdata(pdev, info);
+
+	ret = max77802_rtc_init_reg(info);
+
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to initialize RTC reg:%d\n", ret);
+		return ret;
+	}
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	info->rtc_dev = devm_rtc_device_register(&pdev->dev, "max77802-rtc",
+						 &max77802_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(info->rtc_dev)) {
+		ret = PTR_ERR(info->rtc_dev);
+		dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);
+		if (ret == 0)
+			ret = -EINVAL;
+		return ret;
+	}
+
+	if (!max77802->rtc_irq_data) {
+		dev_err(&pdev->dev, "No RTC regmap IRQ chip\n");
+		return -EINVAL;
+	}
+
+	info->virq = regmap_irq_get_virq(max77802->rtc_irq_data,
+					 MAX77686_RTCIRQ_RTCA1);
+
+	if (info->virq <= 0) {
+		dev_err(&pdev->dev, "Failed to get virtual IRQ %d\n",
+			MAX77686_RTCIRQ_RTCA1);
+		return -EINVAL;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, info->virq, NULL,
+					max77802_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);
+
+	return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max77802_rtc_suspend(struct device *dev)
+{
+	if (device_may_wakeup(dev)) {
+		struct max77802_rtc_info *info = dev_get_drvdata(dev);
+
+		return enable_irq_wake(info->virq);
+	}
+
+	return 0;
+}
+
+static int max77802_rtc_resume(struct device *dev)
+{
+	if (device_may_wakeup(dev)) {
+		struct max77802_rtc_info *info = dev_get_drvdata(dev);
+
+		return disable_irq_wake(info->virq);
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max77802_rtc_pm_ops,
+			 max77802_rtc_suspend, max77802_rtc_resume);
+
+static const struct platform_device_id rtc_id[] = {
+	{ "max77802-rtc", 0 },
+	{},
+};
+
+static struct platform_driver max77802_rtc_driver = {
+	.driver		= {
+		.name	= "max77802-rtc",
+		.owner	= THIS_MODULE,
+		.pm	= &max77802_rtc_pm_ops,
+	},
+	.probe		= max77802_rtc_probe,
+	.id_table	= rtc_id,
+};
+
+module_platform_driver(max77802_rtc_driver);
+
+MODULE_DESCRIPTION("Maxim MAX77802 RTC driver");
+MODULE_AUTHOR("Simon Glass <sjg@chromium.org>");
+MODULE_LICENSE("GPL");
-- 
2.1.0


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

* Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday
  2014-09-19 10:26 ` [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday Javier Martinez Canillas
@ 2014-09-19 14:39   ` Joe Perches
  2014-09-19 19:27     ` Javier Martinez Canillas
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2014-09-19 14:39 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Andrew Morton, Alessandro Zummo, Doug Anderson,
	Krzysztof Kozlowski, rtc-linux, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
> The function max77686_rtc_calculate_wday() is used to
> calculate the day of the week to be filled in struct
> rtc_time but that function only calculates the number
> of bits shifted. So the ffs() function can be used to
> find the first bit set instead of a special function.

This isn't the same logic.  Perhaps you want fls.

> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
[]
> -static inline int max77686_rtc_calculate_wday(u8 shifted)
> -{
> -	int counter = -1;
> -	while (shifted) {
> -		shifted >>= 1;
> -		counter++;
> -	}
> -	return counter;
> -}
> -
>  static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
>  				   int rtc_24hr_mode)
>  {
> @@ -93,7 +83,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
>  			tm->tm_hour += 12;
>  	}
>  
> -	tm->tm_wday = max77686_rtc_calculate_wday(data[RTC_WEEKDAY] & 0x7f);
> +	tm->tm_wday = ffs(data[RTC_WEEKDAY] & 0x7f) - 1;




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

* Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday
  2014-09-19 14:39   ` Joe Perches
@ 2014-09-19 19:27     ` Javier Martinez Canillas
  2014-09-19 19:52       ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 19:27 UTC (permalink / raw)
  To: Joe Perches
  Cc: Andrew Morton, Alessandro Zummo, Doug Anderson,
	Krzysztof Kozlowski, rtc-linux, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

Hello Joe,

On 09/19/2014 04:39 PM, Joe Perches wrote:
> On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
>> The function max77686_rtc_calculate_wday() is used to
>> calculate the day of the week to be filled in struct
>> rtc_time but that function only calculates the number
>> of bits shifted. So the ffs() function can be used to
>> find the first bit set instead of a special function.
> 
> This isn't the same logic.  Perhaps you want fls.
>

Right, the removed function has the same logic than fls() - 1 but the value
stored in data[RTC_WEEKDAY] is:

data[RTC_WEEKDAY] = 1 << tm->tm_wday;

so for this particular case, it doesn't matter since ffs() == fls() always.

>> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> []
>> -static inline int max77686_rtc_calculate_wday(u8 shifted)
>> -{
>> -	int counter = -1;
>> -	while (shifted) {
>> -		shifted >>= 1;
>> -		counter++;
>> -	}
>> -	return counter;
>> -}
>> -
>>  static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
>>  				   int rtc_24hr_mode)
>>  {
>> @@ -93,7 +83,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
>>  			tm->tm_hour += 12;
>>  	}
>>  
>> -	tm->tm_wday = max77686_rtc_calculate_wday(data[RTC_WEEKDAY] & 0x7f);
>> +	tm->tm_wday = ffs(data[RTC_WEEKDAY] & 0x7f) - 1;

I did wonder which function to use and the question is when you want to know
the number of shifted bits, do you look for the first or the last bit set?

Of course is the same for powers of two but I did a naive search to have an
usage count:

$ git grep "shift = ffs(" | wc -l
39
$ git grep "shift = fls(" | wc -l
17

so it seems that ffs() is twice more popular than fls() for this case so I
decided to use ffs() but I don't have a strong opinion tbh.

Best regards,
Javier

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

* Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday
  2014-09-19 19:27     ` Javier Martinez Canillas
@ 2014-09-19 19:52       ` Joe Perches
  2014-09-19 21:44         ` Javier Martinez Canillas
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2014-09-19 19:52 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Andrew Morton, Alessandro Zummo, Doug Anderson,
	Krzysztof Kozlowski, rtc-linux, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

On Fri, 2014-09-19 at 21:27 +0200, Javier Martinez Canillas wrote:
> Hello Joe,

Hello Javier.

> On 09/19/2014 04:39 PM, Joe Perches wrote:
> > On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
> >> The function max77686_rtc_calculate_wday() is used to
> >> calculate the day of the week to be filled in struct
> >> rtc_time but that function only calculates the number
> >> of bits shifted. So the ffs() function can be used to
> >> find the first bit set instead of a special function.
> > 
> > This isn't the same logic.  Perhaps you want fls.
> >
> 
> Right, the removed function has the same logic than fls() - 1 but the value
> stored in data[RTC_WEEKDAY] is:
> 
> data[RTC_WEEKDAY] = 1 << tm->tm_wday;
> 
> so for this particular case, it doesn't matter since ffs() == fls() always.

I didn't look that far.

I just wanted to show that the logic wasn't the same.

Perhaps you can specify that ffs==fls in the changelog.



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

* Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday
  2014-09-19 19:52       ` Joe Perches
@ 2014-09-19 21:44         ` Javier Martinez Canillas
  0 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2014-09-19 21:44 UTC (permalink / raw)
  To: Joe Perches
  Cc: Andrew Morton, Alessandro Zummo, Doug Anderson,
	Krzysztof Kozlowski, rtc-linux, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

Hello Joe,

On 09/19/2014 09:52 PM, Joe Perches wrote:
> On Fri, 2014-09-19 at 21:27 +0200, Javier Martinez Canillas wrote:
>> On 09/19/2014 04:39 PM, Joe Perches wrote:
>> > On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
>> >> The function max77686_rtc_calculate_wday() is used to
>> >> calculate the day of the week to be filled in struct
>> >> rtc_time but that function only calculates the number
>> >> of bits shifted. So the ffs() function can be used to
>> >> find the first bit set instead of a special function.
>> > 
>> > This isn't the same logic.  Perhaps you want fls.
>> >
>> 
>> Right, the removed function has the same logic than fls() - 1 but the value
>> stored in data[RTC_WEEKDAY] is:
>> 
>> data[RTC_WEEKDAY] = 1 << tm->tm_wday;
>> 
>> so for this particular case, it doesn't matter since ffs() == fls() always.
> 
> I didn't look that far.
> 
> I just wanted to show that the logic wasn't the same.
> 

No worries, thanks a lot for your feedback. Probably it would had been good to
mention that in the commit message.

> Perhaps you can specify that ffs==fls in the changelog.
> 

I won't say I'm thrilled to do a whole re-spin of the series just to change
that ;)

Specially since I already did too many revisions and this series have been
floating around for months but is up to Andrew to decide if is worth it.

Best regards,
Javier



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

* Re: [PATCH v10 0/6] Add Maxim 77802 RTC support
  2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
                   ` (5 preceding siblings ...)
  2014-09-19 10:26 ` [PATCH v10 6/6] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock Javier Martinez Canillas
@ 2014-09-20  0:14 ` Doug Anderson
  6 siblings, 0 replies; 12+ messages in thread
From: Doug Anderson @ 2014-09-20  0:14 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Andrew Morton, Alessandro Zummo, Krzysztof Kozlowski, rtc-linux,
	linux-samsung-soc, linux-arm-kernel, linux-kernel

Javier,

On Fri, Sep 19, 2014 at 3:26 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Hello Andrew,
>
> This series add support for the Real Time clock present in
> the Maxim 77802 Power Managment IC. The version number is
> quite high because it previously was part of a bigger series
> [0] that aimed to add support for all the devices in the
> max77802 PMIC. But now that the max77802 dependencies were
> already merged for 3.17, the series were split but I kept
> the version numbering.
>
> While working on the max77802 rtc support a lot of feedback
> was given and the issues pointed out also apply to a driver
> for a similar PMIC RTC (max77686). So patches 01/06 to 05/06
> in the series are cleanups for the max77686 driver and patch
> 06/06 adds the support for the max77802 RTC.
>
> This version address the issues pointed out on the previous
> version [1] and changelog are present on each patch when is
> applicable.
>
> The series were tested on an Exynos5250 Snow (max77686) and
> Exynos5420 Peach Pit (max77802) machines and applies cleanly
> to both 3.17-rc1 and today's linux-next (20140919).
>
> NOTE: The patches from the previous version [1] were already
> present in the -mm tree [2] so I didn't know if I should had
> sent this as a delta or as a new revision. I decided to do
> the later but please let me know if you expected the former.

My records show that Andrew has already accepted most of these.  They
may not show up in linuxnext yet, but that doesn't mean Andrew hasn't
taken them (I think).


> Doug Anderson (1):
>   rtc: max77686: Allow the max77686 rtc to wakeup the system

    http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-allow-the-max77686-rtc-to-wakeup-the-system.patch

> Javier Martinez Canillas (5):
>   rtc: max77686: Remove dead code for SMPL and WTSR.

    http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-remove-dead-code-for-smpl-and-wtsr.patch

>   rtc: max77686: Fail to probe if no RTC regmap irqchip is set

    http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-fail-to-probe-if-no-rtc-regmap-irqchip-is-set.patch

>   rtc: max77686: Remove unneded info log

    http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-remove-unneded-info-log.patch

>   rtc: max77686: Use ffs() to calculate tm_wday

This one hasn't been accepted...

>   rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock

    http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-driver-for-maxim-77802-pmic-real-time-clock.patch


-Doug

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

end of thread, other threads:[~2014-09-20  0:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 1/6] rtc: max77686: Allow the max77686 rtc to wakeup the system Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 2/6] rtc: max77686: Remove dead code for SMPL and WTSR Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 3/6] rtc: max77686: Fail to probe if no RTC regmap irqchip is set Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 4/6] rtc: max77686: Remove unneded info log Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday Javier Martinez Canillas
2014-09-19 14:39   ` Joe Perches
2014-09-19 19:27     ` Javier Martinez Canillas
2014-09-19 19:52       ` Joe Perches
2014-09-19 21:44         ` Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 6/6] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock Javier Martinez Canillas
2014-09-20  0:14 ` [PATCH v10 0/6] Add Maxim 77802 RTC support Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).