Re: [SeaBIOS] [PATCH] geodevga: make use of readl and writel for mmio access

[Kevin O'Connor]

On Tue, Jul 17, 2012 at 12:08:59PM +0200, Christian Gmeiner wrote:
> This commit just simplifies some code.
> Signed-off-by: Christian Gmeiner
>
> vgasrc/geodevga.c | 48 +++++++++---------------------------------------
>  1 file changed, 9 insertions(+), 39 deletions(-)
>
> diff --git a/vgasrc/geodevga.c b/vgasrc/geodevga.c
> index 5c6caf0..4e3d912 100644
> --- a/vgasrc/geodevga.c
> +++ b/vgasrc/geodevga.c

[...]
>  /* Set mmio registers
>  * there may be some timing issues here, the reads seem
>  * to slow things down enough work reliably
>  */
>
> reg = geode_memRead(vp+VP_MISC);
> reg = readl(vp+VP_MISC);

The code is run in 16bit "big real" mode.  This means all of memory is
available, but one still needs to be careful about segment usage.
SeaBIOS always calls the option roms with a %ss == %ds == 0, but I
don't think it is wise to rely upon that.  So, it's better to use
GET_FARVAR(0, *(vp+VP_MISC)).

-Kevin

[PATCH] geodevga: make use of readl and writel for mmio access

[Christian Gmeiner]

Maybe I am too blind but I don't get it:

out/../vgasrc/geodevga.c: In function ‘init_video_mode’:
out/../vgasrc/geodevga.c:443:11: error: variable or field ‘__val’ declared void
out/../vgasrc/geodevga.c:443:11: warning: dereferencing ‘void *’
pointer [enabled by default]
out/../vgasrc/geodevga.c:443:11: warning: dereferencing ‘void *’
pointer [enabled by default]
out/../vgasrc/geodevga.c:443:11: warning: dereferencing ‘void *’
pointer [enabled by default]
out/../vgasrc/geodevga.c:443:11: warning: dereferencing ‘void *’
pointer [enabled by default]
out/../vgasrc/geodevga.c:443:11: warning: taking address of expression
of type ‘void’ [enabled by default]
make: *** [out/vgaccode16.raw.s] Error 1

void *VG = (void *)pci_config_readl(GET_GLOBAL(VgaBDF), PCI_BASE_ADDRESS_3);
...
u32 val = GET_FARVAR(0, *(VG + 0x50));

I only want to read a u32 value with offset to 0x50 from VG.

---
Christian Gmeiner, MSc

2012/7/24 Kevin O'Connor :
> On Tue, Jul 17, 2012 at 12:08:59PM +0200, Christian Gmeiner wrote:
> > This commit just simplifies some code.
> > Signed-off-by: Christian Gmeiner
> >
> > vgasrc/geodevga.c | 48 +++++++++---------------------------------------
> >  1 file changed, 9 insertions(+), 39 deletions(-)
> >
> > diff --git a/vgasrc/geodevga.c b/vgasrc/geodevga.c
> > index 5c6caf0..4e3d912 100644
> > --- a/vgasrc/geodevga.c
> > +++ b/vgasrc/geodevga.c
>
> [...]
> >  /* Set mmio registers
> >  * there may be some timing issues here, the reads seem
> >  * to slow things down enough work reliably
> >  */
> >
> > reg = geode_memRead(vp+VP_MISC);
> > reg = readl(vp+VP_MISC);
>
> The code is run in 16bit "big real" mode.  This means all of memory is
> available, but one still needs to be careful about segment usage.
> SeaBIOS always calls the option roms with a %ss == %ds == 0, but I
> don't think it is wise to rely upon that.  So, it's better to use
> GET_FARVAR(0, *(vp+VP_MISC)).
>
> -Kevin