# Re: \[PATCH\] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

- From: Krzysztof Helt \<hidden\>
- Date: 2008-08-20 17:36:38 
- List: linux-fbdev
- Page: https://ratatoskr.run/linux-fbdev/2008/08/13458665
- Thread: https://ratatoskr.run/linux-fbdev/2008/08/13458644/t.md

```text
On Wed, 20 Aug 2008 18:31:09 +0200
Sascha Hauer [ref] wrote:

> On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> > On Tue, 19 Aug 2008 17:06:41 +0200
> > Sascha Hauer [ref] wrote:
> > 
> > > From: Juergen Beisert <redacted>
> > > 
> > > This patch prepares the current i.MX1 framebuffer driver for usage in the
> > > whole i.MX family. It switches to iowrite/ioread for register accesses.
> > > Also it moves the register definitions to the driver where they belong.
> > > 
> > > Signed-off-by: Juergen Beisert <redacted>
> > > ---
> > >  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
> > >  drivers/video/imxfb.h |    4 +-
> > >  2 files changed, 173 insertions(+), 39 deletions(-)
> > > 
> > > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > > index a990d43..0622589 100644
> > > --- a/drivers/video/imxfb.c
> > > +++ b/drivers/video/imxfb.c
> > > @@ -16,7 +16,6 @@
> > >   *	linux-arm-kernel@lists.arm.linux.org.uk
> > >   */
> > >  
> > > -//#define DEBUG 1
> > >  
> > >  #include <linux/module.h>
> > >  #include <linux/kernel.h>
> > > @@ -33,7 +32,6 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/dma-mapping.h>
> > >  
> > > -#include <mach/hardware.h>
> > >  #include <asm/io.h>
> > >  #include <mach/imxfb.h>
> > >  
> > > @@ -44,6 +42,124 @@
> > >  
> > >  #include "imxfb.h"
> > >  
> > > +#define DRIVER_NAME "imx-fb"
> > > +
> > 
> > All constants below should really be put into the imxfb.h. There is already such a file.
> 
> AFAIK the-new-beautiful-way is not to have these header files next to
> the .c files at all and put this stuff into the .c file. So maybe it's
> better to move the rest of imxfb.h here aswell.
> 

I was not aware about "the-new-beautiful-way". I am just curious about this.
Normally, the headers for frame buffers are located at include/video. 

It can stay this way (no header) if you insist.

> 
>  >  
> > > -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
> > >  static int
> > >  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> > >  		       u_int trans, struct fb_info *info)
> > > @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> > >  		      (CNVT_TOHW(green,4) << 4) |
> > >  		      CNVT_TOHW(blue,  4);
> > >  
> > > -		LCDC_PALETTE(regno) = val;
> > > +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
> > 
> > One may define an inline function imxfb_write(fbi, val, reg) :
> > 
> >  imxfb_iowrite(fbi, val, reg) 
> > {
> > 	iowrite32(val, fbi->regs + reg);
> > }
> > 
> > It would make the code more readable. The imxfb_ioread() could be used as well.
> >
> 
> Well, I think this is a matter of taste, I prefer not using wrappers
> around the access functions.
> 

Ok.

Regards,
Krzysztof

---------------------------------------------------------------------- 	
Prosty kredyt na wszystkie potrzeby. 
Od 1000 zl do 120 000 zl. 
Bez poreczycieli i zabezpieczen >>> http://link.interia.pl/f1edf


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
```