rtc: snvs: add Freescale rtc-snvs driver
Subsystems (2)
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
Add an RTC driver for Freescale Secure Non-Volatile Storage (SNVS) Low Power (LP) RTC.
--- Documentation/devicetree/bindings/rtc/snvs-rtc.txt | 19 + drivers/rtc/Kconfig | 10 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-snvs.c | 350 ++++++++++++++++++++ 4 files changed, 380 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/snvs-rtc.txt create mode 100644 drivers/rtc/rtc-snvs.c
Documentation/devicetree/bindings/rtc/snvs-rtc.txt new file +19 -0
@@ -0,0 +1,19 @@ +* Freescale Secure Non-Volatile Storage (SNVS) RTC + +Required properties: +- compatible: Should be "fsl,sec-v4.0-mon" + See Documentation/devicetree/bindings/crypto/fsl-sec4.txt for details. +- reg: Physical base address of the controller and length of memory mapped + region. +- interrupts: Two interrupt numbers to the cpu should be specified. First + interrupt number is the consolidated interrupt in non TrustZone. The second + interrupt number is the security interrupt in TrustZone. The number of cells + representing a interrupt depends on the parent interrupt controller. + +Example: + +snvs at 020cc000 { + compatible = "fsl,sec-v4.0-mon"; + reg = <0x020cc000 0x4000>; + interrupts = <0 19 0x04 0 20 0x04>; +};
drivers/rtc/Kconfig +10 -0
@@ -1087,4 +1087,14 @@ config RTC_DRV_MXC This driver can also be built as a module, if so, the module will be called "rtc-mxc". +config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXC + help + If you say yes here you get support for the Freescale SNVS + Low Power (LP) RTC module. + + This driver can also be built as a module, if so, the module + will be called "rtc-snvs". + endif # RTC_CLASS
drivers/rtc/Makefile +1 -0
@@ -95,6 +95,7 @@ obj-$(CONFIG_RTC_DRV_S35390A) += rtc-s35390a.o obj-$(CONFIG_RTC_DRV_S3C) += rtc-s3c.o obj-$(CONFIG_RTC_DRV_SA1100) += rtc-sa1100.o obj-$(CONFIG_RTC_DRV_SH) += rtc-sh.o +obj-$(CONFIG_RTC_DRV_SNVS) += rtc-snvs.o obj-$(CONFIG_RTC_DRV_SPEAR) += rtc-spear.o obj-$(CONFIG_RTC_DRV_STARFIRE) += rtc-starfire.o obj-$(CONFIG_RTC_DRV_STK17TA8) += rtc-stk17ta8.o
drivers/rtc/rtc-snvs.c new file +350 -0
350 lines not on this page · raw message
Signature · 1 line
1.7.5.4
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1092
Quoted 11 lines · Shawn Guo
drivers/rtc/Kconfig +3
-0@@ -1087,4 +1087,14 @@ config RTC_DRV_MXC This driver can also be built as a module, if so, the module will be called "rtc-mxc". +config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXC
If you want more build coverage you can depend on HAS_IOMEM and drop the ARCH_MXC part.
Quoted 12 lines
drivers/rtc/Kconfig +4
-0+ help + If you say yes here you get support for the Freescale SNVS + Low Power (LP) RTC module. +drivers/rtc/rtc-snvs.c new file +2
-0@@ -0,0 +1,350 @@ +/* +* Copyright (C) 2011-2012 Freescale Semiconductor, Inc.
drivers/rtc/rtc-snvs.c line 12
Quoted 10 lines
drivers/rtc/rtc-snvs.c new file +10
-0+* +* The code contained herein is licensed under the GNU General Public +* License. You may obtain a copy of the GNU General Public License +* Version 2 or later at the following locations: +* +* http://www.opensource.org/licenses/gpl-license.html +* http://www.gnu.org/copyleft/gpl.html +*/ + +#include <linux/delay.h>
Is this include used?
drivers/rtc/rtc-snvs.c line 212
Quoted 2 lines
drivers/rtc/rtc-snvs.c new file +2
-0+ +static struct rtc_class_ops snvs_rtc_ops = {
const?
drivers/rtc/rtc-snvs.c line 223
Quoted 4 lines
drivers/rtc/rtc-snvs.c new file +4
-0+static irqreturn_t snvs_rtc_irq_handler (int irq,void *dev_id) +{ +struct device *dev = dev_id; +struct snvs_rtc_data *data =dev_get_drvdata (dev);
Why not just pass the svsns_rtc_data to request_irq() so you can cast dev_id directly?
Quoted 6 lines
drivers/rtc/rtc-snvs.c new file +6
-0+static int __devinit snvs_rtc_probe (struct platform_device *pdev) +{ +struct snvs_rtc_data *data; +struct resource *res; +int ret; +
drivers/rtc/rtc-snvs.c line 259
Quoted 10 lines
drivers/rtc/rtc-snvs.c new file +10
-0+ data =devm_kzalloc (&pdev->dev,sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + data->ioaddr =devm_request_and_ioremap (&pdev->dev, res); +if (!data->ioaddr) +return -EADDRNOTAVAIL ; + + data->irq =platform_get_irq (pdev,0 );
What if this irq fails to exist?
drivers/rtc/rtc-snvs.c line 329
Quoted 1 line
drivers/rtc/rtc-snvs.c new file +1
-0+static const struct of_device_id __devinitdata snvs_dt_ids[] = {
Is this supposed to be __devinitconst?
drivers/rtc/rtc-snvs.c line 333
Quoted 4 lines
drivers/rtc/rtc-snvs.c new file +4
-0+ { .compatible = "fsl,sec-v4.0-mon" }, + {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, snvs_dt_ids);
Surround this of_device table in #ifdef CONFIG_OF?
drivers/rtc/rtc-snvs.c line 339
Quoted 6 lines
drivers/rtc/rtc-snvs.c new file +6
-0+ +static struct platform_driver snvs_rtc_driver = { + .driver = { + .name ="snvs_rtc" , + .owner =THIS_MODULE , + .of_match_table = snvs_dt_ids,
.of_match_table = of_match_ptr(svns_dt_ids)
drivers/rtc/rtc-snvs.c line 344
Quoted 5 lines
drivers/rtc/rtc-snvs.c new file +5
-0+ }, + .probe = snvs_rtc_probe, + .remove = __devexit_p(snvs_rtc_remove), + .suspend = snvs_rtc_suspend, + .resume = snvs_rtc_resume,
Can you use dev_pm_ops?
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
Thanks for looking at it, Stephen.
Quoted 9 lines · Stephen Boyd
Quoted 8 lines · Shawn Guo
drivers/rtc/Kconfig +0
-0@@ -1087,4 +1087,14 @@ config RTC_DRV_MXC This driver can also be built as a module, if so, the module will be called "rtc-mxc".
Quoted 7 lines
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1092
Quoted 3 lines
drivers/rtc/Kconfig +3
-0+config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXCIf you want more build coverage you can depend on
HAS_IOMEMand drop theARCH_MXCpart.
Dropping ARCH_MXC sounds like a good idea. But why do we need to depend on HAS_IOMEM? readl/writel are not always available?
I would add "depends on OF" though.
Quoted 16 lines
Quoted 16 lines
drivers/rtc/Kconfig +4
-0+ help + If you say yes here you get support for the Freescale SNVS + Low Power (LP) RTC module. +drivers/rtc/rtc-snvs.c new file +6
-0@@ -0,0 +1,350 @@ +/* +* Copyright (C) 2011-2012 Freescale Semiconductor, Inc. +* +* The code contained herein is licensed under the GNU General Public +* License. You may obtain a copy of the GNU General Public License +* Version 2 or later at the following locations:
Quoted 9 lines
drivers/rtc/rtc-snvs.c line 12
Quoted 6 lines
drivers/rtc/rtc-snvs.c new file +6
-0+* +* http://www.opensource.org/licenses/gpl-license.html +* http://www.gnu.org/copyleft/gpl.html +*/ + +#include <linux/delay.h> Is this include used?
No. It should be removed.
Quoted 5 lines
drivers/rtc/rtc-snvs.c line 212
Quoted 2 lines
drivers/rtc/rtc-snvs.c new file +2
-0+ +static struct rtc_class_ops snvs_rtc_ops = {const?
Indeed.
Quoted 8 lines
drivers/rtc/rtc-snvs.c line 223
Quoted 4 lines
drivers/rtc/rtc-snvs.c new file +4
-0+static irqreturn_t snvs_rtc_irq_handler (int irq,void *dev_id) +{ +struct device *dev = dev_id; +struct snvs_rtc_data *data =dev_get_drvdata (dev);Why not just pass the
svsns_rtc_datatorequest_irq()so you can castdev_iddirectly?
Because snvs_rtc_alarm_irq_enable(dev, 0) is being called in there with dev as the first parameter.
Quoted 10 lines
Quoted 10 lines
drivers/rtc/rtc-snvs.c new file +10
-0+static int __devinit snvs_rtc_probe (struct platform_device *pdev) +{ +struct snvs_rtc_data *data; +struct resource *res; +int ret; + + data =devm_kzalloc (&pdev->dev,sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; +
Quoted 9 lines
drivers/rtc/rtc-snvs.c line 259
Quoted 6 lines
drivers/rtc/rtc-snvs.c new file +6
-0+ res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + data->ioaddr =devm_request_and_ioremap (&pdev->dev, res); +if (!data->ioaddr) +return -EADDRNOTAVAIL ; + + data->irq =platform_get_irq (pdev,0 );What if this irq fails to exist?
The later devm_request_irq() call will report error then.
Quoted 5 lines
drivers/rtc/rtc-snvs.c line 329
Quoted 2 lines
drivers/rtc/rtc-snvs.c new file +1
-0+static const struct of_device_id __devinitdata snvs_dt_ids[] = {Is this supposed to be
__devinitconst?
Right.
Quoted 7 lines
drivers/rtc/rtc-snvs.c line 333
Quoted 4 lines
drivers/rtc/rtc-snvs.c new file +4
-0+ { .compatible = "fsl,sec-v4.0-mon" }, + {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, snvs_dt_ids);Surround this
of_devicetable in #ifdefCONFIG_OF?
Maybe not. I intend to have the driver OF only.
Quoted 9 lines
drivers/rtc/rtc-snvs.c line 339
Quoted 6 lines
drivers/rtc/rtc-snvs.c new file +6
-0+ +static struct platform_driver snvs_rtc_driver = { + .driver = { + .name ="snvs_rtc" , + .owner =THIS_MODULE , + .of_match_table = snvs_dt_ids,.of_match_table =
of_match_ptr(svns_dt_ids)
Ditto
Quoted 8 lines
drivers/rtc/rtc-snvs.c line 344
Quoted 5 lines
drivers/rtc/rtc-snvs.c new file +5
-0+ }, + .probe = snvs_rtc_probe, + .remove = __devexit_p(snvs_rtc_remove), + .suspend = snvs_rtc_suspend, + .resume = snvs_rtc_resume,Can you use
dev_pm_ops?
Ok.
Signature · 2 lines
Regards, Shawn
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
Quoted 10 lines · Shawn Guo
Quoted 9 lines · Stephen Boyd
Quoted 8 lines · Shawn Guo
drivers/rtc/Kconfig +0
-0@@ -1087,4 +1087,14 @@ config RTC_DRV_MXC This driver can also be built as a module, if so, the module will be called "rtc-mxc".
Quoted 8 lines
Quoted 6 lines
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1092
Quoted 3 lines
drivers/rtc/Kconfig +3
-0+config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXCIf you want more build coverage you can depend on
HAS_IOMEMand drop theARCH_MXCpart.Dropping
ARCH_MXCsounds like a good idea. But why do we need to depend onHAS_IOMEM? readl/writel are not always available?
Yes, that is my understanding.
Quoted 11 lines
I would add "depends on OF" though.
Quoted 7 lines
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1096
Quoted 4 lines
drivers/rtc/Kconfig +4
-0+static irqreturn_t snvs_rtc_irq_handler(int irq, void *dev_id) +{ + struct device *dev = dev_id; + struct snvs_rtc_data *data = dev_get_drvdata(dev);Why not just pass the
svsns_rtc_datatorequest_irq()so you can castdev_iddirectly?
Quoted 2 lines
Because
snvs_rtc_alarm_irq_enable(dev, 0)is being called in there with dev as the first parameter.
Ah, sorry for the noise.
Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
drivers/rtc/Kconfig +9
-0+static int __devinit snvs_rtc_probe(struct platform_device *pdev) +{ + struct snvs_rtc_data *data; + struct resource *res; + int ret; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM;Quoted 9 lines
drivers/rtc/Kconfig
Quoted 7 lines
drivers/rtc/Kconfig +7
-0+ + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + data->ioaddr = devm_request_and_ioremap(&pdev->dev, res); + if (!data->ioaddr) + return -EADDRNOTAVAIL; + + data->irq = platform_get_irq(pdev, 0);What if this irq fails to exist?
Quoted 1 line
The later
devm_request_irq()call will report error then.
request_irq() takes an unsigned int. Are you relying on there being no descriptor for that extremely high irq number? Or maybe I'm missing something in the code.
Quoted 9 lines
Quoted 7 lines
drivers/rtc/Kconfig
Quoted 5 lines
drivers/rtc/Kconfig +4
-0+ { .compatible = "fsl,sec-v4.0-mon" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, snvs_dt_ids);Surround this
of_devicetable in #ifdefCONFIG_OF?Maybe not. I intend to have the driver OF only.
Ok. I guess if someone wants to use it non-OF they can always send a patch.
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
In reply to Shawn GuoQuoted 18 lines · On Tue, 3 Jul 2012 00:09:15 +0800 Shawn Guo <[email protected]> wrote:
create mode 100644 Documentation/devicetree/bindings/rtc/snvs-rtc.txt create mode 100644 drivers/rtc/rtc-snvs.cDocumentation/devicetree/bindings/rtc/snvs-rtc.txt new file +9
-0@@ -0,0 +1,19 @@ +* Freescale Secure Non-Volatile Storage (SNVS) RTC + +Required properties: +- compatible: Should be "fsl,sec-v4.0-mon" + See Documentation/devicetree/bindings/crypto/fsl-sec4.txt for details. +- reg: Physical base address of the controller and length of memory mapped + region. +- interrupts: Two interrupt numbers to the cpu should be specified. First + interrupt number is the consolidated interrupt in non TrustZone. The second
Documentation/devicetree/bindings/rtc/snvs-rtc.txt line 19
Quoted 10 lines
Documentation/devicetree/bindings/rtc/snvs-rtc.txt new file +10
-0+ interrupt number is the security interrupt in TrustZone. The number of cells + representing a interrupt depends on the parent interrupt controller. + +Example: + +snvs at 020cc000 { + compatible = "fsl,sec-v4.0-mon"; + reg = <0x020cc000 0x4000>; + interrupts = <0 19 0x04 0 20 0x04>; +};
Instead of duplicating the binding documentation entry, can this be merged in with crypto/fsl-sec4.txt? Also, don't we need the rtc to be a 'sub-device' of "fsl,sec-vX.Y-mon", and to have its own compatible, e.g., "fsl,sec-v4.0-mon-rtc-lp"?
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1092
Quoted 11 lines
drivers/rtc/Kconfig +3
-0@@ -1087,4 +1087,14 @@ config RTC_DRV_MXC This driver can also be built as a module, if so, the module will be called "rtc-mxc". +config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXC
should also depend on || FSL_SOC - it was mentioned in the review of the last version of this patch that this h/w exists on other, non-ARM based, SoCs.
Kim
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
Quoted 2 lines · Kim Phillips
Instead of duplicating the binding documentation entry, can this be merged in with
crypto/fsl-sec4.txt?
Yes. We can do that. But I would still keep a plain Documentation/devicetree/bindings/rtc/snvs-rtc.txt there with only a pointer to Documentation/devicetree/bindings/crypto/fsl-sec4.txt. Otherwise, I'm afraid that people will hardly know the bindings of snvs-rtc is documented in fsl-sec4.txt.
Quoted 4 lines
Also, don't we need the rtc to
be a 'sub-device' of "fsl,sec-vX.Y-mon", and to have its own compatible, e.g., "fsl,sec-v4.0-mon-rtc-lp"?
Sounds sensible. But how will reg and interrupts properties be documented? The registers for lp-rtc is not so consecutive and interrupts are shared by hp and lp.
Quoted 8 lines
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1092
Quoted 3 lines
drivers/rtc/Kconfig +3
-0+config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXCshould also depend on ||
FSL_SOC- it was mentioned in the review of the last version of this patch that this h/w exists on other, non-ARM based, SoCs.
I would not do that until the driver is tested on FSL_SOC. Also I'm considering to take Stephen's suggestion to drop even ARCH_MXC.
Signature · 2 lines
Regards, Shawn
PATCH 1/1 rtc: snvs: add Freescale rtc-snvs driver
Quoted 9 lines · On Tue, 3 Jul 2012 23:04:08 +0800 Shawn Guo <[email protected]> wrote:
Quoted 2 lines · Kim Phillips
Instead of duplicating the binding documentation entry, can this be merged in with
crypto/fsl-sec4.txt?Yes. We can do that. But I would still keep a plain
Documentation/devicetree/bindings/rtc/snvs-rtc.txtthere with only a pointer toDocumentation/devicetree/bindings/crypto/fsl-sec4.txt. Otherwise, I'm afraid that people will hardly know the bindings of snvs-rtc is documented in fsl-sec4.txt.
that's understandable.
Quoted 6 lines
Quoted 4 lines
Also, don't we need the rtc to
be a 'sub-device' of "fsl,sec-vX.Y-mon", and to have its own compatible, e.g., "fsl,sec-v4.0-mon-rtc-lp"?Sounds sensible. But how will reg and interrupts properties be documented? The registers for lp-rtc is not so consecutive and
LP registers go from 0x34 (LPLR) to 0x8b (LPZMKR7), consecutively. HP registers technically would have a split range, but this patch doesn't address the HP part.
Quoted 1 line
interrupts are shared by hp and lp.
that's why we return IRQ_NONE, so we can request_irq IRQF_SHARED.
Quoted 9 lines
Quoted 8 lines
drivers/rtc/Kconfig config RTC_DRV_MXC · line 1092
Quoted 3 lines
drivers/rtc/Kconfig +3
-0+config RTC_DRV_SNVS + tristate "Freescale SNVS RTC support" + depends on ARCH_MXCshould also depend on ||
FSL_SOC- it was mentioned in the review of the last version of this patch that this h/w exists on other, non-ARM based, SoCs.I would not do that until the driver is tested on
FSL_SOC.
I can test it.
Quoted 2 lines
Also I'm
considering to take Stephen's suggestion to drop evenARCH_MXC.
fine with me.
Kim