From: Daniel Hellstrom <daniel@gaisler.com>

While reset the GRCAN baud-rates are preserved, since GRCANFD has the
baud-rate in different registers we need to add saving of those
registers too.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/can/grcan.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 51a10fae2faf..b0e2367fb163 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -589,9 +589,19 @@ static void grcan_reset(struct net_device *dev)
 	struct grcan_priv *priv = netdev_priv(dev);
 	struct grcan_registers __iomem *regs = priv->regs;
 	u32 config = grcan_read_reg(&regs->conf);
+	u32 nbtr, fdbtr;
+
+	if (priv->hwcap->fd) {
+		nbtr = grcan_read_reg(&regs->nbtr);
+		fdbtr = grcan_read_reg(&regs->fdbtr);
+	}

 	grcan_set_bits(&regs->ctrl, GRCAN_CTRL_RESET);
 	grcan_write_reg(&regs->conf, config);
+	if (priv->hwcap->fd) {
+		grcan_write_reg(&regs->nbtr, nbtr);
+		grcan_write_reg(&regs->fdbtr, fdbtr);
+	}

 	priv->eskbp = grcan_read_reg(&regs->txrd);
 	priv->can.state = CAN_STATE_STOPPED;
--
2.51.0

On 18.11.2025 10:21:13, Arun Muthusamy wrote:
> From: Daniel Hellstrom <daniel@gaisler.com>
>
> While reset the GRCAN baud-rates are preserved, since GRCANFD has the
> baud-rate in different registers we need to add saving of those
> registers too.

What about removing the do_set_bittiming callback

	priv->can.do_set_bittiming = grcan_set_bittiming;

and calling grcan_set_bittiming() explicitly after the reset?

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   |

Thank you for your suggestion.

 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. 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.

-- 
BR,

Arun Muthusamy
Software Engineer
Frontgrade Gaisler
T : +46 (0) 700 558 528
arun.muthusamy@gaisler.com

Frontgrade Gaisler AB, Kungsgatan 12, SE-411 19 GÖTEBORG, Sweden.
+46 (0) 31 775 8650, www.gaisler.com

On 21.11.2025 13:50, Marc Kleine-Budde wrote:
> On 18.11.2025 10:21:13, Arun Muthusamy wrote:
>> From: Daniel Hellstrom <daniel@gaisler.com>
>> 
>> While reset the GRCAN baud-rates are preserved, since GRCANFD has the
>> baud-rate in different registers we need to add saving of those
>> registers too.
> 
> What about removing the do_set_bittiming callback
> 
> 	priv->can.do_set_bittiming = grcan_set_bittiming;
> 
> and calling grcan_set_bittiming() explicitly after the reset?
> 
> regards,
> Marc

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   |