linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] libata: Use IGN_SIMPLEX for ALi
  2009-05-13 13:59 [PATCH] libata: Use IGN_SIMPLEX for ALi Alan Cox
@ 2009-05-13 13:32 ` Bartlomiej Zolnierkiewicz
  2009-05-13 13:49   ` Alan Cox
  0 siblings, 1 reply; 4+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-05-13 13:32 UTC (permalink / raw)
  To: Alan Cox; +Cc: jeff, linux-ide

On Wednesday 13 May 2009 15:59:09 Alan Cox wrote:
> From: Alan Cox <alan@linux.intel.com>
> 
> Some ALi devices report simplex if they have been disabled and re-enabled, and
> restoring the byte does not work. Ignore it - the needed supporting logic is
> already present for the SATA ULi ports.
> 
> Signed-off-by: Alan Cox <alan@linux.intel.com>

ACK, the original driver has always ignored simplex

[ Sigh, this issue seems to go back to:

  commit 669a5db411d85a14f86cd92bc16bf7ab5b8aa235
  Author: Jeff Garzik <jeff@garzik.org>
  Date:   Tue Aug 29 18:12:40 2006 -0400

    [libata] Add a bunch of PATA drivers. ]

> ---
> 
>  drivers/ata/pata_ali.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c
> index 751b7ea..fc9c5d6 100644
> --- a/drivers/ata/pata_ali.c
> +++ b/drivers/ata/pata_ali.c
> @@ -497,14 +497,16 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	};
>  	/* Revision 0x20 added DMA */
>  	static const struct ata_port_info info_20 = {
> -		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
> +		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
> +							ATA_FLAG_IGN_SIMPLEX,
>  		.pio_mask = ATA_PIO4,
>  		.mwdma_mask = ATA_MWDMA2,
>  		.port_ops = &ali_20_port_ops
>  	};
>  	/* Revision 0x20 with support logic added UDMA */
>  	static const struct ata_port_info info_20_udma = {
> -		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
> +		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
> +							ATA_FLAG_IGN_SIMPLEX,
>  		.pio_mask = ATA_PIO4,
>  		.mwdma_mask = ATA_MWDMA2,
>  		.udma_mask = ATA_UDMA2,
> @@ -512,7 +514,8 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	};
>  	/* Revision 0xC2 adds UDMA66 */
>  	static const struct ata_port_info info_c2 = {
> -		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
> +		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
> +							ATA_FLAG_IGN_SIMPLEX,
>  		.pio_mask = ATA_PIO4,
>  		.mwdma_mask = ATA_MWDMA2,
>  		.udma_mask = ATA_UDMA4,
> @@ -520,7 +523,8 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	};
>  	/* Revision 0xC3 is UDMA66 for now */
>  	static const struct ata_port_info info_c3 = {
> -		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
> +		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
> +							ATA_FLAG_IGN_SIMPLEX,
>  		.pio_mask = ATA_PIO4,
>  		.mwdma_mask = ATA_MWDMA2,
>  		.udma_mask = ATA_UDMA4,
> @@ -528,7 +532,8 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	};
>  	/* Revision 0xC4 is UDMA100 */
>  	static const struct ata_port_info info_c4 = {
> -		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
> +		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
> +							ATA_FLAG_IGN_SIMPLEX,
>  		.pio_mask = ATA_PIO4,
>  		.mwdma_mask = ATA_MWDMA2,
>  		.udma_mask = ATA_UDMA5,
> @@ -536,7 +541,7 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	};
>  	/* Revision 0xC5 is UDMA133 with LBA48 DMA */
>  	static const struct ata_port_info info_c5 = {
> -		.flags = ATA_FLAG_SLAVE_POSS,
> +		.flags = ATA_FLAG_SLAVE_POSS | 	ATA_FLAG_IGN_SIMPLEX,
>  		.pio_mask = ATA_PIO4,
>  		.mwdma_mask = ATA_MWDMA2,
>  		.udma_mask = ATA_UDMA6,

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] libata: Use IGN_SIMPLEX for ALi
  2009-05-13 13:32 ` Bartlomiej Zolnierkiewicz
@ 2009-05-13 13:49   ` Alan Cox
  2009-05-13 14:15     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 4+ messages in thread
From: Alan Cox @ 2009-05-13 13:49 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: jeff, linux-ide

> [ Sigh, this issue seems to go back to:
> 
>   commit 669a5db411d85a14f86cd92bc16bf7ab5b8aa235
>   Author: Jeff Garzik <jeff@garzik.org>
>   Date:   Tue Aug 29 18:12:40 2006 -0400
> 
>     [libata] Add a bunch of PATA drivers. ]

No its quite recent actually - broke around 2.6.27

The libata drivers try and reset the simplex bits to check if a device is
merely in simplex mode by configuration. Unfortunately on the ALi if you
put the device into a power saving mode and bring it back the simplex
bits get corrupted and the register becomes unwritable (even to restore)

Somewhere around 2.6.27 something started rudely dumping the device into
power saving mode outside of the ATA layer on certain PCs.

So we switch to ignoring the bit in the first place on this chip.

Alan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] libata: Use IGN_SIMPLEX for ALi
@ 2009-05-13 13:59 Alan Cox
  2009-05-13 13:32 ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 4+ messages in thread
From: Alan Cox @ 2009-05-13 13:59 UTC (permalink / raw)
  To: jeff, linux-ide

From: Alan Cox <alan@linux.intel.com>

Some ALi devices report simplex if they have been disabled and re-enabled, and
restoring the byte does not work. Ignore it - the needed supporting logic is
already present for the SATA ULi ports.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/pata_ali.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)


diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c
index 751b7ea..fc9c5d6 100644
--- a/drivers/ata/pata_ali.c
+++ b/drivers/ata/pata_ali.c
@@ -497,14 +497,16 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	};
 	/* Revision 0x20 added DMA */
 	static const struct ata_port_info info_20 = {
-		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
+		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
+							ATA_FLAG_IGN_SIMPLEX,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.port_ops = &ali_20_port_ops
 	};
 	/* Revision 0x20 with support logic added UDMA */
 	static const struct ata_port_info info_20_udma = {
-		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
+		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
+							ATA_FLAG_IGN_SIMPLEX,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.udma_mask = ATA_UDMA2,
@@ -512,7 +514,8 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	};
 	/* Revision 0xC2 adds UDMA66 */
 	static const struct ata_port_info info_c2 = {
-		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
+		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
+							ATA_FLAG_IGN_SIMPLEX,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.udma_mask = ATA_UDMA4,
@@ -520,7 +523,8 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	};
 	/* Revision 0xC3 is UDMA66 for now */
 	static const struct ata_port_info info_c3 = {
-		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
+		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
+							ATA_FLAG_IGN_SIMPLEX,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.udma_mask = ATA_UDMA4,
@@ -528,7 +532,8 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	};
 	/* Revision 0xC4 is UDMA100 */
 	static const struct ata_port_info info_c4 = {
-		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48,
+		.flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_LBA48 |
+							ATA_FLAG_IGN_SIMPLEX,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.udma_mask = ATA_UDMA5,
@@ -536,7 +541,7 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	};
 	/* Revision 0xC5 is UDMA133 with LBA48 DMA */
 	static const struct ata_port_info info_c5 = {
-		.flags = ATA_FLAG_SLAVE_POSS,
+		.flags = ATA_FLAG_SLAVE_POSS | 	ATA_FLAG_IGN_SIMPLEX,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.udma_mask = ATA_UDMA6,


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] libata: Use IGN_SIMPLEX for ALi
  2009-05-13 13:49   ` Alan Cox
@ 2009-05-13 14:15     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 4+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-05-13 14:15 UTC (permalink / raw)
  To: Alan Cox; +Cc: jeff, linux-ide

On Wednesday 13 May 2009 15:49:04 Alan Cox wrote:
> > [ Sigh, this issue seems to go back to:
> > 
> >   commit 669a5db411d85a14f86cd92bc16bf7ab5b8aa235
> >   Author: Jeff Garzik <jeff@garzik.org>
> >   Date:   Tue Aug 29 18:12:40 2006 -0400
> > 
> >     [libata] Add a bunch of PATA drivers. ]
> 
> No its quite recent actually - broke around 2.6.27
> 
> The libata drivers try and reset the simplex bits to check if a device is
> merely in simplex mode by configuration. Unfortunately on the ALi if you
> put the device into a power saving mode and bring it back the simplex
> bits get corrupted and the register becomes unwritable (even to restore)
> 
> Somewhere around 2.6.27 something started rudely dumping the device into
> power saving mode outside of the ATA layer on certain PCs.
> 
> So we switch to ignoring the bit in the first place on this chip.

Ok, thanks for explaining it.

BTW you may consider removing ata_pci_bmdma_clear_simplex() call
from ali_init_chipset() now (before somebody decides to fix the code
to check the former's function return value).

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-05-13 14:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-13 13:59 [PATCH] libata: Use IGN_SIMPLEX for ALi Alan Cox
2009-05-13 13:32 ` Bartlomiej Zolnierkiewicz
2009-05-13 13:49   ` Alan Cox
2009-05-13 14:15     ` Bartlomiej Zolnierkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).