2006-11-22 17:31:10

by Alan

[permalink] [raw]
Subject: [PATCH] pata_sil680 suspend/resume

The SI680 can come back from s2ram with the clocks disabled (crash time)
or wrong (ugly as this can cause CRC errors, and in theory corruption).
On a resume we must put the clock back.

Signed-off-by: Alan Cox <[email protected]>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.19-rc5-mm2/drivers/ata/pata_sil680.c linux-2.6.19-rc5-mm2/drivers/ata/pata_sil680.c
--- linux.vanilla-2.6.19-rc5-mm2/drivers/ata/pata_sil680.c 2006-11-15 13:26:00.000000000 +0000
+++ linux-2.6.19-rc5-mm2/drivers/ata/pata_sil680.c 2006-11-22 14:44:57.221814608 +0000
@@ -33,7 +33,7 @@
#include <linux/libata.h>

#define DRV_NAME "pata_sil680"
-#define DRV_VERSION "0.3.2"
+#define DRV_VERSION "0.4.1"

/**
* sil680_selreg - return register base
@@ -262,32 +262,20 @@
.host_stop = ata_host_stop
};

-static int sil680_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
+/**
+ * sil680_init_chip - chip setup
+ * @pdev: PCI device
+ *
+ * Perform all the chip setup which must be done both when the device
+ * is powered up on boot and when we resume in case we resumed from RAM.
+ * Returns the final clock settings.
+ */
+
+static u8 sil680_init_chip(struct pci_dev *pdev)
{
- static struct ata_port_info info = {
- .sht = &sil680_sht,
- .flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
- .pio_mask = 0x1f,
- .mwdma_mask = 0x07,
- .udma_mask = 0x7f,
- .port_ops = &sil680_port_ops
- };
- static struct ata_port_info info_slow = {
- .sht = &sil680_sht,
- .flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
- .pio_mask = 0x1f,
- .mwdma_mask = 0x07,
- .udma_mask = 0x3f,
- .port_ops = &sil680_port_ops
- };
- static struct ata_port_info *port_info[2] = {&info, &info};
- static int printed_version;
u32 class_rev = 0;
u8 tmpbyte = 0;

- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
-
pci_read_config_dword(pdev, PCI_CLASS_REVISION, &class_rev);
class_rev &= 0xff;
/* FIXME: double check */
@@ -322,8 +310,6 @@
pci_read_config_byte(pdev, 0x8A, &tmpbyte);
printk(KERN_INFO "sil680: BA5_EN = %d clock = %02X\n",
tmpbyte & 1, tmpbyte & 0x30);
- if ((tmpbyte & 0x30) == 0)
- port_info[0] = port_info[1] = &info_slow;

pci_write_config_byte(pdev, 0xA1, 0x72);
pci_write_config_word(pdev, 0xA2, 0x328A);
@@ -342,11 +328,51 @@
case 0x20: printk(KERN_INFO "sil680: Using PCI clock.\n");break;
/* This last case is _NOT_ ok */
case 0x30: printk(KERN_ERR "sil680: Clock disabled ?\n");
- return -EIO;
+ }
+ return tmpbyte & 0x30;
+}
+
+static int sil680_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+ static struct ata_port_info info = {
+ .sht = &sil680_sht,
+ .flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
+ .pio_mask = 0x1f,
+ .mwdma_mask = 0x07,
+ .udma_mask = 0x7f,
+ .port_ops = &sil680_port_ops
+ };
+ static struct ata_port_info info_slow = {
+ .sht = &sil680_sht,
+ .flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
+ .pio_mask = 0x1f,
+ .mwdma_mask = 0x07,
+ .udma_mask = 0x3f,
+ .port_ops = &sil680_port_ops
+ };
+ static struct ata_port_info *port_info[2] = {&info, &info};
+ static int printed_version;
+
+ if (!printed_version++)
+ dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
+
+ switch(sil680_init_chip(pdev))
+ {
+ case 0:
+ port_info[0] = port_info[1] = &info_slow;
+ break;
+ case 0x30:
+ return -ENODEV;
}
return ata_pci_init_one(pdev, port_info, 2);
}

+static int sil680_reinit_one(struct pci_dev *pdev)
+{
+ sil680_init_chip(pdev);
+ return ata_pci_device_resume(pdev);
+}
+
static const struct pci_device_id sil680[] = {
{ PCI_VDEVICE(CMD, PCI_DEVICE_ID_SII_680), },

@@ -357,7 +383,9 @@
.name = DRV_NAME,
.id_table = sil680,
.probe = sil680_init_one,
- .remove = ata_pci_remove_one
+ .remove = ata_pci_remove_one,
+ .suspend = ata_pci_device_suspend,
+ .resume = sil680_reinit_one,
};

static int __init sil680_init(void)


2007-02-26 21:17:43

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH] pata_sil680 suspend/resume

Alan,

I'm trying to suspend to RAM a KuroboxHG: powerpc MPC8241 based system
with a sil680

00:0c.0 IDE interface: Silicon Image, Inc. PCI0680 Ultra ATA-133 Host Controller (rev 02)

PATA controller, using pata_sil680. This is not a real str, i.e., power is
not switched off, just pull the whole system through suspend, and at the
end put the peripheral controller and the CPU core to sleep.

To suspend I just use a script like:

hdparm -Y /dev/sda
echo -n "mem" > /sys/power/state
hdparm -y /dev/sda

where the last hdparm is meant to wake up the drive (ok, it doesn't:-)),
tried without it too.

With an approximately 2.6.20 kernel I could suspend, but on resume I got

ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen
ata1.00: tag 0 cmd 0xe0 Emask 0x4 stat 0x40 err 0x0 (timeout)
ata1: port is slow to respond, please be patient (Status 0xd0)
ata1: port failed to respond (30 secs, Status 0xd0)
ata1: soft resetting port
ata1.00: configured for UDMA/100
ata1: EH complete
SCSI device sda: 40020624 512-byte hdwr sectors (20491 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write back

but then it worked - congratulations to all authors on a robust error
handling! If I remove the "hdparm -y" I get a similar error on the first
access.

With a post 2.6.20 kernel from powerpc.git I cannot suspend at all:

pata_sil680 0000:00:0c.0: suspend
ata1: suspend failed, device 0 still active
pci_device_suspend(): ata_pci_device_suspend+0x0/0x74() returns -16
suspend_device(): pci_device_suspend+0x0/0xac() returns -16
Could not suspend device 0000:00:0c.0: error -16

Same without "hdparm -Y". Should I use sdparm instead? but
"sdparm --command stop /dev/sda" doesn't spin the disk down.

Looking at drivers/ata/libata-eh.c it looks like ata_eh_suspend() should
spin the disk down by itself and set the ATA_DFLAG_SUSPENDED flag, but it
doesn't seem to happen. Or is it only for SATA?

Thanks
Guennadi
---
Guennadi Liakhovetski

2007-02-26 21:38:52

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH] pata_sil680 suspend/resume

On Mon, 26 Feb 2007, Guennadi Liakhovetski wrote:

> With an approximately 2.6.20 kernel I could suspend, but on resume I got

Sorry, a "small" correction - it turned out to be a 2.6.19-rc4-ish kernel,
so, it is still without your suspend / resume patch, hence we can just
forget it. Remains to clarify why the new one doesn't allow to suspend.

Thanks
Guennadi
---
Guennadi Liakhovetski

2007-02-28 21:19:19

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH] pata_sil680 suspend/resume

On Mon, 26 Feb 2007, Guennadi Liakhovetski wrote:

> With a post 2.6.20 kernel from powerpc.git I cannot suspend at all:
>
> pata_sil680 0000:00:0c.0: suspend
> ata1: suspend failed, device 0 still active
> pci_device_suspend(): ata_pci_device_suspend+0x0/0x74() returns -16
> suspend_device(): pci_device_suspend+0x0/0xac() returns -16
> Could not suspend device 0000:00:0c.0: error -16

AFAICS, "still active" is printed from ata_host_suspend() if a device
(disk) on the host to be suspended doesn't have ATA_DFLAG_SUSPENDED flag
set. This flag is only set in ata_eh_suspend(), which is only called from
ata_eh_recover(), like this:

generic_error_handler()
ata_bmdma_drive_eh()
ata_do_eh()
ata_eh_recover()
ata_eh_suspend()
dev->flags |= ATA_DFLAG_SUSPENDED;

but I don't understand why the error handler should be envoked? Should the
"disk" be suspended before the host and is it when the eh should set the
flag? If my guess is right - why doesn't the disk get suspended on my
machine? Shall I suspend it explicitely from userspace? I do "hdparm -Y",
and it does stop spinning", but I still get the error.

Thanks
Guennadi
---
Guennadi Liakhovetski