linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/26] OMAP dmtimer prep series
@ 2014-04-24 21:43 Joel Fernandes
  2014-04-24 21:43 ` [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook Joel Fernandes
                   ` (25 more replies)
  0 siblings, 26 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Here are a couple of patches moving things around in dmtimer and system timer
code simplying, rewriting many parts of it, inorder to prep them for moving out
of plat-omap and mach-omap2 to a generic clocksource driver. These patches are
required before the clocksource migration can be done.

Many of these patches have been out for a while now, I'd like to get any
final feedback and have them merged for v3.16.

The series includes all earlier series posted for OMAP1 timers, OMAP2+ timers,
and common platform dmtimer code [1] [2].

Tests have been performed with system timers on AM335x, DRA7 and OMAP4 Panda.

The code delta is slightly more because a bit of redundancy introduced for fall
back cases in mach-omap2/timer.c and a few more checks etc to simply things in
plat-omap/dmtimer.c. Finally once we move things out to drivers/, there will be
a lot of negative delta so this is a small cost for migration.

[1] https://lkml.org/lkml/2014/4/16/737
[2] http://www.kernelhub.org/?msg=453407&p=2

Joel Fernandes (26):
  ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through
    platform hook
  ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework
  ARM: OMAP: dmtimer: Add note to set parent from DT
  ARM: OMAP: dmtimer: Add function to check if timer is running
  ARM: OMAP1: dmtimer: Rewrite modify of IDLECT mask to use new
    is_running function
  ARM: OMAP: dmtimer: Add a write_ctrl function to simplify bit setting
  ARM: OMAP: dmtimer: Have __omap_dm_timer_load_start set ST bit in
    CTRL instead of caller
  ARM: OMAP: dmtimer: Add function to check for timer availability
  ARM: OMAP: dmtimer: Get rid of check for mem resource error
  ARM: OMAP: dmtimer: Check return of pm_runtime_get_sync
  ARM: OMAP2+: timer: Add a powerup function
  ARM: OMAP2+: timer: Simplify clock event/source name setting
  ARM: OMAP2+: timer: Add comment on timer clk parenting
  ARM: OMAP2+: timer: Remove hwmod look-up dependency for DT-boot
  ARM: OMAP2+: timer: Use of_clk_get for DT platforms
  ARM: OMAP2+: timer: Fix error message to not use hwmod structure
  ARM: OMAP2+: timer: Add fallback for of_clk_get
  ARM: OMAP2+: timer: Add legacy code for old way of getting fclk
  ARM: OMAP: dmtimer: Remove API __omap_dm_timer_load_start
  ARM: OMAP: dmtimer: Fold back private stop function
  ARM: OMAP: dmtimer: Add systimer flag to dmtimer structure
  ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_write_status function
  ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_read_counter function
  ARM: OMAP: dmtimer: Move private functions into dmtimer core and
    export others
  ARM: OMAP: dmtimer: Eliminate omap_dm_timer_int_enable function
  ARM: OMAP: dmtimer: Use is_timer_available function in
    omap_dm_timer_trigger

 arch/arm/mach-omap1/include/mach/hardware.h  |    2 +
 arch/arm/mach-omap1/timer.c                  |   34 +-
 arch/arm/mach-omap2/timer.c                  |  151 ++++++---
 arch/arm/plat-omap/dmtimer.c                 |  449 +++++++++++++++++---------
 arch/arm/plat-omap/include/plat/dmtimer.h    |  153 +--------
 drivers/staging/tidspbridge/core/dsp-clock.c |    2 +-
 include/linux/platform_data/dmtimer-omap.h   |    2 -
 7 files changed, 451 insertions(+), 342 deletions(-)

-- 
1.7.9.5


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

* [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-05-07 15:19   ` Tony Lindgren
  2014-04-24 21:43 ` [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework Joel Fernandes
                   ` (24 subsequent siblings)
  25 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

There is a platform specific hook just for OMAP1 to set its clk parent.  Remove
this hook and have OMAP1 set its parent in omap1_dm_timer_init.  If OMAP1 is
ever migrated to clock framework, the correct way to do this would be through
clk_set_parent like other platforms.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap1/timer.c                |    8 +++++++-
 arch/arm/plat-omap/dmtimer.c               |    8 +++-----
 include/linux/platform_data/dmtimer-omap.h |    2 --
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
index bde7a35..4b9c604 100644
--- a/arch/arm/mach-omap1/timer.c
+++ b/arch/arm/mach-omap1/timer.c
@@ -140,7 +140,13 @@ static int __init omap1_dm_timer_init(void)
 			goto err_free_pdata;
 		}
 
-		pdata->set_timer_src = omap1_dm_timer_set_src;
+		/*
+		 * Since OMAP1 doesn't support clock framework, set timer clock
+		 * source to 32KHz here instead of expecting it to be set by
+		 * dmtimer code.
+		 */
+		omap1_dm_timer_set_src(pdev, 0x01);
+
 		pdata->timer_capability = OMAP_TIMER_ALWON |
 				OMAP_TIMER_NEEDS_RESET | OMAP_TIMER_HAS_DSP_IRQ;
 
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 869254c..ecd3f97 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -494,12 +494,10 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
 		return -EINVAL;
 
 	/*
-	 * FIXME: Used for OMAP1 devices only because they do not currently
-	 * use the clock framework to set the parent clock. To be removed
-	 * once OMAP1 migrated to using clock framework for dmtimers
+	 * For OMAP1, timer source is already set during omap1_dm_timer_init.
 	 */
-	if (pdata && pdata->set_timer_src)
-		return pdata->set_timer_src(timer->pdev, source);
+	if (timer->capability & OMAP_TIMER_NEEDS_RESET)
+		return 0;
 
 	if (IS_ERR(timer->fclk))
 		return -EINVAL;
diff --git a/include/linux/platform_data/dmtimer-omap.h b/include/linux/platform_data/dmtimer-omap.h
index a19b78d..9f42b06 100644
--- a/include/linux/platform_data/dmtimer-omap.h
+++ b/include/linux/platform_data/dmtimer-omap.h
@@ -21,8 +21,6 @@
 #define __PLATFORM_DATA_DMTIMER_OMAP_H__
 
 struct dmtimer_platform_data {
-	/* set_timer_src - Only used for OMAP1 devices */
-	int (*set_timer_src)(struct platform_device *pdev, int source);
 	u32 timer_capability;
 	u32 timer_errata;
 	int (*get_context_loss_count)(struct device *);
-- 
1.7.9.5


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

* [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
  2014-04-24 21:43 ` [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-05-07 15:20   ` Tony Lindgren
  2014-04-24 21:43 ` [PATCH 03/26] ARM: OMAP: dmtimer: Add note to set parent from DT Joel Fernandes
                   ` (23 subsequent siblings)
  25 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

OMAP1 doesn't support clock framework, add a comment where needed
and correct a FIXME.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index ecd3f97..f5a674c 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -142,8 +142,7 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)
 	int rc;
 
 	/*
-	 * FIXME: OMAP1 devices do not use the clock framework for dmtimers so
-	 * do not call clk_get() for these devices.
+	 * Do not call clk_get() for OMAP1 due to no clock framework support.
 	 */
 	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET)) {
 		timer->fclk = clk_get(&timer->pdev->dev, "fck");
@@ -461,6 +460,7 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
 	if (unlikely(!timer))
 		return -EINVAL;
 
+	/* OMAP1 is not converted to clk framework so avoid clk_get_rate here */
 	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET))
 		rate = clk_get_rate(timer->fclk);
 
-- 
1.7.9.5


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

* [PATCH 03/26] ARM: OMAP: dmtimer: Add note to set parent from DT
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
  2014-04-24 21:43 ` [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook Joel Fernandes
  2014-04-24 21:43 ` [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running Joel Fernandes
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Once clock-parents or default-parent support for DT clocks is available,
we should use it to set clock parent and turn clk_set_parent into a NOOP.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index f5a674c..4debb3d 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -165,6 +165,10 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)
 	__omap_dm_timer_enable_posted(timer);
 	omap_dm_timer_disable(timer);
 
+	/*
+	 * FIXME: Once DT clock-parents or set-parents support is upstream,
+	 * this is to become a NOOP.
+	 */
 	return omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
 }
 
-- 
1.7.9.5


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

* [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (2 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 03/26] ARM: OMAP: dmtimer: Add note to set parent from DT Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-05-07 15:25   ` Tony Lindgren
  2014-04-24 21:43 ` [PATCH 05/26] ARM: OMAP1: dmtimer: Rewrite modify of IDLECT mask to use new is_running function Joel Fernandes
                   ` (21 subsequent siblings)
  25 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Inorder to move non-DM timer specific code that modifies the "idlect"
mask on OMAP1, from dmtimer code, to OMAP1 specific timer initialization code,
we introduce a new function that can possibly be reused for other purposes in
the future. The function just checks if a timer is running based on the timer ID
which should be same as pdev->id. This allows us to cleanly separate the timer vs
non-timer bits and keep the timer bits in the dmtimer code.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c              |   29 +++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/dmtimer.h |    2 ++
 2 files changed, 31 insertions(+)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 4debb3d..86b2641 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -187,6 +187,35 @@ int omap_dm_timer_reserve_systimer(int id)
 	return 0;
 }
 
+/*
+ * Check if a timer is running based on timer_id, used for OMAP1 currently.
+ */
+int omap_dm_timer_is_running(int timer_id)
+{
+	int i = 1, ret = 0;
+	struct omap_dm_timer *timer = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dm_timer_lock, flags);
+	list_for_each_entry(timer, &omap_timer_list, node) {
+		if (i == timer_id) {
+			u32 l;
+			l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
+			if (l & OMAP_TIMER_CTRL_ST) {
+				ret = 1;
+				goto done;
+			} else {
+				goto done;
+			}
+		}
+		i++;
+	}
+done:
+	spin_unlock_irqrestore(&dm_timer_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(omap_dm_timer_is_running);
+
 static struct omap_dm_timer *_omap_dm_timer_request(int req_type, void *data)
 {
 	struct omap_dm_timer *timer = NULL, *t;
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 2861b15..41df0a6 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -135,6 +135,8 @@ void omap_dm_timer_disable(struct omap_dm_timer *timer);
 
 int omap_dm_timer_get_irq(struct omap_dm_timer *timer);
 
+int omap_dm_timer_is_running(int timer_id);
+
 u32 omap_dm_timer_modify_idlect_mask(u32 inputmask);
 struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer);
 
-- 
1.7.9.5


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

* [PATCH 05/26] ARM: OMAP1: dmtimer: Rewrite modify of IDLECT mask to use new is_running function
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (3 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 06/26] ARM: OMAP: dmtimer: Add a write_ctrl function to simplify bit setting Joel Fernandes
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

While at it, also delete the old definition of the function in dmtimer.c code.
This completes the separation and removal of OMAP1 header dependency in dmtimer
code and removes references to MOD_CONF_CTRL registers in dmtimer.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap1/include/mach/hardware.h |    2 ++
 arch/arm/mach-omap1/timer.c                 |   26 +++++++++++++++
 arch/arm/plat-omap/dmtimer.c                |   48 ---------------------------
 arch/arm/plat-omap/include/plat/dmtimer.h   |    1 -
 4 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/arch/arm/mach-omap1/include/mach/hardware.h b/arch/arm/mach-omap1/include/mach/hardware.h
index 5875a50..46de040 100644
--- a/arch/arm/mach-omap1/include/mach/hardware.h
+++ b/arch/arm/mach-omap1/include/mach/hardware.h
@@ -70,6 +70,8 @@ static inline u32 omap_cs3_phys(void)
 			? 0 : OMAP_CS3_PHYS;
 }
 
+__u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask);
+
 #endif	/* ifndef __ASSEMBLER__ */
 
 #define OMAP1_IO_OFFSET		0x01000000	/* Virtual IO = 0xfefb0000 */
diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
index 4b9c604..0a039f1 100644
--- a/arch/arm/mach-omap1/timer.c
+++ b/arch/arm/mach-omap1/timer.c
@@ -42,6 +42,32 @@
 
 #define OMAP1_DM_TIMER_COUNT		8
 
+/**
+ * omap_dm_timer_modify_idlect_mask - Check if any running timers use ARMXOR
+ * @inputmask: current value of idlect mask
+ */
+__u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
+{
+	int i;
+
+	/* If ARMXOR cannot be idled this function call is unnecessary */
+	if (!(inputmask & (1 << 1)))
+		return inputmask;
+
+	for (i = 1; i <= OMAP1_DM_TIMER_COUNT; i++) {
+		if (omap_dm_timer_is_running(i)) {
+			if (((omap_readl(MOD_CONF_CTRL_1) >> ((i-1) * 2))
+			      & 0x03) == 0)
+				inputmask &= ~(1 << 1);
+			else
+				inputmask &= ~(1 << 2);
+		}
+		i++;
+	}
+
+	return inputmask;
+}
+
 static int omap1_dm_timer_set_src(struct platform_device *pdev,
 				int source)
 {
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 86b2641..0e96ad2 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -397,44 +397,6 @@ int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
 
-#if defined(CONFIG_ARCH_OMAP1)
-#include <mach/hardware.h>
-/**
- * omap_dm_timer_modify_idlect_mask - Check if any running timers use ARMXOR
- * @inputmask: current value of idlect mask
- */
-__u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
-{
-	int i = 0;
-	struct omap_dm_timer *timer = NULL;
-	unsigned long flags;
-
-	/* If ARMXOR cannot be idled this function call is unnecessary */
-	if (!(inputmask & (1 << 1)))
-		return inputmask;
-
-	/* If any active timer is using ARMXOR return modified mask */
-	spin_lock_irqsave(&dm_timer_lock, flags);
-	list_for_each_entry(timer, &omap_timer_list, node) {
-		u32 l;
-
-		l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
-		if (l & OMAP_TIMER_CTRL_ST) {
-			if (((omap_readl(MOD_CONF_CTRL_1) >> (i * 2)) & 0x03) == 0)
-				inputmask &= ~(1 << 1);
-			else
-				inputmask &= ~(1 << 2);
-		}
-		i++;
-	}
-	spin_unlock_irqrestore(&dm_timer_lock, flags);
-
-	return inputmask;
-}
-EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
-
-#else
-
 struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
 {
 	if (timer && !IS_ERR(timer->fclk))
@@ -443,16 +405,6 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
 
-__u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
-{
-	BUG();
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
-
-#endif
-
 int omap_dm_timer_trigger(struct omap_dm_timer *timer)
 {
 	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 41df0a6..16ea9fd 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -137,7 +137,6 @@ int omap_dm_timer_get_irq(struct omap_dm_timer *timer);
 
 int omap_dm_timer_is_running(int timer_id);
 
-u32 omap_dm_timer_modify_idlect_mask(u32 inputmask);
 struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer);
 
 int omap_dm_timer_trigger(struct omap_dm_timer *timer);
-- 
1.7.9.5


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

* [PATCH 06/26] ARM: OMAP: dmtimer: Add a write_ctrl function to simplify bit setting
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (4 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 05/26] ARM: OMAP1: dmtimer: Rewrite modify of IDLECT mask to use new is_running function Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 07/26] ARM: OMAP: dmtimer: Have __omap_dm_timer_load_start set ST bit in CTRL instead of caller Joel Fernandes
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

A common pattern in dmtimer code is to read the control reg, set and reset
certain bits, and write it back. We abstract this pattern and introduce a
new function to do so.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c |   63 ++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 0e96ad2..782ff10 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -91,6 +91,18 @@ static void omap_dm_timer_write_reg(struct omap_dm_timer *timer, u32 reg,
 	__omap_dm_timer_write(timer, reg, value, timer->posted);
 }
 
+static u32 omap_dm_timer_write_ctrl(struct omap_dm_timer *timer, u32 mask,
+		u32 value)
+{
+	u32 l;
+
+	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
+	l &= mask;
+	l |= value;
+	omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
+	return l;
+}
+
 static void omap_timer_restore_context(struct omap_dm_timer *timer)
 {
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_WAKEUP_EN_REG,
@@ -521,23 +533,22 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_source);
 int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
 			    unsigned int load)
 {
-	u32 l;
+	u32 mask = ~0, val = 0;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
 	omap_dm_timer_enable(timer);
-	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
 	if (autoreload)
-		l |= OMAP_TIMER_CTRL_AR;
+		val |= OMAP_TIMER_CTRL_AR;
 	else
-		l &= ~OMAP_TIMER_CTRL_AR;
-	omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
+		mask &= ~OMAP_TIMER_CTRL_AR;
+	val = omap_dm_timer_write_ctrl(timer, mask, val);
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_LOAD_REG, load);
 
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
 	/* Save the context */
-	timer->context.tclr = l;
+	timer->context.tclr = val;
 	timer->context.tldr = load;
 	omap_dm_timer_disable(timer);
 	return 0;
@@ -577,22 +588,22 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);
 int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
 			     unsigned int match)
 {
-	u32 l;
+	u32 mask = ~0, val = 0;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
 	omap_dm_timer_enable(timer);
-	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
 	if (enable)
-		l |= OMAP_TIMER_CTRL_CE;
+		val |= OMAP_TIMER_CTRL_CE;
 	else
-		l &= ~OMAP_TIMER_CTRL_CE;
+		mask &= ~OMAP_TIMER_CTRL_CE;
+
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_MATCH_REG, match);
-	omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
+	val = omap_dm_timer_write_ctrl(timer, mask, val);
 
 	/* Save the context */
-	timer->context.tclr = l;
+	timer->context.tclr = val;
 	timer->context.tmar = match;
 	omap_dm_timer_disable(timer);
 	return 0;
@@ -602,24 +613,23 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_match);
 int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
 			   int toggle, int trigger)
 {
-	u32 l;
+	u32 mask = ~0, val = 0;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
 	omap_dm_timer_enable(timer);
-	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
-	l &= ~(OMAP_TIMER_CTRL_GPOCFG | OMAP_TIMER_CTRL_SCPWM |
+	mask &= ~(OMAP_TIMER_CTRL_GPOCFG | OMAP_TIMER_CTRL_SCPWM |
 	       OMAP_TIMER_CTRL_PT | (0x03 << 10));
 	if (def_on)
-		l |= OMAP_TIMER_CTRL_SCPWM;
+		val |= OMAP_TIMER_CTRL_SCPWM;
 	if (toggle)
-		l |= OMAP_TIMER_CTRL_PT;
-	l |= trigger << 10;
-	omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
+		val |= OMAP_TIMER_CTRL_PT;
+	val |= trigger << 10;
+	val = omap_dm_timer_write_ctrl(timer, mask, val);
 
 	/* Save the context */
-	timer->context.tclr = l;
+	timer->context.tclr = val;
 	omap_dm_timer_disable(timer);
 	return 0;
 }
@@ -627,22 +637,21 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_pwm);
 
 int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
 {
-	u32 l;
+	u32 mask = ~0, val = 0;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
 	omap_dm_timer_enable(timer);
-	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
-	l &= ~(OMAP_TIMER_CTRL_PRE | (0x07 << 2));
+	mask &= ~(OMAP_TIMER_CTRL_PRE | (0x07 << 2));
 	if (prescaler >= 0x00 && prescaler <= 0x07) {
-		l |= OMAP_TIMER_CTRL_PRE;
-		l |= prescaler << 2;
+		val |= OMAP_TIMER_CTRL_PRE;
+		val |= prescaler << 2;
 	}
-	omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
+	val = omap_dm_timer_write_ctrl(timer, mask, val);
 
 	/* Save the context */
-	timer->context.tclr = l;
+	timer->context.tclr = val;
 	omap_dm_timer_disable(timer);
 	return 0;
 }
-- 
1.7.9.5


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

* [PATCH 07/26] ARM: OMAP: dmtimer: Have __omap_dm_timer_load_start set ST bit in CTRL instead of caller
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (5 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 06/26] ARM: OMAP: dmtimer: Add a write_ctrl function to simplify bit setting Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 08/26] ARM: OMAP: dmtimer: Add function to check for timer availability Joel Fernandes
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

"load_start" implies start, so it makes sense to set the ST bit in
__omap_dm_timer_load_start instead of callers.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c               |    6 +++---
 arch/arm/plat-omap/dmtimer.c              |    1 -
 arch/arm/plat-omap/include/plat/dmtimer.h |    1 +
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 74044aa..dfb19df 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -95,7 +95,7 @@ static struct irqaction omap2_gp_timer_irq = {
 static int omap2_gp_timer_set_next_event(unsigned long cycles,
 					 struct clock_event_device *evt)
 {
-	__omap_dm_timer_load_start(&clkev, OMAP_TIMER_CTRL_ST,
+	__omap_dm_timer_load_start(&clkev, 0,
 				   0xffffffff - cycles, OMAP_TIMER_POSTED);
 
 	return 0;
@@ -116,7 +116,7 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
 		__omap_dm_timer_write(&clkev, OMAP_TIMER_LOAD_REG,
 				      0xffffffff - period, OMAP_TIMER_POSTED);
 		__omap_dm_timer_load_start(&clkev,
-					OMAP_TIMER_CTRL_AR | OMAP_TIMER_CTRL_ST,
+					OMAP_TIMER_CTRL_AR,
 					0xffffffff - period, OMAP_TIMER_POSTED);
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
@@ -469,7 +469,7 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 	BUG_ON(res);
 
 	__omap_dm_timer_load_start(&clksrc,
-				   OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0,
+				   OMAP_TIMER_CTRL_AR, 0,
 				   OMAP_TIMER_NONPOSTED);
 	sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate);
 
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 782ff10..8a4a97c 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -573,7 +573,6 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
 	} else {
 		l &= ~OMAP_TIMER_CTRL_AR;
 	}
-	l |= OMAP_TIMER_CTRL_ST;
 
 	__omap_dm_timer_load_start(timer, l, load, timer->posted);
 
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 16ea9fd..fe3780a 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -393,6 +393,7 @@ static inline void __omap_dm_timer_load_start(struct omap_dm_timer *timer,
 						u32 ctrl, unsigned int load,
 						int posted)
 {
+	ctrl |= OMAP_TIMER_CTRL_ST;
 	__omap_dm_timer_write(timer, OMAP_TIMER_COUNTER_REG, load, posted);
 	__omap_dm_timer_write(timer, OMAP_TIMER_CTRL_REG, ctrl, posted);
 }
-- 
1.7.9.5


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

* [PATCH 08/26] ARM: OMAP: dmtimer: Add function to check for timer availability
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (6 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 07/26] ARM: OMAP: dmtimer: Have __omap_dm_timer_load_start set ST bit in CTRL instead of caller Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error Joel Fernandes
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Simplify the check for a timer availability in atleast 4 places by providing a
function to do the same.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 8a4a97c..7e806f9 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -704,14 +704,22 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);
 
+static int is_timer_available(struct omap_dm_timer *timer)
+{
+	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
+		pr_err("Timer not available or enabled.\n");
+		WARN_ON(1);
+		return 0;
+	}
+	return 1;
+}
+
 unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
 {
 	unsigned int l;
 
-	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
-		pr_err("%s: timer not available or enabled.\n", __func__);
+	if (!is_timer_available(timer))
 		return 0;
-	}
 
 	l = __raw_readl(timer->irq_stat);
 
@@ -721,7 +729,7 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_read_status);
 
 int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
 {
-	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev)))
+	if (!is_timer_available(timer))
 		return -EINVAL;
 
 	__omap_dm_timer_write_status(timer, value);
@@ -732,10 +740,8 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_write_status);
 
 unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
 {
-	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
-		pr_err("%s: timer not iavailable or enabled.\n", __func__);
+	if (!is_timer_available(timer))
 		return 0;
-	}
 
 	return __omap_dm_timer_read_counter(timer, timer->posted);
 }
@@ -743,10 +749,8 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_read_counter);
 
 int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
 {
-	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
-		pr_err("%s: timer not available or enabled.\n", __func__);
+	if (!is_timer_available(timer))
 		return -EINVAL;
-	}
 
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_COUNTER_REG, value);
 
-- 
1.7.9.5


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

* [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (7 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 08/26] ARM: OMAP: dmtimer: Add function to check for timer availability Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-05-07 15:24   ` Tony Lindgren
  2014-04-24 21:43 ` [PATCH 10/26] ARM: OMAP: dmtimer: Check return of pm_runtime_get_sync Joel Fernandes
                   ` (16 subsequent siblings)
  25 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

The subsequent devm_ioremap_resource will catch it and print an error, let it
be checked there.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 7e806f9..1fd30fa 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -810,10 +810,6 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (unlikely(!mem)) {
-		dev_err(dev, "%s: no memory resource.\n", __func__);
-		return -ENODEV;
-	}
 
 	timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
 	if (!timer) {
-- 
1.7.9.5


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

* [PATCH 10/26] ARM: OMAP: dmtimer: Check return of pm_runtime_get_sync
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (8 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 11/26] ARM: OMAP2+: timer: Add a powerup function Joel Fernandes
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

omap_timer_enable doesn't check return value, this can lead to nasty bus
errors. Add a check and report issues.

Tested-by: Joachim Eastwood <manabian@gmail.com>
Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c              |   62 +++++++++++++++++++++++------
 arch/arm/plat-omap/include/plat/dmtimer.h |    2 +-
 2 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 1fd30fa..b52d34d 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -164,7 +164,9 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)
 		}
 	}
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
 
 	if (timer->capability & OMAP_TIMER_NEEDS_RESET) {
 		rc = omap_dm_timer_reset(timer);
@@ -375,11 +377,16 @@ int omap_dm_timer_free(struct omap_dm_timer *timer)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_free);
 
-void omap_dm_timer_enable(struct omap_dm_timer *timer)
+int omap_dm_timer_enable(struct omap_dm_timer *timer)
 {
-	int c;
+	int c, err;
 
-	pm_runtime_get_sync(&timer->pdev->dev);
+	err = pm_runtime_get_sync(&timer->pdev->dev);
+	if (err < 0) {
+		dev_err(&timer->pdev->dev, "%s: pm_runtime_get_sync failed (err=%d).",
+			__func__, err);
+		return err;
+	}
 
 	if (!(timer->capability & OMAP_TIMER_ALWON)) {
 		if (timer->get_context_loss_count) {
@@ -392,6 +399,7 @@ void omap_dm_timer_enable(struct omap_dm_timer *timer)
 			omap_timer_restore_context(timer);
 		}
 	}
+	return 0;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
 
@@ -432,11 +440,14 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_trigger);
 int omap_dm_timer_start(struct omap_dm_timer *timer)
 {
 	u32 l;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
 
 	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
 	if (!(l & OMAP_TIMER_CTRL_ST)) {
@@ -534,11 +545,15 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
 			    unsigned int load)
 {
 	u32 mask = ~0, val = 0;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
+
 	if (autoreload)
 		val |= OMAP_TIMER_CTRL_AR;
 	else
@@ -560,11 +575,14 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
                             unsigned int load)
 {
 	u32 l;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
 
 	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
 	if (autoreload) {
@@ -588,11 +606,15 @@ int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
 			     unsigned int match)
 {
 	u32 mask = ~0, val = 0;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
+
 	if (enable)
 		val |= OMAP_TIMER_CTRL_CE;
 	else
@@ -613,11 +635,15 @@ int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
 			   int toggle, int trigger)
 {
 	u32 mask = ~0, val = 0;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
+
 	mask &= ~(OMAP_TIMER_CTRL_GPOCFG | OMAP_TIMER_CTRL_SCPWM |
 	       OMAP_TIMER_CTRL_PT | (0x03 << 10));
 	if (def_on)
@@ -637,11 +663,15 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_pwm);
 int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
 {
 	u32 mask = ~0, val = 0;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
+
 	mask &= ~(OMAP_TIMER_CTRL_PRE | (0x07 << 2));
 	if (prescaler >= 0x00 && prescaler <= 0x07) {
 		val |= OMAP_TIMER_CTRL_PRE;
@@ -659,10 +689,15 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_prescaler);
 int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
 				  unsigned int value)
 {
+	int rc;
+
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
+
 	__omap_dm_timer_int_enable(timer, value);
 
 	/* Save the context */
@@ -683,11 +718,14 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_enable);
 int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
 {
 	u32 l = mask;
+	int rc;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	omap_dm_timer_enable(timer);
+	rc = omap_dm_timer_enable(timer);
+	if (rc)
+		return rc;
 
 	if (timer->revision == 1)
 		l = __raw_readl(timer->irq_ena) & ~mask;
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index fe3780a..6b6fbd2 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -130,7 +130,7 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int timer_id);
 struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap);
 struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np);
 int omap_dm_timer_free(struct omap_dm_timer *timer);
-void omap_dm_timer_enable(struct omap_dm_timer *timer);
+int omap_dm_timer_enable(struct omap_dm_timer *timer);
 void omap_dm_timer_disable(struct omap_dm_timer *timer);
 
 int omap_dm_timer_get_irq(struct omap_dm_timer *timer);
-- 
1.7.9.5


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

* [PATCH 11/26] ARM: OMAP2+: timer: Add a powerup function
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (9 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 10/26] ARM: OMAP: dmtimer: Check return of pm_runtime_get_sync Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 12/26] ARM: OMAP2+: timer: Simplify clock event/source name setting Joel Fernandes
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

In an effort to isolate the time power initialization for future purposes, add
a function to do the same. This primarily involves a hwmod lookup, setup and
enable.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index dfb19df..ec427e6 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -210,6 +210,31 @@ static void __init omap_dmtimer_init(void)
 	}
 }
 
+int __init omap_dmtimer_powerup(struct device_node *np)
+{
+	struct omap_hwmod *oh;
+	const char *oh_name = NULL;
+	int ret;
+
+	of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
+	if (!oh_name)
+		return -ENODEV;
+
+	oh = omap_hwmod_lookup(oh_name);
+	if (!oh)
+		return -ENODEV;
+
+	ret = omap_hwmod_setup_one(oh_name);
+	if (ret)
+		return ret;
+
+	ret = omap_hwmod_enable(oh);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 /**
  * omap_dm_timer_get_errata - get errata flags for a timer
  *
-- 
1.7.9.5


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

* [PATCH 12/26] ARM: OMAP2+: timer: Simplify clock event/source name setting
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (10 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 11/26] ARM: OMAP2+: timer: Add a powerup function Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 13/26] ARM: OMAP2+: timer: Add comment on timer clk parenting Joel Fernandes
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

For the OMAPs, we're setting up only one clockevent and clocksource. Further,
for DT boot there's no easy way to get the timer name and currently this is
done in an ugly way by reading a hwmod entry. So let's just set it to a simpler
name like timer_clkev and timer_clksrc.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ec427e6..f192a41 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -251,7 +251,6 @@ static u32 __init omap_dm_timer_get_errata(void)
 static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 					 const char *fck_source,
 					 const char *property,
-					 const char **timer_name,
 					 int posted)
 {
 	char name[10]; /* 10 = sizeof("gptXX_Xck0") */
@@ -290,8 +289,6 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	if (!oh)
 		return -ENODEV;
 
-	*timer_name = oh->name;
-
 	if (!of_have_populated_dt()) {
 		r = omap_hwmod_get_resource_byname(oh, IORESOURCE_IRQ, NULL,
 						   &irq);
@@ -365,8 +362,10 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 	 */
 	__omap_dm_timer_override_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767);
 
+	clockevent_gpt.name = "timer_clkev";
+
 	res = omap_dm_timer_init_one(&clkev, fck_source, property,
-				     &clockevent_gpt.name, OMAP_TIMER_POSTED);
+				     OMAP_TIMER_POSTED);
 	BUG_ON(res);
 
 	omap2_gp_timer_irq.dev_id = &clkev;
@@ -489,10 +488,11 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 	clksrc.errata = omap_dm_timer_get_errata();
 
 	res = omap_dm_timer_init_one(&clksrc, fck_source, property,
-				     &clocksource_gpt.name,
 				     OMAP_TIMER_NONPOSTED);
 	BUG_ON(res);
 
+	clocksource_gpt.name = "timer_clksrc";
+
 	__omap_dm_timer_load_start(&clksrc,
 				   OMAP_TIMER_CTRL_AR, 0,
 				   OMAP_TIMER_NONPOSTED);
-- 
1.7.9.5


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

* [PATCH 13/26] ARM: OMAP2+: timer: Add comment on timer clk parenting
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (11 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 12/26] ARM: OMAP2+: timer: Simplify clock event/source name setting Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 14/26] ARM: OMAP2+: timer: Remove hwmod look-up dependency for DT-boot Joel Fernandes
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Add a comment describing the state of system timer clock parenting.  The code
following the comment should be deleted once all platforms can do DT boot, and
specially should not be executed for DT platforms once we can specify default
clock parents through DT.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index f192a41..9fdff5b 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -313,6 +313,10 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	if (IS_ERR(timer->fclk))
 		return PTR_ERR(timer->fclk);
 
+	/*
+	 * Clock reparenting code, goes away for DT-boot atleast,
+	 * once clock layer can do it through DT.
+	 */
 	src = clk_get(NULL, fck_source);
 	if (IS_ERR(src))
 		return PTR_ERR(src);
-- 
1.7.9.5


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

* [PATCH 14/26] ARM: OMAP2+: timer: Remove hwmod look-up dependency for DT-boot
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (12 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 13/26] ARM: OMAP2+: timer: Add comment on timer clk parenting Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 15/26] ARM: OMAP2+: timer: Use of_clk_get for DT platforms Joel Fernandes
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

hwmod look ups should no longer directly happen for dmtimer users of
DT-boot platforms. We separate out the code for platforms that are
still non-DT. Ultimately this code should be deleted if all platforms
are converted to DT-boot.

We use omap_dmtimer_powerup introduced earlier in the series to abstract
out the hwmod dependency for the same. This will help us move the generic
DT specific code out into drivers/clocksource/ later.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |   31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 9fdff5b..ea91ef9 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -266,10 +266,6 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (!np)
 			return -ENODEV;
 
-		of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
-		if (!oh_name)
-			return -ENODEV;
-
 		timer->irq = irq_of_parse_and_map(np, 0);
 		if (!timer->irq)
 			return -ENXIO;
@@ -277,19 +273,24 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		timer->io_base = of_iomap(np, 0);
 
 		of_node_put(np);
+
+		if (!timer->io_base)
+			return -ENXIO;
+
+		r = omap_dmtimer_powerup(np);
+		if (r)
+			return r;
 	} else {
 		if (omap_dm_timer_reserve_systimer(timer->id))
 			return -ENODEV;
 
 		sprintf(name, "timer%d", timer->id);
 		oh_name = name;
-	}
 
-	oh = omap_hwmod_lookup(oh_name);
-	if (!oh)
-		return -ENODEV;
+		oh = omap_hwmod_lookup(oh_name);
+		if (!oh)
+			return -ENODEV;
 
-	if (!of_have_populated_dt()) {
 		r = omap_hwmod_get_resource_byname(oh, IORESOURCE_IRQ, NULL,
 						   &irq);
 		if (r)
@@ -303,11 +304,14 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 
 		/* Static mapping, never released */
 		timer->io_base = ioremap(mem.start, mem.end - mem.start);
-	}
 
-	if (!timer->io_base)
-		return -ENXIO;
+		if (!timer->io_base)
+			return -ENXIO;
+
 
+		omap_hwmod_setup_one(oh_name);
+		omap_hwmod_enable(oh);
+	}
 	/* After the dmtimer is using hwmod these clocks won't be needed */
 	timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
 	if (IS_ERR(timer->fclk))
@@ -333,10 +337,7 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 
 	clk_put(src);
 
-	omap_hwmod_setup_one(oh_name);
-	omap_hwmod_enable(oh);
 	__omap_dm_timer_init_regs(timer);
-
 	if (posted)
 		__omap_dm_timer_enable_posted(timer);
 
-- 
1.7.9.5


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

* [PATCH 15/26] ARM: OMAP2+: timer: Use of_clk_get for DT platforms
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (13 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 14/26] ARM: OMAP2+: timer: Remove hwmod look-up dependency for DT-boot Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:43 ` [PATCH 16/26] ARM: OMAP2+: timer: Fix error message to not use hwmod structure Joel Fernandes
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

For DT-booting platforms, use of_clk_get to get the fclk for system timers.
Separate out the legacy code for non-DT platform use.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ea91ef9..4fcfd7a 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -277,6 +277,10 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (!timer->io_base)
 			return -ENXIO;
 
+		timer->fclk = of_clk_get(np, 0);
+		if (IS_ERR(timer->fclk))
+			return PTR_ERR(timer->fclk);
+
 		r = omap_dmtimer_powerup(np);
 		if (r)
 			return r;
@@ -308,14 +312,13 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (!timer->io_base)
 			return -ENXIO;
 
+		timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
+		if (IS_ERR(timer->fclk))
+			return PTR_ERR(timer->fclk);
 
 		omap_hwmod_setup_one(oh_name);
 		omap_hwmod_enable(oh);
 	}
-	/* After the dmtimer is using hwmod these clocks won't be needed */
-	timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
-	if (IS_ERR(timer->fclk))
-		return PTR_ERR(timer->fclk);
 
 	/*
 	 * Clock reparenting code, goes away for DT-boot atleast,
-- 
1.7.9.5


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

* [PATCH 16/26] ARM: OMAP2+: timer: Fix error message to not use hwmod structure
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (14 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 15/26] ARM: OMAP2+: timer: Use of_clk_get for DT platforms Joel Fernandes
@ 2014-04-24 21:43 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 17/26] ARM: OMAP2+: timer: Add fallback for of_clk_get Joel Fernandes
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:43 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

oh->name is no longer used, correct the error message.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 4fcfd7a..519ccfd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -331,8 +331,8 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	if (clk_get_parent(timer->fclk) != src) {
 		r = clk_set_parent(timer->fclk, src);
 		if (r < 0) {
-			pr_warn("%s: %s cannot set source\n", __func__,
-				oh->name);
+			pr_warn("%s: cannot set timer's source\n", __func__);
+			WARN_ON(1);
 			clk_put(src);
 			return r;
 		}
-- 
1.7.9.5


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

* [PATCH 17/26] ARM: OMAP2+: timer: Add fallback for of_clk_get
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (15 preceding siblings ...)
  2014-04-24 21:43 ` [PATCH 16/26] ARM: OMAP2+: timer: Fix error message to not use hwmod structure Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 18/26] ARM: OMAP2+: timer: Add legacy code for old way of getting fclk Joel Fernandes
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Not all platforms currently will support of_clk_get on timer
because they may not have clock property in their DT nodes. Add
code to handle this case so that things are kept working. Finally
we can delete this code once all system timer nodes have a clock
property.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 519ccfd..b3db1da 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -277,13 +277,19 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (!timer->io_base)
 			return -ENXIO;
 
-		timer->fclk = of_clk_get(np, 0);
-		if (IS_ERR(timer->fclk))
-			return PTR_ERR(timer->fclk);
-
 		r = omap_dmtimer_powerup(np);
 		if (r)
 			return r;
+
+		timer->fclk = of_clk_get(np, 0);
+		if (IS_ERR(timer->fclk)) {
+			/*
+			 * Support DT platforms temporarily that don't have
+			 * clock property in their dts yet. Ultimately this
+			 * fall back code is to be deleted and we're to return
+			 * PTR_ERR(timer->fclk) here.
+			 */
+		}
 	} else {
 		if (omap_dm_timer_reserve_systimer(timer->id))
 			return -ENODEV;
@@ -312,12 +318,17 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (!timer->io_base)
 			return -ENXIO;
 
+		omap_hwmod_setup_one(oh_name);
+		omap_hwmod_enable(oh);
+	}
+
+	if (!timer->fclk || IS_ERR(timer->fclk)) {
+		oh = omap_hwmod_lookup(oh_name);
+		if (!oh)
+			return -ENODEV;
 		timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
 		if (IS_ERR(timer->fclk))
 			return PTR_ERR(timer->fclk);
-
-		omap_hwmod_setup_one(oh_name);
-		omap_hwmod_enable(oh);
 	}
 
 	/*
-- 
1.7.9.5


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

* [PATCH 18/26] ARM: OMAP2+: timer: Add legacy code for old way of getting fclk
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (16 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 17/26] ARM: OMAP2+: timer: Add fallback for of_clk_get Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 19/26] ARM: OMAP: dmtimer: Remove API __omap_dm_timer_load_start Joel Fernandes
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Separate out legacy code for getting timer->fclk and reuse it for
the DT-case as a fallback. All DT users should ideally be specifying
a clock property with a phandle of its clock node. Till the migration
is complete, add a legacy function to keep things working.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c |   32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index b3db1da..81a29b1 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -248,6 +248,19 @@ static u32 __init omap_dm_timer_get_errata(void)
 	return OMAP_TIMER_ERRATA_I103_I767;
 }
 
+static int legacy_hwmod_clkget(struct omap_dm_timer *timer, const char *oh_name)
+{
+	struct omap_hwmod *oh;
+
+	oh = omap_hwmod_lookup(oh_name);
+	if (!oh)
+		return -ENODEV;
+
+	timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
+	if (IS_ERR(timer->fclk))
+		return PTR_ERR(timer->fclk);
+}
+
 static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 					 const char *fck_source,
 					 const char *property,
@@ -289,6 +302,14 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 			 * fall back code is to be deleted and we're to return
 			 * PTR_ERR(timer->fclk) here.
 			 */
+			of_property_read_string_index(np, "ti,hwmods", 0,
+						      &oh_name);
+			if (!oh_name)
+				return -ENODEV;
+
+			r = legacy_hwmod_clkget(timer, oh_name);
+			if (r)
+				return r;
 		}
 	} else {
 		if (omap_dm_timer_reserve_systimer(timer->id))
@@ -320,15 +341,10 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 
 		omap_hwmod_setup_one(oh_name);
 		omap_hwmod_enable(oh);
-	}
 
-	if (!timer->fclk || IS_ERR(timer->fclk)) {
-		oh = omap_hwmod_lookup(oh_name);
-		if (!oh)
-			return -ENODEV;
-		timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
-		if (IS_ERR(timer->fclk))
-			return PTR_ERR(timer->fclk);
+		r = legacy_hwmod_clkget(timer, oh_name);
+		if (r)
+			return r;
 	}
 
 	/*
-- 
1.7.9.5


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

* [PATCH 19/26] ARM: OMAP: dmtimer: Remove API __omap_dm_timer_load_start
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (17 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 18/26] ARM: OMAP2+: timer: Add legacy code for old way of getting fclk Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 20/26] ARM: OMAP: dmtimer: Fold back private stop function Joel Fernandes
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Fold back functionality of __omap_dm_timer_load_start from the header into the
public dmtimer.c omap_dmtimer_set_load_start API, and convert all dependencies
to use it. All users should use the existing omap_dmtimer_set_load_start
function for such uses.  This is in the direction of making the system timer
code independent of the private functions which we're trying to eliminate so
that they can just use the public ones which we want to expose.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c                  |   17 +++++--------
 arch/arm/plat-omap/dmtimer.c                 |   34 +++++++++++++++-----------
 arch/arm/plat-omap/include/plat/dmtimer.h    |   12 ++-------
 drivers/staging/tidspbridge/core/dsp-clock.c |    2 +-
 4 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 81a29b1..6735e2f 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -95,8 +95,7 @@ static struct irqaction omap2_gp_timer_irq = {
 static int omap2_gp_timer_set_next_event(unsigned long cycles,
 					 struct clock_event_device *evt)
 {
-	__omap_dm_timer_load_start(&clkev, 0,
-				   0xffffffff - cycles, OMAP_TIMER_POSTED);
+	omap_dm_timer_set_load_start(&clkev, 0xffffffff - cycles, 0, 0);
 
 	return 0;
 }
@@ -112,12 +111,7 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
 	case CLOCK_EVT_MODE_PERIODIC:
 		period = clkev.rate / HZ;
 		period -= 1;
-		/* Looks like we need to first set the load value separately */
-		__omap_dm_timer_write(&clkev, OMAP_TIMER_LOAD_REG,
-				      0xffffffff - period, OMAP_TIMER_POSTED);
-		__omap_dm_timer_load_start(&clkev,
-					OMAP_TIMER_CTRL_AR,
-					0xffffffff - period, OMAP_TIMER_POSTED);
+		omap_dm_timer_set_load_start(&clkev, 0xffffffff - period, 1, 0);
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
 		break;
@@ -259,6 +253,8 @@ static int legacy_hwmod_clkget(struct omap_dm_timer *timer, const char *oh_name)
 	timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
 	if (IS_ERR(timer->fclk))
 		return PTR_ERR(timer->fclk);
+
+	return 0;
 }
 
 static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
@@ -528,9 +524,8 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 
 	clocksource_gpt.name = "timer_clksrc";
 
-	__omap_dm_timer_load_start(&clksrc,
-				   OMAP_TIMER_CTRL_AR, 0,
-				   OMAP_TIMER_NONPOSTED);
+	omap_dm_timer_set_load_start(&clksrc, 0, 1, 0);
+
 	sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate);
 
 	if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index b52d34d..dee70be 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -571,33 +571,39 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_load);
 
 /* Optimized set_load which removes costly spin wait in timer_start */
-int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
-                            unsigned int load)
+int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, unsigned int load,
+				 int autoreload, int pm)
 {
-	u32 l;
 	int rc;
+	u32 mask = ~0, val = 0;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	rc = omap_dm_timer_enable(timer);
-	if (rc)
-		return rc;
+	if (pm) {
+		rc = omap_dm_timer_enable(timer);
+		if (rc)
+			return rc;
+	}
 
-	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
 	if (autoreload) {
-		l |= OMAP_TIMER_CTRL_AR;
+		val |= OMAP_TIMER_CTRL_AR;
 		omap_dm_timer_write_reg(timer, OMAP_TIMER_LOAD_REG, load);
 	} else {
-		l &= ~OMAP_TIMER_CTRL_AR;
+		mask &= ~OMAP_TIMER_CTRL_AR;
 	}
 
-	__omap_dm_timer_load_start(timer, l, load, timer->posted);
+	val |= OMAP_TIMER_CTRL_ST;
 
-	/* Save the context */
-	timer->context.tclr = l;
-	timer->context.tldr = load;
-	timer->context.tcrr = load;
+	omap_dm_timer_write_reg(timer, OMAP_TIMER_COUNTER_REG, load);
+	val = omap_dm_timer_write_ctrl(timer, mask, val);
+
+	if (pm) {
+		/* Save the context */
+		timer->context.tclr = val;
+		timer->context.tldr = load;
+		timer->context.tcrr = load;
+	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 6b6fbd2..7432bf3 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -145,7 +145,8 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer);
 
 int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source);
 int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload, unsigned int value);
-int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload, unsigned int value);
+int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, unsigned int load,
+				 int autoreload, int pm);
 int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable, unsigned int match);
 int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on, int toggle, int trigger);
 int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler);
@@ -389,15 +390,6 @@ static inline void __omap_dm_timer_stop(struct omap_dm_timer *timer,
 	__raw_writel(OMAP_TIMER_INT_OVERFLOW, timer->irq_stat);
 }
 
-static inline void __omap_dm_timer_load_start(struct omap_dm_timer *timer,
-						u32 ctrl, unsigned int load,
-						int posted)
-{
-	ctrl |= OMAP_TIMER_CTRL_ST;
-	__omap_dm_timer_write(timer, OMAP_TIMER_COUNTER_REG, load, posted);
-	__omap_dm_timer_write(timer, OMAP_TIMER_CTRL_REG, ctrl, posted);
-}
-
 static inline void __omap_dm_timer_int_enable(struct omap_dm_timer *timer,
 						unsigned int value)
 {
diff --git a/drivers/staging/tidspbridge/core/dsp-clock.c b/drivers/staging/tidspbridge/core/dsp-clock.c
index 2f084e18..2788158 100644
--- a/drivers/staging/tidspbridge/core/dsp-clock.c
+++ b/drivers/staging/tidspbridge/core/dsp-clock.c
@@ -189,7 +189,7 @@ void dsp_gpt_wait_overflow(short int clk_id, unsigned int load)
 	 * Set counter value to overflow counter after
 	 * one tick and start timer.
 	 */
-	omap_dm_timer_set_load_start(gpt, 0, load);
+	omap_dm_timer_set_load_start(gpt, load, 0, 1);
 
 	/* Wait 80us for timer to overflow */
 	udelay(80);
-- 
1.7.9.5


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

* [PATCH 20/26] ARM: OMAP: dmtimer: Fold back private stop function
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (18 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 19/26] ARM: OMAP: dmtimer: Remove API __omap_dm_timer_load_start Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 21/26] ARM: OMAP: dmtimer: Add systimer flag to dmtimer structure Joel Fernandes
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

In this patch, we fold back the functionality of the __omap_dm_timer_stop
function into omap_dm_timer_stop. This reduces code size as we don't need any
hacks or parameter passing. All public users are converted to use the
omap_dm_timer_stop function.  The clk rate is setup into the dmtimer structure
either by the early boot code for the specific platforms, or by calling a
clk_get_rate once the timer is requested and prepared.

We're also using omap_dm_timer_{read,write}_reg functions instead of underscore
prefixed ones so that avoids passing of posted flags.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c                  |    2 +-
 arch/arm/plat-omap/dmtimer.c                 |   45 ++++++++++++++++++--------
 arch/arm/plat-omap/include/plat/dmtimer.h    |   26 +--------------
 drivers/media/rc/ir-rx51.c                   |    4 +--
 drivers/staging/tidspbridge/core/dsp-clock.c |    2 +-
 5 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6735e2f..2d8ea96 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -105,7 +105,7 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
 {
 	u32 period;
 
-	__omap_dm_timer_stop(&clkev, OMAP_TIMER_POSTED, clkev.rate);
+	omap_dm_timer_stop(&clkev, 0);
 
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index dee70be..8602586 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -310,6 +310,12 @@ found:
 
 	if (!timer)
 		pr_debug("%s: timer request failed!\n", __func__);
+	/*
+	 * The prepare stage would've setup the parent clock or we wouldn't
+	 * get so far. Set the rate of the timer now.
+	 */
+	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET))
+		timer->rate = clk_get_rate(timer->fclk);
 
 	return timer;
 }
@@ -461,27 +467,38 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_start);
 
-int omap_dm_timer_stop(struct omap_dm_timer *timer)
+int omap_dm_timer_stop(struct omap_dm_timer *timer, int pm)
 {
-	unsigned long rate = 0;
+	u32 l;
+	unsigned long rate;
 
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	/* OMAP1 is not converted to clk framework so avoid clk_get_rate here */
-	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET))
-		rate = clk_get_rate(timer->fclk);
+	rate = timer->rate;
+
+	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
+	if (l & OMAP_TIMER_CTRL_ST) {
+		l &= ~0x1;
+		omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
+#ifdef CONFIG_ARCH_OMAP2PLUS
+		/* Readback to make sure write has completed */
+		omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
+		/*
+		 * Wait for functional clock period x 3.5 to make sure that
+		 * timer is stopped
+		 */
+		udelay(3500000 / rate + 1);
+#endif
+	}
 
-	__omap_dm_timer_stop(timer, timer->posted, rate);
+	/* Ack possibly pending interrupt */
+	__raw_writel(OMAP_TIMER_INT_OVERFLOW, timer->irq_stat);
 
-	/*
-	 * Since the register values are computed and written within
-	 * __omap_dm_timer_stop, we need to use read to retrieve the
-	 * context.
-	 */
-	timer->context.tclr =
-			omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
-	omap_dm_timer_disable(timer);
+	if (pm) {
+		timer->context.tclr = l;
+		omap_dm_timer_disable(timer);
+	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_stop);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 7432bf3..1d3d9bb 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -141,7 +141,7 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer);
 
 int omap_dm_timer_trigger(struct omap_dm_timer *timer);
 int omap_dm_timer_start(struct omap_dm_timer *timer);
-int omap_dm_timer_stop(struct omap_dm_timer *timer);
+int omap_dm_timer_stop(struct omap_dm_timer *timer, int pm);
 
 int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source);
 int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload, unsigned int value);
@@ -366,30 +366,6 @@ static inline void __omap_dm_timer_override_errata(struct omap_dm_timer *timer,
 	timer->errata &= ~errata;
 }
 
-static inline void __omap_dm_timer_stop(struct omap_dm_timer *timer,
-					int posted, unsigned long rate)
-{
-	u32 l;
-
-	l = __omap_dm_timer_read(timer, OMAP_TIMER_CTRL_REG, posted);
-	if (l & OMAP_TIMER_CTRL_ST) {
-		l &= ~0x1;
-		__omap_dm_timer_write(timer, OMAP_TIMER_CTRL_REG, l, posted);
-#ifdef CONFIG_ARCH_OMAP2PLUS
-		/* Readback to make sure write has completed */
-		__omap_dm_timer_read(timer, OMAP_TIMER_CTRL_REG, posted);
-		/*
-		 * Wait for functional clock period x 3.5 to make sure that
-		 * timer is stopped
-		 */
-		udelay(3500000 / rate + 1);
-#endif
-	}
-
-	/* Ack possibly pending interrupt */
-	__raw_writel(OMAP_TIMER_INT_OVERFLOW, timer->irq_stat);
-}
-
 static inline void __omap_dm_timer_int_enable(struct omap_dm_timer *timer,
 						unsigned int value)
 {
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index b1e19a2..3dc736d 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -165,8 +165,8 @@ end:
 	/* Stop TX here */
 	lirc_rx51_off(lirc_rx51);
 	lirc_rx51->wbuf_index = -1;
-	omap_dm_timer_stop(lirc_rx51->pwm_timer);
-	omap_dm_timer_stop(lirc_rx51->pulse_timer);
+	omap_dm_timer_stop(lirc_rx51->pwm_timer, 1);
+	omap_dm_timer_stop(lirc_rx51->pulse_timer, 1);
 	omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
 	wake_up_interruptible(&lirc_rx51->wqueue);
 
diff --git a/drivers/staging/tidspbridge/core/dsp-clock.c b/drivers/staging/tidspbridge/core/dsp-clock.c
index 2788158..6ac36d1 100644
--- a/drivers/staging/tidspbridge/core/dsp-clock.c
+++ b/drivers/staging/tidspbridge/core/dsp-clock.c
@@ -300,7 +300,7 @@ int dsp_clk_disable(enum dsp_clk_id clk_id)
 		clk_disable(iva2_clk);
 		break;
 	case GPT_CLK:
-		status = omap_dm_timer_stop(timer[clk_id - 1]);
+		status = omap_dm_timer_stop(timer[clk_id - 1], 1);
 		break;
 #ifdef CONFIG_OMAP_MCBSP
 	case MCBSP_CLK:
-- 
1.7.9.5


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

* [PATCH 21/26] ARM: OMAP: dmtimer: Add systimer flag to dmtimer structure
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (19 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 20/26] ARM: OMAP: dmtimer: Fold back private stop function Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 22/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_write_status function Joel Fernandes
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Add a systimer flag to mark that a timer is used for system timers, this will
help simplify cases where we have to use runtime PM for non system timers, and
we don't have explicitly pass arguments to functions to tell it to do runtime
PM and context save or not.

Also will be useful in upcoming patches to not do certain pm runtime checks
for !systimers.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c                  |   12 ++++++------
 arch/arm/plat-omap/dmtimer.c                 |   11 ++++++-----
 arch/arm/plat-omap/include/plat/dmtimer.h    |    5 +++--
 drivers/media/rc/ir-rx51.c                   |    4 ++--
 drivers/staging/tidspbridge/core/dsp-clock.c |    4 ++--
 5 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 2d8ea96..5cceaae 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -64,7 +64,7 @@
 
 /* Clockevent code */
 
-static struct omap_dm_timer clkev;
+static struct omap_dm_timer clkev = { .systimer = 1 };
 static struct clock_event_device clockevent_gpt;
 
 #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
@@ -95,7 +95,7 @@ static struct irqaction omap2_gp_timer_irq = {
 static int omap2_gp_timer_set_next_event(unsigned long cycles,
 					 struct clock_event_device *evt)
 {
-	omap_dm_timer_set_load_start(&clkev, 0xffffffff - cycles, 0, 0);
+	omap_dm_timer_set_load_start(&clkev, 0xffffffff - cycles, 0);
 
 	return 0;
 }
@@ -105,13 +105,13 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
 {
 	u32 period;
 
-	omap_dm_timer_stop(&clkev, 0);
+	omap_dm_timer_stop(&clkev);
 
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		period = clkev.rate / HZ;
 		period -= 1;
-		omap_dm_timer_set_load_start(&clkev, 0xffffffff - period, 1, 0);
+		omap_dm_timer_set_load_start(&clkev, 0xffffffff - period, 1);
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
 		break;
@@ -415,7 +415,7 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 }
 
 /* Clocksource code */
-static struct omap_dm_timer clksrc;
+static struct omap_dm_timer clksrc = { .systimer = 1 };
 static bool use_gptimer_clksrc;
 
 /*
@@ -524,7 +524,7 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 
 	clocksource_gpt.name = "timer_clksrc";
 
-	omap_dm_timer_set_load_start(&clksrc, 0, 1, 0);
+	omap_dm_timer_set_load_start(&clksrc, 0, 1);
 
 	sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate);
 
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 8602586..504784f 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -467,7 +467,7 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_start);
 
-int omap_dm_timer_stop(struct omap_dm_timer *timer, int pm)
+int omap_dm_timer_stop(struct omap_dm_timer *timer)
 {
 	u32 l;
 	unsigned long rate;
@@ -495,7 +495,7 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer, int pm)
 	/* Ack possibly pending interrupt */
 	__raw_writel(OMAP_TIMER_INT_OVERFLOW, timer->irq_stat);
 
-	if (pm) {
+	if (!timer->systimer) {
 		timer->context.tclr = l;
 		omap_dm_timer_disable(timer);
 	}
@@ -589,7 +589,7 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_load);
 
 /* Optimized set_load which removes costly spin wait in timer_start */
 int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, unsigned int load,
-				 int autoreload, int pm)
+				 int autoreload)
 {
 	int rc;
 	u32 mask = ~0, val = 0;
@@ -597,7 +597,7 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, unsigned int load,
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	if (pm) {
+	if (!timer->systimer) {
 		rc = omap_dm_timer_enable(timer);
 		if (rc)
 			return rc;
@@ -615,12 +615,13 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, unsigned int load,
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_COUNTER_REG, load);
 	val = omap_dm_timer_write_ctrl(timer, mask, val);
 
-	if (pm) {
+	if (!timer->systimer) {
 		/* Save the context */
 		timer->context.tclr = val;
 		timer->context.tldr = load;
 		timer->context.tcrr = load;
 	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 1d3d9bb..7005a1e 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -118,6 +118,7 @@ struct omap_dm_timer {
 	int (*get_context_loss_count)(struct device *);
 	int ctx_loss_count;
 	int revision;
+	int systimer;
 	u32 capability;
 	u32 errata;
 	struct platform_device *pdev;
@@ -141,12 +142,12 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer);
 
 int omap_dm_timer_trigger(struct omap_dm_timer *timer);
 int omap_dm_timer_start(struct omap_dm_timer *timer);
-int omap_dm_timer_stop(struct omap_dm_timer *timer, int pm);
+int omap_dm_timer_stop(struct omap_dm_timer *timer);
 
 int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source);
 int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload, unsigned int value);
 int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, unsigned int load,
-				 int autoreload, int pm);
+				 int autoreload);
 int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable, unsigned int match);
 int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on, int toggle, int trigger);
 int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler);
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 3dc736d..b1e19a2 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -165,8 +165,8 @@ end:
 	/* Stop TX here */
 	lirc_rx51_off(lirc_rx51);
 	lirc_rx51->wbuf_index = -1;
-	omap_dm_timer_stop(lirc_rx51->pwm_timer, 1);
-	omap_dm_timer_stop(lirc_rx51->pulse_timer, 1);
+	omap_dm_timer_stop(lirc_rx51->pwm_timer);
+	omap_dm_timer_stop(lirc_rx51->pulse_timer);
 	omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
 	wake_up_interruptible(&lirc_rx51->wqueue);
 
diff --git a/drivers/staging/tidspbridge/core/dsp-clock.c b/drivers/staging/tidspbridge/core/dsp-clock.c
index 6ac36d1..e4b4181 100644
--- a/drivers/staging/tidspbridge/core/dsp-clock.c
+++ b/drivers/staging/tidspbridge/core/dsp-clock.c
@@ -189,7 +189,7 @@ void dsp_gpt_wait_overflow(short int clk_id, unsigned int load)
 	 * Set counter value to overflow counter after
 	 * one tick and start timer.
 	 */
-	omap_dm_timer_set_load_start(gpt, load, 0, 1);
+	omap_dm_timer_set_load_start(gpt, load, 0);
 
 	/* Wait 80us for timer to overflow */
 	udelay(80);
@@ -300,7 +300,7 @@ int dsp_clk_disable(enum dsp_clk_id clk_id)
 		clk_disable(iva2_clk);
 		break;
 	case GPT_CLK:
-		status = omap_dm_timer_stop(timer[clk_id - 1], 1);
+		status = omap_dm_timer_stop(timer[clk_id - 1]);
 		break;
 #ifdef CONFIG_OMAP_MCBSP
 	case MCBSP_CLK:
-- 
1.7.9.5


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

* [PATCH 22/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_write_status function
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (20 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 21/26] ARM: OMAP: dmtimer: Add systimer flag to dmtimer structure Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 23/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_read_counter function Joel Fernandes
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

__omap_dm_timer_write_status is folded back into omap_dm_timer_write_status,
all users are converted to use it, and old API is removed. This works thanks to
the systimer flag just introduced in the series.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c               |    2 +-
 arch/arm/plat-omap/dmtimer.c              |    5 ++++-
 arch/arm/plat-omap/include/plat/dmtimer.h |    6 ------
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 5cceaae..7c0cd72 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -80,7 +80,7 @@ static irqreturn_t omap2_gp_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = &clockevent_gpt;
 
-	__omap_dm_timer_write_status(&clkev, OMAP_TIMER_INT_OVERFLOW);
+	omap_dm_timer_write_status(&clkev, OMAP_TIMER_INT_OVERFLOW);
 
 	evt->event_handler(evt);
 	return IRQ_HANDLED;
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 504784f..af67e30 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -768,6 +768,9 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);
 
 static int is_timer_available(struct omap_dm_timer *timer)
 {
+	if (timer && timer->systimer)
+		return 1;
+
 	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
 		pr_err("Timer not available or enabled.\n");
 		WARN_ON(1);
@@ -794,7 +797,7 @@ int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
 	if (!is_timer_available(timer))
 		return -EINVAL;
 
-	__omap_dm_timer_write_status(timer, value);
+	__raw_writel(value, timer->irq_stat);
 
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 7005a1e..f542ef2 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -380,10 +380,4 @@ __omap_dm_timer_read_counter(struct omap_dm_timer *timer, int posted)
 	return __omap_dm_timer_read(timer, OMAP_TIMER_COUNTER_REG, posted);
 }
 
-static inline void __omap_dm_timer_write_status(struct omap_dm_timer *timer,
-						unsigned int value)
-{
-	__raw_writel(value, timer->irq_stat);
-}
-
 #endif /* __ASM_ARCH_DMTIMER_H */
-- 
1.7.9.5


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

* [PATCH 23/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_read_counter function
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (21 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 22/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_write_status function Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 24/26] ARM: OMAP: dmtimer: Move private functions into dmtimer core and export others Joel Fernandes
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

__omap_dm_timer_read_counter functionality is folded back into
omap_dm_timer_read_counter and all users converted to use it.

Made possible due to the systimer flags introduced in the series. Also there's
no need to pass posted flags anymore as this data is available in the dmtimer
structure.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c               |    6 ++----
 arch/arm/plat-omap/dmtimer.c              |    2 +-
 arch/arm/plat-omap/include/plat/dmtimer.h |    6 ------
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 7c0cd72..39b3e34 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -423,8 +423,7 @@ static bool use_gptimer_clksrc;
  */
 static cycle_t clocksource_read_cycles(struct clocksource *cs)
 {
-	return (cycle_t)__omap_dm_timer_read_counter(&clksrc,
-						     OMAP_TIMER_NONPOSTED);
+	return (cycle_t)omap_dm_timer_read_counter(&clksrc);
 }
 
 static struct clocksource clocksource_gpt = {
@@ -437,8 +436,7 @@ static struct clocksource clocksource_gpt = {
 static u64 notrace dmtimer_read_sched_clock(void)
 {
 	if (clksrc.reserved)
-		return __omap_dm_timer_read_counter(&clksrc,
-						    OMAP_TIMER_NONPOSTED);
+		return omap_dm_timer_read_counter(&clksrc);
 
 	return 0;
 }
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index af67e30..44cc336 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -808,7 +808,7 @@ unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
 	if (!is_timer_available(timer))
 		return 0;
 
-	return __omap_dm_timer_read_counter(timer, timer->posted);
+	return omap_dm_timer_read_reg(timer, OMAP_TIMER_COUNTER_REG);
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_read_counter);
 
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index f542ef2..6ab801b 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -374,10 +374,4 @@ static inline void __omap_dm_timer_int_enable(struct omap_dm_timer *timer,
 	__omap_dm_timer_write(timer, OMAP_TIMER_WAKEUP_EN_REG, value, 0);
 }
 
-static inline unsigned int
-__omap_dm_timer_read_counter(struct omap_dm_timer *timer, int posted)
-{
-	return __omap_dm_timer_read(timer, OMAP_TIMER_COUNTER_REG, posted);
-}
-
 #endif /* __ASM_ARCH_DMTIMER_H */
-- 
1.7.9.5


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

* [PATCH 24/26] ARM: OMAP: dmtimer: Move private functions into dmtimer core and export others
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (22 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 23/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_read_counter function Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 25/26] ARM: OMAP: dmtimer: Eliminate omap_dm_timer_int_enable function Joel Fernandes
  2014-04-24 21:44 ` [PATCH 26/26] ARM: OMAP: dmtimer: Use is_timer_available function in omap_dm_timer_trigger Joel Fernandes
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

omap_dm_timer_read/write are no longer used by anyone else outside of dmtimer.c,
This allows us to remove it completely from the dmtimer.h header.

The reset of the functions are moved into dmtimer.c and the "__" prefix is removed,
and they're exported for use by others such as OMAP2+ system timers.
omap_dm_timer_init_regs
omap_dm_timer_enable_posted
omap_dm_timer_override_errata
omap_dm_timer_int_enable

All other functions in dmtimer.h have been either removed, or folded into other similar
functions, and such users converted to use the one folded into.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c               |    8 +--
 arch/arm/plat-omap/dmtimer.c              |  105 ++++++++++++++++++++++++++++-
 arch/arm/plat-omap/include/plat/dmtimer.h |  103 ++--------------------------
 3 files changed, 113 insertions(+), 103 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 39b3e34..afb22eb 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -363,9 +363,9 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 
 	clk_put(src);
 
-	__omap_dm_timer_init_regs(timer);
+	omap_dm_timer_init_regs(timer);
 	if (posted)
-		__omap_dm_timer_enable_posted(timer);
+		omap_dm_timer_enable_posted(timer);
 
 	/* Check that the intended posted configuration matches the actual */
 	if (posted != timer->posted)
@@ -391,7 +391,7 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 	 * so we are not impacted by errata i103 and i767. Therefore,
 	 * we can safely ignore this errata for clock-event timers.
 	 */
-	__omap_dm_timer_override_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767);
+	omap_dm_timer_override_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767);
 
 	clockevent_gpt.name = "timer_clkev";
 
@@ -402,7 +402,7 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 	omap2_gp_timer_irq.dev_id = &clkev;
 	setup_irq(clkev.irq, &omap2_gp_timer_irq);
 
-	__omap_dm_timer_int_enable(&clkev, OMAP_TIMER_INT_OVERFLOW);
+	omap_dm_timer_int_enable(&clkev, OMAP_TIMER_INT_OVERFLOW);
 
 	clockevent_gpt.cpumask = cpu_possible_mask;
 	clockevent_gpt.irq = omap_dm_timer_get_irq(&clkev);
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 44cc336..f7c1d14 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -59,6 +59,26 @@ enum {
 	REQUEST_BY_NODE,
 };
 
+static inline u32 __omap_dm_timer_read(struct omap_dm_timer *timer, u32 reg,
+		int posted)
+{
+	if (posted)
+		while (__raw_readl(timer->pend) & (reg >> WPSHIFT))
+			cpu_relax();
+
+	return __raw_readl(timer->func_base + (reg & 0xff));
+}
+
+static inline void __omap_dm_timer_write(struct omap_dm_timer *timer,
+		u32 reg, u32 val, int posted)
+{
+	if (posted)
+		while (__raw_readl(timer->pend) & (reg >> WPSHIFT))
+			cpu_relax();
+
+	__raw_writel(val, timer->func_base + (reg & 0xff));
+}
+
 /**
  * omap_dm_timer_read_reg - read timer registers in posted and non-posted mode
  * @timer:      timer pointer over which read operation to perform
@@ -176,7 +196,7 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)
 		}
 	}
 
-	__omap_dm_timer_enable_posted(timer);
+	omap_dm_timer_enable_posted(timer);
 	omap_dm_timer_disable(timer);
 
 	/*
@@ -201,6 +221,85 @@ int omap_dm_timer_reserve_systimer(int id)
 	return 0;
 }
 
+void omap_dm_timer_init_regs(struct omap_dm_timer *timer)
+{
+	u32 tidr;
+
+	/* Assume v1 ip if bits [31:16] are zero */
+	tidr = __raw_readl(timer->io_base);
+	if (!(tidr >> 16)) {
+		timer->revision = 1;
+		timer->irq_stat = timer->io_base + OMAP_TIMER_V1_STAT_OFFSET;
+		timer->irq_ena = timer->io_base + OMAP_TIMER_V1_INT_EN_OFFSET;
+		timer->irq_dis = timer->io_base + OMAP_TIMER_V1_INT_EN_OFFSET;
+		timer->pend = timer->io_base + _OMAP_TIMER_WRITE_PEND_OFFSET;
+		timer->func_base = timer->io_base;
+	} else {
+		timer->revision = 2;
+		timer->irq_stat = timer->io_base + OMAP_TIMER_V2_IRQSTATUS;
+		timer->irq_ena = timer->io_base + OMAP_TIMER_V2_IRQENABLE_SET;
+		timer->irq_dis = timer->io_base + OMAP_TIMER_V2_IRQENABLE_CLR;
+		timer->pend = timer->io_base +
+			_OMAP_TIMER_WRITE_PEND_OFFSET +
+				OMAP_TIMER_V2_FUNC_OFFSET;
+		timer->func_base = timer->io_base + OMAP_TIMER_V2_FUNC_OFFSET;
+	}
+}
+EXPORT_SYMBOL_GPL(omap_dm_timer_init_regs);
+
+/*
+ * omap_dm_timer_enable_posted - enables write posted mode
+ * @timer:      pointer to timer instance handle
+ *
+ * Enables the write posted mode for the timer. When posted mode is enabled
+ * writes to certain timer registers are immediately acknowledged by the
+ * internal bus and hence prevents stalling the CPU waiting for the write to
+ * complete. Enabling this feature can improve performance for writing to the
+ * timer registers.
+ */
+void omap_dm_timer_enable_posted(struct omap_dm_timer *timer)
+{
+	if (timer->posted)
+		return;
+
+	if (timer->errata & OMAP_TIMER_ERRATA_I103_I767) {
+		timer->posted = OMAP_TIMER_NONPOSTED;
+		__omap_dm_timer_write(timer, OMAP_TIMER_IF_CTRL_REG, 0, 0);
+		return;
+	}
+
+	__omap_dm_timer_write(timer, OMAP_TIMER_IF_CTRL_REG,
+			      OMAP_TIMER_CTRL_POSTED, 0);
+	timer->context.tsicr = OMAP_TIMER_CTRL_POSTED;
+	timer->posted = OMAP_TIMER_POSTED;
+}
+EXPORT_SYMBOL_GPL(omap_dm_timer_enable_posted);
+
+/**
+ * omap_dm_timer_override_errata - override errata flags for a timer
+ * @timer:      pointer to timer handle
+ * @errata:	errata flags to be ignored
+ *
+ * For a given timer, override a timer errata by clearing the flags
+ * specified by the errata argument. A specific erratum should only be
+ * overridden for a timer if the timer is used in such a way the erratum
+ * has no impact.
+ */
+void omap_dm_timer_override_errata(struct omap_dm_timer *timer,
+						   u32 errata)
+{
+	timer->errata &= ~errata;
+}
+EXPORT_SYMBOL_GPL(omap_dm_timer_override_errata);
+
+void omap_dm_timer_int_enable(struct omap_dm_timer *timer,
+						unsigned int value)
+{
+	__raw_writel(value, timer->irq_ena);
+	__omap_dm_timer_write(timer, OMAP_TIMER_WAKEUP_EN_REG, value, 0);
+}
+EXPORT_SYMBOL_GPL(omap_dm_timer_int_enable);
+
 /*
  * Check if a timer is running based on timer_id, used for OMAP1 currently.
  */
@@ -722,7 +821,7 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
 	if (rc)
 		return rc;
 
-	__omap_dm_timer_int_enable(timer, value);
+	omap_dm_timer_int_enable(timer, value);
 
 	/* Save the context */
 	timer->context.tier = value;
@@ -917,7 +1016,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
 
 	if (!timer->reserved) {
 		pm_runtime_get_sync(dev);
-		__omap_dm_timer_init_regs(timer);
+		omap_dm_timer_init_regs(timer);
 		pm_runtime_put(dev);
 	}
 
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 6ab801b..9b52607 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -162,6 +162,13 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
 
 int omap_dm_timers_active(void);
 
+void omap_dm_timer_init_regs(struct omap_dm_timer *timer);
+void omap_dm_timer_enable_posted(struct omap_dm_timer *timer);
+void omap_dm_timer_override_errata(struct omap_dm_timer *timer,
+				   u32 errata);
+void omap_dm_timer_int_enable(struct omap_dm_timer *timer,
+			      unsigned int value);
+
 /*
  * Do not use the defines below, they are not needed. They should be only
  * used by dmtimer.c and sys_timer related code.
@@ -278,100 +285,4 @@ int omap_dm_timers_active(void);
 
 #define OMAP_TIMER_TICK_INT_MASK_COUNT_REG				\
 		(_OMAP_TIMER_TICK_INT_MASK_COUNT_OFFSET | (WP_TOWR << WPSHIFT))
-
-static inline u32 __omap_dm_timer_read(struct omap_dm_timer *timer, u32 reg,
-						int posted)
-{
-	if (posted)
-		while (__raw_readl(timer->pend) & (reg >> WPSHIFT))
-			cpu_relax();
-
-	return __raw_readl(timer->func_base + (reg & 0xff));
-}
-
-static inline void __omap_dm_timer_write(struct omap_dm_timer *timer,
-					u32 reg, u32 val, int posted)
-{
-	if (posted)
-		while (__raw_readl(timer->pend) & (reg >> WPSHIFT))
-			cpu_relax();
-
-	__raw_writel(val, timer->func_base + (reg & 0xff));
-}
-
-static inline void __omap_dm_timer_init_regs(struct omap_dm_timer *timer)
-{
-	u32 tidr;
-
-	/* Assume v1 ip if bits [31:16] are zero */
-	tidr = __raw_readl(timer->io_base);
-	if (!(tidr >> 16)) {
-		timer->revision = 1;
-		timer->irq_stat = timer->io_base + OMAP_TIMER_V1_STAT_OFFSET;
-		timer->irq_ena = timer->io_base + OMAP_TIMER_V1_INT_EN_OFFSET;
-		timer->irq_dis = timer->io_base + OMAP_TIMER_V1_INT_EN_OFFSET;
-		timer->pend = timer->io_base + _OMAP_TIMER_WRITE_PEND_OFFSET;
-		timer->func_base = timer->io_base;
-	} else {
-		timer->revision = 2;
-		timer->irq_stat = timer->io_base + OMAP_TIMER_V2_IRQSTATUS;
-		timer->irq_ena = timer->io_base + OMAP_TIMER_V2_IRQENABLE_SET;
-		timer->irq_dis = timer->io_base + OMAP_TIMER_V2_IRQENABLE_CLR;
-		timer->pend = timer->io_base +
-			_OMAP_TIMER_WRITE_PEND_OFFSET +
-				OMAP_TIMER_V2_FUNC_OFFSET;
-		timer->func_base = timer->io_base + OMAP_TIMER_V2_FUNC_OFFSET;
-	}
-}
-
-/*
- * __omap_dm_timer_enable_posted - enables write posted mode
- * @timer:      pointer to timer instance handle
- *
- * Enables the write posted mode for the timer. When posted mode is enabled
- * writes to certain timer registers are immediately acknowledged by the
- * internal bus and hence prevents stalling the CPU waiting for the write to
- * complete. Enabling this feature can improve performance for writing to the
- * timer registers.
- */
-static inline void __omap_dm_timer_enable_posted(struct omap_dm_timer *timer)
-{
-	if (timer->posted)
-		return;
-
-	if (timer->errata & OMAP_TIMER_ERRATA_I103_I767) {
-		timer->posted = OMAP_TIMER_NONPOSTED;
-		__omap_dm_timer_write(timer, OMAP_TIMER_IF_CTRL_REG, 0, 0);
-		return;
-	}
-
-	__omap_dm_timer_write(timer, OMAP_TIMER_IF_CTRL_REG,
-			      OMAP_TIMER_CTRL_POSTED, 0);
-	timer->context.tsicr = OMAP_TIMER_CTRL_POSTED;
-	timer->posted = OMAP_TIMER_POSTED;
-}
-
-/**
- * __omap_dm_timer_override_errata - override errata flags for a timer
- * @timer:      pointer to timer handle
- * @errata:	errata flags to be ignored
- *
- * For a given timer, override a timer errata by clearing the flags
- * specified by the errata argument. A specific erratum should only be
- * overridden for a timer if the timer is used in such a way the erratum
- * has no impact.
- */
-static inline void __omap_dm_timer_override_errata(struct omap_dm_timer *timer,
-						   u32 errata)
-{
-	timer->errata &= ~errata;
-}
-
-static inline void __omap_dm_timer_int_enable(struct omap_dm_timer *timer,
-						unsigned int value)
-{
-	__raw_writel(value, timer->irq_ena);
-	__omap_dm_timer_write(timer, OMAP_TIMER_WAKEUP_EN_REG, value, 0);
-}
-
 #endif /* __ASM_ARCH_DMTIMER_H */
-- 
1.7.9.5


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

* [PATCH 25/26] ARM: OMAP: dmtimer: Eliminate omap_dm_timer_int_enable function
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (23 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 24/26] ARM: OMAP: dmtimer: Move private functions into dmtimer core and export others Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  2014-04-24 21:44 ` [PATCH 26/26] ARM: OMAP: dmtimer: Use is_timer_available function in omap_dm_timer_trigger Joel Fernandes
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Fold this function back into omap_dm_timer_set_int_enable and
use the systimer flag to not call PM functions for system timer.
Then remove it from the header file, and convert users to use
omap_dm_timer_set_int_enable.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/timer.c               |    2 +-
 arch/arm/plat-omap/dmtimer.c              |   48 +++++++++++++++--------------
 arch/arm/plat-omap/include/plat/dmtimer.h |    3 --
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index afb22eb..ba75385 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -402,7 +402,7 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 	omap2_gp_timer_irq.dev_id = &clkev;
 	setup_irq(clkev.irq, &omap2_gp_timer_irq);
 
-	omap_dm_timer_int_enable(&clkev, OMAP_TIMER_INT_OVERFLOW);
+	omap_dm_timer_set_int_enable(&clkev, OMAP_TIMER_INT_OVERFLOW);
 
 	clockevent_gpt.cpumask = cpu_possible_mask;
 	clockevent_gpt.irq = omap_dm_timer_get_irq(&clkev);
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index f7c1d14..1059881 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -292,14 +292,6 @@ void omap_dm_timer_override_errata(struct omap_dm_timer *timer,
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_override_errata);
 
-void omap_dm_timer_int_enable(struct omap_dm_timer *timer,
-						unsigned int value)
-{
-	__raw_writel(value, timer->irq_ena);
-	__omap_dm_timer_write(timer, OMAP_TIMER_WAKEUP_EN_REG, value, 0);
-}
-EXPORT_SYMBOL_GPL(omap_dm_timer_int_enable);
-
 /*
  * Check if a timer is running based on timer_id, used for OMAP1 currently.
  */
@@ -817,16 +809,21 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	rc = omap_dm_timer_enable(timer);
-	if (rc)
-		return rc;
+	if (!timer->systimer) {
+		rc = omap_dm_timer_enable(timer);
+		if (rc)
+			return rc;
+	}
 
-	omap_dm_timer_int_enable(timer, value);
+	__raw_writel(value, timer->irq_ena);
+	__omap_dm_timer_write(timer, OMAP_TIMER_WAKEUP_EN_REG, value, 0);
 
-	/* Save the context */
-	timer->context.tier = value;
-	timer->context.twer = value;
-	omap_dm_timer_disable(timer);
+	if (!timer->systimer) {
+		/* Save the context */
+		timer->context.tier = value;
+		timer->context.twer = value;
+		omap_dm_timer_disable(timer);
+	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_enable);
@@ -846,9 +843,11 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
 	if (unlikely(!timer))
 		return -EINVAL;
 
-	rc = omap_dm_timer_enable(timer);
-	if (rc)
-		return rc;
+	if (!timer->systimer) {
+		rc = omap_dm_timer_enable(timer);
+		if (rc)
+			return rc;
+	}
 
 	if (timer->revision == 1)
 		l = __raw_readl(timer->irq_ena) & ~mask;
@@ -857,10 +856,13 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
 	l = omap_dm_timer_read_reg(timer, OMAP_TIMER_WAKEUP_EN_REG) & ~mask;
 	omap_dm_timer_write_reg(timer, OMAP_TIMER_WAKEUP_EN_REG, l);
 
-	/* Save the context */
-	timer->context.tier &= ~mask;
-	timer->context.twer &= ~mask;
-	omap_dm_timer_disable(timer);
+
+	if (!timer->systimer) {
+		/* Save the context */
+		timer->context.tier &= ~mask;
+		timer->context.twer &= ~mask;
+		omap_dm_timer_disable(timer);
+	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 9b52607..2f27dca 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -166,9 +166,6 @@ void omap_dm_timer_init_regs(struct omap_dm_timer *timer);
 void omap_dm_timer_enable_posted(struct omap_dm_timer *timer);
 void omap_dm_timer_override_errata(struct omap_dm_timer *timer,
 				   u32 errata);
-void omap_dm_timer_int_enable(struct omap_dm_timer *timer,
-			      unsigned int value);
-
 /*
  * Do not use the defines below, they are not needed. They should be only
  * used by dmtimer.c and sys_timer related code.
-- 
1.7.9.5


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

* [PATCH 26/26] ARM: OMAP: dmtimer: Use is_timer_available function in omap_dm_timer_trigger
  2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
                   ` (24 preceding siblings ...)
  2014-04-24 21:44 ` [PATCH 25/26] ARM: OMAP: dmtimer: Eliminate omap_dm_timer_int_enable function Joel Fernandes
@ 2014-04-24 21:44 ` Joel Fernandes
  25 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-04-24 21:44 UTC (permalink / raw)
  To: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List
  Cc: Tony Lindgren, Joel Fernandes

Timer availability can be checked with is_timer_available function. Move it
before omap_dm_timer_trigger and use it.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/plat-omap/dmtimer.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 1059881..0d0e83b 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -221,6 +221,19 @@ int omap_dm_timer_reserve_systimer(int id)
 	return 0;
 }
 
+static int is_timer_available(struct omap_dm_timer *timer)
+{
+	if (timer && timer->systimer)
+		return 1;
+
+	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
+		pr_err("Timer not available or enabled.\n");
+		WARN_ON(1);
+		return 0;
+	}
+	return 1;
+}
+
 void omap_dm_timer_init_regs(struct omap_dm_timer *timer)
 {
 	u32 tidr;
@@ -524,7 +537,7 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
 
 int omap_dm_timer_trigger(struct omap_dm_timer *timer)
 {
-	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
+	if (!is_timer_available(timer)) {
 		pr_err("%s: timer not available or enabled.\n", __func__);
 		return -EINVAL;
 	}
@@ -867,19 +880,6 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);
 
-static int is_timer_available(struct omap_dm_timer *timer)
-{
-	if (timer && timer->systimer)
-		return 1;
-
-	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
-		pr_err("Timer not available or enabled.\n");
-		WARN_ON(1);
-		return 0;
-	}
-	return 1;
-}
-
 unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
 {
 	unsigned int l;
-- 
1.7.9.5


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

* Re: [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook
  2014-04-24 21:43 ` [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook Joel Fernandes
@ 2014-05-07 15:19   ` Tony Lindgren
  2014-05-07 21:43     ` Joel Fernandes
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 15:19 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140424 14:44]:
> There is a platform specific hook just for OMAP1 to set its clk parent.  Remove
> this hook and have OMAP1 set its parent in omap1_dm_timer_init.  If OMAP1 is
> ever migrated to clock framework, the correct way to do this would be through
> clk_set_parent like other platforms.
> 
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
>  arch/arm/mach-omap1/timer.c                |    8 +++++++-
>  arch/arm/plat-omap/dmtimer.c               |    8 +++-----
>  include/linux/platform_data/dmtimer-omap.h |    2 --
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
> index bde7a35..4b9c604 100644
> --- a/arch/arm/mach-omap1/timer.c
> +++ b/arch/arm/mach-omap1/timer.c
> @@ -140,7 +140,13 @@ static int __init omap1_dm_timer_init(void)
>  			goto err_free_pdata;
>  		}
>  
> -		pdata->set_timer_src = omap1_dm_timer_set_src;
> +		/*
> +		 * Since OMAP1 doesn't support clock framework, set timer clock
> +		 * source to 32KHz here instead of expecting it to be set by
> +		 * dmtimer code.
> +		 */
> +		omap1_dm_timer_set_src(pdev, 0x01);
> +
>  		pdata->timer_capability = OMAP_TIMER_ALWON |
>  				OMAP_TIMER_NEEDS_RESET | OMAP_TIMER_HAS_DSP_IRQ;
>  

This does not sound right, omap1 does support clock framework just fine.
It is not using the common clock framework though.

This breaks omap_dm_timer_set_source() for sure. Setting the source
during init is not a right solution here. Probably best to keep the
pdata hook around, drivers can support pdata and DT data together
just fine.

Regards,

Tony

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

* Re: [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework
  2014-04-24 21:43 ` [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework Joel Fernandes
@ 2014-05-07 15:20   ` Tony Lindgren
  2014-05-07 21:48     ` Joel Fernandes
  2014-05-07 22:10     ` Joel Fernandes
  0 siblings, 2 replies; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 15:20 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140424 14:44]:
> OMAP1 doesn't support clock framework, add a comment where needed
> and correct a FIXME.

This is at least a wrong comment, the original comment seems right
to me.

Tony
 
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
>  arch/arm/plat-omap/dmtimer.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> index ecd3f97..f5a674c 100644
> --- a/arch/arm/plat-omap/dmtimer.c
> +++ b/arch/arm/plat-omap/dmtimer.c
> @@ -142,8 +142,7 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)
>  	int rc;
>  
>  	/*
> -	 * FIXME: OMAP1 devices do not use the clock framework for dmtimers so
> -	 * do not call clk_get() for these devices.
> +	 * Do not call clk_get() for OMAP1 due to no clock framework support.
>  	 */
>  	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET)) {
>  		timer->fclk = clk_get(&timer->pdev->dev, "fck");
> @@ -461,6 +460,7 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
>  	if (unlikely(!timer))
>  		return -EINVAL;
>  
> +	/* OMAP1 is not converted to clk framework so avoid clk_get_rate here */
>  	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET))
>  		rate = clk_get_rate(timer->fclk);
>  
> -- 
> 1.7.9.5
> 

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

* Re: [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error
  2014-04-24 21:43 ` [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error Joel Fernandes
@ 2014-05-07 15:24   ` Tony Lindgren
  2014-05-07 21:52     ` Joel Fernandes
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 15:24 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140424 14:44]:
> The subsequent devm_ioremap_resource will catch it and print an error, let it
> be checked there.
> 
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
>  arch/arm/plat-omap/dmtimer.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> index 7e806f9..1fd30fa 100644
> --- a/arch/arm/plat-omap/dmtimer.c
> +++ b/arch/arm/plat-omap/dmtimer.c
> @@ -810,10 +810,6 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>  	}
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (unlikely(!mem)) {
> -		dev_err(dev, "%s: no memory resource.\n", __func__);
> -		return -ENODEV;
> -	}
>  
>  	timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
>  	if (!timer) {

We still need to return an error here and not try to continue though.

Tony

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

* Re: [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running
  2014-04-24 21:43 ` [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running Joel Fernandes
@ 2014-05-07 15:25   ` Tony Lindgren
  2014-05-07 22:00     ` Joel Fernandes
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 15:25 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140424 14:44]:
> Inorder to move non-DM timer specific code that modifies the "idlect"
> mask on OMAP1, from dmtimer code, to OMAP1 specific timer initialization code,
> we introduce a new function that can possibly be reused for other purposes in
> the future. The function just checks if a timer is running based on the timer ID
> which should be same as pdev->id. This allows us to cleanly separate the timer vs
> non-timer bits and keep the timer bits in the dmtimer code.
> 
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
>  arch/arm/plat-omap/dmtimer.c              |   29 +++++++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/dmtimer.h |    2 ++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> index 4debb3d..86b2641 100644
> --- a/arch/arm/plat-omap/dmtimer.c
> +++ b/arch/arm/plat-omap/dmtimer.c
> @@ -187,6 +187,35 @@ int omap_dm_timer_reserve_systimer(int id)
>  	return 0;
>  }
>  
> +/*
> + * Check if a timer is running based on timer_id, used for OMAP1 currently.
> + */
> +int omap_dm_timer_is_running(int timer_id)
> +{
> +	int i = 1, ret = 0;
> +	struct omap_dm_timer *timer = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&dm_timer_lock, flags);
> +	list_for_each_entry(timer, &omap_timer_list, node) {
> +		if (i == timer_id) {
> +			u32 l;
> +			l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
> +			if (l & OMAP_TIMER_CTRL_ST) {
> +				ret = 1;
> +				goto done;
> +			} else {
> +				goto done;
> +			}
> +		}
> +		i++;
> +	}
> +done:
> +	spin_unlock_irqrestore(&dm_timer_lock, flags);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(omap_dm_timer_is_running);

Let's not add new exported custom functions, let's instead try to get
rid of them. What needs to use this one?

Regards,


Tony

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

* Re: [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook
  2014-05-07 15:19   ` Tony Lindgren
@ 2014-05-07 21:43     ` Joel Fernandes
  2014-05-07 22:04       ` Tony Lindgren
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 21:43 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 10:19 AM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>> There is a platform specific hook just for OMAP1 to set its clk parent.  Remove
>> this hook and have OMAP1 set its parent in omap1_dm_timer_init.  If OMAP1 is
>> ever migrated to clock framework, the correct way to do this would be through
>> clk_set_parent like other platforms.
>>
>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>> ---
>>  arch/arm/mach-omap1/timer.c                |    8 +++++++-
>>  arch/arm/plat-omap/dmtimer.c               |    8 +++-----
>>  include/linux/platform_data/dmtimer-omap.h |    2 --
>>  3 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
>> index bde7a35..4b9c604 100644
>> --- a/arch/arm/mach-omap1/timer.c
>> +++ b/arch/arm/mach-omap1/timer.c
>> @@ -140,7 +140,13 @@ static int __init omap1_dm_timer_init(void)
>>  			goto err_free_pdata;
>>  		}
>>  
>> -		pdata->set_timer_src = omap1_dm_timer_set_src;
>> +		/*
>> +		 * Since OMAP1 doesn't support clock framework, set timer clock
>> +		 * source to 32KHz here instead of expecting it to be set by
>> +		 * dmtimer code.
>> +		 */
>> +		omap1_dm_timer_set_src(pdev, 0x01);
>> +
>>  		pdata->timer_capability = OMAP_TIMER_ALWON |
>>  				OMAP_TIMER_NEEDS_RESET | OMAP_TIMER_HAS_DSP_IRQ;
>>  
> 
> This does not sound right, omap1 does support clock framework just fine.
> It is not using the common clock framework though.
> 
> This breaks omap_dm_timer_set_source() for sure. Setting the source
> during init is not a right solution here. Probably best to keep the
> pdata hook around, drivers can support pdata and DT data together
> just fine.

Actually pdata hook in this case is used only in 1 place, and is OMAP1
specific so I felt its better to clean it up.

Can you elaborate a bit more on why it breaks? I have no way of testing
OMAP1 without hardware.

What difference is it to set MOD_CONF_CTRL_1 in omap1's
arch_initcall(omap1_dm_timer_init)  versus doing so later?

-Joel


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

* Re: [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework
  2014-05-07 15:20   ` Tony Lindgren
@ 2014-05-07 21:48     ` Joel Fernandes
  2014-05-07 22:07       ` Tony Lindgren
  2014-05-07 22:10     ` Joel Fernandes
  1 sibling, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 21:48 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 10:20 AM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>> OMAP1 doesn't support clock framework, add a comment where needed
>> and correct a FIXME.
> 
> This is at least a wrong comment, the original comment seems right
> to me.

Can you elaborate a bit more please on the wrongness of the comment?

I can change the comment to say "common clock" instead of "clock" if
that's the only problem here.

 thanks,

-Joel

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

* Re: [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error
  2014-05-07 15:24   ` Tony Lindgren
@ 2014-05-07 21:52     ` Joel Fernandes
  2014-05-07 22:10       ` Tony Lindgren
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 21:52 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 10:24 AM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>> The subsequent devm_ioremap_resource will catch it and print an error, let it
>> be checked there.
>>
>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>> ---
>>  arch/arm/plat-omap/dmtimer.c |    4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>> index 7e806f9..1fd30fa 100644
>> --- a/arch/arm/plat-omap/dmtimer.c
>> +++ b/arch/arm/plat-omap/dmtimer.c
>> @@ -810,10 +810,6 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	if (unlikely(!mem)) {
>> -		dev_err(dev, "%s: no memory resource.\n", __func__);
>> -		return -ENODEV;
>> -	}
>>  
>>  	timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
>>  	if (!timer) {
> 
> We still need to return an error here and not try to continue though.

We are returning an error if mem is NULL so the redundant check is
unnecessary:

...
        timer->io_base = devm_ioremap_resource(dev, mem);
        if (IS_ERR(timer->io_base))
                return PTR_ERR(timer->io_base);


thanks,

-Joel


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

* Re: [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running
  2014-05-07 15:25   ` Tony Lindgren
@ 2014-05-07 22:00     ` Joel Fernandes
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 22:00 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 10:25 AM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>> Inorder to move non-DM timer specific code that modifies the "idlect"
>> mask on OMAP1, from dmtimer code, to OMAP1 specific timer initialization code,
>> we introduce a new function that can possibly be reused for other purposes in
>> the future. The function just checks if a timer is running based on the timer ID
>> which should be same as pdev->id. This allows us to cleanly separate the timer vs
>> non-timer bits and keep the timer bits in the dmtimer code.
>>
>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>> ---
>>  arch/arm/plat-omap/dmtimer.c              |   29 +++++++++++++++++++++++++++++
>>  arch/arm/plat-omap/include/plat/dmtimer.h |    2 ++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>> index 4debb3d..86b2641 100644
>> --- a/arch/arm/plat-omap/dmtimer.c
>> +++ b/arch/arm/plat-omap/dmtimer.c
>> @@ -187,6 +187,35 @@ int omap_dm_timer_reserve_systimer(int id)
>>  	return 0;
>>  }
>>  
>> +/*
>> + * Check if a timer is running based on timer_id, used for OMAP1 currently.
>> + */
>> +int omap_dm_timer_is_running(int timer_id)
>> +{
>> +	int i = 1, ret = 0;
>> +	struct omap_dm_timer *timer = NULL;
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&dm_timer_lock, flags);
>> +	list_for_each_entry(timer, &omap_timer_list, node) {
>> +		if (i == timer_id) {
>> +			u32 l;
>> +			l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
>> +			if (l & OMAP_TIMER_CTRL_ST) {
>> +				ret = 1;
>> +				goto done;
>> +			} else {
>> +				goto done;
>> +			}
>> +		}
>> +		i++;
>> +	}
>> +done:
>> +	spin_unlock_irqrestore(&dm_timer_lock, flags);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(omap_dm_timer_is_running);
> 
> Let's not add new exported custom functions, let's instead try to get
> rid of them. What needs to use this one?

This piece of code was entangled with OMAP1 specific logic.

The OMAP1 specific logic is moved into a new function in the OMAP1 timer
layer called: omap_dm_timer_modify_idlect_mask

What's left is code that needs to iterate over all the timers by
checking the OMAP_TIMER_CTRL_ST bit in the control register. This is
left back into the dmtimer code but needs to be exported since we
ultimately will move dmtimer into clock source.

If you notice, I removed export of omap_dm_timer_modify_idlect_mask
since it is moved to omap1 layer and is local there, so it should be OK
to export this symbol instead. We are not adding more exports, and its
still an improvement over the old code as the OMAP1 specific logic is
decoupled from the generic dmtimer code.

thanks,

-Joel


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

* Re: [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook
  2014-05-07 21:43     ` Joel Fernandes
@ 2014-05-07 22:04       ` Tony Lindgren
  2014-05-07 22:08         ` Joel Fernandes
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 22:04 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140507 14:44]:
> On 05/07/2014 10:19 AM, Tony Lindgren wrote:
> > * Joel Fernandes <joelf@ti.com> [140424 14:44]:
> >> There is a platform specific hook just for OMAP1 to set its clk parent.  Remove
> >> this hook and have OMAP1 set its parent in omap1_dm_timer_init.  If OMAP1 is
> >> ever migrated to clock framework, the correct way to do this would be through
> >> clk_set_parent like other platforms.
> >>
> >> Signed-off-by: Joel Fernandes <joelf@ti.com>
> >> ---
> >>  arch/arm/mach-omap1/timer.c                |    8 +++++++-
> >>  arch/arm/plat-omap/dmtimer.c               |    8 +++-----
> >>  include/linux/platform_data/dmtimer-omap.h |    2 --
> >>  3 files changed, 10 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
> >> index bde7a35..4b9c604 100644
> >> --- a/arch/arm/mach-omap1/timer.c
> >> +++ b/arch/arm/mach-omap1/timer.c
> >> @@ -140,7 +140,13 @@ static int __init omap1_dm_timer_init(void)
> >>  			goto err_free_pdata;
> >>  		}
> >>  
> >> -		pdata->set_timer_src = omap1_dm_timer_set_src;
> >> +		/*
> >> +		 * Since OMAP1 doesn't support clock framework, set timer clock
> >> +		 * source to 32KHz here instead of expecting it to be set by
> >> +		 * dmtimer code.
> >> +		 */
> >> +		omap1_dm_timer_set_src(pdev, 0x01);
> >> +
> >>  		pdata->timer_capability = OMAP_TIMER_ALWON |
> >>  				OMAP_TIMER_NEEDS_RESET | OMAP_TIMER_HAS_DSP_IRQ;
> >>  
> > 
> > This does not sound right, omap1 does support clock framework just fine.
> > It is not using the common clock framework though.
> > 
> > This breaks omap_dm_timer_set_source() for sure. Setting the source
> > during init is not a right solution here. Probably best to keep the
> > pdata hook around, drivers can support pdata and DT data together
> > just fine.
> 
> Actually pdata hook in this case is used only in 1 place, and is OMAP1
> specific so I felt its better to clean it up.
> 
> Can you elaborate a bit more on why it breaks? I have no way of testing
> OMAP1 without hardware.
> 
> What difference is it to set MOD_CONF_CTRL_1 in omap1's
> arch_initcall(omap1_dm_timer_init)  versus doing so later?

You're making a driver behave in a different way for omap1
compared to omap2+ where selecting the clock source won't work
for omap1.

Regards,

Tony

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

* Re: [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework
  2014-05-07 21:48     ` Joel Fernandes
@ 2014-05-07 22:07       ` Tony Lindgren
  0 siblings, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 22:07 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140507 14:49]:
> On 05/07/2014 10:20 AM, Tony Lindgren wrote:
> > * Joel Fernandes <joelf@ti.com> [140424 14:44]:
> >> OMAP1 doesn't support clock framework, add a comment where needed
> >> and correct a FIXME.
> > 
> > This is at least a wrong comment, the original comment seems right
> > to me.
> 
> Can you elaborate a bit more please on the wrongness of the comment?
> 
> I can change the comment to say "common clock" instead of "clock" if
> that's the only problem here.

The original comment is just fine, no need to patch it. We do have
clock framework support for omap1, it was the first ever Linux
clock framework that pretty much all other clock frameworks are
based on. Just drop this patch it's misleading.

Regards,

Tony

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

* Re: [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook
  2014-05-07 22:04       ` Tony Lindgren
@ 2014-05-07 22:08         ` Joel Fernandes
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 22:08 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 05:04 PM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140507 14:44]:
>> On 05/07/2014 10:19 AM, Tony Lindgren wrote:
>>> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>>>> There is a platform specific hook just for OMAP1 to set its clk parent.  Remove
>>>> this hook and have OMAP1 set its parent in omap1_dm_timer_init.  If OMAP1 is
>>>> ever migrated to clock framework, the correct way to do this would be through
>>>> clk_set_parent like other platforms.
>>>>
>>>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>>>> ---
>>>>  arch/arm/mach-omap1/timer.c                |    8 +++++++-
>>>>  arch/arm/plat-omap/dmtimer.c               |    8 +++-----
>>>>  include/linux/platform_data/dmtimer-omap.h |    2 --
>>>>  3 files changed, 10 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
>>>> index bde7a35..4b9c604 100644
>>>> --- a/arch/arm/mach-omap1/timer.c
>>>> +++ b/arch/arm/mach-omap1/timer.c
>>>> @@ -140,7 +140,13 @@ static int __init omap1_dm_timer_init(void)
>>>>  			goto err_free_pdata;
>>>>  		}
>>>>  
>>>> -		pdata->set_timer_src = omap1_dm_timer_set_src;
>>>> +		/*
>>>> +		 * Since OMAP1 doesn't support clock framework, set timer clock
>>>> +		 * source to 32KHz here instead of expecting it to be set by
>>>> +		 * dmtimer code.
>>>> +		 */
>>>> +		omap1_dm_timer_set_src(pdev, 0x01);
>>>> +
>>>>  		pdata->timer_capability = OMAP_TIMER_ALWON |
>>>>  				OMAP_TIMER_NEEDS_RESET | OMAP_TIMER_HAS_DSP_IRQ;
>>>>  
>>>
>>> This does not sound right, omap1 does support clock framework just fine.
>>> It is not using the common clock framework though.
>>>
>>> This breaks omap_dm_timer_set_source() for sure. Setting the source
>>> during init is not a right solution here. Probably best to keep the
>>> pdata hook around, drivers can support pdata and DT data together
>>> just fine.
>>
>> Actually pdata hook in this case is used only in 1 place, and is OMAP1
>> specific so I felt its better to clean it up.
>>
>> Can you elaborate a bit more on why it breaks? I have no way of testing
>> OMAP1 without hardware.
>>
>> What difference is it to set MOD_CONF_CTRL_1 in omap1's
>> arch_initcall(omap1_dm_timer_init)  versus doing so later?
> 
> You're making a driver behave in a different way for omap1
> compared to omap2+ where selecting the clock source won't work
> for omap1.

Ok, I'll drop this patch and leave the hook as-is. Thanks.

Regards,
-Joel


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

* Re: [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error
  2014-05-07 21:52     ` Joel Fernandes
@ 2014-05-07 22:10       ` Tony Lindgren
  2014-05-07 22:14         ` Joel Fernandes
  0 siblings, 1 reply; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 22:10 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140507 14:53]:
> On 05/07/2014 10:24 AM, Tony Lindgren wrote:
> > * Joel Fernandes <joelf@ti.com> [140424 14:44]:
> >> The subsequent devm_ioremap_resource will catch it and print an error, let it
> >> be checked there.
> >>
> >> Signed-off-by: Joel Fernandes <joelf@ti.com>
> >> ---
> >>  arch/arm/plat-omap/dmtimer.c |    4 ----
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> >> index 7e806f9..1fd30fa 100644
> >> --- a/arch/arm/plat-omap/dmtimer.c
> >> +++ b/arch/arm/plat-omap/dmtimer.c
> >> @@ -810,10 +810,6 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
> >>  	}
> >>  
> >>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> -	if (unlikely(!mem)) {
> >> -		dev_err(dev, "%s: no memory resource.\n", __func__);
> >> -		return -ENODEV;
> >> -	}
> >>  
> >>  	timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
> >>  	if (!timer) {
> > 
> > We still need to return an error here and not try to continue though.
> 
> We are returning an error if mem is NULL so the redundant check is
> unnecessary:
> 
> ...
>         timer->io_base = devm_ioremap_resource(dev, mem);
>         if (IS_ERR(timer->io_base))
>                 return PTR_ERR(timer->io_base);

Why would you want to even continue omap_dm_timer_probe()
further and allocate memory if platform_get_resource() fails?

Tony

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

* Re: [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework
  2014-05-07 15:20   ` Tony Lindgren
  2014-05-07 21:48     ` Joel Fernandes
@ 2014-05-07 22:10     ` Joel Fernandes
  1 sibling, 0 replies; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 22:10 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 10:20 AM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>> OMAP1 doesn't support clock framework, add a comment where needed
>> and correct a FIXME.
> 
> This is at least a wrong comment, the original comment seems right
> to me.

Ok, I've no issues dropping this patch.

-Joel


> 
> Tony
>  
>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>> ---
>>  arch/arm/plat-omap/dmtimer.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>> index ecd3f97..f5a674c 100644
>> --- a/arch/arm/plat-omap/dmtimer.c
>> +++ b/arch/arm/plat-omap/dmtimer.c
>> @@ -142,8 +142,7 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)
>>  	int rc;
>>  
>>  	/*
>> -	 * FIXME: OMAP1 devices do not use the clock framework for dmtimers so
>> -	 * do not call clk_get() for these devices.
>> +	 * Do not call clk_get() for OMAP1 due to no clock framework support.
>>  	 */
>>  	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET)) {
>>  		timer->fclk = clk_get(&timer->pdev->dev, "fck");
>> @@ -461,6 +460,7 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
>>  	if (unlikely(!timer))
>>  		return -EINVAL;
>>  
>> +	/* OMAP1 is not converted to clk framework so avoid clk_get_rate here */
>>  	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET))
>>  		rate = clk_get_rate(timer->fclk);
>>  
>> -- 
>> 1.7.9.5
>>


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

* Re: [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error
  2014-05-07 22:10       ` Tony Lindgren
@ 2014-05-07 22:14         ` Joel Fernandes
  2014-05-07 22:22           ` Tony Lindgren
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Fernandes @ 2014-05-07 22:14 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

On 05/07/2014 05:10 PM, Tony Lindgren wrote:
> * Joel Fernandes <joelf@ti.com> [140507 14:53]:
>> On 05/07/2014 10:24 AM, Tony Lindgren wrote:
>>> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
>>>> The subsequent devm_ioremap_resource will catch it and print an error, let it
>>>> be checked there.
>>>>
>>>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>>>> ---
>>>>  arch/arm/plat-omap/dmtimer.c |    4 ----
>>>>  1 file changed, 4 deletions(-)
>>>>
>>>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>>>> index 7e806f9..1fd30fa 100644
>>>> --- a/arch/arm/plat-omap/dmtimer.c
>>>> +++ b/arch/arm/plat-omap/dmtimer.c
>>>> @@ -810,10 +810,6 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>>>>  	}
>>>>  
>>>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> -	if (unlikely(!mem)) {
>>>> -		dev_err(dev, "%s: no memory resource.\n", __func__);
>>>> -		return -ENODEV;
>>>> -	}
>>>>  
>>>>  	timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
>>>>  	if (!timer) {
>>>
>>> We still need to return an error here and not try to continue though.
>>
>> We are returning an error if mem is NULL so the redundant check is
>> unnecessary:
>>
>> ...
>>         timer->io_base = devm_ioremap_resource(dev, mem);
>>         if (IS_ERR(timer->io_base))
>>                 return PTR_ERR(timer->io_base);
> 
> Why would you want to even continue omap_dm_timer_probe()
> further and allocate memory if platform_get_resource() fails?
> 

But its freed anyway on error. I just felt that extra LOC could be
removed. Ideally we should do something like


mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
io_base = devm_ioremap_resource(dev, mem);
if (IS_ERR(io_base))
	return PTR_ERR(io_base);

timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
if (!timer) {
 ...
}
timer->io_base = io_base;


That combines the platform_get_resource and devm_ioremap_resource error
paths into 1 path and avoids redundant checks.. I can do it this way, or
if you want drop the patch entirely, I'm OK with it both ways..

thanks,

-Joel


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

* Re: [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error
  2014-05-07 22:14         ` Joel Fernandes
@ 2014-05-07 22:22           ` Tony Lindgren
  0 siblings, 0 replies; 42+ messages in thread
From: Tony Lindgren @ 2014-05-07 22:22 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux OMAP List, Linux ARM Kernel List, Linux Kernel Mailing List

* Joel Fernandes <joelf@ti.com> [140507 15:15]:
> On 05/07/2014 05:10 PM, Tony Lindgren wrote:
> > * Joel Fernandes <joelf@ti.com> [140507 14:53]:
> >> On 05/07/2014 10:24 AM, Tony Lindgren wrote:
> >>> * Joel Fernandes <joelf@ti.com> [140424 14:44]:
> >>>> The subsequent devm_ioremap_resource will catch it and print an error, let it
> >>>> be checked there.
> >>>>
> >>>> Signed-off-by: Joel Fernandes <joelf@ti.com>
> >>>> ---
> >>>>  arch/arm/plat-omap/dmtimer.c |    4 ----
> >>>>  1 file changed, 4 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> >>>> index 7e806f9..1fd30fa 100644
> >>>> --- a/arch/arm/plat-omap/dmtimer.c
> >>>> +++ b/arch/arm/plat-omap/dmtimer.c
> >>>> @@ -810,10 +810,6 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
> >>>>  	}
> >>>>  
> >>>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>>> -	if (unlikely(!mem)) {
> >>>> -		dev_err(dev, "%s: no memory resource.\n", __func__);
> >>>> -		return -ENODEV;
> >>>> -	}
> >>>>  
> >>>>  	timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
> >>>>  	if (!timer) {
> >>>
> >>> We still need to return an error here and not try to continue though.
> >>
> >> We are returning an error if mem is NULL so the redundant check is
> >> unnecessary:
> >>
> >> ...
> >>         timer->io_base = devm_ioremap_resource(dev, mem);
> >>         if (IS_ERR(timer->io_base))
> >>                 return PTR_ERR(timer->io_base);
> > 
> > Why would you want to even continue omap_dm_timer_probe()
> > further and allocate memory if platform_get_resource() fails?
> > 
> 
> But its freed anyway on error. I just felt that extra LOC could be
> removed. Ideally we should do something like
> 
> 
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> io_base = devm_ioremap_resource(dev, mem);
> if (IS_ERR(io_base))
> 	return PTR_ERR(io_base);
> 
> timer = devm_kzalloc(dev, sizeof(struct omap_dm_timer), GFP_KERNEL);
> if (!timer) {
>  ...
> }
> timer->io_base = io_base;
> 
> 
> That combines the platform_get_resource and devm_ioremap_resource error
> paths into 1 path and avoids redundant checks.. I can do it this way, or
> if you want drop the patch entirely, I'm OK with it both ways..

Just drop the dev_err if anything. Removing error checking from
function calls is always going to cause people to wonder what's
going on.

Tony

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

end of thread, other threads:[~2014-05-07 22:22 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-24 21:43 [PATCH 00/26] OMAP dmtimer prep series Joel Fernandes
2014-04-24 21:43 ` [PATCH 01/26] ARM: OMAP: dmtimer: Remove setting of clk parent indirectly through platform hook Joel Fernandes
2014-05-07 15:19   ` Tony Lindgren
2014-05-07 21:43     ` Joel Fernandes
2014-05-07 22:04       ` Tony Lindgren
2014-05-07 22:08         ` Joel Fernandes
2014-04-24 21:43 ` [PATCH 02/26] ARM: OMAP: dmtimer: Add comments on OMAP1 clock framework Joel Fernandes
2014-05-07 15:20   ` Tony Lindgren
2014-05-07 21:48     ` Joel Fernandes
2014-05-07 22:07       ` Tony Lindgren
2014-05-07 22:10     ` Joel Fernandes
2014-04-24 21:43 ` [PATCH 03/26] ARM: OMAP: dmtimer: Add note to set parent from DT Joel Fernandes
2014-04-24 21:43 ` [PATCH 04/26] ARM: OMAP: dmtimer: Add function to check if timer is running Joel Fernandes
2014-05-07 15:25   ` Tony Lindgren
2014-05-07 22:00     ` Joel Fernandes
2014-04-24 21:43 ` [PATCH 05/26] ARM: OMAP1: dmtimer: Rewrite modify of IDLECT mask to use new is_running function Joel Fernandes
2014-04-24 21:43 ` [PATCH 06/26] ARM: OMAP: dmtimer: Add a write_ctrl function to simplify bit setting Joel Fernandes
2014-04-24 21:43 ` [PATCH 07/26] ARM: OMAP: dmtimer: Have __omap_dm_timer_load_start set ST bit in CTRL instead of caller Joel Fernandes
2014-04-24 21:43 ` [PATCH 08/26] ARM: OMAP: dmtimer: Add function to check for timer availability Joel Fernandes
2014-04-24 21:43 ` [PATCH 09/26] ARM: OMAP: dmtimer: Get rid of check for mem resource error Joel Fernandes
2014-05-07 15:24   ` Tony Lindgren
2014-05-07 21:52     ` Joel Fernandes
2014-05-07 22:10       ` Tony Lindgren
2014-05-07 22:14         ` Joel Fernandes
2014-05-07 22:22           ` Tony Lindgren
2014-04-24 21:43 ` [PATCH 10/26] ARM: OMAP: dmtimer: Check return of pm_runtime_get_sync Joel Fernandes
2014-04-24 21:43 ` [PATCH 11/26] ARM: OMAP2+: timer: Add a powerup function Joel Fernandes
2014-04-24 21:43 ` [PATCH 12/26] ARM: OMAP2+: timer: Simplify clock event/source name setting Joel Fernandes
2014-04-24 21:43 ` [PATCH 13/26] ARM: OMAP2+: timer: Add comment on timer clk parenting Joel Fernandes
2014-04-24 21:43 ` [PATCH 14/26] ARM: OMAP2+: timer: Remove hwmod look-up dependency for DT-boot Joel Fernandes
2014-04-24 21:43 ` [PATCH 15/26] ARM: OMAP2+: timer: Use of_clk_get for DT platforms Joel Fernandes
2014-04-24 21:43 ` [PATCH 16/26] ARM: OMAP2+: timer: Fix error message to not use hwmod structure Joel Fernandes
2014-04-24 21:44 ` [PATCH 17/26] ARM: OMAP2+: timer: Add fallback for of_clk_get Joel Fernandes
2014-04-24 21:44 ` [PATCH 18/26] ARM: OMAP2+: timer: Add legacy code for old way of getting fclk Joel Fernandes
2014-04-24 21:44 ` [PATCH 19/26] ARM: OMAP: dmtimer: Remove API __omap_dm_timer_load_start Joel Fernandes
2014-04-24 21:44 ` [PATCH 20/26] ARM: OMAP: dmtimer: Fold back private stop function Joel Fernandes
2014-04-24 21:44 ` [PATCH 21/26] ARM: OMAP: dmtimer: Add systimer flag to dmtimer structure Joel Fernandes
2014-04-24 21:44 ` [PATCH 22/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_write_status function Joel Fernandes
2014-04-24 21:44 ` [PATCH 23/26] ARM: OMAP: dmtimer: Eliminate __omap_dm_timer_read_counter function Joel Fernandes
2014-04-24 21:44 ` [PATCH 24/26] ARM: OMAP: dmtimer: Move private functions into dmtimer core and export others Joel Fernandes
2014-04-24 21:44 ` [PATCH 25/26] ARM: OMAP: dmtimer: Eliminate omap_dm_timer_int_enable function Joel Fernandes
2014-04-24 21:44 ` [PATCH 26/26] ARM: OMAP: dmtimer: Use is_timer_available function in omap_dm_timer_trigger Joel Fernandes

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).