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

Stefan Bader stefan.bader at canonical.com
Fri Oct 9 14:23:17 UTC 2009


Tim Gardner wrote:
> 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.

Still it might be worthwhile to discuss upstream / with Jeff. Maybe there is 
another solution but it might be a starting point to get rid of a totally 
useless warning.

-Stefan

> 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


-- 

When all other means of communication fail, try words!






More information about the kernel-team mailing list