Re: [PATCH] ata: add CONFIG_SATA_HOST config option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Bartlomiej Zolnierkiewicz wrote:

Add CONFIG_SATA_HOST config option (for selecting SATA Host
support) to make setup easier on PATA-only systems.

Additionally this allows us to save ~9.5k of the output code
size (x86-64) on such systems:

   text    data     bss     dec     hex filename
  49909    6576      57   56542    dcde drivers/ata/libata-core.o.before
  31862      16       2   31880    7c88 drivers/ata/libata-eh.o.before
  20256       0      19   20275    4f33 drivers/ata/libata-sff.o.before
  44495    6576      57   51128    c7b8 drivers/ata/libata-core.o.after
  27831      16       2   27849    6cc9 drivers/ata/libata-eh.o.after
  20144       0      19   20163    4ec3 drivers/ata/libata-sff.o.after

   Good job!

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
I also wonder if  SATA code should be moved to libata-sata.c (to avoid ifdefs)?

 drivers/ata/Kconfig       |   25 ++++++++--
 drivers/ata/libata-core.c |  107 +++++++++++++++++++++++++++++++++++++++-------
 drivers/ata/libata-eh.c   |   21 ++++++++-
 drivers/ata/libata-sff.c  |    8 +++
 4 files changed, 141 insertions(+), 20 deletions(-)

Index: b/drivers/ata/Kconfig
===================================================================
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
[...]
@@ -122,7 +133,7 @@ comment "SFF controllers with custom DMA
config PDC_ADMA
 	tristate "Pacific Digital ADMA support"
-	depends on PCI
+	depends on PCI && SATA_HOST

I thought ADMA spec. was designed for PATA... are you sure this should depend on CONFIG_SATA_HOST?

@@ -165,8 +176,6 @@ config ATA_BMDMA
if ATA_BMDMA -comment "SATA SFF controllers with BMDMA"
-
 config ATA_PIIX
 	tristate "Intel ESB, ICH, PIIX3, PIIX4 PATA/SATA support"
 	depends on PCI
@@ -177,6 +186,10 @@ config ATA_PIIX
If unsure, say N. +if SATA_HOST
+
+comment "SATA/PATA SFF controllers with BMDMA"

   How it can be "SATA/PATA" if it's only enabled when CONFIG_SATA_HOST=y?

Index: b/drivers/ata/libata-core.c
===================================================================
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
[...]
@@ -601,6 +609,9 @@ void ata_tf_from_fis(const u8 *fis, stru
 	tf->nsect	= fis[12];
 	tf->hob_nsect	= fis[13];
 }
+#else
+void ata_tf_from_fis(const u8 *fis, struct ata_taskfile *tf) { }
+#endif

Is it really ever called by non-SATA code? Ah, it's exported at end of file and so should be defined... isn't it better to carry over the export from the bottom of the file (and thus silence one of checkpatch.pl's warnings :-).

@@ -2690,6 +2718,7 @@ struct ata_device *ata_dev_pair(struct a
 	return pair;
 }
+#ifdef CONFIG_SATA_HOST
 /**
  *	sata_down_spd_limit - adjust SATA spd limit downward
  *	@link: Link to adjust SATA spd limit for
@@ -2843,6 +2872,18 @@ int sata_set_spd(struct ata_link *link)
return 1;
 }
+#else
[...]
+int sata_set_spd(struct ata_link *link)
+{
+	return 0;
+}

   Again, I don't think that's called by non-SATA code...

+#endif
+EXPORT_SYMBOL_GPL(sata_set_spd);

   Here you're carrying over the export...

@@ -3436,6 +3477,7 @@ int ata_wait_after_reset(struct ata_link
 	return ata_wait_ready(link, deadline, check_ready);
 }
+#ifdef CONFIG_SATA_HOST
 /**
  *	sata_link_debounce - debounce SATA phy status
  *	@link: ATA link to debounce SATA phy status for
@@ -3638,6 +3680,26 @@ int sata_link_scr_lpm(struct ata_link *l
 	ehc->i.serror &= ~SERR_PHYRDY_CHG;
 	return sata_scr_write(link, SCR_ERROR, SERR_PHYRDY_CHG);
 }
+#else
+int sata_link_debounce(struct ata_link *link, const unsigned long *params,
+		       unsigned long deadline)
+{
+	return -EOPNOTSUPP;
+}

   Again, not called by non-SATA code...

+int sata_link_resume(struct ata_link *link, const unsigned long *params,
+		     unsigned long deadline)
+{
+	return -EOPNOTSUPP;
+}

   This one is not called by non-SATA code now either...

WBR, Sergei

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


[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux