From: "unicorn chang" <unicorn.mail@gmail.com>
To: "Jeff Garzik" <jeff@garzik.org>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
akpm@osdl.org, albertcc@tw.ibm.com, greg@kroah.com
Subject: Re: [PATCH v1] ata_piix: attempt to fix
Date: Thu, 13 Jul 2006 15:12:06 +0800 [thread overview]
Message-ID: <e4c012fe0607130012p1749e438vd0b575e80d303b7a@mail.gmail.com> (raw)
In-Reply-To: <20060711160202.GA2503@havoc.gtf.org>
I test this patch on IBM Intellistation M Pro with Quantum GoVault
(kernel 2.6.18-rc1),
the "long boot delay" and "ghost devices" are still existing.
I also test tejun's patch for "ghost devices", it's worked for me.
(http://marc.theaimsgroup.com/?l=linux-ide&m=115164294506130&w=2)
SATA controller is configured to "Compatible" mode.
Intel Corporation 82801GB/GR/GH (ICH7 Family) Serial ATA Storage
Controllers cc=IDE (rev 01)
Unicorn
---------- dmesg
---------------------------------------------------------------------------------------------------------
ata_piix 0000:00:1f.2: MAP [ P0 P2 P1 P3 ]
piix_init_pcs: updating PCS from 0x33 to 0x3f
ata_pci_init_one: ENTER
Device `[IDE1]' is not power manageable<6>ACPI: PCI Interrupt
0000:00:1f.2[B] -> GSI 19 (level, low) -> IRQ 19
PCI: Setting latency timer of device 0000:00:1f.2 to 64
ata_device_add: ENTER
ata_host_add: ENTER
ata_port_start: prd alloc, virt dfa03000, dma 1fa03000
ata3: SATA max UDMA/133 cmd 0x3828 ctl 0x381E bmdma 0x34F0 irq 19
__ata_port_freeze: ata3 port frozen
ata_host_add: ENTER
ata_port_start: prd alloc, virt dfa93000, dma 1fa93000
ata4: SATA max UDMA/133 cmd 0x3820 ctl 0x381A bmdma 0x34F8 irq 19
__ata_port_freeze: ata4 port frozen
ata_device_add: probe begin
scsi2 : ata_piix
PM: Adding info for No Bus:host2
ata_port_schedule_eh: port EH scheduled
ata_scsi_error: ENTER
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata3: ata_port_flush_task: flush #2
ata3: ata_port_flush_task: EXIT
ata_eh_autopsy: ENTER
ata_eh_recover: ENTER
ata_eh_prep_resume: ENTER
ata_eh_prep_resume: EXIT
__ata_port_freeze: ata3 port frozen
piix_sata_prereset: ata3: ENTER, pcs=0x3f base=0
piix_sata_prereset: ata3: LEAVE, pcs=0x3f present_mask=0x1
ata_std_softreset: ENTER
ata_std_softreset: about to softreset, devmask=3
ata_bus_softreset: ata3: bus reset via SRST
ata_dev_classify: found ATA device by sig
ata_dev_classify: found ATA device by sig
ata_std_softreset: EXIT, classes[0]=1 [1]=5
ata_std_postreset: ENTER
ata_std_postreset: EXIT
ata_eh_thaw_port: ata3 port thawed
ata_eh_revalidate_and_attach: ENTER
ata3.00: ata_dev_read_id: ENTER, host 3, dev 0
ata_pio_sector: data read
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata3: ata_port_flush_task: flush #2
ata3: ata_port_flush_task: EXIT
ata3.00: ATA-7, max UDMA/133, 156312576 sectors: LBA48 NCQ (depth 0/32)
ata3.00: ata3: dev 0 multi count 16
ata_eh_revalidate_and_attach: EXIT
ata_eh_resume: ENTER
ata_eh_resume: EXIT
ata_dev_set_xfermode: set features - xfer mode
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata3: ata_port_flush_task: flush #2
ata3: ata_port_flush_task: EXIT
ata_dev_set_xfermode: EXIT, err_mask=0
ata3.00: ata_dev_read_id: ENTER, host 3, dev 0
ata_pio_sector: data read
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata3: ata_port_flush_task: flush #2
ata3: ata_port_flush_task: EXIT
ata_dev_set_mode: xfer_shift=8, xfer_mode=0x46
ata3.00: configured for UDMA/133
ata_eh_suspend: ENTER
ata_eh_suspend: EXIT
ata_eh_recover: EXIT, rc=0
ata_scsi_error: EXIT
scsi3 : ata_piix
PM: Adding info for No Bus:host3
ata_port_schedule_eh: port EH scheduled
ata_scsi_error: ENTER
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata4: ata_port_flush_task: flush #2
ata4: ata_port_flush_task: EXIT
ata_eh_autopsy: ENTER
ata_eh_recover: ENTER
ata_eh_prep_resume: ENTER
ata_eh_prep_resume: EXIT
__ata_port_freeze: ata4 port frozen
piix_sata_prereset: ata4: ENTER, pcs=0x3f base=2
piix_sata_prereset: ata4: LEAVE, pcs=0x3f present_mask=0x1
ata_std_softreset: ENTER
ata_std_softreset: about to softreset, devmask=3
ata_bus_softreset: ata4: bus reset via SRST
ata_dev_classify: found ATAPI device by sig
ata_dev_classify: found ATAPI device by sig
ata_std_softreset: EXIT, classes[0]=3 [1]=3
ata_std_postreset: ENTER
ata_std_postreset: EXIT
ata_eh_thaw_port: ata4 port thawed
ata_eh_revalidate_and_attach: ENTER
ata4.00: ata_dev_read_id: ENTER, host 4, dev 0
ata_exec_command_pio: ata4: cmd 0xA1
ata_hsm_move: ata4: protocol 2 task_state 2 (dev_stat 0x58)
ata_pio_sector: data read
ata_hsm_move: ata4: protocol 2 task_state 3 (dev_stat 0x50)
ata_hsm_move: ata4: dev 0 command complete, drv_stat 0x50
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata4: ata_port_flush_task: flush #2
ata4: ata_port_flush_task: EXIT
ata4.00: ATAPI, max UDMA/66, CDB intr
ata4.01: ata_dev_read_id: ENTER, host 4, dev 1
ata_exec_command_pio: ata4: cmd 0xA1
ata_port_flush_task: ENTER
ata_port_flush_task: flush #1
ata4: ata_port_flush_task: flush #2
ata4: ata_port_flush_task: EXIT
__ata_port_freeze: ata4 port frozen
ata4.01: qc timeout (cmd 0xa1)
ata4.01: failed to IDENTIFY (I/O error, err_mask=0x4)
ata_eh_revalidate_and_attach: EXIT
ata4: failed to recover some devices, retrying in 5 secs
On 7/12/06, Jeff Garzik <jeff@garzik.org> wrote:
> This patch attempts to address problems on ata_piix that people have
> been reporting: long boot delay, ghost devices, and ICH8 brokenness.
>
> Testing feedback is requested as soon as possible, so that we can
> potentially stick this into 2.6.18-rc2.
>
> I'm booting up several boxes locally here, mainly ICH5 and ICH8, as well.
>
>
> drivers/scsi/ata_piix.c | 151 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 106 insertions(+), 45 deletions(-)
>
>
> commit aeff1cdc5e86f0ce6267c1ae76d150064e05faf6
> Author: Jeff Garzik <jeff@garzik.org>
> Date: Tue Jul 11 11:57:44 2006 -0400
>
> [libata] ata_piix: attempt to fix ICH8 support
>
> Take into account the fact that ICH8 changed the register layout of
> the MAP and PCS register bits.
>
> Signed-off-by: Jeff Garzik <jeff@garzik.org>
>
> commit 15dcf5b9868382026c268529d2bb922db676cf63
> Author: Jeff Garzik <jeff@garzik.org>
> Date: Tue Jul 11 11:48:50 2006 -0400
>
> [libata] ata_piix: Consolidate PCS register writing
>
> Prior to this patch, the driver would do this for each port:
> read 8-bit PCS
> write 8-bit PCS
> read 8-bit PCS
> write 8-bit PCS
>
> In the field, flaky behavior has been observed related to this register.
> In particular, these overzealous register writes can cause misdetection
> problems.
>
> Update to do the following once (not once per port) at boot:
> read 16-bit PCS
> if needs changing,
> write 16-bit PCS
>
> And thereafter, we only perform a 'read 16-bit PCS' per port.
>
> This should eliminate all PCS writes in many cases, and be more friendly
> in the cases where we do need to enable ports.
>
> Signed-off-by: Jeff Garzik <jeff@garzik.org>
>
> commit 38c7dbfd2883dccfdeaaa22cd809561000577908
> Author: Tejun Heo <htejun@gmail.com>
> Date: Thu Jun 29 01:58:28 2006 +0900
>
> [PATCH] ata_piix: add host_set private structure
>
> Add host_set private structure piix_host_priv. Currently the only
> field is ->map which used to be stored directly at
> host_set->private_data. This change allows more host_set private
> fields to be added.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Signed-off-by: Jeff Garzik <jeff@garzik.org>
>
>
>
> diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
> index 94b1261..a960bb7 100644
> --- a/drivers/scsi/ata_piix.c
> +++ b/drivers/scsi/ata_piix.c
> @@ -126,6 +126,7 @@ enum {
> ich6_sata = 4,
> ich6_sata_ahci = 5,
> ich6m_sata_ahci = 6,
> + ich8_sata_ahci = 7,
>
> /* constants for mapping table */
> P0 = 0, /* port 0 */
> @@ -141,11 +142,19 @@ enum {
>
> struct piix_map_db {
> const u32 mask;
> + const u32 port_enable;
> + const int present_shift;
> const int map[][4];
> };
>
> +struct piix_host_priv {
> + const int *map;
> + const struct piix_map_db *map_db;
> +};
> +
> static int piix_init_one (struct pci_dev *pdev,
> const struct pci_device_id *ent);
> +static void piix_host_stop(struct ata_host_set *host_set);
> static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
> static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
> static void piix_pata_error_handler(struct ata_port *ap);
> @@ -186,11 +195,11 @@ #endif
> /* Enterprise Southbridge 2 (where's the datasheet?) */
> { 0x8086, 0x2680, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6_sata_ahci },
> /* SATA Controller 1 IDE (ICH8, no datasheet yet) */
> - { 0x8086, 0x2820, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6_sata_ahci },
> + { 0x8086, 0x2820, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_ahci },
> /* SATA Controller 2 IDE (ICH8, ditto) */
> - { 0x8086, 0x2825, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6_sata_ahci },
> + { 0x8086, 0x2825, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_ahci },
> /* Mobile SATA Controller IDE (ICH8M, ditto) */
> - { 0x8086, 0x2828, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6m_sata_ahci },
> + { 0x8086, 0x2828, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_ahci },
>
> { } /* terminate list */
> };
> @@ -254,7 +263,7 @@ static const struct ata_port_operations
>
> .port_start = ata_port_start,
> .port_stop = ata_port_stop,
> - .host_stop = ata_host_stop,
> + .host_stop = piix_host_stop,
> };
>
> static const struct ata_port_operations piix_sata_ops = {
> @@ -284,11 +293,13 @@ static const struct ata_port_operations
>
> .port_start = ata_port_start,
> .port_stop = ata_port_stop,
> - .host_stop = ata_host_stop,
> + .host_stop = piix_host_stop,
> };
>
> -static struct piix_map_db ich5_map_db = {
> +static const struct piix_map_db ich5_map_db = {
> .mask = 0x7,
> + .port_enable = 0x3,
> + .present_shift = 4,
> .map = {
> /* PM PS SM SS MAP */
> { P0, NA, P1, NA }, /* 000b */
> @@ -302,8 +313,10 @@ static struct piix_map_db ich5_map_db =
> },
> };
>
> -static struct piix_map_db ich6_map_db = {
> +static const struct piix_map_db ich6_map_db = {
> .mask = 0x3,
> + .port_enable = 0xf,
> + .present_shift = 4,
> .map = {
> /* PM PS SM SS MAP */
> { P0, P2, P1, P3 }, /* 00b */
> @@ -313,8 +326,10 @@ static struct piix_map_db ich6_map_db =
> },
> };
>
> -static struct piix_map_db ich6m_map_db = {
> +static const struct piix_map_db ich6m_map_db = {
> .mask = 0x3,
> + .port_enable = 0x5,
> + .present_shift = 4,
> .map = {
> /* PM PS SM SS MAP */
> { P0, P2, RV, RV }, /* 00b */
> @@ -324,6 +339,28 @@ static struct piix_map_db ich6m_map_db =
> },
> };
>
> +static const struct piix_map_db ich8_map_db = {
> + .mask = 0x3,
> + .port_enable = 0x3,
> + .present_shift = 8,
> + .map = {
> + /* PM PS SM SS MAP */
> + { P0, RV, P1, RV }, /* 00b (hardwired) */
> + { RV, RV, RV, RV },
> + { RV, RV, RV, RV }, /* 10b (never) */
> + { RV, RV, RV, RV },
> + },
> +};
> +
> +static const struct piix_map_db *piix_map_db_table[] = {
> + [ich5_sata] = &ich5_map_db,
> + [esb_sata] = &ich5_map_db,
> + [ich6_sata] = &ich6_map_db,
> + [ich6_sata_ahci] = &ich6_map_db,
> + [ich6m_sata_ahci] = &ich6m_map_db,
> + [ich8_sata_ahci] = &ich8_map_db,
> +};
> +
> static struct ata_port_info piix_port_info[] = {
> /* piix4_pata */
> {
> @@ -362,7 +399,6 @@ #endif
> .mwdma_mask = 0x07, /* mwdma0-2 */
> .udma_mask = 0x7f, /* udma0-6 */
> .port_ops = &piix_sata_ops,
> - .private_data = &ich5_map_db,
> },
>
> /* i6300esb_sata */
> @@ -374,7 +410,6 @@ #endif
> .mwdma_mask = 0x07, /* mwdma0-2 */
> .udma_mask = 0x7f, /* udma0-6 */
> .port_ops = &piix_sata_ops,
> - .private_data = &ich5_map_db,
> },
>
> /* ich6_sata */
> @@ -386,7 +421,6 @@ #endif
> .mwdma_mask = 0x07, /* mwdma0-2 */
> .udma_mask = 0x7f, /* udma0-6 */
> .port_ops = &piix_sata_ops,
> - .private_data = &ich6_map_db,
> },
>
> /* ich6_sata_ahci */
> @@ -399,7 +433,6 @@ #endif
> .mwdma_mask = 0x07, /* mwdma0-2 */
> .udma_mask = 0x7f, /* udma0-6 */
> .port_ops = &piix_sata_ops,
> - .private_data = &ich6_map_db,
> },
>
> /* ich6m_sata_ahci */
> @@ -412,7 +445,18 @@ #endif
> .mwdma_mask = 0x07, /* mwdma0-2 */
> .udma_mask = 0x7f, /* udma0-6 */
> .port_ops = &piix_sata_ops,
> - .private_data = &ich6m_map_db,
> + },
> +
> + /* ich8_sata_ahci */
> + {
> + .sht = &piix_sht,
> + .host_flags = ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
> + PIIX_FLAG_CHECKINTR | PIIX_FLAG_SCR |
> + PIIX_FLAG_AHCI,
> + .pio_mask = 0x1f, /* pio0-4 */
> + .mwdma_mask = 0x07, /* mwdma0-2 */
> + .udma_mask = 0x7f, /* udma0-6 */
> + .port_ops = &piix_sata_ops,
> },
> };
>
> @@ -508,46 +552,29 @@ static void piix_pata_error_handler(stru
> static int piix_sata_prereset(struct ata_port *ap)
> {
> struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
> - const unsigned int *map = ap->host_set->private_data;
> + struct piix_host_priv *hpriv = ap->host_set->private_data;
> + const unsigned int *map = hpriv->map;
> int base = 2 * ap->hard_port_no;
> - unsigned int present_mask = 0;
> + unsigned int present = 0;
> int port, i;
> - u8 pcs;
> + u16 pcs;
>
> - pci_read_config_byte(pdev, ICH5_PCS, &pcs);
> + pci_read_config_word(pdev, ICH5_PCS, &pcs);
> DPRINTK("ata%u: ENTER, pcs=0x%x base=%d\n", ap->id, pcs, base);
>
> - /* enable all ports on this ap and wait for them to settle */
> - for (i = 0; i < 2; i++) {
> - port = map[base + i];
> - if (port >= 0)
> - pcs |= 1 << port;
> - }
> -
> - pci_write_config_byte(pdev, ICH5_PCS, pcs);
> - msleep(100);
> -
> - /* let's see which devices are present */
> - pci_read_config_byte(pdev, ICH5_PCS, &pcs);
> -
> for (i = 0; i < 2; i++) {
> port = map[base + i];
> if (port < 0)
> continue;
> - if (ap->flags & PIIX_FLAG_IGNORE_PCS || pcs & 1 << (4 + port))
> - present_mask |= 1 << i;
> - else
> - pcs &= ~(1 << port);
> + if ((ap->flags & PIIX_FLAG_IGNORE_PCS) ||
> + (pcs & 1 << (hpriv->map_db->present_shift + port)))
> + present = 1;
> }
>
> - /* disable offline ports on non-AHCI controllers */
> - if (!(ap->flags & PIIX_FLAG_AHCI))
> - pci_write_config_byte(pdev, ICH5_PCS, pcs);
> -
> DPRINTK("ata%u: LEAVE, pcs=0x%x present_mask=0x%x\n",
> ap->id, pcs, present_mask);
>
> - if (!present_mask) {
> + if (!present) {
> ata_port_printk(ap, KERN_INFO, "SATA port has no device.\n");
> ap->eh_context.i.action &= ~ATA_EH_RESET_MASK;
> return 0;
> @@ -761,10 +788,27 @@ static int __devinit piix_check_450nx_er
> return no_piix_dma;
> }
>
> +static void __devinit piix_init_pcs(struct pci_dev *pdev,
> + 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);
> + }
> +}
> +
> static void __devinit piix_init_sata_map(struct pci_dev *pdev,
> - struct ata_port_info *pinfo)
> + struct ata_port_info *pinfo,
> + const struct piix_map_db *map_db)
> {
> - struct piix_map_db *map_db = pinfo[0].private_data;
> + struct piix_host_priv *hpriv = pinfo[0].private_data;
> const unsigned int *map;
> int i, invalid_map = 0;
> u8 map_value;
> @@ -805,8 +849,8 @@ static void __devinit piix_init_sata_map
> dev_printk(KERN_ERR, &pdev->dev,
> "invalid MAP value %u\n", map_value);
>
> - pinfo[0].private_data = (void *)map;
> - pinfo[1].private_data = (void *)map;
> + hpriv->map = map;
> + hpriv->map_db = map_db;
> }
>
> /**
> @@ -829,6 +873,7 @@ static int piix_init_one (struct pci_dev
> static int printed_version;
> struct ata_port_info port_info[2];
> struct ata_port_info *ppinfo[2] = { &port_info[0], &port_info[1] };
> + struct piix_host_priv *hpriv;
> unsigned long host_flags;
>
> if (!printed_version++)
> @@ -839,8 +884,14 @@ static int piix_init_one (struct pci_dev
> if (!in_module_init)
> return -ENODEV;
>
> + hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
> + if (!hpriv)
> + return -ENOMEM;
> +
> port_info[0] = piix_port_info[ent->driver_data];
> port_info[1] = piix_port_info[ent->driver_data];
> + port_info[0].private_data = hpriv;
> + port_info[1].private_data = hpriv;
>
> host_flags = port_info[0].host_flags;
>
> @@ -855,8 +906,11 @@ static int piix_init_one (struct pci_dev
> }
>
> /* Initialize SATA map */
> - if (host_flags & ATA_FLAG_SATA)
> - piix_init_sata_map(pdev, port_info);
> + if (host_flags & ATA_FLAG_SATA) {
> + piix_init_sata_map(pdev, port_info,
> + piix_map_db_table[ent->driver_data]);
> + piix_init_pcs(pdev, 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.
> @@ -879,6 +933,13 @@ static int piix_init_one (struct pci_dev
> return ata_pci_init_one(pdev, ppinfo, 2);
> }
>
> +static void piix_host_stop(struct ata_host_set *host_set)
> +{
> + if (host_set->next == NULL)
> + kfree(host_set->private_data);
> + ata_host_stop(host_set);
> +}
> +
> static int __init piix_init(void)
> {
> int rc;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2006-07-13 7:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-07-11 16:02 Jeff Garzik
2006-07-13 7:12 ` unicorn chang [this message]
2006-07-13 23:50 ` Greg KH
2006-07-14 3:54 ` cjacker huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e4c012fe0607130012p1749e438vd0b575e80d303b7a@mail.gmail.com \
--to=unicorn.mail@gmail.com \
--cc=akpm@osdl.org \
--cc=albertcc@tw.ibm.com \
--cc=greg@kroah.com \
--cc=jeff@garzik.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®