linux-kernel - Re: [PATCH 08/10] can: grcan: Add saving and restoring of CAN FD baud-rate registers

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251211-saffron-ara-of-weather-e83dda-mkl@pengutronix.de>
Date: Thu, 11 Dec 2025 12:37:41 +0100
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Arun Muthusamy <arun.muthusamy@...sler.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	mailhol@...nel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-can@...r.kernel.org, Daniel Hellstrom <daniel@...sler.com>
Subject: Re: [PATCH 08/10] can: grcan: Add saving and restoring of CAN FD
 baud-rate registers

On 11.12.2025 10:13:15, Arun Muthusamy wrote:
> From the design point of view, I prefer to retain the "do_set_bittiming"
> callback to maintain flexibility in adjusting baud rates by the framework.

If you don't implement the do_set_bittiming callback you don't loose any
flexibility.

> Since CAN and CANFD configurations differ as they use different registers
> for timing configuration and Specifically, the timing configuration is
> closely tied to the reset logic only in scenarios where the baud rate for
> CANFD is stored in a register. This differentiation is not applicable to CAN
> timing configuration, as CAN and CANFD are handled differently.

From my point of view not implementing the do_set_bittiming makes it
easier from the driver's perspective.

Now
---

If the interface is down do_set_bittiming may be called at any time.

Consider a scenario where the device and driver support deep sleep,
power down clocks/voltages etc. .... In the do_set_bittiming callback,
you must switch on the device, write the bit timing information and
switch the device off again. Some devices lose their configuration
when they are switched off. It therefore makes no sense to implement
this callback on these devices.

What I propose
--------------

Do not implement do_set_bittiming.

If the interface is down the user can configure the bit timing. The
information is stored as usual in priv->can.bittiming,
priv->can.data_bittiming.

If the user brings up the interface the open callback is executed. In
this callback you power on the device, do a reset and then call
grcan_set_bittiming() explicitly. You don't have to take care to
preserve the timing register information around the reset.

Does this make sense?

regards,
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)