U300 COH 901 331 RTC driver

This adds a driver for the RTC COH 901 331 found in the ST-Ericsson
U300 series mobile platforms to the RTC subsystem. It integrates
to the ARM kernel support recently added to RMKs ARM tree and will
be enabled in the U300 defconfig in due time.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/rtc/Kconfig         |   12 ++
 drivers/rtc/Makefile        |    1 +
 drivers/rtc/rtc-coh901331.c |  346 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 359 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-coh901331.c

On Mon, 4 May 2009 10:28:35 +0200
Linus Walleij <linus.ml.walleij@gmail.com> wrote:

 Hi,

  thanks for your contribute. comments below.
  please also consult the checklist at http://groups.google.com/group/rtc-linux/web/checklist
  wrto many details and the use of platform_driver_probe
> 
> This adds a driver for the RTC COH 901 331 found in the ST-Ericsson
> U300 series mobile platforms to the RTC subsystem. It integrates
> to the ARM kernel support recently added to RMKs ARM tree and will
> be enabled in the U300 defconfig in due time.
> 
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/rtc/Kconfig         |   12 ++
>  drivers/rtc/Makefile        |    1 +
>  drivers/rtc/rtc-coh901331.c |  346 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 359 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/rtc/rtc-coh901331.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 4e9851f..8262788 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -750,4 +750,16 @@ config RTC_DRV_PS3
>  	  This driver can also be built as a module. If so, the module
>  	  will be called rtc-ps3.
> 
> +config RTC_DRV_COH901331
> + 	tristate "ST-Ericsson COH 901 331 RTC"
> +	depends on ARCH_U300
> +	help
> +	  If you say Y here you will get access to ST-Ericsson
> +	  COH 901 331 RTC clock found in some ST-Ericsson Mobile
> +	  Platforms.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called "rtc-coh901331".
> +
> +
>  endif # RTC_CLASS
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 6c0639a..4bb930b 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -77,3 +77,4 @@ obj-$(CONFIG_RTC_DRV_WM8350)	+= rtc-wm8350.o
>  obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
>  obj-$(CONFIG_RTC_DRV_PCF50633)	+= rtc-pcf50633.o
>  obj-$(CONFIG_RTC_DRV_PS3)	+= rtc-ps3.o
> +obj-$(CONFIG_RTC_DRV_COH901331)	+= rtc-coh901331.o
> diff --git a/drivers/rtc/rtc-coh901331.c b/drivers/rtc/rtc-coh901331.c
> new file mode 100644
> index 0000000..160b0ab
> --- /dev/null
> +++ b/drivers/rtc/rtc-coh901331.c
> @@ -0,0 +1,346 @@
> +/*
> + *
> + * drivers/rtc/rtc-coh901331.c

 no file names in the source code.

> + *
> + *
> + * Copyright (C) 2007-2009 ST-Ericsson AB
> + * License terms: GNU General Public License (GPL) version 2
> + * Real Time Clock interface for ST-Ericsson AB COH 901 331 RTC.
> + * Author: Linus Walleij <linus.walleij@stericsson.com>
> + * Based on rtc-pl031.c by Deepak Saxena <dsaxena@plexity.net>
> + * Copyright 2006 (c) MontaVista Software, Inc.
> + */
> +#include <linux/module.h>
> +#include <linux/rtc.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +
> +/*
> + * Registers in the COH 901 331
> + */
> +/* Alarm value 32bit (R/W) */
> +#define COH901331_ALARM		0x00U
> +/* Used to set current time 32bit (R/W) */
> +#define COH901331_SET_TIME	0x04U
> +/* Indication if current time is valid 32bit (R/-) */
> +#define COH901331_VALID		0x08U
> +/* Read the current time 32bit (R/-) */
> +#define COH901331_CUR_TIME	0x0cU
> +/* Event register for the "alarm" interrupt */
> +#define COH901331_IRQ_EVENT	0x10U
> +/* Mask register for the "alarm" interrupt */
> +#define COH901331_IRQ_MASK	0x14U
> +/* Force register for the "alarm" interrupt */
> +#define COH901331_IRQ_FORCE	0x18U
> +
> +/*
> + * Reference to RTC block clock
> + * Notice that the frequent clk_enable()/clk_disable() on this
> + * clock is mainly to be able to turn on/off other clocks in the
> + * hierarchy as needed, the RTC clock is always on anyway.
> + */
> +struct coh901331_port {
> +	struct rtc_device *rtc;
> +	struct clk *clk;
> +	u32 phybase;
> +	u32 physize;
> +	void __iomem *virtbase;
> +	int irq;
> +#ifdef CONFIG_PM
> +	u32 irqmaskstore;
> +#endif
> +};
> +
> +static irqreturn_t coh901331_interrupt(int irq, void *data)
> +{
> +	struct coh901331_port *rtap = data;
> +
> +	clk_enable(rtap->clk);
> +	/* Ack IRQ */
> +	writel(1, rtap->virtbase + COH901331_IRQ_EVENT);
> +	clk_disable(rtap->clk);
> +	/* Set alarm flag */
> +	rtc_update_irq(rtap->rtc, 1, RTC_AF);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int coh901331_open(struct device *dev)
> +{
> +	/* Not much to do here */
> +	return 0;
> +}
> +
> +static void coh901331_release(struct device *dev)
> +{
> +	/* Not much to do here */
> +}

 if not needed, do not declare those two routines.

> +static int coh901331_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(dev);
> +
> +	clk_enable(rtap->clk);
> +	/* Check if the time is valid */
> +	if (readl(rtap->virtbase + COH901331_VALID)) {
> +		rtc_time_to_tm(readl(rtap->virtbase + COH901331_CUR_TIME), tm);
> +		clk_disable(rtap->clk);
> +		return 0;
> +	}
> +	/* We couldn't read the time, set default value */
> +	writel(76550400, rtap->virtbase + COH901331_SET_TIME);
> +	rtc_time_to_tm(76550400, tm);
> +	clk_disable(rtap->clk);

 why a default? why this specific default? what could cause a failure in reading? 

> +	return 0;
> +}

> +static int coh901331_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	unsigned long time;
> +	struct coh901331_port *rtap = dev_get_drvdata(dev);
> +
> +	rtc_tm_to_time(tm, &time);
> +	clk_enable(rtap->clk);
> +	writel(time, rtap->virtbase + COH901331_SET_TIME);
> +	clk_disable(rtap->clk);
> +
> +	return 0;
> +}

 given the type of rtc, you should implement set_mmss instead of set_time
 and get_mmss instead of get_time


> +static int coh901331_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(dev);
> +
> +	clk_enable(rtap->clk);
> +	rtc_time_to_tm(readl(rtap->virtbase + COH901331_ALARM), &alarm->time);
> +	alarm->pending = readl(rtap->virtbase + COH901331_IRQ_EVENT) & 1U;
> +	alarm->enabled = readl(rtap->virtbase + COH901331_IRQ_MASK) & 1U;
> +	clk_disable(rtap->clk);
> +
> +	return 0;
> +}
> +
> +static int coh901331_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(dev);
> +	unsigned long time;
> +
> +	rtc_tm_to_time(&alarm->time, &time);
> +	clk_enable(rtap->clk);
> +	writel(time, rtap->virtbase + COH901331_ALARM);
> +	writel(alarm->enabled, rtap->virtbase + COH901331_IRQ_MASK);
> +	clk_disable(rtap->clk);
> +
> +	return 0;
> +}
> +
> +static int coh901331_ioctl(struct device *dev, unsigned int cmd,
> +			   unsigned long arg)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(dev);
> +
> +	switch (cmd) {
> +	case RTC_AIE_OFF:
> +		clk_enable(rtap->clk);
> +		writel(0, rtap->virtbase + COH901331_IRQ_MASK);
> +		clk_disable(rtap->clk);
> +		return 0;
> +	case RTC_AIE_ON:
> +		clk_enable(rtap->clk);
> +		writel(1, rtap->virtbase + COH901331_IRQ_MASK);
> +		clk_disable(rtap->clk);
> +		return 0;
> +	}
> +
> +	return -ENOIOCTLCMD;
> +}

 unnecessary ioctl, please implement them via the ops structure.

> +static struct rtc_class_ops coh901331_ops = {
> +	.open = coh901331_open,
> +	.release = coh901331_release,
> +	.ioctl = coh901331_ioctl,
> +	.read_time = coh901331_read_time,
> +	.set_time = coh901331_set_time,
> +	.read_alarm = coh901331_read_alarm,
> +	.set_alarm = coh901331_set_alarm,
> +};
> +
> +static int __devexit coh901331_remove(struct platform_device *pdev)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(&pdev->dev);
> +
> +	if (rtap) {
> +		platform_set_drvdata(pdev, NULL);
 
 this is better done after unregister

> +		free_irq(rtap->irq, rtap);
> +		rtc_device_unregister(rtap->rtc);
> +		clk_put(rtap->clk);
> +		iounmap(rtap->virtbase);
> +		release_mem_region(rtap->phybase, rtap->physize);
> +		kfree(rtap);
> +	}
> +
> +	return 0;
> +}
> +
> +
> +static int __devinit coh901331_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct coh901331_port *rtap;
> +	struct resource *res;
> +
> +

 just a whiteline would do.

> +	rtap = kzalloc(sizeof(struct coh901331_port), GFP_KERNEL);
> +	if (!rtap) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}

 just return -ENOMEM;

> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		ret = -ENOMEM;
> +		goto out_no_resource;
> +	}

 -ENOMEM is inappropriate here.

> +	rtap->phybase = res->start;
> +	rtap->physize = res->end - res->start;

 use resource_size

> +	if (request_mem_region(rtap->phybase, rtap->physize,
> +			       "rtc-coh901331") == NULL) {
> +		ret = -EBUSY;
> +		goto out_no_memregion;
> +	}
> +
> +
> +	rtap->virtbase = ioremap(rtap->phybase, rtap->physize);
> +	if (!rtap->virtbase) {
> +		ret = -ENOMEM;
> +		goto out_no_remap;
> +	}
> +
> +	rtap->irq = platform_get_irq(pdev, 0);
> +	if (request_irq(rtap->irq, coh901331_interrupt, IRQF_DISABLED,
> +			"RTC COH 901 331 Alarm", rtap)) {
> +		ret = -EIO;
> +		goto out_no_irq;
> +	}
> +
> +	rtap->clk = clk_get(&pdev->dev, "RTC");
> +	if (IS_ERR(rtap->clk)) {
> +		ret = PTR_ERR(rtap->clk);
> +		dev_err(&pdev->dev, "could not get clock\n");
> +		goto out_no_clk;
> +	}
> +
> +	/* We enable/disable the clock only to assure it works */
> +	ret = clk_enable(rtap->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not enable clock\n");
> +		goto out_no_clk_enable;
> +	}
> +	clk_disable(rtap->clk);
> +
> +	rtap->rtc = rtc_device_register("coh901331", &pdev->dev, &coh901331_ops,
> +					 THIS_MODULE);
> +	if (IS_ERR(rtap->rtc)) {
> +		ret = PTR_ERR(rtap->rtc);
> +		goto out_no_rtc;
> +	}
> +
> +	platform_set_drvdata(pdev, rtap);
> +
> +	return 0;
> +
> + out_no_rtc:
> + out_no_clk_enable:
> +	clk_put(rtap->clk);
> + out_no_clk:
> +	free_irq(rtap->irq, rtap);
> + out_no_irq:
> +	iounmap(rtap->virtbase);
> + out_no_remap:
> +	platform_set_drvdata(pdev, NULL);
> + out_no_memregion:
> +	release_mem_region(rtap->phybase, SZ_4K);
> + out_no_resource:
> +	kfree(rtap);
> + out:
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PM
> +static int coh901331_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(&pdev->dev);
> +
> +	/*
> +	 * If this RTC alarm will be used for waking the system up,
> +	 * don't disable it of course. Else we just disable the alarm
> +	 * and await suspension.
> +	 */
> +	if (device_may_wakeup(&pdev->dev)) {
> +		enable_irq_wake(rtap->irq);
> +	} else {
> +		clk_enable(rtap->clk);
> +		rtap->irqmaskstore = readl(rtap->virtbase + COH901331_IRQ_MASK);
> +		writel(0, rtap->virtbase + COH901331_IRQ_MASK);
> +		clk_disable(rtap->clk);
> +	}
> +	return 0;
> +}
> +
> +static int coh901331_resume(struct platform_device *pdev)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(&pdev->dev);
> +
> +	if (device_may_wakeup(&pdev->dev))
> +		disable_irq_wake(rtap->irq);
> +	else
> +		clk_enable(rtap->clk);
> +		writel(rtap->irqmaskstore, rtap->virtbase + COH901331_IRQ_MASK);
> +		clk_disable(rtap->clk);
> +	return 0;
> +}
> +#else
> +#define coh901331_suspend NULL
> +#define coh901331_resume NULL
> +#endif
> +
> +static void coh901331_shutdown(struct platform_device *pdev)
> +{
> +	struct coh901331_port *rtap = dev_get_drvdata(&pdev->dev);
> +
> +	clk_enable(rtap->clk);
> +	writel(0, rtap->virtbase + COH901331_IRQ_MASK);
> +	clk_disable(rtap->clk);
> +}
> +
> +static struct platform_driver coh901331_driver = {
> +	.driver = {
> +		.name = "rtc-coh901331",
> +	},
> +	.probe = coh901331_probe,
> +	.remove = __devexit_p(coh901331_remove),
> +	.suspend = coh901331_suspend,
> +	.resume = coh901331_resume,
> +	.shutdown = coh901331_shutdown,
> +};
> +
> +static int __init coh901331_init(void)
> +{
> +	return platform_driver_register(&coh901331_driver);
> +}
> +
> +static void __exit coh901331_exit(void)
> +{
> +	platform_driver_unregister(&coh901331_driver);
> +}
> +
> +module_init(coh901331_init);
> +module_exit(coh901331_exit);
> +
> +MODULE_AUTHOR("Linus Walleij <linus.walleij@stericsson.com>");
> +MODULE_DESCRIPTION("ST-Ericsson AB COH 901 331 RTC Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.6.2.1
> 
> >

2009/5/4 Alessandro Zummo <alessandro.zummo@towertech.it>:

>> +     /* Check if the time is valid */
>> +     if (readl(rtap->virtbase + COH901331_VALID)) {
>> +             rtc_time_to_tm(readl(rtap->virtbase + COH901331_CUR_TIME), tm);
>> +             clk_disable(rtap->clk);
>> +             return 0;
>> +     }
>> +     /* We couldn't read the time, set default value */
>> +     writel(76550400, rtap->virtbase + COH901331_SET_TIME);
>> +     rtc_time_to_tm(76550400, tm);
>> +     clk_disable(rtap->clk);

>  why a default? why this specific default?

There needs to be a default (if even 0x00000000U) because this is not
a battery-backed RTC (we have another RTC which is battery and capacitor
backed too, will add code for that later). If the COH901331_VALID register
is not set, the value in COH901331_CUR_TIME is unreliable (can be just
noise).

Since I'm a solipsist I believe time begins with me so I set this to my
birthday, midnight UTC time. :-)

Actually it should be 76550400 in this time zone. I can put in a comment
for it, if you don't like easter eggs like that I can set it to 0 if you prefer.

>  what could cause a failure in reading?

The bit falls down on power loss. Just noise remains in the register when
it c

>> +static int coh901331_set_time(struct device *dev, struct rtc_time *tm)
>> (...)
>> +     rtc_tm_to_time(tm, &time);

>  given the type of rtc, you should implement set_mmss instead of set_time
>  and get_mmss instead of get_time

I converted set_time over to use set_mmss instead. But get_mmss is nowhere
to be found in the struct rtc_class_ops I have in my git trees (including
linux-next).

Do you have some special git with get_mmss() in rtc_class_ops that
I can clone and adjust the patch to?

I fixed all the other issues, so as soon as I can clear these remaining
issues you can have a finalized version.

Yours,
Linus Walleij

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---

Pinging on this (OK sorry for stressing you...)

Since .get_mmss isn't available anywhere, can I send a version
using the old .get_time to integrate for the time being?

Yours,
Linus Walleij

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
-~----------~----~----~----~------~----~------~--~---

On Fri, 8 May 2009 10:20:09 +0200
Linus Walleij <linus.ml.walleij@gmail.com> wrote:

> Pinging on this (OK sorry for stressing you...)
> 
> Since .get_mmss isn't available anywhere, can I send a version
> using the old .get_time to integrate for the time being?

 sure!

 bu sure to use rtc_Valid_tm