Message ID | 20240429-fix-cocci-v3-26-3c4865f5a4b0@chromium.org (mailing list archive) |
---|---|
State | New |
Delegated to: | Hans Verkuil |
Headers |
Received: from sv.mirrors.kernel.org ([139.178.88.99]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-10356-patchwork=linuxtv.org@vger.kernel.org>) id 1s1Sfk-0006za-05 for patchwork@linuxtv.org; Mon, 29 Apr 2024 15:12:04 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 7F54128455E for <patchwork@linuxtv.org>; Mon, 29 Apr 2024 15:12:03 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id F220712DDBF; Mon, 29 Apr 2024 15:05:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="cs/Z3XNn" X-Original-To: linux-media@vger.kernel.org Received: from mail-qk1-f169.google.com (mail-qk1-f169.google.com [209.85.222.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E25A212BF08 for <linux-media@vger.kernel.org>; Mon, 29 Apr 2024 15:05:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.169 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714403124; cv=none; b=OWXoVuGya5RxApxLyy1TcXmARxz4IBT+hWo/JyR/bIvxS7aMBdQlGvy2FK1Z+ysbj06pZGqFcJis2Zhy+yL9P7YLJZU6y9OIQmHJjVti3nMMywJzXup26YakfQwu561opxR3a8hK246jpVIaAiwYj7t1oaE1vKZpgqPEQGTzxlA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714403124; c=relaxed/simple; bh=17e3seaVPIolCVRnz9dvrXMTILwJmsuH1BK6VcvWdM0=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=iZiCYnUEqu64VFB+Fd79fri1NYFndsIofp7rC6v7e/MxS67U25ysYkq5wmTW4rCj+ZBLHyjt+Hi1fEb9kUwK785Gr9RnfIjgoptm+G5dbTQfgxKMjImc6vV9JdiVpEhzCegESAlUuNQgqIDg9J2dO27pjEVZjSvBfoabA78GsMs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=cs/Z3XNn; arc=none smtp.client-ip=209.85.222.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Received: by mail-qk1-f169.google.com with SMTP id af79cd13be357-790eedf44faso116876285a.3 for <linux-media@vger.kernel.org>; Mon, 29 Apr 2024 08:05:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1714403111; x=1715007911; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=STqazHEagBxWlHC69lQFp5WbvQ2Sdx8rB6+uKcUB0cU=; b=cs/Z3XNnHKQx1vr6EMlWwKFDUVk+wBR22JAaohZkk/k3TKtqbsfKePvDSqEa6SOO2O r20oF1/L/C4ljsjF5+o1uIoiXJsiVNRWNMXvQgf6J+Icu248tynLoEiMDP77WvTKyGd7 CvXvXriOxlgYpIGgAgxhsMEwehNZfeTk7IejQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714403111; x=1715007911; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=STqazHEagBxWlHC69lQFp5WbvQ2Sdx8rB6+uKcUB0cU=; b=bDlW071fy+3OltiLPyVJWfHdz50AE4YyaFzSp6rmIb/R6QtO+aYUfFrVu7HRTWUcPY UU/CUu/kahzHabW0mpFw1smFm+2c7Qrim4PqYj2ObvUVAh4ABwBaUoRoO2Umf6HT+a+y Z0J7PlEfRVmFlRSG0kh12H0zzfcCvnOAwe6kdkC6n5vRvZ9RWsO+sIb9aHLznuqD2d6H rIZq6nevplYJV1m/C582d5YqNo8Q1aD0upKVM2ym/og8ilYVKmX2RHHEOVd76CNIDcde 8f/YkYs9V1aYsTaKMzpnuSssQwUEH8Rqj++mg5TV7e95Tv27xPFQH6K6lQOvL7Rhmm0n QJgQ== X-Gm-Message-State: AOJu0Yy3yZbARZzoXZReeL8X980qafNQYVh8NVfjzZ/ZxQ5vBkxS6Dy3 gqqXsNsIi6nG4zoraO2Ta7ATFhPsGBEAgrfhpc9r0tI/RSO3xkyLgnrXHbKPOA== X-Google-Smtp-Source: AGHT+IHvPitwm/34xW/kQUa8TA8n6UCg3fwO9ex2lf7MaH+MC5XxlTHiUGq0Riy9QnEe78dxXREmpg== X-Received: by 2002:a05:620a:5e48:b0:790:c81e:7840 with SMTP id ya8-20020a05620a5e4800b00790c81e7840mr6919928qkn.12.1714403111405; Mon, 29 Apr 2024 08:05:11 -0700 (PDT) Received: from denia.c.googlers.com (114.152.245.35.bc.googleusercontent.com. [35.245.152.114]) by smtp.gmail.com with ESMTPSA id p7-20020a05620a056700b0078d3b9139edsm10568591qkp.97.2024.04.29.08.05.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Apr 2024 08:05:11 -0700 (PDT) From: Ricardo Ribalda <ribalda@chromium.org> Date: Mon, 29 Apr 2024 15:05:05 +0000 Subject: [PATCH v3 26/26] media: dvb-frontends: tda10048: Make the range of z explicit. Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20240429-fix-cocci-v3-26-3c4865f5a4b0@chromium.org> References: <20240429-fix-cocci-v3-0-3c4865f5a4b0@chromium.org> In-Reply-To: <20240429-fix-cocci-v3-0-3c4865f5a4b0@chromium.org> To: Martin Tuma <martin.tuma@digiteqautomotive.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Hugues Fruchet <hugues.fruchet@foss.st.com>, Alain Volmat <alain.volmat@foss.st.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Paul Kocialkowski <paul.kocialkowski@bootlin.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Samuel Holland <samuel@sholland.org>, Sakari Ailus <sakari.ailus@linux.intel.com>, Thierry Reding <thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, Sowjanya Komatineni <skomatineni@nvidia.com>, Luca Ceresoli <luca.ceresoli@bootlin.com>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, Hans Verkuil <hverkuil@xs4all.nl>, Sergey Kozlov <serjk@netup.ru>, Abylay Ospan <aospan@netup.ru>, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>, Dmitry Osipenko <digetx@gmail.com>, Benjamin Mugnier <benjamin.mugnier@foss.st.com>, Sylvain Petinot <sylvain.petinot@foss.st.com>, Stanimir Varbanov <stanimir.k.varbanov@gmail.com>, Vikash Garodia <quic_vgarodia@quicinc.com>, Bryan O'Donoghue <bryan.odonoghue@linaro.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org> Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-staging@lists.linux.dev, linux-sunxi@lists.linux.dev, linux-tegra@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-arm-msm@vger.kernel.org, Ricardo Ribalda <ribalda@chromium.org> X-Mailer: b4 0.12.4 X-LSpam-Score: -5.9 (-----) X-LSpam-Report: No, score=-5.9 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIMWL_WL_HIGH=-1,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_PASS=-0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_MED=-2.3,SPF_HELO_NONE=0.001,SPF_PASS=-0.001 autolearn=ham autolearn_force=no |
Series |
media: Fix coccinelle warning/errors
|
|
Commit Message
Ricardo Ribalda
April 29, 2024, 3:05 p.m. UTC
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 <ribalda@chromium.org>
---
drivers/media/dvb-frontends/tda10048.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Comments
Em Mon, 29 Apr 2024 15:05:05 +0000 Ricardo Ribalda <ribalda@chromium.org> escreveu: > 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 <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 */ 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. If you're sure, please add a note at the description mentioning on what part of the datasheet you got it. Otherwise, let's stick with the current code and address cocci warning on a different way. Regards, Mauro PS.: I partially applied this patch series. I left a few patches out of the merge to let other people review/comment (and/or for me to take a deeper look later on). Regards, Mauro
On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote: > Em Mon, 29 Apr 2024 15:05:05 +0000 > Ricardo Ribalda <ribalda@chromium.org> escreveu: > > > 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 <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 */ > > 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. 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
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? Thanks! On Fri, 3 May 2024 at 13:55, Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote: > > Em Mon, 29 Apr 2024 15:05:05 +0000 > > Ricardo Ribalda <ribalda@chromium.org> escreveu: > > > > > 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 <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 */ > > > > 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. > > 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 >
Hello Ricardo, On 2024-05-03 13:56, Ricardo Ribalda wrote: > 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 > On Fri, 3 May 2024 at 13:55, Dan Carpenter <dan.carpenter@linaro.org> > wrote: >> >> On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote: >> > Em Mon, 29 Apr 2024 15:05:05 +0000 >> > Ricardo Ribalda <ribalda@chromium.org> escreveu: >> > >> > > 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 <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 */ >> > >> > 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. >> >> 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 >>
On Fri, 3 May 2024 at 16:08, Dragan Simic <dsimic@manjaro.org> wrote: > > Hello Ricardo, > > On 2024-05-03 13:56, Ricardo Ribalda wrote: > > 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? > > > > On Fri, 3 May 2024 at 13:55, Dan Carpenter <dan.carpenter@linaro.org> > > wrote: > >> > >> On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote: > >> > Em Mon, 29 Apr 2024 15:05:05 +0000 > >> > Ricardo Ribalda <ribalda@chromium.org> escreveu: > >> > > >> > > 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 <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 */ > >> > > >> > 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. > >> > >> 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 > >>
On Mon, May 13, 2024 at 9:38 AM Ricardo Ribalda <ribalda@chromium.org> wrote: > > On Fri, 3 May 2024 at 16:08, Dragan Simic <dsimic@manjaro.org> wrote: > > > > Hello Ricardo, > > > > On 2024-05-03 13:56, Ricardo Ribalda wrote: > > > 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? > > > > > > > > > On Fri, 3 May 2024 at 13:55, Dan Carpenter <dan.carpenter@linaro.org> > > > wrote: > > >> > > >> On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote: > > >> > Em Mon, 29 Apr 2024 15:05:05 +0000 > > >> > Ricardo Ribalda <ribalda@chromium.org> escreveu: > > >> > > > >> > > 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 <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 */ > > >> > > > >> > 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. > > > > >> > > >> 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 > Nice to hear from you! :-) I believe that I may have a copy of it on an old "spinny" hard drive somewhere in one of the ancient desktop computers I have lining my basement walls, lol. It will take me some time to locate it. I hope this isn't urgent o:-) ...It so happens that the dev box I used when I worked on that driver is up right now, but the datasheet isn't in my home directory. There are two other drives in the chassis but not connected / powered - I'll give these a look and let you know if I find anything. Best, Michael Krufky
Hi It does not seem that we will find the datasheet. Any objection to land the patch as it? It is as incorrect as the original code ;) Regards! On Mon, 13 May 2024 at 16:26, Michael Ira Krufky <mkrufky@linuxtv.org> wrote: > > On Mon, May 13, 2024 at 9:38 AM Ricardo Ribalda <ribalda@chromium.org> wrote: > > > > On Fri, 3 May 2024 at 16:08, Dragan Simic <dsimic@manjaro.org> wrote: > > > > > > Hello Ricardo, > > > > > > On 2024-05-03 13:56, Ricardo Ribalda wrote: > > > > 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? > > > > > > > > > > > > > > On Fri, 3 May 2024 at 13:55, Dan Carpenter <dan.carpenter@linaro.org> > > > > wrote: > > > >> > > > >> On Fri, May 03, 2024 at 11:27:58AM +0100, Mauro Carvalho Chehab wrote: > > > >> > Em Mon, 29 Apr 2024 15:05:05 +0000 > > > >> > Ricardo Ribalda <ribalda@chromium.org> escreveu: > > > >> > > > > >> > > 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 <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 */ > > > >> > > > > >> > 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. > > > > > > > >> > > > >> 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 > > > > Nice to hear from you! :-) > > I believe that I may have a copy of it on an old "spinny" hard drive > somewhere in one of the ancient desktop computers I have lining my > basement walls, lol. It will take me some time to locate it. I hope > this isn't urgent o:-) > > ...It so happens that the dev box I used when I worked on that driver > is up right now, but the datasheet isn't in my home directory. There > are two other drives in the chassis but not connected / powered - I'll > give these a look and let you know if I find anything. > > Best, > Michael Krufky
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;