From owner-p4-projects@FreeBSD.ORG Sun Jun 7 11:14:29 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 5F0271065673; Sun, 7 Jun 2009 11:14:29 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 06EC11065679 for ; Sun, 7 Jun 2009 11:14:29 +0000 (UTC) (envelope-from mav@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id E807E8FC1B for ; Sun, 7 Jun 2009 11:14:28 +0000 (UTC) (envelope-from mav@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n57BESY7085552 for ; Sun, 7 Jun 2009 11:14:28 GMT (envelope-from mav@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n57BESW6085550 for perforce@freebsd.org; Sun, 7 Jun 2009 11:14:28 GMT (envelope-from mav@freebsd.org) Date: Sun, 7 Jun 2009 11:14:28 GMT Message-Id: <200906071114.n57BESW6085550@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to mav@freebsd.org using -f From: Alexander Motin To: Perforce Change Reviews Cc: Subject: PERFORCE change 163700 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Jun 2009 11:14:30 -0000 http://perforce.freebsd.org/chv.cgi?CH=163700 Change 163700 by mav@mav_mavbook on 2009/06/07 11:14:02 Change the way in which ATA commands are passed. Instead of 4 formalized fileds, pass full set of ATA registers. It was not so bad idea, but I have found that some ATA commands has different format, and moving formatting code to SIM will lead to code duplication and making code more cryptic. Add field for command result register set. Affected files ... .. //depot/projects/scottl-camlock/src/sys/cam/ata/ata_all.c#6 edit .. //depot/projects/scottl-camlock/src/sys/cam/ata/ata_all.h#6 edit .. //depot/projects/scottl-camlock/src/sys/cam/ata/ata_da.c#7 edit .. //depot/projects/scottl-camlock/src/sys/cam/ata/ata_xpt.c#11 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_ccb.h#19 edit .. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#15 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/cam/ata/ata_all.c#6 (text+ko) ==== @@ -80,4 +80,57 @@ printf(" device\n"); } +void +ata_36bit_cmd(struct ccb_ataio *ataio, uint8_t cmd, uint8_t features, + uint32_t lba, uint8_t sector_count) +{ + bzero(&ataio->cmd, sizeof(ataio->cmd)); + ataio->cmd.flags = 0; + ataio->cmd.command = cmd; + ataio->cmd.features = features; + ataio->cmd.lba_low = lba; + ataio->cmd.lba_mid = lba >> 8; + ataio->cmd.lba_high = lba >> 16; + ataio->cmd.device = 0x40 | ((lba >> 24) & 0x0f); + ataio->cmd.sector_count = sector_count; +} + +void +ata_48bit_cmd(struct ccb_ataio *ataio, uint8_t cmd, uint16_t features, + uint64_t lba, uint16_t sector_count) +{ + bzero(&ataio->cmd, sizeof(ataio->cmd)); + ataio->cmd.flags = CAM_ATAIO_48BIT; + ataio->cmd.command = cmd; + ataio->cmd.features = features; + ataio->cmd.lba_low = lba; + ataio->cmd.lba_mid = lba >> 8; + ataio->cmd.lba_high = lba >> 16; + ataio->cmd.device = 0x40; + ataio->cmd.lba_low_exp = lba >> 24; + ataio->cmd.lba_mid_exp = lba >> 32; + ataio->cmd.lba_high_exp = lba >> 40; + ataio->cmd.features_exp = features >> 8; + ataio->cmd.sector_count = sector_count; + ataio->cmd.sector_count_exp = sector_count >> 8; +} + +void +ata_ncq_cmd(struct ccb_ataio *ataio, uint8_t cmd, + uint64_t lba, uint16_t sector_count) +{ + bzero(&ataio->cmd, sizeof(ataio->cmd)); + ataio->cmd.flags = CAM_ATAIO_48BIT | CAM_ATAIO_FPDMA; + ataio->cmd.command = cmd; + ataio->cmd.features = sector_count; + ataio->cmd.lba_low = lba; + ataio->cmd.lba_mid = lba >> 8; + ataio->cmd.lba_high = lba >> 16; + ataio->cmd.device = 0x40; + ataio->cmd.lba_low_exp = lba >> 24; + ataio->cmd.lba_mid_exp = lba >> 32; + ataio->cmd.lba_high_exp = lba >> 40; + ataio->cmd.features_exp = sector_count >> 8; +} + #endif /* _KERNEL */ ==== //depot/projects/scottl-camlock/src/sys/cam/ata/ata_all.h#6 (text+ko) ==== @@ -34,16 +34,56 @@ union ccb; struct ata_cmd { - u_int8_t command; /* command reg */ u_int8_t flags; /* ATA command flags */ #define CAM_ATAIO_48BIT 0x01 /* Command has 48-bit format */ #define CAM_ATAIO_FPDMA 0x02 /* FPDMA command */ - u_int16_t feature; /* feature reg */ - u_int16_t count; /* count reg */ - u_int64_t lba; /* lba reg */ +#define CAM_ATAIO_CONTROL 0x04 /* Control, not a command */ + + u_int8_t command; + u_int8_t features; + + u_int8_t lba_low; + u_int8_t lba_mid; + u_int8_t lba_high; + u_int8_t device; + + u_int8_t lba_low_exp; + u_int8_t lba_mid_exp; + u_int8_t lba_high_exp; + u_int8_t features_exp; + + u_int8_t sector_count; + u_int8_t sector_count_exp; + u_int8_t control; +}; + +struct ata_res { + u_int8_t flags; /* ATA command flags */ +#define CAM_ATAIO_48BIT 0x01 /* Command has 48-bit format */ + + u_int8_t status; + u_int8_t error; + + u_int8_t lba_low; + u_int8_t lba_mid; + u_int8_t lba_high; + u_int8_t device; + + u_int8_t lba_low_exp; + u_int8_t lba_mid_exp; + u_int8_t lba_high_exp; + + u_int8_t sector_count; + u_int8_t sector_count_exp; }; -void -ata_print_ident(struct ata_params *ident_data); +void ata_print_ident(struct ata_params *ident_data); + +void ata_36bit_cmd(struct ccb_ataio *ataio, uint8_t cmd, uint8_t features, + uint32_t lba, uint8_t sector_count); +void ata_48bit_cmd(struct ccb_ataio *ataio, uint8_t cmd, uint16_t features, + uint64_t lba, uint16_t sector_count); +void ata_ncq_cmd(struct ccb_ataio *ataio, uint8_t cmd, + uint64_t lba, uint16_t sector_count); #endif ==== //depot/projects/scottl-camlock/src/sys/cam/ata/ata_da.c#7 (text+ko) ==== @@ -747,6 +747,10 @@ switch (bp->bio_cmd) { case BIO_READ: case BIO_WRITE: + { + uint64_t lba = bp->bio_pblkno; + uint16_t count = bp->bio_bcount / softc->params.secsize; + cam_fill_ataio(ataio, da_retry_count, dadone, @@ -757,31 +761,34 @@ bp->bio_bcount, da_default_timeout*1000); - ataio->cmd.feature = 0; - ataio->cmd.lba = bp->bio_pblkno; - ataio->cmd.count = bp->bio_bcount / softc->params.secsize; if (softc->flags & DA_FLAG_CAN_NCQ) { - if (bp->bio_cmd == BIO_READ) - ataio->cmd.command = ATA_READ_FPDMA_QUEUED; - else - ataio->cmd.command = ATA_WRITE_FPDMA_QUEUED; - ataio->cmd.flags = CAM_ATAIO_48BIT | CAM_ATAIO_FPDMA; + if (bp->bio_cmd == BIO_READ) { + ata_ncq_cmd(ataio, ATA_READ_FPDMA_QUEUED, + lba, count); + } else { + ata_ncq_cmd(ataio, ATA_WRITE_FPDMA_QUEUED, + lba, count); + } } else if ((softc->flags & DA_FLAG_CAN_48BIT) && - (ataio->cmd.lba + ataio->cmd.count >= ATA_MAX_28BIT_LBA || - ataio->cmd.count >= 256)) { - if (bp->bio_cmd == BIO_READ) - ataio->cmd.command = ATA_READ_DMA48; - else - ataio->cmd.command = ATA_WRITE_DMA48; - ataio->cmd.flags = CAM_ATAIO_48BIT; + (lba + count >= ATA_MAX_28BIT_LBA || + count >= 256)) { + if (bp->bio_cmd == BIO_READ) { + ata_48bit_cmd(ataio, ATA_READ_DMA48, + 0, lba, count); + } else { + ata_48bit_cmd(ataio, ATA_WRITE_DMA48, + 0, lba, count); + } } else { - if (bp->bio_cmd == BIO_READ) - ataio->cmd.command = ATA_READ_DMA; - else - ataio->cmd.command = ATA_WRITE_DMA; - ataio->cmd.flags = 0; + if (bp->bio_cmd == BIO_READ) { + ata_36bit_cmd(ataio, ATA_READ_DMA, + 0, lba, count); + } else { + ata_36bit_cmd(ataio, ATA_WRITE_DMA, + 0, lba, count); + } } - + } break; case BIO_FLUSH: cam_fill_ataio(ataio, @@ -793,16 +800,10 @@ 0, da_default_timeout*1000); - if (softc->flags & DA_FLAG_CAN_48BIT) { - ataio->cmd.command = ATA_FLUSHCACHE48; - ataio->cmd.flags = CAM_ATAIO_48BIT; - } else { - ataio->cmd.command = ATA_FLUSHCACHE; - ataio->cmd.flags = 0; - } - ataio->cmd.feature = 0; - ataio->cmd.lba = 0; - ataio->cmd.count = 0; + if (softc->flags & DA_FLAG_CAN_48BIT) + ata_48bit_cmd(ataio, ATA_FLUSHCACHE48, 0, 0, 0); + else + ata_48bit_cmd(ataio, ATA_FLUSHCACHE, 0, 0, 0); break; } start_ccb->ccb_h.ccb_state = DA_CCB_BUFFER_IO; ==== //depot/projects/scottl-camlock/src/sys/cam/ata/ata_xpt.c#11 (text+ko) ==== @@ -355,9 +355,9 @@ 30 * 1000); if (periph->path->device->protocol == PROTO_ATA) - ataio->cmd.command = ATA_ATA_IDENTIFY; + ata_36bit_cmd(ataio, ATA_ATA_IDENTIFY, 0, 0, 0); else - ataio->cmd.command = ATA_ATAPI_IDENTIFY; + ata_36bit_cmd(ataio, ATA_ATAPI_IDENTIFY, 0, 0, 0); break; } case PROBE_INQUIRY: ==== //depot/projects/scottl-camlock/src/sys/cam/cam_ccb.h#19 (text+ko) ==== @@ -632,7 +632,8 @@ struct ccb_hdr ccb_h; union ccb *next_ccb; /* Ptr for next CCB for action */ u_int8_t *req_map; /* Ptr to mapping info */ - struct ata_cmd cmd; + struct ata_cmd cmd; /* ATA command register set */ + struct ata_res res; /* ATA result register set */ u_int8_t *data_ptr; /* Ptr to the data buf/SG list */ u_int32_t dxfer_len; /* Data transfer length */ /* Autosense storage */ @@ -1027,11 +1028,6 @@ ataio->data_ptr = data_ptr; ataio->dxfer_len = dxfer_len; ataio->tag_action = tag_action; - ataio->cmd.command = 0; - ataio->cmd.flags = 0; - ataio->cmd.feature = 0; - ataio->cmd.count = 0; - ataio->cmd.lba = 0; } void cam_calc_geometry(struct ccb_calc_geometry *ccg, int extended); ==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#15 (text+ko) ==== @@ -1562,39 +1562,26 @@ } fis[7] = ATA_D_LBA; fis[15] = ATA_A_4BIT; - } else if (ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA) { - fis[0] = 0x27; /* host to device */ - fis[1] = 0x80 | (ccb->ccb_h.target_id & 0x0f); - fis[2] = ccb->ataio.cmd.command; - fis[3] = ccb->ataio.cmd.count; - fis[4] = ccb->ataio.cmd.lba; - fis[5] = ccb->ataio.cmd.lba >> 8; - fis[6] = ccb->ataio.cmd.lba >> 16; - fis[7] = ATA_D_LBA; - fis[8] = ccb->ataio.cmd.lba >> 24; - fis[9] = ccb->ataio.cmd.lba >> 32; - fis[10] = ccb->ataio.cmd.lba >> 40; - fis[11] = ccb->ataio.cmd.count >> 8; - fis[12] = tag << 3; - fis[13] = 0; - fis[15] = ATA_A_4BIT; } else { fis[0] = 0x27; /* host to device */ fis[1] = 0x80 | (ccb->ccb_h.target_id & 0x0f); fis[2] = ccb->ataio.cmd.command; - fis[3] = ccb->ataio.cmd.feature; - fis[4] = ccb->ataio.cmd.lba; - fis[5] = ccb->ataio.cmd.lba >> 8; - fis[6] = ccb->ataio.cmd.lba >> 16; - fis[7] = ATA_D_LBA; - if (!(ccb->ataio.cmd.flags & CAM_ATAIO_48BIT)) - fis[7] |= (ATA_D_IBM | (ccb->ataio.cmd.lba >> 24 & 0x0f)); - fis[8] = ccb->ataio.cmd.lba >> 24; - fis[9] = ccb->ataio.cmd.lba >> 32; - fis[10] = ccb->ataio.cmd.lba >> 40; - fis[11] = ccb->ataio.cmd.feature >> 8; - fis[12] = ccb->ataio.cmd.count; - fis[13] = ccb->ataio.cmd.count >> 8; + fis[3] = ccb->ataio.cmd.features; + fis[4] = ccb->ataio.cmd.lba_low; + fis[5] = ccb->ataio.cmd.lba_mid; + fis[6] = ccb->ataio.cmd.lba_high; + fis[7] = ccb->ataio.cmd.device; + fis[8] = ccb->ataio.cmd.lba_low_exp; + fis[9] = ccb->ataio.cmd.lba_mid_exp; + fis[10] = ccb->ataio.cmd.lba_high_exp; + fis[11] = ccb->ataio.cmd.features_exp; + if (ccb->ataio.cmd.flags & CAM_ATAIO_FPDMA) { + fis[12] = tag << 3; + fis[13] = 0; + } else { + fis[12] = ccb->ataio.cmd.sector_count; + fis[13] = ccb->ataio.cmd.sector_count_exp; + } fis[15] = ATA_A_4BIT; } return (20);