Re: [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device()

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

[ The last few days were extra busy with pushing previously queued
  IDE bits upstream but I'm back to reviewing the new stuff. ]

Thanks for splitting your patch on smaller chunks.

On Thursday 25 October 2007, Jeff Garzik wrote:
> Like ide_setup_pci_device(), except with additional array argument
> 'u8 *idx' that permits low-level driver to become aware of its assigned
> hwifs.

I would prefer to avoid exporting this information to host drivers
if possible (they shouldn't need to know about higher-layer decisions,
plus this change opens the door for various "creative" abuses).

In case of sc1200 host driver fixes (patches #2-3/3) it should be
possible to remove the need for the below patch and at the same time
simplify sc1200 code further (more details in review of patch #2/2).

> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/ide/setup-pci.c |   17 ++++++++++++-----
>  include/linux/ide.h     |    1 +
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> index 02d14bf..4960b9f 100644
> --- a/drivers/ide/setup-pci.c
> +++ b/drivers/ide/setup-pci.c
> @@ -666,12 +666,10 @@ out:
>  	return ret;
>  }
>  
> -int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> +int __ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d,
> +			   u8 *idx)
>  {
> -	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> -	int ret;
> -
> -	ret = do_ide_setup_pci_device(dev, d, &idx[0], 1);
> +	int ret = do_ide_setup_pci_device(dev, d, idx, 1);
>  
>  	if (ret >= 0)
>  		ide_device_add(idx);
> @@ -679,6 +677,15 @@ int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
>  	return ret;
>  }
>  
> +EXPORT_SYMBOL_GPL(__ide_setup_pci_device);
> +
> +int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> +{
> +	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> +
> +	return __ide_setup_pci_device(dev, d, &idx[0]);
> +}
> +
>  EXPORT_SYMBOL_GPL(ide_setup_pci_device);
>  
>  int ide_setup_pci_devices(struct pci_dev *dev1, struct pci_dev *dev2,
> diff --git a/include/linux/ide.h b/include/linux/ide.h
> index 4ed4777..3404fb9 100644
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1244,6 +1244,7 @@ struct ide_port_info {
>  	u8			udma_mask;
>  };
>  
> +int __ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *, u8 *);
>  int ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *);
>  int ide_setup_pci_devices(struct pci_dev *, struct pci_dev *, const struct ide_port_info *);