[LinuxBIOS] FW: ADLO progress

[Carl-Daniel Hailfinger]

On 04.10.2007 00:48, Myles Watson wrote:
> These are all patches to ADLO/bios/rombios.c
> 
> They should be applied in this order:
> whitespace.patch
> defines.patch
> printf.patch
> ide_atapi.patch
> 
> ide.2.3.5.patch is a patch against the latest bios from Bochs.
> 
> The status is that I've eliminated the flakiness.  It boots into the windows
> install CD every time, reports my IDE drives correctly, and can install
> windows.

Nice. The preferred way to inclusion (at least as far as I am concerned)
would be like this:
1. Find out the remaining differences between our version of rombios.c
and (a possibly older version of) upstream and if there are any, try to
get them merged upstream (with reasonable changelog, if possible).
2. Prepare patches with detailed changelogs against upstream rombios.c
3. Take feedback from upstream and refine the patches
4. Submit a megapatch to LinuxBIOS which brings the ADLO version of
rombios.c to the state of upstream.

However, I'm still confused why Bochs can install/boot Vista without
problems (at least that's what was reported on their mailing list) and
ADLO seems to need patches.

A couple of comments below:

> +int await_ide();
> +static int await_ide(when_done,base,timeout)
> +  Bit8u when_done;
> +  Bit16u base;
> +  Bit16u timeout;
> +{
> +  Bit32u time=0,last=0;
> +  Bit16u status;
> +  Bit8u result;
> +  status = inb(base + ATA_CB_STAT); // for the times you're supposed to throw one away
> +  for(;;) {
> +    status = inb(base+ATA_CB_STAT);
> +    time++;
> +    if (when_done == BSY)
> +      result = status & ATA_CB_STAT_BSY;
> +    else if (when_done == NOT_BSY)
> +      result = !(status & ATA_CB_STAT_BSY);
> +    else if (when_done == NOT_BSY_DRQ)
> +      result = !(status & ATA_CB_STAT_BSY) && (status & ATA_CB_STAT_DRQ);
> +    else if (when_done == NOT_BSY_NOT_DRQ)
> +      result = !(status & ATA_CB_STAT_BSY) && !(status & ATA_CB_STAT_DRQ);
> +    else if (when_done == NOT_BSY_RDY)
> +      result = !(status & ATA_CB_STAT_BSY) && (status & ATA_CB_STAT_RDY);
> +    else if (when_done == TIMEOUT)
> +      result = 0;

No final else, so result is possibly undefined.

> +
> +    if (result) return 0;
> +    if (time>>16 != last) // mod 2048 each 16 ms

Please explain what the comment has to do with the code.

> +    {
> +      last = time >>16;
> +      BX_DEBUG_ATA("await_ide: (TIMEOUT,BSY,!BSY,!BSY_DRQ,!BSY_!DRQ) %d time= %ld timeout= %d\n",when_done,time>>11, timeout);
> +      print_status(base);
> +    }
> +    if (status & ATA_CB_STAT_ERR)
> +    {
> +      BX_DEBUG_ATA("await_ide: ERROR (TIMEOUT,BSY,!BSY,!BSY_DRQ,!BSY_!DRQ) %d time= %ld timeout= %d\n",when_done,time>>11, timeout);
> +      print_status(base);
> +      return -1;
> +    }
> +    if ((timeout == 0) || ((time>>11) > timeout)) break;
> +  }
> +  BX_INFO("IDE time out\n");
> +  return -1;
> +}
[...]
> @@ -2233,7 +2306,22 @@
>        write_byte(ebda_seg,&EbdaData->ata.devices[device].type,ATA_TYPE_UNKNOWN);
>  
>        // reset the channel
> -      ata_reset(device);
> +        // ATA-4
> +        // 9.3.1-2 Software reset - Device 0 or 1
> +        // 9.3.1 (a), 9.3.2 (a) -- set SRST in DC
> +        outb(iobase2+ATA_CB_DC, ATA_CB_DC_HD15 | ATA_CB_DC_NIEN | ATA_CB_DC_SRST);
> +        udelay(); udelay(); udelay(); udelay(); udelay(); //5 us
> +
> +        // 9.3.1 (c), 9.3.2 (c) -- wait for BSY
> +        await_ide(BSY, iobase1, IDE_TIMEOUT);
> +
> +        // 9.3.1 (f), 9.3.2 (g) -- clear SRST
> +        outb(iobase2+ATA_CB_DC, ATA_CB_DC_HD15 | ATA_CB_DC_NIEN);
> +
> +        // 9.3.1 (l), 9.3.2 (m) -- wait for !BSY
> +        await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
> +
> +        // 9.3.1 (m), 9.3.2 (n) -- wait for !BSY and DRDY if not ATA_TYPE_ATAPI
>  
>        // check for ATA or ATAPI
>        outb(iobase1+ATA_CB_DH, slave ? ATA_CB_DH_DEV1 : ATA_CB_DH_DEV0);

Any reason to open-code ata_reset()?

There are a few other issues with the patches, but since they did not
have any changelogs, I am not sure what the patches try to achieve.

Regards,
Carl-Daniel