Hello Mark,
On 02.06.2015 22:38, Mark Brown wrote:
On Tue, Jun 02, 2015 at 02:09:13AM +0300, Vladimir Zapolskiy wrote:
if (value)val = RT5677_GPIO_OUT_HI(offset);It seems like a greater variation in variable names might be called for here.
thank you for review, you mean "val" vs. "value" I guess. May be you have a better register value naming in mind?
regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
0x1 << (offset * 3 + 1), !!value << (offset * 3 + 1));
RT5677_GPIO_OUT_MASK(offset), val);Besides, isn't the minimal change here just to remove the !! (or do nothing)?
this particular change mainly addresses "clean up gpiolib callbacks" target as it is stated in subject/commit message. Removing of "!!" might be unsafe, because the input value is not necessary 0 or 1 at the moment.
C defines a mapping between boolean and integer values.
As for today it is correct. In the kernel right now there is a movement of replacing for instance explicit integer constants to false/true, when they are used with variables of bool type, e.g. see scripts/coccinelle/misc/bool{init,return}.cocci.
In my opinion changing GPIO level argument from int to bool should be done, when all confusing cases like "!!false << (offset * 3 + 1)" above (if just type of "value" is modified) are cleaned up in the code firstly.
-- With best wishes, Vladimir