[PATCH 1/7] media: aspeed: fix a kernel warning on clk control
Jae Hyun Yoo
jae.hyun.yoo at linux.intel.com
Fri May 10 03:19:28 AEST 2019
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:
>> 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
> 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
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?
More information about the Linux-aspeed