Re: [PATCH v6] can: dev: can_fill_info(): don't wake up controller if interface is down

[Vincent Mailhol <mailhol@kernel.org>]

On 21/10/2026 at 04:33, ChunYuZhiQiang wrote:
> If the interface is down, the CAN controller might be powered down, the
> clock disabled, and/or its external reset asserted.
> 
> Don't call do_get_berr_counter() or do_get_auto_tdcv() to read the CAN
> controller if the interface is down. This fixes a generic logic problem
> that exists on all platforms.
> 
> As a result, IFLA_CAN_BERR_COUNTER is no longer reported for down
> interfaces, and the automatic TDCV readback is skipped. The other TDC
> parameters are still reported, as they are meaningful while the
> interface is down. This is intentional: the counters and the automatic
> TDCV are not meaningful in that state, and calling into the driver can
> wake a runtime-suspended controller or touch registers whose clocks are
> gated.
> 
> Fixes: ec56acfef2af1 ("can: flexcan: flexcan_get_berr_counter(): switch on clocks before accessing ecr register")
> Cc: stable@vger.kernel.org
> Signed-off-by: ChunYuZhiQiang <chunyuzhiqiang@yinhe.ht>
> ---
> v6:
>  - Only gate do_get_auto_tdcv() on netif_running(), as suggested by
>    Vincent Mailhol and Marc Kleine-Budde. The other TDC parameters are
>    still reported for down interfaces.
>  - Adjust can_tdc_get_size() so that only the automatic TDCV size is
>    skipped when the interface is down.
>  - Keep the berr-counter gating unchanged.
> ---
>  drivers/net/can/dev/netlink.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 6f83b87d54fc..ae0cef791ef3 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -517,7 +517,7 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>  }
>  
>  static size_t can_tdc_get_size(struct data_bittiming_params *dbt_params,
> -			       u32 tdc_flags)
> +			       u32 tdc_flags, bool auto_tdcv_running)

Nitpick: I think it is slightly better to pass dev rather than
auto_tdcv_running:

	static size_t can_tdc_get_size(const struct net_device *dev,
				       struct data_bittiming_params *dbt_params,
				       u32 tdc_flags)

The reason is that by passing dev, it would be easier in the future if
for whatever reason we need to retrieve other values. (And to be
honest, I was not going to comment just for this, but because a v7 is
needed anyway, I think you can add this small change while at it).

>  {
>  	bool tdc_manual = tdc_flags & CAN_CTRLMODE_TDC_MANUAL_MASK;
>  	size_t size;
> @@ -538,7 +538,7 @@ static size_t can_tdc_get_size(struct data_bittiming_params *dbt_params,
>  	}
>  
>  	if (tdc_flags) {
> -		if (tdc_manual || dbt_params->do_get_auto_tdcv)
> +		if (tdc_manual || (dbt_params->do_get_auto_tdcv && auto_tdcv_running))

And by passing dev, this becomes:

		if (tdc_manual || (dbt_params->do_get_auto_tdcv && netif_running(dev)))

making the pattern slightly more consistent throughout the file.

>  			size += nla_total_size(sizeof(u32));	/* IFLA_CAN_TDCV */
>  		size += nla_total_size(sizeof(u32));		/* IFLA_CAN_TDCO */
>  		if (dbt_params->tdc_const->tdcf_max)
> @@ -548,8 +548,9 @@ static size_t can_tdc_get_size(struct data_bittiming_params *dbt_params,
>  	return size;
>  }


Yours sincerely,
Vincent Mailhol