[RFC PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[RFC PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Tejun Heo]

>From 2.6.26, libata is defaulting to hardreset for various reasons one
of them being unifying probing, hotplug and EH reset paths uniform.
Previously, broken hardreset could go unnoticed as it wasn't used
during probing but when something goes wrong or after hotplug the
problem will surface and bite hard.

OSDL bug 11195 reports that sata_nv generic flavor falls into this
category.  Hardreset itself succeeds but PHY stays offline after
hardreset.  I tried longer debounce timing but the result was the
same.

  http://bugzilla.kernel.org/show_bug.cgi?id=11195

So, it seems we'll have to drop hardreset from the generic flavor.
Peer Chen and Kuan, does anything ring a bell?  ie. Certain subset are
unaffected, etc...

Thanks.

PLEASE DON'T APPLY JUST YET
---
 drivers/ata/sata_nv.c |   19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 858f706..1e1f3f3 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -309,8 +309,6 @@ static void nv_nf2_freeze(struct ata_port *ap);
 static void nv_nf2_thaw(struct ata_port *ap);
 static void nv_ck804_freeze(struct ata_port *ap);
 static void nv_ck804_thaw(struct ata_port *ap);
-static int nv_hardreset(struct ata_link *link, unsigned int *class,
-			unsigned long deadline);
 static int nv_adma_slave_config(struct scsi_device *sdev);
 static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
 static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
@@ -407,7 +405,7 @@ static struct scsi_host_template nv_swncq_sht = {
 
 static struct ata_port_operations nv_generic_ops = {
 	.inherits		= &ata_bmdma_port_ops,
-	.hardreset		= nv_hardreset,
+	.hardreset		= ATA_OP_NULL,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
 };
@@ -1588,21 +1586,6 @@ static void nv_mcp55_thaw(struct ata_port *ap)
 	ata_sff_thaw(ap);
 }
 
-static int nv_hardreset(struct ata_link *link, unsigned int *class,
-			unsigned long deadline)
-{
-	int rc;
-
-	/* SATA hardreset fails to retrieve proper device signature on
-	 * some controllers.  Request follow up SRST.  For more info,
-	 * see http://bugzilla.kernel.org/show_bug.cgi?id=3352
-	 */
-	rc = sata_sff_hardreset(link, class, deadline);
-	if (rc)
-		return rc;
-	return -EAGAIN;
-}
-
 static void nv_adma_error_handler(struct ata_port *ap)
 {
 	struct nv_adma_port_priv *pp = ap->private_data;

RE: [RFC PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Peer Chen]

Hard reset should work for MCP61 and 2.6.26 work fine on my MCP61 board,
I believe the failure is specific case.
Does this board work fine under windows? Switch the HDD or connecting
the HDD to other ports are also worth trying.

BRs
Peer Chen
> -----Original Message-----
> From: Tejun Heo [mailto:tj@kernel.org]
> Sent: Sunday, August 03, 2008 1:22 PM
> To: linux@sun7.cn; IDE/ATA development list; Jeff Garzik; Peer Chen;
Kuan Luo
> Subject: [RFC PATCH #upstream-fixes] sata_nv: disable hardreset for
generic
> 
> From 2.6.26, libata is defaulting to hardreset for various reasons one
> of them being unifying probing, hotplug and EH reset paths uniform.
> Previously, broken hardreset could go unnoticed as it wasn't used
> during probing but when something goes wrong or after hotplug the
> problem will surface and bite hard.
> 
> OSDL bug 11195 reports that sata_nv generic flavor falls into this
> category.  Hardreset itself succeeds but PHY stays offline after
> hardreset.  I tried longer debounce timing but the result was the
> same.
> 
>   http://bugzilla.kernel.org/show_bug.cgi?id=11195
> 
> So, it seems we'll have to drop hardreset from the generic flavor.
> Peer Chen and Kuan, does anything ring a bell?  ie. Certain subset are
> unaffected, etc...
> 
> Thanks.
> 
> PLEASE DON'T APPLY JUST YET
> ---
>  drivers/ata/sata_nv.c |   19 +------------------
>  1 file changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
> index 858f706..1e1f3f3 100644
> --- a/drivers/ata/sata_nv.c
> +++ b/drivers/ata/sata_nv.c
> @@ -309,8 +309,6 @@ static void nv_nf2_freeze(struct ata_port *ap);
>  static void nv_nf2_thaw(struct ata_port *ap);
>  static void nv_ck804_freeze(struct ata_port *ap);
>  static void nv_ck804_thaw(struct ata_port *ap);
> -static int nv_hardreset(struct ata_link *link, unsigned int *class,
> -			unsigned long deadline);
>  static int nv_adma_slave_config(struct scsi_device *sdev);
>  static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
>  static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
> @@ -407,7 +405,7 @@ static struct scsi_host_template nv_swncq_sht = {
> 
>  static struct ata_port_operations nv_generic_ops = {
>  	.inherits		= &ata_bmdma_port_ops,
> -	.hardreset		= nv_hardreset,
> +	.hardreset		= ATA_OP_NULL,
>  	.scr_read		= nv_scr_read,
>  	.scr_write		= nv_scr_write,
>  };
> @@ -1588,21 +1586,6 @@ static void nv_mcp55_thaw(struct ata_port *ap)
>  	ata_sff_thaw(ap);
>  }
> 
> -static int nv_hardreset(struct ata_link *link, unsigned int *class,
> -			unsigned long deadline)
> -{
> -	int rc;
> -
> -	/* SATA hardreset fails to retrieve proper device signature on
> -	 * some controllers.  Request follow up SRST.  For more info,
> -	 * see http://bugzilla.kernel.org/show_bug.cgi?id=3352
> -	 */
> -	rc = sata_sff_hardreset(link, class, deadline);
> -	if (rc)
> -		return rc;
> -	return -EAGAIN;
> -}
> -
>  static void nv_adma_error_handler(struct ata_port *ap)
>  {
>  	struct nv_adma_port_priv *pp = ap->private_data;
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [RFC PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Tejun Heo]

Peer Chen wrote:
> Hard reset should work for MCP61 and 2.6.26 work fine on my MCP61 board,
> I believe the failure is specific case.
> Does this board work fine under windows? Switch the HDD or connecting
> the HDD to other ports are also worth trying.

Sorry about the delay.  Can you please try to test one?  This one is
rather serious regression as devices fail detection and it's far better
to disable hardreset on all MCP61s if we can't determine which specific
ones are problematic.

Thank you.

-- 
tejun

[PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Tejun Heo]

>From 2.6.26, libata is defaulting to hardreset for various reasons one
of them being unifying probing, hotplug and EH reset paths uniform.
Previously, broken hardreset could go unnoticed as it wasn't used
during probing but when something goes wrong or after hotplug the
problem will surface and bite hard.

OSDL bug 11195 reports that sata_nv generic flavor falls into this
category.  Hardreset itself succeeds but PHY stays offline after
hardreset.  I tried longer debounce timing but the result was the
same.

  http://bugzilla.kernel.org/show_bug.cgi?id=11195

So, it seems we'll have to drop hardreset from the generic flavor.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Peer Chen <pchen@nvidia.com>
---
There have been more than three reports of this problem and there just
isn't good reason to defer this workaround anymore.  Once this gets
into #upstream-fixes, I think this should go to -stable too.

Thanks.

 drivers/ata/sata_nv.c |   19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 858f706..1e1f3f3 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -309,8 +309,6 @@ static void nv_nf2_freeze(struct ata_port *ap);
 static void nv_nf2_thaw(struct ata_port *ap);
 static void nv_ck804_freeze(struct ata_port *ap);
 static void nv_ck804_thaw(struct ata_port *ap);
-static int nv_hardreset(struct ata_link *link, unsigned int *class,
-			unsigned long deadline);
 static int nv_adma_slave_config(struct scsi_device *sdev);
 static int nv_adma_check_atapi_dma(struct ata_queued_cmd *qc);
 static void nv_adma_qc_prep(struct ata_queued_cmd *qc);
@@ -407,7 +405,7 @@ static struct scsi_host_template nv_swncq_sht = {
 
 static struct ata_port_operations nv_generic_ops = {
 	.inherits		= &ata_bmdma_port_ops,
-	.hardreset		= nv_hardreset,
+	.hardreset		= ATA_OP_NULL,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
 };
@@ -1588,21 +1586,6 @@ static void nv_mcp55_thaw(struct ata_port *ap)
 	ata_sff_thaw(ap);
 }
 
-static int nv_hardreset(struct ata_link *link, unsigned int *class,
-			unsigned long deadline)
-{
-	int rc;
-
-	/* SATA hardreset fails to retrieve proper device signature on
-	 * some controllers.  Request follow up SRST.  For more info,
-	 * see http://bugzilla.kernel.org/show_bug.cgi?id=3352
-	 */
-	rc = sata_sff_hardreset(link, class, deadline);
-	if (rc)
-		return rc;
-	return -EAGAIN;
-}
-
 static void nv_adma_error_handler(struct ata_port *ap)
 {
 	struct nv_adma_port_priv *pp = ap->private_data;

Re: [PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Jeff Garzik]

Tejun Heo wrote:
>>From 2.6.26, libata is defaulting to hardreset for various reasons one
> of them being unifying probing, hotplug and EH reset paths uniform.
> Previously, broken hardreset could go unnoticed as it wasn't used
> during probing but when something goes wrong or after hotplug the
> problem will surface and bite hard.
> 
> OSDL bug 11195 reports that sata_nv generic flavor falls into this
> category.  Hardreset itself succeeds but PHY stays offline after
> hardreset.  I tried longer debounce timing but the result was the
> same.
> 
>   http://bugzilla.kernel.org/show_bug.cgi?id=11195
> 
> So, it seems we'll have to drop hardreset from the generic flavor.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Peer Chen <pchen@nvidia.com>
> ---
> There have been more than three reports of this problem and there just
> isn't good reason to defer this workaround anymore.  Once this gets
> into #upstream-fixes, I think this should go to -stable too.
> 
> Thanks.
> 
>  drivers/ata/sata_nv.c |   19 +------------------

applied

Re: [PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Tejun Heo]

Tejun Heo wrote:
> From 2.6.26, libata is defaulting to hardreset for various reasons one
> of them being unifying probing, hotplug and EH reset paths uniform.
> Previously, broken hardreset could go unnoticed as it wasn't used
> during probing but when something goes wrong or after hotplug the
> problem will surface and bite hard.
> 
> OSDL bug 11195 reports that sata_nv generic flavor falls into this
> category.  Hardreset itself succeeds but PHY stays offline after
> hardreset.  I tried longer debounce timing but the result was the
> same.
> 
>   http://bugzilla.kernel.org/show_bug.cgi?id=11195
> 
> So, it seems we'll have to drop hardreset from the generic flavor.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Peer Chen <pchen@nvidia.com>

This one definitely should go into -stable too.  The original patch is...

http://article.gmane.org/gmane.linux.ide/34077/raw

Thanks.

-- 
tejun

Re: [stable] [PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Greg KH]

On Mon, Sep 08, 2008 at 09:17:30PM +0200, Tejun Heo wrote:
> Tejun Heo wrote:
> > From 2.6.26, libata is defaulting to hardreset for various reasons one
> > of them being unifying probing, hotplug and EH reset paths uniform.
> > Previously, broken hardreset could go unnoticed as it wasn't used
> > during probing but when something goes wrong or after hotplug the
> > problem will surface and bite hard.
> > 
> > OSDL bug 11195 reports that sata_nv generic flavor falls into this
> > category.  Hardreset itself succeeds but PHY stays offline after
> > hardreset.  I tried longer debounce timing but the result was the
> > same.
> > 
> >   http://bugzilla.kernel.org/show_bug.cgi?id=11195
> > 
> > So, it seems we'll have to drop hardreset from the generic flavor.
> > 
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > Cc: Peer Chen <pchen@nvidia.com>
> 
> This one definitely should go into -stable too.  The original patch is...
> 
> http://article.gmane.org/gmane.linux.ide/34077/raw

Now applied, thanks.

greg k-h

Re: [stable] [PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Tejun Heo]

Greg KH wrote:
> On Mon, Sep 08, 2008 at 09:17:30PM +0200, Tejun Heo wrote:
>> Tejun Heo wrote:
>>> From 2.6.26, libata is defaulting to hardreset for various reasons one
>>> of them being unifying probing, hotplug and EH reset paths uniform.
>>> Previously, broken hardreset could go unnoticed as it wasn't used
>>> during probing but when something goes wrong or after hotplug the
>>> problem will surface and bite hard.
>>>
>>> OSDL bug 11195 reports that sata_nv generic flavor falls into this
>>> category.  Hardreset itself succeeds but PHY stays offline after
>>> hardreset.  I tried longer debounce timing but the result was the
>>> same.
>>>
>>>   http://bugzilla.kernel.org/show_bug.cgi?id=11195
>>>
>>> So, it seems we'll have to drop hardreset from the generic flavor.
>>>
>>> Signed-off-by: Tejun Heo <tj@kernel.org>
>>> Cc: Peer Chen <pchen@nvidia.com>
>> This one definitely should go into -stable too.  The original patch is...
>>
>> http://article.gmane.org/gmane.linux.ide/34077/raw
> 
> Now applied, thanks.

Greg, can you please hold a bit before releasing this?  This patch
accidentally disables hardreset for all flavors of nv controllers which
causes problems for ck804.  A later patch to only apply it to generic
went in but it then broke nf2 flavor, so it seems generic and nf2
require this workaround but ck804 doesn't like the workaround.  I've
asked a reporter to test to apply the workaround only to generic and nf2
and will forward the third patch as soon as it's confirmed.  So, please
wait till the third one is ready.

Thanks.

-- 
tejun

Re: [stable] [PATCH #upstream-fixes] sata_nv: disable hardreset for generic

[Greg KH]

On Thu, Oct 02, 2008 at 09:22:31AM +0900, Tejun Heo wrote:
> Greg KH wrote:
> > On Mon, Sep 08, 2008 at 09:17:30PM +0200, Tejun Heo wrote:
> >> Tejun Heo wrote:
> >>> From 2.6.26, libata is defaulting to hardreset for various reasons one
> >>> of them being unifying probing, hotplug and EH reset paths uniform.
> >>> Previously, broken hardreset could go unnoticed as it wasn't used
> >>> during probing but when something goes wrong or after hotplug the
> >>> problem will surface and bite hard.
> >>>
> >>> OSDL bug 11195 reports that sata_nv generic flavor falls into this
> >>> category.  Hardreset itself succeeds but PHY stays offline after
> >>> hardreset.  I tried longer debounce timing but the result was the
> >>> same.
> >>>
> >>>   http://bugzilla.kernel.org/show_bug.cgi?id=11195
> >>>
> >>> So, it seems we'll have to drop hardreset from the generic flavor.
> >>>
> >>> Signed-off-by: Tejun Heo <tj@kernel.org>
> >>> Cc: Peer Chen <pchen@nvidia.com>
> >> This one definitely should go into -stable too.  The original patch is...
> >>
> >> http://article.gmane.org/gmane.linux.ide/34077/raw
> > 
> > Now applied, thanks.
> 
> Greg, can you please hold a bit before releasing this?  This patch
> accidentally disables hardreset for all flavors of nv controllers which
> causes problems for ck804.  A later patch to only apply it to generic
> went in but it then broke nf2 flavor, so it seems generic and nf2
> require this workaround but ck804 doesn't like the workaround.  I've
> asked a reporter to test to apply the workaround only to generic and nf2
> and will forward the third patch as soon as it's confirmed.  So, please
> wait till the third one is ready.

Ok, I've dropped:
	sata_nv-disable-hardreset-for-generic.patch
from the 2.6.26-stable queue for now.

If you get it all working, please resend all of the patches that you
wish me to pick up for a -stable release (well send them to
stable@kernel.org, not me directly.)

thanks,

greg k-h