* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
@ 2010-04-07 6:51 Dave Liu
2010-04-07 14:35 ` Timur Tabi
0 siblings, 1 reply; 11+ messages in thread
From: Dave Liu @ 2010-04-07 6:51 UTC (permalink / raw)
To: u-boot
From: Jerry Huang <Chang-Ming.Huang@freescale.com>
Using PPC I/O accessor to DIU I/O space instead of directly
read/write. It will prevent the dozen of compiler order issue
and PPC hardware order issue for accessing I/O space.
Using the toolchain(tc-fsl-x86lnx-e500-dp-4.3.74-2.i386.rpm)
can show up the order issue of DIU driver.
Signed-off-by: Dave Liu <daveliu@freescale.com>
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
---
board/freescale/common/fsl_diu_fb.c | 55 ++++++++++++++++++-----------------
1 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/board/freescale/common/fsl_diu_fb.c b/board/freescale/common/fsl_diu_fb.c
index 2fc878b..36c5d3d 100644
--- a/board/freescale/common/fsl_diu_fb.c
+++ b/board/freescale/common/fsl_diu_fb.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2007 Freescale Semiconductor, Inc.
+ * Copyright 2007, 2010 Freescale Semiconductor, Inc.
* York Sun <yorksun@freescale.com>
*
* FSL DIU Framebuffer driver
@@ -26,6 +26,7 @@
#include <common.h>
#include <i2c.h>
#include <malloc.h>
+#include <asm/io.h>
#include "fsl_diu_fb.h"
@@ -246,9 +247,9 @@ int fsl_diu_init(int xres,
memset(info->screen_base, 0, info->smem_len);
- dr.diu_reg->desc[0] = (unsigned int) &dummy_ad;
- dr.diu_reg->desc[1] = (unsigned int) &dummy_ad;
- dr.diu_reg->desc[2] = (unsigned int) &dummy_ad;
+ out_be32(&dr.diu_reg->desc[0], &dummy_ad);
+ out_be32(&dr.diu_reg->desc[1], &dummy_ad);
+ out_be32(&dr.diu_reg->desc[2], &dummy_ad);
debug("dummy dr.diu_reg->desc[0] = 0x%x\n", dr.diu_reg->desc[0]);
debug("dummy desc[0] = 0x%x\n", hw->desc[0]);
@@ -310,26 +311,26 @@ int fsl_diu_init(int xres,
/* Program DIU registers */
- hw->gamma = (unsigned int) gamma.paddr;
- hw->cursor= (unsigned int) cursor.paddr;
- hw->bgnd = 0x007F7F7F; /* BGND */
- hw->bgnd_wb = 0; /* BGND_WB */
- hw->disp_size = var->yres << 16 | var->xres; /* DISP SIZE */
- hw->wb_size = 0; /* WB SIZE */
- hw->wb_mem_addr = 0; /* WB MEM ADDR */
- hw->hsyn_para = var->left_margin << 22 | /* BP_H */
+ out_be32(&hw->gamma, gamma.paddr);
+ out_be32(&hw->cursor, cursor.paddr);
+ out_be32(&hw->bgnd, 0x007F7F7F);
+ out_be32(&hw->bgnd_wb, 0); /* BGND_WB */
+ out_be32(&hw->disp_size, var->yres << 16 | var->xres); /* DISP SIZE */
+ out_be32(&hw->wb_size, 0); /* WB SIZE */
+ out_be32(&hw->wb_mem_addr, 0); /* WB MEM ADDR */
+ out_be32(&hw->hsyn_para, var->left_margin << 22 | /* BP_H */
var->hsync_len << 11 | /* PW_H */
- var->right_margin; /* FP_H */
- hw->vsyn_para = var->upper_margin << 22 | /* BP_V */
- var->vsync_len << 11 | /* PW_V */
- var->lower_margin; /* FP_V */
+ var->right_margin); /* FP_H */
- hw->syn_pol = 0; /* SYNC SIGNALS POLARITY */
- hw->thresholds = 0x00037800; /* The Thresholds */
- hw->int_status = 0; /* INTERRUPT STATUS */
- hw->int_mask = 0; /* INT MASK */
- hw->plut = 0x01F5F666;
+ out_be32(&hw->vsyn_para, var->upper_margin << 22 | /* BP_V */
+ var->vsync_len << 11 | /* PW_V */
+ var->lower_margin); /* FP_V */
+ out_be32(&hw->syn_pol, 0); /* SYNC SIGNALS POLARITY */
+ out_be32(&hw->thresholds, 0x00037800); /* The Thresholds */
+ out_be32(&hw->int_status, 0); /* INTERRUPT STATUS */
+ out_be32(&hw->int_mask, 0); /* INT MASK */
+ out_be32(&hw->plut, 0x01F5F666);
/* Pixel Clock configuration */
debug("DIU pixclock in ps - %d\n", var->pixclock);
diu_set_pixel_clock(var->pixclock);
@@ -369,8 +370,8 @@ static int fsl_diu_enable_panel(struct fb_info *info)
struct diu_ad *ad = &fsl_diu_fb_ad;
debug("Entered: enable_panel\n");
- if (hw->desc[0] != (unsigned int)ad)
- hw->desc[0] = (unsigned int)ad;
+ if (in_be32(&hw->desc[0]) != (unsigned int)ad)
+ out_be32(&hw->desc[0], ad);
debug("desc[0] = 0x%x\n", hw->desc[0]);
return 0;
}
@@ -380,8 +381,8 @@ static int fsl_diu_disable_panel(struct fb_info *info)
struct diu *hw = dr.diu_reg;
debug("Entered: disable_panel\n");
- if (hw->desc[0] != (unsigned int)&dummy_ad)
- hw->desc[0] = (unsigned int)&dummy_ad;
+ if (in_be32(&hw->desc[0]) != (unsigned int)&dummy_ad)
+ out_be32(&hw->desc[0], &dummy_ad);
return 0;
}
@@ -422,7 +423,7 @@ static void enable_lcdc(void)
debug("Entered: enable_lcdc, fb_enabled = %d\n", fb_enabled);
if (!fb_enabled) {
- hw->diu_mode = dr.mode;
+ out_be32(&hw->diu_mode, dr.mode);
fb_enabled++;
}
debug("diu_mode = %d\n", hw->diu_mode);
@@ -434,7 +435,7 @@ static void disable_lcdc(void)
debug("Entered: disable_lcdc, fb_enabled = %d\n", fb_enabled);
if (fb_enabled) {
- hw->diu_mode = 0;
+ out_be32(&hw->diu_mode, 0);
fb_enabled = 0;
}
}
--
1.6.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-07 6:51 [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space Dave Liu
@ 2010-04-07 14:35 ` Timur Tabi
2010-04-08 6:14 ` Liu Dave-R63238
0 siblings, 1 reply; 11+ messages in thread
From: Timur Tabi @ 2010-04-07 14:35 UTC (permalink / raw)
To: u-boot
Dave Liu wrote:
> @@ -369,8 +370,8 @@ static int fsl_diu_enable_panel(struct fb_info *info)
> struct diu_ad *ad = &fsl_diu_fb_ad;
>
> debug("Entered: enable_panel\n");
> - if (hw->desc[0] != (unsigned int)ad)
> - hw->desc[0] = (unsigned int)ad;
> + if (in_be32(&hw->desc[0]) != (unsigned int)ad)
> + out_be32(&hw->desc[0], ad);
'ad' should be cast to a u32, not an "unsigned int". You shouldn't
compare a sized type with an unsized type.
> debug("desc[0] = 0x%x\n", hw->desc[0]);
> return 0;
> }
> @@ -380,8 +381,8 @@ static int fsl_diu_disable_panel(struct fb_info *info)
> struct diu *hw = dr.diu_reg;
>
> debug("Entered: disable_panel\n");
> - if (hw->desc[0] != (unsigned int)&dummy_ad)
> - hw->desc[0] = (unsigned int)&dummy_ad;
> + if (in_be32(&hw->desc[0]) != (unsigned int)&dummy_ad)
> + out_be32(&hw->desc[0], &dummy_ad);
Same thing here.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-07 14:35 ` Timur Tabi
@ 2010-04-08 6:14 ` Liu Dave-R63238
2010-04-08 7:27 ` Kumar Gala
2010-04-08 10:11 ` Wolfgang Denk
0 siblings, 2 replies; 11+ messages in thread
From: Liu Dave-R63238 @ 2010-04-08 6:14 UTC (permalink / raw)
To: u-boot
> > @@ -369,8 +370,8 @@ static int fsl_diu_enable_panel(struct
> fb_info *info)
> > struct diu_ad *ad = &fsl_diu_fb_ad;
> >
> > debug("Entered: enable_panel\n");
> > - if (hw->desc[0] != (unsigned int)ad)
> > - hw->desc[0] = (unsigned int)ad;
> > + if (in_be32(&hw->desc[0]) != (unsigned int)ad)
> > + out_be32(&hw->desc[0], ad);
> 'ad' should be cast to a u32, not an "unsigned int". You
> shouldn't compare a sized type with an unsized type.
Grep the include/asm-ppc/types.h, I got typedef unsigned int u32;
The u32 is same as "unsigned int".
Kumar, any comments?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 6:14 ` Liu Dave-R63238
@ 2010-04-08 7:27 ` Kumar Gala
2010-04-08 7:32 ` Liu Dave-R63238
2010-04-08 17:15 ` Scott Wood
2010-04-08 10:11 ` Wolfgang Denk
1 sibling, 2 replies; 11+ messages in thread
From: Kumar Gala @ 2010-04-08 7:27 UTC (permalink / raw)
To: u-boot
On Apr 8, 2010, at 1:14 AM, Liu Dave-R63238 wrote:
>>> @@ -369,8 +370,8 @@ static int fsl_diu_enable_panel(struct
>> fb_info *info)
>>> struct diu_ad *ad = &fsl_diu_fb_ad;
>>>
>>> debug("Entered: enable_panel\n");
>>> - if (hw->desc[0] != (unsigned int)ad)
>>> - hw->desc[0] = (unsigned int)ad;
>>> + if (in_be32(&hw->desc[0]) != (unsigned int)ad)
>>> + out_be32(&hw->desc[0], ad);
>
>> 'ad' should be cast to a u32, not an "unsigned int". You
>> shouldn't compare a sized type with an unsized type.
>
> Grep the include/asm-ppc/types.h, I got typedef unsigned int u32;
> The u32 is same as "unsigned int".
>
> Kumar, any comments?
Timur is correct, while u32 is typedef to 'unsigned int' its cleaner in the code to use u32 and if we have a 64-bit port of u-boot on PPC in the future it makes things cleaner.
- k
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 7:27 ` Kumar Gala
@ 2010-04-08 7:32 ` Liu Dave-R63238
2010-04-08 7:58 ` Kumar Gala
2010-04-08 17:15 ` Scott Wood
1 sibling, 1 reply; 11+ messages in thread
From: Liu Dave-R63238 @ 2010-04-08 7:32 UTC (permalink / raw)
To: u-boot
> >>> - if (hw->desc[0] != (unsigned int)ad)
> >>> - hw->desc[0] = (unsigned int)ad;
> >>> + if (in_be32(&hw->desc[0]) != (unsigned int)ad)
> >>> + out_be32(&hw->desc[0], ad);
> >
> >> 'ad' should be cast to a u32, not an "unsigned int". You
> >> shouldn't compare a sized type with an unsized type.
> >
> > Grep the include/asm-ppc/types.h, I got typedef unsigned int u32;
> > The u32 is same as "unsigned int".
> >
> > Kumar, any comments?
>
> Timur is correct, while u32 is typedef to 'unsigned int' its
> cleaner in the code to use u32 and if we have a 64-bit port
> of u-boot on PPC in the future it makes things cleaner.
So, I need to change the (unsigned int)ad to (u32)ad?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 7:32 ` Liu Dave-R63238
@ 2010-04-08 7:58 ` Kumar Gala
0 siblings, 0 replies; 11+ messages in thread
From: Kumar Gala @ 2010-04-08 7:58 UTC (permalink / raw)
To: u-boot
On Apr 8, 2010, at 2:32 AM, Liu Dave-R63238 wrote:
>>>>> - if (hw->desc[0] != (unsigned int)ad)
>>>>> - hw->desc[0] = (unsigned int)ad;
>>>>> + if (in_be32(&hw->desc[0]) != (unsigned int)ad)
>>>>> + out_be32(&hw->desc[0], ad);
>>>
>>>> 'ad' should be cast to a u32, not an "unsigned int". You
>>>> shouldn't compare a sized type with an unsized type.
>>>
>>> Grep the include/asm-ppc/types.h, I got typedef unsigned int u32;
>>> The u32 is same as "unsigned int".
>>>
>>> Kumar, any comments?
>>
>> Timur is correct, while u32 is typedef to 'unsigned int' its
>> cleaner in the code to use u32 and if we have a 64-bit port
>> of u-boot on PPC in the future it makes things cleaner.
>
> So, I need to change the (unsigned int)ad to (u32)ad?
yes, please do. Otherwise patch looked good.
- k
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 6:14 ` Liu Dave-R63238
2010-04-08 7:27 ` Kumar Gala
@ 2010-04-08 10:11 ` Wolfgang Denk
2010-04-08 15:50 ` Kumar Gala
1 sibling, 1 reply; 11+ messages in thread
From: Wolfgang Denk @ 2010-04-08 10:11 UTC (permalink / raw)
To: u-boot
Dear "Liu Dave-R63238",
In message <D7CCA83BB0796C49BC0BB53B6AB12089A9C4E3@zch01exm21.fsl.freescale.net> you wrote:
>
> > 'ad' should be cast to a u32, not an "unsigned int". You
> > shouldn't compare a sized type with an unsized type.
...
> The u32 is same as "unsigned int".
Maybe, maybe not. If you want to be safe, then compare u32 with u32
only.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When you say "I wrote a program that crashed Windows", people just
stare at you blankly and say "Hey, I got those with the system, *for
free*". - Linus Torvalds in <3itc77$9lj@ninurta.fer.uni-lj.si>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 10:11 ` Wolfgang Denk
@ 2010-04-08 15:50 ` Kumar Gala
2010-04-08 16:19 ` Timur Tabi
0 siblings, 1 reply; 11+ messages in thread
From: Kumar Gala @ 2010-04-08 15:50 UTC (permalink / raw)
To: u-boot
On Apr 8, 2010, at 5:11 AM, Wolfgang Denk wrote:
> Dear "Liu Dave-R63238",
>
> In message <D7CCA83BB0796C49BC0BB53B6AB12089A9C4E3@zch01exm21.fsl.freescale.net> you wrote:
>>
>>> 'ad' should be cast to a u32, not an "unsigned int". You
>>> shouldn't compare a sized type with an unsized type.
> ...
>
>> The u32 is same as "unsigned int".
>
> Maybe, maybe not. If you want to be safe, then compare u32 with u32
> only.
Good point, I wasn't look at what the src info was. We should probably fix the struct diu_ad, diu to use 'u32' etc.
- k
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 15:50 ` Kumar Gala
@ 2010-04-08 16:19 ` Timur Tabi
2010-04-10 20:24 ` Wolfgang Denk
0 siblings, 1 reply; 11+ messages in thread
From: Timur Tabi @ 2010-04-08 16:19 UTC (permalink / raw)
To: u-boot
On Thu, Apr 8, 2010 at 10:50 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
> Good point, I wasn't look at what the src info was. ?We should probably fix the struct diu_ad, diu to use 'u32' etc.
Or __be32. I think the consensus is that struct fields for hardware
registers should use endian-specific sized integers.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 7:27 ` Kumar Gala
2010-04-08 7:32 ` Liu Dave-R63238
@ 2010-04-08 17:15 ` Scott Wood
1 sibling, 0 replies; 11+ messages in thread
From: Scott Wood @ 2010-04-08 17:15 UTC (permalink / raw)
To: u-boot
On Thu, Apr 08, 2010 at 02:27:01AM -0500, Kumar Gala wrote:
>
> On Apr 8, 2010, at 1:14 AM, Liu Dave-R63238 wrote:
>
> >>> @@ -369,8 +370,8 @@ static int fsl_diu_enable_panel(struct
> >> fb_info *info)
> >>> struct diu_ad *ad = &fsl_diu_fb_ad;
> >>>
> >>> debug("Entered: enable_panel\n");
> >>> - if (hw->desc[0] != (unsigned int)ad)
> >>> - hw->desc[0] = (unsigned int)ad;
> >>> + if (in_be32(&hw->desc[0]) != (unsigned int)ad)
> >>> + out_be32(&hw->desc[0], ad);
> >
> >> 'ad' should be cast to a u32, not an "unsigned int". You
> >> shouldn't compare a sized type with an unsized type.
> >
> > Grep the include/asm-ppc/types.h, I got typedef unsigned int u32;
> > The u32 is same as "unsigned int".
> >
> > Kumar, any comments?
>
> Timur is correct, while u32 is typedef to 'unsigned int' its cleaner in
> the code to use u32 and if we have a 64-bit port of u-boot on PPC in the
> future it makes things cleaner.
If we have a 64-bit port of u-boot, then casting a pointer to u32 will still
be broken...
-Scott
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space
2010-04-08 16:19 ` Timur Tabi
@ 2010-04-10 20:24 ` Wolfgang Denk
0 siblings, 0 replies; 11+ messages in thread
From: Wolfgang Denk @ 2010-04-10 20:24 UTC (permalink / raw)
To: u-boot
Dear Timur Tabi,
In message <i2xed82fe3e1004080919p5e6a472ek388dbf21a3bd899f@mail.gmail.com> you wrote:
> On Thu, Apr 8, 2010 at 10:50 AM, Kumar Gala <galak@kernel.crashing.org> wro=
> te:
>
> > Good point, I wasn't look at what the src info was. =A0We should probably=
> fix the struct diu_ad, diu to use 'u32' etc.
>
> Or __be32. I think the consensus is that struct fields for hardware
> registers should use endian-specific sized integers.
<sarcasm>
Is there such consensus? Does this include chip designers?
I mean, can we be sure that __be32 will also work if this IP core
shows up in a little-endian ARM system?
</sarcasm>
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Our business is run on trust. We trust you will pay in advance.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-04-10 20:24 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-07 6:51 [U-Boot] [PATCH] fsl-diu: Using I/O accessor to CCSR space Dave Liu
2010-04-07 14:35 ` Timur Tabi
2010-04-08 6:14 ` Liu Dave-R63238
2010-04-08 7:27 ` Kumar Gala
2010-04-08 7:32 ` Liu Dave-R63238
2010-04-08 7:58 ` Kumar Gala
2010-04-08 17:15 ` Scott Wood
2010-04-08 10:11 ` Wolfgang Denk
2010-04-08 15:50 ` Kumar Gala
2010-04-08 16:19 ` Timur Tabi
2010-04-10 20:24 ` Wolfgang Denk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.