[LinuxBIOS] [PATCH] 440bx raminit changes

[Ceri]

See attached diff for details.
Comments pls?

Thanks
C

[Peter Stuge]

On Tue, May 22, 2007 at 02:23:30PM +0100, Ceri wrote:
> Comments pls?

Have you had a chance to test it?
>   NBXCFG, 0x00000000, 0xff00a00c,PCI_REGISTER_32BIT,

[..]
>   FDHC, 0x00, 0x00, PCI_REGISTER_8BIT,
>   for (i = 0; i < max; i += 4) {
>          switch(register_values[i+3]){
>                 case PCI_REGISTER_8BIT: reg = pci_read_config8(ctrl->d0, register_values[i]); break;
>                 case PCI_REGISTER_16BIT: reg = pci_read_config16(ctrl->d0, register_values[i]);break;
>                 case PCI_REGISTER_32BIT: reg = pci_read_config32(ctrl->d0, register_values[i]);break;
>                 default: print_debug("Invalid PCI register size in table\n");
>          }

Please use more lines and the kernel switch style.

switch () {
case ..REG_8:
    reg = ...
    break;
case ..REG_16:
    ..
    break;
}

Maybe even use different temporary storage for different lengths.

//Peter

[Ceri Coburn]

Peter Stuge wrote:
> On Tue, May 22, 2007 at 02:23:30PM +0100, Ceri wrote:
> > Comments pls?
>
> Have you had a chance to test it?
> >   NBXCFG, 0x00000000, 0xff00a00c,PCI_REGISTER_32BIT,
>
> [..]
> >   FDHC, 0x00, 0x00, PCI_REGISTER_8BIT,
> >   for (i = 0; i < max; i += 4) {
> >          switch(register_values[i+3]){
> >                 case PCI_REGISTER_8BIT: reg = pci_read_config8(ctrl->d0, register_values[i]); break;
> >                 case PCI_REGISTER_16BIT: reg = pci_read_config16(ctrl->d0, register_values[i]);break;
> >                 case PCI_REGISTER_32BIT: reg = pci_read_config32(ctrl->d0, register_values[i]);break;
> >                 default: print_debug("Invalid PCI register size in table\n");
> >          }
>
> Please use more lines and the kernel switch style.
>
> switch () {
> case ..REG_8:
>    reg = ...
>    break;
> case ..REG_16:
>    ..
>    break;
> }
>
> Maybe even use different temporary storage for different lengths.
>
> //Peter

No problem, I have changed the switch statement style.

I wanted to limit the number of temp's used as I didn't want ROMCC 
complaining about no more registers available.

I have tested it in VMWare and it works fine.  I don't have a 440bx 
mainboard at hand at the moment to test real hardware, anyone have p2b 
or similar at hand to test the patch?

Thanks
C

[Uwe Hermann]

Hi,

On Tue, May 22, 2007 at 03:14:42PM +0100, Ceri Coburn wrote:
> I wanted to limit the number of temp's used as I didn't want ROMCC 
> complaining about no more registers available.

I was seeing the same problem when I tried to integrate your patch --
romcc runs out of registers.

The method with the register_values[] array is used in many other
northbridges, I think it should work (in general).

The problem with the 440BX register_values[] is that I messed it up ;-)
The bitmasks are set incorrectly (or rather: not at all) currently, that
needs fixing. As I understand it, it doesn't matter much if we do
8/16/32 bit reads and writes on PCI. We _do_ read too much stuff if we
always do a 32bit read, that true. But if the bitmasks are set
correctly, only those bits we _want_ to change will be changed.
All other bits (i.e., those we read but actually we didn't want to read)
will remain the same.

I _think_ this should not cause any problems, but feel free to correct me.

Anyway, the PAM changes are in the repository now, can you verify that
it works for VMWare? It does work on real hardware here...

Uwe.

[none]

Uwe Hermann wrote:
> Hi,
>
> On Tue, May 22, 2007 at 03:14:42PM +0100, Ceri Coburn wrote:
> > I wanted to limit the number of temp's used as I didn't want ROMCC 
> > complaining about no more registers available.
>
> I was seeing the same problem when I tried to integrate your patch --
> romcc runs out of registers.
>
> The method with the register_values[] array is used in many other
> northbridges, I think it should work (in general).
>
> The problem with the 440BX register_values[] is that I messed it up ;-)
> The bitmasks are set incorrectly (or rather: not at all) currently, that
> needs fixing. As I understand it, it doesn't matter much if we do
> 8/16/32 bit reads and writes on PCI. We _do_ read too much stuff if we
> always do a 32bit read, that true. But if the bitmasks are set
> correctly, only those bits we _want_ to change will be changed.
> All other bits (i.e., those we read but actually we didn't want to read)
> will remain the same.
>
> I _think_ this should not cause any problems, but feel free to correct me.

My bad, I'm still getting to grips with the common parts of the code
atm.  Well if that way works, you know what they say.  If it's not
broke, don't fix it.
> Anyway, the PAM changes are in the repository now, can you verify that
> it works for VMWare? It does work on real hardware here...w

I can confirm that the PAM register changes work in VMWare, with the
previous values memtest fails within the lower memory region (as
expected),  with the new values, VMWare passes memtest without any errors.

Ceri

[ron minnich]

On 5/22/07, Ceri Coburn  wrote:
> I wanted to limit the number of temp's used as I didn't want ROMCC
> complaining about no more registers available.

romcc is usually not that simple-minded, it should know about reuse.
Has too many temps
caused trouble for you?
> I have tested it in VMWare and it works fine.

This is probably not a sufficient test :-)

thanks

ron

[Ceri Coburn]

ron minnich wrote:
> On 5/22/07, Ceri Coburn  wrote:
> > I wanted to limit the number of temp's used as I didn't want ROMCC
> > complaining about no more registers available.
>
> romcc is usually not that simple-minded, it should know about reuse.
> Has too many temps
> caused trouble for you?

Not yet, I was just aware that there are limited amount of registers to use.
> > I have tested it in VMWare and it works fine.

Unfortunately that is all I have at the moment.  Although I have a nice 
Willem writer on the way :), so I should be able to test my patches with 
real hardware shortly.
> This is probably not a sufficient test :-)
>
> thanks
>
> ron

[Corey Osgood]

On Mon, 2007-05-28 at 20:23 +0100, Ceri Coburn wrote:
> Although I have a nice 
> Willem writer on the way :), so I should be able to test my patches with 
> real hardware shortly.

Do you have a Windows 98 box to run it on? I haven't been able to get
mine to work on linux (with wine), vista, or xp, with the normal
software. I haven't actually tried it on 98se, but from what I
understand that's what it was designed to work with. You also can always
hotswap the flash chips, ie boot up with the normal bios, then swap a
different chip in, and test with that. After you're done, you just swap
the old one back in, and repeat.

-Corey

[Ceri Coburn]

Corey Osgood wrote:
> On Mon, 2007-05-28 at 20:23 +0100, Ceri Coburn wrote:
> > Although I have a nice 
> > Willem writer on the way :), so I should be able to test my patches with 
> > real hardware shortly.
>
> Do you have a Windows 98 box to run it on? I haven't been able to get
> mine to work on linux (with wine), vista, or xp, with the normal
> software. I haven't actually tried it on 98se, but from what I
> understand that's what it was designed to work with.

Yup, I have it working in XP.  Although I have the True USB version from 
mcumall, so I imagine the drivers for the LPT port version are somewhat 
different.

You also can always
> hotswap the flash chips, ie boot up with the normal bios, then swap a
> different chip in, and test with that. After you're done, you just swap
> the old one back in, and repeat.

Didn't fancy that to be honest.  Hopefully the Willem way, development 
should be quicker.
> -Corey