Fwd: Need guidance regarding fixing styleguide error in rtl871x_pwrctrl.h

[Ankit Pandey]

Thanks a lot Valdis. These are very helpful pointers to look into.

Regards,
Ankit

On Wed, Feb 9, 2022 at 10:06 PM Valdis Klētnieks <valdis.kletnieks at vt.edu>
wrote:

> On Tue, 08 Feb 2022 23:26:54 +0530, Ankit Pandey said:
>
> > And I was asked to verify if there is some specific meaning is attached
> to
> > comment here (which was causing the issue).
> > I would be glad you could explain me how should I approach this issue?
> One
> > way would
> > be to rewrite that these variables could be defined as volatile (just
> add a
> > comment) and then compile driver and see that build goes through without
> > any error.
>
> It turns out that the C keyword 'volatile' usually doesn't actually do what
> needs to happen if a variable actually *is* volatile and subject to change
> while the executing thread isn't looking.
>
> There's a good documentation file on this:
>
> Documentation/process/volatile-considered-harmful.rst
>
> But in summary - "If you thought you needed 'volatile' in your code, you
> probably needed locking primitives instead".
>
> > Other way would be that try to understand what this function is supposed
> to
> > be doing and then figure out author's intent of putting volatile there.
> How
> > should I take decision on these (or if they are wrong approaches) ?
>
> Given that struct pwrctlr_priv already contains a mutex_lock,  what was
> probably *intended* was "the variables cpwm, tog, cpwm_tog, and tgt_rpwm
> are
> protected by the mutex_lock and may only be changed by the mutex holder,
> while
> pwr_mode, smart_ps, and alives are not subject to change on the fly".
>
> But actually reading and understanding the code would be required to verify
> that.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20220209/72552d26/attachment-0001.html>