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

[Surbhi Palande]

Dear Stefan,

Hello!

>> I got a question to this: while the EIO will be handled with printing
the message, ahci_do_softreset is
>> also called from the generic ahci_softreset which does not handle
that. Wouldn't it make sense to add
>> error handling to the generic case?

When ahci_softreset() calls ahci_do_softreset(), it never gets -EIO
return value. It is only returned when ahci_sb600_check_ready() is
called ( i.e when
checkready = ahci_sb600_check_ready() and ahci_sb600_check_ready() is
the caller)
The only return values for the other checkready functions are 0
(SUCCESS), 1 (BUSY) or -ENODEV)
ahci_sb600_check_ready() uses -EIO to indicate that this result could be
because of a hardware bug.


>> Also just for me to learn: what is IPMS and PMP here?

If I understand it correctly, it is the port multiplier where the soft
reset signal has to be sent.

Warm Regards,
Surbhi.








Stefan Bader 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.
>
> I got a question to this: while the EIO will be handled with printing
> the message, ahci_do_softreset is also called from the generic
> ahci_softreset which does not handle that. Wouldn't it make sense to
> add error handling to the generic case?
> Also just for me to learn: what is IPMS and PMP here?
>
>>
>> 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");
>>          }
>>      }
>>
>>
>>
>
>