| Submitter | Bartlomiej Zolnierkiewicz |
|---|---|
| Date | Feb. 9, 2011, 2:15 p.m. |
| Message ID | <201102091515.22677.bzolnier@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/82472/ |
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Comments
Hello. Bartlomiej Zolnierkiewicz wrote: > Turn both helpers (which are used only during LLDs initialization > time and thus are not performance sensitive) into wrappers around > the new ata_pci_init_one() function, this cuts 20 LOC and saves > ~1.1k of the output code size (x86-64): > text data bss dec hex filename > 21392 0 19 21411 53a3 drivers/ata/libata-sff.o.before > 20256 0 19 20275 4f33 drivers/ata/libata-sff.o.after > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> > --- > drivers/ata/libata-sff.c | 112 +++++++++++++++++++---------------------------- > 1 file changed, 46 insertions(+), 66 deletions(-) > Index: b/drivers/ata/libata-sff.c > =================================================================== > --- a/drivers/ata/libata-sff.c > +++ b/drivers/ata/libata-sff.c [...] > @@ -2538,14 +2517,22 @@ int ata_pci_sff_init_one(struct pci_dev > if (rc) > goto out; > > - /* prepare and activate SFF host */ > - rc = ata_pci_sff_prepare_host(pdev, ppi, &host); > + if (bmdma) > + /* prepare and activate BMDMA host */ > + rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host); > + else > + /* prepare and activate SFF host */ > + rc = ata_pci_sff_prepare_host(pdev, ppi, &host); > if (rc) > goto out; > host->private_data = host_priv; > - host->flags |= hflag; > + host->flags |= hflags; > > - rc = ata_pci_sff_activate_host(host, ata_sff_interrupt, sht); > + if (bmdma) { > + pci_set_master(pdev); > + rc = ata_pci_sff_activate_host(host, ata_bmdma_interrupt, sht); > + } else > + rc = ata_pci_sff_activate_host(host, ata_sff_interrupt, sht); Perhaps this will save even more: if (bmdma) pci_set_master(pdev); rc = ata_pci_sff_activate_host(host, bmdma ? ata_bmdma_interrupt : ata_sff_interrupt, sht); > @@ -2554,6 +2541,35 @@ out: > > return rc; > } > + > +/** > + * ata_pci_sff_init_one - Initialize/register PIO-only PCI IDE controller > + * @pdev: Controller to be initialized > + * @ppi: array of port_info, must be enough for two ports > + * @sht: scsi_host_template to use when registering the host > + * @host_priv: host private_data > + * @hflag: host flags > + * > + * This is a helper function which can be called from a driver's > + * xxx_init_one() probe function if the hardware uses traditional > + * IDE taskfile registers and is PIO only. > + * > + * ASSUMPTION: > + * Nobody makes a single channel controller that appears solely as > + * the secondary legacy port on PCI. > + * > + * LOCKING: > + * Inherited from PCI layer (may sleep). > + * > + * RETURNS: > + * Zero on success, negative on errno-based value on error. > + */ > +int ata_pci_sff_init_one(struct pci_dev *pdev, > + const struct ata_port_info * const *ppi, > + struct scsi_host_template *sht, void *host_priv, int hflag) s/hflag/hflags/ for consistency? > +{ > + return ata_pci_init_one(pdev, ppi, sht, host_priv, hflag, 0); s/0/false/? > +} > EXPORT_SYMBOL_GPL(ata_pci_sff_init_one); > > #endif /* CONFIG_PCI */ > @@ -3272,43 +3288,7 @@ int ata_pci_bmdma_init_one(struct pci_de > struct scsi_host_template *sht, void *host_priv, > int hflags) > { [...] > + return ata_pci_init_one(pdev, ppi, sht, host_priv, hflags, 1); s/1/true/? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Perhaps this will save even more: > > if (bmdma) > pci_set_master(pdev); > rc = ata_pci_sff_activate_host(host, bmdma ? ata_bmdma_interrupt : > ata_sff_interrupt, sht); gcc is usually smart enough to work that out - and it's definitely easier to read as Bartlomiej has it. I agree with the comment that libata-sata rather than ifdefs would be good for the SATA bit as well btw. And yes ADMA was designed for PATA -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
Index: b/drivers/ata/libata-sff.c =================================================================== --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -2491,31 +2491,10 @@ static const struct ata_port_info *ata_s return NULL; } -/** - * ata_pci_sff_init_one - Initialize/register PIO-only PCI IDE controller - * @pdev: Controller to be initialized - * @ppi: array of port_info, must be enough for two ports - * @sht: scsi_host_template to use when registering the host - * @host_priv: host private_data - * @hflag: host flags - * - * This is a helper function which can be called from a driver's - * xxx_init_one() probe function if the hardware uses traditional - * IDE taskfile registers and is PIO only. - * - * ASSUMPTION: - * Nobody makes a single channel controller that appears solely as - * the secondary legacy port on PCI. - * - * LOCKING: - * Inherited from PCI layer (may sleep). - * - * RETURNS: - * Zero on success, negative on errno-based value on error. - */ -int ata_pci_sff_init_one(struct pci_dev *pdev, - const struct ata_port_info * const *ppi, - struct scsi_host_template *sht, void *host_priv, int hflag) +static int ata_pci_init_one(struct pci_dev *pdev, + const struct ata_port_info * const *ppi, + struct scsi_host_template *sht, void *host_priv, + int hflags, bool bmdma) { struct device *dev = &pdev->dev; const struct ata_port_info *pi; @@ -2538,14 +2517,22 @@ int ata_pci_sff_init_one(struct pci_dev if (rc) goto out; - /* prepare and activate SFF host */ - rc = ata_pci_sff_prepare_host(pdev, ppi, &host); + if (bmdma) + /* prepare and activate BMDMA host */ + rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host); + else + /* prepare and activate SFF host */ + rc = ata_pci_sff_prepare_host(pdev, ppi, &host); if (rc) goto out; host->private_data = host_priv; - host->flags |= hflag; + host->flags |= hflags; - rc = ata_pci_sff_activate_host(host, ata_sff_interrupt, sht); + if (bmdma) { + pci_set_master(pdev); + rc = ata_pci_sff_activate_host(host, ata_bmdma_interrupt, sht); + } else + rc = ata_pci_sff_activate_host(host, ata_sff_interrupt, sht); out: if (rc == 0) devres_remove_group(&pdev->dev, NULL); @@ -2554,6 +2541,35 @@ out: return rc; } + +/** + * ata_pci_sff_init_one - Initialize/register PIO-only PCI IDE controller + * @pdev: Controller to be initialized + * @ppi: array of port_info, must be enough for two ports + * @sht: scsi_host_template to use when registering the host + * @host_priv: host private_data + * @hflag: host flags + * + * This is a helper function which can be called from a driver's + * xxx_init_one() probe function if the hardware uses traditional + * IDE taskfile registers and is PIO only. + * + * ASSUMPTION: + * Nobody makes a single channel controller that appears solely as + * the secondary legacy port on PCI. + * + * LOCKING: + * Inherited from PCI layer (may sleep). + * + * RETURNS: + * Zero on success, negative on errno-based value on error. + */ +int ata_pci_sff_init_one(struct pci_dev *pdev, + const struct ata_port_info * const *ppi, + struct scsi_host_template *sht, void *host_priv, int hflag) +{ + return ata_pci_init_one(pdev, ppi, sht, host_priv, hflag, 0); +} EXPORT_SYMBOL_GPL(ata_pci_sff_init_one); #endif /* CONFIG_PCI */ @@ -3272,43 +3288,7 @@ int ata_pci_bmdma_init_one(struct pci_de struct scsi_host_template *sht, void *host_priv, int hflags) { - struct device *dev = &pdev->dev; - const struct ata_port_info *pi; - struct ata_host *host = NULL; - int rc; - - DPRINTK("ENTER\n"); - - pi = ata_sff_find_valid_pi(ppi); - if (!pi) { - dev_printk(KERN_ERR, &pdev->dev, - "no valid port_info specified\n"); - return -EINVAL; - } - - if (!devres_open_group(dev, NULL, GFP_KERNEL)) - return -ENOMEM; - - rc = pcim_enable_device(pdev); - if (rc) - goto out; - - /* prepare and activate BMDMA host */ - rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host); - if (rc) - goto out; - host->private_data = host_priv; - host->flags |= hflags; - - pci_set_master(pdev); - rc = ata_pci_sff_activate_host(host, ata_bmdma_interrupt, sht); - out: - if (rc == 0) - devres_remove_group(&pdev->dev, NULL); - else - devres_release_group(&pdev->dev, NULL); - - return rc; + return ata_pci_init_one(pdev, ppi, sht, host_priv, hflags, 1); } EXPORT_SYMBOL_GPL(ata_pci_bmdma_init_one);
Turn both helpers (which are used only during LLDs initialization time and thus are not performance sensitive) into wrappers around the new ata_pci_init_one() function, this cuts 20 LOC and saves ~1.1k of the output code size (x86-64): text data bss dec hex filename 21392 0 19 21411 53a3 drivers/ata/libata-sff.o.before 20256 0 19 20275 4f33 drivers/ata/libata-sff.o.after Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> --- drivers/ata/libata-sff.c | 112 +++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 66 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html