Re: Crash in IDE code

Thanks Geert,

We both know that I should have known better.
Regardless if PPC has an issue, you will.
This I need to flag this better.

This adds a whole new wrinkle in the total rework of the TASK commands.

Cheers,

Andre Hedrick
The Linux ATA/IDE guy

On 8 Jul 2000, Geert Uytterhoeven wrote:

On Sat, 8 Jul 2000, Michel Lanners wrote:

quoted

I've taken the time to disassemble the code part where it crashes. It's
in drivers/ide/ide.c, in this code inlined from ide_intr():

static inline int drive_is_ready (ide_drive_t *drive)
{
        if (drive->waiting_for_dma)
                return HWIF(drive)->dmaproc(ide_dma_test_irq, drive);
#if 0
        udelay(1);      /* need to guarantee 400ns since last command was issued */
#endif
//      if (GET_STAT() & BUSY_STAT)     /* Note: this may clear a pending IRQ!! */
        if (IN_BYTE(IDE_ALTSTATUS_REG) & BUSY_STAT)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^

                return 0;       /* drive busy:  definitely not interrupting */
        return 1;               /* drive ready: *might* be interrupting */
}

The crashing part is marked above; it's the IN_BYTE which tries to read
from addr. 0 :-(

Above code translates to this:

IN_BYTE( (((ide_hwif_t *)((drive)->hwif))->io_ports[IDE_CONTROL_OFFSET]) )

Yep, ALTSTATUS == CONTROL

quoted

So, io_ports[IDE_CONTROL_OFFSET] is obviously zero; it's suposed to be
in GPR11 in the oops above. One point where it is explicitly set to zero
is in ide_pmac.c:

        if (ix >= MAX_HWIFS) {
                /* Probably a PCI interface... */
                for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; ++i)
                        hw->io_ports[i] = data_port + i - IDE_DATA_OFFSET;
                /* XXX is this right? */
                hw->io_ports[IDE_CONTROL_OFFSET] = 0;

So, question is: is it right to zero hw->io_ports[IDE_CONTROL_OFFSET]?
Should I compile without PMac IDE support, since my box has no 'native'
IDE? Or should the test above in drive_is_ready() be protected against a
NULL pointer, like most other occurences are?

It's indeed true that some hardware doesn't have the CONTROL/ALTSTATUS
register. I don't know about the PowerMac, but I know about the Amiga
IDE-doublers[*]. So there _must_ be a test for a non-zero register offset
prior to the usage of CONTROL/ALTSTATUS. Hence this is a bug.

Gr{oetje,eeting}s,

						Geert

[*] IDE-doublers are actually something that can be used on most IDE
    interfaces. IDE defines 2 banks of 8 registers to access the drive. From
    the second bank, only one register is actually used (CONTROL/ALTSTATUS).
    Since CONTROL/ALTSTATUS is not vital to the functionality of the IDE
    interface, the second bank of registers can be sacrificed and one IDE chain
    that supports 2 devices can be `split' into 2 chains supporting 2x2 devices
    using only a few diodes (schematic available on request). Then accessing
    the first bank of registers will still access the first bank of registers
    on the first chain, while accessing the second bank of registers will
    access the first bank of registers on the second chain.
    Driver: see the IDE-doubler option in drivers/ide/gayle.c.
--
Geert Uytterhoeven -- Linux/{m68k~Amiga,PPC~CHRP} -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/