lkml.org 
[lkml]   [2017]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] clockevents/drivers/cs5535: improve resilience to spurious interrupts
On Fri, 20 Oct 2017, Thomas Gleixner wrote:

> On Thu, 19 Oct 2017, David Kozub wrote:
>
>> This solves a BUG on ALIX 2c3 where mfgpt_tick is called before
>> clockevents_config_and_register returns. This caused mfgpt_tick to call a
>> null function pointer.
>>
>> Thanks to Daniel Lezcano and Thomas Gleixner for helping me analyze this
>> and suggesting a solution.
>
> Thanks for sending this! Though I have two minor issues with this:
>
> 1) The changelog.
>
> We try to structure changelogs by explaining the context, the problem and
> its consequences and the fix. If the fix is non obvious it wants an
> elaborate explanation. Let me give you an example:
>
> The interrupt handler mfgpt_tick() is not robust versus spurious
> interrupts which happen before the clock event device is registered and
> fully initialized.
>
> The reason is that the safe guard against spurious interrupts solely
> checks for the clockevents shutdown state, but lacks a check for
> detached state. If the interrupt hits while the device is in detached
> state it passes the safe guard and dereferences the event handler call
> back which is NULL.
>
> Add the missing state check.
>
> See?

Thanks for the feedback. Yes, that is better.

Now that I'm neither the author of the code nor the author of the commit
message it doesn't seem right that I should be submitting this.

>> - if (clockevent_state_shutdown(&cs5535_clockevent))
>> + if (clockevent_state_detached(&cs5535_clockevent) ||
>> + clockevent_state_shutdown(&cs5535_clockevent))
>
> Please do not use random indentation for multiline conditionals
>
> if (clockevent_state_detached(&cs5535_clockevent) ||
> clockevent_state_shutdown(&cs5535_clockevent))
>
> is the style we use through out the code.

OK. I noticed the four spaces. I was swayed by process/coding-style.rst:

Outside of comments, documentation and except in Kconfig, spaces are never
used for indentation,

But now I think I just misunderstood that. The spaces on the second line
of the "if" are not regarded as ident.


Best regards,
David

\
 
 \ /
  Last update: 2017-10-22 17:34    [W:0.139 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site