Thread (35 messages) 35 messages, 6 authors, 2025-01-06

Re: [PATCH v3 26/26] media: dvb-frontends: tda10048: Make the range of z explicit.

From: Ricardo Ribalda <hidden>
Date: 2024-05-13 13:36:27
Also in: linux-arm-msm, linux-media, linux-mediatek, linux-staging, linux-sunxi, linux-tegra, lkml

On Fri, 3 May 2024 at 16:08, Dragan Simic [off-list ref] wrote:
Hello Ricardo,

On 2024-05-03 13:56, Ricardo Ribalda wrote:
quoted
I am trying to get the DS, but
https://www.nxp.com/acrobat_download/literature/9397/75015931.pdf is a
dead links now.

Anyone have access to the datasheet?
It's kind of available on the link below, but for some strange reason
the download fails after downloading the first 128 KB or so.

https://web.archive.org/web/20080907185532/https://www.nxp.com/acrobat_download/literature/9397/75015931.pdf\
Mike, by any chance do you have a copy of the DS?

quoted
On Fri, 3 May 2024 at 13:55, Dan Carpenter [off-list ref]
wrote:
quoted
On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote:
quoted
Em Mon, 29 Apr 2024 15:05:05 +0000
Ricardo Ribalda [off-list ref] escreveu:
quoted
We do not expect the sample_freq to be over 613MHz.

Found by cocci:
drivers/media/dvb-frontends/tda10048.c:345:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.

Signed-off-by: Ricardo Ribalda <redacted>
---
 drivers/media/dvb-frontends/tda10048.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/tda10048.c b/drivers/media/dvb-frontends/tda10048.c
index 3e725cdcc66b..1886f733dbbf 100644
--- a/drivers/media/dvb-frontends/tda10048.c
+++ b/drivers/media/dvb-frontends/tda10048.c
@@ -328,7 +328,8 @@ static int tda10048_set_wref(struct dvb_frontend *fe, u32 sample_freq_hz,
                         u32 bw)
 {
    struct tda10048_state *state = fe->demodulator_priv;
-   u64 t, z;
+   u32 z;
+   u64 t;

    dprintk(1, "%s()\n", __func__);
@@ -341,6 +342,7 @@ static int tda10048_set_wref(struct dvb_frontend *fe, u32 sample_freq_hz,
    /* t *= 2147483648 on 32bit platforms */
    t *= (2048 * 1024);
    t *= 1024;
+   /* Sample frequency is under 613MHz */
Are you sure about that? Some DVB devices have very high frequency
clocks, specially if they're also used for satellite, so I can't
be sure by just looking at the driver's code.

Also, we had already a bunch of regressions with "fixes" like this
that actually broke frontend drivers.
This patch preserves the existing behavior. The sample_freq_hz
variable
is a u32 so, in the original code, z couldn't have been more than
U32_MAX even though it was declared as a u64.

I agree with Dan, we keep the existing behaviour. So it wont hurt to
merge the code...

All  that said, if someone has access to the DS, I do not mind reviewing it.

quoted
quoted
It's possible that the original code was wrong.  We have seen that in
other places in this patchset.  Adding a note about the datasheet is
also a good idea.

regards,
dan carpenter


-- 
Ricardo Ribalda

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help