| 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
| ||
|
Message-ID: <20251112-remarkable-puzzling-fox-3b3202-mkl@pengutronix.de>
Date: Wed, 12 Nov 2025 10:08:56 +0100
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Vincent Mailhol <mailhol@...nel.org>
Cc: Oliver Hartkopp <socketcan@...tkopp.net>, linux-can@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 1/3] can: calc_bittiming: get rid of the incorrect
"nominal" word
On 02.11.2025 23:01:22, Vincent Mailhol wrote:
> The functions can_update_sample_point() and can_calc_bittiming() are
> generic and meant to be used for both the nominal and the data
> bittiming calculation.
""There are 2 hard problems in computer science: cache invalidation,
naming things, and off-by-1 errors.""
Here it's naming things. Back in the days, in commit 7da29f97d6c8 ("can:
dev: can-calc-bit-timing(): better sample point calculation"), I wanted
to distinguish between the sample point the user requested and the
current sample point.
I was thinking about the signal that goes into a control loops, but at
university the lecture was in German, so I picked the wrong term. I
think "set point" or "reference value" are better terms.
> However, those functions use terminologies such as "bitrate nominal"
> or "sample point nominal". This is a leftover from when only Classical
> CAN was supported and now became incorrect.
>
> Remove or replace any occurrences of the word "nominal" with something
> more accurate.
What about replacing "nominal" with "reference"
> Signed-off-by: Vincent Mailhol <mailhol@...nel.org>
> ---
> drivers/net/can/dev/calc_bittiming.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
> index 268ec6fa7c49..222117596704 100644
> --- a/drivers/net/can/dev/calc_bittiming.c
> +++ b/drivers/net/can/dev/calc_bittiming.c
> @@ -24,7 +24,7 @@
> */
> static int
> can_update_sample_point(const struct can_bittiming_const *btc,
> - const unsigned int sample_point_nominal, const unsigned int tseg,
> + unsigned int sp_origin, unsigned int tseg,
Please don't remove the "const".
> unsigned int *tseg1_ptr, unsigned int *tseg2_ptr,
> unsigned int *sample_point_error_ptr)
> {
> @@ -35,8 +35,7 @@ can_update_sample_point(const struct can_bittiming_const *btc,
>
> for (i = 0; i <= 1; i++) {
> tseg2 = tseg + CAN_SYNC_SEG -
> - (sample_point_nominal * (tseg + CAN_SYNC_SEG)) /
> - 1000 - i;
> + (sp_origin * (tseg + CAN_SYNC_SEG)) / 1000 - i;
> tseg2 = clamp(tseg2, btc->tseg2_min, btc->tseg2_max);
> tseg1 = tseg - tseg2;
> if (tseg1 > btc->tseg1_max) {
> @@ -46,9 +45,9 @@ can_update_sample_point(const struct can_bittiming_const *btc,
>
> sample_point = 1000 * (tseg + CAN_SYNC_SEG - tseg2) /
> (tseg + CAN_SYNC_SEG);
> - sample_point_error = abs(sample_point_nominal - sample_point);
> + sample_point_error = abs(sp_origin - sample_point);
>
> - if (sample_point <= sample_point_nominal &&
> + if (sample_point <= sp_origin &&
> sample_point_error < best_sample_point_error) {
> best_sample_point = sample_point;
> best_sample_point_error = sample_point_error;
> @@ -68,11 +67,11 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
> {
> struct can_priv *priv = netdev_priv(dev);
> unsigned int bitrate; /* current bitrate */
> - unsigned int bitrate_error; /* difference between current and nominal value */
> + unsigned int bitrate_error; /* difference between current and calculated value */
What about: "difference between reference and calculated value"
> unsigned int best_bitrate_error = UINT_MAX;
> - unsigned int sample_point_error; /* difference between current and nominal value */
> + unsigned int sample_point_error; /* difference between current and calculated value */
> unsigned int best_sample_point_error = UINT_MAX;
> - unsigned int sample_point_nominal; /* nominal sample point */
> + unsigned int sample_point;
> unsigned int best_tseg = 0; /* current best value for tseg */
> unsigned int best_brp = 0; /* current best value for brp */
> unsigned int brp, tsegall, tseg, tseg1 = 0, tseg2 = 0;
> @@ -81,14 +80,14 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
>
> /* Use CiA recommended sample points */
> if (bt->sample_point) {
> - sample_point_nominal = bt->sample_point;
> + sample_point = bt->sample_point;
> } else {
> if (bt->bitrate > 800 * KILO /* BPS */)
> - sample_point_nominal = 750;
> + sample_point = 750;
> else if (bt->bitrate > 500 * KILO /* BPS */)
> - sample_point_nominal = 800;
> + sample_point = 800;
> else
> - sample_point_nominal = 875;
> + sample_point = 875;
> }
>
> /* tseg even = round down, odd = round up */
> @@ -115,7 +114,7 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
> if (bitrate_error < best_bitrate_error)
> best_sample_point_error = UINT_MAX;
>
> - can_update_sample_point(btc, sample_point_nominal, tseg / 2,
> + can_update_sample_point(btc, sample_point, tseg / 2,
> &tseg1, &tseg2, &sample_point_error);
> if (sample_point_error >= best_sample_point_error)
> continue;
> @@ -146,9 +145,8 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
> }
>
> /* real sample point */
> - bt->sample_point = can_update_sample_point(btc, sample_point_nominal,
> - best_tseg, &tseg1, &tseg2,
> - NULL);
> + bt->sample_point = can_update_sample_point(btc, sample_point, best_tseg,
> + &tseg1, &tseg2, NULL);
>
> v64 = (u64)best_brp * 1000 * 1000 * 1000;
> do_div(v64, priv->clock.freq);
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists