[PATCH] sata_uli: ignore SIMPLEX

[PATCH] sata_uli: ignore SIMPLEX

[Tejun Heo]

Some uli controllers have stuck SIMPLEX bit which can't be cleared
with ata_pci_clear_simplex(), but the controller is capable of doing
DMAs on both channels simultaneously.  Ignore it.

Signed-off-by: Tejun Heo <htejun@gmail.com>

diff --git a/drivers/ata/sata_uli.c b/drivers/ata/sata_uli.c
index 5c603ca..62b9269 100644
--- a/drivers/ata/sata_uli.c
+++ b/drivers/ata/sata_uli.c
@@ -226,6 +226,13 @@ static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	probe_ent->private_data = hpriv;
 
+	/* these chips have stuck dummy simplex bit, ignore it */
+	if (probe_ent->_host_flags & ATA_HOST_SIMPLEX) {
+		dev_printk(KERN_INFO, &pdev->dev,
+			   "BMDMA simplex set, ignored\n");
+		probe_ent->_host_flags &= ~ATA_HOST_SIMPLEX;
+	}
+
 	switch (board_idx) {
 	case uli_5287:
 		hpriv->scr_cfg_addr[0] = ULI5287_BASE;

Re: [PATCH] sata_uli: ignore SIMPLEX

[Alan]

On Tue, 16 Jan 2007 14:55:51 +0900
Tejun Heo <htejun@gmail.com> wrote:

> Some uli controllers have stuck SIMPLEX bit which can't be cleared
> with ata_pci_clear_simplex(), but the controller is capable of doing
> DMAs on both channels simultaneously.  Ignore it.

Acked-by: Alan Cox <alan@redhat.com>

Re: [PATCH] sata_uli: ignore SIMPLEX

[Jeff Garzik]

Tejun Heo wrote:
> Some uli controllers have stuck SIMPLEX bit which can't be cleared
> with ata_pci_clear_simplex(), but the controller is capable of doing
> DMAs on both channels simultaneously.  Ignore it.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> diff --git a/drivers/ata/sata_uli.c b/drivers/ata/sata_uli.c
> index 5c603ca..62b9269 100644
> --- a/drivers/ata/sata_uli.c
> +++ b/drivers/ata/sata_uli.c
> @@ -226,6 +226,13 @@ static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	probe_ent->private_data = hpriv;
>  
> +	/* these chips have stuck dummy simplex bit, ignore it */
> +	if (probe_ent->_host_flags & ATA_HOST_SIMPLEX) {
> +		dev_printk(KERN_INFO, &pdev->dev,
> +			   "BMDMA simplex set, ignored\n");
> +		probe_ent->_host_flags &= ~ATA_HOST_SIMPLEX;
> +	}

This is going to be the common case going forward.  Please create an 
'ignore simplex bit' flag, and set it in sata_uli

	Jeff

Re: [PATCH] sata_uli: ignore SIMPLEX

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Some uli controllers have stuck SIMPLEX bit which can't be cleared
>> with ata_pci_clear_simplex(), but the controller is capable of doing
>> DMAs on both channels simultaneously.  Ignore it.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> This is going to be the common case going forward.  Please create an
> 'ignore simplex bit' flag, and set it in sata_uli

I'm a little bit hesitant to do that.

1. I don't think we're gonna have a lot of drivers which need this.
Just a few.  We've just found one now.

2. I think libata core layer (sff included) is burdened with too much
already.  Also, now that devres is acked, we can easily transition to
alloc - init - register model which gives LLDs much more flexibility and
this kind of stuff can easily be done in init_one()'s.  It just fits there.

But, it's your call.

Thanks.

-- 
tejun

Re: [PATCH] sata_uli: ignore SIMPLEX

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> Some uli controllers have stuck SIMPLEX bit which can't be cleared
>>> with ata_pci_clear_simplex(), but the controller is capable of doing
>>> DMAs on both channels simultaneously.  Ignore it.
>>>
>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>> This is going to be the common case going forward.  Please create an
>> 'ignore simplex bit' flag, and set it in sata_uli
> 
> I'm a little bit hesitant to do that.
> 
> 1. I don't think we're gonna have a lot of drivers which need this.
> Just a few.  We've just found one now.
> 
> 2. I think libata core layer (sff included) is burdened with too much
> already.  Also, now that devres is acked, we can easily transition to
> alloc - init - register model which gives LLDs much more flexibility and
> this kind of stuff can easily be done in init_one()'s.  It just fits there.

We need to correct a false assumption in the core.

The PCI IDE spec specifies the simplex bit, but a /vast majority/ of 
controllers, PATA as well as SATA, hardwire the bit, or program it to a 
value set by BIOS.  As such, most drivers really should /just know/ that 
their hardware need never worry about this odd simplex concept.

	Jeff

Re: [PATCH] sata_uli: ignore SIMPLEX

[Alan]

> The PCI IDE spec specifies the simplex bit, but a /vast majority/ of 
> controllers, PATA as well as SATA, hardwire the bit, or program it to a 
> value set by BIOS.  As such, most drivers really should /just know/ that 
> their hardware need never worry about this odd simplex concept.

The ULi is the first one I've come across that actually messes this up.
It's so clear in the spec from day one that even the "usual suspects" got
it right.

[PATCH] libata: implement ATA_FLAG_IGN_SIMPLEX and use it in sata_uli

[Tejun Heo]

Some uli controllers have stuck SIMPLEX bit which can't be cleared
with ata_pci_clear_simplex(), but the controller is capable of doing
DMAs on both channels simultaneously.  Implement ATA_FLAG_IGN_SIMPLEX
which makes libata ignore the simplex bit and use it in sata_uli.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Jeff, implemented as ATA_FLAG_* as requested.  The patch is against
#upstream-fixes and should probably included in 2.6.20.  Thanks.

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 623cec9..71e6ddd 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -870,7 +870,8 @@ ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int
 			pci_resource_start(pdev, 1) | ATA_PCI_CTL_OFS;
 		bmdma = pci_resource_start(pdev, 4);
 		if (bmdma) {
-			if (inb(bmdma + 2) & 0x80)
+			if (!(port[p]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+			    inb(bmdma + 2) & 0x80)
 				probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
 			probe_ent->port[p].bmdma_addr = bmdma;
 		}
@@ -886,7 +887,8 @@ ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int
 		bmdma = pci_resource_start(pdev, 4);
 		if (bmdma) {
 			bmdma += 8;
-			if(inb(bmdma + 2) & 0x80)
+			if (!(port[p]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+			    inb(bmdma + 2) & 0x80)
 				probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
 			probe_ent->port[p].bmdma_addr = bmdma;
 		}
@@ -920,7 +922,8 @@ static struct ata_probe_ent *ata_pci_init_legacy_port(struct pci_dev *pdev,
 		probe_ent->port[0].ctl_addr = ATA_PRIMARY_CTL;
 		if (bmdma) {
 			probe_ent->port[0].bmdma_addr = bmdma;
-			if (inb(bmdma + 2) & 0x80)
+			if (!(port[0]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+			    inb(bmdma + 2) & 0x80)
 				probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
 		}
 		ata_std_ports(&probe_ent->port[0]);
@@ -937,7 +940,8 @@ static struct ata_probe_ent *ata_pci_init_legacy_port(struct pci_dev *pdev,
 		probe_ent->port[1].ctl_addr = ATA_SECONDARY_CTL;
 		if (bmdma) {
 			probe_ent->port[1].bmdma_addr = bmdma + 8;
-			if (inb(bmdma + 10) & 0x80)
+			if (!(port[1]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+			    inb(bmdma + 10) & 0x80)
 				probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
 		}
 		ata_std_ports(&probe_ent->port[1]);
diff --git a/drivers/ata/sata_uli.c b/drivers/ata/sata_uli.c
index 5c603ca..a43aec6 100644
--- a/drivers/ata/sata_uli.c
+++ b/drivers/ata/sata_uli.c
@@ -128,7 +128,8 @@ static const struct ata_port_operations uli_ops = {
 
 static struct ata_port_info uli_port_info = {
 	.sht            = &uli_sht,
-	.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+	.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+			  ATA_FLAG_IGN_SIMPLEX,
 	.pio_mask       = 0x1f,		/* pio0-4 */
 	.udma_mask      = 0x7f,		/* udma0-6 */
 	.port_ops       = &uli_ops,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index df76fc4..22aa69e 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -177,6 +177,7 @@ enum {
 					      * Register FIS clearing BSY */
 	ATA_FLAG_DEBUGMSG	= (1 << 13),
 	ATA_FLAG_SETXFER_POLLING= (1 << 14), /* use polling for SETXFER */
+	ATA_FLAG_IGN_SIMPLEX	= (1 << 15), /* ignore SIMPLEX */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be

Re: [PATCH] libata: implement ATA_FLAG_IGN_SIMPLEX and use it in sata_uli

[Jeff Garzik]

Tejun Heo wrote:
> Some uli controllers have stuck SIMPLEX bit which can't be cleared
> with ata_pci_clear_simplex(), but the controller is capable of doing
> DMAs on both channels simultaneously.  Implement ATA_FLAG_IGN_SIMPLEX
> which makes libata ignore the simplex bit and use it in sata_uli.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> Jeff, implemented as ATA_FLAG_* as requested.  The patch is against
> #upstream-fixes and should probably included in 2.6.20.  Thanks.
> 
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 623cec9..71e6ddd 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -870,7 +870,8 @@ ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int
>  			pci_resource_start(pdev, 1) | ATA_PCI_CTL_OFS;
>  		bmdma = pci_resource_start(pdev, 4);
>  		if (bmdma) {
> -			if (inb(bmdma + 2) & 0x80)
> +			if (!(port[p]->flags & ATA_FLAG_IGN_SIMPLEX) &&
> +			    inb(bmdma + 2) & 0x80)
>  				probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
>  			probe_ent->port[p].bmdma_addr = bmdma;
>  		}
> @@ -886,7 +887,8 @@ ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int
>  		bmdma = pci_resource_start(pdev, 4);
>  		if (bmdma) {
>  			bmdma += 8;
> -			if(inb(bmdma + 2) & 0x80)
> +			if (!(port[p]->flags & ATA_FLAG_IGN_SIMPLEX) &&
> +			    inb(bmdma + 2) & 0x80)
>  				probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
>  			probe_ent->port[p].bmdma_addr = bmdma;
>  		}

applied to #upstream-fixes, after adding some additional parens for 
readability