[PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[Ashish Jangam]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 10079 bytes --]

Hi Paul,

RTC Driver for Dialog Semiconductor DA9052 PMICs.

Changes made since last submission:
. read and write operation moved to MFD

Linux Kernel Version: 2.6.37

Signed-off-by: D. Chen <dchen@diasemi.com>
---
diff -Naur orig_linux-2.6.37/drivers/rtc/Kconfig linux-2.6.37/drivers/rtc/Kconfig
--- orig_linux-2.6.37/drivers/rtc/Kconfig	2011-01-05 05:50:19.000000000 +0500
+++ linux-2.6.37/drivers/rtc/Kconfig	2011-03-31 21:07:39.000000000 +0500
@@ -664,6 +664,13 @@
 	help
 	  If you say yes here you get support for the RTC subsystem of the
 	  NUC910/NUC920 used in embedded systems.
+
+config RTC_DRV_DA9052
+	tristate "Dialog DA9052 RTC"
+	depends on PMIC_DA9052
+	help
+	  Say y here to support the RTC driver for
+	  Dialog Semiconductor DA9052 PMIC.

 comment "on-CPU RTC drivers"

diff -Naur orig_linux-2.6.37/drivers/rtc/Makefile linux-2.6.37/drivers/rtc/Makefile
--- orig_linux-2.6.37/drivers/rtc/Makefile	2011-01-05 05:50:19.000000000 +0500
+++ linux-2.6.37/drivers/rtc/Makefile	2011-03-31 21:07:34.000000000 +0500
@@ -28,6 +28,7 @@
 obj-$(CONFIG_RTC_DRV_BQ4802)		+= rtc-bq4802.o
 obj-$(CONFIG_RTC_DRV_CMOS)		+= rtc-cmos.o
 obj-$(CONFIG_RTC_DRV_COH901331)	+= rtc-coh901331.o
+obj-$(CONFIG_RTC_DRV_DA9052)		+= rtc-da9052.o
 obj-$(CONFIG_RTC_DRV_DAVINCI)	+= rtc-davinci.o
 obj-$(CONFIG_RTC_DRV_DM355EVM)	+= rtc-dm355evm.o
 obj-$(CONFIG_RTC_DRV_DS1216)		+= rtc-ds1216.o
diff -Naur orig_linux-2.6.37/drivers/rtc/rtc-da9052.c linux-2.6.37/drivers/rtc/rtc-da9052.c
--- orig_linux-2.6.37/drivers/rtc/rtc-da9052.c	1970-01-01 05:00:00.000000000 +0500
+++ linux-2.6.37/drivers/rtc/rtc-da9052.c	2011-03-31 21:07:47.000000000 +0500
@@ -0,0 +1,338 @@
+/*
+ *rtc-da9052.c: Real time clock driver for DA9052
+ *
+ *Copyright(c) 2009 Dialog Semiconductor Ltd.
+ *
+ *Author: Dajun Chen <dajun.chen@diasemi.com>
+ *
+ * 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/platform_device.h>
+#include <linux/rtc.h>
+
+#include <linux/mfd/da9052/da9052.h>
+#include <linux/mfd/da9052/reg.h>
+
+struct da9052_rtc {
+	struct rtc_device *rtc;
+	struct da9052 *da9052;
+	int irq;
+};
+
+static int da9052_rtc_enable_alarm(struct da9052 *da9052, unsigned char flag)
+{
+	int ret = 0;
+	if (flag) {
+		ret = da9052_set_bits(da9052, DA9052_ALARM_Y_REG,
+					DA9052_ALARM_Y_ALARM_ON);
+		if (ret != 0)
+			dev_err(da9052->dev, "Failed to enable ALM: %d\n", ret);
+	} else {
+		ret = da9052_clear_bits(da9052, DA9052_ALARM_Y_REG,
+					DA9052_ALARM_Y_ALARM_ON);
+		if (ret != 0)
+			dev_err(da9052->dev, "da9052_rtc_enable_alarm -> \
+					da9052_clear_bits error %d\n", ret);
+	}
+	return ret;
+}
+
+static irqreturn_t da9052_rtc_irq(int irq, void *data)
+{
+	struct da9052_rtc *rtc = (struct da9052_rtc *)data;
+	int ret = 0;
+
+	ret = da9052_reg_read(rtc->da9052, DA9052_ALARM_MI_REG);
+	if (ret < 0) {
+		dev_err(rtc->da9052->dev, "da9052_rtc_notifier -> \
+					da9052_reg_read error %d\n", ret);
+		return IRQ_NONE;
+	}
+	if (ret & DA9052_ALARMMI_ALARMTYPE)
+		da9052_rtc_enable_alarm(rtc->da9052, 0);
+
+	return IRQ_HANDLED;
+}
+
+static int da9052_read_alarm(struct da9052 *da9052, struct rtc_time *rtc_tm)
+{
+
+	int ret = 0;
+	uint8_t v[5] = {0, 0, 0, 0, 0};
+	ret = da9052_group_read(da9052, DA9052_ALARM_MI_REG, 5, v);
+	if (ret != 0) {
+		dev_err(da9052->dev, "Failed to group read ALM: %d\n", ret);
+		return ret;
+	}
+
+	rtc_tm->tm_year = v[4] & DA9052_RTC_YEAR;
+	rtc_tm->tm_mon  = v[3] & DA9052_RTC_MONTH;
+	rtc_tm->tm_mday = v[2] & DA9052_RTC_DAY;
+	rtc_tm->tm_hour = v[1] & DA9052_RTC_HOUR;
+	rtc_tm->tm_min  = v[0] & DA9052_RTC_MIN;
+
+	ret = rtc_valid_tm(rtc_tm);
+	if (ret != 0)
+		return ret;
+
+	rtc_tm->tm_year += 100;
+	rtc_tm->tm_mon -= 1;
+
+	return ret;
+}
+
+static int da9052_set_alarm(struct da9052 *da9052, struct rtc_time *rtc_tm)
+{
+	int ret = 0;
+	uint8_t v[3] = {0, 0, 0};
+
+	rtc_tm->tm_sec = 0;
+	rtc_tm->tm_year -= 100;
+	rtc_tm->tm_mon += 1;
+
+	ret = rtc_valid_tm(rtc_tm);
+	if (ret)
+		return ret;
+
+	ret = da9052_reg_update(da9052, DA9052_ALARM_MI_REG,
+				DA9052_RTC_MIN, rtc_tm->tm_min);
+	if (ret != 0) {
+		dev_err(da9052->dev, "Failed to write ALRM MIN: %d\n", ret);
+		return ret;
+	}
+
+	ret = da9052_reg_update(da9052, DA9052_ALARM_Y_REG,
+				DA9052_RTC_YEAR, rtc_tm->tm_year);
+	if (ret != 0) {
+		dev_err(da9052->dev, "Failed to write ALRM YEAR: %d\n", ret);
+		return ret;
+	}
+
+	v[0] = rtc_tm->tm_hour;
+	v[1] = rtc_tm->tm_mday;
+	v[2] = rtc_tm->tm_mon;
+
+	return da9052_group_write(da9052, DA9052_ALARM_H_REG, 3, v);
+}
+
+static int da9052_rtc_get_alarm_status(struct da9052 *da9052)
+{
+	int ret = 0;
+	ret = da9052_reg_read(da9052, DA9052_ALARM_Y_REG);
+	if (ret < 0) {
+		dev_err(da9052->dev, "Failed to read ALM: %d\n", ret);
+		return ret;
+	}
+	ret &= DA9052_ALARM_Y_ALARM_ON;
+	return (ret > 0) ? 1 : 0;
+}
+
+static int da9052_rtc_read_time
+			(struct device *dev, struct rtc_time *rtc_tm)
+{
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+	uint8_t v[6] = {0, 0, 0, 0, 0, 0};
+	int ret;
+
+	ret = da9052_group_read(rtc->da9052, DA9052_COUNT_S_REG, 6, v);
+	if (ret != 0) {
+		dev_err(rtc->da9052->dev, "Failed to read \
+			RTC time : %d\n", ret);
+		return ret;
+	}
+
+	rtc_tm->tm_year = v[5] & DA9052_RTC_YEAR;
+	rtc_tm->tm_mon  = v[4] & DA9052_RTC_MONTH;
+	rtc_tm->tm_mday = v[3] & DA9052_RTC_DAY;
+	rtc_tm->tm_hour = v[2] & DA9052_RTC_HOUR;
+	rtc_tm->tm_min  = v[1] & DA9052_RTC_MIN;
+	rtc_tm->tm_sec  = v[0] & DA9052_RTC_SEC;
+
+	ret = rtc_valid_tm(rtc_tm);
+	if (ret != 0) {
+		dev_err(rtc->da9052->dev,
+		"da9052_rtc_read_time -> rtc_valid_tm failed %d\n", ret);
+		return ret;
+	}
+
+	rtc_tm->tm_year += 100;
+	rtc_tm->tm_mon -= 1;
+
+	return 0;
+}
+
+
+static int da9052_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	int ret;
+	struct da9052_rtc *rtc;
+	uint8_t v[6] = {0, 0, 0, 0, 0, 0};
+//	struct rtc *rtc = dev_get_drvdata(dev);
+
+	rtc = dev_get_drvdata(dev);
+	// rtc->da9052 = rtc->dev.parent;
+
+
+	tm->tm_year -= 100;
+	tm->tm_mon += 1;
+
+	ret = rtc_valid_tm(tm);
+	if (ret != 0)
+		return ret;
+
+	v[0] = tm->tm_sec;
+	v[1] = tm->tm_min;
+	v[2] = tm->tm_hour;
+	v[3] = tm->tm_mday;
+	v[4] = tm->tm_mon;
+	v[5] = tm->tm_year;
+	return da9052_group_write(rtc->da9052, DA9052_COUNT_S_REG, 6, v);
+}
+
+static int da9052_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	int ret = 0;
+	struct rtc_time *tm = &alrm->time;
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	ret = da9052_read_alarm(rtc->da9052, tm);
+
+	if (ret)
+		return ret;
+
+	alrm->enabled = da9052_rtc_get_alarm_status(rtc->da9052);
+
+	return 0;
+}
+
+static int da9052_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	int ret = 0;
+	struct rtc_time *tm = &alrm->time;
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	ret = da9052_set_alarm(rtc->da9052, tm);
+
+	if (ret)
+		return ret;
+
+	ret = da9052_rtc_enable_alarm(rtc->da9052, 1);
+
+	return ret;
+}
+
+static int da9052_rtc_update_irq_enable(struct device *dev,
+		unsigned int enabled)
+{
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		return da9052_set_bits(rtc->da9052, DA9052_ALARM_Y_REG,
+					DA9052_ALARM_Y_TICK_ON);
+	else
+		return da9052_clear_bits(rtc->da9052, DA9052_ALARM_Y_REG,
+					 DA9052_ALARM_Y_TICK_ON);
+}
+
+static int da9052_rtc_alarm_irq_enable(struct device *dev,
+			unsigned int enabled)
+{
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		return da9052_rtc_enable_alarm(rtc->da9052, enabled);
+	else
+		return da9052_rtc_enable_alarm(rtc->da9052, enabled);
+}
+
+static const struct rtc_class_ops da9052_rtc_ops = {
+	.read_time	= da9052_rtc_read_time,
+	.set_time	= da9052_rtc_set_time,
+	.read_alarm	= da9052_rtc_read_alarm,
+	.set_alarm	= da9052_rtc_set_alarm,
+	.update_irq_enable = da9052_rtc_update_irq_enable,
+	.alarm_irq_enable = da9052_rtc_alarm_irq_enable,
+};
+
+static int __devinit da9052_rtc_probe(struct platform_device *pdev)
+{
+	struct da9052_rtc *rtc;
+	int ret = 0;
+
+	rtc = kzalloc(sizeof(struct da9052_rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	rtc->da9052 = dev_get_drvdata(pdev->dev.parent);
+	rtc->irq = platform_get_irq_byname(pdev, "ALM");
+
+	ret = da9052_request_irq(rtc->da9052, rtc->irq,
+					da9052_rtc_irq, "ALM", rtc);
+	if (ret != 0) {
+		dev_err(rtc->da9052->dev,
+			"Da9052 RTC failed irq registration: %d\n", ret);
+		goto err_mem;
+	}
+
+	da9052_set_bits(rtc->da9052, DA9052_ALARM_Y_REG,
+			DA9052_ALARM_Y_TICK_ON);
+
+	rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
+				       &da9052_rtc_ops, THIS_MODULE);
+	if (IS_ERR(rtc->rtc)) {
+		ret = PTR_ERR(rtc->rtc);
+		goto err_free_irq;
+	}
+
+	platform_set_drvdata(pdev, rtc);
+
+	return 0;
+
+err_free_irq:
+	da9052_free_irq(rtc->da9052, rtc->irq, NULL);
+err_mem:
+	kfree(rtc);
+	return ret;
+}
+
+static int __devexit da9052_rtc_remove(struct platform_device *pdev)
+{
+	struct da9052_rtc *rtc = pdev->dev.platform_data;
+
+	rtc_device_unregister(rtc->rtc);
+	da9052_free_irq(rtc->da9052, rtc->irq, NULL);
+	platform_set_drvdata(pdev, NULL);
+	kfree(rtc);
+
+	return 0;
+}
+
+static struct platform_driver da9052_rtc_driver = {
+	.driver.name	= "da9052-rtc",
+	.driver.owner	= THIS_MODULE,
+	.probe		= da9052_rtc_probe,
+	.remove		= __devexit_p(da9052_rtc_remove),
+};
+
+static int __init da9052_rtc_init(void)
+{
+	return platform_driver_register(&da9052_rtc_driver);
+}
+module_init(da9052_rtc_init);
+
+static void __exit da9052_rtc_exit(void)
+{
+	platform_driver_unregister(&da9052_rtc_driver);
+}
+module_exit(da9052_rtc_exit);
+
+MODULE_AUTHOR("Dialog Semiconductor Ltd <dchen@diasemi.com>");
+MODULE_DESCRIPTION("RTC driver for Dialog DA9052 PMIC");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:da9052-rtc");
+

Regards,
Ashish


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[Andrew Morton]

On Wed, 6 Apr 2011 18:47:29 +0530
Ashish Jangam <Ashish.Jangam@kpitcummins.com> wrote:

> Hi Paul,
> 
> RTC Driver for Dialog Semiconductor DA9052 PMICs.
> 
> Changes made since last submission:
> . read and write operation moved to MFD
> 
> Linux Kernel Version: 2.6.37

The patch looks OK(ish) to me from a quick read.

> --- orig_linux-2.6.37/drivers/rtc/Kconfig	2011-01-05 05:50:19.000000000 +0500
> +++ linux-2.6.37/drivers/rtc/Kconfig	2011-03-31 21:07:39.000000000 +0500
> @@ -664,6 +664,13 @@
>  	help
>  	  If you say yes here you get support for the RTC subsystem of the
>  	  NUC910/NUC920 used in embedded systems.
> +
> +config RTC_DRV_DA9052
> +	tristate "Dialog DA9052 RTC"
> +	depends on PMIC_DA9052
> +	help
> +	  Say y here to support the RTC driver for
> +	  Dialog Semiconductor DA9052 PMIC.

But there's not much I can do with it because PMIC_DA9052 does not
exist in mainline or in linux-next.

What is a PMIC_DA9052, anyway?  What CPU architectures support it, etc?

Have you identified a maintainer who will be merging the main patch
which enables PMIC_DA9052?


Please feed all the patches through scritps/checkpatch.pl if you haven't
already done so, to clean up lots of trivial errors.

For example, "MFD: MFD module of DA9052 PMIC driver":

	total: 449 errors, 832 warnings, 2326 lines checked


A couple of minor comments:

> +static int da9052_rtc_enable_alarm(struct da9052 *da9052, unsigned char flag)
> +{
> +	int ret = 0;
> +	if (flag) {
> +		ret = da9052_set_bits(da9052, DA9052_ALARM_Y_REG,
> +					DA9052_ALARM_Y_ALARM_ON);
> +		if (ret != 0)
> +			dev_err(da9052->dev, "Failed to enable ALM: %d\n", ret);
> +	} else {
> +		ret = da9052_clear_bits(da9052, DA9052_ALARM_Y_REG,
> +					DA9052_ALARM_Y_ALARM_ON);
> +		if (ret != 0)
> +			dev_err(da9052->dev, "da9052_rtc_enable_alarm -> \
> +					da9052_clear_bits error %d\n", ret);
> +	}
> +	return ret;
> +}

"flag" is a poor identifier - it's largely meaningless.  Perhaps
"enable" would be a better choice in this case.  Making it have the
bool type wouild make sense also.

> +static irqreturn_t da9052_rtc_irq(int irq, void *data)
> +{
> +	struct da9052_rtc *rtc = (struct da9052_rtc *)data;

typecasting a void* like this is unneeded and is in fact undesirable,
as it will suppress possibly-useful warnings.


> +	int ret = 0;
> +
> +	ret = da9052_reg_read(rtc->da9052, DA9052_ALARM_MI_REG);
> +	if (ret < 0) {
> +		dev_err(rtc->da9052->dev, "da9052_rtc_notifier -> \
> +					da9052_reg_read error %d\n", ret);
> +		return IRQ_NONE;
> +	}
> +	if (ret & DA9052_ALARMMI_ALARMTYPE)
> +		da9052_rtc_enable_alarm(rtc->da9052, 0);
> +
> +	return IRQ_HANDLED;
> +}

Re: [PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[Joe Perches]

On Tue, 2011-04-12 at 16:37 -0700, Andrew Morton wrote:
> On Wed, 6 Apr 2011 18:47:29 +0530
> Ashish Jangam <Ashish.Jangam@kpitcummins.com> wrote:
> Please feed all the patches through scritps/checkpatch.pl if you haven't
> already done so, to clean up lots of trivial errors.
> For example, "MFD: MFD module of DA9052 PMIC driver":
> 	total: 449 errors, 832 warnings, 2326 lines checked

And a couple of more comments...

> > +static int da9052_rtc_enable_alarm(struct da9052 *da9052, unsigned char flag)
[]
> > +		if (ret != 0)
> > +			dev_err(da9052->dev, "da9052_rtc_enable_alarm -> \
> > +					da9052_clear_bits error %d\n", ret);
[]
> > +	ret = da9052_reg_read(rtc->da9052, DA9052_ALARM_MI_REG);
> > +	if (ret < 0) {
> > +		dev_err(rtc->da9052->dev, "da9052_rtc_notifier -> \
> > +					da9052_reg_read error %d\n", ret);

Line continuations in the middle a format string are very
error prone to whitespace errors, just like these introduce
bad whitespace after the ->.

These are better as:

		dev_err(rtc->da9052->dev, "%s: da9052_reg_read error: %d\n",
			__func__, ret);

Or maybe use some new macro/function(s) like

#define rtc_err(rtc, fmt, ...)	\
	dev_err((rtc)->da9052->dev, "%s: " fmt, __func__, ##__VA_ARGS__)

so these can be:

		rtc_err(rtc, "da9052_reg_read error: %d\n, ret);

[PATCH] checkpatch: Add check for line continuations in quoted strings

[Joe Perches]

Add a warning for unterminated quoted strings with line continuations
as these frequently add unwanted whitespace.

Signed-off-by: Joe Perches <joe@perches.com>

---

 scripts/checkpatch.pl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d867081..f3f907b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2748,6 +2748,11 @@ sub process {
 			WARN("sizeof(& should be avoided\n" . $herecurr);
 		}
 
+# check for line continuations in quoted strings with odd counts of "
+		if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) {
+			WARN("Avoid line continuations in quoted strings\n" . $herecurr);
+		}
+
 # check for new externs in .c files.
 		if ($realfile =~ /\.c$/ && defined $stat &&
 		    $stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s)

RE: [PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[Ashish Jangam]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4137 bytes --]

Hi Andrew,

Thanks for the review comments. We have addresses most of the comments in the posting done recently. For some of your queries, kindly see our response below. 

Regards,
Ashish J

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]
> Sent: Wednesday, April 13, 2011 5:08 AM
> To: Ashish Jangam
> Cc: Paul Gortmaker; linux-kernel@vger.kernel.org
> Subject: Re: [PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver
> 
> On Wed, 6 Apr 2011 18:47:29 +0530
> Ashish Jangam <Ashish.Jangam@kpitcummins.com> wrote:
> 
> > Hi Paul,
> >
> > RTC Driver for Dialog Semiconductor DA9052 PMICs.
> >
> > Changes made since last submission:
> > . read and write operation moved to MFD
> >
> > Linux Kernel Version: 2.6.37
> 
> The patch looks OK(ish) to me from a quick read.
> 
> > --- orig_linux-2.6.37/drivers/rtc/Kconfig	2011-01-05 05:50:19.000000000
> +0500
> > +++ linux-2.6.37/drivers/rtc/Kconfig	2011-03-31 21:07:39.000000000 +0500
> > @@ -664,6 +664,13 @@
> >  	help
> >  	  If you say yes here you get support for the RTC subsystem of the
> >  	  NUC910/NUC920 used in embedded systems.
> > +
> > +config RTC_DRV_DA9052
> > +	tristate "Dialog DA9052 RTC"
> > +	depends on PMIC_DA9052
> > +	help
> > +	  Say y here to support the RTC driver for
> > +	  Dialog Semiconductor DA9052 PMIC.
> 
> But there's not much I can do with it because PMIC_DA9052 does not
> exist in mainline or in linux-next.
DA9052 RTC has been placed under the "comment "Platform RTC drivers" section of drivers\rtc\Kconfig as the DA9052 MFD supports both the SPI and I2C serial protocols.
> 
> What is a PMIC_DA9052, anyway?  What CPU architectures support it, etc?
The DA9052 are designed to support application processors and associated peripherals. The DA9052 provides 11 LDO's and 4 high efficiency programmable Buck Converters which deliver high efficiency across a wide range of line and load conditions. DA9052 PMIC is widely used in portable navigation devices and other handhelds for efficient power management.
> Have you identified a maintainer who will be merging the main patch
> which enables PMIC_DA9052?
We have  sent the DA9052 MFD patch which will enable the PMIC DA9052 for review comments to Mark Brown and the LKML mailing list community for their views on it.
> 
> 
> Please feed all the patches through scritps/checkpatch.pl if you haven't
> already done so, to clean up lots of trivial errors.
> 
> For example, "MFD: MFD module of DA9052 PMIC driver":
> 
> 	total: 449 errors, 832 warnings, 2326 lines checked
> 
> 
> A couple of minor comments:
> 
> > +static int da9052_rtc_enable_alarm(struct da9052 *da9052, unsigned char
> flag)
> > +{
> > +	int ret = 0;
> > +	if (flag) {
> > +		ret = da9052_set_bits(da9052, DA9052_ALARM_Y_REG,
> > +					DA9052_ALARM_Y_ALARM_ON);
> > +		if (ret != 0)
> > +			dev_err(da9052->dev, "Failed to enable ALM: %d\n", ret);
> > +	} else {
> > +		ret = da9052_clear_bits(da9052, DA9052_ALARM_Y_REG,
> > +					DA9052_ALARM_Y_ALARM_ON);
> > +		if (ret != 0)
> > +			dev_err(da9052->dev, "da9052_rtc_enable_alarm -> \
> > +					da9052_clear_bits error %d\n", ret);
> > +	}
> > +	return ret;
> > +}
> 
> "flag" is a poor identifier - it's largely meaningless.  Perhaps
> "enable" would be a better choice in this case.  Making it have the
> bool type wouild make sense also.
> 
> > +static irqreturn_t da9052_rtc_irq(int irq, void *data)
> > +{
> > +	struct da9052_rtc *rtc = (struct da9052_rtc *)data;
> 
> typecasting a void* like this is unneeded and is in fact undesirable,
> as it will suppress possibly-useful warnings.
> 
> 
> > +	int ret = 0;
> > +
> > +	ret = da9052_reg_read(rtc->da9052, DA9052_ALARM_MI_REG);
> > +	if (ret < 0) {
> > +		dev_err(rtc->da9052->dev, "da9052_rtc_notifier -> \
> > +					da9052_reg_read error %d\n", ret);
> > +		return IRQ_NONE;
> > +	}
> > +	if (ret & DA9052_ALARMMI_ALARMTYPE)
> > +		da9052_rtc_enable_alarm(rtc->da9052, 0);
> > +
> > +	return IRQ_HANDLED;
> > +}
> 



ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[Ashish Jangam]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 10225 bytes --]

Hi Joe,


RTC Driver for Dialog Semiconductor DA9052 PMICs.

Changes made since last submission:
. Directly calling 'request_threaded_irq' instead of calling MFD function
. Formatting changes done
. Defined a macro for error notification
. Ported on 2.6.38.2 kernel
Linux Kernel Version: 2.6.38.2

Signed-off-by: D. Chen <dchen@diasemi.com>
---
diff -Naur linux-2.6.38.2/drivers/rtc/Kconfig wrk_linux-2.6.38.2/drivers/rtc/Kconfig
--- linux-2.6.38.2/drivers/rtc/Kconfig	2011-03-27 23:37:20.000000000 +0500
+++ wrk_linux-2.6.38.2/drivers/rtc/Kconfig	2011-04-14 14:38:59.000000000 +0500
@@ -475,6 +475,13 @@
 	style programming interface is mostly conserved, but any
 	updates are done via IPC calls to the system controller FW.

+config RTC_DRV_DA9052
+	tristate "Dialog DA9052 RTC"
+	depends on PMIC_DA9052
+	help
+	  Say y here to support the RTC driver for
+	  Dialog Semiconductor DA9052 PMIC.
+
 config RTC_DRV_DS1216
 	tristate "Dallas DS1216"
 	depends on SNI_RM
diff -Naur linux-2.6.38.2/drivers/rtc/Makefile wrk_linux-2.6.38.2/drivers/rtc/Makefile
--- linux-2.6.38.2/drivers/rtc/Makefile	2011-03-27 23:37:20.000000000 +0500
+++ wrk_linux-2.6.38.2/drivers/rtc/Makefile	2011-04-14 14:43:16.000000000 +0500
@@ -28,6 +28,7 @@
 obj-$(CONFIG_RTC_DRV_BQ4802)	+= rtc-bq4802.o
 obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc-cmos.o
 obj-$(CONFIG_RTC_DRV_COH901331)	+= rtc-coh901331.o
+obj-$(CONFIG_RTC_DRV_DA9052)	+= rtc-da9052.o
 obj-$(CONFIG_RTC_DRV_DAVINCI)	+= rtc-davinci.o
 obj-$(CONFIG_RTC_DRV_DM355EVM)	+= rtc-dm355evm.o
 obj-$(CONFIG_RTC_DRV_VRTC)	+= rtc-mrst.o
diff -Naur linux-2.6.38.2/drivers/rtc/rtc-da9052.c wrk_linux-2.6.38.2/drivers/rtc/rtc-da9052.c
--- linux-2.6.38.2/drivers/rtc/rtc-da9052.c	1970-01-01 05:00:00.000000000 +0500
+++ wrk_linux-2.6.38.2/drivers/rtc/rtc-da9052.c	2011-04-14 14:38:46.000000000 +0500
@@ -0,0 +1,340 @@
+/*
+ * Real time clock driver for DA9052
+ *
+ *Copyright(c) 2009 Dialog Semiconductor Ltd.
+ *
+ *Author: Dajun Chen <dajun.chen@diasemi.com>
+ *
+ * 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/platform_device.h>
+#include <linux/rtc.h>
+
+#include <linux/mfd/da9052/da9052.h>
+#include <linux/mfd/da9052/reg.h>
+
+#define rtc_err(da9052, fmt, ...) \
+		dev_err(da9052->dev, "%s: " fmt, __func__, ##__VA_ARGS__)
+
+struct da9052_rtc {
+	struct rtc_device *rtc;
+	struct da9052 *da9052;
+	int irq;
+};
+
+static int da9052_rtc_enable_alarm(struct da9052 *da9052, bool enable)
+{
+	int ret = 0;
+	if (enable) {
+		ret = da9052_set_bits(da9052, DA9052_ALARM_Y_REG,
+					DA9052_ALARM_Y_ALARM_ON);
+		if (ret != 0)
+			rtc_err(da9052, "Failed to enable ALM: %d\n", ret);
+	} else {
+		ret = da9052_clear_bits(da9052, DA9052_ALARM_Y_REG,
+					DA9052_ALARM_Y_ALARM_ON);
+		if (ret != 0)
+			rtc_err(da9052, "da9052_rtc_enable_alarm write error %d\n",
+					ret);
+	}
+	return ret;
+}
+
+static irqreturn_t da9052_rtc_irq(int irq, void *data)
+{
+	struct da9052_rtc *rtc = data;
+	int ret = 0;
+
+	ret = da9052_reg_read(rtc->da9052, DA9052_ALARM_MI_REG);
+	if (ret < 0) {
+		rtc_err(rtc->da9052, "da9052_rtc_notifier read error %d\n",
+					ret);
+		return IRQ_NONE;
+	}
+	if (ret & DA9052_ALARMMI_ALARMTYPE)
+		da9052_rtc_enable_alarm(rtc->da9052, 0);
+
+	return IRQ_HANDLED;
+}
+
+static int da9052_read_alarm(struct da9052 *da9052, struct rtc_time *rtc_tm)
+{
+
+	int ret = 0;
+	uint8_t v[5] = {0, 0, 0, 0, 0};
+	ret = da9052_group_read(da9052, DA9052_ALARM_MI_REG, 5, v);
+	if (ret != 0) {
+		rtc_err(da9052, "Failed to group read ALM: %d\n", ret);
+		return ret;
+	}
+
+	rtc_tm->tm_year = v[4] & DA9052_RTC_YEAR;
+	rtc_tm->tm_mon  = v[3] & DA9052_RTC_MONTH;
+	rtc_tm->tm_mday = v[2] & DA9052_RTC_DAY;
+	rtc_tm->tm_hour = v[1] & DA9052_RTC_HOUR;
+	rtc_tm->tm_min  = v[0] & DA9052_RTC_MIN;
+
+	ret = rtc_valid_tm(rtc_tm);
+	if (ret != 0)
+		return ret;
+
+	rtc_tm->tm_year += 100;
+	rtc_tm->tm_mon -= 1;
+
+	return ret;
+}
+
+static int da9052_set_alarm(struct da9052 *da9052, struct rtc_time *rtc_tm)
+{
+	int ret = 0;
+	uint8_t v[3] = {0, 0, 0};
+
+	rtc_tm->tm_sec = 0;
+	rtc_tm->tm_year -= 100;
+	rtc_tm->tm_mon += 1;
+
+	ret = rtc_valid_tm(rtc_tm);
+	if (ret)
+		return ret;
+
+	ret = da9052_reg_update(da9052, DA9052_ALARM_MI_REG,
+				DA9052_RTC_MIN, rtc_tm->tm_min);
+	if (ret != 0) {
+		rtc_err(da9052, "Failed to write ALRM MIN: %d\n", ret);
+		return ret;
+	}
+
+	ret = da9052_reg_update(da9052, DA9052_ALARM_Y_REG,
+				DA9052_RTC_YEAR, rtc_tm->tm_year);
+	if (ret != 0) {
+		rtc_err(da9052, "Failed to write ALRM YEAR: %d\n", ret);
+		return ret;
+	}
+
+	v[0] = rtc_tm->tm_hour;
+	v[1] = rtc_tm->tm_mday;
+	v[2] = rtc_tm->tm_mon;
+
+	return da9052_group_write(da9052, DA9052_ALARM_H_REG, 3, v);
+}
+
+static int da9052_rtc_get_alarm_status(struct da9052 *da9052)
+{
+	int ret = 0;
+	ret = da9052_reg_read(da9052, DA9052_ALARM_Y_REG);
+	if (ret < 0) {
+		rtc_err(da9052, "Failed to read ALM: %d\n", ret);
+		return ret;
+	}
+	ret &= DA9052_ALARM_Y_ALARM_ON;
+	return (ret > 0) ? 1 : 0;
+}
+
+static int da9052_rtc_read_time
+			(struct device *dev, struct rtc_time *rtc_tm)
+{
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+	uint8_t v[6] = {0, 0, 0, 0, 0, 0};
+	int ret;
+
+	ret = da9052_group_read(rtc->da9052, DA9052_COUNT_S_REG, 6, v);
+	if (ret != 0) {
+		rtc_err(rtc->da9052, "Failed to read RTC time : %d\n",
+					ret);
+		return ret;
+	}
+
+	rtc_tm->tm_year = v[5] & DA9052_RTC_YEAR;
+	rtc_tm->tm_mon  = v[4] & DA9052_RTC_MONTH;
+	rtc_tm->tm_mday = v[3] & DA9052_RTC_DAY;
+	rtc_tm->tm_hour = v[2] & DA9052_RTC_HOUR;
+	rtc_tm->tm_min  = v[1] & DA9052_RTC_MIN;
+	rtc_tm->tm_sec  = v[0] & DA9052_RTC_SEC;
+
+	ret = rtc_valid_tm(rtc_tm);
+	if (ret != 0) {
+		rtc_err(rtc->da9052, "da9052_rtc_read_time valid time failed %d\n",
+					ret);
+		return ret;
+	}
+
+	rtc_tm->tm_year += 100;
+	rtc_tm->tm_mon -= 1;
+
+	return 0;
+}
+
+
+static int da9052_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	int ret;
+	struct da9052_rtc *rtc;
+	uint8_t v[6] = {0, 0, 0, 0, 0, 0};
+
+	rtc = dev_get_drvdata(dev);
+
+	tm->tm_year -= 100;
+	tm->tm_mon += 1;
+
+	ret = rtc_valid_tm(tm);
+	if (ret != 0)
+		return ret;
+
+	v[0] = tm->tm_sec;
+	v[1] = tm->tm_min;
+	v[2] = tm->tm_hour;
+	v[3] = tm->tm_mday;
+	v[4] = tm->tm_mon;
+	v[5] = tm->tm_year;
+	return da9052_group_write(rtc->da9052, DA9052_COUNT_S_REG, 6, v);
+}
+
+static int da9052_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	int ret = 0;
+	struct rtc_time *tm = &alrm->time;
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	ret = da9052_read_alarm(rtc->da9052, tm);
+
+	if (ret)
+		return ret;
+
+	alrm->enabled = da9052_rtc_get_alarm_status(rtc->da9052);
+
+	return 0;
+}
+
+static int da9052_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	int ret = 0;
+	struct rtc_time *tm = &alrm->time;
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	ret = da9052_set_alarm(rtc->da9052, tm);
+
+	if (ret)
+		return ret;
+
+	ret = da9052_rtc_enable_alarm(rtc->da9052, 1);
+
+	return ret;
+}
+
+static int da9052_rtc_update_irq_enable(struct device *dev,
+		unsigned int enabled)
+{
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		return da9052_set_bits(rtc->da9052, DA9052_ALARM_Y_REG,
+					DA9052_ALARM_Y_TICK_ON);
+	else
+		return da9052_clear_bits(rtc->da9052, DA9052_ALARM_Y_REG,
+					 DA9052_ALARM_Y_TICK_ON);
+}
+
+static int da9052_rtc_alarm_irq_enable(struct device *dev,
+			unsigned int enabled)
+{
+	struct da9052_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		return da9052_rtc_enable_alarm(rtc->da9052, enabled);
+	else
+		return da9052_rtc_enable_alarm(rtc->da9052, enabled);
+}
+
+static const struct rtc_class_ops da9052_rtc_ops = {
+	.read_time	= da9052_rtc_read_time,
+	.set_time	= da9052_rtc_set_time,
+	.read_alarm	= da9052_rtc_read_alarm,
+	.set_alarm	= da9052_rtc_set_alarm,
+	.update_irq_enable = da9052_rtc_update_irq_enable,
+	.alarm_irq_enable = da9052_rtc_alarm_irq_enable,
+};
+
+static int __devinit da9052_rtc_probe(struct platform_device *pdev)
+{
+	struct da9052_rtc *rtc;
+	int ret = 0;
+
+	rtc = kzalloc(sizeof(struct da9052_rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	rtc->da9052 = dev_get_drvdata(pdev->dev.parent);
+	rtc->irq = platform_get_irq_byname(pdev, "ALM");
+
+	ret = request_threaded_irq(rtc->da9052->irq_base + rtc->irq,
+				NULL, da9052_rtc_irq,
+				IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				"ALM", rtc);
+	if (ret != 0) {
+		rtc_err(rtc->da9052, "DA9052 RTC failed irq registration: %d\n",
+					ret);
+		goto err_mem;
+	}
+
+	da9052_set_bits(rtc->da9052, DA9052_ALARM_Y_REG,
+			DA9052_ALARM_Y_TICK_ON);
+
+	rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
+				       &da9052_rtc_ops, THIS_MODULE);
+	if (IS_ERR(rtc->rtc)) {
+		ret = PTR_ERR(rtc->rtc);
+		goto err_free_irq;
+	}
+
+	platform_set_drvdata(pdev, rtc);
+
+	return 0;
+
+err_free_irq:
+	free_irq(rtc->da9052->irq_base + rtc->irq, NULL);
+err_mem:
+	kfree(rtc);
+	return ret;
+}
+
+static int __devexit da9052_rtc_remove(struct platform_device *pdev)
+{
+	struct da9052_rtc *rtc = pdev->dev.platform_data;
+
+	rtc_device_unregister(rtc->rtc);
+	free_irq(rtc->da9052->irq_base + rtc->irq, NULL);
+	platform_set_drvdata(pdev, NULL);
+	kfree(rtc);
+
+	return 0;
+}
+
+static struct platform_driver da9052_rtc_driver = {
+	.driver.name	= "da9052-rtc",
+	.driver.owner	= THIS_MODULE,
+	.probe		= da9052_rtc_probe,
+	.remove	= __devexit_p(da9052_rtc_remove),
+};
+
+static int __init da9052_rtc_init(void)
+{
+	return platform_driver_register(&da9052_rtc_driver);
+}
+module_init(da9052_rtc_init);
+
+static void __exit da9052_rtc_exit(void)
+{
+	platform_driver_unregister(&da9052_rtc_driver);
+}
+module_exit(da9052_rtc_exit);
+
+MODULE_AUTHOR("Dialog Semiconductor Ltd <dchen@diasemi.com>");
+MODULE_DESCRIPTION("RTC driver for Dialog DA9052 PMIC");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:da9052-rtc");
+

Regards,
Ashish J


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCHv1 3/11] RTC: RTC module of DA9052 PMIC driver

[Joe Perches]

On Thu, 2011-04-14 at 16:32 +0530, Ashish Jangam wrote:
> RTC Driver for Dialog Semiconductor DA9052 PMICs.

Just some comments on messaging.

Because you have __func__ defined in rtc_err, you don't
need to repeat it in the message format.

> diff -Naur linux-2.6.38.2/drivers/rtc/rtc-da9052.c wrk_linux-2.6.38.2/drivers/rtc/rtc-da9052.c
[]
> +#define rtc_err(da9052, fmt, ...) \
> +		dev_err(da9052->dev, "%s: " fmt, __func__, ##__VA_ARGS__)
[]
> +static int da9052_rtc_enable_alarm(struct da9052 *da9052, bool enable)
> +{
[]
> +			rtc_err(da9052, "da9052_rtc_enable_alarm write error %d\n",
> +					ret);

			rtc_err(da9052, "write error: %d\n", ret);

> +static irqreturn_t da9052_rtc_irq(int irq, void *data)
> +{
[]
> +	ret = da9052_reg_read(rtc->da9052, DA9052_ALARM_MI_REG);
> +	if (ret < 0) {
> +		rtc_err(rtc->da9052, "da9052_rtc_notifier read error %d\n",
> +					ret);

Error message doesn't make sense to me.  Maybe:
		rtc_err(rtc->da9052, "read error: %d\n", ret);

[]
> +static int da9052_rtc_read_time
> +			(struct device *dev, struct rtc_time *rtc_tm)
> +{
[]
> +	ret = rtc_valid_tm(rtc_tm);
> +	if (ret != 0) {
> +		rtc_err(rtc->da9052, "da9052_rtc_read_time valid time failed %d\n",
> +					ret);

		rtc_err(rtc->da9052, "rtc_valid_tm failed: %d\n", ret);

> +static int __devinit da9052_rtc_probe(struct platform_device *pdev)
> +{
[]
> +	ret = request_threaded_irq(rtc->da9052->irq_base + rtc->irq,
> +				NULL, da9052_rtc_irq,
> +				IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +				"ALM", rtc);
> +	if (ret != 0) {
> +		rtc_err(rtc->da9052, "DA9052 RTC failed irq registration: %d\n",
> +					ret);

		rtc_err(rtc->da9052, "irq registration failed: %d\n", ret);