All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
@ 2007-11-05 22:53 Alan Cox
  2007-11-06  2:42 ` Andrew Morton
  2007-11-06 14:45 ` Sergei Shtylyov
  0 siblings, 2 replies; 5+ messages in thread
From: Alan Cox @ 2007-11-05 22:53 UTC (permalink / raw)
  To: akpm, jeff, linux-ide

- Read frequency correctly
- Correct cable detect handling
- Fix wrong filter test

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

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.24-rc1/drivers/ata/pata_hpt37x.c linux-2.6.24-rc1/drivers/ata/pata_hpt37x.c
--- linux.vanilla-2.6.24-rc1/drivers/ata/pata_hpt37x.c	2007-11-01 11:41:54.000000000 +0000
+++ linux-2.6.24-rc1/drivers/ata/pata_hpt37x.c	2007-11-05 22:21:31.000000000 +0000
@@ -295,7 +295,7 @@
 
 static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask)
 {
-	if (adev->class != ATA_DEV_ATA) {
+	if (adev->class == ATA_DEV_ATA) {
 		if (hpt_dma_blacklisted(adev, "UDMA100", bad_ata100_5))
 			mask &= ~ (0x1F << ATA_SHIFT_UDMA);
 	}
@@ -359,28 +359,25 @@
 		{ 0x50, 1, 0x04, 0x04 },
 		{ 0x54, 1, 0x04, 0x04 }
 	};
-	u16 mcr3, mcr6;
+	u16 mcr3;
 	u8 ata66;
 	struct ata_port *ap = link->ap;
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	unsigned int mcrbase = 0x50 + 4 * ap->port_no;
 
 	if (!pci_test_config_bits(pdev, &hpt37x_enable_bits[ap->port_no]))
 		return -ENOENT;
 
 	/* Do the extra channel work */
-	pci_read_config_word(pdev, 0x52, &mcr3);
-	pci_read_config_word(pdev, 0x56, &mcr6);
+	pci_read_config_word(pdev, mcrbase + 2, &mcr3);
 	/* Set bit 15 of 0x52 to enable TCBLID as input
-	   Set bit 15 of 0x56 to enable FCBLID as input
 	 */
-	pci_write_config_word(pdev, 0x52, mcr3 | 0x8000);
-	pci_write_config_word(pdev, 0x56, mcr6 | 0x8000);
+	pci_write_config_word(pdev, mcrbase + 2, mcr3 | 0x8000);
 	pci_read_config_byte(pdev, 0x5A, &ata66);
 	/* Reset TCBLID/FCBLID to output */
 	pci_write_config_word(pdev, 0x52, mcr3);
-	pci_write_config_word(pdev, 0x56, mcr6);
 
-	if (ata66 & (1 << ap->port_no))
+	if (ata66 & (2 >> ap->port_no))
 		ap->cbl = ATA_CBL_PATA40;
 	else
 		ap->cbl = ATA_CBL_PATA80;
@@ -844,6 +841,25 @@
 	/* Never went stable */
 	return 0;
 }
+
+static u32 hpt374_read_freq(struct pci_dev *pdev)
+{
+	u32 freq;
+	unsigned long io_base = pci_resource_start(pdev, 4);
+	if (PCI_FUNC(pdev->devfn) & 1) {
+		struct pci_dev *pdev_0 = pci_get_slot(pdev->bus, pdev->devfn - 1);
+		/* Someone hot plugged the controller on us ? */
+		if (pdev_0 == NULL)
+			return 0;
+		io_base = pci_resource_start(pdev_0, 4);
+		freq = inl(io_base + 0x90);
+		pci_dev_put(pdev_0);
+	}
+	else
+		freq = inl(io_base + 0x90);
+	return freq;
+}
+
 /**
  *	hpt37x_init_one		-	Initialise an HPT37X/302
  *	@dev: PCI device
@@ -902,7 +918,7 @@
 		.flags = ATA_FLAG_SLAVE_POSS,
 		.pio_mask = 0x1f,
 		.mwdma_mask = 0x07,
-		.udma_mask = 0x0f,
+		.udma_mask = ATA_UDMA5,
 		.port_ops = &hpt370_port_ops
 	};
 	/* HPT370A - UDMA100 */
@@ -911,7 +927,7 @@
 		.flags = ATA_FLAG_SLAVE_POSS,
 		.pio_mask = 0x1f,
 		.mwdma_mask = 0x07,
-		.udma_mask = 0x0f,
+		.udma_mask = ATA_UDMA5,
 		.port_ops = &hpt370a_port_ops
 	};
 	/* HPT371, 372 and friends - UDMA133 */
@@ -1047,9 +1063,16 @@
 		outb(0x0e, iobase + 0x9c);
 
 	/* Some devices do not let this value be accessed via PCI space
-	   according to the old driver */
+	   according to the old driver. In addition we must use the value
+	   from FN 0 on the HPT374 */
+
+	if (chip_table == &hpt374) {
+		freq = hpt374_read_freq(dev);
+		if (freq == 0)
+			return -ENODEV;
+	} else
+		freq = inl(iobase + 0x90);
 
-	freq = inl(iobase + 0x90);
 	if ((freq >> 12) != 0xABCDE) {
 		int i;
 		u8 sr;

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

* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
  2007-11-05 22:53 [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Alan Cox
@ 2007-11-06  2:42 ` Andrew Morton
  2007-11-06 10:40   ` Alan Cox
  2007-11-06 14:45 ` Sergei Shtylyov
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2007-11-06  2:42 UTC (permalink / raw)
  To: Alan Cox; +Cc: jeff, linux-ide

On Mon, 5 Nov 2007 22:53:38 +0000 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> +static u32 hpt374_read_freq(struct pci_dev *pdev)
> +{
> +	u32 freq;
> +	unsigned long io_base = pci_resource_start(pdev, 4);
> +	if (PCI_FUNC(pdev->devfn) & 1) {
> +		struct pci_dev *pdev_0 = pci_get_slot(pdev->bus, pdev->devfn - 1);
> +		/* Someone hot plugged the controller on us ? */
> +		if (pdev_0 == NULL)
> +			return 0;
> +		io_base = pci_resource_start(pdev_0, 4);
> +		freq = inl(io_base + 0x90);
> +		pci_dev_put(pdev_0);
> +	}
> +	else
> +		freq = inl(io_base + 0x90);
> +	return freq;
> +}

hm, pci_resource_start() returns a resource_size_t and I guess this (and a
heck of a lot of other) code is bust on (whatever machine we added that for).

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

* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
  2007-11-06  2:42 ` Andrew Morton
@ 2007-11-06 10:40   ` Alan Cox
  0 siblings, 0 replies; 5+ messages in thread
From: Alan Cox @ 2007-11-06 10:40 UTC (permalink / raw)
  To: Andrew Morton; +Cc: jeff, linux-ide

> hm, pci_resource_start() returns a resource_size_t and I guess this (and a
> heck of a lot of other) code is bust on (whatever machine we added that for).

inl takes an unsigned long.

pci_resource_start historically was 32bit and became 64bit so we should
have no problems.

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

* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
  2007-11-05 22:53 [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Alan Cox
  2007-11-06  2:42 ` Andrew Morton
@ 2007-11-06 14:45 ` Sergei Shtylyov
  2007-11-06 14:59   ` Alan Cox
  1 sibling, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2007-11-06 14:45 UTC (permalink / raw)
  To: Alan Cox; +Cc: akpm, jeff, linux-ide

Alan Cox wrote:

> - Read frequency correctly
> - Correct cable detect handling

    I'm still not sure it's correct...

> - Fix wrong filter test

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

> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.24-rc1/drivers/ata/pata_hpt37x.c linux-2.6.24-rc1/drivers/ata/pata_hpt37x.c
> --- linux.vanilla-2.6.24-rc1/drivers/ata/pata_hpt37x.c	2007-11-01 11:41:54.000000000 +0000
> +++ linux-2.6.24-rc1/drivers/ata/pata_hpt37x.c	2007-11-05 22:21:31.000000000 +0000
> @@ -295,7 +295,7 @@
>  
>  static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask)
>  {
> -	if (adev->class != ATA_DEV_ATA) {
> +	if (adev->class == ATA_DEV_ATA) {
>  		if (hpt_dma_blacklisted(adev, "UDMA100", bad_ata100_5))
>  			mask &= ~ (0x1F << ATA_SHIFT_UDMA);
>  	}
> @@ -359,28 +359,25 @@
>  		{ 0x50, 1, 0x04, 0x04 },
>  		{ 0x54, 1, 0x04, 0x04 }
>  	};
> -	u16 mcr3, mcr6;
> +	u16 mcr3;
>  	u8 ata66;
>  	struct ata_port *ap = link->ap;
>  	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> +	unsigned int mcrbase = 0x50 + 4 * ap->port_no;
>  
>  	if (!pci_test_config_bits(pdev, &hpt37x_enable_bits[ap->port_no]))
>  		return -ENOENT;
>  
>  	/* Do the extra channel work */
> -	pci_read_config_word(pdev, 0x52, &mcr3);
> -	pci_read_config_word(pdev, 0x56, &mcr6);
> +	pci_read_config_word(pdev, mcrbase + 2, &mcr3);
>  	/* Set bit 15 of 0x52 to enable TCBLID as input
> -	   Set bit 15 of 0x56 to enable FCBLID as input
>  	 */
> -	pci_write_config_word(pdev, 0x52, mcr3 | 0x8000);
> -	pci_write_config_word(pdev, 0x56, mcr6 | 0x8000);
> +	pci_write_config_word(pdev, mcrbase + 2, mcr3 | 0x8000);
>  	pci_read_config_byte(pdev, 0x5A, &ata66);
>  	/* Reset TCBLID/FCBLID to output */
>  	pci_write_config_word(pdev, 0x52, mcr3);
> -	pci_write_config_word(pdev, 0x56, mcr6);
>  
> -	if (ata66 & (1 << ap->port_no))
> +	if (ata66 & (2 >> ap->port_no))

    But has the same issue with hpt37x_pre_reset() been corrected?  I don't 
see this in the current driver.

>  		ap->cbl = ATA_CBL_PATA40;
>  	else
>  		ap->cbl = ATA_CBL_PATA80;
> @@ -844,6 +841,25 @@
>  	/* Never went stable */
>  	return 0;
>  }
> +
> +static u32 hpt374_read_freq(struct pci_dev *pdev)
> +{
> +	u32 freq;
> +	unsigned long io_base = pci_resource_start(pdev, 4);

    No new line after declaration block...

> +	if (PCI_FUNC(pdev->devfn) & 1) {
> +		struct pci_dev *pdev_0 = pci_get_slot(pdev->bus, pdev->devfn - 1);
> +		/* Someone hot plugged the controller on us ? */
> +		if (pdev_0 == NULL)
> +			return 0;
> +		io_base = pci_resource_start(pdev_0, 4);
> +		freq = inl(io_base + 0x90);
> +		pci_dev_put(pdev_0);
> +	}
> +	else
> +		freq = inl(io_base + 0x90);
> +	return freq;
> +}
> +
>  /**
>   *	hpt37x_init_one		-	Initialise an HPT37X/302
>   *	@dev: PCI device
> @@ -902,7 +918,7 @@
>  		.flags = ATA_FLAG_SLAVE_POSS,
>  		.pio_mask = 0x1f,
>  		.mwdma_mask = 0x07,
> -		.udma_mask = 0x0f,
> +		.udma_mask = ATA_UDMA5,
>  		.port_ops = &hpt370_port_ops
>  	};
>  	/* HPT370A - UDMA100 */
> @@ -911,7 +927,7 @@
>  		.flags = ATA_FLAG_SLAVE_POSS,
>  		.pio_mask = 0x1f,
>  		.mwdma_mask = 0x07,
> -		.udma_mask = 0x0f,
> +		.udma_mask = ATA_UDMA5,
>  		.port_ops = &hpt370a_port_ops
>  	};
>  	/* HPT371, 372 and friends - UDMA133 */
> @@ -1047,9 +1063,16 @@
>  		outb(0x0e, iobase + 0x9c);
>  
>  	/* Some devices do not let this value be accessed via PCI space
> -	   according to the old driver */
> +	   according to the old driver. In addition we must use the value
> +	   from FN 0 on the HPT374 */
> +
> +	if (chip_table == &hpt374) {
> +		freq = hpt374_read_freq(dev);
> +		if (freq == 0)
> +			return -ENODEV;

    Not sure whether this check is necessary...

> +	} else
> +		freq = inl(iobase + 0x90);
>  
> -	freq = inl(iobase + 0x90);
>  	if ((freq >> 12) != 0xABCDE) {
>  		int i;
>  		u8 sr;

MBR, Sergei

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

* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
  2007-11-06 14:45 ` Sergei Shtylyov
@ 2007-11-06 14:59   ` Alan Cox
  0 siblings, 0 replies; 5+ messages in thread
From: Alan Cox @ 2007-11-06 14:59 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: akpm, jeff, linux-ide

> > +	if (chip_table == &hpt374) {
> > +		freq = hpt374_read_freq(dev);
> > +		if (freq == 0)
> > +			return -ENODEV;
> 
>     Not sure whether this check is necessary...

If the device is hot unplugged we can get a zero return error case. It
seems cleaner to catch the error case rather than attempt to prove it
does no harm later.

pre_reset one fixed in my tree and will check and push.

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

end of thread, other threads:[~2007-11-06 14:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-05 22:53 [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Alan Cox
2007-11-06  2:42 ` Andrew Morton
2007-11-06 10:40   ` Alan Cox
2007-11-06 14:45 ` Sergei Shtylyov
2007-11-06 14:59   ` Alan Cox

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.