Re: [PATCH] sata_promise: fix missing PATA cable detection

Re: [PATCH] sata_promise: fix missing PATA cable detection

[Mikael Pettersson]

On Thu, 08 Feb 2007 08:50:09 -0500, Jeff Garzik wrote:
> Mikael Pettersson wrote:
> > This patch fixes an oversight which caused sata_promise to
> > not perform cable detection on the TX2plus chips' PATA ports.
> > 
> > Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> > 
> > ---
> > 
> > This patch adds yet another is-PATA-or-SATA? check, but it's in a
> > cold path so shouldn't matter much. This will be cleaned up if/when
> > PATA ports and SATA ports start using different ops structures.
> > 
> > --- linux-2.6.20/drivers/ata/sata_promise.c.~1~	2007-02-07 20:30:20.000000000 +0100
> > +++ linux-2.6.20/drivers/ata/sata_promise.c	2007-02-07 20:57:34.000000000 +0100
> > @@ -601,6 +601,13 @@ static void pdc_thaw(struct ata_port *ap
> >  	readl(mmio + PDC_CTLSTAT); /* flush */
> >  }
> >  
> > +static int pdc_pre_reset(struct ata_port *ap)
> > +{
> > +	if (!sata_scr_valid(ap))
> > +		pdc_pata_cbl_detect(ap);
> > +	return ata_std_prereset(ap);
> > +}
> > +
> 
> Since it was a bug in the original patch, I just dropped it, waiting for 
> you to resend the whole thing.  That keeps the git history much more clean.

I hope you dropped the 20619 new EH patch but applied this one.

Maybe the patch description was too terse given the broken 20619 patch:
This patch corrects TX2plus PATA in current #upstream. It's currently
unrelated to 20619 (since 20619 has its own ops and a ->phy_reset which
calls pdc_pata_cbl_detect()), but later when 20619 is converted to new EH,
it ensures that 20619 continues to do PATA cable detection.

The bug originated in the #promise-sata-pata branch, which added
TX2plus PATA support but didn't hook in any cable detection.
I failed to observe that omission when reworking #promise-sata-pata
into the form that got merged.

/Mikael

Re: [PATCH] sata_promise: fix missing PATA cable detection

[Jeff Garzik]

Mikael Pettersson wrote:
> On Thu, 08 Feb 2007 08:50:09 -0500, Jeff Garzik wrote:
>> Mikael Pettersson wrote:
>>> This patch fixes an oversight which caused sata_promise to
>>> not perform cable detection on the TX2plus chips' PATA ports.
>>>
>>> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
>>>
>>> ---
>>>
>>> This patch adds yet another is-PATA-or-SATA? check, but it's in a
>>> cold path so shouldn't matter much. This will be cleaned up if/when
>>> PATA ports and SATA ports start using different ops structures.
>>>
>>> --- linux-2.6.20/drivers/ata/sata_promise.c.~1~	2007-02-07 20:30:20.000000000 +0100
>>> +++ linux-2.6.20/drivers/ata/sata_promise.c	2007-02-07 20:57:34.000000000 +0100
>>> @@ -601,6 +601,13 @@ static void pdc_thaw(struct ata_port *ap
>>>  	readl(mmio + PDC_CTLSTAT); /* flush */
>>>  }
>>>  
>>> +static int pdc_pre_reset(struct ata_port *ap)
>>> +{
>>> +	if (!sata_scr_valid(ap))
>>> +		pdc_pata_cbl_detect(ap);
>>> +	return ata_std_prereset(ap);
>>> +}
>>> +
>> Since it was a bug in the original patch, I just dropped it, waiting for 
>> you to resend the whole thing.  That keeps the git history much more clean.
> 
> I hope you dropped the 20619 new EH patch but applied this one.
> 
> Maybe the patch description was too terse given the broken 20619 patch:
> This patch corrects TX2plus PATA in current #upstream. It's currently
> unrelated to 20619 (since 20619 has its own ops and a ->phy_reset which
> calls pdc_pata_cbl_detect()), but later when 20619 is converted to new EH,
> it ensures that 20619 continues to do PATA cable detection.
> 
> The bug originated in the #promise-sata-pata branch, which added
> TX2plus PATA support but didn't hook in any cable detection.
> I failed to observe that omission when reworking #promise-sata-pata
> into the form that got merged.

Ah, I misunderstood.  I thought it was an incremental patch to your 
new-EH patch, at first glance.

It is now in my queue.

	Jeff

[PATCH] sata_promise: fix missing PATA cable detection

[Mikael Pettersson]

This patch fixes an oversight which caused sata_promise to
not perform cable detection on the TX2plus chips' PATA ports.

Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>

---

This patch adds yet another is-PATA-or-SATA? check, but it's in a
cold path so shouldn't matter much. This will be cleaned up if/when
PATA ports and SATA ports start using different ops structures.

--- linux-2.6.20/drivers/ata/sata_promise.c.~1~	2007-02-07 20:30:20.000000000 +0100
+++ linux-2.6.20/drivers/ata/sata_promise.c	2007-02-07 20:57:34.000000000 +0100
@@ -601,6 +601,13 @@ static void pdc_thaw(struct ata_port *ap
 	readl(mmio + PDC_CTLSTAT); /* flush */
 }
 
+static int pdc_pre_reset(struct ata_port *ap)
+{
+	if (!sata_scr_valid(ap))
+		pdc_pata_cbl_detect(ap);
+	return ata_std_prereset(ap);
+}
+
 static void pdc_error_handler(struct ata_port *ap)
 {
 	ata_reset_fn_t hardreset;
@@ -613,7 +620,7 @@ static void pdc_error_handler(struct ata
 		hardreset = sata_std_hardreset;
 
 	/* perform recovery */
-	ata_do_eh(ap, ata_std_prereset, ata_std_softreset, hardreset,
+	ata_do_eh(ap, pdc_pre_reset, ata_std_softreset, hardreset,
 		  ata_std_postreset);
 }
 

Re: [PATCH] sata_promise: fix missing PATA cable detection

[Jeff Garzik]

Mikael Pettersson wrote:
> This patch fixes an oversight which caused sata_promise to
> not perform cable detection on the TX2plus chips' PATA ports.
> 
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> 
> ---
> 
> This patch adds yet another is-PATA-or-SATA? check, but it's in a
> cold path so shouldn't matter much. This will be cleaned up if/when
> PATA ports and SATA ports start using different ops structures.
> 
> --- linux-2.6.20/drivers/ata/sata_promise.c.~1~	2007-02-07 20:30:20.000000000 +0100
> +++ linux-2.6.20/drivers/ata/sata_promise.c	2007-02-07 20:57:34.000000000 +0100
> @@ -601,6 +601,13 @@ static void pdc_thaw(struct ata_port *ap
>  	readl(mmio + PDC_CTLSTAT); /* flush */
>  }
>  
> +static int pdc_pre_reset(struct ata_port *ap)
> +{
> +	if (!sata_scr_valid(ap))
> +		pdc_pata_cbl_detect(ap);
> +	return ata_std_prereset(ap);
> +}
> +

Since it was a bug in the original patch, I just dropped it, waiting for 
you to resend the whole thing.  That keeps the git history much more clean.

I agree with your comment above -- SATA and PATA ports should use 
different ops structures

	Jeff

Re: [PATCH] sata_promise: fix missing PATA cable detection

[Tejun Heo]

Jeff Garzik wrote:
>> This patch adds yet another is-PATA-or-SATA? check, but it's in a
>> cold path so shouldn't matter much. This will be cleaned up if/when
>> PATA ports and SATA ports start using different ops structures.
[--snip--]
> I agree with your comment above -- SATA and PATA ports should use 
> different ops structures

Working on it.  It will be done in a few weeks.

-- 
tejun

Re: [PATCH] sata_promise: fix missing PATA cable detection

[Jeff Garzik]

Mikael Pettersson wrote:
> This patch fixes an oversight which caused sata_promise to
> not perform cable detection on the TX2plus chips' PATA ports.
> 
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> 
> ---
> 
> This patch adds yet another is-PATA-or-SATA? check, but it's in a
> cold path so shouldn't matter much. This will be cleaned up if/when
> PATA ports and SATA ports start using different ops structures.

applied