Hello.

On 11-02-2011 16:33, Bartlomiej Zolnierkiewicz wrote:

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<bzolnier@gmail.com>

[...]

[...]

    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...

+#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];
-
-	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?

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

    ... and here?

[...]

+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...

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

    Here too...

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

[...]

+int sata_link_debounce(struct ata_link *link, const unsigned long *params,
+		       unsigned long deadline)
+{
+	unsigned long interval = params[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...

+/**
+ *	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:
+ *	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...

+		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)))
+			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...

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

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

    Here too...

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

[...]

+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
+		 * 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...

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

    Here too...

+		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