[PATCH 0/3] add devicetree bindings for rtc-m48t86

[PATCH 0/3] add devicetree bindings for rtc-m48t86

[Alexander Clouter]

Currently there are two users of rtc-m48t86 (mach-ep93xx/ts72xx.c and
mach-orion5x/ts78xx-setup.c) and both just use {read,write}b against
a memory mapped region.  As I am devicetree'ing the TS-7800, this
driver needs converting and thats what this patchset does.

The patch does the following:
 * remove platform specific ops hooks, moving ioremap'ing and
	everything into the driver
 * utilises named resources to indicate index/data ranges
 * moves the RTC detection routine from ts78xx-setup.c into rtc-m48t86.c
 * and, of course, enable devicetree hooks and include documentation

Awkward step, the first patch breaks both boards, the two following
patches fix them.  Happy to re-work this if folks give me a pointer
on how to do this in an acceptable way.

My vote is to break fast, fix fast, spend the time writing other code :)

Signed-off-by: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>

Alexander Clouter (3):
  rtc: rtc-m48t86: add devicetree bindings
  arm: orion5x: fixup ts78xx to be able to use the rtc-m48t86 again.
  arm: ep93xx: fixup ts72xx to be able to use the rtc-m48t86 again.

 .../devicetree/bindings/rtc/rtc-m48t86.txt         |   17 ++
 arch/arm/mach-ep93xx/ts72xx.c                      |   29 +--
 arch/arm/mach-orion5x/ts78xx-setup.c               |   79 ++----
 drivers/rtc/rtc-m48t86.c                           |  254 +++++++++++++++-----
 include/linux/m48t86.h                             |   16 --
 5 files changed, 239 insertions(+), 156 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
 delete mode 100644 include/linux/m48t86.h

-- 
1.7.10.4

[PATCH 1/3] rtc: rtc-m48t86: add devicetree bindings

[Alexander Clouter]

This patch allows rtc-m48t86 to be used via devicetree.

Signed-off-by: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>
---
 .../devicetree/bindings/rtc/rtc-m48t86.txt         |   17 ++
 drivers/rtc/rtc-m48t86.c                           |  254 +++++++++++++++-----
 include/linux/m48t86.h                             |   16 --
 3 files changed, 213 insertions(+), 74 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
 delete mode 100644 include/linux/m48t86.h

diff --git a/Documentation/devicetree/bindings/rtc/rtc-m48t86.txt b/Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
new file mode 100644
index 0000000..1336c98
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
@@ -0,0 +1,17 @@
+ST M48T86 / Dallas DS12887 RTC driver
+
+Required properties:
+- compatible: "rtc-m48t86"
+- reg: array of two address ranges of the RTC index and data registers
+- reg-names: must have "rtc_index" and "rtc_data" present
+
+Example:
+
+rtc@808 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	compatible = "rtc-m48t86";
+	reg =   <0x808 0x04>,
+		<0x80c 0x04>;
+	reg-names = "rtc_index", "rtc_data";
+};
diff --git a/drivers/rtc/rtc-m48t86.c b/drivers/rtc/rtc-m48t86.c
index 2ffbcac..47e9fd1 100644
--- a/drivers/rtc/rtc-m48t86.c
+++ b/drivers/rtc/rtc-m48t86.c
@@ -16,8 +16,10 @@
 #include <linux/module.h>
 #include <linux/rtc.h>
 #include <linux/platform_device.h>
-#include <linux/m48t86.h>
 #include <linux/bcd.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/io.h>
 
 #define M48T86_REG_SEC		0x00
 #define M48T86_REG_SECALRM	0x01
@@ -41,40 +43,61 @@
 
 #define DRV_VERSION "0.1"
 
+struct m48t86_rtc_private_data {
+	void __iomem		*io_index;
+	void __iomem		*io_data;
+
+	struct rtc_device	*rtc;
+};
+
+static unsigned char m48t86_rtc_readbyte(
+			struct m48t86_rtc_private_data *priv,
+			unsigned long addr)
+{
+	writeb(addr, priv->io_index);
+	return readb(priv->io_data);
+}
+
+static void m48t86_rtc_writebyte(
+			struct m48t86_rtc_private_data *priv,
+			unsigned char value, unsigned long addr)
+{
+	writeb(addr, priv->io_index);
+	writeb(value, priv->io_data);
+}
 
 static int m48t86_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	unsigned char reg;
-	struct platform_device *pdev = to_platform_device(dev);
-	struct m48t86_ops *ops = pdev->dev.platform_data;
+	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
 
-	reg = ops->readbyte(M48T86_REG_B);
+	reg = m48t86_rtc_readbyte(priv, M48T86_REG_B);
 
 	if (reg & M48T86_REG_B_DM) {
 		/* data (binary) mode */
-		tm->tm_sec	= ops->readbyte(M48T86_REG_SEC);
-		tm->tm_min	= ops->readbyte(M48T86_REG_MIN);
-		tm->tm_hour	= ops->readbyte(M48T86_REG_HOUR) & 0x3F;
-		tm->tm_mday	= ops->readbyte(M48T86_REG_DOM);
+		tm->tm_sec	= m48t86_rtc_readbyte(priv, M48T86_REG_SEC);
+		tm->tm_min	= m48t86_rtc_readbyte(priv, M48T86_REG_MIN);
+		tm->tm_hour	= m48t86_rtc_readbyte(priv, M48T86_REG_HOUR) & 0x3F;
+		tm->tm_mday	= m48t86_rtc_readbyte(priv, M48T86_REG_DOM);
 		/* tm_mon is 0-11 */
-		tm->tm_mon	= ops->readbyte(M48T86_REG_MONTH) - 1;
-		tm->tm_year	= ops->readbyte(M48T86_REG_YEAR) + 100;
-		tm->tm_wday	= ops->readbyte(M48T86_REG_DOW);
+		tm->tm_mon	= m48t86_rtc_readbyte(priv, M48T86_REG_MONTH) - 1;
+		tm->tm_year	= m48t86_rtc_readbyte(priv, M48T86_REG_YEAR) + 100;
+		tm->tm_wday	= m48t86_rtc_readbyte(priv, M48T86_REG_DOW);
 	} else {
 		/* bcd mode */
-		tm->tm_sec	= bcd2bin(ops->readbyte(M48T86_REG_SEC));
-		tm->tm_min	= bcd2bin(ops->readbyte(M48T86_REG_MIN));
-		tm->tm_hour	= bcd2bin(ops->readbyte(M48T86_REG_HOUR) & 0x3F);
-		tm->tm_mday	= bcd2bin(ops->readbyte(M48T86_REG_DOM));
+		tm->tm_sec	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_SEC));
+		tm->tm_min	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_MIN));
+		tm->tm_hour	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_HOUR) & 0x3F);
+		tm->tm_mday	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_DOM));
 		/* tm_mon is 0-11 */
-		tm->tm_mon	= bcd2bin(ops->readbyte(M48T86_REG_MONTH)) - 1;
-		tm->tm_year	= bcd2bin(ops->readbyte(M48T86_REG_YEAR)) + 100;
-		tm->tm_wday	= bcd2bin(ops->readbyte(M48T86_REG_DOW));
+		tm->tm_mon	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_MONTH)) - 1;
+		tm->tm_year	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_YEAR)) + 100;
+		tm->tm_wday	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_DOW));
 	}
 
 	/* correct the hour if the clock is in 12h mode */
 	if (!(reg & M48T86_REG_B_H24))
-		if (ops->readbyte(M48T86_REG_HOUR) & 0x80)
+		if (m48t86_rtc_readbyte(priv, M48T86_REG_HOUR) & 0x80)
 			tm->tm_hour += 12;
 
 	return rtc_valid_tm(tm);
@@ -83,38 +106,37 @@ static int m48t86_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int m48t86_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	unsigned char reg;
-	struct platform_device *pdev = to_platform_device(dev);
-	struct m48t86_ops *ops = pdev->dev.platform_data;
+	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
 
-	reg = ops->readbyte(M48T86_REG_B);
+	reg = m48t86_rtc_readbyte(priv, M48T86_REG_B);
 
 	/* update flag and 24h mode */
 	reg |= M48T86_REG_B_SET | M48T86_REG_B_H24;
-	ops->writebyte(reg, M48T86_REG_B);
+	m48t86_rtc_writebyte(priv, reg, M48T86_REG_B);
 
 	if (reg & M48T86_REG_B_DM) {
 		/* data (binary) mode */
-		ops->writebyte(tm->tm_sec, M48T86_REG_SEC);
-		ops->writebyte(tm->tm_min, M48T86_REG_MIN);
-		ops->writebyte(tm->tm_hour, M48T86_REG_HOUR);
-		ops->writebyte(tm->tm_mday, M48T86_REG_DOM);
-		ops->writebyte(tm->tm_mon + 1, M48T86_REG_MONTH);
-		ops->writebyte(tm->tm_year % 100, M48T86_REG_YEAR);
-		ops->writebyte(tm->tm_wday, M48T86_REG_DOW);
+		m48t86_rtc_writebyte(priv, tm->tm_sec, M48T86_REG_SEC);
+		m48t86_rtc_writebyte(priv, tm->tm_min, M48T86_REG_MIN);
+		m48t86_rtc_writebyte(priv, tm->tm_hour, M48T86_REG_HOUR);
+		m48t86_rtc_writebyte(priv, tm->tm_mday, M48T86_REG_DOM);
+		m48t86_rtc_writebyte(priv, tm->tm_mon + 1, M48T86_REG_MONTH);
+		m48t86_rtc_writebyte(priv, tm->tm_year % 100, M48T86_REG_YEAR);
+		m48t86_rtc_writebyte(priv, tm->tm_wday, M48T86_REG_DOW);
 	} else {
 		/* bcd mode */
-		ops->writebyte(bin2bcd(tm->tm_sec), M48T86_REG_SEC);
-		ops->writebyte(bin2bcd(tm->tm_min), M48T86_REG_MIN);
-		ops->writebyte(bin2bcd(tm->tm_hour), M48T86_REG_HOUR);
-		ops->writebyte(bin2bcd(tm->tm_mday), M48T86_REG_DOM);
-		ops->writebyte(bin2bcd(tm->tm_mon + 1), M48T86_REG_MONTH);
-		ops->writebyte(bin2bcd(tm->tm_year % 100), M48T86_REG_YEAR);
-		ops->writebyte(bin2bcd(tm->tm_wday), M48T86_REG_DOW);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_sec), M48T86_REG_SEC);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_min), M48T86_REG_MIN);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_hour), M48T86_REG_HOUR);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_mday), M48T86_REG_DOM);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_mon + 1), M48T86_REG_MONTH);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_year % 100), M48T86_REG_YEAR);
+		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_wday), M48T86_REG_DOW);
 	}
 
 	/* update ended */
 	reg &= ~M48T86_REG_B_SET;
-	ops->writebyte(reg, M48T86_REG_B);
+	m48t86_rtc_writebyte(priv, reg, M48T86_REG_B);
 
 	return 0;
 }
@@ -122,15 +144,14 @@ static int m48t86_rtc_set_time(struct device *dev, struct rtc_time *tm)
 static int m48t86_rtc_proc(struct device *dev, struct seq_file *seq)
 {
 	unsigned char reg;
-	struct platform_device *pdev = to_platform_device(dev);
-	struct m48t86_ops *ops = pdev->dev.platform_data;
+	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
 
-	reg = ops->readbyte(M48T86_REG_B);
+	reg = m48t86_rtc_readbyte(priv, M48T86_REG_B);
 
 	seq_printf(seq, "mode\t\t: %s\n",
 		 (reg & M48T86_REG_B_DM) ? "binary" : "bcd");
 
-	reg = ops->readbyte(M48T86_REG_D);
+	reg = m48t86_rtc_readbyte(priv, M48T86_REG_D);
 
 	seq_printf(seq, "battery\t\t: %s\n",
 		 (reg & M48T86_REG_D_VRT) ? "ok" : "exhausted");
@@ -144,42 +165,159 @@ static const struct rtc_class_ops m48t86_rtc_ops = {
 	.proc		= m48t86_rtc_proc,
 };
 
-static int m48t86_rtc_probe(struct platform_device *dev)
+/*
+ * The RTC chip has 114 bytes upper bytes that can be used as user storage
+ * space which we can use to test if the chip is present; for example it is
+ * an optional feature and not all boards will have it present.
+ *
+ * I've used the method Technologic Systems use in their rtc7800.c example
+ * for the detection.
+ *
+ * TODO: track down a guinea pig without an RTC to see if we can work out a
+ *		better RTC detection routine
+ */
+static int m48t86_rtc_detect(struct device *dev)
+{
+	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
+
+	unsigned char tmp_rtc0, tmp_rtc1;
+
+	tmp_rtc0 = m48t86_rtc_readbyte(priv, 126);
+	tmp_rtc1 = m48t86_rtc_readbyte(priv, 127);
+
+	m48t86_rtc_writebyte(priv, 0x00, 126);
+	m48t86_rtc_writebyte(priv, 0x55, 127);
+	if (m48t86_rtc_readbyte(priv, 127) == 0x55) {
+		m48t86_rtc_writebyte(priv, 0xaa, 127);
+		if (m48t86_rtc_readbyte(priv, 127) == 0xaa
+				&& m48t86_rtc_readbyte(priv, 126) == 0x00) {
+			m48t86_rtc_writebyte(priv, tmp_rtc0, 126);
+			m48t86_rtc_writebyte(priv, tmp_rtc1, 127);
+
+			return 0;
+		}
+	}
+
+	dev_info(dev, "RTC not found\n");
+	return -ENODEV;
+}
+
+static int m48t86_rtc_probe(struct platform_device *pdev)
 {
+	struct m48t86_rtc_private_data *priv;
+	struct resource *res_index, *res_data;
 	unsigned char reg;
-	struct m48t86_ops *ops = dev->dev.platform_data;
-	struct rtc_device *rtc = rtc_device_register("m48t86",
-				&dev->dev, &m48t86_rtc_ops, THIS_MODULE);
+	int err;
+
+	res_index = platform_get_resource_byname(
+				pdev, IORESOURCE_MEM, "rtc_index");
+	if (!res_index)
+		return -ENXIO;
+	res_data = platform_get_resource_byname(
+				pdev, IORESOURCE_MEM, "rtc_data");
+	if (!res_data)
+		return -ENXIO;
+
+	/* Allocate memory for the device structure (and zero it) */
+	priv = kzalloc(sizeof(struct m48t86_rtc_private_data), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&pdev->dev, "failed to allocate device structure.\n");
+		return -ENOMEM;
+	}
+
+	if (!request_mem_region(res_index->start, resource_size(res_index),
+				dev_name(&pdev->dev))) {
+		dev_err(&pdev->dev, "request_mem_region failed\n");
+		err = -EBUSY;
+		goto out_free;
+	}
+	if (!request_mem_region(res_data->start, resource_size(res_data),
+				dev_name(&pdev->dev))) {
+		dev_err(&pdev->dev, "request_mem_region failed\n");
+		err = -EBUSY;
+		goto out_release_io_index;
+	}
+
+	priv->io_index = ioremap(res_index->start, resource_size(res_index));
+	if (priv->io_index == NULL) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		err = -EIO;
+		goto out_release_io_data;
+	}
+	priv->io_data = ioremap(res_data->start, resource_size(res_data));
+	if (priv->io_data == NULL) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		err = -EIO;
+		goto out_ctrl;
+	}
+
+	platform_set_drvdata(pdev, priv);
 
-	if (IS_ERR(rtc))
-		return PTR_ERR(rtc);
+	err = m48t86_rtc_detect(&pdev->dev);
+	if (err)
+		goto out;
 
-	platform_set_drvdata(dev, rtc);
+	priv->rtc = rtc_device_register("m48t86",
+				&pdev->dev, &m48t86_rtc_ops, THIS_MODULE);
+	if (IS_ERR(priv->rtc)) {
+		err = PTR_ERR(priv->rtc);
+		goto out;
+	}
 
 	/* read battery status */
-	reg = ops->readbyte(M48T86_REG_D);
-	dev_info(&dev->dev, "battery %s\n",
+	reg = m48t86_rtc_readbyte(priv, M48T86_REG_D);
+	dev_info(&pdev->dev, "battery %s\n",
 		(reg & M48T86_REG_D_VRT) ? "ok" : "exhausted");
 
 	return 0;
+
+out:
+	platform_set_drvdata(pdev, NULL);
+	iounmap(priv->io_data);
+out_ctrl:
+	iounmap(priv->io_index);
+out_release_io_data:
+	release_mem_region(res_data->start, resource_size(res_data));
+out_release_io_index:
+	release_mem_region(res_index->start, resource_size(res_index));
+out_free:
+	kfree(priv);
+	return err;
 }
 
-static int m48t86_rtc_remove(struct platform_device *dev)
+static int m48t86_rtc_remove(struct platform_device *pdev)
 {
-	struct rtc_device *rtc = platform_get_drvdata(dev);
+	struct m48t86_rtc_private_data *priv = platform_get_drvdata(pdev);
+	struct resource *res;
+
+	rtc_device_unregister(priv->rtc);
 
- 	if (rtc)
-		rtc_device_unregister(rtc);
+	platform_set_drvdata(pdev, NULL);
 
-	platform_set_drvdata(dev, NULL);
+	iounmap(priv->io_data);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rtc_data");
+	release_mem_region(res->start, resource_size(res));
+
+	iounmap(priv->io_index);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rtc_index");
+	release_mem_region(res->start, resource_size(res));
+
+	kfree(priv);
 
 	return 0;
 }
 
+static const struct of_device_id m48t86_rtc_match[] = {
+	{ .compatible = "rtc-m48t86" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, m48t86_rtc_match);
+
 static struct platform_driver m48t86_rtc_platform_driver = {
 	.driver		= {
-		.name	= "rtc-m48t86",
-		.owner	= THIS_MODULE,
+		.name		= "rtc-m48t86",
+		.owner		= THIS_MODULE,
+		.of_match_table = m48t86_rtc_match,
 	},
 	.probe		= m48t86_rtc_probe,
 	.remove		= m48t86_rtc_remove,
diff --git a/include/linux/m48t86.h b/include/linux/m48t86.h
deleted file mode 100644
index 915d6b4..0000000
--- a/include/linux/m48t86.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/*
- * ST M48T86 / Dallas DS12887 RTC driver
- * Copyright (c) 2006 Tower Technologies
- *
- * Author: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
- *
- * 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.
-*/
-
-struct m48t86_ops
-{
-	void (*writebyte)(unsigned char value, unsigned long addr);
-	unsigned char (*readbyte)(unsigned long addr);
-};
-- 
1.7.10.4

[PATCH 2/3] arm: orion5x: fixup ts78xx to be able to use the rtc-m48t86 again.

[Alexander Clouter]

This patch fixes the board to be able to use the rtc-m48t86 driver again.

Signed-off-by: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>
---
 arch/arm/mach-orion5x/ts78xx-setup.c |   79 ++++++++--------------------------
 1 file changed, 18 insertions(+), 61 deletions(-)

diff --git a/arch/arm/mach-orion5x/ts78xx-setup.c b/arch/arm/mach-orion5x/ts78xx-setup.c
index e960855..4ed4c56 100644
--- a/arch/arm/mach-orion5x/ts78xx-setup.c
+++ b/arch/arm/mach-orion5x/ts78xx-setup.c
@@ -16,13 +16,13 @@
 #include <linux/platform_device.h>
 #include <linux/mv643xx_eth.h>
 #include <linux/ata_platform.h>
-#include <linux/m48t86.h>
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/timeriomem-rng.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
+#include <linux/ioport.h>
 #include <mach/orion5x.h>
 #include "common.h"
 #include "mpp.h"
@@ -80,78 +80,35 @@ static struct mv_sata_platform_data ts78xx_sata_data = {
 /*****************************************************************************
  * RTC M48T86 - nicked^Wborrowed from arch/arm/mach-ep93xx/ts72xx.c
  ****************************************************************************/
-#define TS_RTC_CTRL	(TS78XX_FPGA_REGS_VIRT_BASE + 0x808)
-#define TS_RTC_DATA	(TS78XX_FPGA_REGS_VIRT_BASE + 0x80c)
+#define TS_RTC_INDEX	(TS78XX_FPGA_REGS_PHYS_BASE + 0x808)
+#define TS_RTC_DATA	(TS78XX_FPGA_REGS_PHYS_BASE + 0x80c)
 
-static unsigned char ts78xx_ts_rtc_readbyte(unsigned long addr)
-{
-	writeb(addr, TS_RTC_CTRL);
-	return readb(TS_RTC_DATA);
-}
-
-static void ts78xx_ts_rtc_writebyte(unsigned char value, unsigned long addr)
-{
-	writeb(addr, TS_RTC_CTRL);
-	writeb(value, TS_RTC_DATA);
-}
-
-static struct m48t86_ops ts78xx_ts_rtc_ops = {
-	.readbyte	= ts78xx_ts_rtc_readbyte,
-	.writebyte	= ts78xx_ts_rtc_writebyte,
+static struct resource ts78xx_ts_rtc_resources[] = {
+	DEFINE_RES_NAMED(TS_RTC_INDEX, 4, "rtc_index", IORESOURCE_MEM),
+	DEFINE_RES_NAMED(TS_RTC_DATA, 4, "rtc_data", IORESOURCE_MEM),
 };
 
 static struct platform_device ts78xx_ts_rtc_device = {
 	.name		= "rtc-m48t86",
 	.id		= -1,
-	.dev		= {
-		.platform_data	= &ts78xx_ts_rtc_ops,
-	},
-	.num_resources	= 0,
+	.resource	= ts78xx_ts_rtc_resources,
+	.num_resources	= ARRAY_SIZE(ts78xx_ts_rtc_resources),
 };
 
-/*
- * TS uses some of the user storage space on the RTC chip so see if it is
- * present; as it's an optional feature at purchase time and not all boards
- * will have it present
- *
- * I've used the method TS use in their rtc7800.c example for the detection
- *
- * TODO: track down a guinea pig without an RTC to see if we can work out a
- *		better RTC detection routine
- */
 static int ts78xx_ts_rtc_load(void)
 {
 	int rc;
-	unsigned char tmp_rtc0, tmp_rtc1;
-
-	tmp_rtc0 = ts78xx_ts_rtc_readbyte(126);
-	tmp_rtc1 = ts78xx_ts_rtc_readbyte(127);
-
-	ts78xx_ts_rtc_writebyte(0x00, 126);
-	ts78xx_ts_rtc_writebyte(0x55, 127);
-	if (ts78xx_ts_rtc_readbyte(127) == 0x55) {
-		ts78xx_ts_rtc_writebyte(0xaa, 127);
-		if (ts78xx_ts_rtc_readbyte(127) == 0xaa
-				&& ts78xx_ts_rtc_readbyte(126) == 0x00) {
-			ts78xx_ts_rtc_writebyte(tmp_rtc0, 126);
-			ts78xx_ts_rtc_writebyte(tmp_rtc1, 127);
-
-			if (ts78xx_fpga.supports.ts_rtc.init == 0) {
-				rc = platform_device_register(&ts78xx_ts_rtc_device);
-				if (!rc)
-					ts78xx_fpga.supports.ts_rtc.init = 1;
-			} else
-				rc = platform_device_add(&ts78xx_ts_rtc_device);
-
-			if (rc)
-				pr_info("RTC could not be registered: %d\n",
-					rc);
-			return rc;
-		}
-	}
 
-	pr_info("RTC not found\n");
-	return -ENODEV;
+	if (ts78xx_fpga.supports.ts_rtc.init == 0) {
+		rc = platform_device_register(&ts78xx_ts_rtc_device);
+		if (!rc)
+			ts78xx_fpga.supports.ts_rtc.init = 1;
+	} else
+		rc = platform_device_add(&ts78xx_ts_rtc_device);
+
+	if (rc)
+		pr_info("RTC could not be registered: %d\n", rc);
+	return rc;
 };
 
 static void ts78xx_ts_rtc_unload(void)
-- 
1.7.10.4

[PATCH 3/3] arm: ep93xx: fixup ts72xx to be able to use the rtc-m48t86 again.

[Alexander Clouter]

This patch fixes the board to be able to use the rtc-m48t86 driver again.

Signed-off-by: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>
---
 arch/arm/mach-ep93xx/ts72xx.c |   29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
index 61f4b5d..c0ed3bf 100644
--- a/arch/arm/mach-ep93xx/ts72xx.c
+++ b/arch/arm/mach-ep93xx/ts72xx.c
@@ -16,7 +16,7 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
-#include <linux/m48t86.h>
+#include <linux/ioport.h>
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 
@@ -179,31 +179,18 @@ static void __init ts72xx_register_flash(void)
 	}
 }
 
-
-static unsigned char ts72xx_rtc_readbyte(unsigned long addr)
-{
-	__raw_writeb(addr, TS72XX_RTC_INDEX_VIRT_BASE);
-	return __raw_readb(TS72XX_RTC_DATA_VIRT_BASE);
-}
-
-static void ts72xx_rtc_writebyte(unsigned char value, unsigned long addr)
-{
-	__raw_writeb(addr, TS72XX_RTC_INDEX_VIRT_BASE);
-	__raw_writeb(value, TS72XX_RTC_DATA_VIRT_BASE);
-}
-
-static struct m48t86_ops ts72xx_rtc_ops = {
-	.readbyte	= ts72xx_rtc_readbyte,
-	.writebyte	= ts72xx_rtc_writebyte,
+static struct resource ts72xx_rtc_resources[] = {
+	DEFINE_RES_NAMED(TS72XX_RTC_INDEX_PHYS_BASE, 4,
+				"rtc_index", IORESOURCE_MEM),
+	DEFINE_RES_NAMED(TS72XX_RTC_DATA_PHYS_BASE, 4,
+				"rtc_data", IORESOURCE_MEM),
 };
 
 static struct platform_device ts72xx_rtc_device = {
 	.name		= "rtc-m48t86",
 	.id		= -1,
-	.dev		= {
-		.platform_data	= &ts72xx_rtc_ops,
-	},
-	.num_resources	= 0,
+	.resource	= ts72xx_rtc_resources,
+	.num_resources	= ARRAY_SIZE(ts72xx_rtc_resources),
 };
 
 static struct resource ts72xx_wdt_resources[] = {
-- 
1.7.10.4

Re: [PATCH 0/3] add devicetree bindings for rtc-m48t86

[Ryan Mallon]

On 01/04/13 08:56, Alexander Clouter wrote:
> Currently there are two users of rtc-m48t86 (mach-ep93xx/ts72xx.c and
> mach-orion5x/ts78xx-setup.c) and both just use {read,write}b against
> a memory mapped region.  As I am devicetree'ing the TS-7800, this
> driver needs converting and thats what this patchset does.
> 
> The patch does the following:
>  * remove platform specific ops hooks, moving ioremap'ing and
> 	everything into the driver
>  * utilises named resources to indicate index/data ranges
>  * moves the RTC detection routine from ts78xx-setup.c into rtc-m48t86.c
>  * and, of course, enable devicetree hooks and include documentation
> 
> Awkward step, the first patch breaks both boards, the two following
> patches fix them.  Happy to re-work this if folks give me a pointer
> on how to do this in an acceptable way.

Sorry, that's no good. It breaks things like git bisect.

> 
> My vote is to break fast, fix fast, spend the time writing other code :)

The patch series will need to be reworked so that there is no
build/runtime breakage between any of the patches. I'll have a read
through and see if I can suggest something.

~Ryan


> Signed-off-by: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>
> 
> Alexander Clouter (3):
>   rtc: rtc-m48t86: add devicetree bindings
>   arm: orion5x: fixup ts78xx to be able to use the rtc-m48t86 again.
>   arm: ep93xx: fixup ts72xx to be able to use the rtc-m48t86 again.
> 
>  .../devicetree/bindings/rtc/rtc-m48t86.txt         |   17 ++
>  arch/arm/mach-ep93xx/ts72xx.c                      |   29 +--
>  arch/arm/mach-orion5x/ts78xx-setup.c               |   79 ++----
>  drivers/rtc/rtc-m48t86.c                           |  254 +++++++++++++++-----
>  include/linux/m48t86.h                             |   16 --
>  5 files changed, 239 insertions(+), 156 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
>  delete mode 100644 include/linux/m48t86.h
> 

Re: [PATCH 0/3] add devicetree bindings for rtc-m48t86

[Alexander Clouter]

On Tue, Apr 02, 2013 at 08:44:10AM +1100, Ryan Mallon wrote:
>On 01/04/13 08:56, Alexander Clouter wrote:
>> Currently there are two users of rtc-m48t86 (mach-ep93xx/ts72xx.c and
>> mach-orion5x/ts78xx-setup.c) and both just use {read,write}b against
>> a memory mapped region.  As I am devicetree'ing the TS-7800, this
>> driver needs converting and thats what this patchset does.
>>
>> The patch does the following:
>>  * remove platform specific ops hooks, moving ioremap'ing and
>> 	everything into the driver
>>  * utilises named resources to indicate index/data ranges
>>  * moves the RTC detection routine from ts78xx-setup.c into rtc-m48t86.c
>>  * and, of course, enable devicetree hooks and include documentation
>>
>> Awkward step, the first patch breaks both boards, the two following
>> patches fix them.  Happy to re-work this if folks give me a pointer
>> on how to do this in an acceptable way.
>
>Sorry, that's no good. It breaks things like git bisect.

Bah :)

>> My vote is to break fast, fix fast, spend the time writing other code :)
>
>The patch series will need to be reworked so that there is no
>build/runtime breakage between any of the patches. I'll have a read
>through and see if I can suggest something.

I am currently working through a new patchset now.  It maintains the original {write,read}byte 
ops but if not defined, and the required named resources are present, it moves to using driver 
side mem mapped regions and what not...

'watch this space'

Cheers

-- 
Alexander Clouter
.sigmonster says: Deflector shields just came on, Captain.

Re: [PATCH 1/3] rtc: rtc-m48t86: add devicetree bindings

[Ryan Mallon]

On 01/04/13 08:56, Alexander Clouter wrote:
> This patch allows rtc-m48t86 to be used via devicetree.
> 
> Signed-off-by: Alexander Clouter <alex-L4GPcECwBoDe9xe1eoZjHA@public.gmane.org>
> ---
>  .../devicetree/bindings/rtc/rtc-m48t86.txt         |   17 ++
>  drivers/rtc/rtc-m48t86.c                           |  254 +++++++++++++++-----
>  include/linux/m48t86.h                             |   16 --
>  3 files changed, 213 insertions(+), 74 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
>  delete mode 100644 include/linux/m48t86.h

Some comments below.

> diff --git a/Documentation/devicetree/bindings/rtc/rtc-m48t86.txt b/Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
> new file mode 100644
> index 0000000..1336c98
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-m48t86.txt
> @@ -0,0 +1,17 @@
> +ST M48T86 / Dallas DS12887 RTC driver
> +
> +Required properties:
> +- compatible: "rtc-m48t86"
> +- reg: array of two address ranges of the RTC index and data registers
> +- reg-names: must have "rtc_index" and "rtc_data" present
> +
> +Example:
> +
> +rtc@808 {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	compatible = "rtc-m48t86";
> +	reg =   <0x808 0x04>,
> +		<0x80c 0x04>;
> +	reg-names = "rtc_index", "rtc_data";
> +};
> diff --git a/drivers/rtc/rtc-m48t86.c b/drivers/rtc/rtc-m48t86.c
> index 2ffbcac..47e9fd1 100644
> --- a/drivers/rtc/rtc-m48t86.c
> +++ b/drivers/rtc/rtc-m48t86.c
> @@ -16,8 +16,10 @@
>  #include <linux/module.h>
>  #include <linux/rtc.h>
>  #include <linux/platform_device.h>
> -#include <linux/m48t86.h>
>  #include <linux/bcd.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
>  
>  #define M48T86_REG_SEC		0x00
>  #define M48T86_REG_SECALRM	0x01
> @@ -41,40 +43,61 @@
>  
>  #define DRV_VERSION "0.1"
>  
> +struct m48t86_rtc_private_data {
> +	void __iomem		*io_index;
> +	void __iomem		*io_data;
> +
> +	struct rtc_device	*rtc;
> +};
> +
> +static unsigned char m48t86_rtc_readbyte(
> +			struct m48t86_rtc_private_data *priv,
> +			unsigned long addr)

Line breaks like this are a bit ugly. Breaking after the return type
would be better. You can shorten this by changing 'unsigned char' to
'u8', and 'm48t86_rtc_private_data' to something like 'm48t86_priv'. Why
is addr unsigned long if you are passing it to writeb?

> +{
> +	writeb(addr, priv->io_index);
> +	return readb(priv->io_data);
> +}
> +
> +static void m48t86_rtc_writebyte(
> +			struct m48t86_rtc_private_data *priv,
> +			unsigned char value, unsigned long addr)
> +{
> +	writeb(addr, priv->io_index);
> +	writeb(value, priv->io_data);
> +}
>  
>  static int m48t86_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	unsigned char reg;
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct m48t86_ops *ops = pdev->dev.platform_data;
> +	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
>  
> -	reg = ops->readbyte(M48T86_REG_B);
> +	reg = m48t86_rtc_readbyte(priv, M48T86_REG_B);
>  
>  	if (reg & M48T86_REG_B_DM) {
>  		/* data (binary) mode */
> -		tm->tm_sec	= ops->readbyte(M48T86_REG_SEC);
> -		tm->tm_min	= ops->readbyte(M48T86_REG_MIN);
> -		tm->tm_hour	= ops->readbyte(M48T86_REG_HOUR) & 0x3F;
> -		tm->tm_mday	= ops->readbyte(M48T86_REG_DOM);
> +		tm->tm_sec	= m48t86_rtc_readbyte(priv, M48T86_REG_SEC);
> +		tm->tm_min	= m48t86_rtc_readbyte(priv, M48T86_REG_MIN);
> +		tm->tm_hour	= m48t86_rtc_readbyte(priv, M48T86_REG_HOUR) & 0x3F;
> +		tm->tm_mday	= m48t86_rtc_readbyte(priv, M48T86_REG_DOM);
>  		/* tm_mon is 0-11 */
> -		tm->tm_mon	= ops->readbyte(M48T86_REG_MONTH) - 1;
> -		tm->tm_year	= ops->readbyte(M48T86_REG_YEAR) + 100;
> -		tm->tm_wday	= ops->readbyte(M48T86_REG_DOW);
> +		tm->tm_mon	= m48t86_rtc_readbyte(priv, M48T86_REG_MONTH) - 1;
> +		tm->tm_year	= m48t86_rtc_readbyte(priv, M48T86_REG_YEAR) + 100;
> +		tm->tm_wday	= m48t86_rtc_readbyte(priv, M48T86_REG_DOW);
>  	} else {
>  		/* bcd mode */
> -		tm->tm_sec	= bcd2bin(ops->readbyte(M48T86_REG_SEC));
> -		tm->tm_min	= bcd2bin(ops->readbyte(M48T86_REG_MIN));
> -		tm->tm_hour	= bcd2bin(ops->readbyte(M48T86_REG_HOUR) & 0x3F);
> -		tm->tm_mday	= bcd2bin(ops->readbyte(M48T86_REG_DOM));
> +		tm->tm_sec	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_SEC));
> +		tm->tm_min	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_MIN));
> +		tm->tm_hour	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_HOUR) & 0x3F);
> +		tm->tm_mday	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_DOM));
>  		/* tm_mon is 0-11 */
> -		tm->tm_mon	= bcd2bin(ops->readbyte(M48T86_REG_MONTH)) - 1;
> -		tm->tm_year	= bcd2bin(ops->readbyte(M48T86_REG_YEAR)) + 100;
> -		tm->tm_wday	= bcd2bin(ops->readbyte(M48T86_REG_DOW));
> +		tm->tm_mon	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_MONTH)) - 1;
> +		tm->tm_year	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_YEAR)) + 100;
> +		tm->tm_wday	= bcd2bin(m48t86_rtc_readbyte(priv, M48T86_REG_DOW));
>  	}
>  
>  	/* correct the hour if the clock is in 12h mode */
>  	if (!(reg & M48T86_REG_B_H24))
> -		if (ops->readbyte(M48T86_REG_HOUR) & 0x80)
> +		if (m48t86_rtc_readbyte(priv, M48T86_REG_HOUR) & 0x80)
>  			tm->tm_hour += 12;
>  
>  	return rtc_valid_tm(tm);
> @@ -83,38 +106,37 @@ static int m48t86_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  static int m48t86_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  {
>  	unsigned char reg;
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct m48t86_ops *ops = pdev->dev.platform_data;
> +	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
>  
> -	reg = ops->readbyte(M48T86_REG_B);
> +	reg = m48t86_rtc_readbyte(priv, M48T86_REG_B);
>  
>  	/* update flag and 24h mode */
>  	reg |= M48T86_REG_B_SET | M48T86_REG_B_H24;
> -	ops->writebyte(reg, M48T86_REG_B);
> +	m48t86_rtc_writebyte(priv, reg, M48T86_REG_B);
>  
>  	if (reg & M48T86_REG_B_DM) {
>  		/* data (binary) mode */
> -		ops->writebyte(tm->tm_sec, M48T86_REG_SEC);
> -		ops->writebyte(tm->tm_min, M48T86_REG_MIN);
> -		ops->writebyte(tm->tm_hour, M48T86_REG_HOUR);
> -		ops->writebyte(tm->tm_mday, M48T86_REG_DOM);
> -		ops->writebyte(tm->tm_mon + 1, M48T86_REG_MONTH);
> -		ops->writebyte(tm->tm_year % 100, M48T86_REG_YEAR);
> -		ops->writebyte(tm->tm_wday, M48T86_REG_DOW);
> +		m48t86_rtc_writebyte(priv, tm->tm_sec, M48T86_REG_SEC);
> +		m48t86_rtc_writebyte(priv, tm->tm_min, M48T86_REG_MIN);
> +		m48t86_rtc_writebyte(priv, tm->tm_hour, M48T86_REG_HOUR);
> +		m48t86_rtc_writebyte(priv, tm->tm_mday, M48T86_REG_DOM);
> +		m48t86_rtc_writebyte(priv, tm->tm_mon + 1, M48T86_REG_MONTH);
> +		m48t86_rtc_writebyte(priv, tm->tm_year % 100, M48T86_REG_YEAR);
> +		m48t86_rtc_writebyte(priv, tm->tm_wday, M48T86_REG_DOW);
>  	} else {
>  		/* bcd mode */
> -		ops->writebyte(bin2bcd(tm->tm_sec), M48T86_REG_SEC);
> -		ops->writebyte(bin2bcd(tm->tm_min), M48T86_REG_MIN);
> -		ops->writebyte(bin2bcd(tm->tm_hour), M48T86_REG_HOUR);
> -		ops->writebyte(bin2bcd(tm->tm_mday), M48T86_REG_DOM);
> -		ops->writebyte(bin2bcd(tm->tm_mon + 1), M48T86_REG_MONTH);
> -		ops->writebyte(bin2bcd(tm->tm_year % 100), M48T86_REG_YEAR);
> -		ops->writebyte(bin2bcd(tm->tm_wday), M48T86_REG_DOW);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_sec), M48T86_REG_SEC);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_min), M48T86_REG_MIN);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_hour), M48T86_REG_HOUR);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_mday), M48T86_REG_DOM);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_mon + 1), M48T86_REG_MONTH);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_year % 100), M48T86_REG_YEAR);
> +		m48t86_rtc_writebyte(priv, bin2bcd(tm->tm_wday), M48T86_REG_DOW);
>  	}
>  
>  	/* update ended */
>  	reg &= ~M48T86_REG_B_SET;
> -	ops->writebyte(reg, M48T86_REG_B);
> +	m48t86_rtc_writebyte(priv, reg, M48T86_REG_B);
>  
>  	return 0;
>  }
> @@ -122,15 +144,14 @@ static int m48t86_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  static int m48t86_rtc_proc(struct device *dev, struct seq_file *seq)
>  {
>  	unsigned char reg;
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct m48t86_ops *ops = pdev->dev.platform_data;
> +	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
>  
> -	reg = ops->readbyte(M48T86_REG_B);
> +	reg = m48t86_rtc_readbyte(priv, M48T86_REG_B);
>  
>  	seq_printf(seq, "mode\t\t: %s\n",
>  		 (reg & M48T86_REG_B_DM) ? "binary" : "bcd");
>  
> -	reg = ops->readbyte(M48T86_REG_D);
> +	reg = m48t86_rtc_readbyte(priv, M48T86_REG_D);
>  
>  	seq_printf(seq, "battery\t\t: %s\n",
>  		 (reg & M48T86_REG_D_VRT) ? "ok" : "exhausted");
> @@ -144,42 +165,159 @@ static const struct rtc_class_ops m48t86_rtc_ops = {
>  	.proc		= m48t86_rtc_proc,
>  };
>  
> -static int m48t86_rtc_probe(struct platform_device *dev)
> +/*
> + * The RTC chip has 114 bytes upper bytes that can be used as user storage
> + * space which we can use to test if the chip is present; for example it is
> + * an optional feature and not all boards will have it present.
> + *
> + * I've used the method Technologic Systems use in their rtc7800.c example
> + * for the detection.
> + *
> + * TODO: track down a guinea pig without an RTC to see if we can work out a
> + *		better RTC detection routine
> + */
> +static int m48t86_rtc_detect(struct device *dev)
> +{
> +	struct m48t86_rtc_private_data *priv = dev_get_drvdata(dev);
> +
> +	unsigned char tmp_rtc0, tmp_rtc1;
> +
> +	tmp_rtc0 = m48t86_rtc_readbyte(priv, 126);
> +	tmp_rtc1 = m48t86_rtc_readbyte(priv, 127);

These should probably have #defines, even if it is just something like:

  #define REG_SCRATCH_1 126
  #define REG_SCRATCH_2 127

> +
> +	m48t86_rtc_writebyte(priv, 0x00, 126);
> +	m48t86_rtc_writebyte(priv, 0x55, 127);
> +	if (m48t86_rtc_readbyte(priv, 127) == 0x55) {
> +		m48t86_rtc_writebyte(priv, 0xaa, 127);
> +		if (m48t86_rtc_readbyte(priv, 127) == 0xaa
> +				&& m48t86_rtc_readbyte(priv, 126) == 0x00) {
> +			m48t86_rtc_writebyte(priv, tmp_rtc0, 126);
> +			m48t86_rtc_writebyte(priv, tmp_rtc1, 127);
> +
> +			return 0;
> +		}
> +	}
> +
> +	dev_info(dev, "RTC not found\n");

Don't spam the log with stuff like this.

> +	return -ENODEV;
> +}
> +
> +static int m48t86_rtc_probe(struct platform_device *pdev)
>  {
> +	struct m48t86_rtc_private_data *priv;
> +	struct resource *res_index, *res_data;
>  	unsigned char reg;
> -	struct m48t86_ops *ops = dev->dev.platform_data;
> -	struct rtc_device *rtc = rtc_device_register("m48t86",
> -				&dev->dev, &m48t86_rtc_ops, THIS_MODULE);
> +	int err;
> +
> +	res_index = platform_get_resource_byname(
> +				pdev, IORESOURCE_MEM, "rtc_index");

Nitpick - please break lines like this:

	res_index = platform_get_resource_byname(pdev,
						 IO_RESOURCE_MEM,
                                                 "rtc_index");

You can use the double tab stop indent if you want, but please try to
fill the 80 columns before wrapping arguments onto the next line.


> +	if (!res_index)
> +		return -ENXIO;
> +	res_data = platform_get_resource_byname(
> +				pdev, IORESOURCE_MEM, "rtc_data");
> +	if (!res_data)
> +		return -ENXIO;
> +
> +	/* Allocate memory for the device structure (and zero it) */
> +	priv = kzalloc(sizeof(struct m48t86_rtc_private_data), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(&pdev->dev, "failed to allocate device structure.\n");

Don't need the warning. kalloc failures will automatically print a
warning and stacktrace.

> +		return -ENOMEM;
> +	}
> +
> +	if (!request_mem_region(res_index->start, resource_size(res_index),
> +				dev_name(&pdev->dev))) {
> +		dev_err(&pdev->dev, "request_mem_region failed\n");
> +		err = -EBUSY;
> +		goto out_free;
> +	}
> +	if (!request_mem_region(res_data->start, resource_size(res_data),
> +				dev_name(&pdev->dev))) {
> +		dev_err(&pdev->dev, "request_mem_region failed\n");
> +		err = -EBUSY;
> +		goto out_release_io_index;
> +	}
> +
> +	priv->io_index = ioremap(res_index->start, resource_size(res_index));
> +	if (priv->io_index == NULL) {
> +		dev_err(&pdev->dev, "ioremap failed\n");
> +		err = -EIO;
> +		goto out_release_io_data;
> +	}
> +	priv->io_data = ioremap(res_data->start, resource_size(res_data));
> +	if (priv->io_data == NULL) {
> +		dev_err(&pdev->dev, "ioremap failed\n");
> +		err = -EIO;
> +		goto out_ctrl;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
>  
> -	if (IS_ERR(rtc))
> -		return PTR_ERR(rtc);
> +	err = m48t86_rtc_detect(&pdev->dev);
> +	if (err)
> +		goto out;
>  
> -	platform_set_drvdata(dev, rtc);
> +	priv->rtc = rtc_device_register("m48t86",
> +				&pdev->dev, &m48t86_rtc_ops, THIS_MODULE);
> +	if (IS_ERR(priv->rtc)) {
> +		err = PTR_ERR(priv->rtc);
> +		goto out;
> +	}
>  
>  	/* read battery status */
> -	reg = ops->readbyte(M48T86_REG_D);
> -	dev_info(&dev->dev, "battery %s\n",
> +	reg = m48t86_rtc_readbyte(priv, M48T86_REG_D);
> +	dev_info(&pdev->dev, "battery %s\n",
>  		(reg & M48T86_REG_D_VRT) ? "ok" : "exhausted");
>  
>  	return 0;
> +
> +out:
> +	platform_set_drvdata(pdev, NULL);

I don't think this is needed.

> +	iounmap(priv->io_data);
> +out_ctrl:
> +	iounmap(priv->io_index);
> +out_release_io_data:
> +	release_mem_region(res_data->start, resource_size(res_data));
> +out_release_io_index:
> +	release_mem_region(res_index->start, resource_size(res_index));
> +out_free:
> +	kfree(priv);
> +	return err;
>  }
>  
> -static int m48t86_rtc_remove(struct platform_device *dev)
> +static int m48t86_rtc_remove(struct platform_device *pdev)
>  {
> -	struct rtc_device *rtc = platform_get_drvdata(dev);
> +	struct m48t86_rtc_private_data *priv = platform_get_drvdata(pdev);
> +	struct resource *res;
> +
> +	rtc_device_unregister(priv->rtc);
>  
> - 	if (rtc)
> -		rtc_device_unregister(rtc);
> +	platform_set_drvdata(pdev, NULL);
>  
> -	platform_set_drvdata(dev, NULL);
> +	iounmap(priv->io_data);
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rtc_data");
> +	release_mem_region(res->start, resource_size(res));
> +
> +	iounmap(priv->io_index);
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rtc_index");
> +	release_mem_region(res->start, resource_size(res));
> +
> +	kfree(priv);
>  
>  	return 0;
>  }
>  
> +static const struct of_device_id m48t86_rtc_match[] = {
> +	{ .compatible = "rtc-m48t86" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, m48t86_rtc_match);
> +
>  static struct platform_driver m48t86_rtc_platform_driver = {
>  	.driver		= {
> -		.name	= "rtc-m48t86",
> -		.owner	= THIS_MODULE,
> +		.name		= "rtc-m48t86",
> +		.owner		= THIS_MODULE,
> +		.of_match_table = m48t86_rtc_match,
>  	},
>  	.probe		= m48t86_rtc_probe,
>  	.remove		= m48t86_rtc_remove,
> diff --git a/include/linux/m48t86.h b/include/linux/m48t86.h
> deleted file mode 100644
> index 915d6b4..0000000
> --- a/include/linux/m48t86.h
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -/*
> - * ST M48T86 / Dallas DS12887 RTC driver
> - * Copyright (c) 2006 Tower Technologies
> - *
> - * Author: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
> - *
> - * 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.
> -*/
> -
> -struct m48t86_ops
> -{
> -	void (*writebyte)(unsigned char value, unsigned long addr);
> -	unsigned char (*readbyte)(unsigned long addr);
> -};
> 

Re: [PATCH 0/3] add devicetree bindings for rtc-m48t86

[Ryan Mallon]

On 02/04/13 08:44, Ryan Mallon wrote:
> On 01/04/13 08:56, Alexander Clouter wrote:
>> Currently there are two users of rtc-m48t86 (mach-ep93xx/ts72xx.c and
>> mach-orion5x/ts78xx-setup.c) and both just use {read,write}b against
>> a memory mapped region.  As I am devicetree'ing the TS-7800, this
>> driver needs converting and thats what this patchset does.
>>
>> The patch does the following:
>>  * remove platform specific ops hooks, moving ioremap'ing and
>> 	everything into the driver
>>  * utilises named resources to indicate index/data ranges
>>  * moves the RTC detection routine from ts78xx-setup.c into rtc-m48t86.c
>>  * and, of course, enable devicetree hooks and include documentation
>>
>> Awkward step, the first patch breaks both boards, the two following
>> patches fix them.  Happy to re-work this if folks give me a pointer
>> on how to do this in an acceptable way.
> 
> Sorry, that's no good. It breaks things like git bisect.
> 
>>
>> My vote is to break fast, fix fast, spend the time writing other code :)
> 
> The patch series will need to be reworked so that there is no
> build/runtime breakage between any of the patches. I'll have a read
> through and see if I can suggest something.

So looking through the patches, you are going to need to modify the rtc
driver and the platform code in the same patch at least once since you
are changing the interface. You could break the patches down like this:

1) Move the rtc detection from the orion5x board to the rtc driver.
   This adds the detection support for the ep93xx board also.

2) Replace the board read/write ops structure with ioremapped regions.

3) Add the device tree bindings to the rtc driver.

The first two patches could be combined, but will touch both the rtc
driver and the platform code, and do the prep work needed for adding the
dt bindings. The third patch can then stand on its own, and more clearly
just adds the dt bindings.

~Ryan

Re: [PATCH 0/3] add devicetree bindings for rtc-m48t86

[Jason Cooper]

On Tue, Apr 02, 2013 at 09:12:02AM +1100, Ryan Mallon wrote:
> On 02/04/13 08:44, Ryan Mallon wrote:
> > On 01/04/13 08:56, Alexander Clouter wrote:
> >> Currently there are two users of rtc-m48t86 (mach-ep93xx/ts72xx.c and
> >> mach-orion5x/ts78xx-setup.c) and both just use {read,write}b against
> >> a memory mapped region.  As I am devicetree'ing the TS-7800, this
> >> driver needs converting and thats what this patchset does.
> >>
> >> The patch does the following:
> >>  * remove platform specific ops hooks, moving ioremap'ing and
> >> 	everything into the driver
> >>  * utilises named resources to indicate index/data ranges
> >>  * moves the RTC detection routine from ts78xx-setup.c into rtc-m48t86.c
> >>  * and, of course, enable devicetree hooks and include documentation
> >>
> >> Awkward step, the first patch breaks both boards, the two following
> >> patches fix them.  Happy to re-work this if folks give me a pointer
> >> on how to do this in an acceptable way.
> > 
> > Sorry, that's no good. It breaks things like git bisect.
> > 
> >>
> >> My vote is to break fast, fix fast, spend the time writing other code :)
> > 
> > The patch series will need to be reworked so that there is no
> > build/runtime breakage between any of the patches. I'll have a read
> > through and see if I can suggest something.
> 
> So looking through the patches, you are going to need to modify the rtc
> driver and the platform code in the same patch at least once since you
> are changing the interface. You could break the patches down like this:
> 
> 1) Move the rtc detection from the orion5x board to the rtc driver.
>    This adds the detection support for the ep93xx board also.
> 
> 2) Replace the board read/write ops structure with ioremapped regions.
> 
> 3) Add the device tree bindings to the rtc driver.
> 
> The first two patches could be combined, but will touch both the rtc
> driver and the platform code, and do the prep work needed for adding the
> dt bindings. 

Please keep me and Andrew Lunn in the Cc: and once we see the final
series, I'll ack the platform changes for going though the rtc tree.
It's best to keep this series together, and the changes to the platform
code should be pretty isolated from other changes we are doing.

thx,

Jason.