Re: Radeon regression fix

[Alex Deucher]

On Fri, Sep 30, 2011 at 1:14 AM, Brad Campbell
<lists2009@xxxxxxxxxxxxxxx> wrote:
> On 30/09/11 12:59, Alex Deucher wrote:
>>
>> On Thu, Sep 29, 2011 at 8:23 PM, Brad Campbell
>
>>> Looking at it with a nights sleep, it's obvious the code path in
>>> aux_native_write is ok. Is this a bit cleaner than the last patch?
>>
>> Looks pretty good.  I was thinking of something more like this (sorry
>> for the lack of a patch, I'm away from my source trees at the moment):
>>
>>        while (1) {
>>                ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
>>                                            msg, msg_bytes, recv,
>> recv_bytes, delay,&ack);
>>
>>                if (ret<  0)
>>                        return ret;
>>                if ((ack&  AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
>>                        return ret;
>>                else if ((ack&  AUX_NATIVE_REPLY_MASK) ==
>> AUX_NATIVE_REPLY_DEFER)
>>                        udelay(400);
>>                else if (ret == 0)
>>                        return -EPROTO;
>>                else
>>                        return -EIO;
>>        }
>
> Yep, that looks cleaner.
>
> My only thought was the pre-3.0 code had a limit to the number of retries.
> Was that for a specific reason or is it ok to attempt to retry indefinitely
> if we receive a DEFER ?

I need to double check the DP spec, but I think it's 4.  The while (1)
loops in radeon_dp_aux_native_write() and radeon_dp_aux_native_read()
should probably be changed to:
for (retry = 0; retry < 4; retry++)
to match what we do in radeon_dp_i2c_aux_ch().

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/