libata: reduce ata_pci_[sff,bmdma]_init_one() size
PATCH 1/1 libata: reduce ata_pci_[sff,bmdma]_init_one() size
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
--- drivers/ata/libata-sff.c | 112 +++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 66 deletions(-)
drivers/ata/libata-sff.c +46 -66
@@ -2491,31 +2491,10 @@ static const struct ata_port_info *ata_sreturn 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_devif (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);
Hello.
Quoted 18 lines · Bartlomiej Zolnierkiewicz
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.afterSigned-off-by Bartlomiej Zolnierkiewicz <[email protected]>--- drivers/ata/libata-sff.c | 112 +++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 66 deletions(-)Index:
b/drivers/ata/libata-sff.c
===================================================================drivers/ata/libata-sff.c +0
-0
[...]
Quoted 14 lines
drivers/ata/libata-sff.c +6
-2@@ -2538,14 +2517,22 @@ int ata_pci_sff_init_one(struct pci_devif (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;
drivers/ata/libata-sff.c int ata_pci_sff_init_one(struct pci_dev · line 2535
Quoted 10 lines
drivers/ata/libata-sff.c +6
-2host->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);
Quoted 20 lines
drivers/ata/libata-sff.c +16
-0@@ -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.
drivers/ata/libata-sff.c out: · line 2569
Quoted 10 lines
drivers/ata/libata-sff.c +10
-0+* +* 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?
drivers/ata/libata-sff.c out: · line 2571
Quoted 2 lines
drivers/ata/libata-sff.c +2
-0+{ +return ata_pci_init_one (pdev, ppi, sht, host_priv, hflag,0 );
s/0/false/?
Quoted 8 lines
drivers/ata/libata-sff.c +1
-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) {
[...]
drivers/ata/libata-sff.c int ata_pci_bmdma_init_one(struct pci_de · line 3291
Quoted 1 line
drivers/ata/libata-sff.c +1
-0+return ata_pci_init_one (pdev, ppi, sht, host_priv, hflags,1 );
s/1/true/?
WBR, Sergei
Quoted 6 lines
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