Re: [PATCH v7 06/15] can: grcan: Simplify timing configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch removes the hardware-specific bounds check `ps1 > ps2` under the false assumption that it is a redundant check handled by the CAN core.
--

commit 245e4bc80a0c4a622684c9f656acd758d80ee6a0
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: Simplify timing configuration

This commit simplifies the CAN bit timing configuration by removing
error checks deemed redundant and using FIELD_PREP for bit assignments.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 8d7bde1c006f2..8ba21d94e87df 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
[ ... ]
> @@ -396,41 +396,26 @@ static const struct can_bittiming_const grcan_bittiming_const = {
>  static int grcan_set_bittiming(struct net_device *dev)
>  {
[ ... ]
>  	ps1 = (bt->prop_seg + bt->phase_seg1) - 1; /* tseg1 - 1 */
>  	ps2 = bt->phase_seg2;
> -	scaler = (bt->brp - 1);
> -	netdev_dbg(dev, "Request for BPR=%d, RSJ=%d, PS1=%d, PS2=%d, SCALER=%d",
> -		   bpr, rsj, ps1, ps2, scaler);
> -	if (!(ps1 > ps2)) {
> -		netdev_err(dev, "PS1 > PS2 must hold: PS1=%d, PS2=%d\n",
> -			   ps1, ps2);
> -		return -EINVAL;
> -	}

The commit message claims the error checks are redundant. While the
ps2 >= rsj constraint is enforced by the CAN framework via can_sjw_check(),
does the CAN core actually enforce the ps1 > ps2 constraint?

A user with network admin privileges can configure CAN standard values via
netlink that result in ps1 <= ps2. For example, configuring an unusually low
sample point can lead can_calc_bittiming() to generate parameters where
ps1 <= ps2.

If the hardware strictly requires ps1 > ps2, as the original error message
suggests, could removing this safeguard allow unsupported configurations to
be programmed into the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=6