Re: [PATCH 3/9] OMAP2/3/4 : Dual mode timer device registration.

2010-06-03 Thread Kevin Hilman
Thara Gopinath  writes:

> This patch converts OMAP2/OMAP3/OMAP4 dual mode timers into
> platform devices using omap hwmod, omap device and runtime pm frameworks.
> This patch also allows GPTIMER1 and GPTIMER2 to be registered as
> early devices. This will allow GPTIMER1 to be registered as
> system timer very early on in the system boot up sequence.
> Later during normal plaform_device_register these are converted
> to normal platform device.
>
> Signed-off-by: Thara Gopinath 

[...]

> +#define OMAP2_DM_TIMER_COUNT 12
> +#define OMAP3PLUS_DM_TIMER_COUNT 11

Also, I noticed this patch drops support for handling GPT12, but is
not explained in the changelog or in PATCH 0/x description.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/9] OMAP2/3/4 : Dual mode timer device registration.

2010-06-03 Thread Kevin Hilman
Thara Gopinath  writes:

> This patch converts OMAP2/OMAP3/OMAP4 dual mode timers into
> platform devices using omap hwmod, omap device and runtime pm frameworks.
> This patch also allows GPTIMER1 and GPTIMER2 to be registered as
> early devices. This will allow GPTIMER1 to be registered as
> system timer very early on in the system boot up sequence.
> Later during normal plaform_device_register these are converted
> to normal platform device.
>
> Signed-off-by: Thara Gopinath 

First, a couple of general comments.

You're using the runtime PM API here in the device layer hooks.  This
should be in the driver.  It will be a noop for OMAP1 since there will
be no runtime PM implementation, but that is fine.  Moving this into
the driver should allow you to get rid of the enable/disable clock
hooks you currently have in platform_data as well (for early timers
you could leave them enabled until they are converted to "normal"
timers.)

Also, I suggested this to Charu as well for GPIO and WDT, but we
should leave out the handling of the #ifndef CONFIG_PM_RUNTIME from
this code.  I'd rather not have every driver handling that special
case.  Instead I propose we handle this in the omap_hwmod init sequence
by enabling all the hwmods if !CONFIG_PM_RUNTIME.

> ---
>  arch/arm/mach-omap2/Makefile   |3 +-
>  arch/arm/mach-omap2/dmtimers.c |  296 
> 
>  arch/arm/mach-omap2/dmtimers.h |   57 
>  arch/arm/mach-omap2/timer-gp.c |2 -
>  4 files changed, 355 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/mach-omap2/dmtimers.c
>  create mode 100644 arch/arm/mach-omap2/dmtimers.h
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index dd3a24a..9299e4f 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -3,7 +3,8 @@
>  #
>  
>  # Common support
> -obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer-gp.o
> +obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer-gp.o \
> + dmtimers.o
>  
>  omap-2-3-common  = irq.o sdrc.o
>  hwmod-common = omap_hwmod.o \
> diff --git a/arch/arm/mach-omap2/dmtimers.c b/arch/arm/mach-omap2/dmtimers.c
> new file mode 100644
> index 000..6a2caf3
> --- /dev/null
> +++ b/arch/arm/mach-omap2/dmtimers.c
> @@ -0,0 +1,296 @@
> +/**
> + * OMAP2 Dual-Mode Timers
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + * Thara Gopinath 
> + *
> + * 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "dmtimers.h"
> +
> +#define NO_EARLY_TIMERS  2

Please use NR_EARLY_TIMERS

> +#define OMAP2_DM_TIMER_COUNT 12
> +#define OMAP3PLUS_DM_TIMER_COUNT 11

and these you should get rid of as Benoit pointed out.  More on this
below.

> +static char *omap2_dm_source_names[] __initdata = {
> + "sys_ck",
> + "func_32k_ck",
> + "alt_ck",
> + NULL
> +};
> +static struct clk *omap2_dm_source_clocks[3];
> +
> +static char *omap3_dm_source_names[] __initdata = {
> + "sys_ck",
> + "omap_32k_fck",
> + NULL
> +};
> +static struct clk *omap3_dm_source_clocks[2];
> +
> +static char *omap4_dm_source_names[] __initdata = {
> + "sys_ck",
> + "sys_32k_ck",
> + NULL
> +};
> +static struct clk *omap4_dm_source_clocks[2];
> +
> +static struct clk **omap_dm_source_clocks;
> +
> +static void omap2_dm_timer_enable(struct platform_device *pdev)
> +{
> +
> + if (pm_runtime_get_sync(&pdev->dev))
> + dev_warn(&pdev->dev, "%s: Unable to enable the timer\n",
> + __func__);
> +#ifndef CONFIG_PM_RUNTIME
> + if (omap_device_enable(pdev))
> + dev_warn(&pdev->dev, "%s: Unable to enable the timer\n",
> + __func__);
> +#endif
> +}
> +
> +static void omap2_dm_timer_disable(struct platform_device *pdev)
> +{
> +
> + if (pm_runtime_put_sync(&pdev->dev))
> + dev_warn(&pdev->dev, "%s: Unable to disable the timer\n",
> + __func__);
> +#ifndef CONFIG_PM_RUNTIME
> + if (omap_device_idle(pdev))
> + dev_warn(&pdev->dev, "%s: Unable to disable the timer\n",
> + __func__);
> +#endif
> +}
> +
> +static int omap2_dm_timer_set_src(struct platform_device *pdev,
> + struct clk *timer_clk, int source)
> +{
> + int ret;
> +
> + if (IS_ERR(timer_clk)) {
> + dev_warn(&pdev->dev, "%s: Not able get the clock pointer\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + clk_disable(timer_clk);
> + ret = clk_set_parent(timer_clk, omap_dm_source_clocks[source]);

RE: [PATCH 3/9] OMAP2/3/4 : Dual mode timer device registration.

2010-06-03 Thread Gopinath, Thara


>>-Original Message-
>>From: Tony Lindgren [mailto:t...@atomide.com]
>>Sent: Tuesday, June 01, 2010 2:51 PM
>>To: Cousson, Benoit
>>Cc: Gopinath, Thara; linux-omap@vger.kernel.org; khil...@deeprootsystems.com; 
>>Sawant, Anand
>>Subject: Re: [PATCH 3/9] OMAP2/3/4 : Dual mode timer device registration.
>>
>>* Benoit Cousson  [100531 01:57]:
>>> >+
>>> >+#define NO_EARLY_TIMERS2
>>> The "NO_" is a little bit confusing, you should maybe use _COUNT.
>>
>>Also NR_ is commonly used.

I will fix this in the repost

>>
>>Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/9] OMAP2/3/4 : Dual mode timer device registration.

2010-06-01 Thread Tony Lindgren
* Benoit Cousson  [100531 01:57]:
> >+
> >+#define NO_EARLY_TIMERS2
> The "NO_" is a little bit confusing, you should maybe use _COUNT.

Also NR_ is commonly used.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/9] OMAP2/3/4 : Dual mode timer device registration.

2010-05-30 Thread Benoit Cousson

Hi Thara,

On 5/29/2010 4:37 PM, Gopinath, Thara wrote:

This patch converts OMAP2/OMAP3/OMAP4 dual mode timers into
platform devices using omap hwmod, omap device and runtime pm frameworks.
This patch also allows GPTIMER1 and GPTIMER2 to be registered as
early devices. This will allow GPTIMER1 to be registered as
system timer very early on in the system boot up sequence.
Later during normal plaform_device_register these are converted
to normal platform device.

Signed-off-by: Thara Gopinath
---
  arch/arm/mach-omap2/Makefile   |3 +-
  arch/arm/mach-omap2/dmtimers.c |  296 
  arch/arm/mach-omap2/dmtimers.h |   57 
  arch/arm/mach-omap2/timer-gp.c |2 -
  4 files changed, 355 insertions(+), 3 deletions(-)
  create mode 100644 arch/arm/mach-omap2/dmtimers.c
  create mode 100644 arch/arm/mach-omap2/dmtimers.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index dd3a24a..9299e4f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -3,7 +3,8 @@
  #

  # Common support
-obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer-gp.o
+obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer-gp.o \
+   dmtimers.o

  omap-2-3-common= irq.o sdrc.o
  hwmod-common   = omap_hwmod.o \
diff --git a/arch/arm/mach-omap2/dmtimers.c b/arch/arm/mach-omap2/dmtimers.c
new file mode 100644
index 000..6a2caf3
--- /dev/null
+++ b/arch/arm/mach-omap2/dmtimers.c
@@ -0,0 +1,296 @@
+/**
+ * OMAP2 Dual-Mode Timers
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ * Thara Gopinath
+ *
+ * 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.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "dmtimers.h"
+
+#define NO_EARLY_TIMERS2

The "NO_" is a little bit confusing, you should maybe use _COUNT.


+#define OMAP2_DM_TIMER_COUNT   12
+#define OMAP3PLUS_DM_TIMER_COUNT   11

Why do you need such SoC specific info hard coded here?
You should extract that from hwmod data.


+
+static char *omap2_dm_source_names[] __initdata = {
+   "sys_ck",
+   "func_32k_ck",
+   "alt_ck",
+   NULL
+};
+static struct clk *omap2_dm_source_clocks[3];
+
+static char *omap3_dm_source_names[] __initdata = {
+   "sys_ck",
+   "omap_32k_fck",
+   NULL
+};
+static struct clk *omap3_dm_source_clocks[2];
+
+static char *omap4_dm_source_names[] __initdata = {
+   "sys_ck",
+   "sys_32k_ck",
+   NULL
+};
+static struct clk *omap4_dm_source_clocks[2];
+
+static struct clk **omap_dm_source_clocks;
+
+static void omap2_dm_timer_enable(struct platform_device *pdev)
+{
+
+   if (pm_runtime_get_sync(&pdev->dev))
+   dev_warn(&pdev->dev, "%s: Unable to enable the timer\n",
+   __func__);
+#ifndef CONFIG_PM_RUNTIME
+   if (omap_device_enable(pdev))
+   dev_warn(&pdev->dev, "%s: Unable to enable the timer\n",
+   __func__);
+#endif
+}
+
+static void omap2_dm_timer_disable(struct platform_device *pdev)
+{
+
+   if (pm_runtime_put_sync(&pdev->dev))
+   dev_warn(&pdev->dev, "%s: Unable to disable the timer\n",
+   __func__);
+#ifndef CONFIG_PM_RUNTIME
+   if (omap_device_idle(pdev))
+   dev_warn(&pdev->dev, "%s: Unable to disable the timer\n",
+   __func__);
+#endif
+}
+
+static int omap2_dm_timer_set_src(struct platform_device *pdev,
+   struct clk *timer_clk, int source)
+{
+   int ret;
+
+   if (IS_ERR(timer_clk)) {
+   dev_warn(&pdev->dev, "%s: Not able get the clock pointer\n",
+   __func__);
+   return -EINVAL;
+   }
+
+   clk_disable(timer_clk);
+   ret = clk_set_parent(timer_clk, omap_dm_source_clocks[source]);
+   if (ret)
+   dev_warn(&pdev->dev, "%s: Not able to change the"
+   "fclk source\n", __func__);
+
+   clk_enable(timer_clk);
+   /*
+* When the functional clock disappears, too quick writes seem
+* to cause an abort. XXX Is this still necessary?
+*/
Good question... it is still necessary? Do you know what platform has 
that bug?



+   __delay(15);
+   return ret;
+}
+
+static int omap2_dm_timer_set_clk(struct platform_device *pdev, int source)
+{
+   struct clk *timer_clk = clk_get(&pdev->dev, "fck");
+
+   return omap2_dm_timer_set_src(pdev, timer_clk, source);
+}
+
+static struct clk *omap2_dm_timer_get_fclk(struct platform_device *pdev)
+{
+   return clk_get(&pdev->dev, "fck");
+}
+
+/* API's to be used by early timer devices */
+static void __init omap2_dm_early_t