[PATCH 1/7] media: aspeed: fix a kernel warning on clk control

[Jae Hyun Yoo]

On 5/8/2019 7:16 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2019-05-08 at 18:19 -0700, Jae Hyun Yoo wrote:
>> I changed that from a bool because the maintainer of this code, Eddie
>> doesn't like adding of an additional flag. I'll change it back with
>> codes in the first submit:
>> https://www.spinics.net/lists/linux-media/msg148955.html
>>
>> Eddie,
>> Please let me know if you have any objection on that.
> 
> Ok, so random flags ... ugh.
> 
> Well, you can approach it either way. Have them all be bitops or all be
> bool.
> 
> The tricky thing however is that if they are bitops you need to ensure
> that they are *all* manipulated under the same lock. If not you have to
> use the atomic bitops variants.
> 
> The reason I don't like that is that experience shows that most uses of
> such atomic variants in drivers usually are failed attempts at papering
> over broken locking.
> 
> If everything is covered by a lock, then using the non-atomic versions
> is more efficient, but so is using bool (optionally with :1 bitfield
> qualifiers to avoid wasting memory), which from a pure C language
> perspective I think is more expressive of what you are doing and more
> readable.

Okay. Probably I need to add one another patch in this series to address
what you pointed out.

I have one question. I reviewed again all bitops in this driver and
checked that some bitops are protected by a spinlock and some others
are not. In this case, can I mix use atomic and non-atomic bitops
depend on each bitop's protection state by the spinlock? Or, would it be
better to change all of them to bool in this case?

Thanks,
Jae

> 
> Cheers,
> Ben.
>