Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 9 Mar 2003 13:13:34 -0800 (PST)
From:      "Bruce R. Montague" <brucem@cruzio.com>
To:        freebsd-current@FreeBSD.ORG
Cc:        sos@FreeBSD.ORG
Subject:   ATA CS5530 (cyrix) driver panic (ata_cyrix_setmode())
Message-ID:  <200303092113.h29LDYS00731@cruzio.com>

next in thread | raw e-mail | index | archive | help


Hi, the current -current ata driver panics at boot
when using the CS5530 (National GX1, ex-"cyrix").
This driver worked in the past on -current, likely
up until the major rework that appears to be
underway as of 20-Feb-2003 (that is, the creation
of "ata-chipset.c", etc.)

Routine  "ata_cyrix_setmode()" in "ata-chipset.c"
appears to be assuming that "channel->dma" has a
valid pointer to a "struct ata_dma_funcs", and
that "channel->r_bmio" is a valid bus resource
id. This is not the case, both "channel->dma" and
"channel->r_bmio" (bus master I/O supported) are
0, which will result in panics. The first use (and
panic) occurs at:

  atadev->channel->dma->alignment = 16;

"ata_cyrix_setmode+0x8b:  movl $0x10,0x20(%eax)"
on my build (%eax is 0). 

These panics occur regardless of the setting of
TUNABLE_INIT() "ata_dma".

Routine "ata_dmainit()", which mallocs the "struct
ata_dma_funcs" is (likely correctly) never called.
If required due to DMA support, it is allocated
during the driver probe via "ctlr->dmainit(ch)"
in "ata_pcisub_probe()" in "ata-pci.c".

To make the system "come up", I replaced
"ata_cyrix_setmode()" with the following:

static void
ata_cyrix_setmode(struct ata_device *atadev, int mode)
{
    int error;

    mode = ata_limit_mode(atadev, mode, ATA_UDMA2);

    error = ata_command(atadev, ATA_C_SETFEATURES, 0, mode,
			ATA_C_F_SETXFER, ATA_WAIT_READY);

    if (bootverbose)
	ata_prtdev(atadev, "%s setting %s on Cyrix chip\n",
		   (error) ? "failed" : "success", ata_mode2str(mode));

    atadev->mode = mode;
}


This seems to work, I am using the system without
apparent problems, but it is strictly a "by guess
and by god" fix - I haven't studied or understood
the whole new ata driver scaffolding.

What is (a) correct fix? Is there a better and
more complete thing envisoned? Is there a tunable
I don't understand? Or a feature flag? Is the
current "cyrix" code in transit and untested?
Should "ata_dma" or "r_bmio" be checked in the
setmode codepath? Can I help assure this is fixed
right? I can at least test, if need be.

As a minor question, is the style to allocate
malloced data structures (such as "ata_dma_funcs")
in the probe code instead of attach code, as seems
to be the intent in this driver, and leave permanent
bus resource allocation until the attach? (in this
case the "ata_pcisub_probe()" never reaches the
allocation code because it checks and finds "r_bmio"
is 0).





 - bruce

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200303092113.h29LDYS00731>