Re: [PATCH v1 i2c/for-next] i2c-i801: recover from hardware PEC errors

[Ellen Wang <ellen-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>]

Thanks!  Given that the driver is fully serialized, there really 
shouldn't be any subtleties there.

I'll update the patch after some more testing.

On 04/24/2015 04:00 AM, Daniel Kurtz wrote:
> Hi guys,
>
> On Fri, Apr 24, 2015 at 6:08 PM, Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org> wrote:
>> Hi Ellen,
>>
>> On Mon, 20 Apr 2015 15:52:47 -0700, Ellen Wang wrote:
>>> This leads to a related question:  If the driver is serialized, then the
>>> (status = priv->status) inside wait_event_timeout() isn't strictly
>>> necessary, correct?  It can just be priv->status.  I'm just want to
>>> double check that there's no race and I can add a priv->auxsts and not
>>> have to stick something like this inside the wait_event_timeout():
>>>
>>>      (auxsts = priv->auxsts, status = priv->status)
>>
>> To be honest I'm not 100% certain. I think this was only a minor
>> optimization, but I may be wrong. Adding Daniel to Cc, he added that
>> code to the i2c-i801 driver. Daniel, any comment on this?
>>
>> Equally mysterious to me is:
>>
>>                  priv->status |= status;
>>
>> in i801_isr() where it would seem that a simple "=" would suffice.
>
> Sorry, it has been a long time since i looked at i801 code,
> I think you guys are correct.  You can probably just do:
>
>              wait_event(priv->waitq, priv->status);
>              status = priv->status;
>              priv->status = 0;
>              return i801_check_post(priv, status);
>
> And, I agree with Jean in i801_isr(),
>     priv->status |= status;
>
> Could probably just be:
>     priv->status = status;
>
> -Dan
>
>>
>> --
>> Jean Delvare
>> SUSE L3 Support