From: [email protected]
There was an ASIC bug in the SB600 SATA controller of low revision (<=13) and CD burning may hang (only SATA ODD has this issue, and SATA HDD works well). The patch provides a workaround for this issue.
Signed-off-by: Luugi Marsan <[email protected]>
--- linux-2.6.19-rc4-git5/drivers/ata/ahci.c.orig 2006-11-04 03:56:22.000000000 +0800
+++ linux-2.6.19-rc4-git5/drivers/ata/ahci.c 2006-11-04 04:20:36.000000000 +0800
@@ -189,6 +189,7 @@ struct ahci_host_priv {
unsigned long flags;
u32 cap; /* cache of HOST_CAP register */
u32 port_map; /* cache of HOST_PORTS_IMPL reg */
+ u8 rev; /* PCI Revision ID */
};
struct ahci_port_priv {
@@ -220,6 +221,7 @@ static int ahci_port_resume(struct ata_p
static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
static int ahci_pci_device_resume(struct pci_dev *pdev);
static void ahci_remove_one (struct pci_dev *pdev);
+static int ahci_check_atapi_dma(struct ata_queued_cmd *qc);
static struct scsi_host_template ahci_sht = {
.module = THIS_MODULE,
@@ -251,6 +253,8 @@ static const struct ata_port_operations
.tf_read = ahci_tf_read,
+ .check_atapi_dma = ahci_check_atapi_dma,
+
.qc_prep = ahci_qc_prep,
.qc_issue = ahci_qc_issue,
@@ -906,6 +910,28 @@ static unsigned int ahci_fill_sg(struct
return n_sg;
}
+static int ahci_check_atapi_dma(struct ata_queued_cmd *qc)
+{
+ struct pci_dev *pdev = to_pci_dev(qc->ap->host->dev);
+
+ /* walkaround for SB600 SATA ODD isuue */
+ if (0x1002 == pdev->vendor && 0x4380 == pdev->device)
+ {
+ struct ahci_host_priv *priv = qc->ap->host->private_data;
+ u32 rq_len, low_8k;
+
+ if ( 13 < priv->rev )
+ return 0;
+
+ rq_len = qc->scsicmd->request_bufflen;
+ low_8k = rq_len & 0x1fff;
+
+ if ( (rq_len & 0xffffe000) && low_8k && (512 > low_8k) )
+ return 1;
+ }
+ return 0;
+}
+
static void ahci_qc_prep(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc->ap;
@@ -1366,6 +1392,7 @@ static int ahci_host_init(struct ata_pro
hpriv->cap = readl(mmio + HOST_CAP);
hpriv->port_map = readl(mmio + HOST_PORTS_IMPL);
+ pci_read_config_byte(pdev, PCI_REVISION_ID, &hpriv->rev);
probe_ent->n_ports = (hpriv->cap & 0x1f) + 1;
VPRINTK("cap 0x%x port_map 0x%x n_ports %d\n",
On Fri, Nov 03, 2006 at 02:00:04PM -0500, Luugi Marsan wrote:
> From: [email protected]
>
> There was an ASIC bug in the SB600 SATA controller of low revision (<=13) and CD burning may hang (only SATA ODD has this issue, and SATA HDD works well). The patch provides a workaround for this issue.
Please make the code ahear to Documentation/CodingStyle (aka the style
used everywhere else in ahci.c..)
Something like:
static int ahci_check_atapi_dma(struct ata_queued_cmd *qc)
{
struct pci_dev *pdev = to_pci_dev(qc->ap->host->dev);
/*
* Walkaround for ATI/AMD SB600 SATA ODD isue.
*/
if (pdev->vendor = PCI_VENDOR_ID_ATI && pdev->device == 0x4380) {
struct ahci_host_priv *priv = qc->ap->host->private_data;
if (priv->rev < 14) {
u32 rq_len = qc->scsicmd->request_bufflen;
u32 low_8k = rq_len & 0x1fff;
if ((rq_len & 0xffffe000) && low_8k && low_8k < 512)
return 1;
}
}
return 0;
}
On Fri, 2006-11-03 at 14:00 -0500, Luugi Marsan wrote:
> From: [email protected]
>
> There was an ASIC bug in the SB600 SATA controller of low revision (<=13) and CD burning may hang (only SATA ODD has this issue, and SATA HDD works well). The patch provides a workaround for this issue.
>
> Signed-off-by: Luugi Marsan <[email protected]>
>
> --- linux-2.6.19-rc4-git5/drivers/ata/ahci.c.orig 2006-11-04 03:56:22.000000000 +0800
> +++ linux-2.6.19-rc4-git5/drivers/ata/ahci.c 2006-11-04 04:20:36.000000000 +0800
> @@ -189,6 +189,7 @@ struct ahci_host_priv {
> unsigned long flags;
> u32 cap; /* cache of HOST_CAP register */
> u32 port_map; /* cache of HOST_PORTS_IMPL reg */
> + u8 rev; /* PCI Revision ID */
> };
>
why put this into the ahci struct rather than in the pci device struct?
In the place you use it you already have the pci device struct already..
and it's really a pci device property so putting it in that struct makes
a whole lot of sense conceptually anyway...
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
On Sat, Nov 04, 2006 at 06:36:49PM +0100, Arjan van de Ven wrote:
> why put this into the ahci struct rather than in the pci device struct?
> In the place you use it you already have the pci device struct already..
> and it's really a pci device property so putting it in that struct makes
> a whole lot of sense conceptually anyway...
I think having a revision field in pci_dev that's initialized by the pci
code early on would be immensivly useful. For now it probably makes sense
to do in ahci what all other drivers do and read it on it's own.
Adding it to the pci core and switching all the drivers over would
be a separate project. Maybe we can sign AMD up for it as a general
kernel contribution thing? :)
On Sat, 2006-11-04 at 18:36 +0100, Arjan van de Ven wrote:
> On Fri, 2006-11-03 at 14:00 -0500, Luugi Marsan wrote:
> > From: [email protected]
> >
> > There was an ASIC bug in the SB600 SATA controller of low revision (<=13) and CD burning may hang (only SATA ODD has this issue, and SATA HDD works well). The patch provides a workaround for this issue.
> >
> > Signed-off-by: Luugi Marsan <[email protected]>
> >
> > --- linux-2.6.19-rc4-git5/drivers/ata/ahci.c.orig 2006-11-04 03:56:22.000000000 +0800
> > +++ linux-2.6.19-rc4-git5/drivers/ata/ahci.c 2006-11-04 04:20:36.000000000 +0800
> > @@ -189,6 +189,7 @@ struct ahci_host_priv {
> > unsigned long flags;
> > u32 cap; /* cache of HOST_CAP register */
> > u32 port_map; /* cache of HOST_PORTS_IMPL reg */
> > + u8 rev; /* PCI Revision ID */
> > };
> >
>
> why put this into the ahci struct rather than in the pci device struct?
> In the place you use it you already have the pci device struct already..
> and it's really a pci device property so putting it in that struct makes
> a whole lot of sense conceptually anyway...
>
>
Hi Arjan,
Yes, I also thought it's more reasonable to add "u8 rev;" to pci_dev
than to ahci_host_priv, but that will effect source code in large scope,
so I added it here :(
Maybe I should resend a patch both for pci_dev and SB600 SATA at the
same time.
Best regards,
Conke Hu
Hello,
Luugi Marsan wrote:
> From: [email protected]
>
> There was an ASIC bug in the SB600 SATA controller of low revision (<=13) and CD burning may hang (only SATA ODD has this issue, and SATA HDD works well). The patch provides a workaround for this issue.
>
> Signed-off-by: Luugi Marsan <[email protected]>
As others have pointed out, code style seems a bit odd.
> --- linux-2.6.19-rc4-git5/drivers/ata/ahci.c.orig 2006-11-04 03:56:22.000000000 +0800
> +++ linux-2.6.19-rc4-git5/drivers/ata/ahci.c 2006-11-04 04:20:36.000000000 +0800
> @@ -189,6 +189,7 @@ struct ahci_host_priv {
> unsigned long flags;
> u32 cap; /* cache of HOST_CAP register */
> u32 port_map; /* cache of HOST_PORTS_IMPL reg */
> + u8 rev; /* PCI Revision ID */
> };
>
> struct ahci_port_priv {
> @@ -220,6 +221,7 @@ static int ahci_port_resume(struct ata_p
> static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
> static int ahci_pci_device_resume(struct pci_dev *pdev);
> static void ahci_remove_one (struct pci_dev *pdev);
> +static int ahci_check_atapi_dma(struct ata_queued_cmd *qc);
>
> static struct scsi_host_template ahci_sht = {
> .module = THIS_MODULE,
> @@ -251,6 +253,8 @@ static const struct ata_port_operations
>
> .tf_read = ahci_tf_read,
>
> + .check_atapi_dma = ahci_check_atapi_dma,
> +
> .qc_prep = ahci_qc_prep,
> .qc_issue = ahci_qc_issue,
>
Please make a separate port ops for broken controllers and use it only
for broken controllers. Say, ahci_old_sb600_ops?
> @@ -906,6 +910,28 @@ static unsigned int ahci_fill_sg(struct
> return n_sg;
> }
>
> +static int ahci_check_atapi_dma(struct ata_queued_cmd *qc)
> +{
> + struct pci_dev *pdev = to_pci_dev(qc->ap->host->dev);
> +
> + /* walkaround for SB600 SATA ODD isuue */
s/walkaround/workaround/
> + if (0x1002 == pdev->vendor && 0x4380 == pdev->device)
> + {
> + struct ahci_host_priv *priv = qc->ap->host->private_data;
> + u32 rq_len, low_8k;
> +
> + if ( 13 < priv->rev )
> + return 0;
> +
> + rq_len = qc->scsicmd->request_bufflen;
> + low_8k = rq_len & 0x1fff;
> +
> + if ( (rq_len & 0xffffe000) && low_8k && (512 > low_8k) )
> + return 1;
> + }
> + return 0;
> +}
And you won't need vendor/device/rev check...
> @@ -1366,6 +1392,7 @@ static int ahci_host_init(struct ata_pro
>
> hpriv->cap = readl(mmio + HOST_CAP);
> hpriv->port_map = readl(mmio + HOST_PORTS_IMPL);
> + pci_read_config_byte(pdev, PCI_REVISION_ID, &hpriv->rev);
> probe_ent->n_ports = (hpriv->cap & 0x1f) + 1;
>
> VPRINTK("cap 0x%x port_map 0x%x n_ports %d\n",
if you use ahci_old_sb600_ops only for controller which have the
problem. e.g. Do something like the following in ahci_host_init()
/* probe_ent initialization from port_info */
if (vendor, device and rev match)
probe_ent->port_ops = ahci_old_sb600_ops;
--
tejun
Luugi Marsan wrote:
> From: [email protected]
>
> There was an ASIC bug in the SB600 SATA controller of low revision (<=13) and CD burning may hang (only SATA ODD has this issue, and SATA HDD works well). The patch provides a workaround for this issue.
>
> Signed-off-by: Luugi Marsan <[email protected]>
ACK technical content, but it needs one revision: like someone else
(Alan or Tejun?) mentioned, this patch should be revised to use a
separate ata_port_operations for the affected chip(s).
Copy ahci_ops to sb600_ops, add your check-atapi-dma function without
the vendor/device ID check. Move the vendor/device check to the
pci_device_id table by creating a board_ahci_sb600 constant and
associated table data.
I agree with other posters that we should move the revision check to
more generic code, but such a move should not block this patch. The two
efforts can occur in parallel.
Jeff