Re: [Outreachy kernel] [PATCH 1/5] staging: comedi: ni_mio_common.c: reformat code

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 12 Mar 2020 00:13:23 +0530
Kaaira Gupta <kgupta@es.iitr.ac.in> wrote:

> reformat the arguments given to ni_stc_writew() to increase code
> readability in ni_mio_common.c
> 
> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>
> ---
>  drivers/staging/comedi/drivers/ni_mio_common.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
> index b72a40a79930..06ed4b1dd875 100644
> --- a/drivers/staging/comedi/drivers/ni_mio_common.c
> +++ b/drivers/staging/comedi/drivers/ni_mio_common.c
> @@ -2445,9 +2445,9 @@ static int ni_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  #endif
>  
>  	if (cmd->start_src == TRIG_NOW) {
> -		ni_stc_writew(dev, NISTC_AI_CMD2_START1_PULSE |
> -				   devpriv->ai_cmd2,
> -			      NISTC_AI_CMD2_REG);
> +		ni_stc_writew(dev, devpriv->ai_cmd2 |
> +				NISTC_AI_CMD2_START1_PULSE,
> +				NISTC_AI_CMD2_REG);

This becomes confusing: you're aligning NISTC_AI_CMD2_REG with
NISTC_AI_CMD2_START1_PULSE, but it's two different arguments, and
breaking the alignment between devpriv->ai_cmd2 and
NISTC_AI_CMD2_START1_PULSE, which should instead be preserved as
they are the two operands.

Did you find this with an automated tool? What was the reported
issue? At a quick visual check, I don't see any.

-- 
Stefano