From owner-freebsd-current  Sun Mar  9 13: 8: 3 2003
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP
	id 33DF037B401; Sun,  9 Mar 2003 13:08:00 -0800 (PST)
Received: from spider.deepcore.dk (cpe.atm2-0-56339.0x50c6aa0a.abnxx2.customer.tele.dk [80.198.170.10])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id E5D1B43FE0; Sun,  9 Mar 2003 13:07:57 -0800 (PST)
	(envelope-from sos@spider.deepcore.dk)
Received: (from sos@localhost)
	by spider.deepcore.dk (8.12.8/8.12.8) id h29L7qgi038028;
	Sun, 9 Mar 2003 22:07:52 +0100 (CET)
	(envelope-from sos)
From: Soeren Schmidt <sos@spider.deepcore.dk>
Message-Id: <200303092107.h29L7qgi038028@spider.deepcore.dk>
Subject: Re: ATA CS5530 (cyrix) driver panic (ata_cyrix_setmode())
In-Reply-To: <200303092113.h29LDYS00731@cruzio.com>
To: "Bruce R. Montague" <brucem@cruzio.com>
Date: Sun, 9 Mar 2003 22:07:52 +0100 (CET)
Cc: freebsd-current@FreeBSD.ORG, sos@FreeBSD.ORG
X-Mailer: ELM [version 2.4ME+ PL98b (25)]
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=ISO-8859-1
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-current.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-current>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-current>
X-Loop: FreeBSD.ORG

It seems Bruce R. Montague wrote:
> 

I'll look into it, thanks for reporting!

> 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
> 

-Søren

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