[PATCH 1/2] pata_hpt37x: add proper cable detection methods

[PATCH 1/2] pata_hpt37x: add proper cable detection methods

[Bartlomiej Zolnierkiewicz]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ata/pata_hpt37x.c |   86 +++++++++++++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 30 deletions(-)

Index: b/drivers/ata/pata_hpt37x.c
===================================================================
--- a/drivers/ata/pata_hpt37x.c
+++ b/drivers/ata/pata_hpt37x.c
@@ -303,6 +303,59 @@ static unsigned long hpt370a_filter(stru
 }
 
 /**
+ *	hpt37x_cable_detect	-	Detect the cable type
+ *	@ap: ATA port to detect on
+ *
+ *	Return the cable type attached to this port
+ */
+
+static int hpt37x_cable_detect(struct ata_port *ap)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u8 scr2, ata66;
+
+	pci_read_config_byte(pdev, 0x5B, &scr2);
+	pci_write_config_byte(pdev, 0x5B, scr2 & ~0x01);
+	/* Cable register now active */
+	pci_read_config_byte(pdev, 0x5A, &ata66);
+	/* Restore state */
+	pci_write_config_byte(pdev, 0x5B, scr2);
+
+	if (ata66 & (2 >> ap->port_no))
+		return ATA_CBL_PATA40;
+	else
+		return ATA_CBL_PATA80;
+}
+
+/**
+ *	hpt374_fn1_cable_detect	-	Detect the cable type
+ *	@ap: ATA port to detect on
+ *
+ *	Return the cable type attached to this port
+ */
+
+static int hpt374_fn1_cable_detect(struct ata_port *ap)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	unsigned int mcrbase = 0x50 + 4 * ap->port_no;
+	u16 mcr3;
+	u8 ata66;
+
+	/* Do the extra channel work */
+	pci_read_config_word(pdev, mcrbase + 2, &mcr3);
+	/* Set bit 15 of 0x52 to enable TCBLID as input */
+	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, mcrbase + 2, mcr3);
+
+	if (ata66 & (2 >> ap->port_no))
+		return ATA_CBL_PATA40;
+	else
+		return ATA_CBL_PATA80;
+}
+
+/**
  *	hpt37x_pre_reset	-	reset the hpt37x bus
  *	@link: ATA link to reset
  *	@deadline: deadline jiffies for the operation
@@ -312,7 +365,6 @@ static unsigned long hpt370a_filter(stru
 
 static int hpt37x_pre_reset(struct ata_link *link, unsigned long deadline)
 {
-	u8 scr2, ata66;
 	struct ata_port *ap = link->ap;
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
 	static const struct pci_bits hpt37x_enable_bits[] = {
@@ -322,18 +374,6 @@ static int hpt37x_pre_reset(struct ata_l
 	if (!pci_test_config_bits(pdev, &hpt37x_enable_bits[ap->port_no]))
 		return -ENOENT;
 
-	pci_read_config_byte(pdev, 0x5B, &scr2);
-	pci_write_config_byte(pdev, 0x5B, scr2 & ~0x01);
-	/* Cable register now active */
-	pci_read_config_byte(pdev, 0x5A, &ata66);
-	/* Restore state */
-	pci_write_config_byte(pdev, 0x5B, scr2);
-
-	if (ata66 & (2 >> ap->port_no))
-		ap->cbl = ATA_CBL_PATA40;
-	else
-		ap->cbl = ATA_CBL_PATA80;
-
 	/* Reset the state machine */
 	pci_write_config_byte(pdev, 0x50 + 4 * ap->port_no, 0x37);
 	udelay(100);
@@ -347,29 +387,12 @@ static int hpt374_fn1_pre_reset(struct a
 		{ 0x50, 1, 0x04, 0x04 },
 		{ 0x54, 1, 0x04, 0x04 }
 	};
-	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, mcrbase + 2, &mcr3);
-	/* Set bit 15 of 0x52 to enable TCBLID as input
-	 */
-	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, mcrbase + 2, mcr3);
-
-	if (ata66 & (2 >> ap->port_no))
-		ap->cbl = ATA_CBL_PATA40;
-	else
-		ap->cbl = ATA_CBL_PATA80;
-
 	/* Reset the state machine */
 	pci_write_config_byte(pdev, 0x50 + 4 * ap->port_no, 0x37);
 	udelay(100);
@@ -584,6 +607,7 @@ static struct ata_port_operations hpt370
 	.bmdma_stop	= hpt370_bmdma_stop,
 
 	.mode_filter	= hpt370_filter,
+	.cable_detect	= hpt37x_cable_detect,
 	.set_piomode	= hpt370_set_piomode,
 	.set_dmamode	= hpt370_set_dmamode,
 	.prereset	= hpt37x_pre_reset,
@@ -608,6 +632,7 @@ static struct ata_port_operations hpt372
 
 	.bmdma_stop	= hpt37x_bmdma_stop,
 
+	.cable_detect	= hpt37x_cable_detect,
 	.set_piomode	= hpt372_set_piomode,
 	.set_dmamode	= hpt372_set_dmamode,
 	.prereset	= hpt37x_pre_reset,
@@ -620,6 +645,7 @@ static struct ata_port_operations hpt372
 
 static struct ata_port_operations hpt374_fn1_port_ops = {
 	.inherits	= &hpt372_port_ops,
+	.cable_detect	= hpt374_fn1_cable_detect,
 	.prereset	= hpt374_fn1_pre_reset,
 };
 

Re: [PATCH 1/2] pata_hpt37x: add proper cable detection methods

[Alan Cox]

On Thu, 19 Nov 2009 19:10:44 +0100
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
>  drivers/ata/pata_hpt37x.c |   86 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 56 insertions(+), 30 deletions(-)
> 
> Index: b/drivers/ata/pata_hpt37x.c

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

I suspect this may also need the udelay(10) adding but even if it does
its cleaner this way.

Re: [PATCH 1/2] pata_hpt37x: add proper cable detection methods

[Jeff Garzik]

On 11/19/2009 01:10 PM, Bartlomiej Zolnierkiewicz wrote:
> Signed-off-by: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
> ---
>   drivers/ata/pata_hpt37x.c |   86 +++++++++++++++++++++++++++++-----------------
>   1 file changed, 56 insertions(+), 30 deletions(-)

applied 1-2

Re: [PATCH 1/2] pata_hpt37x: add proper cable detection methods

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
>  drivers/ata/pata_hpt37x.c |   86 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 56 insertions(+), 30 deletions(-)

> Index: b/drivers/ata/pata_hpt37x.c
> ===================================================================
> --- a/drivers/ata/pata_hpt37x.c
> +++ b/drivers/ata/pata_hpt37x.c
> @@ -303,6 +303,59 @@ static unsigned long hpt370a_filter(stru

    As this driver still carries a version info, don't we need to update it?

MBR, Sergei

Re: [PATCH 1/2] pata_hpt37x: add proper cable detection methods

[Bartlomiej Zolnierkiewicz]

On Monday 23 November 2009 08:42:23 pm Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> >  drivers/ata/pata_hpt37x.c |   86 +++++++++++++++++++++++++++++-----------------
> >  1 file changed, 56 insertions(+), 30 deletions(-)
> 
> > Index: b/drivers/ata/pata_hpt37x.c
> > ===================================================================
> > --- a/drivers/ata/pata_hpt37x.c
> > +++ b/drivers/ata/pata_hpt37x.c
> > @@ -303,6 +303,59 @@ static unsigned long hpt370a_filter(stru
> 
>     As this driver still carries a version info, don't we need to update it?

While doing the previously mentioned PATA rework I've looked at
the ratio of patches actually updating version numbers and not
doing so.  This lead me to the same conclusions as with version
numbers in IDE drivers previously..

However this is just my opinion so if you for some reason would
like to bump version numbers of some drivers I won't complain..

--
Bartlomiej Zolnierkiewicz

Re: [PATCH 1/2] pata_hpt37x: add proper cable detection methods

[Alan Cox]

On Mon, 23 Nov 2009 22:42:23 +0300
Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:

> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> >  drivers/ata/pata_hpt37x.c |   86 +++++++++++++++++++++++++++++-----------------
> >  1 file changed, 56 insertions(+), 30 deletions(-)
> 
> > Index: b/drivers/ata/pata_hpt37x.c
> > ===================================================================
> > --- a/drivers/ata/pata_hpt37x.c
> > +++ b/drivers/ata/pata_hpt37x.c
> > @@ -303,6 +303,59 @@ static unsigned long hpt370a_filter(stru
> 
>     As this driver still carries a version info, don't we need to update it?

I don't think the version info is really useful any more - it was when
they were being developed but it has become a bit meaningless