From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mark Marshall <mark.marshall@omicron.at>
Date: Tue, 23 Oct 2012 15:51:45 +0200
Subject: [U-Boot] [PATCH 02/20] scsi: Provide support for a list of AHCI
 controllers.
In-Reply-To: <508699D3.4090908@omicron.at>
References: <508699D3.4090908@omicron.at>
Message-ID: <5086A0F1.1080307@omicron.at>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 19/10/12 05:44, Simon Glass wrote:
> From: Vadim Bendebury <vbendeb@chromium.org>
> 
> 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.

I've written almost exactly the same code a few weeks ago, and was
about to try to push it.  I won't, but I'll make some comments below.

> 
> Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  common/cmd_scsi.c |   43 ++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c
> index 22d0119..3890afd 100644
> --- a/common/cmd_scsi.c
> +++ b/common/cmd_scsi.c
> @@ -34,6 +34,14 @@
>  #include <image.h>
>  #include <pci.h>
>  
> +struct scsi_device {
> +	u16	scsi_vendor_id;
> +	u16	scsi_dev_id;
> +};

This isn't needed, you should use "struct pci_device_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;
> +	}

Again, you don't need a for loop, use "pci_find_devices"
(with an S at the end)

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