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

From: Bartlomiej Zolnierkiewicz
Date: Thu Oct 25 2007 - 16:07:14 EST



[ 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@xxxxxxxxxx>
> ---
> 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 *);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/