The patch series remove the incorrect patch, and submit the correct one.
Chao Xie (2):
Revert "drivers/rtc/rtc-pxa.c: fix set time sync time issue"
rtc: pxa: add pxa95x rtc support
drivers/rtc/rtc-pxa.c | 141 +++++++++++++++++++++++++++--------------------
include/linux/rtc-pxa.h | 18 ------
2 files changed, 82 insertions(+), 77 deletions(-)
delete mode 100644 include/linux/rtc-pxa.h
--
1.7.4.1
This patch have something wrong.
1.The pxa_sync_time API is not needed. The RTC sync can be done
by user space applications, so in kernel this API is not needed.
2.The pxa_rtc_open can not be deleted. This change has been
declined during review at the mail list
3.Based on specification and talked with owner of driver, it
does not matter to set WOM and DOW.
Only need to keep PSBR control for pxa95x
This reverts commit 57489fabb7f3fc02483df2125fdbfb8b1bb1fcd8.
---
drivers/rtc/rtc-pxa.c | 63 +++++-----------------------------------------
include/linux/rtc-pxa.h | 18 -------------
2 files changed, 7 insertions(+), 74 deletions(-)
delete mode 100644 include/linux/rtc-pxa.h
diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index a2c75c1..e261e2f 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -81,29 +81,22 @@
#define RYAR1 0x1c
#define RTCPICR 0x34
#define PIAR 0x38
-#define PSBR_RTC 0x00
#define rtc_readl(pxa_rtc, reg) \
__raw_readl((pxa_rtc)->base + (reg))
#define rtc_writel(pxa_rtc, reg, value) \
__raw_writel((value), (pxa_rtc)->base + (reg))
-#define rtc_readl_psbr(pxa_rtc, reg) \
- __raw_readl((pxa_rtc)->base_psbr + (reg))
-#define rtc_writel_psbr(pxa_rtc, reg, value) \
- __raw_writel((value), (pxa_rtc)->base_psbr + (reg))
struct pxa_rtc {
struct resource *ress;
- struct resource *ress_psbr;
void __iomem *base;
- void __iomem *base_psbr;
int irq_1Hz;
int irq_Alrm;
struct rtc_device *rtc;
spinlock_t lock; /* Protects this structure */
};
-static struct pxa_rtc *rtc_info;
+
static u32 ryxr_calc(struct rtc_time *tm)
{
return ((tm->tm_year + 1900) << RYxR_YEAR_S)
@@ -125,7 +118,7 @@ static void tm_calc(u32 rycr, u32 rdcr, struct rtc_time *tm)
tm->tm_year = ((rycr & RYxR_YEAR_MASK) >> RYxR_YEAR_S) - 1900;
tm->tm_mon = (((rycr & RYxR_MONTH_MASK) >> RYxR_MONTH_S)) - 1;
tm->tm_mday = (rycr & RYxR_DAY_MASK);
- tm->tm_wday = ((rdcr & RDxR_DOW_MASK) >> RDxR_DOW_S) - 1;
+ tm->tm_wday = ((rycr & RDxR_DOW_MASK) >> RDxR_DOW_S) - 1;
tm->tm_hour = (rdcr & RDxR_HOUR_MASK) >> RDxR_HOUR_S;
tm->tm_min = (rdcr & RDxR_MIN_MASK) >> RDxR_MIN_S;
tm->tm_sec = rdcr & RDxR_SEC_MASK;
@@ -183,6 +176,7 @@ static irqreturn_t pxa_rtc_irq(int irq, void *dev_id)
/* enable back rtc interrupts */
rtc_writel(pxa_rtc, RTSR, rtsr & ~RTSR_TRIG_MASK);
+
spin_unlock(&pxa_rtc->lock);
return IRQ_HANDLED;
}
@@ -257,45 +251,12 @@ static int pxa_rtc_read_time(struct device *dev, struct rtc_time *tm)
static int pxa_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct pxa_rtc *pxa_rtc = dev_get_drvdata(dev);
- /* sequence to wirte pxa rtc register RCNR RDCR RYCR is
- *1. set PSBR[RWE] bit, take 2x32-khz to complete
- *2. write to RTC register,take 2x32-khz to complete
- *3. clear PSBR[RWE] bit,take 2x32-khz to complete
- */
- if ((tm->tm_year < 70) || (tm->tm_year > 138))
- return -EINVAL;
- rtc_writel_psbr(rtc_info, PSBR_RTC, 0x01);
- udelay(100);
+
rtc_writel(pxa_rtc, RYCR, ryxr_calc(tm));
rtc_writel(pxa_rtc, RDCR, rdxr_calc(tm));
- udelay(100);
- rtc_writel_psbr(rtc_info, PSBR_RTC, 0x00);
- udelay(100);
- pxa_rtc_read_time(dev, tm);
- dev_info(dev, "tm.year = %d, tm.month = %d, tm.day = %d\n",
- tm->tm_year + 1900, tm->tm_mon, tm->tm_mday);
- return 0;
-}
-int pxa_rtc_sync_time(unsigned int ticks)
-{
- /* sequence to wirte pxa rtc register RCNR RDCR RYCR is
- *1. set PSBR[RWE] bit, take 2x32-khz to complete
- *2. write to RTC register,take 2x32-khz to complete
- *3. clear PSBR[RWE] bit,take 2x32-khz to complete
- */
- struct rtc_time tm;
- rtc_time_to_tm(ticks, &tm);
- rtc_writel_psbr(rtc_info, PSBR_RTC, 0x01);
- udelay(100);
- rtc_writel(rtc_info, RYCR, ryxr_calc(&tm));
- rtc_writel(rtc_info, RDCR, rdxr_calc(&tm));
- udelay(100);
- rtc_writel_psbr(rtc_info, PSBR_RTC, 0x00);
- udelay(100);
return 0;
}
-EXPORT_SYMBOL(pxa_rtc_sync_time);
static int pxa_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
{
@@ -367,7 +328,7 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
pxa_rtc = kzalloc(sizeof(struct pxa_rtc), GFP_KERNEL);
if (!pxa_rtc)
return -ENOMEM;
- rtc_info = pxa_rtc;
+
spin_lock_init(&pxa_rtc->lock);
platform_set_drvdata(pdev, pxa_rtc);
@@ -377,11 +338,6 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
dev_err(dev, "No I/O memory resource defined\n");
goto err_ress;
}
- pxa_rtc->ress_psbr = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (!pxa_rtc->ress_psbr) {
- dev_err(dev, "No I/O memory resource defined\n");
- goto err_ress;
- }
pxa_rtc->irq_1Hz = platform_get_irq(pdev, 0);
if (pxa_rtc->irq_1Hz < 0) {
@@ -393,6 +349,7 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
dev_err(dev, "No alarm IRQ resource defined\n");
goto err_ress;
}
+ pxa_rtc_open(dev);
ret = -ENOMEM;
pxa_rtc->base = ioremap(pxa_rtc->ress->start,
resource_size(pxa_rtc->ress));
@@ -401,12 +358,6 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
goto err_map;
}
- pxa_rtc->base_psbr = ioremap(pxa_rtc->ress_psbr->start,
- resource_size(pxa_rtc->ress_psbr));
- if (!pxa_rtc->base_psbr) {
- dev_err(&pdev->dev, "Unable to map pxa RTC PSBR I/O memory\n");
- goto err_map;
- }
/*
* If the clock divider is uninitialized then reset it to the
* default value to get the 1Hz clock.
@@ -429,7 +380,7 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
}
device_init_wakeup(dev, 1);
- pxa_rtc_open(dev);
+
return 0;
err_rtc_reg:
diff --git a/include/linux/rtc-pxa.h b/include/linux/rtc-pxa.h
deleted file mode 100644
index 71bc45f..0000000
--- a/include/linux/rtc-pxa.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * include/linux/rtc-pxa.h
- *
- * RTC PXA Header file
- *
- * Copyright (C) 2010 Marvell International Ltd.
- *
- * 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.
- */
-
-#ifndef __LINUX_RTC_PXA_H
-#define __LINUX_RTC_PXA_H
-
-extern int pxa_rtc_sync_time(unsigned int ticks);
-
-#endif /* __LINUX_RTC_PXA_H */
--
1.7.4.1
the pxa95x rtc need access PBSR register before write to RTTR, RCNR, RDCR, and RYCR registers.
Signed-off-by: Chao Xie <[email protected]>
---
drivers/rtc/rtc-pxa.c | 96 ++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index e261e2f..4ffdd57 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -82,14 +82,28 @@
#define RTCPICR 0x34
#define PIAR 0x38
+#define PSBR_RTC 0x00
+
#define rtc_readl(pxa_rtc, reg) \
__raw_readl((pxa_rtc)->base + (reg))
#define rtc_writel(pxa_rtc, reg, value) \
__raw_writel((value), (pxa_rtc)->base + (reg))
-
+#define rtc_readl_psbr(pxa_rtc, reg) \
+ __raw_readl((pxa_rtc)->base_psbr + (reg))
+#define rtc_writel_psbr(pxa_rtc, reg, value) \
+ __raw_writel((value), (pxa_rtc)->base_psbr + (reg))
+
+enum {
+ RTC_PXA27X,
+ RTC_PXA95X,
+};
+
struct pxa_rtc {
struct resource *ress;
+ struct resource *ress_psbr;
+ unsigned int id;
void __iomem *base;
+ void __iomem *base_psbr;
int irq_1Hz;
int irq_Alrm;
struct rtc_device *rtc;
@@ -252,9 +266,26 @@ static int pxa_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct pxa_rtc *pxa_rtc = dev_get_drvdata(dev);
+ /*
+ * sequence to wirte pxa rtc register RCNR RDCR RYCR is
+ * 1. set PSBR[RWE] bit, take 2x32-khz to complete
+ * 2. write to RTC register,take 2x32-khz to complete
+ * 3. clear PSBR[RWE] bit,take 2x32-khz to complete
+ */
+ if (pxa_rtc->id == RTC_PXA95X) {
+ rtc_writel_psbr(pxa_rtc, PSBR_RTC, 0x01);
+ usleep_range(100, 120);
+ }
+
rtc_writel(pxa_rtc, RYCR, ryxr_calc(tm));
rtc_writel(pxa_rtc, RDCR, rdxr_calc(tm));
+ if (pxa_rtc->id == RTC_PXA95X) {
+ usleep_range(100, 120);
+ rtc_writel_psbr(pxa_rtc, PSBR_RTC, 0x00);
+ usleep_range(100, 120);
+ }
+
return 0;
}
@@ -318,6 +349,20 @@ static const struct rtc_class_ops pxa_rtc_ops = {
.proc = pxa_rtc_proc,
};
+static struct of_device_id pxa_rtc_dt_ids[] = {
+ { .compatible = "marvell,pxa-rtc", .data = (void *)RTC_PXA27X },
+ { .compatible = "marvell,pxa95x-rtc", .data = (void *)RTC_PXA95X },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pxa_rtc_dt_ids);
+
+static const struct platform_device_id pxa_rtc_id_table[] = {
+ { "pxa-rtc", RTC_PXA27X },
+ { "pxa95x-rtc", RTC_PXA95X },
+ { },
+};
+MODULE_DEVICE_TABLE(platform, pxa_rtc_id_table);
+
static int __init pxa_rtc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -332,13 +377,34 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
spin_lock_init(&pxa_rtc->lock);
platform_set_drvdata(pdev, pxa_rtc);
+ if (pdev->dev.of_node) {
+ const struct of_device_id *of_id =
+ of_match_device(pxa_rtc_dt_ids, &pdev->dev);
+
+ pxa_rtc->id = (unsigned int)(of_id->data);
+ } else {
+ const struct platform_device_id *id =
+ platform_get_device_id(pdev);
+
+ pxa_rtc->id = id->driver_data;
+ }
+
ret = -ENXIO;
pxa_rtc->ress = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!pxa_rtc->ress) {
- dev_err(dev, "No I/O memory resource defined\n");
+ dev_err(dev, "No I/O memory resource(id=0) defined\n");
goto err_ress;
}
+ if (pxa_rtc->id == RTC_PXA95X) {
+ pxa_rtc->ress_psbr =
+ platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!pxa_rtc->ress_psbr) {
+ dev_err(dev, "No I/O memory resource(id=1) defined\n");
+ goto err_ress;
+ }
+ }
+
pxa_rtc->irq_1Hz = platform_get_irq(pdev, 0);
if (pxa_rtc->irq_1Hz < 0) {
dev_err(dev, "No 1Hz IRQ resource defined\n");
@@ -355,7 +421,17 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
resource_size(pxa_rtc->ress));
if (!pxa_rtc->base) {
dev_err(&pdev->dev, "Unable to map pxa RTC I/O memory\n");
- goto err_map;
+ goto err_map_base;
+ }
+
+ if (pxa_rtc->id == RTC_PXA95X) {
+ pxa_rtc->base_psbr = ioremap(pxa_rtc->ress_psbr->start,
+ resource_size(pxa_rtc->ress_psbr));
+ if (!pxa_rtc->base_psbr) {
+ dev_err(&pdev->dev,
+ "Unable to map pxa RTC PSBR I/O memory\n");
+ goto err_map_base_psbr;
+ }
}
/*
@@ -384,9 +460,12 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
return 0;
err_rtc_reg:
+ if (pxa_rtc->id == RTC_PXA95X)
+ iounmap(pxa_rtc->base_psbr);
+err_map_base_psbr:
iounmap(pxa_rtc->base);
+err_map_base:
err_ress:
-err_map:
kfree(pxa_rtc);
return ret;
}
@@ -409,14 +488,6 @@ static int __exit pxa_rtc_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_OF
-static struct of_device_id pxa_rtc_dt_ids[] = {
- { .compatible = "marvell,pxa-rtc" },
- {}
-};
-MODULE_DEVICE_TABLE(of, pxa_rtc_dt_ids);
-#endif
-
#ifdef CONFIG_PM_SLEEP
static int pxa_rtc_suspend(struct device *dev)
{
@@ -446,6 +517,7 @@ static struct platform_driver pxa_rtc_driver = {
.of_match_table = of_match_ptr(pxa_rtc_dt_ids),
.pm = &pxa_rtc_pm_ops,
},
+ .id_table = pxa_rtc_id_table,
};
module_platform_driver_probe(pxa_rtc_driver, pxa_rtc_probe);
--
1.7.4.1
On Tue, May 7, 2013 at 9:17 AM, Chao Xie <[email protected]> wrote:
> the pxa95x rtc need access PBSR register before write to RTTR, RCNR, RDCR, and RYCR registers.
>
> Signed-off-by: Chao Xie <[email protected]>
> ---
Do we really need this for other SoC? PXA95x is removed already since
it's a block on supporting
multi-platform & not used widely.
Regards
Haojian
Chao Xie <[email protected]> writes:
> This patch have something wrong.
> 1.The pxa_sync_time API is not needed. The RTC sync can be done
> by user space applications, so in kernel this API is not needed.
> 2.The pxa_rtc_open can not be deleted. This change has been
> declined during review at the mail list
> 3.Based on specification and talked with owner of driver, it
> does not matter to set WOM and DOW.
>
> Only need to keep PSBR control for pxa95x
>
> This reverts commit 57489fabb7f3fc02483df2125fdbfb8b1bb1fcd8.
This commit doesn't exist in Linus's tree. Maybe in linux-next ?
Anyway, as you are reverting, please consider this :
the commit that exists is : c4243de70f7d536d95196b8a31539298bb15238c
"drivers/rtc/rtc-pxa.c: drivers/rtc/rtc-pxa.c: fix alarm not match issue".
This commit should be reverted :
- because it's wrong :
> tm->tm_wday = ((rycr & RDxR_DOW_MASK) >> RDxR_DOW_S) - 1;
See how RYCR is used with a RDCR mask ...
- because as maintainer I have already nacked this kind of patch :
https://patchwork.kernel.org/patch/1819851/
The reasons given here are still valid AFAIK.
I'm pretty sure I didn't receive this patch in my mailbox. After I refused
ChaoXie first patch, the second "same approach" patch from Liangs was sent in my
back. I don't understand how that could happen ...
Liangs, please make sure to add me to the reviewers next time ?
Cheers.
--
Robert