On Thu, Sep 18, 2003 at 10:05:15PM +0000, Charles M. Hannum wrote:
> 
> This is a snapshot of some work I'm doing to simplify the SCSI and
> IDE/ATAPI code.  I'm posting it here for early review.
> 
> Besides reducing both source and compiled binary size noticably, I've
> added XS_CTL_XFER_ONSTACK, which gives callers direct access to
> scsipi_xfer structures, enabling them to do a more thorough inspection
> of the state (residual count, SCSI status code, sense data, etc.) when
> deciding what to do.  This will be used to make some things (INQUIRY
> response handling, in particular) more robust.

Yes, I remember missing access to this for some work I did some time ago
(maybe for some changes to st(4), I'm not sure)

> 
> I have not (yet) updated some of the wdc frontends used on non-x86
> platforms.  It would be useful for people to test this as much as
> possible, though.
> 
> 
> 1) Use config_defer() to attach IDE and ATAPI drives.  This eliminates most
>    polling and will simplify the control path a little.

Arg, this confligs with some changes I've been working on for about a month.
I planned to send a mail about this this week-end.
Basically, I did add a per-channel kernel thread and per-channel device
(atabus*). The kernel thread is used for all operations that require
polling (reset, wait for ready before sending command, etc). channels
reset and probes are deffered to run in the kernel thread context.
This allows, amongs others things, to do the channel resets in
parallel, speeding up the boot noticably on systems with several pciide
adapters. This also should avoid hangs for several minutes when a 
drive goes bad to the point that it doesn't even anserws to a reset.

The per-channel device is there mostly in preparation for hot-plug, where
we'll need to do some per-channel operations (tri-states the adapter,
scan the channel). We can also add a reset operation to it.

I'd also like to split the pciide driver into different controller-specific
drivers (piixide, pdcide, viaide, siside, etc ...) to allow builing smaller
custom kernels.

I'd like to do the 2 changes almost at the same time, because both of
them need a kernel config file update.
More news about this in following mails, with patches for the per-channels
thread. 

BTW, my change doesn't require doing the config_defer() dance from all the
IDE front-end, it's all in wdc.c


> 2) Add XS_CTL_XFER_ONSTACK, which is used to create an initialized scsipi_xfer
>    that can be handed directly to scsipi_execute_xs(), and use this in a number
>    of places.  There are several components to this:
>    a) Move the XS_CTL_DATA_ONSTACK PHOLD()/PRELE() from scsipi_command() to
>       scsipi_execute_xs().  Add an assertion that XS_CTL_*_ONSTACK is
>       incompatible with XS_CTL_ASYNC.
>    b) Move assertions from *_scsipi_cmd() to scsipi_execute_xs().
>    c) Call scsipi_execute_xs() from scsipi_command(), not *_scsipi_cmd() -- now
>       *_scsipi_cmd() is just used for the protocol-specific fixups, and is
>       called from scsipi_execute_xs().
>    d) Do the command block copy in scsipi_execute_xs(), not scsipi_make_xs()
>       (also reduces code size a bit).
> 3) Never do a biodone() in the generic SCSI layer -- always do it in the
>    upper device driver (i.e. from the "done" routine, or the "start" routine
>    if there is an allocation failure).  Pass the error code from
>    scsipi_complete() down to accomplish this.  Now the buf pointer is not used
>    by the low-level SCSI code, and can be made into an opaque device-specific
>    pointer.  (Note: This interface is going to change later.)
> 4) XS_CTL_NOSLEEP is never used without XS_CTL_ASYNC or XS_CTL_POLL, and vice
>    versa.  Therefore, define XS_CTL_NOSLEEP as XS_CTL_ASYNC|XS_CTL_POLL (to
>    avoid changing tests in low-level code), but remove the separate flag.
> 5) Make scsipi_enqueue() never fail.
> 6) Clean up some goofiness in pciide -- get rid of the whole "candisable" path

I also already have removed it in my private tree.

>    (it's gratuitous)

Not completely. I did it for the cmd 0640 (which is weird in a lot of ways)
but I don't remember why exactly. It may be when I had a box with the
secondary pciide disabled, and a ISA controller as the second IDE port -
at this time "candisable" did disable properly the secondary channel on the
IDE controller and attached the ISA fron-end instead.

But I suspect this stopped working years ago because of some changes,
so it probably can be killed.

> and simplify the code by calling pciide_map_compat_intr()
>    and *_set_modes() from central locations.

I also did this *_set_modes() change in my local tree.

Can we try to coordinate on these changes ?

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 24 ans d'experience feront toujours la difference
--