[PATCH] UBUNTU: SAUCE: Fixed ata error message printing when sb600 workaround is tried out

[Tim Gardner]

Surbhi Palande wrote:
> This patch fixes bug 444228 reported in launchpad.
> It prevents early erroneous printing of ata error message.
> It helps in a clean boot sequence.
> 
> This could also be applied for a merge upstream. Kindly do consider
> merging this for Karmic.
> 
> 
> The following changes since commit 823da90960aa2f2442bec8cb0dc711b49f7a48ca:
>   John Johansen (1):
>         UBUNTU: SAUCE: AppArmor: Fix off by 2 error in getprocattr mem
> allocation
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/surbhi/ubuntu-karmic.git lp444228
> 
> Surbhi Palande (1):
>       UBUNTU: SAUCE: Fixed ata error message printing when sb600 work around
> for a hardware bug is tried out
> 
>  drivers/ata/ahci.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> From 4ad8fa70a83537307bec3300c318194e7e2188cc
> Author: Surbhi Palande <surbhi.palande at canonical.com>
> Date:   Fri Oct 9 15:02:09 2009 +0300
>     
>     Buglink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/444228
>     
>     Soft reset fails on some ATI chips with IPMS set when PMP is enabled but
>     SATA HDD/ODD is connected to SATA port. This is a hardware bug and a work
>     around is to try a soft reset later at port 0.  Before this work around is
>     tried out a error message indicating that the device is not ready was
>     flashed out too early. Fixed this, to first try the work around if possible,
>     otherwise flash an error message.
>     
>     Signed-off-by: Surbhi Palande <surbhi.palande at canonical.com>
> 
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 289c4f8..1af5203 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1704,6 +1704,15 @@ static int ahci_do_softreset(struct ata_link *link, unsigned int *class,
>  
>  	/* wait for link to become ready */
>  	rc = ata_wait_after_reset(link, deadline, check_ready);
> +
> +	/*
> +	* Soft reset fails on some ATI chips with IPMS set when PMP
> +	* is enabled but SATA HDD/ODD is connected to SATA port.
> +	* A workaround shall be applied in the calling function
> +	* and we dont want to print error message without trying first
> +	*/
> +	if (rc == -EIO)
> +		return rc;
>  	if (rc == -EBUSY && hpriv->flags & AHCI_HFLAG_SRST_TOUT_IS_OFFLINE) {
>  		/*
>  		 * Workaround for cases where link online status can't
> @@ -1789,6 +1798,14 @@ static int ahci_sb600_softreset(struct ata_link *link, unsigned int *class,
>  					"and retrying\n");
>  			rc = ahci_do_softreset(link, class, 0, deadline,
>  					       ahci_check_ready);
> +		} else {
> +			/* since no message was printed in ahci_do_softreset
> +			* when ret value was -EIO and since we are not trying
> +			* any work around, we should print the err msg here,
> +			* (ideally we should not reach here)
> +			*/
> +			ata_link_printk(link, KERN_ERR, "softreset failed "
> +					"(device not ready)\n");
>  		}
>  	}
> 
> 
> 

NAK - I don't think this is necessary. Its a bonafide warning message
that merely indicates a workaround for a hardware issue. While your
patch works for the SB600 case, it _does_ propagate device specific
behavior into the generic layer (-EIO), not something that the
maintainer (Garzik) would likely allow.

Given that the default console log level has been raised to 2 (and the
message ought no longer be displayed on the console), I suggest you mark
this bug 'Invalid' because its not a software bug.

rtg
-- 
Tim Gardner tim.gardner at canonical.com