Date: Thu, 26 Jun 2003 14:57:40 -0700 (PDT) From: wpaul@FreeBSD.ORG (Bill Paul) To: imp@bsdimp.com (M. Warner Losh) Cc: sos@FreeBSD.ORG Subject: Re: Heads up: checking in change to ata-card.c Message-ID: <20030626215740.EFC6A37B401@hub.freebsd.org> In-Reply-To: <20030626.131417.101814768.imp@bsdimp.com> from "M. Warner Losh" at "Jun 26, 2003 01:14:17 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
> Here's a better patch, basesd on wpaul's input. Bill, can you try it
> an see if it works for you? If so, i would be better to commit this
> one. If not, I'll work with you to fix it.
Close, but this doesn't quite do it. I thought something didn't look
right when I read it, and I tested it just to be certain. Detecting the
card via the table is fine, but you're not skipping the right piece of
code. The existing code logic says:
if (getting the altio port range with bus_get_resource() succeeds)
then (set the resource offset depending on port range size)
else
(release the ioport resource and bail with ENXIO)
With my drive, it's the bus_get_resource() that fails, so we end up
failing the probe with ENXIO. Your patch changes this to:
if (the ONE_REGION flag is not set?) and (getting the port range
with bus_get_resource() succeeds)
then (set the resource offset depending on port range size)
else
(release the ioport resource and bail with ENXIO)
So if the ONE_REGION bit is set, we still bail with ENXIO. I put the
question mark up there because I think there's a typo:
- if (bus_get_resource(dev, SYS_RES_IOPORT, ATA_ALTADDR_RID, &tmp, &tmp)) {
+ if ((ap == NULL || (ap->flags & ONE_REGION) != 0) &&
+ bus_get_resource(dev, SYS_RES_IOPORT, ATA_ALTADDR_RID, &tmp, &tmp)) {
I think this:
if ((ap == NULL || (ap->flags & ONE_REGION) != 0) && ...
should be this:
if ((ap == NULL || (ap->flags & ONE_REGION) == 0) && ...
I might be wrong. In any case, it still doesn't work. I think the logic
should be:
if (the ONE_REGION flag is not set)
then (do all the other stuff)
This way, we skip the whole altio remapping step entirely. I just tested
this and it seems to work. Here's a modified version of your patch
showing what I mean:
http://www.freebsd.org/~wpaul/ata-card.c.diff2
Also, since Soren asked, here are verbose boot messages in both the
failure and success cases:
http://www.freebsd.org/~wpaul/dmesg.bad
http://www.freebsd.org/~wpaul/dmesg.good
> If you are uninterested in working with us to get things in, then your
> patch will not lasts the evening as such an approach is unacceptibe.
Well, with all due respect, the fact that this is still broken is
also unacceptable. All I want is to insure that this is fixed and
that it stays fixed. I don't care if it's your patch or mine, as
long as it works, and doesn't regress.
And for the record, if somebody came to me and said: "Bill! Your
<so-and-so> driver hasn't worked right on <such-and-such card I don't
have> in months! I'm at wit's end! You leave me *NO* alternative!
I'm just going to have to fix it for you! At gunpoint! No no, don't
try and stop me! This is for your own good!" well, then, gosh, I'd
let 'em.
But I guess that's just me.
-Bill
--
=============================================================================
-Bill Paul (510) 749-2329 | Senior Engineer, Master of Unix-Fu
wpaul@windriver.com | Wind River Systems
=============================================================================
"If stupidity were a handicap, you'd have the best parking spot."
=============================================================================
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030626215740.EFC6A37B401>