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

[Rob Herring]

Wolfgang,

On 07/04/2011 05:17 AM, Wolfgang Denk wrote:
> Dear Rob Herring,
> 
> In message <1309275583-11763-6-git-send-email-robherring2 at gmail.com> you wrote:
>> From: Rob Herring <rob.herring at calxeda.com>
>>
>> Add support for AHCI controllers that are not PCI based.
>>
>> Signed-off-by: Rob Herring <rob.herring at calxeda.com>
>> Cc: Wolfgang Denk <wd at 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