Re: [PATCH] can: flexcan: simplify the calculation of priv->mb_count

From: Vincent Mailhol
Date: Mon Nov 04 2024 - 05:33:07 EST


Hi Liu,

On Mon. 4 Nov. 2024 at 18:05, baozhu.liu <lucas.liu@xxxxxxxxxxxx> wrote:
> Since mb is a fixed-size two-dimensional array (u8 mb[2][512]),
> "priv->mb_count = sizeof(priv->regs->mb)/priv->mb_size;",
> this expression calculates mb_count correctly and is more concise.

When using integers,

(a1 / q) + (a2 / q)

is not necessarily equal to

(a1 + a2) / q.


If the decimal place of

sizeof(priv->regs->mb[0]) / priv->mb_size

were to be greater than or equal to 0.5, the result would have changed
because of the rounding.

This is illustrated in https://godbolt.org/z/bfnhKcKPo.

Here, luckily enough, the two valid results are, for CAN CC:

sizeof(priv->regs->mb[0]) / priv->mb_size = 512 / 16
= 64

and for CAN FD:

sizeof(priv->regs->mb[0]) / priv->mb_size = 512 / 72
= 14.22

and both of these have no rounding issues.

I am not sure if we should take this patch. It is correct at the end
because you "won a coin flip", but the current code is doing the
correct logical calculation which would always yield the correct
result regardless of the rounding.

> Signed-off-by: baozhu.liu <lucas.liu@xxxxxxxxxxxx>
> ---
> drivers/net/can/flexcan/flexcan-core.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 6d638c939..e3a8bad21 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -1371,8 +1371,7 @@ static int flexcan_rx_offload_setup(struct net_device *dev)
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_MB_16)
> priv->mb_count = 16;
> else
> - priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
> - (sizeof(priv->regs->mb[1]) / priv->mb_size);
> + priv->mb_count = sizeof(priv->regs->mb) / priv->mb_size;
>
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_USE_RX_MAILBOX)
> priv->tx_mb_reserved =

Yours sincerely,
Vincent Mailhol