[U-Boot-Users] [PATCH] Merge code duplication in ata.h and libata.h
ata.h and libata.h contains duplicate and conflicting definitions. This patch tries to resolve this and make them both includeable in a sourcefile.
Also updated is the fsl_sata driver which uses libata.h
Signed-off-by: Tor Krill tor@excito.com --- drivers/block/fsl_sata.c | 15 ++++++++------- include/ata.h | 15 +++++++++++++-- include/libata.h | 45 +-------------------------------------------- 3 files changed, 22 insertions(+), 53 deletions(-)
diff --git a/drivers/block/fsl_sata.c b/drivers/block/fsl_sata.c index 7436c4d..2b63ac4 100644 --- a/drivers/block/fsl_sata.c +++ b/drivers/block/fsl_sata.c @@ -23,6 +23,7 @@ #include <asm/io.h> #include <malloc.h> #include <libata.h> +#include <ata.h> #include <fis.h> #include "fsl_sata.h"
@@ -609,7 +610,7 @@ static u32 fsl_sata_rw_cmd(int dev, u32 start, u32 blkcnt, u8 *buffer, int is_wr cfis->lba_low = block & 0xff; cfis->sector_count = (u8)(blkcnt & 0xff);
- fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer, ATA_SECT_SIZE * blkcnt); + fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer, ATA_SECT_BYTESIZE * blkcnt); return blkcnt; }
@@ -658,7 +659,7 @@ static u32 fsl_sata_rw_cmd_ext(int dev, u32 start, u32 blkcnt, u8 *buffer, int i cfis->sector_count_exp = (blkcnt >> 8) & 0xff; cfis->sector_count = blkcnt & 0xff;
- fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer, ATA_SECT_SIZE * blkcnt); + fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer, ATA_SECT_BYTESIZE * blkcnt); return blkcnt; }
@@ -703,7 +704,7 @@ u32 fsl_sata_rw_ncq_cmd(int dev, u32 start, u32 blkcnt, u8 *buffer, int is_write ncq_channel = sata->queue_depth - 1;
/* Use the latest queue */ - fsl_sata_exec_cmd(sata, cfis, CMD_NCQ, ncq_channel, buffer, ATA_SECT_SIZE * blkcnt); + fsl_sata_exec_cmd(sata, cfis, CMD_NCQ, ncq_channel, buffer, ATA_SECT_BYTESIZE * blkcnt); return blkcnt; }
@@ -779,7 +780,7 @@ u32 ata_low_level_rw_lba48(int dev, u32 blknr, u32 blkcnt, void *buffer, int is_ fsl_sata_rw_ncq_cmd(dev, start, max_blks, addr, is_write); start += max_blks; blks -= max_blks; - addr += ATA_SECT_SIZE * max_blks; + addr += ATA_SECT_BYTESIZE * max_blks; } else { if (fsl_sata_info[dev].flags != FLAGS_FPDMA) fsl_sata_rw_cmd_ext(dev, start, blks, addr, is_write); @@ -787,7 +788,7 @@ u32 ata_low_level_rw_lba48(int dev, u32 blknr, u32 blkcnt, void *buffer, int is_ fsl_sata_rw_ncq_cmd(dev, start, blks, addr, is_write); start += blks; blks = 0; - addr += ATA_SECT_SIZE * blks; + addr += ATA_SECT_BYTESIZE * blks; } } while (blks != 0);
@@ -810,12 +811,12 @@ u32 ata_low_level_rw_lba28(int dev, u32 blknr, u32 blkcnt, void *buffer, int is_ fsl_sata_rw_cmd(dev, start, max_blks, addr, is_write); start += max_blks; blks -= max_blks; - addr += ATA_SECT_SIZE * max_blks; + addr += ATA_SECT_BYTESIZE * max_blks; } else { fsl_sata_rw_cmd(dev, start, blks, addr, is_write); start += blks; blks = 0; - addr += ATA_SECT_SIZE * blks; + addr += ATA_SECT_BYTESIZE * blks; } } while (blks != 0);
diff --git a/include/ata.h b/include/ata.h index aa6e90d..54a1455 100644 --- a/include/ata.h +++ b/include/ata.h @@ -119,8 +119,10 @@ enum { ATA_REG_BYTEH = ATA_REG_LBAH, ATA_REG_DEVSEL = ATA_REG_DEVICE, ATA_REG_IRQ = ATA_REG_NSECT, +};
- /* SETFEATURES stuff */ +/* SETFEATURES stuff */ +enum ata_set_features{ SETFEATURES_XFER = 0x03, XFER_UDMA_7 = 0x47, XFER_UDMA_6 = 0x46, @@ -130,9 +132,13 @@ enum { XFER_UDMA_2 = 0x42, XFER_UDMA_1 = 0x41, XFER_UDMA_0 = 0x40, + XFER_MW_DMA_4 = 0x24, /* CFA only */ + XFER_MW_DMA_3 = 0x23, /* CFA only */ XFER_MW_DMA_2 = 0x22, XFER_MW_DMA_1 = 0x21, XFER_MW_DMA_0 = 0x20, + XFER_PIO_6 = 0x0E, /* CFA only */ + XFER_PIO_5 = 0x0D, /* CFA only */ XFER_PIO_4 = 0x0C, XFER_PIO_3 = 0x0B, XFER_PIO_2 = 0x0A, @@ -141,7 +147,12 @@ enum { XFER_SW_DMA_2 = 0x12, XFER_SW_DMA_1 = 0x11, XFER_SW_DMA_0 = 0x10, - XFER_PIO_SLOW = 0x00 + XFER_PIO_SLOW = 0x00, + + SETFEATURES_WC_ON = 0x02, /* Enable write cache */ + SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + + SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ }; /* * ATA Commands (only mandatory commands listed here) diff --git a/include/libata.h b/include/libata.h index aedba74..246dcd3 100644 --- a/include/libata.h +++ b/include/libata.h @@ -49,39 +49,6 @@ enum ata_cmd { ATA_CMD_WRITE_FPDMA_QUEUED = 0x61, };
-/* SETFEATURES stuff - */ -enum ata_set_features { - /* SETFEATURES stuff */ - SETFEATURES_XFER = 0x03, - XFER_UDMA_7 = 0x47, - XFER_UDMA_6 = 0x46, - XFER_UDMA_5 = 0x45, - XFER_UDMA_4 = 0x44, - XFER_UDMA_3 = 0x43, - XFER_UDMA_2 = 0x42, - XFER_UDMA_1 = 0x41, - XFER_UDMA_0 = 0x40, - XFER_MW_DMA_4 = 0x24, /* CFA only */ - XFER_MW_DMA_3 = 0x23, /* CFA only */ - XFER_MW_DMA_2 = 0x22, - XFER_MW_DMA_1 = 0x21, - XFER_MW_DMA_0 = 0x20, - XFER_PIO_6 = 0x0E, /* CFA only */ - XFER_PIO_5 = 0x0D, /* CFA only */ - XFER_PIO_4 = 0x0C, - XFER_PIO_3 = 0x0B, - XFER_PIO_2 = 0x0A, - XFER_PIO_1 = 0x09, - XFER_PIO_0 = 0x08, - XFER_PIO_SLOW = 0x00, - - SETFEATURES_WC_ON = 0x02, /* Enable write cache */ - SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ - - SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ -}; - enum ata_protocol { ATA_PROT_UNKNOWN, /* unknown */ ATA_PROT_NODATA, /* no data */ @@ -101,26 +68,16 @@ enum ata_dev_typed { };
enum { - ATA_SECT_SIZE = 512, + ATA_SECT_BYTESIZE = 512, ATA_MAX_SECTORS_128 = 128, ATA_MAX_SECTORS = 256, ATA_MAX_SECTORS_LBA48 = 65535,
/* bits in ATA command block registers */ - ATA_HOB = (1 << 7), /* LBA48 selector */ - ATA_NIEN = (1 << 1), /* disable-irq flag */ ATA_LBA = (1 << 6), /* LBA28 selector */ - ATA_DEV1 = (1 << 4), /* Select Device 1 (slave) */ - ATA_BUSY = (1 << 7), /* BSY status bit */ - ATA_DRDY = (1 << 6), /* device ready */ - ATA_DF = (1 << 5), /* device fault */ - ATA_DRQ = (1 << 3), /* data request i/o */ - ATA_ERR = (1 << 0), /* have an error */ - ATA_SRST = (1 << 2), /* software reset */ ATA_ICRC = (1 << 7), /* interface CRC error */ ATA_UNC = (1 << 6), /* uncorrectable media error */ ATA_IDNF = (1 << 4), /* ID not found */ - ATA_ABORTED = (1 << 2), /* command aborted */
ATA_ID_WORDS = 256, ATA_ID_SERNO = 10,
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_SIZE * blkcnt);
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_BYTESIZE * blkcnt); return blkcnt; }
@@ -658,7 +659,7 @@ static u32 fsl_sata_rw_cmd_ext(int dev, u32 start, u32 blkcnt, u8 *buffer, int i cfis->sector_count_exp = (blkcnt >> 8) & 0xff; cfis->sector_count = blkcnt & 0xff;
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_SIZE * blkcnt);
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_BYTESIZE * blkcnt); return blkcnt;
Nack.
Don't do this.
I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.
The libata.h is closely than the latest linux kernel.
On 3/31/2008, "Dave Liu" r63238@freescale.com wrote:
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_SIZE * blkcnt);
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_BYTESIZE * blkcnt); return blkcnt; }
@@ -658,7 +659,7 @@ static u32 fsl_sata_rw_cmd_ext(int dev, u32 start, u32 blkcnt, u8 *buffer, int i cfis->sector_count_exp = (blkcnt >> 8) & 0xff; cfis->sector_count = blkcnt & 0xff;
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_SIZE * blkcnt);
fsl_sata_exec_cmd(sata, cfis, CMD_ATA, 0, buffer,ATA_SECT_BYTESIZE * blkcnt); return blkcnt;
Nack.
Don't do this.
I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.
The big problem here is that ata.h defines ATA_SECT_SIZE as 256 Words where libata defines it as 512 Bytes.
I did not want to change the old define since it would possibly break more than the fsl driver.
If its ok to change the definition to 512 Bytes and only change the ata_piix driver ignoring other possible uses of it i could resubmit the patch.
But the problem still remains, it is currently impossible to use both libata.h and ata.h.
/Tor
Don't do this.
I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.
The big problem here is that ata.h defines ATA_SECT_SIZE as 256 Words where libata defines it as 512 Bytes.
I did not want to change the old define since it would possibly break more than the fsl driver.
When I grep 'ATA_SECT_SIZE', I can *not* find any files including the ATA_SECT_SIZE before the fsl_sata.c.
But the problem still remains, it is currently impossible to use both libata.h and ata.h.
Why need use both them? I believe the libata.h is enough.
If the libata.h is not enough for you, I suggest you update it from the latest linux kernel.
I think it is correct way to handle it. It will be more easy to to maintain the header.
On 3/31/2008, "Dave Liu" r63238@freescale.com wrote:
Don't do this.
I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.
The big problem here is that ata.h defines ATA_SECT_SIZE as 256 Words where libata defines it as 512 Bytes.
I did not want to change the old define since it would possibly break more than the fsl driver.
When I grep 'ATA_SECT_SIZE', I can *not* find any files including the ATA_SECT_SIZE before the fsl_sata.c.
Well, the ata_piix driver for one uses it, but only includes ata.h. But that is the only one in the tree right now.
But the problem still remains, it is currently impossible to use both libata.h and ata.h.
Why need use both them? I believe the libata.h is enough.
Since my driver for the sil3114 uses "legacy mode" it uses a lot of the ata commands directly in PIO mode. These are defined in ata.h (Both in u-boot and Linux)
If the libata.h is not enough for you, I suggest you update it from the latest linux kernel.
Still, the kernel has both a ata.h and a libata.h, where libata.h includes ata.h. And where ATA_SECT_SIZE, and the other defines i use, actually is defined in ata.h albeit as 512 bytes :).
I think it is correct way to handle it. It will be more easy to to maintain the header.
I have no problem with either solution. If its ok to add the defines to libata.h instead i will do that?
/Tor
When I grep 'ATA_SECT_SIZE', I can *not* find any files including the ATA_SECT_SIZE before the fsl_sata.c.
Well, the ata_piix driver for one uses it, but only includes ata.h. But that is the only one in the tree right now.
But the problem still remains, it is currently impossible to use both libata.h and ata.h.
Why need use both them? I believe the libata.h is enough.
Since my driver for the sil3114 uses "legacy mode" it uses a lot of the ata commands directly in PIO mode. These are defined in ata.h (Both in u-boot and Linux)
I believe the linux kernel happen at legacy mode same as you, but kernel can handle the legacy mode, why you can not hanlde the legacy mode in u- boot with same header file.
If you're using the I/O register descriptions and status register bits in ata.h of u-boot. the kernel header have the same register/bit definition. just like
ATA_REG_DATA ATA_REG_ERR ...
ATA_BUSY ATA_DRDY ... ATA_ERR ATA_SRST
In message MdNGBCPY.1206951652.2688260.tor@localhost you wrote:
I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.
Actually it's worse.
The big problem here is that ata.h defines ATA_SECT_SIZE as 256 Words where libata defines it as 512 Bytes.
But "BYTESIZE" reads to me as "size of byte", just as if ATA sectors used something different than 8 bits per byte.
If you need such a new definition, then please call it different.
Best regards,
Wolfgang Denk
participants (3)
-
Dave Liu -
Tor Krill -
Wolfgang Denk