Hello, Keith Owens.
Sorry about late respond. I'm quite occupied with personal stuff these
days.
Keith Owens wrote:
(2) I have seen the same intermittent bug on ICH7 SATA but
PIIX_FLAG_IGNORE_PCS is only set for ich5 and i6300esb_sata. It
probably needs to be set for ich7 as well.
No, ICH7 up to this point has been believed to have well-behaving PCS.
If you report PCS problem, you'll be the first. Also, note that ICH7
suffers from ghost device probing problem if PCS is not honored exactly.
Are you sure it's the same problem?
It definitely looks like it. Stock 2.6.18-rc5 plus this patch to
activate ata_debug from boot until just after probing drives.
[--snip--]
<3>piix_sata_present_mask: ata1: ENTER, pcs=0x15 base=0
<3>piix_sata_present_mask: ata1: LEAVE, pcs=0x15 present_mask=0x3
<3>piix_sata_present_mask: ata1: ENTER, pcs=0x0 base=0
<3>piix_sata_present_mask: ata1: LEAVE, pcs=0x0 present_mask=0x3
<3>piix_sata_present_mask: ata1: ENTER, pcs=0x15 base=0
<3>piix_sata_present_mask: ata1: LEAVE, pcs=0x15 present_mask=0x3
<3>piix_sata_present_mask: ata1: ENTER, pcs=0x0 base=0
<3>piix_sata_present_mask: ata1: LEAVE, pcs=0x0 present_mask=0x3
<3>piix_sata_present_mask: ata1: ENTER, pcs=0x15 base=0
Yeah, it definitely looks like it. Does the kernel print a message
which looks like the following before those debug messages?
"updating PCS from 0x0 to 0x5"
Note the pcs=0x0 values. Adding PIIX_FLAG_IGNORE_PCS to
ich6m_sata_ahci gets past the failure to detect pcs, with no sign of
any ghost devices. BTW, dropping down to 2.6.17 with the same config
has no problem detecting the disk, even without PIIX_FLAG_IGNORE_PCS on
ich6m_sata_ahci.
BTW, you can set PIIX_FLAG_IGNORE_PCS by using force_pcs=1 module
parameter now.
lspci extract, this is an ICH7M.
00:1f.0 Class 0601: 8086:27b9 (rev 02)
Subsystem: 1033:832c
Flags: bus master, medium devsel, latency 0
Capabilities: [e0] Vendor Specific Information
00:1f.2 Class 0101: 8086:27c4 (rev 02) (prog-if 80)
Subsystem: 1033:832c
Flags: bus master, 66MHz, medium devsel, latency 0, IRQ 18
I/O ports at <unassigned>
I/O ports at <unassigned>
I/O ports at <unassigned>
I/O ports at <unassigned>
I/O ports at 18b0 [size=16]
Capabilities: [70] Power Management version 2
00:1f.3 Class 0c05: 8086:27da (rev 02)
Subsystem: 1033:832c
Flags: medium devsel, IRQ 11
I/O ports at 18c0 [size=32]
Hmm... Can you try the attached patch and see what happens? ATM, I'm on
the road and can't test the patch, so it's only compile-tested. This
patch basically reverts some of the effects of the following commit and
makes PCS update a little bit more aggressive iff necessary.
ea35d29e2fa8b3d766a2ce8fbcce599dce8d2734
[libata] ata_piix: Consolidate PCS register writing
If this works for you ich7m, can you please test this on your formerly
problematic ich5 with force_pcs=2 specified? I initially thought that
the ich5 problem was caused by exact PCS map change and thus added
IGNORE_PCS as workaround but if the same problem occurs on ich7 and is
fixed by the attached patch, it's due to conservative PCS update change
and thus the original IGNORE_PCS fix on ich5 might not be necessary.
Sorry this PCS business causes you and other people so much trouble. It
just doesn't work quite as ata_piix developers expect.
Thanks.
--
tejun
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 2d20caf..46f7c9b 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -553,15 +553,42 @@ static unsigned int piix_sata_present_ma
{
struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
struct piix_host_priv *hpriv = ap->host_set->private_data;
+ const struct piix_map_db *map_db = hpriv->map_db;
const unsigned int *map = hpriv->map;
int base = 2 * ap->hard_port_no;
unsigned int present_mask = 0;
int port, i;
- u16 pcs;
+ u16 pcs, new_pcs;
pci_read_config_word(pdev, ICH5_PCS, &pcs);
DPRINTK("ata%u: ENTER, pcs=0x%x base=%d\n", ap->id, pcs, base);
+ new_pcs = pcs | map_db->port_enable;
+
+ if (pcs != new_pcs) {
+ u16 old_pcs = pcs;
+
+ for (i = 0; i < 10; i++) {
+ pci_write_config_word(pdev, ICH5_PCS, new_pcs);
+ msleep(150);
+ pci_read_config_word(pdev, ICH5_PCS, &pcs);
+
+ new_pcs = pcs | map_db->port_enable;
+ if (pcs == new_pcs)
+ break;
+ }
+
+ if (pcs == new_pcs)
+ ata_port_printk(ap, KERN_INFO, "updated PCS from "
+ "0x%x to 0x%x (%d tries)\n",
+ old_pcs, pcs, i);
+ else
+ ata_port_printk(ap, KERN_WARNING,
+ "failed to update PCS after %d tries, "
+ "old=0x%x cur=0x%x new=0x%x\n",
+ i, old_pcs, pcs, new_pcs);
+ }
+
for (i = 0; i < 2; i++) {
port = map[base + i];
if (port < 0)
@@ -816,35 +843,6 @@ static int __devinit piix_check_450nx_er
return no_piix_dma;
}
-static void __devinit piix_init_pcs(struct pci_dev *pdev,
- struct ata_port_info *pinfo,
- const struct piix_map_db *map_db)
-{
- u16 pcs, new_pcs;
-
- pci_read_config_word(pdev, ICH5_PCS, &pcs);
-
- new_pcs = pcs | map_db->port_enable;
-
- if (new_pcs != pcs) {
- DPRINTK("updating PCS from 0x%x to 0x%x\n", pcs, new_pcs);
- pci_write_config_word(pdev, ICH5_PCS, new_pcs);
- msleep(150);
- }
-
- if (force_pcs == 1) {
- dev_printk(KERN_INFO, &pdev->dev,
- "force ignoring PCS (0x%x)\n", new_pcs);
- pinfo[0].host_flags |= PIIX_FLAG_IGNORE_PCS;
- pinfo[1].host_flags |= PIIX_FLAG_IGNORE_PCS;
- } else if (force_pcs == 2) {
- dev_printk(KERN_INFO, &pdev->dev,
- "force honoring PCS (0x%x)\n", new_pcs);
- pinfo[0].host_flags &= ~PIIX_FLAG_IGNORE_PCS;
- pinfo[1].host_flags &= ~PIIX_FLAG_IGNORE_PCS;
- }
-}
-
static void __devinit piix_init_sata_map(struct pci_dev *pdev,
struct ata_port_info *pinfo,
const struct piix_map_db *map_db)
@@ -893,6 +891,17 @@ static void __devinit piix_init_sata_map
hpriv->map = map;
hpriv->map_db = map_db;
+
+ /* handle force_pcs module parameter */
+ if (force_pcs == 1) {
+ dev_printk(KERN_INFO, &pdev->dev, "force ignoring PCS\n");
+ pinfo[0].host_flags |= PIIX_FLAG_IGNORE_PCS;
+ pinfo[1].host_flags |= PIIX_FLAG_IGNORE_PCS;
+ } else if (force_pcs == 2) {
+ dev_printk(KERN_INFO, &pdev->dev, "force honoring PCS\n");
+ pinfo[0].host_flags &= ~PIIX_FLAG_IGNORE_PCS;
+ pinfo[1].host_flags &= ~PIIX_FLAG_IGNORE_PCS;
+ }
}
/**
@@ -948,12 +957,9 @@ static int piix_init_one (struct pci_dev
}
/* Initialize SATA map */
- if (host_flags & ATA_FLAG_SATA) {
+ if (host_flags & ATA_FLAG_SATA)
piix_init_sata_map(pdev, port_info,
piix_map_db_table[ent->driver_data]);
- piix_init_pcs(pdev, port_info,
- piix_map_db_table[ent->driver_data]);
- }
/* On ICH5, some BIOSen disable the interrupt using the
* PCI_COMMAND_INTX_DISABLE bit added in PCI 2.3.
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
