linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove
@ 2012-12-05  6:49 Chao Xie
  2012-12-05  6:49 ` [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support Chao Xie
  2012-12-05  7:05 ` [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove Haojian Zhuang
  0 siblings, 2 replies; 5+ messages in thread
From: Chao Xie @ 2012-12-05  6:49 UTC (permalink / raw)
  To: haojian.zhuang, xiechao.mail, linux, a.zummo, rtc-linux,
	linux-kernel, linux-arm-kernel, robert.jarzmik
  Cc: Chao Xie

The original sa1100_rtc_open/sa1100_rtc_release will be called
when the /dev/rtc0 is opened or closed.
In fact, these two functions will enable/disable the clock.
Disabling clock will make rtc not work.
So only enable/disable clock when probe/remove the device.

Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
 drivers/rtc/rtc-sa1100.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index 50a5c4a..34f66b8 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -108,9 +108,6 @@ static int sa1100_rtc_open(struct device *dev)
 	struct rtc_device *rtc = info->rtc;
 	int ret;
 
-	ret = clk_prepare_enable(info->clk);
-	if (ret)
-		goto fail_clk;
 	ret = request_irq(info->irq_1hz, sa1100_rtc_interrupt, 0, "rtc 1Hz", dev);
 	if (ret) {
 		dev_err(dev, "IRQ %d already in use.\n", info->irq_1hz);
@@ -130,7 +127,6 @@ static int sa1100_rtc_open(struct device *dev)
 	free_irq(info->irq_1hz, dev);
  fail_ui:
 	clk_disable_unprepare(info->clk);
- fail_clk:
 	return ret;
 }
 
@@ -144,7 +140,6 @@ static void sa1100_rtc_release(struct device *dev)
 
 	free_irq(info->irq_alarm, dev);
 	free_irq(info->irq_1hz, dev);
-	clk_disable_unprepare(info->clk);
 }
 
 static int sa1100_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
@@ -253,6 +248,9 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 	spin_lock_init(&info->lock);
 	platform_set_drvdata(pdev, info);
 
+	ret = clk_prepare_enable(info->clk);
+	if (ret)
+		goto err_enable_clk;
 	/*
 	 * According to the manual we should be able to let RTTR be zero
 	 * and then a default diviser for a 32.768KHz clock is used.
@@ -305,6 +303,8 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 
 	return 0;
 err_dev:
+	clk_disable_unprepare(info->clk);
+err_enable_clk:
 	platform_set_drvdata(pdev, NULL);
 	clk_put(info->clk);
 err_clk:
@@ -318,6 +318,7 @@ static int sa1100_rtc_remove(struct platform_device *pdev)
 
 	if (info) {
 		rtc_device_unregister(info->rtc);
+		clk_disable_unprepare(info->clk);
 		clk_put(info->clk);
 		platform_set_drvdata(pdev, NULL);
 		kfree(info);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support
  2012-12-05  6:49 [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove Chao Xie
@ 2012-12-05  6:49 ` Chao Xie
  2012-12-05  7:04   ` Haojian Zhuang
  2012-12-05  7:05 ` [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove Haojian Zhuang
  1 sibling, 1 reply; 5+ messages in thread
From: Chao Xie @ 2012-12-05  6:49 UTC (permalink / raw)
  To: haojian.zhuang, xiechao.mail, linux, a.zummo, rtc-linux,
	linux-kernel, linux-arm-kernel, robert.jarzmik
  Cc: Chao Xie

the pxa95x rtc need access PBSR register before write to
RTTR, RCNR, RDCR, and RYCR registers.

Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
 drivers/rtc/rtc-pxa.c |  100 +++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index f771b2e..aee23cb 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/delay.h>
 
 #include <mach/hardware.h>
 
@@ -77,14 +78,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;
@@ -242,9 +257,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);
+		udelay(100);
+	}
+
 	rtc_writel(pxa_rtc, RYCR, ryxr_calc(tm));
 	rtc_writel(pxa_rtc, RDCR, rdxr_calc(tm));
 
+	if (pxa_rtc->id == RTC_PXA95X) {
+		udelay(100);
+		rtc_writel_psbr(pxa_rtc, PSBR_RTC, 0x00);
+		udelay(100);
+	}
+
 	return 0;
 }
 
@@ -310,6 +342,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;
@@ -324,13 +370,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");
@@ -347,7 +414,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;
+		}
 	}
 
 	/*
@@ -376,9 +453,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;
 }
@@ -398,14 +478,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
 static int pxa_rtc_suspend(struct device *dev)
 {
@@ -440,14 +512,12 @@ static struct platform_driver pxa_rtc_driver = {
 		.pm	= &pxa_rtc_pm_ops,
 #endif
 	},
+	.id_table	= pxa_rtc_id_table,
 };
 
 static int __init pxa_rtc_init(void)
 {
-	if (cpu_is_pxa27x() || cpu_is_pxa3xx())
-		return platform_driver_probe(&pxa_rtc_driver, pxa_rtc_probe);
-
-	return -ENODEV;
+	return platform_driver_probe(&pxa_rtc_driver, pxa_rtc_probe);
 }
 
 static void __exit pxa_rtc_exit(void)
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support
  2012-12-05  6:49 ` [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support Chao Xie
@ 2012-12-05  7:04   ` Haojian Zhuang
  2012-12-06  1:28     ` Chao Xie
  0 siblings, 1 reply; 5+ messages in thread
From: Haojian Zhuang @ 2012-12-05  7:04 UTC (permalink / raw)
  To: Chao Xie
  Cc: xie chao, Linux Russell, Alessandro Zummo, rtc-linux,
	linux-kernel, linux-arm-kernel, Robert Jarzmik

On Wed, Dec 5, 2012 at 2:49 PM, Chao Xie <chao.xie@marvell.com> wrote:
> the pxa95x rtc need access PBSR register before write to
> RTTR, RCNR, RDCR, and RYCR registers.
>
> Signed-off-by: Chao Xie <chao.xie@marvell.com>
> ---
>  drivers/rtc/rtc-pxa.c |  100 +++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
> index f771b2e..aee23cb 100644
> --- a/drivers/rtc/rtc-pxa.c
> +++ b/drivers/rtc/rtc-pxa.c
> @@ -29,6 +29,7 @@
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/delay.h>
>
>  #include <mach/hardware.h>
>
> @@ -77,14 +78,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;
> @@ -242,9 +257,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);
> +               udelay(100);
> +       }

How about define PSBP operation as a new clock, rtc interface clock.
Then you could put
the enable/disable into clock framework. And you only need to check
whether the interface
clock is NULL or not at here. If it's available, you can call
clk_prepare_enable().

> +
>         rtc_writel(pxa_rtc, RYCR, ryxr_calc(tm));
>         rtc_writel(pxa_rtc, RDCR, rdxr_calc(tm));
>
> +       if (pxa_rtc->id == RTC_PXA95X) {
> +               udelay(100);
> +               rtc_writel_psbr(pxa_rtc, PSBR_RTC, 0x00);
> +               udelay(100);
> +       }
> +
>         return 0;
>  }
>
> @@ -310,6 +342,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;
> @@ -324,13 +370,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");
> @@ -347,7 +414,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;
> +               }
>         }
>
>         /*
> @@ -376,9 +453,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;
>  }
> @@ -398,14 +478,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
>  static int pxa_rtc_suspend(struct device *dev)
>  {
> @@ -440,14 +512,12 @@ static struct platform_driver pxa_rtc_driver = {
>                 .pm     = &pxa_rtc_pm_ops,
>  #endif
>         },
> +       .id_table       = pxa_rtc_id_table,
>  };
>
>  static int __init pxa_rtc_init(void)
>  {
> -       if (cpu_is_pxa27x() || cpu_is_pxa3xx())
> -               return platform_driver_probe(&pxa_rtc_driver, pxa_rtc_probe);
> -
> -       return -ENODEV;
> +       return platform_driver_probe(&pxa_rtc_driver, pxa_rtc_probe);
>  }
>
>  static void __exit pxa_rtc_exit(void)
> --
> 1.7.4.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove
  2012-12-05  6:49 [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove Chao Xie
  2012-12-05  6:49 ` [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support Chao Xie
@ 2012-12-05  7:05 ` Haojian Zhuang
  1 sibling, 0 replies; 5+ messages in thread
From: Haojian Zhuang @ 2012-12-05  7:05 UTC (permalink / raw)
  To: Chao Xie
  Cc: xie chao, Linux Russell, Alessandro Zummo, rtc-linux,
	linux-kernel, linux-arm-kernel, Robert Jarzmik

On Wed, Dec 5, 2012 at 2:49 PM, Chao Xie <chao.xie@marvell.com> wrote:
> The original sa1100_rtc_open/sa1100_rtc_release will be called
> when the /dev/rtc0 is opened or closed.
> In fact, these two functions will enable/disable the clock.
> Disabling clock will make rtc not work.
> So only enable/disable clock when probe/remove the device.
>
> Signed-off-by: Chao Xie <chao.xie@marvell.com>
> ---
>  drivers/rtc/rtc-sa1100.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
> index 50a5c4a..34f66b8 100644
> --- a/drivers/rtc/rtc-sa1100.c
> +++ b/drivers/rtc/rtc-sa1100.c
> @@ -108,9 +108,6 @@ static int sa1100_rtc_open(struct device *dev)
>         struct rtc_device *rtc = info->rtc;
>         int ret;
>
> -       ret = clk_prepare_enable(info->clk);
> -       if (ret)
> -               goto fail_clk;
>         ret = request_irq(info->irq_1hz, sa1100_rtc_interrupt, 0, "rtc 1Hz", dev);
>         if (ret) {
>                 dev_err(dev, "IRQ %d already in use.\n", info->irq_1hz);
> @@ -130,7 +127,6 @@ static int sa1100_rtc_open(struct device *dev)
>         free_irq(info->irq_1hz, dev);
>   fail_ui:
>         clk_disable_unprepare(info->clk);
> - fail_clk:
>         return ret;
>  }
>
> @@ -144,7 +140,6 @@ static void sa1100_rtc_release(struct device *dev)
>
>         free_irq(info->irq_alarm, dev);
>         free_irq(info->irq_1hz, dev);
> -       clk_disable_unprepare(info->clk);
>  }
>
>  static int sa1100_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> @@ -253,6 +248,9 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
>         spin_lock_init(&info->lock);
>         platform_set_drvdata(pdev, info);
>
> +       ret = clk_prepare_enable(info->clk);
> +       if (ret)
> +               goto err_enable_clk;
>         /*
>          * According to the manual we should be able to let RTTR be zero
>          * and then a default diviser for a 32.768KHz clock is used.
> @@ -305,6 +303,8 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
>
>         return 0;
>  err_dev:
> +       clk_disable_unprepare(info->clk);
> +err_enable_clk:
>         platform_set_drvdata(pdev, NULL);
>         clk_put(info->clk);
>  err_clk:
> @@ -318,6 +318,7 @@ static int sa1100_rtc_remove(struct platform_device *pdev)
>
>         if (info) {
>                 rtc_device_unregister(info->rtc);
> +               clk_disable_unprepare(info->clk);
>                 clk_put(info->clk);
>                 platform_set_drvdata(pdev, NULL);
>                 kfree(info);
> --
> 1.7.4.1
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support
  2012-12-05  7:04   ` Haojian Zhuang
@ 2012-12-06  1:28     ` Chao Xie
  0 siblings, 0 replies; 5+ messages in thread
From: Chao Xie @ 2012-12-06  1:28 UTC (permalink / raw)
  To: Haojian Zhuang
  Cc: Chao Xie, Linux Russell, Alessandro Zummo, rtc-linux,
	linux-kernel, linux-arm-kernel, Robert Jarzmik

On Wed, Dec 5, 2012 at 3:04 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> On Wed, Dec 5, 2012 at 2:49 PM, Chao Xie <chao.xie@marvell.com> wrote:
>> the pxa95x rtc need access PBSR register before write to
>> RTTR, RCNR, RDCR, and RYCR registers.
>>
>> Signed-off-by: Chao Xie <chao.xie@marvell.com>
>> ---
>>  drivers/rtc/rtc-pxa.c |  100 +++++++++++++++++++++++++++++++++++++++++-------
>>  1 files changed, 85 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
>> index f771b2e..aee23cb 100644
>> --- a/drivers/rtc/rtc-pxa.c
>> +++ b/drivers/rtc/rtc-pxa.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/delay.h>
>>
>>  #include <mach/hardware.h>
>>
>> @@ -77,14 +78,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;
>> @@ -242,9 +257,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);
>> +               udelay(100);
>> +       }
>
> How about define PSBP operation as a new clock, rtc interface clock.
> Then you could put
> the enable/disable into clock framework. And you only need to check
> whether the interface
> clock is NULL or not at here. If it's available, you can call
> clk_prepare_enable().
>
>> +
>>         rtc_writel(pxa_rtc, RYCR, ryxr_calc(tm));
>>         rtc_writel(pxa_rtc, RDCR, rdxr_calc(tm));
>>
>> +       if (pxa_rtc->id == RTC_PXA95X) {
>> +               udelay(100);
>> +               rtc_writel_psbr(pxa_rtc, PSBR_RTC, 0x00);
>> +               udelay(100);
>> +       }
>> +
>>         return 0;
>>  }
>>
>> @@ -310,6 +342,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;
>> @@ -324,13 +370,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");
>> @@ -347,7 +414,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;
>> +               }
>>         }
>>
>>         /*
>> @@ -376,9 +453,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;
>>  }
>> @@ -398,14 +478,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
>>  static int pxa_rtc_suspend(struct device *dev)
>>  {
>> @@ -440,14 +512,12 @@ static struct platform_driver pxa_rtc_driver = {
>>                 .pm     = &pxa_rtc_pm_ops,
>>  #endif
>>         },
>> +       .id_table       = pxa_rtc_id_table,
>>  };
>>
>>  static int __init pxa_rtc_init(void)
>>  {
>> -       if (cpu_is_pxa27x() || cpu_is_pxa3xx())
>> -               return platform_driver_probe(&pxa_rtc_driver, pxa_rtc_probe);
>> -
>> -       return -ENODEV;
>> +       return platform_driver_probe(&pxa_rtc_driver, pxa_rtc_probe);
>>  }
>>
>>  static void __exit pxa_rtc_exit(void)
>> --
>> 1.7.4.1
>>
I do not think so.
First, from solicon, PBSR is not a clock. it only prorect RDCR, RYCR,
RNCR. There are still some other registers that do not need its
protection.
Second, in fact, the RTC has its own clock which is similar with
sa1100-rtc. define PBSR operation in the clock will finally make the
driver to handle two clocks. It will make the driver harder to handle
it, and even impact the clock device tree support for devices.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-12-06  1:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-05  6:49 [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove Chao Xie
2012-12-05  6:49 ` [V2 PATCH 2/2] rtc: pxa: add pxa95x rtc support Chao Xie
2012-12-05  7:04   ` Haojian Zhuang
2012-12-06  1:28     ` Chao Xie
2012-12-05  7:05 ` [V2 PATCH 1/2] rtc: sa1100: move clock enable/disable to probe/remove Haojian Zhuang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).