Skip to contentSkip to conversation

AHCI / SATA Improvements

PATCH New Simon Glass 4 participants 33 messages Oct 19, 2012 – Oct 31, 2012 Block layer

lore
Share & tools
lore

Simon Glass

author
#0 lore raw

cover AHCI / SATA Improvements

This series contains a set of improvements for the SATA susbsystem, mostly targeted at solid-state drivers and improving start-up time.

The patches are tested on various x86 Chromebooks.

Gabe Black (2):
ahci: Make sending the SETFEATURES_XFER command optional
ahci: Make the AHCI code find the capacity of disks > 128 GB properly

Hung-Te Lin (2):
scsi: Add scsi_write to SCSI driver
ahci: support scsi writing in AHCI driver.

Marc Jones (2):
ahci: Support spin-up and link-up separately
ahci: Perform SATA flush after disk write.

Simon Glass (1):
x86: config: Enable AHCI support for coreboot

Stefan Reinauer (4):
scsi: Add function to report number of scsi drives
ahci: Optimise AHCI controller reset and start-up
ahci: Improve AHCI debugging
ahci: cosmetics and cleanup

Taylor Hutt (5):
ahci: Use sizeof(fis) instead of hardcoding '20'
ahci: Put all uses of ahci_set_feature() under
CONFIG_AHCI_SETFEATURES_XFER
scsi: Put 'scsi_device_list' variable under CONFIG_PCI
ahci: Use virt_to_phys() to denote physical addresses for DMA
ahci: flush / invalidate dcache around SATA commands

Vadim Bendebury (2):
ahci: Support splitting of read transactions into multiple chunks
scsi: Provide support for a list of AHCI controllers.

Walter Murphy (2):
ahci: Adjust SATA timeouts for hard disk (spinup delay & command
timeout)
ahci: Expand HDD Logical Block addressability up to 32 bits

 common/cmd_scsi.c          |  124 +++++++++++++--
 drivers/block/ahci.c       |  401 +++++++++++++++++++++++++++++++++++---------
 include/ahci.h             |    1 +
 include/ata.h              |    3 +
 include/configs/coreboot.h |   21 +++
 include/scsi.h             |    4 +
 6 files changed, 467 insertions(+), 87 deletions(-)
Signature · 1 line
1.7.7.3
author
#1 lore raw

PATCH 1/20 ahci: Support splitting of read transactions into multiple chunks

From: Vadim Bendebury <[email protected]>

With an Intel AHCI controller, the driver does not operate properly if the requested amount of blocks to read exceeds 255.

It is probably possible to specify 0 as the block count and the driver will read 256 blocks, but it was decided to limit the number of blocks read at once to 128 (it should be a power of 2 for the optimal performance of solid state drives).

Signed-off-by Vadim Bendebury <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   98 +++++++++++++++++++++++++++++++++++---------------
 1 files changed, 69 insertions(+), 29 deletions(-)
drivers/block/ahci.c +69 -29
index 7b2ec50..d94da1f 100644
@@ -42,6 +42,14 @@ hd_driveid_t *ataid[AHCI_MAX_PORTS];
 
 #define writel_with_flush(a,b)	do { writel(a,b); readl(b); } while (0)
 
+/*
+ * Some controllers limit number of blocks they can read at once. Contemporary
+ * SSD devices work much faster if the read size is aligned to a power of 2.
+ * Let's set default to 128 and allowing to be overwritten if needed.
+ */
+#ifndef MAX_SATA_BLOCKS_READ
+#define MAX_SATA_BLOCKS_READ	0x80
+#endif
 
 static inline u32 ahci_port_base(u32 base, u32 port)
 {

@@ -88,6 +96,8 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 	int i, j;
 	volatile u8 *port_mmio;
 
+	debug("ahci_host_init: start\n");
+
 	cap_save = readl(mmio + HOST_CAP);
 	cap_save &= ((1 << 28) | (1 << 17));
 	cap_save |= (1 << 27);

@@ -129,6 +139,9 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 	debug("cap 0x%x  port_map 0x%x  n_ports %d\n",
 	      probe_ent->cap, probe_ent->port_map, probe_ent->n_ports);
 
+	if (probe_ent->n_ports > CONFIG_SYS_SCSI_MAX_SCSI_ID)
+		probe_ent->n_ports = CONFIG_SYS_SCSI_MAX_SCSI_ID;
+
 	for (i = 0; i < probe_ent->n_ports; i++) {
 		probe_ent->port[i].port_mmio = ahci_port_base((u32) mmio, i);
 		port_mmio = (u8 *) probe_ent->port[i].port_mmio;

@@ -277,8 +290,8 @@ static int ahci_init_one(pci_dev_t pdev)
 	probe_ent->pio_mask = 0x1f;
 	probe_ent->udma_mask = 0x7f;	/*Fixme,assume to support UDMA6 */
 
-	probe_ent->mmio_base = (u32)pci_map_bar(pdev, AHCI_PCI_BAR,
-						PCI_REGION_MEM);
+	pci_read_config_dword(pdev, PCI_BASE_ADDRESS_5, &probe_ent->mmio_base);
+	debug("ahci mmio_base=0x%08x\n", probe_ent->mmio_base);
 
 	/* Take from kernel:
 	 * JMicron-specific fixup:

@@ -398,7 +411,7 @@ static int ahci_port_start(u8 port)
 	 * 32 bytes each in size
 	 */
 	pp->cmd_slot = (struct ahci_cmd_hdr *)mem;
-	debug("cmd_slot = %p\n", pp->cmd_slot);
+	debug("cmd_slot = 0x%x\n", (unsigned)pp->cmd_slot);
 	mem += (AHCI_CMD_SLOT_SZ + 224);
 
 	/*

@@ -561,42 +574,69 @@ static int ata_scsiop_inquiry(ccb *pccb)
  */
 static int ata_scsiop_read10(ccb * pccb)
 {
-	u32 len = 0;
+	u32 lba = 0;
+	u16 blocks = 0;
 	u8 fis[20];
+	u8 *user_buffer = pccb->pdata;
+	u32 user_buffer_size = pccb->datalen;
 
-	len = (((u32) pccb->cmd[7]) << 8) | ((u32) pccb->cmd[8]);
+	/* Retrieve the base LBA number from the ccb structure. */
+	memcpy(&lba, pccb->cmd + 2, sizeof(lba));
+	lba = be32_to_cpu(lba);
 
-	/* For 10-byte and 16-byte SCSI R/W commands, transfer
+	/*
+	 * And the number of blocks.
+	 *
+	 * For 10-byte and 16-byte SCSI R/W commands, transfer
 	 * length 0 means transfer 0 block of data.
 	 * However, for ATA R/W commands, sector count 0 means
 	 * 256 or 65536 sectors, not 0 sectors as in SCSI.
 	 *
 	 * WARNING: one or two older ATA drives treat 0 as 0...
 	 */
-	if (!len)
-		return 0;
+	blocks = (((u16)pccb->cmd[7]) << 8) | ((u16) pccb->cmd[8]);
+
+	debug("scsi_ahci: read %d blocks starting from lba 0x%x\n",
+	      (unsigned)lba, blocks);
+
+	/* Preset the FIS */
 	memset(fis, 0, 20);
+	fis[0] = 0x27;		 /* Host to device FIS. */
+	fis[1] = 1 << 7;	 /* Command FIS. */
+	fis[2] = ATA_CMD_RD_DMA; /* Command byte. */
 
-	/* Construct the FIS */
-	fis[0] = 0x27;		/* Host to device FIS. */
-	fis[1] = 1 << 7;	/* Command FIS. */
-	fis[2] = ATA_CMD_RD_DMA;	/* Command byte. */
-
-	/* LBA address, only support LBA28 in this driver */
-	fis[4] = pccb->cmd[5];
-	fis[5] = pccb->cmd[4];
-	fis[6] = pccb->cmd[3];
-	fis[7] = (pccb->cmd[2] & 0x0f) | 0xe0;
-
-	/* Sector Count */
-	fis[12] = pccb->cmd[8];
-	fis[13] = pccb->cmd[7];
-
-	/* Read from ahci */
-	if (get_ahci_device_data(pccb->target, (u8 *) & fis, 20,
-				 pccb->pdata, pccb->datalen)) {
-		debug("scsi_ahci: SCSI READ10 command failure.\n");
-		return -EIO;
+	while (blocks) {
+		u16 now_blocks; /* number of blocks per iteration */
+		u32 transfer_size; /* number of bytes per iteration */
+
+		now_blocks = min(MAX_SATA_BLOCKS_READ, blocks);
+
+		transfer_size = ATA_BLOCKSIZE * now_blocks;
+		if (transfer_size > user_buffer_size) {
+			printf("scsi_ahci: Error: buffer too small.\n");
+			return -EIO;
+		}
+
+		/* LBA address, only support LBA28 in this driver */
+		fis[4] = (lba >> 0) & 0xff;
+		fis[5] = (lba >> 8) & 0xff;
+		fis[6] = (lba >> 16) & 0xff;
+		fis[7] = ((lba >> 24) & 0xf) | 0xe0;
+
+		/* Block (sector) count */
+		fis[12] = (now_blocks >> 0) & 0xff;
+		fis[13] = (now_blocks >> 8) & 0xff;
+
+		/* Read from ahci */
+		if (get_ahci_device_data(pccb->target, (u8 *) &fis, sizeof(fis),
+					 user_buffer, user_buffer_size)) {
+			debug("scsi_ahci: SCSI READ10 command failure.\n");
+			return -EIO;
+		}
+		user_buffer += transfer_size;
+		user_buffer_size -= transfer_size;
+		blocks -= now_blocks;
+		lba += now_blocks;
 	}
 
 	return 0;

@@ -617,7 +657,7 @@ static int ata_scsiop_read_capacity10(ccb *pccb)
 		return -EPERM;
 	}
 
-	cap = le32_to_cpu(ataid[pccb->target]->lba_capacity);
+	cap = be32_to_cpu(ataid[pccb->target]->lba_capacity);
 	memcpy(pccb->pdata, &cap, sizeof(cap));
 
 	pccb->pdata[4] = pccb->pdata[5] = 0;
Signature · 1 line
1.7.7.3
author
#2 lore raw

PATCH 2/20 scsi: Provide support for a list of AHCI controllers.

In reply to Simon Glass

From: Vadim Bendebury <[email protected]>

Many AHCI controllers are identical, the main (and often the only) difference being the PCI Vendor ID/Device ID combination reported by the device.

This change allows the config file to define a list of PCI vendor ID/device ID pairs. The driver would scan the list and initialize the first device it finds.

No actual multiple device list is introduced yet, this change just add the framework.

Signed-off-by Vadim Bendebury <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 common/cmd_scsi.c |   43 ++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 38 insertions(+), 5 deletions(-)
common/cmd_scsi.c +38 -5
index 22d0119..3890afd 100644
@@ -34,6 +34,14 @@
 #include <image.h>
 #include <pci.h>
 
+struct scsi_device {
+	u16	scsi_vendor_id;
+	u16	scsi_dev_id;
+};
+
+#ifdef CONFIG_SCSI_DEV_LIST
+#define SCSI_DEV_LIST CONFIG_SCSI_DEV_LIST
+#else
 #ifdef CONFIG_SCSI_SYM53C8XX
 #define SCSI_VEND_ID	0x1000
 #ifndef CONFIG_SCSI_DEV_ID

@@ -49,8 +57,10 @@
 #elif !defined(CONFIG_SCSI_AHCI_PLAT)
 #error no scsi device defined
 #endif
+#define SCSI_DEV_LIST {SCSI_VEND_ID, SCSI_DEV_ID}
+#endif
 
-
+static struct scsi_device scsi_device_list[] = { SCSI_DEV_LIST };
 static ccb tempccb;	/* temporary scsi command buffer */
 
 static unsigned char tempbuff[512]; /* temporary data buffer */

@@ -178,15 +188,38 @@ removable:
 void scsi_init(void)
 {
 	int busdevfunc;
+	int i;
+	/*
+	 * Find a device from the list, this driver will support a single
+	 * controller.
+	 */
+	for (i = 0; i < ARRAY_SIZE(scsi_device_list); i++) {
+		/* get PCI Device ID */
+		busdevfunc = pci_find_device(scsi_device_list[i].scsi_vendor_id,
+					     scsi_device_list[i].scsi_dev_id,
+					     0);
+		if (busdevfunc != -1)
+			break;
+	}
 
-	busdevfunc=pci_find_device(SCSI_VEND_ID,SCSI_DEV_ID,0); /* get PCI Device ID */
-	if(busdevfunc==-1) {
-		printf("Error SCSI Controller (%04X,%04X) not found\n",SCSI_VEND_ID,SCSI_DEV_ID);
+	if (busdevfunc == -1) {
+		printf("Error: SCSI Controller(s) ");
+		for (i = 0; i < ARRAY_SIZE(scsi_device_list); i++) {
+			printf("%04X:%04X ",
+			       scsi_device_list[i].scsi_vendor_id,
+			       scsi_device_list[i].scsi_dev_id);
+		}
+		printf("not found\n");
 		return;
 	}
 #ifdef DEBUG
 	else {
-		printf("SCSI Controller (%04X,%04X) found (%d:%d:%d)\n",SCSI_VEND_ID,SCSI_DEV_ID,(busdevfunc>>16)&0xFF,(busdevfunc>>11)&0x1F,(busdevfunc>>8)&0x7);
+		printf("SCSI Controller (%04X,%04X) found (%d:%d:%d)\n",
+		       scsi_device_list[i].scsi_vendor_id,
+		       scsi_device_list[i].scsi_dev_id,
+		       (busdevfunc >> 16) & 0xFF,
+		       (busdevfunc >> 11) & 0x1F,
+		       (busdevfunc >> 8) & 0x7);
 	}
 #endif
 	scsi_low_level_init(busdevfunc);
Signature · 1 line
1.7.7.3
author
#3 lore raw

PATCH 3/20 scsi: Add scsi_write to SCSI driver

In reply to Simon Glass

From: Hung-Te Lin <[email protected]>

Implement write functionality in the scsi layer. A ''scsi write' command is also added to console for testing.

Signed-off-by Hung-Te Lin <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 common/cmd_scsi.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 66 insertions(+), 6 deletions(-)
common/cmd_scsi.c +66 -6
index 3890afd..30d8d12 100644
@@ -82,7 +82,7 @@ void scsi_setup_inquiry(ccb * pccb);
 void scsi_ident_cpy (unsigned char *dest, unsigned char *src, unsigned int len);
 
 
-ulong scsi_read(int device, ulong blknr, ulong blkcnt, void *buffer);
+static ulong scsi_read(int device, ulong blknr, lbaint_t blkcnt, void *buffer);
 
 
 /*********************************************************************************

@@ -349,9 +349,10 @@ int do_scsi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 #define SCSI_MAX_READ_BLK 0xFFFF /* almost the maximum amount of the scsi_ext command.. */
 
-ulong scsi_read(int device, ulong blknr, ulong blkcnt, void *buffer)
+static ulong scsi_read(int device, ulong blknr, lbaint_t blkcnt, void *buffer)
 {
-	ulong start,blks, buf_addr;
+	lbaint_t start, blks;
+	uintptr_t buf_addr;
 	unsigned short smallblks;
 	ccb* pccb=(ccb *)&tempccb;
 	device&=0xff;

@@ -362,7 +363,9 @@ ulong scsi_read(int device, ulong blknr, ulong blkcnt, void *buffer)
 	buf_addr=(unsigned long)buffer;
 	start=blknr;
 	blks=blkcnt;
-	debug ("\nscsi_read: dev %d startblk %lx, blccnt %lx buffer %lx\n",device,start,blks,(unsigned long)buffer);
+	debug("\nscsi_read: dev %d startblk " LBAF
+	      ", blccnt " LBAF " buffer %lx\n",
+	      device, start, blks, (unsigned long)buffer);
 	do {
 		pccb->pdata=(unsigned char *)buf_addr;
 		if(blks>SCSI_MAX_READ_BLK) {

@@ -379,7 +382,9 @@ ulong scsi_read(int device, ulong blknr, ulong blkcnt, void *buffer)
 			start+=blks;
 			blks=0;
 		}
-		debug ("scsi_read_ext: startblk %lx, blccnt %x buffer %lx\n",start,smallblks,buf_addr);
+		debug("scsi_read_ext: startblk " LBAF
+		      ", blccnt %x buffer %lx\n",
+		      start, smallblks, buf_addr);
 		if(scsi_exec(pccb)!=TRUE) {
 			scsi_print_error(pccb);
 			blkcnt-=blks;

@@ -387,10 +392,65 @@ ulong scsi_read(int device, ulong blknr, ulong blkcnt, void *buffer)
 		}
 		buf_addr+=pccb->datalen;
 	} while(blks!=0);
-	debug ("scsi_read_ext: end startblk %lx, blccnt %x buffer %lx\n",start,smallblks,buf_addr);
+	debug("scsi_read_ext: end startblk " LBAF
+	      ", blccnt %x buffer %lx\n", start, smallblks, buf_addr);
 	return(blkcnt);
 }
 
+/*******************************************************************************
+ * scsi_write
+ */
+
+/* Almost the maximum amount of the scsi_ext command.. */
+#define SCSI_MAX_WRITE_BLK 0xFFFF
+
+static ulong scsi_write(int device, ulong blknr,
+			lbaint_t blkcnt, const void *buffer)
+{
+	lbaint_t start, blks;
+	uintptr_t buf_addr;
+	unsigned short smallblks;
+	ccb* pccb = (ccb *)&tempccb;
+	device &= 0xff;
+	/* Setup  device
+	 */
+	pccb->target = scsi_dev_desc[device].target;
+	pccb->lun = scsi_dev_desc[device].lun;
+	buf_addr = (unsigned long)buffer;
+	start = blknr;
+	blks = blkcnt;
+	debug("\n%s: dev %d startblk " LBAF ", blccnt " LBAF " buffer %lx\n",
+	      __func__, device, start, blks, (unsigned long)buffer);
+	do {
+		pccb->pdata = (unsigned char *)buf_addr;
+		if (blks > SCSI_MAX_WRITE_BLK) {
+			pccb->datalen = (scsi_dev_desc[device].blksz *
+					 SCSI_MAX_WRITE_BLK);
+			smallblks = SCSI_MAX_WRITE_BLK;
+			scsi_setup_write_ext(pccb, start, smallblks);
+			start += SCSI_MAX_WRITE_BLK;
+			blks -= SCSI_MAX_WRITE_BLK;
+		} else {
+			pccb->datalen = scsi_dev_desc[device].blksz * blks;
+			smallblks = (unsigned short)blks;
+			scsi_setup_write_ext(pccb, start, smallblks);
+			start += blks;
+			blks = 0;
+		}
+		debug("%s: startblk " LBAF ", blccnt %x buffer %lx\n",
+		      __func__, start, smallblks, buf_addr);
+		if (scsi_exec(pccb) != TRUE) {
+			scsi_print_error(pccb);
+			blkcnt -= blks;
+			break;
+		}
+		buf_addr += pccb->datalen;
+	} while (blks != 0);
+	debug("%s: end startblk " LBAF ", blccnt %x buffer %lx\n",
+	      __func__, start, smallblks, buf_addr);
+	return blkcnt;
+}
+
 /* copy src to dest, skipping leading and trailing blanks
  * and null terminate the string
  */
Signature · 1 line
1.7.7.3
author
#4 lore raw

PATCH 4/20 scsi: Add function to report number of scsi drives

In reply to Simon Glass

From: Stefan Reinauer <[email protected]>

Add a new function to find out the number of available SCSI disks.

Signed-off-by Stefan Reinauer <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 common/cmd_scsi.c |    7 +++++++
 include/scsi.h    |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)
common/cmd_scsi.c +7 -0
index 30d8d12..f64d6f4 100644
@@ -182,6 +182,13 @@ removable:
 		scsi_curr_dev=0;
 	else
 		scsi_curr_dev = -1;
+
+	printf("Found %d device(s).\n", scsi_max_devs);
+}
+
+int scsi_get_disk_count(void)
+{
+	return scsi_max_devs;
 }
 
 #ifdef CONFIG_PCI
include/scsi.h +2 -0
index 89ae45f..9681d19 100644
@@ -189,6 +189,8 @@ void scsi_low_level_init(int busdevfunc);
 void scsi_init(void);
 void scsi_scan(int mode);
 
+/** @return the number of scsi disks */
+int scsi_get_disk_count(void);
 
 #define SCSI_IDENTIFY					0xC0  /* not used */
Signature · 1 line
1.7.7.3
#5 lore raw

PATCH 4/20 scsi: Add function to report number of scsi drives

Quoted 3 lines · Simon Glass

From: Stefan Reinauer <[email protected]>

Add a new function to find out the number of available SCSI disks.

Wouldn't setting an environment variable be more useful? Then we can loop over devices in boot scripts.

Rob

Quoted 40 lines
Signed-off-by Stefan Reinauer <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 common/cmd_scsi.c |    7 +++++++
 include/scsi.h    |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)
common/cmd_scsi.c +7 -0
index 30d8d12..f64d6f4 100644
@@ -182,6 +182,13 @@ removable:
 		scsi_curr_dev=0;
 	else
 		scsi_curr_dev = -1;
+
+	printf("Found %d device(s).\n", scsi_max_devs);
+}
+
+int scsi_get_disk_count(void)
+{
+	return scsi_max_devs;
 }
 
 #ifdef CONFIG_PCI
include/scsi.h +2 -0
index 89ae45f..9681d19 100644
@@ -189,6 +189,8 @@ void scsi_low_level_init(int busdevfunc);
 void scsi_init(void);
 void scsi_scan(int mode);
 
+/** @return the number of scsi disks */
+int scsi_get_disk_count(void);
 
 #define SCSI_IDENTIFY					0xC0  /* not used */
author
#6 lore raw

PATCH 4/20 scsi: Add function to report number of scsi drives

Hi Rob,

Quoted 7 lines · Rob Herring
Quoted 3 lines · Simon Glass

From: Stefan Reinauer <[email protected]>

Add a new function to find out the number of available SCSI disks.

Wouldn't setting an environment variable be more useful? Then we can loop over devices in boot scripts.

Yes that's a good idea - will add that too. The idea with this patch is to provide an internal API to read this info within U-Boot.

Regards,
Simon

Quoted 43 lines

Rob

Quoted 40 lines
Signed-off-by Stefan Reinauer <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 common/cmd_scsi.c |    7 +++++++
 include/scsi.h    |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)
common/cmd_scsi.c +7 -0
index 30d8d12..f64d6f4 100644
@@ -182,6 +182,13 @@ removable:
              scsi_curr_dev=0;
      else
              scsi_curr_dev = -1;
+
+     printf("Found %d device(s).\n", scsi_max_devs);
+}
+
+int scsi_get_disk_count(void)
+{
+     return scsi_max_devs;
 }
 
 #ifdef CONFIG_PCI
include/scsi.h +2 -0
index 89ae45f..9681d19 100644
@@ -189,6 +189,8 @@ void scsi_low_level_init(int busdevfunc);
 void scsi_init(void);
 void scsi_scan(int mode);
 
+/** @return the number of scsi disks */
+int scsi_get_disk_count(void);
 
 #define SCSI_IDENTIFY                                        0xC0  /* not used */
author
#7 lore raw

PATCH 5/20 ahci: Optimise AHCI controller reset and start-up

In reply to Simon Glass

From: Stefan Reinauer <[email protected]>

The existing code waits a whole second for the AHCI controller to reset. Instead, let's poll the status register to see if the reset has succeeded and return earlier if possible. This brings down the time for AHCI probing from 1s to 20ms.

Signed-off-by Stefan Reinauer <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)
drivers/block/ahci.c +15 -9
index d94da1f..ad397dc 100644
@@ -110,13 +110,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 	/* reset must complete within 1 second, or
 	 * the hardware should be considered fried.
 	 */
-	ssleep(1);
-
-	tmp = readl(mmio + HOST_CTL);
-	if (tmp & HOST_RESET) {
-		debug("controller reset failed (0x%x)\n", tmp);
-		return -1;
-	}
+	i = 1000;
+	do {
+		udelay(1000);
+		tmp = readl(mmio + HOST_CTL);
+		if (!i--) {
+			debug("controller reset failed (0x%x)\n", tmp);
+			return -1;
+		}
+	} while (tmp & HOST_RESET);
 
 	writel_with_flush(HOST_AHCI_EN, mmio + HOST_CTL);
 	writel(cap_save, mmio + HOST_CAP);

@@ -164,13 +166,17 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 		writel(PORT_CMD_SPIN_UP, port_mmio + PORT_CMD);
 
 		j = 0;
-		while (j < 100) {
-			msleep(10);
+		while (j < 1000) {
 			tmp = readl(port_mmio + PORT_SCR_STAT);
 			if ((tmp & 0xf) == 0x3)
 				break;
+			udelay(1000);
 			j++;
 		}
+		if (j == 1000)
+			debug("timeout.\n");
+		else
+			debug("ok.\n");
 
 		tmp = readl(port_mmio + PORT_SCR_ERR);
 		debug("PORT_SCR_ERR 0x%x\n", tmp);
Signature · 1 line
1.7.7.3
author
#8 lore raw

PATCH 6/20 ahci: Improve AHCI debugging

In reply to Simon Glass

From: Stefan Reinauer <[email protected]>

- remove unused ssleep macro
- add some useful debugging information

Signed-off-by Stefan Reinauer <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/block/ahci.c +2 -1
index ad397dc..af31c97 100644
@@ -68,7 +68,6 @@ static void ahci_setup_port(struct ahci_ioports *port, unsigned long base,
 
 
 #define msleep(a) udelay(a * 1000)
-#define ssleep(a) msleep(a * 1000)
 
 static int waiting_for_cmd_completed(volatile u8 *offset,
 				     int timeout_msec,

@@ -153,6 +152,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 		tmp = readl(port_mmio + PORT_CMD);
 		if (tmp & (PORT_CMD_LIST_ON | PORT_CMD_FIS_ON |
 			   PORT_CMD_FIS_RX | PORT_CMD_START)) {
+			debug("Port %d is active. Deactivating.\n", i);
 			tmp &= ~(PORT_CMD_LIST_ON | PORT_CMD_FIS_ON |
 				 PORT_CMD_FIS_RX | PORT_CMD_START);
 			writel_with_flush(tmp, port_mmio + PORT_CMD);

@@ -163,6 +163,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 			msleep(500);
 		}
 
+		debug("Spinning up port %d... ", i);
 		writel(PORT_CMD_SPIN_UP, port_mmio + PORT_CMD);
 
 		j = 0;
Signature · 1 line
1.7.7.3
author
#9 lore raw

PATCH 7/20 ahci: cosmetics and cleanup

In reply to Simon Glass

From: Stefan Reinauer <[email protected]>

- print the correct speed
- print all the AHCI capability flags
(information taken from Linux kernel driver)
- clean up some comments

For example, this might show the following string:
AHCI 0001.0300 32 slots 6 ports 6 Gbps 0x3 impl SATA mode

Signed-off-by Stefan Reinauer <[email protected]>

Commit-Ready: Stefan Reinauer <[email protected]>

Signed-off-by Simon Glass <[email protected]>
Tested-by Stefan Reinauer <[email protected]>
---
 drivers/block/ahci.c |   25 ++++++++++++++++++-------
 include/ahci.h       |    1 +
 2 files changed, 19 insertions(+), 7 deletions(-)
drivers/block/ahci.c +18 -7
index af31c97..0a7ad81 100644
@@ -194,7 +194,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 		/* set irq mask (enables interrupts) */
 		writel(DEF_PORT_IRQ, port_mmio + PORT_IRQ_MASK);
 
-		/*register linkup ports */
+		/* register linkup ports */
 		tmp = readl(port_mmio + PORT_SCR_STAT);
 		debug("Port %d status: 0x%x\n", i, tmp);
 		if ((tmp & 0xf) == 0x03)

@@ -222,12 +222,13 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 	u16 cc;
 #endif
 	volatile u8 *mmio = (volatile u8 *)probe_ent->mmio_base;
-	u32 vers, cap, impl, speed;
+	u32 vers, cap, cap2, impl, speed;
 	const char *speed_s;
 	const char *scc_s;
 
 	vers = readl(mmio + HOST_VERSION);
 	cap = probe_ent->cap;
+	cap2 = readl(mmio + HOST_CAP2);
 	impl = probe_ent->port_map;
 
 	speed = (cap >> 20) & 0xf;

@@ -235,6 +236,8 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 		speed_s = "1.5";
 	else if (speed == 2)
 		speed_s = "3";
+	else if (speed == 3)
+		speed_s = "6";
 	else
 		speed_s = "?";
 

@@ -260,8 +263,9 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 	       ((cap >> 8) & 0x1f) + 1, (cap & 0x1f) + 1, speed_s, impl, scc_s);
 
 	printf("flags: "
-	       "%s%s%s%s%s%s"
-	       "%s%s%s%s%s%s%s\n",
+	       "%s%s%s%s%s%s%s"
+	       "%s%s%s%s%s%s%s"
+	       "%s%s%s%s%s%s\n",
 	       cap & (1 << 31) ? "64bit " : "",
 	       cap & (1 << 30) ? "ncq " : "",
 	       cap & (1 << 28) ? "ilck " : "",

@@ -272,9 +276,16 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 	       cap & (1 << 19) ? "nz " : "",
 	       cap & (1 << 18) ? "only " : "",
 	       cap & (1 << 17) ? "pmp " : "",
+	       cap & (1 << 16) ? "fbss " : "",
 	       cap & (1 << 15) ? "pio " : "",
 	       cap & (1 << 14) ? "slum " : "",
-	       cap & (1 << 13) ? "part " : "");
+	       cap & (1 << 13) ? "part " : "",
+	       cap & (1 << 7) ? "ccc " : "",
+	       cap & (1 << 6) ? "ems " : "",
+	       cap & (1 << 5) ? "sxs " : "",
+	       cap2 & (1 << 2) ? "apst " : "",
+	       cap2 & (1 << 1) ? "nvmp " : "",
+	       cap2 & (1 << 0) ? "boh " : "");
 }
 
 #ifndef CONFIG_SCSI_AHCI_PLAT

@@ -369,7 +380,7 @@ static void ahci_set_feature(u8 port)
 	u32 cmd_fis_len = 5;	/* five dwords */
 	u8 fis[20];
 
-	/*set feature */
+	/* set feature */
 	memset(fis, 0, 20);
 	fis[0] = 0x27;
 	fis[1] = 1 << 7;

@@ -383,7 +394,7 @@ static void ahci_set_feature(u8 port)
 	readl(port_mmio + PORT_CMD_ISSUE);
 
 	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE, 150, 0x1)) {
-		printf("set feature error!\n");
+		printf("set feature error on port %d!\n", port);
 	}
 }
include/ahci.h +1 -0
index c4fb9e7..babbdc6 100644
@@ -51,6 +51,7 @@
 #define HOST_IRQ_STAT		0x08 /* interrupt status */
 #define HOST_PORTS_IMPL		0x0c /* bitmap of implemented ports */
 #define HOST_VERSION		0x10 /* AHCI spec. version compliancy */
+#define HOST_CAP2		0x24 /* host capabilities, extended */
 
 /* HOST_CTL bits */
 #define HOST_RESET		(1 << 0)  /* reset controller; self-clear */
Signature · 1 line
1.7.7.3
author
#10 lore raw

PATCH 8/20 ahci: Make sending the SETFEATURES_XFER command optional

In reply to Simon Glass

From: Gabe Black <[email protected]>

This command doesn't really do anything when talking to a SATA device, and sending it confuses some of them. This change makes sending the command optional, and defaults to not. The situations where it should be sent are not the common case.

With the standard SSD in the machine, here are some times with the option turned off:
1. 8277
2. 8273
3. 8050

And turned on:
1. 8303
2. 8155
3. 8276

Sending that command seems to have no meaningful effect on performance.

This fixes problems with an SSD marked Toshiba NV6424, Taiwan 11159AE P and TC58NVG5D2FTA10.

Signed-off-by Gabe Black <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/block/ahci.c +4 -0
index 0a7ad81..7b3cf8e 100644
@@ -373,6 +373,7 @@ static void ahci_fill_cmd_slot(struct ahci_ioports *pp, u32 opts)
 }
 
 
+#ifdef CONFIG_AHCI_SETFEATURES_XFER
 static void ahci_set_feature(u8 port)
 {
 	struct ahci_ioports *pp = &(probe_ent->port[port]);

@@ -397,6 +398,7 @@ static void ahci_set_feature(u8 port)
 		printf("set feature error on port %d!\n", port);
 	}
 }
+#endif
 
 
 static int ahci_port_start(u8 port)

@@ -743,7 +745,9 @@ void scsi_low_level_init(int busdevfunc)
 				printf("Can not start port %d\n", i);
 				continue;
 			}
+#ifdef CONFIG_AHCI_SETFEATURES_XFER
 			ahci_set_feature((u8) i);
+#endif
 		}
 	}
 }
Signature · 1 line
1.7.7.3
author
#11 lore raw

PATCH 9/20 ahci: support scsi writing in AHCI driver.

In reply to Simon Glass

From: Hung-Te Lin <[email protected]>

The "scsi write" command requires support from underlying driver. This CL enables SCSI_WRITE10 in AHCI driver.

Tested in U-Boot console, try to i/o with sector #64:
scsi read 1000 40 1
md.b 1000 200 # check if things are not 0xcc mw.b 1000 cc 200 # try to fill with 0xcc
scsi write 1000 40 1
mw.b 1000 0 200 # fill with zero
md.b 1000 200 # should be all 0
scsi read 1000 40 1
md.b 1000 200 # should be all 0xcc

Signed-off-by Hung-Te Lin <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   54 +++++++++++++++++++++++++++----------------------
 1 files changed, 30 insertions(+), 24 deletions(-)
drivers/block/ahci.c +30 -24
index 7b3cf8e..22b2754 100644
@@ -43,12 +43,13 @@ hd_driveid_t *ataid[AHCI_MAX_PORTS];
 #define writel_with_flush(a,b)	do { writel(a,b); readl(b); } while (0)
 
 /*
- * Some controllers limit number of blocks they can read at once. Contemporary
- * SSD devices work much faster if the read size is aligned to a power of 2.
- * Let's set default to 128 and allowing to be overwritten if needed.
+ * Some controllers limit number of blocks they can read/write at once.
+ * Contemporary SSD devices work much faster if the read/write size is aligned
+ * to a power of 2.  Let's set default to 128 and allowing to be overwritten if
+ * needed.
  */
-#ifndef MAX_SATA_BLOCKS_READ
-#define MAX_SATA_BLOCKS_READ	0x80
+#ifndef MAX_SATA_BLOCKS_READ_WRITE
+#define MAX_SATA_BLOCKS_READ_WRITE	0x80
 #endif
 
 static inline u32 ahci_port_base(u32 base, u32 port)

@@ -464,8 +465,8 @@ static int ahci_port_start(u8 port)
 }
 
 
-static int get_ahci_device_data(u8 port, u8 *fis, int fis_len, u8 *buf,
-				int buf_len)
+static int ahci_device_data_io(u8 port, u8 *fis, int fis_len, u8 *buf,
+				int buf_len, u8 is_write)
 {
 
 	struct ahci_ioports *pp = &(probe_ent->port[port]);

@@ -474,7 +475,7 @@ static int get_ahci_device_data(u8 port, u8 *fis, int fis_len, u8 *buf,
 	u32 port_status;
 	int sg_count;
 
-	debug("Enter get_ahci_device_data: for port %d\n", port);
+	debug("Enter %s: for port %d\n", __func__, port);
 
 	if (port > probe_ent->n_ports) {
 		printf("Invaild port number %d\n", port);

@@ -490,7 +491,7 @@ static int get_ahci_device_data(u8 port, u8 *fis, int fis_len, u8 *buf,
 	memcpy((unsigned char *)pp->cmd_tbl, fis, fis_len);
 
 	sg_count = ahci_fill_sg(port, buf, buf_len);
-	opts = (fis_len >> 2) | (sg_count << 16);
+	opts = (fis_len >> 2) | (sg_count << 16) | (is_write << 6);
 	ahci_fill_cmd_slot(pp, opts);
 
 	writel_with_flush(1, port_mmio + PORT_CMD_ISSUE);

@@ -499,8 +500,7 @@ static int get_ahci_device_data(u8 port, u8 *fis, int fis_len, u8 *buf,
 		printf("timeout exit!\n");
 		return -1;
 	}
-	debug("get_ahci_device_data: %d byte transferred.\n",
-	      pp->cmd_slot->status);
+	debug("%s: %d byte transferred.\n", __func__, pp->cmd_slot->status);
 
 	return 0;
 }

@@ -570,8 +570,8 @@ static int ata_scsiop_inquiry(ccb *pccb)
 	if (!(tmpid = malloc(sizeof(hd_driveid_t))))
 		return -ENOMEM;
 
-	if (get_ahci_device_data(port, (u8 *) & fis, 20,
-				 tmpid, sizeof(hd_driveid_t))) {
+	if (ahci_device_data_io(port, (u8 *) &fis, 20, tmpid,
+				sizeof(hd_driveid_t), 0)) {
 		debug("scsi_ahci: SCSI inquiry command failure.\n");
 		return -EIO;
 	}

@@ -590,9 +590,9 @@ static int ata_scsiop_inquiry(ccb *pccb)
 
 
 /*
- * SCSI READ10 command operation.
+ * SCSI READ10/WRITE10 command operation.
  */
-static int ata_scsiop_read10(ccb * pccb)
+static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 {
 	u32 lba = 0;
 	u16 blocks = 0;

@@ -616,20 +616,21 @@ static int ata_scsiop_read10(ccb * pccb)
 	 */
 	blocks = (((u16)pccb->cmd[7]) << 8) | ((u16) pccb->cmd[8]);
 
-	debug("scsi_ahci: read %d blocks starting from lba 0x%x\n",
-	      (unsigned)lba, blocks);
+	debug("scsi_ahci: %s %d blocks starting from lba 0x%x\n",
+	      is_write ?  "write" : "read", (unsigned)lba, blocks);
 
 	/* Preset the FIS */
 	memset(fis, 0, 20);
 	fis[0] = 0x27;		 /* Host to device FIS. */
 	fis[1] = 1 << 7;	 /* Command FIS. */
-	fis[2] = ATA_CMD_RD_DMA; /* Command byte. */
+	/* Command byte (read/write). */
+	fis[2] = is_write ? ATA_CMD_WR_DMA : ATA_CMD_RD_DMA;
 
 	while (blocks) {
 		u16 now_blocks; /* number of blocks per iteration */
 		u32 transfer_size; /* number of bytes per iteration */
 
-		now_blocks = min(MAX_SATA_BLOCKS_READ, blocks);
+		now_blocks = min(MAX_SATA_BLOCKS_READ_WRITE, blocks);
 
 		transfer_size = ATA_BLOCKSIZE * now_blocks;
 		if (transfer_size > user_buffer_size) {

@@ -647,10 +648,12 @@ static int ata_scsiop_read10(ccb * pccb)
 		fis[12] = (now_blocks >> 0) & 0xff;
 		fis[13] = (now_blocks >> 8) & 0xff;
 
-		/* Read from ahci */
-		if (get_ahci_device_data(pccb->target, (u8 *) &fis, sizeof(fis),
-					 user_buffer, user_buffer_size)) {
-			debug("scsi_ahci: SCSI READ10 command failure.\n");
+		/* Read/Write from ahci */
+		if (ahci_device_data_io(pccb->target, (u8 *) &fis, sizeof(fis),
+					user_buffer, user_buffer_size,
+					is_write)) {
+			debug("scsi_ahci: SCSI %s10 command failure.\n",
+			      is_write ? "WRITE" : "READ");
 			return -EIO;
 		}
 		user_buffer += transfer_size;

@@ -703,7 +706,10 @@ int scsi_exec(ccb *pccb)
 
 	switch (pccb->cmd[0]) {
 	case SCSI_READ10:
-		ret = ata_scsiop_read10(pccb);
+		ret = ata_scsiop_read_write(pccb, 0);
+		break;
+	case SCSI_WRITE10:
+		ret = ata_scsiop_read_write(pccb, 1);
 		break;
 	case SCSI_RD_CAPAC:
 		ret = ata_scsiop_read_capacity10(pccb);
Signature · 1 line
1.7.7.3
author
#12 lore raw

PATCH 10/20 ahci: Make the AHCI code find the capacity of disks > 128 GB properly

In reply to Simon Glass

From: Gabe Black <[email protected]>

In the structure returned by the ATA identify device command, there are two fields which describe the device capacity. One is a 32 bit data type which reports the number of sectors as a 28 bit LBA, and the other is a 64 bit data type which is for a 48 bit LBA. If the device doesn't support 48 bit LBAs, the small value is the only value with the correct size. If it supports more, if the number of sectors is small enough to fit into 28 bits, both fields reflect the correct value. If it's too large, the smaller field has 28 bits of 1s, 0xfffffff, and the other field has the correct value.

The AHCI driver is implemented by attaching to the generic SCSI code and translating on the fly between SCSI binary data structures and AHCI data structures. It responds to requests to execute specific SCSI commands by executing the equivalent AHCI commands and then crafting a response which matches what a SCSI disk would send.

The AHCI driver now considers both fields and chooses the correct one when implementing both the SCSI READ CAPACITY (10) and READ CAPACITY (16) commands.

Signed-off-by Gabe Black <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   55 +++++++++++++++++++++++++++++++++++++++++++++----
 include/scsi.h       |    2 +
 2 files changed, 52 insertions(+), 5 deletions(-)
drivers/block/ahci.c +50 -5
index 22b2754..e1d0881 100644
@@ -672,6 +672,7 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 static int ata_scsiop_read_capacity10(ccb *pccb)
 {
 	u32 cap;
+	u32 block_size;
 
 	if (!ataid[pccb->target]) {
 		printf("scsi_ahci: SCSI READ CAPACITY10 command failure. "

@@ -680,12 +681,53 @@ static int ata_scsiop_read_capacity10(ccb *pccb)
 		return -EPERM;
 	}
 
-	cap = be32_to_cpu(ataid[pccb->target]->lba_capacity);
+	cap = le32_to_cpu(ataid[pccb->target]->lba_capacity);
+	if (cap == 0xfffffff) {
+		unsigned short *cap48 = ataid[pccb->target]->lba48_capacity;
+		if (cap48[2] || cap48[3]) {
+			cap = 0xffffffff;
+		} else {
+			cap = (le16_to_cpu(cap48[1]) << 16) |
+			      (le16_to_cpu(cap48[0]));
+		}
+	}
+
+	cap = cpu_to_be32(cap);
 	memcpy(pccb->pdata, &cap, sizeof(cap));
 
-	pccb->pdata[4] = pccb->pdata[5] = 0;
-	pccb->pdata[6] = 512 >> 8;
-	pccb->pdata[7] = 512 & 0xff;
+	block_size = cpu_to_be32((u32)512);
+	memcpy(&pccb->pdata[4], &block_size, 4);
+
+	return 0;
+}
+
+
+/*
+ * SCSI READ CAPACITY16 command operation.
+ */
+static int ata_scsiop_read_capacity16(ccb *pccb)
+{
+	u64 cap;
+	u64 block_size;
+
+	if (!ataid[pccb->target]) {
+		printf("scsi_ahci: SCSI READ CAPACITY16 command failure. "
+		       "\tNo ATA info!\n"
+		       "\tPlease run SCSI commmand INQUIRY firstly!\n");
+		return -EPERM;
+	}
+
+	cap = le32_to_cpu(ataid[pccb->target]->lba_capacity);
+	if (cap == 0xfffffff) {
+		memcpy(&cap, ataid[pccb->target]->lba48_capacity, sizeof(cap));
+		cap = le64_to_cpu(cap);
+	}
+
+	cap = cpu_to_be64(cap);
+	memcpy(pccb->pdata, &cap, sizeof(cap));
+
+	block_size = cpu_to_be64((u64)512);
+	memcpy(&pccb->pdata[8], &block_size, 8);
 
 	return 0;
 }

@@ -711,9 +753,12 @@ int scsi_exec(ccb *pccb)
 	case SCSI_WRITE10:
 		ret = ata_scsiop_read_write(pccb, 1);
 		break;
-	case SCSI_RD_CAPAC:
+	case SCSI_RD_CAPAC10:
 		ret = ata_scsiop_read_capacity10(pccb);
 		break;
+	case SCSI_RD_CAPAC16:
+		ret = ata_scsiop_read_capacity16(pccb);
+		break;
 	case SCSI_TST_U_RDY:
 		ret = ata_scsiop_test_unit_ready(pccb);
 		break;
include/scsi.h +2 -0
index 9681d19..9da764b 100644
@@ -150,6 +150,8 @@ typedef struct SCSI_cmd_block{
 #define SCSI_READ6		0x08		/* Read 6-byte (MANDATORY) */
 #define SCSI_READ10		0x28		/* Read 10-byte (MANDATORY) */
 #define SCSI_RD_CAPAC	0x25		/* Read Capacity (MANDATORY) */
+#define SCSI_RD_CAPAC10	SCSI_RD_CAPAC	/* Read Capacity (10) */
+#define SCSI_RD_CAPAC16	0x9e		/* Read Capacity (16) */
 #define SCSI_RD_DEFECT	0x37		/* Read Defect Data (O) */
 #define SCSI_READ_LONG	0x3E		/* Read Long (O) */
 #define SCSI_REASS_BLK	0x07		/* Reassign Blocks (O) */
Signature · 1 line
1.7.7.3
author
#13 lore raw

PATCH 11/20 ahci: Use sizeof(fis) instead of hardcoding '20'

In reply to Simon Glass

From: Taylor Hutt <[email protected]>

This cleanup replaces the hardcoded use of '20', which represents the number of bytes in the FIS, with sizeof(fis).

Signed-off-by Taylor Hutt <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
drivers/block/ahci.c +5 -5
index e1d0881..2c60eb9 100644
@@ -383,14 +383,14 @@ static void ahci_set_feature(u8 port)
 	u8 fis[20];
 
 	/* set feature */
-	memset(fis, 0, 20);
+	memset(fis, 0, sizeof(fis));
 	fis[0] = 0x27;
 	fis[1] = 1 << 7;
 	fis[2] = ATA_CMD_SETF;
 	fis[3] = SETFEATURES_XFER;
 	fis[12] = __ilog2(probe_ent->udma_mask + 1) + 0x40 - 0x01;
 
-	memcpy((unsigned char *)pp->cmd_tbl, fis, 20);
+	memcpy((unsigned char *)pp->cmd_tbl, fis, sizeof(fis));
 	ahci_fill_cmd_slot(pp, cmd_fis_len);
 	writel(1, port_mmio + PORT_CMD_ISSUE);
 	readl(port_mmio + PORT_CMD_ISSUE);

@@ -559,7 +559,7 @@ static int ata_scsiop_inquiry(ccb *pccb)
 	if (pccb->datalen <= 35)
 		return 0;
 
-	memset(fis, 0, 20);
+	memset(fis, 0, sizeof(fis));
 	/* Construct the FIS */
 	fis[0] = 0x27;		/* Host to device FIS. */
 	fis[1] = 1 << 7;	/* Command FIS. */

@@ -570,7 +570,7 @@ static int ata_scsiop_inquiry(ccb *pccb)
 	if (!(tmpid = malloc(sizeof(hd_driveid_t))))
 		return -ENOMEM;
 
-	if (ahci_device_data_io(port, (u8 *) &fis, 20, tmpid,
+	if (ahci_device_data_io(port, (u8 *) &fis, sizeof(fis), tmpid,
 				sizeof(hd_driveid_t), 0)) {
 		debug("scsi_ahci: SCSI inquiry command failure.\n");
 		return -EIO;

@@ -620,7 +620,7 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 	      is_write ?  "write" : "read", (unsigned)lba, blocks);
 
 	/* Preset the FIS */
-	memset(fis, 0, 20);
+	memset(fis, 0, sizeof(fis));
 	fis[0] = 0x27;		 /* Host to device FIS. */
 	fis[1] = 1 << 7;	 /* Command FIS. */
 	/* Command byte (read/write). */
Signature · 1 line
1.7.7.3
author
#14 lore raw

PATCH 12/20 ahci: Put all uses of ahci_set_feature() under CONFIG_AHCI_SETFEATURES_XFER

In reply to Simon Glass

From: Taylor Hutt <[email protected]>

The function ahci_set_feature() is only compiled when the manifest constant 'CONFIG_AHCI_SETFEATURES_XFER' is defined, but it is used once outside of such a conditional.

This change puts all uses of the function under the same preprocessor conditional.

It also fixes a spelling error in a message which can be output to the console.

Signed-off-by Taylor Hutt <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/block/ahci.c +3 -1
index 2c60eb9..20c5336 100644
@@ -478,7 +478,7 @@ static int ahci_device_data_io(u8 port, u8 *fis, int fis_len, u8 *buf,
 	debug("Enter %s: for port %d\n", __func__, port);
 
 	if (port > probe_ent->n_ports) {
-		printf("Invaild port number %d\n", port);
+		printf("Invalid port number %d\n", port);
 		return -1;
 	}
 

@@ -839,7 +839,9 @@ int ahci_init(u32 base)
 				printf("Can not start port %d\n", i);
 				continue;
 			}
+#ifdef CONFIG_AHCI_SETFEATURES_XFER
 			ahci_set_feature((u8) i);
+#endif
 		}
 	}
 err_out:
Signature · 1 line
1.7.7.3
author
#15 lore raw

PATCH 13/20 scsi: Put 'scsi_device_list' variable under CONFIG_PCI

In reply to Simon Glass

From: Taylor Hutt <[email protected]>

This change removes warnings about 'scsi_device_list' being declared but never used.

Signed-off-by Taylor Hutt <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 common/cmd_scsi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
common/cmd_scsi.c +2 -0
index f64d6f4..f58a220 100644
@@ -60,7 +60,9 @@ struct scsi_device {
 #define SCSI_DEV_LIST {SCSI_VEND_ID, SCSI_DEV_ID}
 #endif
 
+#ifdef CONFIG_PCI
 static struct scsi_device scsi_device_list[] = { SCSI_DEV_LIST };
+#endif
 static ccb tempccb;	/* temporary scsi command buffer */
 
 static unsigned char tempbuff[512]; /* temporary data buffer */
Signature · 1 line
1.7.7.3
author
#16 lore raw

PATCH 14/20 ahci: Use virt_to_phys() to denote physical addresses for DMA

In reply to Simon Glass

From: Taylor Hutt <[email protected]>

Update the assignment of various physical memory buffers used by the SATA controller to explicitly be denoted as physical addresses.

The memory is identity-mapped, so these function calls are a nop, but they provide good semantic documentation for any maintainers.

The return value of virt_to_phys() is 'unsigned long'. On machines where sizeof(unsigned long) != sizeof(pointer), a cast through (uintptr_t) is needed to appease the compiler due to the potential of losing the upper 32 bits of the address.

In compilation this scenario, a physical address could be 64-bits, yet the C pointer environment only allows 32-bit addresses; the constraint is that pointers cannot address more than 4Gb of memory and if virt_to_phys() ever returns an out-of-range value for the physical address, there are issues with emmory mapping which must be solved. However, since the memory is identify mappeed, there is no problem introducing the cast: the original pointer will reside in 32-bits, so the physical address will also be within in 32-bits.

Signed-off-by Taylor Hutt <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/block/ahci.c +6 -4
index 20c5336..00de086 100644
@@ -431,25 +431,27 @@ static int ahci_port_start(u8 port)
 	 * First item in chunk of DMA memory: 32-slot command table,
 	 * 32 bytes each in size
 	 */
-	pp->cmd_slot = (struct ahci_cmd_hdr *)mem;
+	pp->cmd_slot =
+		(struct ahci_cmd_hdr *)(uintptr_t)virt_to_phys((void *)mem);
 	debug("cmd_slot = 0x%x\n", (unsigned)pp->cmd_slot);
 	mem += (AHCI_CMD_SLOT_SZ + 224);
 
 	/*
 	 * Second item: Received-FIS area
 	 */
-	pp->rx_fis = mem;
+	pp->rx_fis = virt_to_phys((void *)mem);
 	mem += AHCI_RX_FIS_SZ;
 
 	/*
 	 * Third item: data area for storing a single command
 	 * and its scatter-gather table
 	 */
-	pp->cmd_tbl = mem;
+	pp->cmd_tbl = virt_to_phys((void *)mem);
 	debug("cmd_tbl_dma = 0x%x\n", pp->cmd_tbl);
 
 	mem += AHCI_CMD_TBL_HDR;
-	pp->cmd_tbl_sg = (struct ahci_sg *)mem;
+	pp->cmd_tbl_sg =
+			(struct ahci_sg *)(uintptr_t)virt_to_phys((void *)mem);
 
 	writel_with_flush((u32) pp->cmd_slot, port_mmio + PORT_LST_ADDR);
Signature · 1 line
1.7.7.3
author
#17 lore raw

PATCH 15/20 ahci: flush / invalidate dcache around SATA commands

In reply to Simon Glass

From: Taylor Hutt <[email protected]>

Exynos5 automatically performs DMA when the SATA controller executes commands. This adds the necessary dcache-to-memory flush & invalidation calls to allow the DMA to properly function.

Signed-off-by Taylor Hutt <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
drivers/block/ahci.c +39 -0
index 00de086..a05d9cf 100644
@@ -70,6 +70,39 @@ static void ahci_setup_port(struct ahci_ioports *port, unsigned long base,
 
 #define msleep(a) udelay(a * 1000)
 
+static void ahci_dcache_flush_range(unsigned begin, unsigned len)
+{
+	const unsigned long start = begin;
+	const unsigned long end = start + len;
+
+	debug("%s: flush dcache: [%#lx, %#lx)\n", __func__, start, end);
+	flush_dcache_range(start, end);
+}
+
+/*
+ * SATA controller DMAs to physical RAM.  Ensure data from the
+ * controller is invalidated from dcache; next access comes from
+ * physical RAM.
+ */
+static void ahci_dcache_invalidate_range(unsigned begin, unsigned len)
+{
+	const unsigned long start = begin;
+	const unsigned long end = start + len;
+
+	debug("%s: invalidate dcache: [%#lx, %#lx)\n", __func__, start, end);
+	invalidate_dcache_range(start, end);
+}
+
+/*
+ * Ensure data for SATA controller is flushed out of dcache and
+ * written to physical memory.
+ */
+static void ahci_dcache_flush_sata_cmd(struct ahci_ioports *pp)
+{
+	ahci_dcache_flush_range((unsigned long)pp->cmd_slot,
+				AHCI_PORT_PRIV_DMA_SZ);
+}
+
 static int waiting_for_cmd_completed(volatile u8 *offset,
 				     int timeout_msec,
 				     u32 sign)

@@ -392,6 +425,7 @@ static void ahci_set_feature(u8 port)
 
 	memcpy((unsigned char *)pp->cmd_tbl, fis, sizeof(fis));
 	ahci_fill_cmd_slot(pp, cmd_fis_len);
+	ahci_dcache_flush_sata_cmd(pp);
 	writel(1, port_mmio + PORT_CMD_ISSUE);
 	readl(port_mmio + PORT_CMD_ISSUE);
 

@@ -496,12 +530,17 @@ static int ahci_device_data_io(u8 port, u8 *fis, int fis_len, u8 *buf,
 	opts = (fis_len >> 2) | (sg_count << 16) | (is_write << 6);
 	ahci_fill_cmd_slot(pp, opts);
 
+	ahci_dcache_flush_sata_cmd(pp);
+	ahci_dcache_flush_range((unsigned)buf, (unsigned)buf_len);
+
 	writel_with_flush(1, port_mmio + PORT_CMD_ISSUE);
 
 	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE, 150, 0x1)) {
 		printf("timeout exit!\n");
 		return -1;
 	}
+
+	ahci_dcache_invalidate_range((unsigned)buf, (unsigned)buf_len);
 	debug("%s: %d byte transferred.\n", __func__, pp->cmd_slot->status);
 
 	return 0;
Signature · 1 line
1.7.7.3
author
#18 lore raw

PATCH 16/20 ahci: Adjust SATA timeouts for hard disk (spinup delay & command timeout)

In reply to Simon Glass

From: Walter Murphy <[email protected]>

Note: These are timeout values and not delay values, so the event being timed out will complete whenever it is actually ready, with a measurement granularity of 1 millisecond, up till the timeout value. Therefore, there is no effect on SSD booting.

The values were determined by instrumenting the code and measuring the actual time taken by several different models of HDD for each of the parameters and then adding 50% more for the spinup value and just doubling the command timeout value.

Signed-off-by Walter Murphy <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)
drivers/block/ahci.c +10 -4
index a05d9cf..719574f 100644
@@ -52,6 +52,10 @@ hd_driveid_t *ataid[AHCI_MAX_PORTS];
 #define MAX_SATA_BLOCKS_READ_WRITE	0x80
 #endif
 
+/* Maximum timeouts for each event */
+#define WAIT_MS_DATAIO	5000
+#define WAIT_MS_LINKUP	4
+
 static inline u32 ahci_port_base(u32 base, u32 port)
 {
 	return base + 0x100 + (port * 0x80);

@@ -201,14 +205,14 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 		writel(PORT_CMD_SPIN_UP, port_mmio + PORT_CMD);
 
 		j = 0;
-		while (j < 1000) {
+		while (j < WAIT_MS_LINKUP) {
 			tmp = readl(port_mmio + PORT_SCR_STAT);
 			if ((tmp & 0xf) == 0x3)
 				break;
 			udelay(1000);
 			j++;
 		}
-		if (j == 1000)
+		if (j == WAIT_MS_LINKUP)
 			debug("timeout.\n");
 		else
 			debug("ok.\n");

@@ -429,7 +433,8 @@ static void ahci_set_feature(u8 port)
 	writel(1, port_mmio + PORT_CMD_ISSUE);
 	readl(port_mmio + PORT_CMD_ISSUE);
 
-	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE, 150, 0x1)) {
+	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE,
+				WAIT_MS_DATAIO, 0x1)) {
 		printf("set feature error on port %d!\n", port);
 	}
 }

@@ -535,7 +540,8 @@ static int ahci_device_data_io(u8 port, u8 *fis, int fis_len, u8 *buf,
 
 	writel_with_flush(1, port_mmio + PORT_CMD_ISSUE);
 
-	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE, 150, 0x1)) {
+	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE,
+				WAIT_MS_DATAIO, 0x1)) {
 		printf("timeout exit!\n");
 		return -1;
 	}
Signature · 1 line
1.7.7.3
author
#19 lore raw

PATCH 17/20 ahci: Support spin-up and link-up separately

In reply to Simon Glass

From: Marc Jones <[email protected]>

Add HDD handling to the SSD-only AHCI driver, by separately dealing with spin-up and link-up.

Signed-off-by Marc Jones <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   45 +++++++++++++++++++++++++++++++++++++++------
 1 files changed, 39 insertions(+), 6 deletions(-)
drivers/block/ahci.c +39 -6
index 719574f..19c5f13 100644
@@ -53,6 +53,7 @@ hd_driveid_t *ataid[AHCI_MAX_PORTS];
 #endif
 
 /* Maximum timeouts for each event */
+#define WAIT_MS_SPINUP	10000
 #define WAIT_MS_DATAIO	5000
 #define WAIT_MS_LINKUP	4
 

@@ -129,7 +130,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 	unsigned short vendor;
 #endif
 	volatile u8 *mmio = (volatile u8 *)probe_ent->mmio_base;
-	u32 tmp, cap_save;
+	u32 tmp, cap_save, cmd;
 	int i, j;
 	volatile u8 *port_mmio;
 

@@ -137,7 +138,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 
 	cap_save = readl(mmio + HOST_CAP);
 	cap_save &= ((1 << 28) | (1 << 17));
-	cap_save |= (1 << 27);
+	cap_save |= (1 << 27);  /* Staggered Spin-up. Not needed. */
 
 	/* global controller reset */
 	tmp = readl(mmio + HOST_CTL);

@@ -201,9 +202,18 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 			msleep(500);
 		}
 
-		debug("Spinning up port %d... ", i);
-		writel(PORT_CMD_SPIN_UP, port_mmio + PORT_CMD);
-
+		/* Add the spinup command to whatever mode bits may
+		 * already be on in the command register.
+		 */
+		cmd = readl(port_mmio + PORT_CMD);
+		cmd |= PORT_CMD_FIS_RX;
+		cmd |= PORT_CMD_SPIN_UP;
+		writel_with_flush(cmd, port_mmio + PORT_CMD);
+
+		/* Bring up SATA link.
+		 * SATA link bringup time is usually less than 1 ms; only very
+		 * rarely has it taken between 1-2 ms. Never seen it above 2 ms.
+		 */
 		j = 0;
 		while (j < WAIT_MS_LINKUP) {
 			tmp = readl(port_mmio + PORT_SCR_STAT);

@@ -212,7 +222,30 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 			udelay(1000);
 			j++;
 		}
-		if (j == WAIT_MS_LINKUP)
+		if (j == WAIT_MS_LINKUP) {
+			printf("SATA link %d timeout.\n", i);
+			continue;
+		} else {
+			debug("SATA link ok.\n");
+		}
+
+		/* Clear error status */
+		tmp = readl(port_mmio + PORT_SCR_ERR);
+		if (tmp)
+			writel(tmp, port_mmio + PORT_SCR_ERR);
+
+		debug("Spinning up device on SATA port %d... ", i);
+
+		j = 0;
+		while (j < WAIT_MS_SPINUP) {
+			tmp = readl(port_mmio + PORT_TFDATA);
+			if (!(tmp & (ATA_STAT_BUSY | ATA_STAT_DRQ)))
+				break;
+			udelay(1000);
+			j++;
+		}
+		printf("Target spinup took %d ms.\n", j);
+		if (j == WAIT_MS_SPINUP)
 			debug("timeout.\n");
 		else
 			debug("ok.\n");
Signature · 1 line
1.7.7.3
author
#20 lore raw

PATCH 18/20 ahci: Perform SATA flush after disk write.

In reply to Simon Glass

From: Marc Jones <[email protected]>

Writes in u-boot are so rare, and the logic to know when is the last write and do a flush only there is sufficiently difficult. Just do a flush after every write. This incurs, usually, one extra flush when the rare writes do happen.

Signed-off-by Marc Jones <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++-
 include/ata.h        |    3 ++
 2 files changed, 54 insertions(+), 1 deletions(-)
drivers/block/ahci.c +51 -1
index 19c5f13..963efec 100644
@@ -37,6 +37,8 @@
 #include <linux/ctype.h>
 #include <ahci.h>
 
+static int ata_io_flush(u8 port);
+
 struct ahci_probe_ent *probe_ent = NULL;
 hd_driveid_t *ataid[AHCI_MAX_PORTS];
 

@@ -55,6 +57,7 @@ hd_driveid_t *ataid[AHCI_MAX_PORTS];
 /* Maximum timeouts for each event */
 #define WAIT_MS_SPINUP	10000
 #define WAIT_MS_DATAIO	5000
+#define WAIT_MS_FLUSH	5000
 #define WAIT_MS_LINKUP	4
 
 static inline u32 ahci_port_base(u32 base, u32 port)

@@ -267,7 +270,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 
 		/* register linkup ports */
 		tmp = readl(port_mmio + PORT_SCR_STAT);
-		debug("Port %d status: 0x%x\n", i, tmp);
+		debug("SATA port %d status: 0x%x\n", i, tmp);
 		if ((tmp & 0xf) == 0x03)
 			probe_ent->link_port_map |= (0x01 << i);
 	}

@@ -736,6 +739,17 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 			      is_write ? "WRITE" : "READ");
 			return -EIO;
 		}
+
+		/* If this transaction is a write, do a following flush.
+		 * Writes in u-boot are so rare, and the logic to know when is
+		 * the last write and do a flush only there is sufficiently
+		 * difficult. Just do a flush after every write. This incurs,
+		 * usually, one extra flush when the rare writes do happen.
+		 */
+		if (is_write) {
+			if (-EIO == ata_io_flush(pccb->target))
+				return -EIO;
+		}
 		user_buffer += transfer_size;
 		user_buffer_size -= transfer_size;
 		blocks -= now_blocks;

@@ -929,6 +943,42 @@ err_out:
 }
 #endif
 
+/*
+ * In the general case of generic rotating media it makes sense to have a
+ * flush capability. It probably even makes sense in the case of SSDs because
+ * one cannot always know for sure what kind of internal cache/flush mechanism
+ * is embodied therein. At first it was planned to invoke this after the last
+ * write to disk and before rebooting. In practice, knowing, a priori, which
+ * is the last write is difficult. Because writing to the disk in u-boot is
+ * very rare, this flush command will be invoked after every block write.
+ */
+static int ata_io_flush(u8 port)
+{
+	u8 fis[20];
+	struct ahci_ioports *pp = &(probe_ent->port[port]);
+	volatile u8 *port_mmio = (volatile u8 *)pp->port_mmio;
+	u32 cmd_fis_len = 5;	/* five dwords */
+
+	/* Preset the FIS */
+	memset(fis, 0, 20);
+	fis[0] = 0x27;		 /* Host to device FIS. */
+	fis[1] = 1 << 7;	 /* Command FIS. */
+	fis[2] = ATA_CMD_FLUSH;
+
+	memcpy((unsigned char *)pp->cmd_tbl, fis, 20);
+	ahci_fill_cmd_slot(pp, cmd_fis_len);
+	writel_with_flush(1, port_mmio + PORT_CMD_ISSUE);
+
+	if (waiting_for_cmd_completed(port_mmio + PORT_CMD_ISSUE,
+			WAIT_MS_FLUSH, 0x1)) {
+		debug("scsi_ahci: flush command timeout on port %d.\n", port);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+
 void scsi_bus_reset(void)
 {
 	/*Not implement*/
include/ata.h +3 -0
index 3b2d737..a614724 100644
@@ -114,6 +114,9 @@
 #define ATA_CMD_WRITE_EXT	0x34	/* Write Sectores (with retries) with 48bit addressing */
 #define ATA_CMD_VRFY_EXT	0x42	/* Read Verify	(with retries)	with 48bit addressing */
 
+#define ATA_CMD_FLUSH 0xE7 /* Flush drive cache */
+#define ATA_CMD_FLUSH_EXT 0xEA /* Flush drive cache, with 48bit addressing */
+
 /*
  * ATAPI Commands
  */
Signature · 1 line
1.7.7.3
author
#21 lore raw

PATCH 19/20 ahci: Expand HDD Logical Block addressability up to 32 bits

In reply to Simon Glass

From: Walter Murphy <[email protected]>

Currently, this driver uses a 28bit interface to AHCI, this limits the number of blocks addressable to 2^28, or the max disk size to 512(2^28) or about 137GB. This change allows supporting drives up to about 2TB.

Testing this is a bit difficult. There is test code that can be inserted into U-Boot that will write test patterns into certain unused blocks. These patterns can be manually checked using 'dd' after boot. Another way is to confirm the original error that exposed this bug is fixed. IOW: see if AU (Auto Update) will now work on the drive. Also, check that there are no warning messages from the 'cgpt' utility.

Signed-off-by Walter Murphy <[email protected]>
Signed-off-by Simon Glass <[email protected]>
---
 drivers/block/ahci.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)
drivers/block/ahci.c +8 -4
index 963efec..8c785ae 100644
@@ -707,7 +707,7 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 	fis[0] = 0x27;		 /* Host to device FIS. */
 	fis[1] = 1 << 7;	 /* Command FIS. */
 	/* Command byte (read/write). */
-	fis[2] = is_write ? ATA_CMD_WR_DMA : ATA_CMD_RD_DMA;
+	fis[2] = is_write ? ATA_CMD_WRITE_EXT : ATA_CMD_READ_EXT;
 
 	while (blocks) {
 		u16 now_blocks; /* number of blocks per iteration */

@@ -721,11 +721,15 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 			return -EIO;
 		}
 
-		/* LBA address, only support LBA28 in this driver */
+		/* LBA48 SATA command but only use 32bit address range within
+		 * that. The next smaller command range (28bit) is too small.
+		 */
 		fis[4] = (lba >> 0) & 0xff;
 		fis[5] = (lba >> 8) & 0xff;
 		fis[6] = (lba >> 16) & 0xff;
-		fis[7] = ((lba >> 24) & 0xf) | 0xe0;
+		fis[7] = 1 << 6; /* device reg: set LBA mode */
+		fis[8] = ((lba >> 24) & 0xff);
+		fis[3] = 0xe0; /* features */
 
 		/* Block (sector) count */
 		fis[12] = (now_blocks >> 0) & 0xff;

@@ -963,7 +967,7 @@ static int ata_io_flush(u8 port)
 	memset(fis, 0, 20);
 	fis[0] = 0x27;		 /* Host to device FIS. */
 	fis[1] = 1 << 7;	 /* Command FIS. */
-	fis[2] = ATA_CMD_FLUSH;
+	fis[2] = ATA_CMD_FLUSH_EXT;
 
 	memcpy((unsigned char *)pp->cmd_tbl, fis, 20);
 	ahci_fill_cmd_slot(pp, cmd_fis_len);
Signature · 1 line
1.7.7.3
author
#22 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

In reply to Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +21 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
 
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL		1
+#define CONFIG_SCSI_DEV_LIST		{PCI_VENDOR_ID_INTEL, \
+			PCI_DEVICE_ID_INTEL_NM10_AHCI},	      \
+	{PCI_VENDOR_ID_INTEL,		\
+			PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE}, \
+	{PCI_VENDOR_ID_INTEL, \
+			PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \
+	{PCI_VENDOR_ID_INTEL,		\
+			PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID	2
+#define CONFIG_SYS_SCSI_MAX_LUN		1
+#define CONFIG_SYS_SCSI_MAX_DEVICE	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
+					 CONFIG_SYS_SCSI_MAX_LUN)
+#endif
+
 /*-----------------------------------------------------------------------
  * Real Time Clock Configuration
  */
Signature · 1 line
1.7.7.3
#23 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Simon,

Quoted 19 lines · Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +4 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
 
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+

include/configs/coreboot.h line 61

Quoted 10 lines
include/configs/coreboot.h +10 -0
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL              1
+#define CONFIG_SCSI_DEV_LIST           {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_NM10_AHCI},       \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE}, \
+       {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}

This implies every coreboot board is Intel. When you start to introduce hardware specific U-Boot components, you need to introduce a board specific config file.

Would it be better to have a CONFIG_X86_COREBOOT and a coreboot 'SoC' and no coreboot board?

Regards,

Graeme

author
#24 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Graeme,

Quoted 37 lines · Graeme Russ

Hi Simon,

Quoted 19 lines · Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +4 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
 
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+

include/configs/coreboot.h line 61

Quoted 10 lines
include/configs/coreboot.h +10 -0
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL              1
+#define CONFIG_SCSI_DEV_LIST           {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_NM10_AHCI},       \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE}, \
+       {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}

This implies every coreboot board is Intel. When you start to introduce hardware specific U-Boot components, you need to introduce a board specific config file.

Quoted 2 lines

Would it be better to have a CONFIG_X86_COREBOOT and a coreboot 'SoC' and no coreboot board?

I am not sure about using the SOC - after all we might need that concept soon on x86. Maybe we should create a new board config that includes coreboot.h?

Having said that I'm not sure how important it is right now. So far, coreboot.h is actually a particular class of boards, all Intel based. We can name it whatever we want when we actually have other boards which are coreboot but not Intel. Up to you....

Regards,
SImon

Quoted 4 lines

Regards,

Graeme

#25 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Simon,

Quoted 40 lines · Simon Glass

Hi Graeme,

Quoted 37 lines · Graeme Russ

Hi Simon,

Quoted 19 lines · Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +4 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
 
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+

include/configs/coreboot.h line 61

Quoted 10 lines
include/configs/coreboot.h +10 -0
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL              1
+#define CONFIG_SCSI_DEV_LIST           {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_NM10_AHCI},       \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE}, \
+       {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}

This implies every coreboot board is Intel. When you start to introduce hardware specific U-Boot components, you need to introduce a board specific config file.

Quoted 6 lines
Quoted 2 lines

Would it be better to have a CONFIG_X86_COREBOOT and a coreboot 'SoC' and no coreboot board?

I am not sure about using the SOC - after all we might need that concept soon on x86. Maybe we should create a new board config that includes coreboot.h?

SoC was the wrong abstraction - I think coreboot library is better (see my email I just sent)

Quoted 4 lines

Having said that I'm not sure how important it is right now. So far, coreboot.h is actually a particular class of boards, all Intel based. We can name it whatever we want when we actually have other boards which are coreboot but not Intel. Up to you....

I plan on doing dev work on a AMD E350 based board 'soon'. The E350 is already supported by coreboot, so I'm planning on getting coreboot ported for this board and then run U-Boot from coreboot. So we can no longer assume all coreboot boards will be Intel based.

Regards,

Graeme

author
#26 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Graeme,

Quoted 46 lines · Graeme Russ

Hi Simon,

Quoted 43 lines · Simon Glass

Hi Graeme,

Quoted 39 lines · Graeme Russ

Hi Simon,

Quoted 19 lines · Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +4 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
 
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+

include/configs/coreboot.h line 61

Quoted 10 lines
include/configs/coreboot.h +10 -0
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL              1
+#define CONFIG_SCSI_DEV_LIST           {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_NM10_AHCI},       \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE}, \
+       {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}

This implies every coreboot board is Intel. When you start to introduce hardware specific U-Boot components, you need to introduce a board specific config file.

Would it be better to have a CONFIG_X86_COREBOOT and a coreboot 'SoC' and no coreboot board?

Quoted 6 lines
Quoted 3 lines

I am not sure about using the SOC - after all we might need that concept soon on x86. Maybe we should create a new board config that includes coreboot.h?

SoC was the wrong abstraction - I think coreboot library is better (see my email I just sent)

Yes, ok. I can do a patch to move it, or do you want to? Presumably this would come in after the patches that are already pending on the mailing list?

Quoted 10 lines
Quoted 5 lines

Having said that I'm not sure how important it is right now. So far, coreboot.h is actually a particular class of boards, all Intel based. We can name it whatever we want when we actually have other boards which are coreboot but not Intel. Up to you....

I plan on doing dev work on a AMD E350 based board 'soon'. The E350 is already supported by coreboot, so I'm planning on getting coreboot ported for this board and then run U-Boot from coreboot. So we can no longer assume all coreboot boards will be Intel based.

Sounds good! Shall we rename coreboot.h to something like chromebook-x86.h?

Regards,
Simon

Quoted 4 lines

Regards,

Graeme

#27 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Simon,

Quoted 4 lines · Simon Glass

Hi Graeme,

On Mon, Oct 22, 2012 at 10:38 PM, Graeme Russ <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Hi Simon,

Quoted 3 lines · Simon Glass

Hi Graeme,

On Thu, Oct 18, 2012 at 9:12 PM, Graeme Russ <[email protected]>

Quoted 18 lines · wrote:
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines

Hi Simon,

Quoted 15 lines · Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +0 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

include/configs/coreboot.h line 62

Quoted 10 lines
include/configs/coreboot.h +10 -0
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL              1
+#define CONFIG_SCSI_DEV_LIST           {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_NM10_AHCI},       \
+       {PCI_VENDOR_ID_INTEL,           \
+

PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE}, \

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

include/configs/coreboot.h line 59

Quoted 2 lines
include/configs/coreboot.h +2 -0
+       {PCI_VENDOR_ID_INTEL, \
+

PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines

include/configs/coreboot.h line 61

Quoted 2 lines
include/configs/coreboot.h +2 -0
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}

This implies every coreboot board is Intel. When you start to introduce hardware specific U-Boot components, you need to introduce a board specific config file.

Would it be better to have a CONFIG_X86_COREBOOT and a coreboot 'SoC' and no coreboot board?

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

I am not sure about using the SOC - after all we might need that concept soon on x86. Maybe we should create a new board config that includes coreboot.h?

SoC was the wrong abstraction - I think coreboot library is better (see my email I just sent)

Yes, ok. I can do a patch to move it, or do you want to?

I think it would be best for you to move it.

Presumably

Quoted 2 lines

this would come in after the patches that are already pending on the mailing list?

Yes. No big hurry

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

Having said that I'm not sure how important it is right now. So far, coreboot.h is actually a particular class of boards, all Intel based. We can name it whatever we want when we actually have other boards which are coreboot but not Intel. Up to you....

Quoted 6 lines
Quoted 4 lines

I plan on doing dev work on a AMD E350 based board 'soon'. The E350 is already supported by coreboot, so I'm planning on getting coreboot ported for this board and then run U-Boot from coreboot. So we can no longer assume all coreboot boards will be Intel based.

Sounds good! Shall we rename coreboot.h to something like

chromebook-x86.h?

Even better would be to use the model name (which I assume would make the x86 tag redundant)

Regards,

Graeme

author
#28 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Graeme,

Quoted 86 lines · Graeme Russ

Hi Simon,

Quoted 54 lines · Simon Glass

Hi Graeme,

Quoted 49 lines · Graeme Russ

Hi Simon,

Quoted 46 lines · Simon Glass

Hi Graeme,

Quoted 41 lines · Graeme Russ

Hi Simon,

Quoted 21 lines · Simon Glass

Enable AHCI driver for Intel SATA devices.

Signed-off-by Simon Glass <[email protected]>
---
 include/configs/coreboot.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
include/configs/coreboot.h +6 -0
index 3df085b..968a9c5 100644
@@ -45,6 +45,27 @@
 #undef CONFIG_WATCHDOG
 #undef CONFIG_HW_WATCHDOG
 
+/* SATA AHCI storage */
+
+#define CONFIG_SCSI_AHCI
+
+#ifdef CONFIG_SCSI_AHCI
+#define CONFIG_SATA_INTEL              1

include/configs/coreboot.h line 63

Quoted 10 lines
include/configs/coreboot.h +8 -0
+#define CONFIG_SCSI_DEV_LIST           {PCI_VENDOR_ID_INTEL, \
+                       PCI_DEVICE_ID_INTEL_NM10_AHCI},       \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_MOBILE},
 \
+       {PCI_VENDOR_ID_INTEL, \
+
 PCI_DEVICE_ID_INTEL_COUGARPOINT_AHCI_SERIES6}, \
+       {PCI_VENDOR_ID_INTEL,           \
+                       PCI_DEVICE_ID_INTEL_PANTHERPOINT_AHCI_MOBILE}

This implies every coreboot board is Intel. When you start to introduce hardware specific U-Boot components, you need to introduce a board specific config file.

Would it be better to have a CONFIG_X86_COREBOOT and a coreboot 'SoC' and no coreboot board?

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

I am not sure about using the SOC - after all we might need that concept soon on x86. Maybe we should create a new board config that includes coreboot.h?

SoC was the wrong abstraction - I think coreboot library is better (see my email I just sent)

Yes, ok. I can do a patch to move it, or do you want to?

I think it would be best for you to move it.

Presumably

Quoted 2 lines

this would come in after the patches that are already pending on the mailing list?

Yes. No big hurry

Quoted 12 lines
Quoted 11 lines
Quoted 6 lines

Having said that I'm not sure how important it is right now. So far, coreboot.h is actually a particular class of boards, all Intel based. We can name it whatever we want when we actually have other boards which are coreboot but not Intel. Up to you....

I plan on doing dev work on a AMD E350 based board 'soon'. The E350 is already supported by coreboot, so I'm planning on getting coreboot ported for this board and then run U-Boot from coreboot. So we can no longer assume all coreboot boards will be Intel based.

Quoted 5 lines
Quoted 2 lines

Sounds good! Shall we rename coreboot.h to something like chromebook-x86.h?

Even better would be to use the model name (which I assume would make the x86 tag redundant)

Well, the model is very device-specific. So far as U-Boot cares all the models are basically the same. What differences there are are taken care of by device tree differences. This is try on ARM for machines which share the some SOC, and true for basically all x86 platforms.

Regards,
Simon

Quoted 4 lines

Regards,

Graeme

#29 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Simon,

Quoted 13 lines · Simon Glass

Hi Graeme,

Quoted 10 lines · Graeme Russ

Hi Simon,

Quoted 7 lines · Simon Glass

Hi Graeme,

Quoted 2 lines · Graeme Russ

Hi Simon,

[snip]

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines

I plan on doing dev work on a AMD E350 based board 'soon'. The E350 is already supported by coreboot, so I'm planning on getting coreboot ported for this board and then run U-Boot from coreboot. So we can no longer assume all coreboot boards will be Intel based.

Sounds good! Shall we rename coreboot.h to something like chromebook-x86.h?

Quoted 8 lines
Quoted 2 lines

Even better would be to use the model name (which I assume would make the x86 tag redundant)

Well, the model is very device-specific. So far as U-Boot cares all the models are basically the same. What differences there are are taken care of by device tree differences. This is try on ARM for machines which share the some SOC, and true for basically all x86 platforms.

Key words: "So far" :) - If all the x86 chromebooks are essentially the same as far as U-Boot is concerned, then chromebook-x86.h will be fine.

I assume the plan is to create a common U-Boot binary with drivers for all supported chromebook hardware and use the device tree (initially built by passing coreboot tables, but maybe one day in the future passed by coreboot) to determine which drivers to initialise? The device tree would then be further fleshed out by U-Boot and passed to the Linux kernel?

Regards,

Graeme

author
#30 lore raw

PATCH 20/20 x86: config: Enable AHCI support for coreboot

Hi Graeme,

Quoted 30 lines · Graeme Russ

Hi Simon,

Quoted 13 lines · Simon Glass

Hi Graeme,

Quoted 10 lines · Graeme Russ

Hi Simon,

Quoted 7 lines · Simon Glass

Hi Graeme,

Quoted 2 lines · Graeme Russ

Hi Simon,

[snip]

Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
Quoted 4 lines

I plan on doing dev work on a AMD E350 based board 'soon'. The E350 is already supported by coreboot, so I'm planning on getting coreboot ported for this board and then run U-Boot from coreboot. So we can no longer assume all coreboot boards will be Intel based.

Sounds good! Shall we rename coreboot.h to something like chromebook-x86.h?

Even better would be to use the model name (which I assume would make the x86 tag redundant)

Quoted 8 lines
Quoted 5 lines

Well, the model is very device-specific. So far as U-Boot cares all the models are basically the same. What differences there are are taken care of by device tree differences. This is try on ARM for machines which share the some SOC, and true for basically all x86 platforms.

Key words: "So far" :) - If all the x86 chromebooks are essentially the same as far as U-Boot is concerned, then chromebook-x86.h will be fine.

OK

Quoted 6 lines

I assume the plan is to create a common U-Boot binary with drivers for all supported chromebook hardware and use the device tree (initially built by passing coreboot tables, but maybe one day in the future passed by coreboot) to determine which drivers to initialise? The device tree would then be further fleshed out by U-Boot and passed to the Linux kernel?

Yes that's right, except that we don't currently pass an fdt to the kernel. I suspect we will start doing this at some point.

Regards,
Simon

Quoted 4 lines

Regards,

Graeme

#31 lore raw

cover AHCI / SATA Improvements

In reply to Simon Glass
Quoted 4 lines · Simon Glass

This series contains a set of improvements for the SATA susbsystem, mostly targeted at solid-state drivers and improving start-up time.

The patches are tested on various x86 Chromebooks.

This unfortunately breaks some mpc85xx powerpc boards. Please grab ELDK 5.2.x for a toolchain if you don't have one handy already and MAKEALL --cpu mpc85xx (there's a few different errors and warnings popping up). Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121026/959c6b0b/attachment.pgp>
author
#32 lore raw

cover AHCI / SATA Improvements

Hi Tom,

Quoted 10 lines · Tom Rini
Quoted 4 lines · Simon Glass

This series contains a set of improvements for the SATA susbsystem, mostly targeted at solid-state drivers and improving start-up time.

The patches are tested on various x86 Chromebooks.

This unfortunately breaks some mpc85xx powerpc boards. Please grab ELDK 5.2.x for a toolchain if you don't have one handy already and MAKEALL --cpu mpc85xx (there's a few different errors and warnings popping up).

Hmmm I did run MAKEALL, but must have done something wrong there. I will try again. Thanks.

Regards,
Simon

Quoted 4 lines

Thanks!

--
Tom