Skip to contentSkip to conversation

Omap fixes for 2.6.32-rc2

PATCH In review Tony Lindgren 3 participants 11 messages Oct 2, 2009 – Oct 6, 2009

Subsystems (5)
lore
Share & tools
lore

Tony Lindgren

author maintainer
#0 lore raw

cover Omap fixes for 2.6.32-rc2

Hi all,

Here are some more fixes for review.

Regards,

Tony

---

Daniel Walker (1):
omap: iovmm: Add missing mutex_unlock

Hiroshi DOYU (1):
omap: iovmm: Fix incorrect spelling

Rajendra Nayak (1):
omap: Lock DPLL5 at boot

Sergio Aguirre (1):
omapfb: Condition mutex acquisition

Tommi Rantala (2):
omapfb: Blizzard: constify register address tables
omapfb: Blizzard: fix pointer to be const

Tony Lindgren (1):
omap: Fix incorrect 730 vs 850 detection

ye janboe (1):
omap: SRAM: flush the right address after memcpy in omap_sram_push

 arch/arm/mach-omap2/clock34xx.c       |   35 +++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/mach/cpu.h |   37 ++++++++++++---------------------
 arch/arm/plat-omap/iovmm.c            |    9 ++++----
 arch/arm/plat-omap/sram.c             |    3 ++-
 drivers/video/omap/blizzard.c         |   10 ++++-----
 drivers/video/omap/omapfb_main.c      |   22 ++++++++++++--------
 6 files changed, 75 insertions(+), 41 deletions(-)
Signature · 1 line
Signature
author maintainer
#1 lore raw

PATCH 1/8 omap: Fix incorrect 730 vs 850 detection

Commit cd92204924fafbd5c7241dfd12ca3176d542e0c5 added support for omap850. However, the patch accidentally removed the wrong ifdef:

# define cpu_is_omap730() 1

 # endif
 #endif
+#else
+# if defined(CONFIG_ARCH_OMAP850)
+#  undef  cpu_is_omap850
+#  define cpu_is_omap850()		1
+# endif
+#endif

...

void omap2_check_revision(void);

#endif /* defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) */
-
-#endif

Instead of removing removing the #endif at the end of the file, the #endif before #else should have been removed.

But we cannot have multiple #else statements as pointed out by Alistair Buxton <[email protected]>. So the fix is to:

- remove the non-multi-omap special handling, as we need to detect between omap730 and omap850 anyways.

- add the missing #endif back to the end of the file

---
 arch/arm/plat-omap/include/mach/cpu.h |   37 ++++++++++++---------------------
 1 files changed, 14 insertions(+), 23 deletions(-)
arch/arm/plat-omap/include/mach/cpu.h +14 -23
index 11e73d9..f129efb 100644
@@ -303,32 +303,21 @@ IS_OMAP_TYPE(3430, 0x3430)
 #define cpu_is_omap2430()		0
 #define cpu_is_omap3430()		0
 
-#if defined(MULTI_OMAP1)
-# if defined(CONFIG_ARCH_OMAP730)
-#  undef  cpu_is_omap730
-#  define cpu_is_omap730()		is_omap730()
-# endif
-# if defined(CONFIG_ARCH_OMAP850)
-#  undef  cpu_is_omap850
-#  define cpu_is_omap850()		is_omap850()
-# endif
-#else
-# if defined(CONFIG_ARCH_OMAP730)
-#  undef  cpu_is_omap730
-#  define cpu_is_omap730()		1
-# endif
-#endif
-#else
-# if defined(CONFIG_ARCH_OMAP850)
-#  undef  cpu_is_omap850
-#  define cpu_is_omap850()		1
-# endif
-#endif
-
 /*
  * Whether we have MULTI_OMAP1 or not, we still need to distinguish
- * between 330 vs. 1510 and 1611B/5912 vs. 1710.
+ * between 730 vs 850, 330 vs. 1510 and 1611B/5912 vs. 1710.
  */
+
+#if defined(CONFIG_ARCH_OMAP730)
+# undef  cpu_is_omap730
+# define cpu_is_omap730()		is_omap730()
+#endif
+
+#if defined(CONFIG_ARCH_OMAP850)
+# undef  cpu_is_omap850
+# define cpu_is_omap850()		is_omap850()
+#endif
+
 #if defined(CONFIG_ARCH_OMAP15XX)
 # undef  cpu_is_omap310
 # undef  cpu_is_omap1510
@@ -433,3 +422,5 @@ IS_OMAP_TYPE(3430, 0x3430)
 
 int omap_chip_is(struct omap_chip_id oci);
 void omap2_check_revision(void);
+
+#endif
author maintainer
#2 lore raw

PATCH 2/8 omap: Lock DPLL5 at boot

In reply to Tony Lindgren

From: Rajendra Nayak <[email protected]>

Lock DPLL5 at 120MHz at boot. The USBHOST 120MHz f-clock and USBTLL f-clock are the only users of this DPLL, and 120MHz is is the only recommended rate for these clocks.

With this patch, the 60 MHz ULPI clock is generated correctly.

Tested on an OMAP3430 SDP.

---
 arch/arm/mach-omap2/clock34xx.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock34xx.c +35 -0
index fafcd32..489556e 100644
@@ -338,6 +338,13 @@ static struct omap_clk omap34xx_clks[] = {
  */
 #define SDRC_MPURATE_LOOPS		96
 
+/*
+ * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
+ * that are sourced by DPLL5, and both of these require this clock
+ * to be at 120 MHz for proper operation.
+ */
+#define DPLL5_FREQ_FOR_USBHOST		120000000
+
 /**
  * omap3430es2_clk_ssi_find_idlest - return CM_IDLEST info for SSI
  * @clk: struct clk * being enabled
@@ -1056,6 +1063,28 @@ void omap2_clk_prepare_for_reboot(void)
 #endif
 }
 
+static void omap3_clk_lock_dpll5(void)
+{
+	struct clk *dpll5_clk;
+	struct clk *dpll5_m2_clk;
+
+	dpll5_clk = clk_get(NULL, "dpll5_ck");
+	clk_set_rate(dpll5_clk, DPLL5_FREQ_FOR_USBHOST);
+	clk_enable(dpll5_clk);
+
+	/* Enable autoidle to allow it to enter low power bypass */
+	omap3_dpll_allow_idle(dpll5_clk);
+
+	/* Program dpll5_m2_clk divider for no division */
+	dpll5_m2_clk = clk_get(NULL, "dpll5_m2_ck");
+	clk_enable(dpll5_m2_clk);
+	clk_set_rate(dpll5_m2_clk, DPLL5_FREQ_FOR_USBHOST);
+
+	clk_disable(dpll5_m2_clk);
+	clk_disable(dpll5_clk);
+	return;
+}
+
 /* REVISIT: Move this init stuff out into clock.c */
 
 /*
@@ -1148,6 +1177,12 @@ int __init omap2_clk_init(void)
 	 */
 	clk_enable_init_clocks();
 
+	/*
+	 * Lock DPLL5 and put it in autoidle.
+	 */
+	if (omap_rev() >= OMAP3430_REV_ES2_0)
+		omap3_clk_lock_dpll5();
+
 	/* Avoid sleeping during omap2_clk_prepare_for_reboot() */
 	/* REVISIT: not yet ready for 343x */
 #if 0
author maintainer
#4 lore raw

PATCH 3/8 omap: SRAM: flush the right address after memcpy in omap_sram_push

In reply to Tony Lindgren

From: ye janboe <[email protected]>

the original flush operation is to flush the function address which is copied from.
But we do not change the function code and it is not necessary to flush it.

---
 arch/arm/plat-omap/sram.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/arm/plat-omap/sram.c +2 -1
index 925f647..75d1f26 100644
@@ -270,7 +270,8 @@ void * omap_sram_push(void * start, unsigned long size)
 	omap_sram_ceil -= size;
 	omap_sram_ceil = ROUND_DOWN(omap_sram_ceil, sizeof(void *));
 	memcpy((void *)omap_sram_ceil, start, size);
-	flush_icache_range((unsigned long)start, (unsigned long)(start + size));
+	flush_icache_range((unsigned long)omap_sram_ceil,
+		(unsigned long)(omap_sram_ceil + size));
 
 	return (void *)omap_sram_ceil;
 }
author maintainer
#5 lore raw

PATCH 4/8 omap: iovmm: Fix incorrect spelling

In reply to Tony Lindgren

From: Hiroshi DOYU <[email protected]>

Fix incorrect spelling

---
 arch/arm/plat-omap/iovmm.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/plat-omap/iovmm.c +3 -3
index 57f7122..0e5573d 100644
@@ -47,7 +47,7 @@
  *	'va':	mpu virtual address
  *
  *	'c':	contiguous memory area
- *	'd':	dicontiguous memory area
+ *	'd':	discontiguous memory area
  *	'a':	anonymous memory allocation
  *	'()':	optional feature
  *
@@ -398,7 +398,7 @@ static inline void sgtable_drain_vmalloc(struct sg_table *sgt)
 {
 	/*
 	 * Actually this is not necessary at all, just exists for
-	 * consistency of the code readibility.
+	 * consistency of the code readability.
 	 */
 	BUG_ON(!sgt);
 }
@@ -434,7 +434,7 @@ static inline void sgtable_drain_kmalloc(struct sg_table *sgt)
 {
 	/*
 	 * Actually this is not necessary at all, just exists for
-	 * consistency of the code readibility
+	 * consistency of the code readability
 	 */
 	BUG_ON(!sgt);
 }
author maintainer
#6 lore raw

PATCH 5/8 omap: iovmm: Add missing mutex_unlock

In reply to Tony Lindgren

From: Daniel Walker <[email protected]>

I was using Coccinelle with the mutex_unlock semantic patch, and it unconvered this problem. It appears to be a valid missing unlock issue. This change should correct it by moving the unlock below the label.

This patch is against the mainline kernel.

---
 arch/arm/plat-omap/iovmm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/arm/plat-omap/iovmm.c +2 -1
index 0e5573d..dc3fac3 100644
@@ -363,8 +363,9 @@ void *da_to_va(struct iommu *obj, u32 da)
 		goto out;
 	}
 	va = area->va;
-	mutex_unlock(&obj->mmap_lock);
 out:
+	mutex_unlock(&obj->mmap_lock);
+
 	return va;
 }
 EXPORT_SYMBOL_GPL(da_to_va);
author maintainer
#7 lore raw

PATCH 6/8 omapfb: Condition mutex acquisition

In reply to Tony Lindgren

From: Sergio Aguirre <[email protected]>

This fixes a bug introduced by this commit ID:

commit 537a1bf059fa312355696fa6db80726e655e7f17
Author: Krzysztof Helt <[email protected]>
Date: Tue Jun 30 11:41:29 2009 -0700

fbdev: add mutex for fb_mmap locking

In which a mutex was added when changing smem_start and smem_len fields, so the mutex inside the fb_mmap() call is actually used.

The problem was that set_fb_fix, which modifies the above 2 fields, was called before and after registering the framebuffer, which when used before registration, lead to a failed attempt to use an uninitialized mutex.

Solution: Don't use mutex before framebuffer registration.

---
 drivers/video/omap/omapfb_main.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)
drivers/video/omap/omapfb_main.c +14 -8
index 125e605..0d0c8c8 100644
@@ -393,7 +393,7 @@ static void omapfb_sync(struct fb_info *fbi)
  * Set fb_info.fix fields and also updates fbdev.
  * When calling this fb_info.var must be set up already.
  */
-static void set_fb_fix(struct fb_info *fbi)
+static void set_fb_fix(struct fb_info *fbi, int from_init)
 {
 	struct fb_fix_screeninfo *fix = &fbi->fix;
 	struct fb_var_screeninfo *var = &fbi->var;
@@ -403,10 +403,16 @@ static void set_fb_fix(struct fb_info *fbi)
 
 	rg = &plane->fbdev->mem_desc.region[plane->idx];
 	fbi->screen_base	= rg->vaddr;
-	mutex_lock(&fbi->mm_lock);
-	fix->smem_start		= rg->paddr;
-	fix->smem_len		= rg->size;
-	mutex_unlock(&fbi->mm_lock);
+
+	if (!from_init) {
+		mutex_lock(&fbi->mm_lock);
+		fix->smem_start		= rg->paddr;
+		fix->smem_len		= rg->size;
+		mutex_unlock(&fbi->mm_lock);
+	} else {
+		fix->smem_start		= rg->paddr;
+		fix->smem_len		= rg->size;
+	}
 
 	fix->type = FB_TYPE_PACKED_PIXELS;
 	bpp = var->bits_per_pixel;
@@ -704,7 +710,7 @@ static int omapfb_set_par(struct fb_info *fbi)
 	int r = 0;
 
 	omapfb_rqueue_lock(fbdev);
-	set_fb_fix(fbi);
+	set_fb_fix(fbi, 0);
 	r = ctrl_change_mode(fbi);
 	omapfb_rqueue_unlock(fbdev);
 
@@ -904,7 +910,7 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 		if (old_size != size) {
 			if (size) {
 				memcpy(&fbi->var, new_var, sizeof(fbi->var));
-				set_fb_fix(fbi);
+				set_fb_fix(fbi, 0);
 			} else {
 				/*
 				 * Set these explicitly to indicate that the
@@ -1504,7 +1510,7 @@ static int fbinfo_init(struct omapfb_device *fbdev, struct fb_info *info)
 	var->bits_per_pixel = fbdev->panel->bpp;
 
 	set_fb_var(info, var);
-	set_fb_fix(info);
+	set_fb_fix(info, 1);
 
 	r = fb_alloc_cmap(&info->cmap, 16, 0);
 	if (r != 0)
author maintainer
#9 lore raw

PATCH 7/8 omapfb: Blizzard: fix pointer to be const

In reply to Tony Lindgren

From: Tommi Rantala <[email protected]>

Fixes a compiler warning:
warning: assignment discards qualifiers from pointer target type

---
 drivers/video/omap/blizzard.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/video/omap/blizzard.c +1 -1
index d5e5955..ce28be6 100644
@@ -191,7 +191,7 @@ struct blizzard_struct {
 
 	struct omapfb_device	*fbdev;
 	struct lcd_ctrl_extif	*extif;
-	struct lcd_ctrl		*int_ctrl;
+	const struct lcd_ctrl	*int_ctrl;
 
 	void			(*power_up)(struct device *dev);
 	void			(*power_down)(struct device *dev);
author maintainer
#10 lore raw

PATCH 8/8 omapfb: Blizzard: constify register address tables

In reply to Tony Lindgren

From: Tommi Rantala <[email protected]>

Constify register address tables

---
 drivers/video/omap/blizzard.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/video/omap/blizzard.c +4 -4
index ce28be6..70dadf9 100644
@@ -93,7 +93,7 @@ struct blizzard_reg_list {
 };
 
 /* These need to be saved / restored separately from the rest. */
-static struct blizzard_reg_list blizzard_pll_regs[] = {
+static const struct blizzard_reg_list blizzard_pll_regs[] = {
 	{
 		.start	= 0x04,		/* Don't save PLL ctrl (0x0C) */
 		.end	= 0x0a,
@@ -104,7 +104,7 @@ static struct blizzard_reg_list blizzard_pll_regs[] = {
 	},
 };
 
-static struct blizzard_reg_list blizzard_gen_regs[] = {
+static const struct blizzard_reg_list blizzard_gen_regs[] = {
 	{
 		.start	= 0x18,		/* SDRAM control */
 		.end	= 0x20,
@@ -1372,7 +1372,7 @@ static void blizzard_get_caps(int plane, struct omapfb_caps *caps)
 			   (1 << OMAPFB_COLOR_YUV420);
 }
 
-static void _save_regs(struct blizzard_reg_list *list, int cnt)
+static void _save_regs(const struct blizzard_reg_list *list, int cnt)
 {
 	int i;
 
@@ -1383,7 +1383,7 @@ static void _save_regs(struct blizzard_reg_list *list, int cnt)
 	}
 }
 
-static void _restore_regs(struct blizzard_reg_list *list, int cnt)
+static void _restore_regs(const struct blizzard_reg_list *list, int cnt)
 {
 	int i;
 
Tested-by on v1 6/8
#8 lore raw

Re: [PATCH 6/8] omapfb: Condition mutex acquisition

In reply to Tony Lindgren
Quoted 92 lines · Tony Lindgren

From: Sergio Aguirre <[email protected]>

This fixes a bug introduced by this commit ID:

?commit 537a1bf059fa312355696fa6db80726e655e7f17
?Author: Krzysztof Helt <[email protected]>
?Date: ? Tue Jun 30 11:41:29 2009 -0700

? ?fbdev: add mutex for fb_mmap locking

In which a mutex was added when changing smem_start and smem_len fields, so the mutex inside the fb_mmap() call is actually used.

The problem was that set_fb_fix, which modifies the above 2 fields, was called before and after registering the framebuffer, which when used before registration, lead to a failed attempt to use an uninitialized mutex.

Solution: Don't use mutex before framebuffer registration.

---
?drivers/video/omap/omapfb_main.c | ? 22 ++++++++++++++-------- ?1 files changed, 14 insertions(+), 8 deletions(-)

drivers/video/omap/omapfb_main.c +0 -0
index 125e605..0d0c8c8 100644
@@ -393,7 +393,7 @@ static void omapfb_sync(struct fb_info *fbi)

?* Set fb_info.fix fields and also updates fbdev. ?* When calling this fb_info.var must be set up already. ?*/

drivers/video/omap/omapfb_main.c +1 -1
-static void set_fb_fix(struct fb_info *fbi)
+static void set_fb_fix(struct fb_info *fbi, int from_init)

?{
? ? ? ?struct fb_fix_screeninfo *fix = &fbi->fix;
? ? ? ?struct fb_var_screeninfo *var = &fbi->var;

drivers/video/omap/omapfb_main.c +12 -6
@@ -403,10 +403,16 @@ static void set_fb_fix(struct fb_info *fbi)
 
 ? ? ? ?rg = &plane->fbdev->mem_desc.region[plane->idx];
 ? ? ? ?fbi->screen_base ? ? ? ?= rg->vaddr;
- ? ? ? mutex_lock(&fbi->mm_lock);
- ? ? ? fix->smem_start ? ? ? ? = rg->paddr;
- ? ? ? fix->smem_len ? ? ? ? ? = rg->size;
- ? ? ? mutex_unlock(&fbi->mm_lock);
+
+ ? ? ? if (!from_init) {
+ ? ? ? ? ? ? ? mutex_lock(&fbi->mm_lock);
+ ? ? ? ? ? ? ? fix->smem_start ? ? ? ? = rg->paddr;
+ ? ? ? ? ? ? ? fix->smem_len ? ? ? ? ? = rg->size;
+ ? ? ? ? ? ? ? mutex_unlock(&fbi->mm_lock);
+ ? ? ? } else {
+ ? ? ? ? ? ? ? fix->smem_start ? ? ? ? = rg->paddr;
+ ? ? ? ? ? ? ? fix->smem_len ? ? ? ? ? = rg->size;
+ ? ? ? }
 
 ? ? ? ?fix->type = FB_TYPE_PACKED_PIXELS;
 ? ? ? ?bpp = var->bits_per_pixel;
@@ -704,7 +710,7 @@ static int omapfb_set_par(struct fb_info *fbi)
 ? ? ? ?int r = 0;
 
 ? ? ? ?omapfb_rqueue_lock(fbdev);
- ? ? ? set_fb_fix(fbi);
+ ? ? ? set_fb_fix(fbi, 0);
 ? ? ? ?r = ctrl_change_mode(fbi);
 ? ? ? ?omapfb_rqueue_unlock(fbdev);
 
@@ -904,7 +910,7 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 ? ? ? ? ? ? ? ?if (old_size != size) {
 ? ? ? ? ? ? ? ? ? ? ? ?if (size) {
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?memcpy(&fbi->var, new_var, sizeof(fbi->var));
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? set_fb_fix(fbi);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? set_fb_fix(fbi, 0);
 ? ? ? ? ? ? ? ? ? ? ? ?} else {

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?/*
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? * Set these explicitly to indicate that the

drivers/video/omap/omapfb_main.c +1 -2
@@ -1504,7 +1510,7 @@ static int fbinfo_init(struct omapfb_device *fbdev, struct fb_info *info)
 ? ? ? ?var->bits_per_pixel = fbdev->panel->bpp;
 
 ? ? ? ?set_fb_var(info, var);
- ? ? ? set_fb_fix(info);
+ ? ? ? set_fb_fix(info, 1);
 
 ? ? ? ?r = fb_alloc_cmap(&info->cmap, 16, 0);
 ? ? ? ?if (r != 0)
 
--
Quoted 3 lines

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

This patch works beautifully for omap850-based devices. I need only add a board file and LCD resolution parameters, and I get a fully working framebuffer.

I have tested with linux-omap master (commit 6469d1c660f8751ff39836c81970cdbe46953057), adding only the board/lcd files on HTC Herald.

- Cory

maintainer
#3 lore raw

Re: [PATCH 2/8] omap: Lock DPLL5 at boot

In reply to Tony Lindgren
Quoted 6 lines · Tony Lindgren

From: Rajendra Nayak <[email protected]>

Lock DPLL5 at 120MHz at boot. The USBHOST 120MHz f-clock and USBTLL f-clock are the only users of this DPLL, and 120MHz is is the only recommended rate for these clocks.

Quoted 7 lines

With this patch, the 60 MHz ULPI clock is generated correctly.

Tested on an OMAP3430 SDP.

- Paul