[alsa-devel] [PATCH 5/7] ASoC: wm5100: remove bitwise operations involving GPIO level value

Mark Brown broonie at kernel.org
Tue Jun 2 22:36:31 CEST 2015


On Tue, Jun 02, 2015 at 11:23:41PM +0300, Vladimir Zapolskiy wrote:
> On 02.06.2015 22:45, Mark Brown wrote:
> > On Tue, Jun 02, 2015 at 02:09:16AM +0300, Vladimir Zapolskiy wrote:

> >> +	unsigned int val = 0;
> >> +
> >> +	if (value)
> >> +		val = 0x1 << WM5100_GP1_LVL_SHIFT;

> > Write this as an if/else so the reader doesn't have to wonder why you've
> > missed the handling of the false case.  

> the only objection I have is that the resulting code will be two lines
> longer. If you think this code is not clear enough (is "val" vs. "value"
> misleading?), I'll change the rest of my patches, which contain the same
> logic structure, please let me know.

Especially after the unrelated style change thing earlier on (which
meant I was reading things more carefully than usual) it'd be good to
make things as clear as possible - you're right that the val vs value
thing isn't helping either.  

Like I say I am a bit surprised that the int/bool conversion doesn't do
the right thing without any code changes other than the type of the
parameter but ICBW, I didn't actually check.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150602/e5c0265e/attachment-0001.sig>


More information about the Alsa-devel mailing list