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

["Valdis Klētnieks" <valdis.kletnieks@vt.edu>]

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.



_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies