| Message ID | 20250107-fix-cocci-v5-1-b26da641f730@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | media: Fix coccinelle warning/errors | expand |
hi Ricardo, according to the datasheet the recommended sampling frequency is 55 MHz (BTW, if you look at the definitions in the source code and make the calculations that is exactly the sampling frequency all currently supported in Linux devices are using as well). also, I spent few minutes time to make the calculations based on the restrains of the PLL build-in tda10048 and in theory the maximum is 69 MHz. so, if you make next revision of this patch, feel free to update the comment accordingly, in short - recommended sampling frequency of 55 MHz, theoretical maximum of 69 MHz. in any case, your assumption is correct and in reality is away less than the maximum value you assumed. Reviewed-by: Kosta Stefanov <costa.stephanoff@gmail.com> --Kosta On Tue, Jan 7, 2025 at 12:54 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > We have not been able to find the relevant datahsheet, but it seems rare > that the device will have a sampling frequency over 613MHz. > > Nonetheless, this patch does not introduce any change in behaviour, it > just adds a comment to make explicit the current limit: div by 32 bits. > > 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 <ribalda@chromium.org> > --- > 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 */ > z = 7 * sample_freq_hz; > do_div(t, z); > t += 5; > > -- > 2.47.1.613.gc27f4b7a9f-goog > >
Thanks Kosta! On Tue, 7 Jan 2025 at 18:28, Kosta Stefanov <costa.stephanoff@gmail.com> wrote: > > hi Ricardo, according to the datasheet the recommended sampling > frequency is 55 MHz (BTW, if you look at the definitions in the source > code and make the calculations that is exactly the sampling frequency > all currently supported in Linux devices are using as well). > > also, I spent few minutes time to make the calculations based on the > restrains of the PLL build-in tda10048 and in theory the maximum is 69 > MHz. so, if you make next revision of this patch, feel free to update > the comment accordingly, in short - recommended sampling frequency of > 55 MHz, theoretical maximum of 69 MHz. > > in any case, your assumption is correct and in reality is away less > than the maximum value you assumed. I have updated the comments in my local copy. I will resend after a couple of days in case there are more reviews. May I ask if you could share the datasheet? Thanks! > > Reviewed-by: Kosta Stefanov <costa.stephanoff@gmail.com> > > --Kosta > > > On Tue, Jan 7, 2025 at 12:54 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > > > We have not been able to find the relevant datahsheet, but it seems rare > > that the device will have a sampling frequency over 613MHz. > > > > Nonetheless, this patch does not introduce any change in behaviour, it > > just adds a comment to make explicit the current limit: div by 32 bits. > > > > 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 <ribalda@chromium.org> > > --- > > 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 */ > > z = 7 * sample_freq_hz; > > do_div(t, z); > > t += 5; > > > > -- > > 2.47.1.613.gc27f4b7a9f-goog > > > >
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 */ z = 7 * sample_freq_hz; do_div(t, z); t += 5;
We have not been able to find the relevant datahsheet, but it seems rare that the device will have a sampling frequency over 613MHz. Nonetheless, this patch does not introduce any change in behaviour, it just adds a comment to make explicit the current limit: div by 32 bits. 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 <ribalda@chromium.org> --- drivers/media/dvb-frontends/tda10048.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)