diff mbox

[U-Boot,5/6] scsi/ahci: add support for non-PCI controllers

Message ID 1309275583-11763-6-git-send-email-robherring2@gmail.com
State Changes Requested
Headers show

Commit Message

Rob Herring June 28, 2011, 3:39 p.m. UTC 
From: Rob Herring <rob.herring@calxeda.com>

Add support for AHCI controllers that are not PCI based.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Wolfgang Denk <wd@denx.de>
---
 common/cmd_scsi.c    |    6 +++-
 drivers/block/ahci.c |   70 +++++++++++++++++++++++++++++++++++++++++++------
 include/ahci.h       |    4 +++
 include/scsi.h       |    1 +
 4 files changed, 70 insertions(+), 11 deletions(-)

Comments

Wolfgang Denk July 4, 2011, 10:17 a.m. UTC | #1 
Dear Rob Herring,

In message <1309275583-11763-6-git-send-email-robherring2@gmail.com> you wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add support for AHCI controllers that are not PCI based.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  common/cmd_scsi.c    |    6 +++-
>  drivers/block/ahci.c |   70 +++++++++++++++++++++++++++++++++++++++++++------
>  include/ahci.h       |    4 +++
>  include/scsi.h       |    1 +
>  4 files changed, 70 insertions(+), 11 deletions(-)
> 
> diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c
> index be4fe74..25a8299 100644
> --- a/common/cmd_scsi.c
> +++ b/common/cmd_scsi.c
> @@ -47,7 +47,8 @@
>  #define SCSI_DEV_ID  0x5288
>  
>  #else
> -#error no scsi device defined
> +#define SCSI_VEND_ID 0
> +#define SCSI_DEV_ID  0
>  #endif

I'm not sure if this is a good idea.  Please explain.

Also, checkpatch says:

ERROR: trailing whitespace
WARNING: please, no spaces at the start of a line
#287: FILE: include/ahci.h:193:
+ $

> +#ifdef CONFIG_PCI
>  	pci_read_config_word(pdev, 0x0a, &cc);
>  	if (cc == 0x0101)
>  		scc_s = "IDE";
> @@ -222,7 +227,9 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent)
>  		scc_s = "RAID";
>  	else
>  		scc_s = "unknown";
> -
> +#else
> +	scc_s = "SATA";
> +#endif

Is SATA really the only possible option here?

> +int ahci_init(u32 base)
> +{
...
> +}

Should this always be compiled in?

Best regards,

Wolfgang Denk
Rob Herring July 4, 2011, 2:51 p.m. UTC | #2 
Wolfgang,

On 07/04/2011 05:17 AM, Wolfgang Denk wrote:
> Dear Rob Herring,
> 
> In message <1309275583-11763-6-git-send-email-robherring2@gmail.com> you wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Add support for AHCI controllers that are not PCI based.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Wolfgang Denk <wd@denx.de>
>> ---
>>  common/cmd_scsi.c    |    6 +++-
>>  drivers/block/ahci.c |   70 +++++++++++++++++++++++++++++++++++++++++++------
>>  include/ahci.h       |    4 +++
>>  include/scsi.h       |    1 +
>>  4 files changed, 70 insertions(+), 11 deletions(-)
>>
>> diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c
>> index be4fe74..25a8299 100644
>> --- a/common/cmd_scsi.c
>> +++ b/common/cmd_scsi.c
>> @@ -47,7 +47,8 @@
>>  #define SCSI_DEV_ID  0x5288
>>  
>>  #else
>> -#error no scsi device defined
>> +#define SCSI_VEND_ID 0
>> +#define SCSI_DEV_ID  0
>>  #endif
> 
> I'm not sure if this is a good idea.  Please explain.

This is the PCI ID and is only used here:

common/cmd_scsi.c:183:
busdevfunc=pci_find_device(SCSI_VEND_ID,SCSI_DEV_ID,0); /* get PCI
Device ID */

For a non-PCI AHCI controller, there is no id. For PCI, I don't think 0
is a valid vendor ID anyway.

> 
> Also, checkpatch says:
> 
> ERROR: trailing whitespace
> WARNING: please, no spaces at the start of a line
> #287: FILE: include/ahci.h:193:
> + $
> 
>> +#ifdef CONFIG_PCI
>>  	pci_read_config_word(pdev, 0x0a, &cc);
>>  	if (cc == 0x0101)
>>  		scc_s = "IDE";
>> @@ -222,7 +227,9 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent)
>>  		scc_s = "RAID";
>>  	else
>>  		scc_s = "unknown";
>> -
>> +#else
>> +	scc_s = "SATA";
>> +#endif
> 
> Is SATA really the only possible option here?

Well I suppose anything is possible, but for embbedded SOCs with AHCI
I've seen, they are SATA only. It's purely informational.

> 
>> +int ahci_init(u32 base)
>> +{
> ...
>> +}
> 
> Should this always be compiled in?

I can add a config option CONFIG_SCSI_AHCI_PLAT. Perhaps this would be
better than using CONFIG_PCI as I suppose you could have non-PCI AHCI
controller on a platform with PCI.

Rob
Wolfgang Denk July 4, 2011, 3:11 p.m. UTC | #3 
Dear Rob,

In message <4E11D372.8090708@calxeda.com> you wrote:
> 
> >> -#error no scsi device defined
> >> +#define SCSI_VEND_ID 0
> >> +#define SCSI_DEV_ID  0
> >>  #endif
> > 
> > I'm not sure if this is a good idea.  Please explain.
> 
> This is the PCI ID and is only used here:
> 
> common/cmd_scsi.c:183:
> busdevfunc=pci_find_device(SCSI_VEND_ID,SCSI_DEV_ID,0); /* get PCI
> Device ID */
> 
> For a non-PCI AHCI controller, there is no id. For PCI, I don't think 0
> is a valid vendor ID anyway.

I think we should rather skip the respective parts of the code instead
of inserting invalid vendor IDs.

> > Should this always be compiled in?
> 
> I can add a config option CONFIG_SCSI_AHCI_PLAT. Perhaps this would be
> better than using CONFIG_PCI as I suppose you could have non-PCI AHCI
> controller on a platform with PCI.

Yes, please.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c
index be4fe74..25a8299 100644
--- a/common/cmd_scsi.c
+++ b/common/cmd_scsi.c
@@ -47,7 +47,8 @@ 
 #define SCSI_DEV_ID  0x5288
 
 #else
-#error no scsi device defined
+#define SCSI_VEND_ID 0
+#define SCSI_DEV_ID  0
 #endif
 
 
@@ -174,7 +175,7 @@  removable:
 		scsi_curr_dev = -1;
 }
 
-
+#ifdef CONFIG_PCI
 void scsi_init(void)
 {
 	int busdevfunc;
@@ -192,6 +193,7 @@  void scsi_init(void)
 	scsi_low_level_init(busdevfunc);
 	scsi_scan(1);
 }
+#endif
 
 block_dev_desc_t * scsi_get_dev(int dev)
 {
diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c
index d431c5a..d12cb71 100644
--- a/drivers/block/ahci.c
+++ b/drivers/block/ahci.c
@@ -78,13 +78,15 @@  static int waiting_for_cmd_completed(volatile u8 *offset,
 
 static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 {
+#ifdef CONFIG_PCI
 	pci_dev_t pdev = probe_ent->dev;
+	u16 tmp16;
+	unsigned short vendor;
+#endif
 	volatile u8 *mmio = (volatile u8 *)probe_ent->mmio_base;
 	u32 tmp, cap_save;
-	u16 tmp16;
 	int i, j;
 	volatile u8 *port_mmio;
-	unsigned short vendor;
 
 	cap_save = readl(mmio + HOST_CAP);
 	cap_save &= ((1 << 28) | (1 << 17));
@@ -110,6 +112,7 @@  static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 	writel(cap_save, mmio + HOST_CAP);
 	writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);
 
+#ifdef CONFIG_PCI
 	pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor);
 
 	if (vendor == PCI_VENDOR_ID_INTEL) {
@@ -118,7 +121,7 @@  static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 		tmp16 |= 0xf;
 		pci_write_config_word(pdev, 0x92, tmp16);
 	}
-
+#endif
 	probe_ent->cap = readl(mmio + HOST_CAP);
 	probe_ent->port_map = readl(mmio + HOST_PORTS_IMPL);
 	probe_ent->n_ports = (probe_ent->cap & 0x1f) + 1;
@@ -183,22 +186,24 @@  static int ahci_host_init(struct ahci_probe_ent *probe_ent)
 	writel(tmp | HOST_IRQ_EN, mmio + HOST_CTL);
 	tmp = readl(mmio + HOST_CTL);
 	debug("HOST_CTL 0x%x\n", tmp);
-
+#ifdef CONFIG_PCI
 	pci_read_config_word(pdev, PCI_COMMAND, &tmp16);
 	tmp |= PCI_COMMAND_MASTER;
 	pci_write_config_word(pdev, PCI_COMMAND, tmp16);
-
+#endif
 	return 0;
 }
 
 
 static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 {
+#ifdef CONFIG_PCI
 	pci_dev_t pdev = probe_ent->dev;
+	u16 cc;
+#endif
 	volatile u8 *mmio = (volatile u8 *)probe_ent->mmio_base;
 	u32 vers, cap, impl, speed;
 	const char *speed_s;
-	u16 cc;
 	const char *scc_s;
 
 	vers = readl(mmio + HOST_VERSION);
@@ -212,7 +217,7 @@  static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 		speed_s = "3";
 	else
 		speed_s = "?";
-
+#ifdef CONFIG_PCI
 	pci_read_config_word(pdev, 0x0a, &cc);
 	if (cc == 0x0101)
 		scc_s = "IDE";
@@ -222,7 +227,9 @@  static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 		scc_s = "RAID";
 	else
 		scc_s = "unknown";
-
+#else
+	scc_s = "SATA";
+#endif
 	printf("AHCI %02x%02x.%02x%02x "
 	       "%u slots %u ports %s Gbps 0x%x impl %s mode\n",
 	       (vers >> 24) & 0xff,
@@ -249,6 +256,7 @@  static void ahci_print_info(struct ahci_probe_ent *probe_ent)
 	       cap & (1 << 13) ? "part " : "");
 }
 
+#ifdef CONFIG_PCI
 static int ahci_init_one(pci_dev_t pdev)
 {
 	u16 vendor;
@@ -291,7 +299,7 @@  static int ahci_init_one(pci_dev_t pdev)
       err_out:
 	return rc;
 }
-
+#endif
 
 #define MAX_DATA_BYTE_COUNT  (4*1024*1024)
 
@@ -667,7 +675,9 @@  void scsi_low_level_init(int busdevfunc)
 	int i;
 	u32 linkmap;
 
+#ifdef CONFIG_PCI
 	ahci_init_one(busdevfunc);
+#endif
 
 	linkmap = probe_ent->link_port_map;
 
@@ -682,6 +692,48 @@  void scsi_low_level_init(int busdevfunc)
 	}
 }
 
+int ahci_init(u32 base)
+{
+	int i, rc = 0;
+	u32 linkmap;
+
+	memset(ataid, 0, sizeof(ataid));
+
+	probe_ent = malloc(sizeof(struct ahci_probe_ent));
+	memset(probe_ent, 0, sizeof(struct ahci_probe_ent));
+
+	probe_ent->host_flags = ATA_FLAG_SATA
+				| ATA_FLAG_NO_LEGACY
+				| ATA_FLAG_MMIO
+				| ATA_FLAG_PIO_DMA
+				| ATA_FLAG_NO_ATAPI;
+	probe_ent->pio_mask = 0x1f;
+	probe_ent->udma_mask = 0x7f;	/*Fixme,assume to support UDMA6 */
+
+	probe_ent->mmio_base = base;
+
+	/* initialize adapter */
+	rc = ahci_host_init(probe_ent);
+	if (rc)
+		goto err_out;
+
+	ahci_print_info(probe_ent);
+
+	linkmap = probe_ent->link_port_map;
+
+	for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) {
+		if (((linkmap >> i) & 0x01)) {
+			if (ahci_port_start((u8) i)) {
+				printf("Can not start port %d\n", i);
+				continue;
+			}
+			ahci_set_feature((u8) i);
+		}
+	}
+err_out:
+	return rc;
+}
+
 
 void scsi_bus_reset(void)
 {
diff --git a/include/ahci.h b/include/ahci.h
index 0c6bbbd..c0a0a47 100644
--- a/include/ahci.h
+++ b/include/ahci.h
@@ -25,6 +25,8 @@ 
 #ifndef _AHCI_H_
 #define _AHCI_H_
 
+#include <pci.h>
+
 #define AHCI_PCI_BAR		0x24
 #define AHCI_MAX_SG		56 /* hardware max is 64K */
 #define AHCI_CMD_SLOT_SZ	32
@@ -187,4 +189,6 @@  struct ahci_probe_ent {
 	u32	link_port_map; /*linkup port map*/
 };
 
+int ahci_init(u32 base);
+ 
 #endif
diff --git a/include/scsi.h b/include/scsi.h
index aaafc9c..c52759c 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -185,6 +185,7 @@  void scsi_low_level_init(int busdevfunc);
  * functions residing inside cmd_scsi.c
  */
 void scsi_init(void);
+void scsi_scan(int mode);
 
 
 #define SCSI_IDENTIFY					0xC0  /* not used */