[PATCH] nouveau/nvkm/subdev/clk/gk20a.c: fix wrong do_div() usage

[PATCH] nouveau/nvkm/subdev/clk/gk20a.c: fix wrong do_div() usage

[Nicolas Pitre]

do_div() must only be used with a u64 dividend.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
index 254094ab7f..5da2aa8cc3 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
@@ -141,9 +141,8 @@ gk20a_pllg_calc_rate(struct gk20a_clk *clk)
 
 	rate = clk->parent_rate * clk->n;
 	divider = clk->m * pl_to_div[clk->pl];
-	do_div(rate, divider);
 
-	return rate / 2;
+	return rate / divider / 2;
 }
 
 static int
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] nouveau/nvkm/subdev/clk/gk20a.c: fix wrong do_div() usage

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1117 bytes --]

On Tue, Nov 03, 2015 at 05:01:46PM -0500, Nicolas Pitre wrote:
> do_div() must only be used with a u64 dividend.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
> index 254094ab7f..5da2aa8cc3 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
> @@ -141,9 +141,8 @@ gk20a_pllg_calc_rate(struct gk20a_clk *clk)
>  
>  	rate = clk->parent_rate * clk->n;
>  	divider = clk->m * pl_to_div[clk->pl];
> -	do_div(rate, divider);
>  
> -	return rate / 2;
> +	return rate / divider / 2;
>  }
>  
>  static int

This causes build breakage on 32-bit ARM. I'm also confused by the
commit message because the code that I'm looking at has u64 rate and u32
divider, which matches the types given in include/asm-generic/div64.h:

 * The semantics of do_div() are:
 *
 * uint32_t do_div(uint64_t *n, uint32_t base)
 * {
 * 	uint32_t remainder = *n % base;
 * 	*n = *n / base;
 * 	return remainder;
 * }

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] nouveau/nvkm/subdev/clk/gk20a.c: fix wrong do_div() usage

[Nicolas Pitre]

On Fri, 4 Dec 2015, Thierry Reding wrote:

> On Tue, Nov 03, 2015 at 05:01:46PM -0500, Nicolas Pitre wrote:
> > do_div() must only be used with a u64 dividend.
> > 
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
> > index 254094ab7f..5da2aa8cc3 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c
> > @@ -141,9 +141,8 @@ gk20a_pllg_calc_rate(struct gk20a_clk *clk)
> >  
> >  	rate = clk->parent_rate * clk->n;
> >  	divider = clk->m * pl_to_div[clk->pl];
> > -	do_div(rate, divider);
> >  
> > -	return rate / 2;
> > +	return rate / divider / 2;
> >  }
> >  
> >  static int
> 
> This causes build breakage on 32-bit ARM.

Funny, because the above _fixes_ build warnings on 32-bit ARM for me.

> I'm also confused by the commit message because the code that I'm 
> looking at has u64 rate and u32 divider,

If I look at drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c 
in v4.4-rc3 between line 137 and line 147, I see this:

gk20a_pllg_calc_rate(struct gk20a_clk *clk)
{
        u32 rate;
        u32 divider;

        rate = clk->parent_rate * clk->n;
        divider = clk->m * pl_to_div[clk->pl];
        do_div(rate, divider);

        return rate / 2;
}

So clearly both rate and divider are u32 variables and do_div() should 
not be used.

Here's the warning:

drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c: In function 'gk20a_pllg_calc_rate':
include/asm-generic/div64.h:207:28: warning: comparison of distinct pointer types lacks a cast
  (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
                            ^
drivers/gpu/drm/nouveau/nvkm/subdev/clk/gk20a.c:144:2: note: in expansion of macro 'do_div'
  do_div(rate, divider);
  ^

The corresponding line number in vanilla v4.4-rc3 for 
include/asm-generic/div64.h is line 43.


Nicolas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel