linux-kernel - Re: [PATCH 4/4] media: cxd2820r: use DIV_ROUND_CLOSEST

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <550C25E8.70202@ieee.org>
Date:	Fri, 20 Mar 2015 08:51:36 -0500
From:	Alex Elder <elder@...e.org>
To:	Javi Merino <javi.merino@....com>, akpm@...ux-foundation.org
CC:	intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, Antti Palosaari <crope@....fi>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>
Subject: Re: [PATCH 4/4] media: cxd2820r: use DIV_ROUND_CLOSEST_ULL()

On 03/20/2015 06:14 AM, Javi Merino wrote:
> Now that the kernel provides DIV_ROUND_CLOSEST_ULL(), drop the internal
> implementation and use the kernel one.
> 
> Cc: Antti Palosaari <crope@....fi>
> Cc: Mauro Carvalho Chehab <mchehab@....samsung.com>
> Signed-off-by: Javi Merino <javi.merino@....com>
> ---
> I've only compile-tested it, I don't have the hardware to run it.
> 
>  drivers/media/dvb-frontends/cxd2820r_c.c    | 2 +-
>  drivers/media/dvb-frontends/cxd2820r_core.c | 6 ------
>  drivers/media/dvb-frontends/cxd2820r_priv.h | 2 --
>  drivers/media/dvb-frontends/cxd2820r_t.c    | 2 +-
>  drivers/media/dvb-frontends/cxd2820r_t2.c   | 2 +-
>  5 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/cxd2820r_c.c b/drivers/media/dvb-frontends/cxd2820r_c.c
> index 149fdca3fb44..72b0e2db3aab 100644
> --- a/drivers/media/dvb-frontends/cxd2820r_c.c
> +++ b/drivers/media/dvb-frontends/cxd2820r_c.c
> @@ -79,7 +79,7 @@ int cxd2820r_set_frontend_c(struct dvb_frontend *fe)
>  
>  	num = if_freq / 1000; /* Hz => kHz */
>  	num *= 0x4000;
> -	if_ctl = 0x4000 - cxd2820r_div_u64_round_closest(num, 41000);
> +	if_ctl = 0x4000 - DIV_ROUND_CLOSEST_ULL(num, 41000);
>  	buf[0] = (if_ctl >> 8) & 0x3f;
>  	buf[1] = (if_ctl >> 0) & 0xff;
>  
> diff --git a/drivers/media/dvb-frontends/cxd2820r_core.c b/drivers/media/dvb-frontends/cxd2820r_core.c
> index 422e84bbb008..490e090048ef 100644
> --- a/drivers/media/dvb-frontends/cxd2820r_core.c
> +++ b/drivers/media/dvb-frontends/cxd2820r_core.c
> @@ -244,12 +244,6 @@ error:
>  	return ret;
>  }
>  
> -/* 64 bit div with round closest, like DIV_ROUND_CLOSEST but 64 bit */
> -u32 cxd2820r_div_u64_round_closest(u64 dividend, u32 divisor)
> -{
> -	return div_u64(dividend + (divisor / 2), divisor);
> -}

Technically, I'd say this has a bug, because the result
needs to be 64 bits wide or your results might be much
different from what might be desired.

Practically though, I'm pretty sure all callers provide
values that ensure the result is valid.

I only call attention because this patch changes the return
type of the function that gets called to do the calculation.

					-Alex

> -
>  static int cxd2820r_set_frontend(struct dvb_frontend *fe)
>  {
>  	struct cxd2820r_priv *priv = fe->demodulator_priv;
> diff --git a/drivers/media/dvb-frontends/cxd2820r_priv.h b/drivers/media/dvb-frontends/cxd2820r_priv.h
> index 7ff5f60c83e1..4b428959b16e 100644
> --- a/drivers/media/dvb-frontends/cxd2820r_priv.h
> +++ b/drivers/media/dvb-frontends/cxd2820r_priv.h
> @@ -64,8 +64,6 @@ int cxd2820r_wr_reg_mask(struct cxd2820r_priv *priv, u32 reg, u8 val,
>  int cxd2820r_wr_regs(struct cxd2820r_priv *priv, u32 reginfo, u8 *val,
>  	int len);
>  
> -u32 cxd2820r_div_u64_round_closest(u64 dividend, u32 divisor);
> -
>  int cxd2820r_wr_regs(struct cxd2820r_priv *priv, u32 reginfo, u8 *val,
>  	int len);
>  
> diff --git a/drivers/media/dvb-frontends/cxd2820r_t.c b/drivers/media/dvb-frontends/cxd2820r_t.c
> index 51401d036530..008cb2ac8480 100644
> --- a/drivers/media/dvb-frontends/cxd2820r_t.c
> +++ b/drivers/media/dvb-frontends/cxd2820r_t.c
> @@ -103,7 +103,7 @@ int cxd2820r_set_frontend_t(struct dvb_frontend *fe)
>  
>  	num = if_freq / 1000; /* Hz => kHz */
>  	num *= 0x1000000;
> -	if_ctl = cxd2820r_div_u64_round_closest(num, 41000);
> +	if_ctl = DIV_ROUND_CLOSEST_ULL(num, 41000);
>  	buf[0] = ((if_ctl >> 16) & 0xff);
>  	buf[1] = ((if_ctl >>  8) & 0xff);
>  	buf[2] = ((if_ctl >>  0) & 0xff);
> diff --git a/drivers/media/dvb-frontends/cxd2820r_t2.c b/drivers/media/dvb-frontends/cxd2820r_t2.c
> index 9c0c4f42175c..35fe364c7182 100644
> --- a/drivers/media/dvb-frontends/cxd2820r_t2.c
> +++ b/drivers/media/dvb-frontends/cxd2820r_t2.c
> @@ -120,7 +120,7 @@ int cxd2820r_set_frontend_t2(struct dvb_frontend *fe)
>  
>  	num = if_freq / 1000; /* Hz => kHz */
>  	num *= 0x1000000;
> -	if_ctl = cxd2820r_div_u64_round_closest(num, 41000);
> +	if_ctl = DIV_ROUND_CLOSEST_ULL(num, 41000);
>  	buf[0] = ((if_ctl >> 16) & 0xff);
>  	buf[1] = ((if_ctl >>  8) & 0xff);
>  	buf[2] = ((if_ctl >>  0) & 0xff);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/