[PATCH] remoteproc: fix a ! vs ~ typo

[PATCH] remoteproc: fix a ! vs ~ typo

[Dan Carpenter]

This is from static analysis and not from testing.  It doesn't really
make sense to talk about !BIT(0) so probably ~BIT(0) was intended.
I had to cast it from unsigned long to unsigned int to silence a GCC
warning.

Fixes: bb6869b21478 ("Peter Griffin <peter.griffin@linaro.org>")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---

Sorry for the crap commit message.  I'm pretty sure the static checker
warning is correct, but I don't know what the run time impact of this
bug would be.

 drivers/remoteproc/st_slim_rproc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index d711d9430a4f..a646dfb9d6cd 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -128,7 +128,7 @@ static int slim_rproc_start(struct rproc *rproc)
 	writel(SLIM_STBUS_SYNC_DIS, slim_rproc->peri + SLIM_STBUS_SYNC_OFST);
 
 	/* enable cpu pipeline clock */
-	writel(!SLIM_CLK_GATE_DIS,
+	writel((unsigned int)~SLIM_CLK_GATE_DIS,
 		slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
 
 	/* clear int & cmd mailbox */
@@ -167,7 +167,7 @@ static int slim_rproc_stop(struct rproc *rproc)
 	/* disable cpu pipeline clock */
 	writel(SLIM_CLK_GATE_DIS, slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
 
-	writel(!SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
+	writel((unsigned int)~SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
 
 	val = readl(slim_rproc->slimcore + SLIM_EN_OFST);
 	if (val & SLIM_EN_RUN)
-- 
2.17.1

Re: [PATCH] remoteproc: fix a ! vs ~ typo

[Patrice CHOTARD]

Hi Dan

On 12/18/18 9:16 AM, Dan Carpenter wrote:
> This is from static analysis and not from testing.  It doesn't really
> make sense to talk about !BIT(0) so probably ~BIT(0) was intended.
> I had to cast it from unsigned long to unsigned int to silence a GCC
> warning.
> 
> Fixes: bb6869b21478 ("Peter Griffin <peter.griffin@linaro.org>")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> 
> Sorry for the crap commit message.  I'm pretty sure the static checker
> warning is correct, but I don't know what the run time impact of this
> bug would be.
> 
>  drivers/remoteproc/st_slim_rproc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
> index d711d9430a4f..a646dfb9d6cd 100644
> --- a/drivers/remoteproc/st_slim_rproc.c
> +++ b/drivers/remoteproc/st_slim_rproc.c
> @@ -128,7 +128,7 @@ static int slim_rproc_start(struct rproc *rproc)
>  	writel(SLIM_STBUS_SYNC_DIS, slim_rproc->peri + SLIM_STBUS_SYNC_OFST);
>  
>  	/* enable cpu pipeline clock */
> -	writel(!SLIM_CLK_GATE_DIS,
> +	writel((unsigned int)~SLIM_CLK_GATE_DIS,
>  		slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
>  
>  	/* clear int & cmd mailbox */
> @@ -167,7 +167,7 @@ static int slim_rproc_stop(struct rproc *rproc)
>  	/* disable cpu pipeline clock */
>  	writel(SLIM_CLK_GATE_DIS, slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
>  
> -	writel(!SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
> +	writel((unsigned int)~SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
>  
>  	val = readl(slim_rproc->slimcore + SLIM_EN_OFST);
>  	if (val & SLIM_EN_RUN)
> 

Writing !SLIM_CLK_GATE_DIS or (unsigned int)~SLIM_CLK_GATE_DIS don't
give the same result:

!SLIM_CLK_GATE_DIS = 0

(unsigned int)~SLIM_CLK_GATE_DIS = 0xfffffffe

Patrice

Re: [PATCH] remoteproc: fix a ! vs ~ typo

[Dan Carpenter]

On Thu, Dec 20, 2018 at 10:05:55AM +0000, Patrice CHOTARD wrote:
> > -	writel(!SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
> > +	writel((unsigned int)~SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
> >  
> >  	val = readl(slim_rproc->slimcore + SLIM_EN_OFST);
> >  	if (val & SLIM_EN_RUN)
> > 
> 
> Writing !SLIM_CLK_GATE_DIS or (unsigned int)~SLIM_CLK_GATE_DIS don't
> give the same result:
> 
> !SLIM_CLK_GATE_DIS = 0
> 
> (unsigned int)~SLIM_CLK_GATE_DIS = 0xfffffffe

Right.  !BIT(0) is zero.  I'm pretty sure we meant to do a bitwise
negate and not a logical negate from looking at the code, but I don't
know the real world impact and can't test it.  It's kind of a crap
patch, but I think it's correct.

regards,
dan carpenter

Re: [PATCH] remoteproc: fix a ! vs ~ typo

[Dan Carpenter]

On Thu, Dec 20, 2018 at 10:05:55AM +0000, Patrice CHOTARD wrote:
> Hi Dan
> 
> On 12/18/18 9:16 AM, Dan Carpenter wrote:
> > This is from static analysis and not from testing.  It doesn't really
> > make sense to talk about !BIT(0) so probably ~BIT(0) was intended.
> > I had to cast it from unsigned long to unsigned int to silence a GCC
> > warning.
> > 
> > Fixes: bb6869b21478 ("Peter Griffin <peter.griffin@linaro.org>")

And oh...  I put Peter's email here instead of the commit title.  Sorry.

I will resend if we decide the patch is required.

regards,
dan carpenter