Skip to contentSkip to conversation

rtc: enable rtc in max8925

PATCH New Haojian Zhuang 3 participants 7 messages Jan 25, 2010 – Feb 5, 2010

lore
Share & tools
lore
#1 lore raw

PATCH 3/4 rtc: enable rtc in max8925

Quoted 8 lines · On Mon, 25 Jan 2010 06:09:04 -0500 Haojian Zhuang <[email protected]>…

From 586baf68b0715b9b40bde8bc6bad5905d989cbe0 Mon Sep 17 00:00:00 2001
From: Haojian Zhuang <[email protected]>
Date: Mon, 25 Jan 2010 06:30:29 -0500
Subject: [PATCH] rtc: enable rtc in max8925

MAX8925 is a PMIC that contains RTC component.

Signed-off-by Haojian Zhuang <[email protected]>

Thanks for your contribution, comments below.
Please also read the checklist at
http://groups.google.com/group/rtc-linux/web/checklist

Quoted 108 lines
---
 drivers/rtc/Kconfig       |   10 ++
 drivers/rtc/Makefile      |    1 +
 drivers/rtc/rtc-max8925.c |  310 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 321 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-max8925.c
drivers/rtc/Kconfig +10 -0
index 8167e9e..6c6a42e 100644
@@ -175,6 +175,16 @@ config RTC_DRV_MAX6900
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max6900.
 
+config RTC_DRV_MAX8925
+	tristate "Maxim MAX8925"
+	depends on MFD_MAX8925
+	help
+	  If you say yes here you will get support for the
+	  RTC of Maxim MAX8925 PMIC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-max8925.
+
 config RTC_DRV_RS5C372
 	tristate "Ricoh R2025S/D, RS5C372A/B, RV5C386, RV5C387A"
 	help
drivers/rtc/Makefile +1 -0
index e5160fd..f427e39 100644
@@ -52,6 +52,7 @@ obj-$(CONFIG_RTC_DRV_M48T59)	+= rtc-m48t59.o
 obj-$(CONFIG_RTC_DRV_M48T86)	+= rtc-m48t86.o
 obj-$(CONFIG_RTC_MXC)		+= rtc-mxc.o
 obj-$(CONFIG_RTC_DRV_MAX6900)	+= rtc-max6900.o
+obj-$(CONFIG_RTC_DRV_MAX8925)	+= rtc-max8925.o
 obj-$(CONFIG_RTC_DRV_MAX6902)	+= rtc-max6902.o
 obj-$(CONFIG_RTC_DRV_MC13783)	+= rtc-mc13783.o
 obj-$(CONFIG_RTC_DRV_MSM6242)	+= rtc-msm6242.o
drivers/rtc/rtc-max8925.c new file +62 -0
new file mode 100644
index 0000000..38f4828
@@ -0,0 +1,310 @@
+/*
+ * RTC driver for Maxim MAX8925
+ *
+ * Copyright (C) 2009-2010 Marvell International Ltd.
+ *	Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/max8925.h>
+
+enum {
+	RTC_SEC = 0,
+	RTC_MIN,
+	RTC_HOUR,
+	RTC_WEEKDAY,
+	RTC_DATE,
+	RTC_MONTH,
+	RTC_YEAR1,
+	RTC_YEAR2,
+};
+
+#define MAX8925_RTC_SEC			0x00
+#define MAX8925_RTC_MIN			0x01
+#define MAX8925_RTC_HOUR		0x02
+#define MAX8925_RTC_WEEKDAY		0x03
+#define MAX8925_RTC_DATE		0x04
+#define MAX8925_RTC_MONTH		0x05
+#define MAX8925_RTC_YEAR1		0x06
+#define MAX8925_RTC_YEAR2		0x07
+#define MAX8925_ALARM0_SEC		0x08
+#define MAX8925_ALARM0_MIN		0x09
+#define MAX8925_ALARM0_HOUR		0x0a
+#define MAX8925_ALARM0_WEEKDAY		0x0b
+#define MAX8925_ALARM0_DATE		0x0c
+#define MAX8925_ALARM0_MON		0x0d
+#define MAX8925_ALARM0_YEAR1		0x0e
+#define MAX8925_ALARM0_YEAR2		0x0f
+#define MAX8925_ALARM1_SEC		0x10
+#define MAX8925_ALARM1_MIN		0x11
+#define MAX8925_ALARM1_HOUR		0x12
+#define MAX8925_ALARM1_WEEKDAY		0x13
+#define MAX8925_ALARM1_DATE		0x14
+#define MAX8925_ALARM1_MON		0x15
+#define MAX8925_ALARM1_YEAR1		0x16
+#define MAX8925_ALARM1_YEAR2		0x17
+#define MAX8925_RTC_CNTL		0x1b
+#define MAX8925_RTC_STATUS		0x20
+
+#define TIME_NUM			8
+#define ALARM_1SEC			(1 << 7)
+#define HOUR_12				(1 << 7)
+#define HOUR_AM_PM			(1 << 5)
+#define ALARM0_IRQ			(1 << 3)
+#define ALARM1_IRQ			(1 << 2)
+#define ALARM0_STATUS			(1 << 2)

drivers/rtc/rtc-max8925.c line 72

Quoted 10 lines
drivers/rtc/rtc-max8925.c new file +10 -0
+#define ALARM1_STATUS			(1 << 1)
+
+
+struct max8925_rtc_info {
+	struct rtc_device	*rtc_dev;
+	struct max8925_chip	*chip;
+	struct i2c_client	*i2c;
+	struct i2c_client	*rtc;
+	struct device		*dev;
+};

why two i2c_client structs? If it's required, please explain in a comment.

Quoted 9 lines
drivers/rtc/rtc-max8925.c new file +9 -0
+
+static irqreturn_t rtc_update_handler(int irq, void *data)
+{
+	struct max8925_rtc_info *info = (struct max8925_rtc_info *)data;
+
+	/* disable ALARM0 except for 1SEC alarm */
+	max8925_set_bits(info->rtc, MAX8925_ALARM0_CNTL, 0x7f, 0);
+	rtc_update_irq(info->rtc_dev, 1, RTC_IRQF | RTC_AF);
+	return IRQ_HANDLED;

drivers/rtc/rtc-max8925.c line 91

Quoted 10 lines
drivers/rtc/rtc-max8925.c new file +10 -0
+}
+
+static int max8925_rtc_open(struct device *dev)
+{
+	return 0;
+}
+
+static void max8925_rtc_release(struct device *dev)
+{
+}

you can avoid declaring it, it's not required.

Quoted 66 lines
drivers/rtc/rtc-max8925.c new file +66 -0
+static int tm_calc(struct rtc_time *tm, unsigned char *buf, int len)
+{
+	if (len < TIME_NUM)
+		return -EINVAL;
+	tm->tm_year = (buf[RTC_YEAR2] >> 4) * 1000
+			+ (buf[RTC_YEAR2] & 0xf) * 100
+			+ (buf[RTC_YEAR1] >> 4) * 10
+			+ (buf[RTC_YEAR1] & 0xf);
+	tm->tm_year -= 1900;
+	tm->tm_mon = ((buf[RTC_MONTH] >> 4) & 0x01) * 10
+			+ (buf[RTC_MONTH] & 0x0f);
+	tm->tm_mday = ((buf[RTC_DATE] >> 4) & 0x03) * 10
+			+ (buf[RTC_DATE] & 0x0f);
+	tm->tm_wday = buf[RTC_WEEKDAY] & 0x07;
+	if (buf[RTC_HOUR] & HOUR_12) {
+		tm->tm_hour = ((buf[RTC_HOUR] >> 4) & 0x1) * 10
+				+ (buf[RTC_HOUR] & 0x0f);
+		if (buf[RTC_HOUR] & HOUR_AM_PM)
+			tm->tm_hour += 12;
+	} else
+		tm->tm_hour = ((buf[RTC_HOUR] >> 4) & 0x03) * 10
+				+ (buf[RTC_HOUR] & 0x0f);
+	tm->tm_min = ((buf[RTC_MIN] >> 4) & 0x7) * 10
+			+ (buf[RTC_MIN] & 0x0f);
+	tm->tm_sec = ((buf[RTC_SEC] >> 4) & 0x7) * 10
+			+ (buf[RTC_SEC] & 0x0f);
+	return 0;
+}
+
+static int data_calc(unsigned char *buf, struct rtc_time *tm, int len)
+{
+	unsigned char high, low;
+
+	if (len < TIME_NUM)
+		return -EINVAL;
+
+	high = (tm->tm_year + 1900) / 1000;
+	low = (tm->tm_year + 1900) / 100;
+	low = low - high * 10;
+	buf[RTC_YEAR2] = (high << 4) + low;
+	high = (tm->tm_year + 1900) / 10;
+	low = tm->tm_year + 1900;
+	low = low - high * 10;
+	high = high - (high / 10) * 10;
+	buf[RTC_YEAR1] = (high << 4) + low;
+	high = tm->tm_mon / 10;
+	low = tm->tm_mon;
+	low = low - high * 10;
+	buf[RTC_MONTH] = (high << 4) + low;
+	high = tm->tm_mday / 10;
+	low = tm->tm_mday;
+	low = low - high * 10;
+	buf[RTC_DATE] = (high << 4) + low;
+	buf[RTC_WEEKDAY] = tm->tm_wday;
+	high = tm->tm_hour / 10;
+	low = tm->tm_hour;
+	low = low - high * 10;
+	buf[RTC_HOUR] = (high << 4) + low;
+	high = tm->tm_min / 10;
+	low = tm->tm_min;
+	low = low - high * 10;
+	buf[RTC_MIN] = (high << 4) + low;
+	high = tm->tm_sec / 10;
+	low = tm->tm_sec;
+	low = low - high * 10;
+	buf[RTC_SEC] = (high << 4) + low;

drivers/rtc/rtc-max8925.c line 167

Quoted 10 lines
drivers/rtc/rtc-max8925.c new file +10 -0
+	return 0;
+}
+
+static int max8925_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct max8925_rtc_info *info = dev_get_drvdata(dev);
+	unsigned char buf[TIME_NUM];
+	int ret;
+
+	max8925_bulk_read(info->rtc, MAX8925_RTC_SEC, TIME_NUM, buf);

no error checking?

drivers/rtc/rtc-max8925.c line 170

Quoted 3 lines
drivers/rtc/rtc-max8925.c new file +3 -0
+	ret = tm_calc(tm, buf, TIME_NUM);
+	return ret;
+}

just return tm_calc ....

drivers/rtc/rtc-max8925.c line 181

Quoted 11 lines
drivers/rtc/rtc-max8925.c new file +11 -0
+
+static int max8925_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct max8925_rtc_info *info = dev_get_drvdata(dev);
+	unsigned char buf[TIME_NUM];
+	int ret;
+
+	ret = data_calc(buf, tm, TIME_NUM);
+	if (ret < 0)
+		return ret;
+	max8925_bulk_write(info->rtc, MAX8925_RTC_SEC, TIME_NUM, buf);

can't this fail?

drivers/rtc/rtc-max8925.c

Quoted 12 lines
drivers/rtc/rtc-max8925.c new file +11 -0
+	return ret;
+}
 
+
+static int max8925_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct max8925_rtc_info *info = dev_get_drvdata(dev);
+	unsigned char buf[TIME_NUM];
+	int ret;
+
+	max8925_bulk_read(info->rtc, MAX8925_ALARM0_SEC, TIME_NUM, buf);
+	ret = tm_calc(&alrm->time, buf, TIME_NUM);

ditto.

drivers/rtc/rtc-max8925.c

Quoted 6 lines
drivers/rtc/rtc-max8925.c new file +6 -0
+	ret = max8925_reg_read(info->rtc, MAX8925_RTC_IRQ_MASK);
+	if ((ret & ALARM0_IRQ) == 0)
+		alrm->enabled = 1;
+	else
+		alrm->enabled = 0;
+	ret = max8925_reg_read(info->rtc, MAX8925_RTC_STATUS);

how can a register read failure be distinguished
from the result?

Quoted 51 lines
drivers/rtc/rtc-max8925.c new file +51 -0
+	if (ret & ALARM0_STATUS)
+		alrm->pending = 1;
+	else
+		alrm->pending = 0;
+	return ret;
+}
+
+static int max8925_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct max8925_rtc_info *info = dev_get_drvdata(dev);
+	unsigned char buf[TIME_NUM];
+	int ret;
+
+	ret = data_calc(buf, &alrm->time, TIME_NUM);
+	if (ret < 0)
+		return ret;
+	max8925_bulk_write(info->rtc, MAX8925_ALARM0_SEC, TIME_NUM, buf);
+	/* only enable alarm on year/month/day/hour/min/sec */
+	max8925_reg_write(info->rtc, MAX8925_ALARM0_CNTL, 0x77);
+	return ret;
+}
+
+static const struct rtc_class_ops max8925_rtc_ops = {
+	.open		= max8925_rtc_open,
+	.release	= max8925_rtc_release,
+	.read_time	= max8925_rtc_read_time,
+	.set_time	= max8925_rtc_set_time,
+	.read_alarm	= max8925_rtc_read_alarm,
+	.set_alarm	= max8925_rtc_set_alarm,
+};
+
+static int __devinit max8925_rtc_probe(struct platform_device *pdev)
+{
+	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct max8925_rtc_info *info;
+	int irq, ret;
+
+	info = kzalloc(sizeof(struct max8925_rtc_info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->chip = chip;
+	info->i2c = chip->i2c;
+	info->rtc = chip->rtc;
+	info->dev = &pdev->dev;
+	irq = chip->irq_base + MAX8925_IRQ_RTC_ALARM0;
+
+	ret = request_threaded_irq(irq, NULL, rtc_update_handler,
+				   IRQF_ONESHOT, "rtc-alarm0", info);
+	if (ret < 0) {
+		dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
+			irq, ret);

drivers/rtc/rtc-max8925.c

Quoted 10 lines
drivers/rtc/rtc-max8925.c new file +10 -0
+		goto out_irq;
+	}
+
+	info->rtc_dev = rtc_device_register("max8925-rtc", &pdev->dev,
+					&max8925_rtc_ops, THIS_MODULE);
+	ret = PTR_ERR(info->rtc_dev);
+	if (IS_ERR(info->rtc_dev)) {
+		dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);
+		goto out_rtc;
+	}

PTR_ERR goes in the if

Quoted 29 lines
drivers/rtc/rtc-max8925.c new file +29 -0
+	dev_set_drvdata(&pdev->dev, info);
+	platform_set_drvdata(pdev, info);
+
+	return 0;
+out_rtc:
+	free_irq(chip->irq_base + MAX8925_IRQ_RTC_ALARM0, info);
+out_irq:
+	kfree(info);
+	return ret;
+}
+
+static int __devexit max8925_rtc_remove(struct platform_device *pdev)
+{
+	struct max8925_rtc_info *info = platform_get_drvdata(pdev);
+
+	if (info) {
+		free_irq(info->chip->irq_base + MAX8925_IRQ_RTC_ALARM0, info);
+		rtc_device_unregister(info->rtc_dev);
+		kfree(info);
+	}
+	return 0;
+}
+
+static struct platform_driver max8925_rtc_driver = {
+	.driver		= {
+		.name	= "max8925-rtc",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= max8925_rtc_probe,

drivers/rtc/rtc-max8925.c

Quoted 10 lines
drivers/rtc/rtc-max8925.c new file +10 -0
+	.remove		= __devexit_p(max8925_rtc_remove),
+};
+
+static int __init max8925_rtc_init(void)
+{
+	return platform_driver_register(&max8925_rtc_driver);
+}
+module_init(max8925_rtc_init);
+
+static void __exit max8925_rtc_exit(void)

this should be __devexit since you used __devexit_p

Quoted 11 lines
drivers/rtc/rtc-max8925.c new file +9 -0
+{
+	platform_driver_unregister(&max8925_rtc_driver);
+}
+module_exit(max8925_rtc_exit);
+
+MODULE_DESCRIPTION("Maxim MAX8925 RTC driver");
+MODULE_AUTHOR("Haojian Zhuang <[email protected]>");
+MODULE_LICENSE("GPL");
+
Signature · 1 line
1.5.6.5
Signature · 6 lines
 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it
author
#2 lore raw

PATCH 3/4 rtc: enable rtc in max8925

Quoted 14 lines · Alessandro Zummo
Quoted 11 lines · On Mon, 25 Jan 2010 06:09:04 -0500 Haojian Zhuang <[email protected]>…
(quoted diff) +11 -0
+
+static struct platform_driver max8925_rtc_driver = {
+ ? ? .driver ? ? ? ? = {
+ ? ? ? ? ? ? .name ? = "max8925-rtc",
+ ? ? ? ? ? ? .owner ?= THIS_MODULE,
+ ? ? },
+ ? ? .probe ? ? ? ? ?= max8925_rtc_probe,
+ ? ? .remove ? ? ? ? = __devexit_p(max8925_rtc_remove),
+};
+
+static int __init max8925_rtc_init(void)
Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+{
+ ? ? return platform_driver_register(&max8925_rtc_driver);
+}
+module_init(max8925_rtc_init);
+
+static void __exit max8925_rtc_exit(void)

?this should be __devexit since you used __devexit_p

I declared __devexit_p on max8925_rtc_remove. So I need to use __devexit on max8925_rtc_remove. I needn't use __devexit on max8925_rtc_exit, is it?

Others are updated.

Thanks
Haojian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-rtc-enable-rtc-in-max8925.patch
Type: text/x-patch
Size: 10270 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100125/d0157fcc/attachment-0001.bin>

#3 lore raw

PATCH 3/4 rtc: enable rtc in max8925

Quoted 25 lines · On Mon, 25 Jan 2010 06:55:50 -0500 Haojian Zhuang <[email protected]>…
Quoted 14 lines · Alessandro Zummo
Quoted 11 lines · On Mon, 25 Jan 2010 06:09:04 -0500 Haojian Zhuang <[email protected]>…
(quoted diff) +11 -0
+
+static struct platform_driver max8925_rtc_driver = {
+ ? ? .driver ? ? ? ? = {
+ ? ? ? ? ? ? .name ? = "max8925-rtc",
+ ? ? ? ? ? ? .owner ?= THIS_MODULE,
+ ? ? },
+ ? ? .probe ? ? ? ? ?= max8925_rtc_probe,
+ ? ? .remove ? ? ? ? = __devexit_p(max8925_rtc_remove),
+};
+
+static int __init max8925_rtc_init(void)
Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+{
+ ? ? return platform_driver_register(&max8925_rtc_driver);
+}
+module_init(max8925_rtc_init);
+
+static void __exit max8925_rtc_exit(void)

?this should be __devexit since you used __devexit_p

Quoted 3 lines

I declared __devexit_p on max8925_rtc_remove. So I need to use __devexit on max8925_rtc_remove. I needn't use __devexit on max8925_rtc_exit, is it?

Correct. My fault, sorry.

Signature · 6 lines
 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it
#4 lore raw

PATCH 3/4 rtc: enable rtc in max8925

In reply to Haojian Zhuang

Hi Haojian,

Review

Quoted 4 lines · Haojian Zhuang
(quoted diff) +4 -0
+struct max8925_rtc_info {
+	struct rtc_device	*rtc_dev;
+	struct max8925_chip	*chip;
+	struct i2c_client	*i2c;

I dont think you need this pointer here, right ?

Cheers,
Samuel.

Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/
author
#5 lore raw

PATCH 3/4 rtc: enable rtc in max8925

Quoted 9 lines · Samuel Ortiz

Hi Haojian,

Quoted 4 lines · Haojian Zhuang

+struct max8925_rtc_info {
+ ? ? struct rtc_device ? ? ? *rtc_dev;
+ ? ? struct max8925_chip ? ? *chip;
+ ? ? struct i2c_client ? ? ? *i2c;

I dont think you need this pointer here, right ?

Yes, this unused pointer should be removed. The refreshed patch is in another mail loop.

Thanks
Haojian

#6 lore raw

PATCH 3/4 rtc: enable rtc in max8925

Quoted 4 lines · Haojian Zhuang
Quoted 3 lines · Samuel Ortiz

Hi Haojian,

On Mon, Jan 25, 2010 at 06:55:50AM -0500, Haojian Zhuang wrote:

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

+struct max8925_rtc_info {
+ ? ? struct rtc_device ? ? ? *rtc_dev;
+ ? ? struct max8925_chip ? ? *chip;
+ ? ? struct i2c_client ? ? ? *i2c;

I dont think you need this pointer here, right ?

Yes, this unused pointer should be removed. The refreshed patch is in another mail loop.

Alessandro, Haojian is talking about the patch below. Does it look fine to you ?