[PATCH net] can: flexcan: Fix probing for m68k/coldfire

[PATCH net] can: flexcan: Fix probing for m68k/coldfire

[Uwe Kleine-König (The Capable Hub)]

When determining the device details was reworked in commit 5e6c3454b405
("net: can: Use device_get_match_data()") there was no replacement for
the previous handling of non-of instantiated devices via the device's
id_entry which then results in a NULL pointer exception. The only
in-tree provider of such a device is arch/m68k/coldfire/device.c.

Given the id_table only contains a single entry just hardcode the result
of `platform_get_device_id(pdev)->driver_data` and drop the unused
assignment to .driver_data from the table.

While touching the id table, drop the unusual comma after the terminator
entry.

Fixes: 5e6c3454b405 ("net: can: Use device_get_match_data()")
Cc: stable@vger.kernel.org
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
Hello,

this is essentially v2 of
https://lore.kernel.org/all/20260528094540.1867783-2-u.kleine-koenig@baylibre.com/,
but that patch was intended to be a cleanup, while the discussion then
revealed that there is a crash to be fixed.

Jean-Michel Hautbois confirmed (off-list) that Marc's and my suspicion
is true and probing the flexcan device provided by
arch/m68k/coldfire/device.c indeed crashes.

Best regards
Uwe

 drivers/net/can/flexcan/flexcan-core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index f5d22c61503f..05b8ca72ca2a 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -2077,10 +2077,9 @@ MODULE_DEVICE_TABLE(of, flexcan_of_match);
 static const struct platform_device_id flexcan_id_table[] = {
 	{
 		.name = "flexcan-mcf5441x",
-		.driver_data = (kernel_ulong_t)&fsl_mcf5441x_devtype_data,
 	}, {
 		/* sentinel */
-	},
+	}
 };
 MODULE_DEVICE_TABLE(platform, flexcan_id_table);
 
@@ -2148,6 +2147,15 @@ static int flexcan_probe(struct platform_device *pdev)
 		return PTR_ERR(regs);
 
 	devtype_data = device_get_match_data(&pdev->dev);
+	if (!devtype_data)
+		/*
+		 * If the device was instantiated via the traditional platform
+		 * mechanisms (i.e. not by device tree), device_get_match_data()
+		 * returns NULL. The only remaining such device is mcf_flexcan0
+		 * (defined in arch/m68k/coldfire/device.c). Pick the right
+		 * device type for that.
+		 */
+		devtype_data = &fsl_mcf5441x_devtype_data;
 
 	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
 	    !((devtype_data->quirks &

base-commit: e2cae00c05d196491c318196792297f2dfbaa02c
-- 
2.47.3

Re: [PATCH net] can: flexcan: Fix probing for m68k/coldfire

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Network device is registered and exposed to userspace before hardware and state initialization are complete, leading to race conditions and a PM runtime underflow on probe failure.
--

commit 3c3a1287a919f302aebc8b9802a583cf3a9bb1c9
Author: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>

can: flexcan: Fix probing for m68k/coldfire

When determining the device details was reworked in commit 5e6c3454b405
("net: can: Use device_get_match_data()") there was no replacement for
the previous handling of non-of instantiated devices via the device's
id_entry which then results in a NULL pointer exception. The only
in-tree provider of such a device is arch/m68k/coldfire/device.c.

Cc: stable@vger.kernel.org

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f5d22c61503fa..05b8ca72ca2ad 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c

[ ... ]

> @@ -2148,6 +2147,15 @@ static int flexcan_probe(struct platform_device *pdev)
>  		return PTR_ERR(regs);
>  
>  	devtype_data = device_get_match_data(&pdev->dev);
> +	if (!devtype_data)
> +		/*
> +		 * If the device was instantiated via the traditional platform
> +		 * mechanisms (i.e. not by device tree), device_get_match_data()
> +		 * returns NULL. The only remaining such device is mcf_flexcan0
> +		 * (defined in arch/m68k/coldfire/device.c). Pick the right
> +		 * device type for that.
> +		 */
> +		devtype_data = &fsl_mcf5441x_devtype_data;
>  
>  	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
>  	    !((devtype_data->quirks &

[Severity: High]
This isn't a bug introduced by this patch, but later in this same function
flexcan_probe(), the network device is registered and exposed to userspace
before hardware and state initialization are complete.

Specifically, flexcan_probe() calls register_flexcandev(), which exposes the
interface to userspace via register_netdev(). Then register_flexcandev()
alters the hardware state and drops the PM runtime count:

register_flexcandev() {
	err = register_candev(dev);
	if (err)
		goto out_chip_disable;

	/* Disable core and let pm_runtime_put() disable the clocks. */
	flexcan_chip_disable(priv);
	pm_runtime_put(priv->dev);
}

Because the device is already visible, userspace can concurrently bring the
interface up, invoking flexcan_open() and flexcan_chip_start(). Doesn't this
race with the unlocked flexcan_chip_disable() call, potentially leaving the
hardware disabled while the network core considers it up?

Also in flexcan_probe(), if flexcan_setup_stop_mode() fails after the device
is registered, it goes to the failed_setup_stop_mode cleanup path:

flexcan_probe() {
	err = register_flexcandev(dev);
	...
	err = flexcan_setup_stop_mode(pdev);
	if (err < 0) {
		goto failed_setup_stop_mode;
	}

	of_can_transceiver(dev);
	...
 failed_setup_stop_mode:
	unregister_flexcandev(dev);
 failed_register:
	pm_runtime_put_noidle(&pdev->dev);
	pm_runtime_disable(&pdev->dev);
}

Since pm_runtime_put() was already called on success inside
register_flexcandev(), does the call to pm_runtime_put_noidle() in the
failed_register path cause a PM runtime usage counter underflow?

Finally, of_can_transceiver() is called after device registration to
initialize priv->can.bitrate_max. Could userspace configure the CAN bitrate
before these maximum limits are enforced?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618140147.142489-2-u.kleine-koenig@baylibre.com?part=1