ata: add CONFIG_SATA_HOST config option

PATCH Superseded Bartlomiej Zolnierkiewicz 2 participants 4 messages Feb 11, 2011 – Feb 14, 2011 libata

lore

Share & tools

lore

Bartlomiej Zolnierkiewicz

author

2011-02-11 13:35 UTC #0 lore raw

PATCH v2 1/1 ata: add CONFIG_SATA_HOST config option

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

Additionally move SATA-specific code to libata-sata.c which allows us to save ~11.5k of the output code size (x86-64) on PATA-only systems for CONFIG_SATA_HOST=n:

CONFIG_SATA_HOST=y:
text data bss dec hex filename

  44283    6576      57   50916    c6e4 drivers/ata/libata-core.o
  29054      16       2   29072    7190 drivers/ata/libata-eh.o
  20085       0      19   20104    4e88 drivers/ata/libata-sff.o
   8699       0       0    8699    21fb drivers/ata/libata-sata.o

CONFIG_SATA_HOST=n:
text data bss dec hex filename

  43754    6576      57   50387    c4d3 drivers/ata/libata-core.o
  26775      16       2   26793    68a9 drivers/ata/libata-eh.o
  20144       0      19   20163    4ec3 drivers/ata/libata-sff.o

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
v2:
- Kconfig fixups per Sergei's comments
- add non-SATA version of ata_std_postreset()
- drop non-SATA versions of sata_link_[debounce,hardreset,resume](), sata_set_spd(), sata_print_link_status() and ata_tf_from_fis()
- move SATA-specific code to libata-sata.c

 drivers/ata/Kconfig       |   23 
 drivers/ata/Makefile      |    1 
 drivers/ata/libata-core.c |  814 ------------------------------
 drivers/ata/libata-eh.c   |  411 ---------------
 drivers/ata/libata-sata.c | 1242 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/ata/libata-sff.c  |   32 -
 drivers/ata/libata.h      |   52 +
 include/linux/libata.h    |   32 -
 8 files changed, 1366 insertions(+), 1241 deletions(-)

drivers/ata/Kconfig +19 -4

Index: b/drivers/ata/Kconfig
===================================================================
@@ -50,13 +50,22 @@ config ATA_ACPI
 	  You can disable this at kernel boot time by using the
 	  option libata.noacpi=1
 
+config SATA_HOST
+	bool "SATA Host support"
+	default y
+	help
+	  This option adds support for SATA Hosts.
+
 config SATA_PMP
 	bool "SATA Port Multiplier support"
+	depends on SATA_HOST
 	default y
 	help
 	  This option adds support for SATA Port Multipliers
 	  (the SATA version of an ethernet hub, or SAS expander).
 
+if SATA_HOST
+
 comment "Controllers with non-SFF native interface"
 
 config SATA_AHCI

@@ -98,6 +107,8 @@ config SATA_SIL24
 
 	  If unsure, say N.
 
+endif # SATA_HOST
+
 config ATA_SFF
 	bool "ATA SFF support"
 	default y

@@ -139,7 +150,7 @@ config PATA_OCTEON_CF
 
 config SATA_QSTOR
 	tristate "Pacific Digital SATA QStor support"
-	depends on PCI
+	depends on PCI && SATA_HOST
 	help
 	  This option enables support for Pacific Digital Serial ATA QStor.

@@ -147,7 +158,7 @@ config SATA_QSTOR
 
 config SATA_SX4
 	tristate "Promise SATA SX4 support (Experimental)"
-	depends on PCI && EXPERIMENTAL
+	depends on PCI && SATA_HOST && EXPERIMENTAL
 	help
 	  This option enables support for Promise Serial ATA SX4.

@@ -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 SFF controllers with BMDMA"
+
 config SATA_DWC
 	tristate "DesignWare Cores SATA support"
 	depends on 460EX

@@ -263,6 +276,8 @@ config SATA_VITESSE
 
 	  If unsure, say N.
 
+endif # SATA_HOST
+
 comment "PATA SFF controllers with BMDMA"
 
 config PATA_ALI

drivers/ata/Makefile +1 -0

Index: b/drivers/ata/Makefile
===================================================================
@@ -95,6 +95,7 @@ obj-$(CONFIG_ATA_GENERIC)	+= ata_generic
 obj-$(CONFIG_PATA_LEGACY)	+= pata_legacy.o
 
 libata-y	:= libata-core.o libata-scsi.o libata-eh.o libata-transport.o
+libata-$(CONFIG_SATA_HOST)	+= libata-sata.o
 libata-$(CONFIG_ATA_SFF)	+= libata-sff.o
 libata-$(CONFIG_SATA_PMP)	+= libata-pmp.o
 libata-$(CONFIG_ATA_ACPI)	+= libata-acpi.o

drivers/ata/libata-core.c +26 ~~-788~~

938 lines not on this page · raw message

drivers/ata/libata-eh.c +5 ~~-406~~

441 lines not on this page · raw message

drivers/ata/libata-sata.c new file +1242 -0

1,242 lines not on this page · raw message

drivers/ata/libata-sff.c +4 ~~-28~~

Index: b/drivers/ata/libata-sff.c
===================================================================
@@ -252,7 +252,7 @@ int ata_sff_busy_sleep(struct ata_port *
 }
 EXPORT_SYMBOL_GPL(ata_sff_busy_sleep);
 
-static int ata_sff_check_ready(struct ata_link *link)
+int ata_sff_check_ready(struct ata_link *link)
 {
 	u8 status = link->ap->ops->sff_check_status(link->ap);

@@ -2061,38 +2061,14 @@ int ata_sff_softreset(struct ata_link *l
 }
 EXPORT_SYMBOL_GPL(ata_sff_softreset);
 
-/**
- *	sata_sff_hardreset - reset host port via SATA phy reset
- *	@link: link to reset
- *	@class: resulting class of attached device
- *	@deadline: deadline jiffies for the operation
- *
- *	SATA phy-reset host port using DET bits of SControl register,
- *	wait for !BSY and classify the attached device.
- *
- *	LOCKING:
- *	Kernel thread context (may sleep)
- *
- *	RETURNS:
- *	0 on success, -errno otherwise.
- */
+#ifndef CONFIG_SATA_HOST
 int sata_sff_hardreset(struct ata_link *link, unsigned int *class,
 		       unsigned long deadline)
 {
-	struct ata_eh_context *ehc = &link->eh_context;
-	const unsigned long *timing = sata_ehc_deb_timing(ehc);
-	bool online;
-	int rc;
-
-	rc = sata_link_hardreset(link, timing, deadline, &online,
-				 ata_sff_check_ready);
-	if (online)
-		*class = ata_sff_dev_classify(link->device, 1, NULL);
-
-	DPRINTK("EXIT, class=%u\n", *class);
-	return rc;
+	return -EOPNOTSUPP;
 }
 EXPORT_SYMBOL_GPL(sata_sff_hardreset);
+#endif
 
 /**
  *	ata_sff_postreset - SFF postreset callback

drivers/ata/libata.h +50 -2

81 lines not on this page · raw message

include/linux/libata.h +19 ~~-13~~

Index: b/include/linux/libata.h
===================================================================
@@ -931,15 +931,13 @@ struct ata_timing {
 };
 
 /*
- * Core layer - drivers/ata/libata-core.c
+ * SATA host - drivers/ata/libata-sata.c
  */
+#ifdef CONFIG_SATA_HOST
 extern const unsigned long sata_deb_timing_normal[];
 extern const unsigned long sata_deb_timing_hotplug[];
 extern const unsigned long sata_deb_timing_long[];
 
-extern struct ata_port_operations ata_dummy_port_ops;
-extern const struct ata_port_info ata_dummy_port_info;
-
 static inline const unsigned long *
 sata_ehc_deb_timing(struct ata_eh_context *ehc)
 {

@@ -949,24 +947,33 @@ sata_ehc_deb_timing(struct ata_eh_contex
 		return sata_deb_timing_normal;
 }
 
+void ata_tf_from_fis(const u8 *fis, struct ata_taskfile *tf);
+int sata_set_spd(struct ata_link *link);
+int sata_link_debounce(struct ata_link *link, const unsigned long *params,
+		       unsigned long deadline);
+int sata_link_resume(struct ata_link *link, const unsigned long *params,
+		     unsigned long deadline);
+int sata_link_hardreset(struct ata_link *link, const unsigned long *timing,
+			unsigned long deadline, bool *online,
+			int (*check_ready)(struct ata_link *));
+#endif
+
+/*
+ * Core layer - drivers/ata/libata-core.c
+ */
+extern struct ata_port_operations ata_dummy_port_ops;
+extern const struct ata_port_info ata_dummy_port_info;
+
 static inline int ata_port_is_dummy(struct ata_port *ap)
 {
 	return ap->ops == &ata_dummy_port_ops;
 }
 
-extern int sata_set_spd(struct ata_link *link);
 extern int ata_std_prereset(struct ata_link *link, unsigned long deadline);
 extern int ata_wait_after_reset(struct ata_link *link, unsigned long deadline,
 				int (*check_ready)(struct ata_link *link));
-extern int sata_link_debounce(struct ata_link *link,
-			const unsigned long *params, unsigned long deadline);
-extern int sata_link_resume(struct ata_link *link, const unsigned long *params,
-			    unsigned long deadline);
 extern int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 			     bool spm_wakeup);
-extern int sata_link_hardreset(struct ata_link *link,
-			const unsigned long *timing, unsigned long deadline,
-			bool *online, int (*check_ready)(struct ata_link *));
 extern int sata_std_hardreset(struct ata_link *link, unsigned int *class,
 			      unsigned long deadline);
 extern void ata_std_postreset(struct ata_link *link, unsigned int *classes);

@@ -1015,7 +1022,6 @@ extern u32 ata_wait_register(struct ata_
 extern int atapi_cmd_type(u8 opcode);
 extern void ata_tf_to_fis(const struct ata_taskfile *tf,
 			  u8 pmp, int is_cmd, u8 *fis);
-extern void ata_tf_from_fis(const u8 *fis, struct ata_taskfile *tf);
 extern unsigned long ata_pack_xfermask(unsigned long pio_mask,
 			unsigned long mwdma_mask, unsigned long udma_mask);
 extern void ata_unpack_xfermask(unsigned long xfer_mask,

Sergei Shtylyov

2011-02-13 14:40 UTC #1 lore raw

Hello.

Quoted 21 lines · Bartlomiej Zolnierkiewicz

Add CONFIG_SATA_HOST config option (for selecting SATA Host support) to make setup easier on PATA-only systems.
Additionally move SATA-specific code to libata-sata.c which allows us to save ~11.5k of the output code size (x86-64) on PATA-only systems for CONFIG_SATA_HOST=n:
CONFIG_SATA_HOST=y:
text data bss dec hex filename
   44283    6576      57   50916    c6e4 drivers/ata/libata-core.o
   29054      16       2   29072    7190 drivers/ata/libata-eh.o
   20085       0      19   20104    4e88 drivers/ata/libata-sff.o
    8699       0       0    8699    21fb drivers/ata/libata-sata.o
CONFIG_SATA_HOST=n:
text data bss dec hex filename
   43754    6576      57   50387    c4d3 drivers/ata/libata-core.o
   26775      16       2   26793    68a9 drivers/ata/libata-eh.o
   20144       0      19   20163    4ec3 drivers/ata/libata-sff.o
Signed-off-by Bartlomiej Zolnierkiewicz<[email protected]>

[...]

Quoted 4 lines

Index: b/drivers/ata/libata-core.c
===================================================================
drivers/ata/libata-core.c +0 -0

[...]

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 495

Quoted 3 lines

drivers/ata/libata-core.c +0 -0

@@ -573,34 +494,26 @@ void ata_tf_to_fis(const struct ata_task
 	fis[19] = 0;
 }

Hm, what about ata_tf_to_fis()? It shouldn't be needed by non-SATA stuff,
moreover it'should be only needed by non-SFF controllers...

Quoted 11 lines

drivers/ata/libata-core.c +3 -7

+#ifndef CONFIG_SATA_HOST
+int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
+		      bool spm_wakeup)
  {
-	tf->command	= fis[2];	/* status */
-	tf->feature	= fis[3];	/* error */
-
-	tf->lbal	= fis[4];
-	tf->lbam	= fis[5];
-	tf->lbah	= fis[6];
-	tf->device	= fis[7];

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 503

Quoted 10 lines

drivers/ata/libata-core.c +3 -7

-
-	tf->hob_lbal	= fis[8];
-	tf->hob_lbam	= fis[9];
-	tf->hob_lbah	= fis[10];
-
-	tf->nsect	= fis[12];
-	tf->hob_nsect	= fis[13];
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(sata_link_scr_lpm);

Shouldn't there be empty line here?

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 509

Quoted 6 lines

drivers/ata/libata-core.c +6 -0

+int sata_std_hardreset(struct ata_link *link, unsigned int *class,
+		       unsigned long deadline)
+{
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(sata_std_hardreset);

... and here?

Quoted 5 lines

Index: b/drivers/ata/libata-sata.c
===================================================================
drivers/ata/libata-sata.c new file +0 -0
@@ -0,0 +1,1242 @@

[...]

drivers/ata/libata-sata.c line 379

Quoted 6 lines

drivers/ata/libata-sata.c new file +6 -0

+int sata_set_spd(struct ata_link *link)
+{
+	u32 scontrol;
+	int rc;
+
+	if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))

I guess checkpatch.pl protests here...

drivers/ata/libata-sata.c line 385

Quoted 6 lines

drivers/ata/libata-sata.c new file +6 -0

+		return rc;
+
+	if (!__sata_set_spd_needed(link,&scontrol))
+		return 0;
+
+	if ((rc = sata_scr_write(link, SCR_CONTROL, scontrol)))

Here too...

Quoted 5 lines

drivers/ata/libata-sata.c new file +5 -0

+		return rc;
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(sata_set_spd);

[...]

Quoted 4 lines

drivers/ata/libata-sata.c new file +4 -0

+int sata_link_debounce(struct ata_link *link, const unsigned long *params,
+		       unsigned long deadline)
+{
+	unsigned long interval = params[0];

drivers/ata/libata-sata.c line 427

Quoted 10 lines

drivers/ata/libata-sata.c new file +10 -0

+	unsigned long duration = params[1];
+	unsigned long last_jiffies, t;
+	u32 last, cur;
+	int rc;
+
+	t = ata_deadline(jiffies, params[2]);
+	if (time_before(t, deadline))
+		deadline = t;
+
+	if ((rc = sata_scr_read(link, SCR_STATUS, &cur)))

And here too...

Quoted 12 lines

drivers/ata/libata-sata.c new file +12 -0

+/**
+ *	sata_link_resume - resume SATA link
+ *	@link: ATA link to resume SATA
+ *	@params: timing parameters { interval, duratinon, timeout } in msec
+ *	@deadline: deadline jiffies for the operation
+ *
+ *	Resume SATA phy @link and debounce it.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:

drivers/ata/libata-sata.c line 484

Quoted 10 lines

drivers/ata/libata-sata.c new file +10 -0

+ *	0 on success, -errno on failure.
+ */
+int sata_link_resume(struct ata_link *link, const unsigned long *params,
+		     unsigned long deadline)
+{
+	int tries = ATA_LINK_RESUME_TRIES;
+	u32 scontrol, serror;
+	int rc;
+
+	if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))

And here...

Quoted 10 lines

drivers/ata/libata-sata.c new file +10 -0

+		return rc;
+
+	/*
+	 * Writes to SControl sometimes get ignored under certain
+	 * controllers (ata_piix SIDPR).  Make sure DET actually is
+	 * cleared.
+	 */
+	do {
+		scontrol = (scontrol & 0x0f0) | 0x300;
+		if ((rc = sata_scr_write(link, SCR_CONTROL, scontrol)))

drivers/ata/libata-sata.c line 504

Quoted 10 lines

drivers/ata/libata-sata.c new file +10 -0

+			return rc;
+		/*
+		 * Some PHYs react badly if SStatus is pounded
+		 * immediately after resuming.  Delay 200ms before
+		 * debouncing.
+		 */
+		ata_msleep(link->ap, 200);
+
+		/* is SControl restored correctly? */
+		if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))

And here...

Quoted 6 lines

drivers/ata/libata-sata.c new file +6 -0

+			return rc;
+	} while ((scontrol&  0xf0f) != 0x300&&  --tries);
+
+	if ((scontrol & 0xf0f) != 0x300) {
+		ata_link_printk(link, KERN_ERR,
+				"failed to resume link (SControl %X)\n",

drivers/ata/libata-sata.c line 520

Quoted 10 lines

drivers/ata/libata-sata.c new file +10 -0

+				scontrol);
+		return 0;
+	}
+
+	if (tries<  ATA_LINK_RESUME_TRIES)
+		ata_link_printk(link, KERN_WARNING,
+				"link resume succeeded after %d retries\n",
+				ATA_LINK_RESUME_TRIES - tries);
+
+	if ((rc = sata_link_debounce(link, params, deadline)))

And here...

drivers/ata/libata-sata.c line 524

Quoted 4 lines

drivers/ata/libata-sata.c new file +4 -0

+		return rc;
+
+	/* clear SError, some PHYs require this even for SRST to work */
+	if (!(rc = sata_scr_read(link, SCR_ERROR, &serror)))

Here too...

Quoted 5 lines

drivers/ata/libata-sata.c new file +5 -0

+		rc = sata_scr_write(link, SCR_ERROR, serror);
+
+	return rc != -EINVAL ? rc : 0;
+}
+EXPORT_SYMBOL_GPL(sata_link_resume);

[...]

Quoted 14 lines

drivers/ata/libata-sata.c new file +14 -0

+int sata_link_hardreset(struct ata_link *link, const unsigned long *timing,
+			unsigned long deadline,
+			bool *online, int (*check_ready)(struct ata_link *))
+{
+	u32 scontrol;
+	int rc;
+
+	DPRINTK("ENTER\n");
+
+	if (online)
+		*online = false;
+
+	if (sata_set_spd_needed(link)) {
+		/* SATA spec says nothing about how to reconfigure

drivers/ata/libata-sata.c line 645

Quoted 10 lines

drivers/ata/libata-sata.c new file +10 -0

+		 * spd.  To be on the safe side, turn off phy during
+		 * reconfiguration.  This works for at least ICH7 AHCI
+		 * and Sil3124.
+		 */
+		if ((rc = sata_scr_read(link, SCR_CONTROL,&scontrol)))
+			goto out;
+
+		scontrol = (scontrol & 0x0f0) | 0x304;
+
+		if ((rc = sata_scr_write(link, SCR_CONTROL, scontrol)))

And here...

drivers/ata/libata-sata.c

Quoted 7 lines

drivers/ata/libata-sata.c new file +7 -0

+			goto out;
+
+		sata_set_spd(link);
+	}
+
+	/* issue phy wake/reset */
+	if ((rc = sata_scr_read(link, SCR_CONTROL,&scontrol)))

Here too...

drivers/ata/libata-sata.c line 657

Quoted 5 lines

drivers/ata/libata-sata.c new file +5 -0

+		goto out;
+
+	scontrol = (scontrol&  0x0f0) | 0x301;
+
+	if ((rc = sata_scr_write_flush(link, SCR_CONTROL, scontrol)))

And here...
I understand that you're only moving the code. Perhaps it's worth fixing
checkpatch.pl's complaints beforehand -- I was going to look at it...

WBR, Sergei

Bartlomiej Zolnierkiewicz

author

2011-02-14 08:36 UTC #2 lore raw

Hi,

Quoted 35 lines · Sergei Shtylyov

Hello.
Quoted 21 lines · Bartlomiej Zolnierkiewicz
Add CONFIG_SATA_HOST config option (for selecting SATA Host support) to make setup easier on PATA-only systems.
Additionally move SATA-specific code to libata-sata.c which allows us to save ~11.5k of the output code size (x86-64) on PATA-only systems for CONFIG_SATA_HOST=n:
CONFIG_SATA_HOST=y:
text data bss dec hex filename
44283 6576 57 50916 c6e4 drivers/ata/libata-core.o 29054 16 2 29072 7190 drivers/ata/libata-eh.o 20085 0 19 20104 4e88 drivers/ata/libata-sff.o 8699 0 0 8699 21fb drivers/ata/libata-sata.o
CONFIG_SATA_HOST=n:
text data bss dec hex filename
43754 6576 57 50387 c4d3 drivers/ata/libata-core.o 26775 16 2 26793 68a9 drivers/ata/libata-eh.o 20144 0 19 20163 4ec3 drivers/ata/libata-sff.o
Signed-off-by Bartlomiej Zolnierkiewicz<[email protected]>
[...]
Quoted 4 lines
Index: b/drivers/ata/libata-core.c
===================================================================
drivers/ata/libata-core.c +0 -0
[...]
Quoted 1 line

Quoted 6 lines

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 495
Quoted 3 lines
drivers/ata/libata-core.c +0 -0
@@ -573,34 +494,26 @@ void ata_tf_to_fis(const struct ata_task
        fis[19] = 0;
  }
Hm, what about ata_tf_to_fis()? It shouldn't be needed by non-SATA stuff, moreover it'should be only needed by non-SFF controllers...

in theory yes but it is actually used by libata-scsi.c

Quoted 15 lines

drivers/ata/libata-core.c +3 ~~-11~~

+#ifndef CONFIG_SATA_HOST
+int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
+                     bool spm_wakeup)
  {
-       tf->command     = fis[2];       /* status */
-       tf->feature     = fis[3];       /* error */
-
-       tf->lbal        = fis[4];
-       tf->lbam        = fis[5];
-       tf->lbah        = fis[6];
-       tf->device      = fis[7];
-
-       tf->hob_lbal    = fis[8];
-       tf->hob_lbam    = fis[9];
-       tf->hob_lbah    = fis[10];

Quoted 8 lines

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 503
Quoted 6 lines
drivers/ata/libata-core.c +3 -3
-
-       tf->nsect       = fis[12];
-       tf->hob_nsect   = fis[13];
+       return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(sata_link_scr_lpm);
Shouldn't there be empty line here?

ata_piix needs it

Quoted 8 lines

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 509
Quoted 6 lines
drivers/ata/libata-core.c +6 -0
+int sata_std_hardreset(struct ata_link *link, unsigned int *class,
+                      unsigned long deadline)
+{
+       return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(sata_std_hardreset);
... and here?

ditto, and it is quite useful to have possibility of building ata_piix w/ CONFIG_SATA_HOST=n

Quoted 8 lines

Quoted 5 lines
Index: b/drivers/ata/libata-sata.c
===================================================================
drivers/ata/libata-sata.c new file +0 -0
@@ -0,0 +1,1242 @@
[...]
Quoted 1 line

Quoted 8 lines

drivers/ata/libata-sata.c line 379
Quoted 6 lines
drivers/ata/libata-sata.c new file +6 -0
+int sata_set_spd(struct ata_link *link)
+{
+       u32 scontrol;
+       int rc;
+
+       if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
I guess checkpatch.pl protests here...

it does ;)

[...]

Quoted 9 lines

drivers/ata/libata-sata.c line 657
Quoted 5 lines
+ goto out;
drivers/ata/libata-sata.c new file +4 -0
+
+       scontrol = (scontrol&  0x0f0) | 0x301;
+
+       if ((rc = sata_scr_write_flush(link, SCR_CONTROL, scontrol)))
And here...
I understand that you're only moving the code. Perhaps it's worth fixing checkpatch.pl's complaints beforehand -- I was going to look at it...

Because code is moved I preferred to leave CodingStyle changes alone but I also think that it would be useful to fix them one day (though I deferred the work for later due to pragmatic reasons)..

Thanks,
Bartlomiej

Sergei Shtylyov

2011-02-14 12:01 UTC #3 lore raw

Hello.

Quoted 31 lines · Bartlomiej Zolnierkiewicz

Quoted 31 lines
Quoted 21 lines
Add CONFIG_SATA_HOST config option (for selecting SATA Host support) to make setup easier on PATA-only systems.
Additionally move SATA-specific code to libata-sata.c which allows us to save ~11.5k of the output code size (x86-64) on PATA-only systems for CONFIG_SATA_HOST=n:
CONFIG_SATA_HOST=y:
text data bss dec hex filename
   44283    6576      57   50916    c6e4 drivers/ata/libata-core.o
   29054      16       2   29072    7190 drivers/ata/libata-eh.o
   20085       0      19   20104    4e88 drivers/ata/libata-sff.o
    8699       0       0    8699    21fb drivers/ata/libata-sata.o
CONFIG_SATA_HOST=n:
text data bss dec hex filename
   43754    6576      57   50387    c4d3 drivers/ata/libata-core.o
   26775      16       2   26793    68a9 drivers/ata/libata-eh.o
   20144       0      19   20163    4ec3 drivers/ata/libata-sff.o
Signed-off-by Bartlomiej Zolnierkiewicz<[email protected]>
[...]
Quoted 4 lines
Index: b/drivers/ata/libata-core.c
===================================================================
drivers/ata/libata-core.c +0 -0
[...]
Quoted 1 line

Quoted 8 lines

Quoted 6 lines
drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 495
Quoted 3 lines
drivers/ata/libata-core.c +0 -0
@@ -573,34 +494,26 @@ void ata_tf_to_fis(const struct ata_task
        fis[19] = 0;
  }
Hm, what about ata_tf_to_fis()? It shouldn't be needed by non-SATA stuff,
moreover it'should be only needed by non-SFF controllers...
in theory yes but it is actually used by libata-scsi.c

Ah, didn't look here -- I didn't have the full kernel source to grep in...

Quoted 24 lines

Quoted 15 lines

drivers/ata/libata-core.c +3 ~~-11~~

+#ifndef CONFIG_SATA_HOST
+int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
+                     bool spm_wakeup)
  {
-       tf->command     = fis[2];       /* status */
-       tf->feature     = fis[3];       /* error */
-
-       tf->lbal        = fis[4];
-       tf->lbam        = fis[5];
-       tf->lbah        = fis[6];
-       tf->device      = fis[7];
-
-       tf->hob_lbal    = fis[8];
-       tf->hob_lbam    = fis[9];
-       tf->hob_lbah    = fis[10];

Quoted 8 lines

drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 503
Quoted 6 lines
drivers/ata/libata-core.c +3 -3
-
-       tf->nsect       = fis[12];
-       tf->hob_nsect   = fis[13];
+       return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(sata_link_scr_lpm);
Shouldn't there be empty line here?

Quoted 1 line

ata_piix needs it

I didn't doubt -- I just asked about empty line between functions. :-)

Quoted 11 lines

Quoted 8 lines
drivers/ata/libata-core.c void ata_tf_to_fis(const struct ata_task · line 509
Quoted 6 lines
drivers/ata/libata-core.c +6 -0
+int sata_std_hardreset(struct ata_link *link, unsigned int *class,
+                      unsigned long deadline)
+{
+       return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(sata_std_hardreset);
   ... and here?
ditto, and it is quite useful to have possibility of building ata_piix w/ CONFIG_SATA_HOST=n

Of course... but I just asked about empty line. :-)

Quoted 31 lines

Quoted 8 lines
Quoted 5 lines
Index: b/drivers/ata/libata-sata.c
===================================================================
drivers/ata/libata-sata.c new file +0 -0
@@ -0,0 +1,1242 @@
[...]
Quoted 1 line
Quoted 8 lines
drivers/ata/libata-sata.c
Quoted 6 lines
drivers/ata/libata-sata.c new file +6 -0
+int sata_set_spd(struct ata_link *link)
+{
+       u32 scontrol;
+       int rc;
+
+       if ((rc = sata_scr_read(link, SCR_CONTROL,&scontrol)))
I guess checkpatch.pl protests here...
it does ;)
[...]
Quoted 9 lines
drivers/ata/libata-sata.c line 657
Quoted 5 lines
drivers/ata/libata-sata.c new file +5 -0
+               goto out;
+
+       scontrol = (scontrol&    0x0f0) | 0x301;
+
+       if ((rc = sata_scr_write_flush(link, SCR_CONTROL, scontrol)))
And here...
I understand that you're only moving the code. Perhaps it's worth fixing
checkpatch.pl's complaints beforehand -- I was going to look at it...

Quoted 3 lines

Because code is moved I preferred to leave CodingStyle changes alone but I also think that it would be useful to fix them one day (though I deferred the work for later due to pragmatic reasons)..

OK...

Quoted 2 lines

Thanks,
Bartlomiej

WBR, Sergei