First let me apologize for not getting back with you on the other thread. I only get around here about once a week or so.
Second, zity has made a few good comments and hopefully has pointed you in the correct direction, though I would like to emphasize his/her remarks about using #defines. It sure makes it a lot easier.
Anyway, let me comment within your code.
Very thank you! This is my code:
Code:
// You don't need these. Your 'drive' parameter is either 0 or 1, is it not?
//#define ATAPI_MASTER 0xE0
//#define ATAPI_SLAVE 0xF0
#define ATAPI_PIO_MODE 0
#define ATAPI_DMA_MODE 1
// You should really send the base address along with the call (as you do now). These two values below
// are for Legacy drives only and could be anything on modern machines...
//#define ATAPI_PRIMARY 0x1F0
//#define ATAPI_SECONDARY 0x170
// You only need 2048 bytes, not 2048 words, but too much is always better than too little.
uint16_t cdrom_buffer[2048];
// if you use the BOOL typedef here, the compiler will use the "best" sized variable for a "TRUE" and "FALSE"
volatile uint8_t ide_secondary_interrupt=0;
// see comments below for why you need this
uint8_t global = 0xFF;
// sector is probably a zero based lba, correct?
// base is probably ATAPI_PRIMARY or ATAPI_SECONDARY, correct?
// drive is probably 0 or 1, hopefully?
void atapi_read(uint32_t sector, uint16_t base, uint8_t drive) {
//select drive
// As zity mentioned, you should check to see if it actually was selected successfully
// Also, I have mentioned before, selecting the drive takes time. You should have a
// 'global' variable to hold the value written below. This way if the drive has already
// been selected, you don't select it again.
uint8_t select = 0xE0 | (drive << 4) | ( (sector & 0x0F000000) >> 24);
if (global != select) {
outb(base+6, select);
mdelay(2); // delay 2mS
global = select;
}
// Also, since you are sending to an ATAPI device, using a 12-byte/16-byte packet, there
// is no such thing as a 28-bit LBA command. Therefore, the "((sector & 0x0F000000) >> 24)" is redundant.
// now you should wait for the controller to not by busy. Again, selecting the drive takes time...
//Use PIO
outb(base+1, ATAPI_PIO_MODE);
// wouldn't the following look a lot better and easier to understand when reading it?
//#define ATA_FEATURES 0x001 // wpc4/features register
//outb(base + ATA_FEATURES, ATAPI_PIO_MODE);
//Maximal byte count
outb(base+4, (1024 & 0xff));
outb(base+5, (1024 >> 8));
// the above (as zity mentioned) should be 2048 since it is a "byte" count...
// the SECTOR register ( offset 2 ) should be zero
// the LOW BYTE register ( offset 3 ) is not used, but should still be zero
//ATAPI packet command
outb (base+7, 0xA0);
//Wait
while( (inb(base+7) & 0x88)!=0x08 ) {}
// again, what if the controller never becomes non-busy or the DRQ bit never becomes active????
//Command set
//outw(base+0, 0xA8);
//outw(base+0, ((sector >> 8) & 0xFF00) | ((sector >> 24) & 0xFF));
//outw(base+0, ((sector << 8) & 0xFF00) | ((sector >> 8) & 0xFF));
//outw(base+0, 0);
//outw(base+0, 1);
//outw(base+0, 0);
// Try:
// of coarse placing your packet bytes in a byte array at "packet"
for (i=0; i<ata->packet_size; i+=2)
outpw(base + ATA_DATA, * (uint16_t *) &packet[i]);
// for example, the READ TOC command might look like:
// uint8_t packet[ATAPI_MAX_PACKET_SIZE];
// memset(packet, 0, ATAPI_MAX_PACKET_SIZE);
// packet[ 0] = ATAPI_CMD_READ_TOC;
// if (msf_flag)
// packet[ 1] = 2;
// packet[ 6] = 0;
// packet[ 7] = (uint8_t) ((buflen >> 8) & 0xFF);
// packet[ 8] = (uint8_t) ((buflen >> 0) & 0xFF);
// packet[ 9] = (uint8_t) (format << 6);
// also, please note that this assumes a specific packet type. The IDENTIFY command will
// return the type of packet (SCSI, RBC, etc) that you should send.
//Wait for irq
while( ide_secondary_interrupt==0 ) {}
// what if the IRQ never fires????
// It would be a good idea to check the DRQ bit here as well.
//Read length of input/output
uint16_t size = ( ((unsigned char)(inb(base+5) << 8)) | ((unsigned char)inb(base+4)) );
// ******
// Okay, here is why your size == 0....
// Let's look at what the first few tokens of your code above does:
// ((unsigned char)(inb(base+5) << 8))
// The code will input a byte from the port
// The compiler will then take that value and make it a machine sized type (probably 32-bit)
// The compiler will then shift that value 8 bits to the left.
// We now have a value that takes 16-bits to store.
// * You then cast that value back to a byte, which in turn, zeros the value because you just shifted it to the left 8 bits.......
// Your 'size' will never be greater than 0xFF and since the count of words read is (should be) 2048 which is 0x0800 in hex, the lower half will be zero...
// Hence, this is why you are getting a value of zero as the length....
//
uint16_t length = (size / 2);
// ??????????????
// I don't know what this does....
tp_var(size); //this output is 0
// Write or read data
// You get the length above, why not use it here?
for (int i=0; i<1024; i++) {
cdrom_buffer[i]=inw(base+0);
}
//Prepare for next command
ide_secondary_interrupt=0;
}
It work, but variabile size is 0.