dkhusema@faui49.uucp (Dirk Husemann) (01/16/89)
Hi!
Just out of curiosity I examined the MINIX ST winchester device driver
on friday night. There's one place in it which I've trouble understanding why
it was done the way it's done ...
All of the following code segments (I hope I don't violate any copy-
right laws with this - but in case I do, I'd be glad if someone (ast ?) would
tell me (email :-)) are extracted from the file kernel/stwini.c.
The/my problem: In the procedure winchester_task() an initialization of
the partition description is done for all minor devices. This is accomplished
via a for-loop over all drives starting with drive == 0 up to < NR_DRIVE (which
currently is 1 ). For each drive a dmagrab() is done and then a do_xfer().
...
/*===========================================================================*
* winchester_task *
*===========================================================================*/
PUBLIC winchester_task()
{
register r, drive, minor, caller, procno;
/*
* Initialize the partition description for all minor devices
*/
for (drive = 0; drive < NR_DRIVES; drive++) {
minor = drive << 3;
/* read sector 0 of the drive and copy the partition info */
pi[minor].pi_size = 1;
dmagrab(WINCHESTER, hdcint);
r = do_xfer(drive, (phys_bytes)&hi, 0L, 1, DISK_READ);
...
}
}
In do_xfer() the function cmdhead() is called with two parameters:
the drive number, and the cmd (HD_RD or HD_WR, depending on wether a read or
write is to be done).
/*===========================================================================*
* do_xfer *
*===========================================================================*/
PRIVATE int do_xfer(drive, address, sector, count, rw)
int drive;
phys_bytes address;
long sector;
int count;
int rw;
{
register r, s, wrbit;
...
IDISABLE();
if (rw == DISK_READ) {
cmdhead(drive, HD_RD); /* command code */
dmaaddr(address); /* DMA address setup */
wrbit = 0;
} else {
dmaaddr(address); /* DMA address setup */
cmdhead(drive, HD_WR); /* command code */
wrbit = WRBIT;
}
Now comes the part which causes the trouble! In cmdhead() the first
command byte for the ACSI bus is written to the dma register dma_data.
According to the book (Jankowski et al., ATARI ST Profibuch, pp. 642-
643) the first command byte has the following structure:
N N N O O O O O
with N N N being the 3 bit targetnumber and O O O O O the 5 bit op code. I
found this information in other books, too, thus it seems to be correct (?).
Thus, given a specific drive number, all one has to do is a left shift of 5:
DMA->dma_data = (short) ( (drive<<5) | (cmd & 0x1F) );
But - in cmdhead() things are done a little bit differently:
}
/*===========================================================================*
* cmdhead/tail *
*===========================================================================*/
PRIVATE cmdhead(drive, cmd)
{
DMA->dma_mode = FDC | HDC;
DMA->dma_data = (short)(((drive>>1)<<5) | (cmd&0x1F));
DMA->dma_mode = FDC | HDC | A0;
}
Here the drive number is shifted 1 to the right (division by 2) and
*then* 5 to the left. I think this is wrong! By doing this your are effectively
reducing the number of drives/targets accessible by the ACSI bus from 8 to 4.
Even worse - drive 1 is treated like drive 0, drive 3 like drive 2, and so on.
Do I miss something here? Am I wrong or is there indeed an error in
cmdhead()? I know, right know only * 1 drive * is supported, but the code does
look like it was written to work for more than 1 drive.
While I'm at it: In cmdtail() things seem to be even worse:
DMA->dma_date = (short) ( ( (drive&1)<<5) | ... ;
Here only * 2 drives * are allowed!
Any comments anybody?
------------------ Smile, tomorrow will be worse! --------------
Email: dkhusema@immd4.informatik.uni-erlangen.de
Or: {pyramid,unido}!fauern!faui45!dkhusema
Mail: Dirk Husemann, Aufsess-Str. 19, D-8520 Erlangen,
(Home) West Germany
(Busi- University of Erlangen-Nuremberg, Computer Science Dep.,
ness) IMMD IV, Martensstr. 1, D-8520 Erlangen, West Germany
Phone: (Home) +49 9131 302036, (Business) +49 9131 857908
-- Beam me up, Scotty, there's no intelligent life down here! --
--------------- My opinions are mine, mine, mine ---------------johan@nlgvax.UUCP (Johan Stevenson) (01/19/89)
In article <805@faui10.informatik.uni-erlangen.de> dkhusema@faui49.uucp (Dirk Husemann) writes: > > Hi! > > Just out of curiosity I examined the MINIX ST winchester device driver >on friday night. There's one place in it which I've trouble understanding why >it was done the way it's done ... > >... Although I admit I haven't tested it with more than one drive ever, I still believe the code to be correct. The comment in the file stwini.c explains the reason, although rather terse. The current driver supports up to 8 controllers, each having up to 2 drives, each with 8 partitions (4 real partitions, 4 fake). A minor device number contains the following bit fields: 0 C C C D P P P where C=controller, D=drive_on_controller, P=partition_on_drive. A variable 'drive' contains the following bit fields: 0 0 0 0 C C C D In cmdhead() one needs to specify the controller, so (drive>>1). In cmdtail() one needs to specify the drive_on_controller, so (drive&1). -- Johan W. Stevenson johan@pcg.philips.nl Philips Research