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

[Stefan Bader]

Surbhi Palande wrote:
> 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.

Ah ok, so that message was only created with a sb600 chipset anyways. Ok, so 
this basically just delays printing the message and should be safe to do.

Acked-by: Stefan Bader <stefan.bader at canonical.com>


> 
>>> 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.

Thanks, makes it a bit clearer.

Stefan

> 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");
>>>          }
>>>      }
>>>
>>>
>>>
>>
> 


-- 

When all other means of communication fail, try words!