Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 22 Dec 1999 21:59:30 +0100 (CET)
From:      Soren Schmidt <sos@freebsd.dk>
To:        wpaul@skynet.ctr.columbia.edu (Bill Paul)
Cc:        current@freebsd.org, sos@freebsd.org
Subject:   Re: General ata grousing
Message-ID:  <199912222059.VAA71772@freebsd.dk>
In-Reply-To: <199912222012.PAA09596@skynet.ctr.columbia.edu> from Bill Paul at "Dec 22, 1999 03:12:21 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
It seems Bill Paul wrote:
[snip snap]
 
> I don't want to sound like an ungrateful wretch, unduly criticizing
> someone else's code, especially at so late a date, but there are some 
> other things that just seem like they really shouldn't be there:

We've got used to it, on with matters... 

> - Platform dependencies. The inthand_add() thing I mentioned previously
>   appears to be an x86-specific kludge, and there's an alpha kludge to
>   go along with it. There should be some way to get rid of this.

If you looked at the code, you would see that the ata driver only uses
this ugly method when we are dealing with the standard primary & 
secondary controller which are bound to specific addresses and interrupts.
Those are not configurable. Controllers not on these adresses are attached
with bus_alloc_resource etc etc. So you should not have any problems
with an ata controller on IRQ11. 

> - Magic numbers everywhere. I see lots of places where I/O and PCI config
>   registers are being manipulated using just hard coded register offsets
>   and bitmasks. Magic numbers are bad, mmmm-kay?

Mmmm, what do you want instead?? I think I've put names on most things
but some are just numbers. But frankly I dont se the difference
between a name and a number if you are fiddling HW, unless you want
the entire chip datasheet in there as comments. 
BTW I just had a look at your if_rl driver the other day, cause I got 
a bunch of those cards here, and you have some serioius cleaning house 
to do also, mmmm-kay :)

> - Use of inb/outb instead of bus_space_read_X()/bus_space_write_X().
>   My understanding is that bus_space_read_X()/bus_space_write_X() are
>   the prefered way of doing register accesses. inb/out and friends are
>   deprecated.

Hmm, I was under the impression that it didn't matter much, I feel
strange when I see those bus_space_bla functions in IO related
stuff, but anyhow, I'm just getting old, those parts of the
ata driver was written when the bus* functions was in its infancy.
However those are easily changed with a little script, but franly
I dont see the need right now, more important things matters more.

> Anyway, I'm going to continue trying to hunt down the interrupt setup
> problem once I get home tonight (nice thing about having a laptop for
> a test box: you don't have to leave the test machine at work and frob
> it remotely). If anyone has any insights, please feel free to share
> them.

Good hunt...

-Søren


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?199912222059.VAA71772>