OSDev.org

The Place to Start for Operating System Developers
It is currently Fri Mar 29, 2024 9:48 am

All times are UTC - 6 hours




Post new topic Reply to topic  [ 14 posts ] 
Author Message
 Post subject: problem with reading and writing to hard disc[solved]
PostPosted: Sat Sep 01, 2018 6:14 am 
Offline
Member
Member

Joined: Sat Mar 10, 2018 10:16 am
Posts: 296
Good day, I have a problem with reading and writing to hard disc. This is my code:
Code:
unsigned char buffer[512];

void read_ata(unsigned long addr) {
    int first=0;
    int second=0;
    int readbuffer=0;

    outb(0x1F1, 0x00);
    outb(0x1F2, 0x01);
    outb(0x1F3, (unsigned char)addr);
    outb(0x1F4, (unsigned char)(addr >> 8));
    outb(0x1F5, (unsigned char)(addr >> 16));
    outb(0x1F6, 0x40);
    outb(0x1F7, 0x20);
    tp("reading");

    for (int idx = 0; idx < 256; idx++)

    {

    first = inb(0x1F0);
    second = inb(0x1F0);

    buffer[idx * 2] = (unsigned char)first;
    buffer[idx * 2 + 1] = (unsigned char)second;

    }

    for(int i=0; i<10; i++) {
    readbuffer=buffer[i];
    tpvar(readbuffer);  //my function for writing to display
    }
}

void write_ata(unsigned long addr, unsigned char byte) {
    int first=0;
    int second=0;
    int readbuffer=0;

    outb(0x1F1, 0x00);
    outb(0x1F2, 0x01);
    outb(0x1F3, (unsigned char)addr);
    outb(0x1F4, (unsigned char)(addr >> 8));
    outb(0x1F5, (unsigned char)(addr >> 16));
    outb(0x1F6, 0x40);
    outb(0x1F7, 0x30);
    tp("writing");

    for(int i=0; i<255; i++) {
    buffer[i]=byte;
    }

    for (int idx = 0; idx < 256; idx++)

    {

    first = buffer[8 + idx * 2] | (buffer[8 + idx * 2 + 1] << 8);

    outb(0x1F0, first);

    }
}


When I call write_ata(1, 10); and read_ata(1); output on display is ten "0". Please where is bug?

_________________
https://github.com/VendelinSlezak/BleskOS


Last edited by Klakap on Sat Sep 08, 2018 9:08 am, edited 1 time in total.

Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Sat Sep 01, 2018 12:44 pm 
Offline
Member
Member
User avatar

Joined: Sat Jan 15, 2005 12:00 am
Posts: 8561
Location: At his keyboard!
Hi,

Klakap wrote:
Please where is bug?


For a start; this code isn't giving the ATA controller time to start processing the command before it expects the command to have completed, and it's not checking the controller's status register to determine if the command failed before assuming the command succeeded.


Cheers,

Brendan

_________________
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Sat Sep 01, 2018 1:15 pm 
Offline
Member
Member

Joined: Sat Mar 10, 2018 10:16 am
Posts: 296
This is my code for detecting:
Code:
void detect_ata(void) {
    int drive=0;
    outb(0x1F3, 0x88);
    drive=inb(0x1F3);

    if(drive==0x88) {
        outb(0x1F6, 0xA0);
        drive = inb(0x1F7);
        tpvar(drive);
        harddisc=DETECTED;
    }

    outb(0x173, 0x88);
    drive=inb(0x173);

    if(drive==0x88) {
        cdrom=DETECTED;
    }

}

_________________
https://github.com/VendelinSlezak/BleskOS


Last edited by Klakap on Tue Sep 04, 2018 7:16 am, edited 1 time in total.

Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Sat Sep 01, 2018 2:35 pm 
Offline
Member
Member
User avatar

Joined: Sat Jan 15, 2005 12:00 am
Posts: 8561
Location: At his keyboard!
Hi,

Klakap wrote:
This is my code for detecting:


Where is your code to detect if the ATA controller exists at all; then reset/initialise it properly?

Where is your code to detect if there are none, one or two devices attached to each ATA controller?

Where is your code to detect if each device that is attached to each ATA controller is a hard disk or CD or something else, and determine which transfer modes it supports, what size it is, etc?

Where is all your error handling to determine if any of these pieces of hardware are faulty and/or respond in an unexpected way?


Cheers,

Brendan

_________________
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Sat Sep 01, 2018 3:33 pm 
Offline
Member
Member
User avatar

Joined: Sat Nov 22, 2014 6:33 pm
Posts: 934
Location: USA
As Brendan states, and which are very valid and needful questions and statements, you do need some more work.

However, as a newbie, which I am sure you are, you probably aren't "excited" about that part yet. The enjoyment from the newbies point of view, is reading an actual sector from the disk; seeing that it worked, and you are able to communicate with the hardware. The rest comes later (hopefully, and with Brendan's statements, a lot sooner than later).

So, after you have detected that there is an actual hard drive at that location, along with a lot of other things, you now have come to the point to read a sector. Let's do that first, so that your curiosity is fulfilled, and your keep the excitement of the project.

Code:
void read_ata(unsigned long addr) {
    int first=0;
    int second=0;
    int readbuffer=0;

    // you might wish to create #defines, such as ATA_FEATURES and use
    // outb(base_addr + ATA_FEATURES, 0x00)
    // instead of the below (then you don't need the comments as sort)
    outb(0x1F1, 0x00);  // write 0x00 to the FEATURES register
    outb(0x1F2, 0x01);  // write 0x01 to the Sector Count register
    outb(0x1F3, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(0x1F4, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(0x1F5, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    outb(0x1F6, 0x40);  // select the drive.  See note [1] below
    outb(0x1F7, 0x20);  // Send command, See note [2] below
    tp("reading");
   
    // see note [3] below
    for (int idx = 0; idx < 256; idx++)

   ... snip

}

[1] Selecting the drive is usually done first, before you set the other registers. For example, what if the selection fails?
[2] As Brendan states, you need to make sure the controller is ready to receive a command first. This kind of comes back to note 1 above, making sure it doesn't fail. Select the drive, then make sure the Abort bit does not become set, i.e.: read the Error register and see that there was no error after the selection of the drive. Then wait for the drive to become ready before you modify any of the other registers as well as sending the command.
[3] You need to wait for the drive to be ready for the transfer. Usually, an interrupt will fire (remembering that an interrupt will fire for every sector, even if you told it to read more than one, you still have to wait for the interrupt at each sector). Read the Status register waiting for the ready bit.

For a proper ATA driver, you need to do a lot more than just read and write to the drive. As Brendan stated, this could be an ATAPI device which requires you to use a different form of command transfer. The technique to see if it is an ATA device (older/most hard drives) or an ATAPI device (some modern hard drives/all CDROM's), you have to send the Identify command, *TWICE*. If you send the ATA Identify to an ATAPI device, it will purposely fail to indicate that it is an ATAPI device. However, you then have to send the ATAPI Identify just to make sure the failure was correct, and not just a general failure. (A interesting note is that the ATA Identify requires you to wait for Ready, while the ATAPI Identify does not...)

Once you have sent the ATA Identify and ATAPI Identify, you can then successfully indicate which type of device it is. You can also get an idea at reset time, due to the register settings after reset. However, you *must* use the Identify technique to be sure.

If it is an ATAPI device, it can use 12- or 16-byte command blocks, something you will have to extract from the buffer returned and use throughout your ATAPI driver.

So, for now, and to keep your interest alive, add the few things noted above to your read code, checking for controller/device ready, and you should be able to read a sector. Once you have this done, you should, as Brendan has suggested:
- scan through the PCI space looking for ATA controllers.
- is the controller in Legacy mode or Native mode?
- reset the controller. (remember that resetting one channel will reset the other channel depending on the combination of the drives attached. i.e.: IIRC, resetting the Slave Channel with no Master attached resets both, but resetting the Master with a Slave attached does not, or visa-versa. Read the specs.)
- detect if a drive is attached on each channel. A PCI controller will have up to two channels, two drives each.
- if a drive is detected, detect if ATA or ATAPI (using the Identify commands)
- Now you can see if the drive supports CHS only, LBA, or long LBA addressing (28-bit or 48 bit).
- Does the drive support DMA transfers? Are they PCI Bus Mastering DMA or old ISA DMA transfers?
- How many cylinders, heads, sectors does the drive have, now usually only LBAs are important. Which field of the Identify buffer do you use for count of LBAs (there are a few of them).
- Does the drive expect a "Spin me up" command before you can use it.

There are a lot of things you must do for a proper ATA(PI) driver.

If you wish, I have a book at http://www.fysnet.net/media_storage_devices.htm that explains all of this. However, if this isn't of interest to you, please continue to post here. We will help all we can, *as long as you try first, yourself*. (We don't do homework)

- Ben

P.S. Just for fun, a few days ago I plugged in an old Seagate ST351A/X drive I have. This was an interesting drive of its time. It (supposedly) only had one platter (2 heads) with top-of-the-line formatting/etc. to allow a whopping 40Meg of space to be used on one platter. However, it used less-than-the-best actuator tech to do it... The Identify command returns that it has 5 heads, not 2, and though it isn't completely insane, usually the head count is an even count, one per side. However, there is a difference between physical head count and firmware head count in a lot of devices... It is interesting to see what was used at the dawn of hard drives to squeeze out every megabyte. Today it is Terabytes.... Also, the A/X part meant that it has a jumper that you could use it on an XT or an AT machine (difference was 8- or 16-bit transfers).


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Mon Sep 03, 2018 8:18 am 
Offline
Member
Member

Joined: Sat Mar 10, 2018 10:16 am
Posts: 296
Thank you for the response. I am modified the code for detecting ata. But I tried your code read_ata() do method write_ata() but it does not work. Please can you help me?. I use this code:
Code:
#define UNDETECTED 0
#define DETECTED 1

#define PRIMARY 0xA0
#define SECONDARY 0xB0

unsigned char harddisc=UNDETECTED;
unsigned char cdrom=UNDETECTED;

unsigned char buffer[512];

unsigned char ata_error=UNDETECTED;
int harddisc_primary=UNDETECTED;
int harddisc_secundary=UNDETECTED;
int cdrom_primary=UNDETECTED;
int cdrom_secundary=UNDETECTED;
int drive=0;

void detect_ata(void) {
    /* detecting hard disc */
    outb(0x1F3, 0x88);
    drive=inb(0x1F3);

    if(drive==0x88) {
        //detecting primary
        outb(0x1F6, 0xA0);
        //flush
        outb(0x1F2, 0);
        outb(0x1F3, 0);
        outb(0x1F4, 0);
        outb(0x1F5, 0);
        //identify command
        outb(0x1F7, 0xEC);
        //sleep();
        drive=inb(0x1F7);   // read the status port
        if (drive > 0) {
            harddisc_primary=DETECTED;
            tp("primary");
        }
       
        //detecting secundary
        outb(0x1F6, 0xB0);
        //flush
        outb(0x1F2, 0);
        outb(0x1F3, 0);
        outb(0x1F4, 0);
        outb(0x1F5, 0);
        //identify command
        outb(0x1F7, 0xEC);
        //sleep();
        drive=inb(0x1F7);   // read the status port
        if (drive > 0) {   // see if the busy bit is set
            harddisc_secundary=DETECTED;
            tp("secundary");
        }

        outb(0x3F6, 0x02);
        harddisc=DETECTED;
    }

    /* detecting cdrom */
/*This code now I don't use
    outb(0x173, 0x88);
    drive=inb(0x173);

    if(drive==0x88) {
        //detecting primary
        outb(0x176, 0xA0);
        //flush
        outb(0x172, 0);
        outb(0x173, 0);
        outb(0x174, 0);
        outb(0x175, 0);
        //identify command
        outb(0x177, 0xEC);
        //sleep();
        drive=inb(0x177);   // read the status port
        if (drive > 0) {   // see if the busy bit is set
            cdrom_primary=DETECTED;
            tp("primary cdrom");
        }
       
        //detecting secundary
        outb(0x176, 0xB0);
        //flush
        outb(0x172, 0);
        outb(0x173, 0);
        outb(0x174, 0);
        outb(0x175, 0);
        //identify command
        outb(0x177, 0xEC);
        //sleep();
        drive=inb(0x177);   // read the status port
        if (drive > 0) {   // see if the busy bit is set
            cdrom_secundary=DETECTED;
            tp("secundary cdrom");
        }

        //flush cache
        outb(0x1F7, 0xE7);
        cdrom=DETECTED;
    }*/

}

void read_ata(unsigned long addr, unsigned char drive) {
    int first=0;
    int second=0;
    int readbuffer=0;

    if(drive==PRIMARY && harddisc_primary==UNDETECTED) { ata_error=DETECTED; return; }
    if(drive==SECONDARY && harddisc_secundary==UNDETECTED) { ata_error=DETECTED; return; }

    drive=inb(0x1F7);
    if(drive==0xFF) {
        ata_error=DETECTED;
        tp("ata error - undetected");
        return;
    }
    // you might wish to create #defines, such as ATA_FEATURES and use
    // outb(base_addr + ATA_FEATURES, 0x00)
    // instead of the below (then you don't need the comments as sort)
    outb(0x1F6, 0xE0);
    outb(0x1F1, 0x00);  // write 0x00 to the FEATURES register
    outb(0x1F2, 0x01);  // write 0x01 to the Sector Count register
    outb(0x1F3, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(0x1F4, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(0x1F5, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    outb(0x1F6, 0x40);  // select the drive.  See note [1] below
    outb(0x1F7, 0x20);  // Send command, See note [2] below
    tp("reading");
   
    for (int idx = 0; idx < 256; idx++)

    {

    first = inb(0x1F0);
    second = inb(0x1F0);

    buffer[idx * 2] = (unsigned char)first;

    //buffer[idx * 2 + 1] = (unsigned char)(tmpword >> 8);

    buffer[idx * 2 + 1] = (unsigned char)second;

    }

    for(int i=0; i<10; i++) {
    readbuffer=buffer[i];
    tpvar(readbuffer);
    }

}

void write_ata(unsigned long addr, unsigned char byte, unsigned char drive) {
    int first=0;
    int second=0;
    int readbuffer=0;

    if(drive==PRIMARY && harddisc_primary==UNDETECTED) { ata_error=DETECTED; tp("ata error"); return; }
    if(drive==SECONDARY && harddisc_secundary==UNDETECTED) { ata_error=DETECTED; tp("ata error"); return; }

    drive=inb(0x1F7);
    if(drive==0xFF) {
        ata_error=DETECTED;
        tp("ata error - undetected");
        return;
    }
    // you might wish to create #defines, such as ATA_FEATURES and use
    // outb(base_addr + ATA_FEATURES, 0x00)
    // instead of the below (then you don't need the comments as sort)
    outb(0x1F6, 0xE0);
    outb(0x1F1, 0x00);  // write 0x00 to the FEATURES register
    outb(0x1F2, 0x01);  // write 0x01 to the Sector Count register
    outb(0x1F3, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(0x1F4, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(0x1F5, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    outb(0x1F6, 0x40);  // select the drive.  See note [1] below
    outb(0x1F7, 0x30);  // Send command, See note [2] below
    tp("writing");
   
    for(int i=0; i<255; i++) {
    buffer[i]=byte;
    }

    for (int idx = 0; idx < 256; idx++)

    {

    first = buffer[8 + idx * 2] | (buffer[8 + idx * 2 + 1] << 8);
    //clear flush
    outb(0x1F7, 0xE7);

    outb(0x1F0, first);

    }

}

_________________
https://github.com/VendelinSlezak/BleskOS


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Mon Sep 03, 2018 11:55 am 
Offline
Member
Member
User avatar

Joined: Fri Oct 27, 2006 9:42 am
Posts: 1925
Location: Athens, GA, USA
If you don't mind me interjecting a suggestion that is, well, not directly relevant as far as I can tell, but which follows up on Ben's comment about "you might wish to create #defines, [...]", in a way which might help reduce the code redundancy, as well as make it easier to read, debug, and maintain. While the example given below (not tested, so take it as a guide rather than given code) goes a bit further than just that, I hope it will give you a direction that would help. A significant portion of this is taken from the wiki, specifically the pages PCI IDE Controller and wiki]ATA PIO Mode[/wiki]; I am not an expert on the topic, so I cannot say how accurate the wiki itself is on this, and these are just a quick sketch of how this can be done in any case.

Code:
// ata.h - header file for ATA related constants and offsets

// while I normally don't use enums for unordered value sets,
// in this case I want this as a named type.
enum ATA_TYPE {UNKNOWN, HDD, CDROM};

#define ATA_MASTER 0xA0
#define ATA_SLAVE    0xB0


#define MAX_CHANNELS 16 
// this isn't really an adequate approach but it will
// suffice for demonstration purposes

struct ATA_Channel  {
    uint_16 io, ctl;           // the base ports for the I/O and Control regs
    struct {
        ATA_TYPE type;
        unint_8 buffer[512];  // a poor use of space, but it does mean
                                         // that each drive has a separate buffer
        bool detected;
   } drives[2];
   bool present;                 // only needed if using fixed array of channels
};

// FIXME - as a matter of future expansion, you may want
// to set up some sort of probing later on and allocate this
// array dynamically instead
struct ATA_Channel drive_channels[MAX_CHANNELS];

// ********** as per the wiki
// Status register bitmasks
// used with the values read from either the
// ATA I/O 'status' port, or the
// ATA Control 'status/command' port
// to get the individual flag value, AND the mask
// against the returned value
#define ATA_SR_BSY     0x80    // Busy
#define ATA_SR_DRDY    0x40    // Drive ready
#define ATA_SR_DF      0x20    // Drive write fault
#define ATA_SR_DSC     0x10    // Drive seek complete
#define ATA_SR_DRQ     0x08    // Data request ready
#define ATA_SR_CORR    0x04    // Corrected data
#define ATA_SR_IDX     0x02    // Index
#define ATA_SR_ERR     0x01    // Error

// ********** as per the wiki
// returned error values from the Status/Command regs
#define ATA_ER_BBK      0x80    // Bad block
#define ATA_ER_UNC      0x40    // Uncorrectable data
#define ATA_ER_MC       0x20    // Media changed
#define ATA_ER_IDNF     0x10    // ID mark not found
#define ATA_ER_MCR      0x08    // Media change request
#define ATA_ER_ABRT     0x04    // Command aborted
#define ATA_ER_TK0NF    0x02    // Track 0 not found
#define ATA_ER_AMNF     0x01    // No address mark

// ********** as per the wiki
// ATA commands for the Control register's
// Status/Command port
#define ATA_CMD_READ_PIO          0x20
#define ATA_CMD_READ_PIO_EXT      0x24
#define ATA_CMD_READ_DMA          0xC8
#define ATA_CMD_READ_DMA_EXT      0x25
#define ATA_CMD_WRITE_PIO         0x30
#define ATA_CMD_WRITE_PIO_EXT     0x34
#define ATA_CMD_WRITE_DMA         0xCA
#define ATA_CMD_WRITE_DMA_EXT     0x35
#define ATA_CMD_CACHE_FLUSH       0xE7
#define ATA_CMD_CACHE_FLUSH_EXT   0xEA
#define ATA_CMD_PACKET            0xA0
#define ATA_CMD_IDENTIFY_PACKET   0xA1
#define ATA_CMD_IDENTIFY          0xEC

// ********** as per the wiki
// values returned by ATA identify command?
#define ATA_IDENT_DEVICETYPE   0
#define ATA_IDENT_CYLINDERS    2
#define ATA_IDENT_HEADS        6
#define ATA_IDENT_SECTORS      12
#define ATA_IDENT_SERIAL       20
#define ATA_IDENT_MODEL        54
#define ATA_IDENT_CAPABILITIES 98
#define ATA_IDENT_FIELDVALID   106
#define ATA_IDENT_MAX_LBA      120
#define ATA_IDENT_COMMANDSETS  164
#define ATA_IDENT_MAX_LBA_EXT  200

// Because these next two are ordered value sets
// (specifically, port offset sequences), I am
// defining two enum types for the following
// groups of constants
drive_channels[0].master.
// I/O Port offsets - the offset for each of the ports
// corresponding to the ATA registers, relative
// to the base (data) port of the ATA controller
// use these in conjunction with the controller base.
// offsets postfixed with '_8' are 8-bit both LBA modes
// prefix 'r' - read meaning, 'w' - write meaning
// postfixed '_16' are 16-bit in both LBA modes
// all others are 8 bit LBA28/16 bit LBA48

enum ATA_IO_REG {
    read_16,                   
    rErrors_wFeatures,
    sector_count,
    LBA_sector_num,
    cyl_lo,
    cyl_hi,
    drv_head_8,
    rStatus_wCmd_8 
};

// Control Port offsets.
//  these are 8-bit ports in both LBA modes.
// the base is both an alternate status when read
// and for sending ATA commands when written.
// While it is probably overkill to use an enum
// for this, doing so makes both the I/O
// and Command ports consistent.

enum ATA_CTL_REG {
    rStatus_wCmd,drive_channels[ch]
    drv_head_sel
};


Now let's apply that to what we already have:
Code:


void ata_init(void)
{
    uint_16 drive_type;

    // mostly, we're just setting the default channel values
    // with the most common values
    // TODO - find way to get dynamically at runtime
    drive_channels[0].io = 0x01f0;
    drive_channels[0].ctl = 0x03f6;

    drive_channels[1].io = 0x0170;
    drive_channels[1].ctl = 0x0376; 
    // .... any further ones needed later

    for (uint_8 ch = 0; ch < MAX_CHANNELS; ch++)
    {
         for (uint_8 drv = 0; drv < 2; drv ++)
         {
             drive_channels[ch].drives[drv].type = UNKNOWN;
             drive_channels[ch].drives[drv].detected = false;
             memset(drive_channels[ch].drives[drv].buffer, 0, sizeof(uint_8));
             outb(drive_channels[ch].io | ATA_IO_REG.LBA_sector_count, );
           


I need to step away, and in any case I expect I've already made several mistakes (not counting the incomplete code line at the end), but I'll finish this later. I think you can see where I am going anyway.

_________________
Rev. First Speaker Schol-R-LEA;2 LCF ELF JAM POEE KoR KCO PPWMTF
Ordo OS Project
Lisp programmers tend to seem very odd to outsiders, just like anyone else who has had a religious experience they can't quite explain to others.


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Mon Sep 03, 2018 3:16 pm 
Offline
Member
Member
User avatar

Joined: Fri Oct 27, 2006 9:42 am
Posts: 1925
Location: Athens, GA, USA
OK, before picking up where I left off, I need to apologize for a number of careless errors in the previous code, mostly instances where I got confused about some details of C, such all the places I wrote 'uint_8' and 'uint_16' instead of 'uint8_t' and 'uint16_t', or some of the dumb mistakes I made with the enums. Sorry for that, it has been a while.

_________________
Rev. First Speaker Schol-R-LEA;2 LCF ELF JAM POEE KoR KCO PPWMTF
Ordo OS Project
Lisp programmers tend to seem very odd to outsiders, just like anyone else who has had a religious experience they can't quite explain to others.


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Mon Sep 03, 2018 3:53 pm 
Offline
Member
Member
User avatar

Joined: Sat Nov 22, 2014 6:33 pm
Posts: 934
Location: USA
I don't think you are understanding what we are trying to tell you.

Klakap wrote:
this code:
Code:
void read_ata(unsigned long addr, unsigned char drive) {

    outb(0x1F6, 0xE0);

1) Before selecting the drive, you need to see if it is busy or not. Read the Status register. If the BSY or DRQ bits are set, the controller isn't ready to select a drive.
2) After selecting the drive, by the register write above, you must wait 400ns to 1000ns. Two reads from the ALT_Status register is enough of a wait.
3) Then you *must* read from the Status register (not the Alt_status register) to clear any pending interrupts that may have occurred due to the selection or other action (you may ignore the value read).
4) Then you read the Status register again, checking the BSY and DRQ bits. If either is set, you didn't select the drive....

Note to your future self, and any others reading this: Since selecting the drive can take up to 1mS to complete, using the above technique, this can put a definite delay in your code if you keep selecting the same drive over and over again. Keep a running tab of your current selection register and if trying to set it to that same value, just return. i.e.: Don't "select" the drive if it is already selected.

Klakap wrote:
Code:
    outb(0x1F1, 0x00);  // write 0x00 to the FEATURES register
    outb(0x1F2, 0x01);  // write 0x01 to the Sector Count register
    outb(0x1F3, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(0x1F4, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(0x1F5, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    outb(0x1F6, 0x40);  // select the drive.  See note [1] below
    outb(0x1F7, 0x20);  // Send command, See note [2] below
    tp("reading");

Once you have sent the command, you have to do one of two things:
1) you *must* wait for the interrupt to fire *before* you can start reading from the Data register.
2) you *must* poll the DRQ bit to see if data is ready to be read, before reading from the Data register.

You *must* do one or the other. You cannot just start reading data.

Klakap wrote:
Code:
    for (int idx = 0; idx < 256; idx++)

    {

    first = inb(0x1F0);
    second = inb(0x1F0);

    buffer[idx * 2] = (unsigned char)first;

    //buffer[idx * 2 + 1] = (unsigned char)(tmpword >> 8);

    buffer[idx * 2 + 1] = (unsigned char)second;
    .
    .
    .


Does this help any?
Ben


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Mon Sep 03, 2018 4:23 pm 
Offline
Member
Member
User avatar

Joined: Sat Nov 22, 2014 6:33 pm
Posts: 934
Location: USA
A few more notes:
1) You can read words from the drive. Most any drive after around 1980 or so will handle 16-bit word transfers. In fact, most any drive after 1990 or so can handle 32-bit dword transfers, though you really should check for this first before assuming so.
2) You don't need a separate read() and write() function. The exact same code is used for both *except* the command sent and the inpw() or outpw() call. For example, my transfer_sector code, using ATA transfers, is roughly 220 lines of C code. However, I handle CHS only, LBA, large LBA, and PIO and DMA transfers. This 220 lines of code doesn't include the setup, initiate, or stop of the DMA controller, the port I/O, or the testing for controller readiness. So out of nearly 300 lines of code, only the command sent and the direction of the data is different between reads and writes. No need to have 300 lines for read *and* another 300 lines for write.
3) Your ATAPI transfers can take note 2) above as well. No need for separate read and write functions.
4) If you poll the DRQ bit for data ready, this is perfectly fine. However, your single tasking environment will wait forever for that DRQ bit (1000uS is forever in this line of work). Eventually, you will want to implement interrupt handling, allowing the processor to move on to another task while waiting for the drive to return the data. i.e.: put this task to sleep allowing the interrupt to wake it.

For this code and probably many other parts of your project, I suggest placing debug macros in your code. For example:
Code:
   ... I am doing something here
   ... and here, but I would like to see what is happening
   ATA_DEBUG_OUT(("this is a standard printf() type string out function: example 123 = %i", 123));
   ... more code
   ... more code


Then define the ATA_DEBUG_OUT macro as such:
Code:
// uncomment this line to show debug messages
//#define ATA_DO_DEBUG

#ifdef ATA_DO_DEBUG
  #define ATA_DEBUG_OUT(x) { printf x; puts(""); }
#else
  #define ATA_DEBUG_OUT(x)
#endif

If the ATA_DO_DEBUG macro is defined, the compiler will place all of the code for ATA_DEBUG_OUT within your code. You will see the output. If the ATA_DO_DEBUG macro is not defined, the compiler will *not* include the debug code in the final release. Please note that the initiating macro *must* have two opening '((' and and closing '))' each.

Ben
- http://www.fysnet.net/osdesign_book_series.htm


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Tue Sep 04, 2018 7:58 am 
Offline
Member
Member

Joined: Sat Mar 10, 2018 10:16 am
Posts: 296
Very thank you for the answers. Modified I in the detection of ata:
Code:
if (drive > 0) {
            harddisc_primary=DETECTED;
            first=inb(0x1F4);
            second=inb(0x1F5);
           if (first==0x14 && second==0xEB) { tpvar(0); return ATADEV_PATAPI; }
           if (first==0x69 && second==0x96) { tpvar(1); return ATADEV_SATAPI; }
           if (first==0 && second == 0) { tpvar(2); return ATADEV_PATA; }
           if (first==0x3c && second==0xc3) { tpvar(3); return ATADEV_SATA; }
            //ata is unkown
            tpvar(first);
            tpvar(second);
            return ATADEV_UNKOWN;
}

And I made for reading and writing one method ata_sector, but writing still does not work.
Code:
void ata_sector(unsigned char type, unsigned long addr, unsigned char drive, unsigned char byte) {
    int first=0;
    int second=0;
    int readbuffer=0;

    if(drive==PRIMARY && harddisc_primary==UNDETECTED) { ata_error=DETECTED; return; }
    if(drive==SECONDARY && harddisc_secundary==UNDETECTED) { ata_error=DETECTED; return; }

    drive=inb(0x1F7);
    if(drive==0xFF) {
        ata_error=DETECTED;
        tp("ata error - undetected");
        return;
    }

    // you might wish to create #defines, such as ATA_FEATURES and use
    // outb(base_addr + ATA_FEATURES, 0x00)
    // instead of the below (then you don't need the comments as sort)
    // wait for drive is ready
    while (((inb(0x1F7) & ATA_DRQ)!=ATA_DRQ)) {}
    first=inb(0x1F0);
    first=inb(0x1F0);
    first=inb(0x1F7);
    outb(0x1F6, 0xE0);
    while (((inb(0x1F7) & ATA_DRQ)==0)) {}

    outb(0x1F6, 0xE0);
    outb(0x1F1, 0x00);  // write 0x00 to the FEATURES register
    outb(0x1F2, 0x01);  // write 0x01 to the Sector Count register
    outb(0x1F3, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(0x1F4, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(0x1F5, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    outb(0x1F6, 0x40);  // select the drive.  See note [1] below
    //type of method
    if(type==ATA_READ) {
    outb(0x1F7, 0x20);  // Send command, See note [2] below
    }
    else {
    outb(0x1F7, 0x30);
    }

    //writing
    if(type==ATA_WRITE) {
    for(int i=0; i<255; i++) {
    buffer[i]=byte;
    }
    }

    // wait
    while (((inb(0x1F7) & ATA_DRQ)==0)) {}
    tp("ata reading/writing");
   
    for (int idx = 0; idx < 256; idx++)
    {
    if(type==ATA_READ) {
    first = inb(0x1F0);
    second = inb(0x1F0);
    buffer[idx * 2] = (unsigned char)first;
    buffer[idx * 2 + 1] = (unsigned char)second;
    }
    else {
    first = buffer[8 + idx * 2] | (buffer[8 + idx * 2 + 1] << 8);
    //clear flush
    outb(0x1F7, 0xE7);
    outb(0x1F0, first);
    }
    }

    if(type==ATA_READ) {
    for(int i=0; i<10; i++) {
    readbuffer=buffer[i];
    tpvar(readbuffer);
    }
    }
}

Please, what I doing bad?
PS I found that the Virtualbox virtual hard disk is a type of PATA.
Edit: I have added the method of chs() which use the chs reads 20 bytes from the first sector and I found a very interesting thing: if I write
Code:
int ata=detect_ata();
ata_sector(ATA_WRITE, 1, PRIMARY, someNumber);
chs();

and I run Virtualbox, chs display 0. Then, when the code do I change to
Code:
int ata=detect_ata();
chs();

on the screen it's a someNumber.

_________________
https://github.com/VendelinSlezak/BleskOS


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Tue Sep 04, 2018 12:32 pm 
Offline
Member
Member
User avatar

Joined: Sat Nov 22, 2014 6:33 pm
Posts: 934
Location: USA
I only have a minute, and I quickly read through your code, but noticed... You need to wait for the DRQ for a write as well. The DRQ bit states that the drive is ready for transfer, whether it be from the drive to the host, or from the host to the drive.

Again, except for a few minor things, the difference between a read and a write is two things: The command and the direction of the I/O. Period. Everything else is the same.

If I get a chance, I will come back and read through your stuff a little more.

Ben


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Wed Sep 05, 2018 2:22 pm 
Offline
Member
Member
User avatar

Joined: Sat Nov 22, 2014 6:33 pm
Posts: 934
Location: USA
Code:
void ata_sector(unsigned char type, unsigned long addr, unsigned char drive, unsigned char byte) {
    int first=0;
    int second=0;
    int readbuffer=0;

    // you might wish to create #defines, such as ATA_FEATURES and use
    // outb(base_addr + ATA_FEATURES, 0x00)

I strongly suggest that you take my (and another reader's) suggestion above and add #defines to your code. I have programmed for the ATA for quite a few years but if I am away from it for a while, I still have to go look up addresses such as:
Code:
    outb(0x1F6, 0xE0);

where as
Code:
    outb(base_address + ATA_DRV_HEAD, 0xE0);

Is so much easier to read and understand the code.

Code:
    while (((inb(0x1F7) & ATA_DRQ)!=ATA_DRQ)) {}
If this is the first of your code, you might be waiting a while, a long while.

Code:
    first=inb(0x1F0);
    first=inb(0x1F0);
    first=inb(0x1F7);
    outb(0x1F6, 0xE0);
    while (((inb(0x1F7) & ATA_DRQ)==0)) {}

    outb(0x1F6, 0xE0);

You select the drive above, waiting for the DRQ bit to be zero (when you should be checking the ERR bit instead) and then select the drive again. Why?

Code:
    outb(0x1F1, 0x00);  // write 0x00 to the FEATURES register
    outb(0x1F2, 0x01);  // write 0x01 to the Sector Count register
    outb(0x1F3, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(0x1F4, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(0x1F5, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    outb(0x1F6, 0x40);  // select the drive. 
And select the drive again???

Code:
    if(type==ATA_READ) {
    outb(0x1F7, 0x20);  // Send command
    }
    else {
    outb(0x1F7, 0x30);
    }

    //writing
    if(type==ATA_WRITE) {
    for(int i=0; i<255; i++) {
    buffer[i]=byte;
    }
    }

What are you doing there? Filling in the buffer with a byte? (half of the buffer at that) Shouldn't the buffer already be set before you call the function?

Code:
    // wait
    while (((inb(0x1F7) & ATA_DRQ)==0)) {}

Yes, most of the time the DRQ will become set when the drive is ready to receive/send data. However, you might be waiting an awful long time if there is an error. Try a time-out.

Code:
    tp("ata reading/writing");
   
    for (int idx = 0; idx < 256; idx++)
    {
    if(type==ATA_READ) {
    first = inb(0x1F0);
    second = inb(0x1F0);
    buffer[idx * 2] = (unsigned char)first;
    buffer[idx * 2 + 1] = (unsigned char)second;


buffer[idx * 2] = inw(base_addr + ATA_DATA); // granted that won't compile, but do you see the idea???

Code:
    }
    else {
    first = buffer[8 + idx * 2] | (buffer[8 + idx * 2 + 1] << 8);
    //clear flush
    outb(0x1F7, 0xE7);
    outb(0x1F0, first);
    }
    }

What is with the "clear flush" command? Are you sending a command while the drive is executing a command already? This isn't good....

Code:
    if(type==ATA_READ) {
    for(int i=0; i<10; i++) {
    readbuffer=buffer[i];
    tpvar(readbuffer);
    }
    }
}


What do you have so far? How are you doing with it?

Ben


Top
 Profile  
 
 Post subject: Re: problem with reading and writing to hard disc
PostPosted: Sat Sep 08, 2018 9:04 am 
Offline
Member
Member

Joined: Sat Mar 10, 2018 10:16 am
Posts: 296
Sorry, I read my code non carefully. Here is the corrected code(it working):
Code:
void ata_sleep(uint32_t type, uint32_t byte, uint32_t var) {
    uint32_t first=0;
    ata_error=UNDETECTED;

    for(uint32_t i=0; i<100; i++) {
        first=inb(ata_base+ATA_PORT_DATA);
        if(type==ATA_EQUALS) {
            if( (inb(ata_base+ATA_PORT_STATUS) & byte)==var ) {
                return;
            }
        }
       
        if(type==ATA_NON_EQUALS) {
            if( (inb(ata_base+ATA_PORT_STATUS) & byte)!=var ) {
                return;
            }
        }
    }
    // ata doesnt respond
    ata_error=DETECTED;
    return;
}

void ata_sector(uint16_t base, uint8_t type, uint64_t addr, uint8_t drive) {
    uint16_t tmpword=0;

    ata_base=base;

    ata_error=UNDETECTED;

    if(drive==ATA_MASTER && harddisc_master==UNDETECTED) { ata_error=DETECTED; return; }
    if(drive==ATA_SLAVE && harddisc_slave==UNDETECTED) { ata_error=DETECTED; return; }

    drive=inb(ata_base+ATA_PORT_STATUS);
    if(drive==0xFF) {
        ata_error=DETECTED;
        tp("ata error - undetected");
        return;
    }

    // wait for drive is ready
    ata_sleep(ATA_EQUALS, ATA_SR_DRQ, ATA_SR_DRQ); if(ata_error==DETECTED) { tp("ata error 1"); return; }

    tmpword=inb(ata_base+ATA_PORT_STATUS);
    if(drive==ATA_MASTER) {
    outb(ata_base+ATA_PORT_DRV, 0xE0);
    }
    else { // ATA_SLAVE
    outb(ata_base+ATA_PORT_DRV, 0xF0);
    }

    if ( ( (inb(ATA_PORT_STATUS) & ATA_SR_ERR)==1 ) ) { ata_error==DETECTED; tp("ata error 2"); return;}

    //PIO28
    outb(ata_base+ATA_PORT_FEATURES, 0x00);  // write 0x00 to the FEATURES register
    outb(ata_base+ATA_PORT_SCT_COUNT, 0x01);  // write 0x01 to the Sector Count register
    outb(ata_base+ATA_PORT_SCT_NUMBER, (unsigned char)addr);  // Sector number or LBA Low, most likely LBA Low (but see comments below)
    outb(ata_base+ATA_PORT_CYL_LOW, (unsigned char)(addr >> 8));  // Cyl Low number or LBA Mid
    outb(ata_base+ATA_PORT_CYL_HIGH, (unsigned char)(addr >> 16));  // Cyl High number or LBA High
    //type
    if(type==ATA_READ) {
    outb(ata_base+ATA_PORT_COMMAND, ATA_CMD_READ_PIO);  // Send command
    }
    else {
    outb(ata_base+ATA_PORT_COMMAND, ATA_CMD_WRITE_PIO);
    }

    // wait
    ata_sleep(ATA_NON_EQUALS, ATA_SR_DRQ, 0); if(ata_error==DETECTED) { tp("ata error 3"); return; }
   
    for (int idx = 0; idx < 256; idx++)
    {
    if(type==ATA_READ) {
    buffer[idx] = inw(ata_base + ATA_PORT_DATA);
    }
    else {
    tmpword=(unsigned short)buffer[idx];
    outw(ata_base + ATA_PORT_DATA, tmpword);
    }
    }
}


Very thank you for the help! Now, I'm going to program the ATA DMA.

_________________
https://github.com/VendelinSlezak/BleskOS


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 14 posts ] 

All times are UTC - 6 hours


Who is online

Users browsing this forum: Majestic-12 [Bot] and 166 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group