Skip to contentSkip to conversation

Merge code duplication in ata.h and libata.h

PATCH New Tor Krill 3 participants 7 messages Mar 31, 2008 Block layer

lore
Share & tools
lore

Tor Krill

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
---
 drivers/block/fsl_sata.c |   15 ++++++++-------
 include/ata.h            |   15 +++++++++++++--
 include/libata.h         |   45 +--------------------------------------------
 3 files changed, 22 insertions(+), 53 deletions(-)
drivers/block/fsl_sata.c +8 -7
index 7436c4d..2b63ac4 100644
@@ -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);
include/ata.h +13 -2
index aa6e90d..54a1455 100644
@@ -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)
include/libata.h +1 -44

66 lines not on this page · raw message

Signature · 1 line
1.5.4.4
NAK
#1 lore raw

PATCH 1/1 Merge code duplication in ata.h and libata.h

Quoted 10 lines

- 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;

}

(quoted diff) +0 -0
@@ -658,7 +659,7 @@ static u32 fsl_sata_rw_cmd_ext(int dev, u32 start,

u32 blkcnt, u8 *buffer, int i

Quoted 8 lines
        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.

author
#2 lore raw

PATCH 1/1 Merge code duplication in ata.h and libata.h

Quoted 22 lines · Dave Liu
Quoted 10 lines

- 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;

}

(quoted diff) +0 -0
@@ -658,7 +659,7 @@ static u32 fsl_sata_rw_cmd_ext(int dev, u32 start,

u32 blkcnt, u8 *buffer, int i

Quoted 8 lines
        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.

Quoted 4 lines

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

#3 lore raw

PATCH 1/1 Merge code duplication in ata.h and libata.h

Quoted 10 lines
Quoted 4 lines

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.

Quoted 2 lines

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.

author
#4 lore raw

PATCH 1/1 Merge code duplication in ata.h and libata.h

Quoted 5 lines · Dave Liu
Quoted 5 lines
Quoted 4 lines

Don't do this.

I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.

Quoted 8 lines
Quoted 5 lines

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.

Quoted 5 lines
Quoted 2 lines

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)

Quoted 2 lines

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 :).

Quoted 2 lines

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

#5 lore raw

PATCH 1/1 Merge code duplication in ata.h and libata.h

Quoted 9 lines
Quoted 2 lines

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.

Quoted 3 lines
Quoted 2 lines

But the problem still remains, it is currently impossible to use both libata.h and ata.h.

Quoted 6 lines
Quoted 2 lines

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

#6 lore raw

PATCH 1/1 Merge code duplication in ata.h and libata.h

In reply to Tor Krill
Quoted 3 lines · In message <MdNGBCPY.1206951652.2688260.tor@localhost> you wrote:
Quoted 3 lines

I don't think the ATA_SECT_BYTESIZE is better than the ATA_SECT_SIZE.

Actually it's worse.

Quoted 2 lines

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

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Love sometimes expresses itself in sacrifice.
	-- Kirk, "Metamorphosis", stardate 3220.3