[PATCH v2 0/1] can: mcp251x: Add ACPI device ID table for MCP2515

[PATCH v2 0/1] can: mcp251x: Add ACPI device ID table for MCP2515

[guoqi0226]

Changes in v2:
1. MCP2515 is verified, but MCP251XFD has not been tested.
   When testing MCP2515, MCP251XFD was found missing ACPI support, hence this addition.
   Reference the validated MCP2515 ACPI node as template:
   Device(CAN0){
         Name(_HID, "MCP2515")
         Name(_UID, 0)
         Name (_DSD, Package() {
2. The original 9-character HID "MCP251XFD" violates ACPI format rules; 
   redefine it to standard compliant ACPI ID MCHP15FD.

guoqi0226 (1):
  can: mcp251xfd: Add ACPI device ID table for MCP251XFD

 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

-- 
2.25.1

[PATCH v2] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[guoqi0226]

This patch adds an ACPI match table for the MCP251XFD CAN controller,
enabling driver binding on ACPI-enumerated SPI platforms.

Signed-off-by: guoqi0226 <guoqi0226@163.com>
---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 92a86083c896..031967370728 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
+#include <linux/acpi.h>
 
 #include "mcp251xfd.h"
 
@@ -982,7 +983,6 @@ static int mcp251xfd_handle_rxovif(struct mcp251xfd_priv *priv)
 
 static int mcp251xfd_handle_txatif(struct mcp251xfd_priv *priv)
 {
-	netdev_info(priv->ndev, "%s\n", __func__);
 
 	return 0;
 }
@@ -2212,6 +2212,14 @@ static const struct spi_device_id mcp251xfd_id_table[] = {
 };
 MODULE_DEVICE_TABLE(spi, mcp251xfd_id_table);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id mcp251xfd_acpi_match[] = {
+	{ "MCHP15FD", (kernel_ulong_t)&mcp251xfd_devtype_data_mcp251xfd},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, mcp251xfd_acpi_match);
+#endif
+
 static int mcp251xfd_probe(struct spi_device *spi)
 {
 	struct net_device *ndev;
@@ -2442,6 +2450,7 @@ static struct spi_driver mcp251xfd_driver = {
 		.name = DEVICE_NAME,
 		.pm = &mcp251xfd_pm_ops,
 		.of_match_table = mcp251xfd_of_match,
+		.acpi_match_table = ACPI_PTR(mcp251xfd_acpi_match),
 	},
 	.probe = mcp251xfd_probe,
 	.remove = mcp251xfd_remove,
-- 
2.25.1

Re: [PATCH v2] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[sashiko-bot]

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

New issues:
- [Medium] Undocumented removal of `netdev_info` print in `mcp251xfd_handle_txatif`.

Pre-existing issues:
- [High] Infinite loop / interrupt storm in `mcp251xfd_irq` due to unhandled `TXATIF` interrupt flag.
- [High] NULL pointer dereference when retrieving match data if the device is bound manually via sysfs `driver_override`.
--

commit ac99a03d44f8633aff78ad69c67bdd4c8da1aa29
Author: guoqi0226 <guoqi0226@163.com>

can: mcp251xfd: Add ACPI device ID table for MCP251XFD

This patch adds an ACPI match table for the MCP251XFD CAN controller,
enabling driver binding on ACPI-enumerated SPI platforms.

> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index 92a86083c8962..0319673707289 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c

[ ... ]

> @@ -982,7 +983,6 @@ static int mcp251xfd_handle_rxovif(struct mcp251xfd_priv *priv)
>  
>  static int mcp251xfd_handle_txatif(struct mcp251xfd_priv *priv)
>  {
> -	netdev_info(priv->ndev, "%s\n", __func__);
>  
>  	return 0;
>  }

[Severity: Medium]
Is this removal intentional? The commit message only mentions adding an
ACPI match table, but this modifies the interrupt handler by removing
a debug print. If it is intentional, should it be mentioned in the
commit message?

[Severity: High]
This is a pre-existing issue, but does this empty function cause an
infinite interrupt loop?

Because TXATIF is not part of MCP251XFD_REG_INT_IF_CLEARABLE_MASK, the
hardware flag must be manually cleared in the FIFOSTA register. Since
mcp251xfd_handle_txatif() returns 0 without clearing the flag, won't
the do-while loop in mcp251xfd_irq() continually read TXATIF as pending
and loop infinitely?

[ ... ]

> @@ -2212,6 +2212,14 @@ static const struct spi_device_id mcp251xfd_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(spi, mcp251xfd_id_table);
>  
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id mcp251xfd_acpi_match[] = {
> +	{ "MCHP15FD", (kernel_ulong_t)&mcp251xfd_devtype_data_mcp251xfd},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, mcp251xfd_acpi_match);
> +#endif
> +
>  static int mcp251xfd_probe(struct spi_device *spi)
>  {
>  	struct net_device *ndev;

[Severity: High]
This is a pre-existing issue, but is it safe to unconditionally
dereference the match data retrieved in this function?

Later in mcp251xfd_probe(), the driver does:

priv->devtype_data = *(struct mcp251xfd_devtype_data *)spi_get_device_match_data(spi);

If the driver is bound manually via the sysfs driver_override attribute
without matching any table entries, spi_get_device_match_data() will
return NULL, which would cause a panic when dereferenced. Should there
be a check for NULL before dereferencing it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616085359.89406-1-guoqi0226@163.com?part=1

[PATCH v3 0/1] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[guoqi0226]

Sent incorrect patch accidentally, fixed in this version

changed v3
Recover accidentally deleted netdev_info log line with __func__
-	netdev_info(priv->ndev, "%s\n", __func__);

guoqi0226 (1):
  can: mcp251xfd: Add ACPI device ID table for MCP251XFD

 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

-- 
2.25.1

[PATCH v3 1/1] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[guoqi0226]

This patch adds an ACPI match table for the MCP251XFD CAN controller,
enabling driver binding on ACPI-enumerated SPI platforms.

Signed-off-by: guoqi0226 <guoqi0226@163.com>
---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 92a86083c896..0cde98f51f79 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
+#include <linux/acpi.h>
 
 #include "mcp251xfd.h"
 
@@ -2212,6 +2213,14 @@ static const struct spi_device_id mcp251xfd_id_table[] = {
 };
 MODULE_DEVICE_TABLE(spi, mcp251xfd_id_table);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id mcp251xfd_acpi_match[] = {
+	{ "MCHP15FD", (kernel_ulong_t)&mcp251xfd_devtype_data_mcp251xfd},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, mcp251xfd_acpi_match);
+#endif
+
 static int mcp251xfd_probe(struct spi_device *spi)
 {
 	struct net_device *ndev;
@@ -2442,6 +2451,7 @@ static struct spi_driver mcp251xfd_driver = {
 		.name = DEVICE_NAME,
 		.pm = &mcp251xfd_pm_ops,
 		.of_match_table = mcp251xfd_of_match,
+		.acpi_match_table = ACPI_PTR(mcp251xfd_acpi_match),
 	},
 	.probe = mcp251xfd_probe,
 	.remove = mcp251xfd_remove,
-- 
2.25.1

Re: [PATCH v3 1/1] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[sashiko-bot]

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

Pre-existing issues:
- [High] Unconditional dereference of the pointer returned by spi_get_device_match_data() can cause a NULL pointer dereference.
--

commit 984f1d631588f571112e2f97f02bc97fcb1d8ec1
Author: guoqi0226 <guoqi0226@163.com>

can: mcp251xfd: Add ACPI device ID table for MCP251XFD

This patch adds an ACPI match table for the MCP251XFD CAN controller,
enabling driver binding on ACPI-enumerated SPI platforms.

> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index 92a86083c8962..0cde98f51f797 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
[ ... ]
> @@ -2442,6 +2451,7 @@ static struct spi_driver mcp251xfd_driver = {
>  		.name = DEVICE_NAME,
>  		.pm = &mcp251xfd_pm_ops,
>  		.of_match_table = mcp251xfd_of_match,
> +		.acpi_match_table = ACPI_PTR(mcp251xfd_acpi_match),
>  	},
>  	.probe = mcp251xfd_probe,

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing issue
in mcp251xfd_probe() that might cause a kernel panic.

If the SPI device is bound to the driver manually via sysfs (for example, by
writing to the driver_override and bind files), spi_get_device_match_data()
can return NULL.

In mcp251xfd_probe(), the return value is dereferenced unconditionally:

static int mcp251xfd_probe(struct spi_device *spi)
{
        ...
        priv->devtype_data = *(struct mcp251xfd_devtype_data *)spi_get_device_match_data(spi);
        ...
}

Could we add a NULL check here that returns -ENODEV or falls back to a
default configuration to prevent the dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616091640.93275-1-guoqi0226@163.com?part=1

[PATCH v4 0/2] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[guoqi0226]

hanges in v4:
- Split into two separate patches:
  1. SPI core: add internal NULL guard for spi_get_device_match_data()
  2. mcp251xfd driver: cache and validate match data pointer in probe
- Remove duplicate spi_get_device_match_data() invocation and unsafe dereference

guoqi0226 (2):
  can: mcp251xfd: mcp251xfd: Cache and validate match data pointer in
    probe
  spi: Add NULL check for spi_get_device_id() in
    spi_get_device_match_data()

 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 8 +++++++-
 drivers/spi/spi.c                              | 6 +++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] can: mcp251xfd: mcp251xfd: Cache and validate match data pointer in probe

[guoqi0226]

Cache the result of spi_get_device_match_data() to avoid duplicate calls.
Add NULL check before dereferencing to eliminate unsafe raw pointer access.

Signed-off-by: guoqi0226 <guoqi0226@163.com>
---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 0cde98f51f79..7b50228602f4 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -2231,6 +2231,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	bool pll_enable = false;
 	u32 freq = 0;
 	int err;
+	const struct mcp251xfd_devtype_data *match_data;
 
 	if (!spi->irq)
 		return dev_err_probe(&spi->dev, -ENXIO,
@@ -2282,6 +2283,11 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER)
 		pll_enable = true;
 
+	match_data = spi_get_device_match_data(spi);
+	if (!match_data)
+		return dev_err_probe(&spi->dev, -ENODEV,
+				     "Failed to retrieve device match data\n");
+
 	ndev = alloc_candev(sizeof(struct mcp251xfd_priv),
 			    MCP251XFD_TX_OBJ_NUM_MAX);
 	if (!ndev)
@@ -2317,7 +2323,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	priv->reg_vdd = reg_vdd;
 	priv->reg_xceiver = reg_xceiver;
 	priv->xstbyen = device_property_present(&spi->dev, "microchip,xstbyen");
-	priv->devtype_data = *(struct mcp251xfd_devtype_data *)spi_get_device_match_data(spi);
+	priv->devtype_data = *match_data;
 
 	/* Errata Reference:
 	 * mcp2517fd: DS80000792C 5., mcp2518fd: DS80000789E 4.,
-- 
2.25.1

[PATCH v4 2/2] spi: Add NULL check for spi_get_device_id() in spi_get_device_match_data()

[guoqi0226]

Prevent NULL pointer dereference when spi_get_device_id() returns NULL,
which can happen when using driver_override without matching SPI ID entry.

Signed-off-by: guoqi0226 <guoqi0226@163.com>
---
 drivers/spi/spi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 104279858f56..079ac01a2229 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -355,12 +355,16 @@ EXPORT_SYMBOL_GPL(spi_get_device_id);
 const void *spi_get_device_match_data(const struct spi_device *sdev)
 {
 	const void *match;
+	const struct spi_device_id *id;
 
 	match = device_get_match_data(&sdev->dev);
 	if (match)
 		return match;
 
-	return (const void *)spi_get_device_id(sdev)->driver_data;
+	id = spi_get_device_id(sdev);
+	if (!id)
+		return NULL;
+	return (const void *)id->driver_data;
 }
 EXPORT_SYMBOL_GPL(spi_get_device_match_data);
 
-- 
2.25.1

Re: [PATCH v4 2/2] spi: Add NULL check for spi_get_device_id() in spi_get_device_match_data()

[sashiko-bot]

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

Pre-existing issues:
- [High] spi_get_device_id() unconditionally dereferences a potentially NULL id_table pointer
- [High] Direct callers of spi_get_device_id() unconditionally dereference its result
- [High] Callers of the patched spi_get_device_match_data() unconditionally dereference its NULL return value
--

commit 7e155f379f89eda6ab91795ac56410fd02c722be
Author: guoqi0226 <guoqi0226@163.com>

spi: Add NULL check for spi_get_device_id() in spi_get_device_match_data()

Prevent NULL pointer dereference when spi_get_device_id() returns NULL,
which can happen when using driver_override without matching SPI ID entry.

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 104279858f567..079ac01a2229d 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -355,12 +355,16 @@ EXPORT_SYMBOL_GPL(spi_get_device_id);
>  const void *spi_get_device_match_data(const struct spi_device *sdev)
>  {
>  	const void *match;
> +	const struct spi_device_id *id;
>  
>  	match = device_get_match_data(&sdev->dev);
>  	if (match)
>  		return match;
>  
> -	return (const void *)spi_get_device_id(sdev)->driver_data;
> +	id = spi_get_device_id(sdev);

[Severity: High]
This is a pre-existing issue, but can spi_get_device_id() itself crash
if sdrv->id_table is NULL?

If a spi driver only provides a device tree or acpi match table and lacks
an id_table, sdrv->id_table will evaluate to NULL. When spi_get_device_id()
is called, it passes this NULL pointer to spi_match_id():

drivers/spi/spi.c:spi_get_device_id() {
    ...
    return spi_match_id(sdrv->id_table, sdev->modalias);
}

spi_match_id() then unconditionally evaluates id->name[0] in its while loop:

drivers/spi/spi.c:spi_match_id() {
    ...
    while (id->name[0]) {
    ...
}

Would this result in a NULL pointer dereference when driver_override is used?

[Severity: High]
This is also a pre-existing issue, but do many direct callers of
spi_get_device_id() unconditionally dereference its result?

For example, in drivers/iio/adc/max1118.c:max1118_probe():

    indio_dev->name = spi_get_device_id(spi)->name;

If driver_override is used to force a bind to a device whose modalias
does not match the driver's id_table, spi_get_device_id() can return NULL.
Does this mean individual spi drivers might immediately crash if they
don't check the return value?

> +	if (!id)
> +		return NULL;

[Severity: High]
This isn't a bug introduced by this patch, but will returning NULL here just
shift the crash to callers that don't check the return value?

For example, in drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe():

    smd->mode = spi_get_device_match_data(spi);
    ...
    smd->pitch = (SHARP_ADDR_PERIOD + smd->mode->hdisplay + ...) / 8;

If spi_get_device_match_data() returns NULL when driver_override is used,
will callers unconditionally dereference it and still cause a kernel panic?

> +	return (const void *)id->driver_data;
>  }
>  EXPORT_SYMBOL_GPL(spi_get_device_match_data);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616103018.105612-1-guoqi0226@163.com?part=2

Re: [PATCH v4 0/2] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

On Tue, Jun 16, 2026 at 06:30:16PM +0800, guoqi0226 wrote:
> hanges in v4:
> - Split into two separate patches:
>   1. SPI core: add internal NULL guard for spi_get_device_match_data()
>   2. mcp251xfd driver: cache and validate match data pointer in probe
> - Remove duplicate spi_get_device_match_data() invocation and unsafe dereference

Why is this a series, are there any dependencies between these two
patches?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Re: [PATCH v4 0/2] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1056 bytes --]

On Tue, Jun 16, 2026 at 06:47:06PM +0800, 崔国旗  wrote:

> Hi Mark, While fixing the mcp251xfd driver, I found a latent flaw in spi_get_device_match_data(), so I sent both fixes as one patch series for unified review. There is no hard build or runtime dependency between the two patches. They can be merged separately into the SPI and CAN subsystem trees respectively. Thanks, guoqi

That's not a good idea since it creates a cross subsystem series which
means the flow for getting things merged gets a lot less clear.  It is
better to keep things for different subsystems separate since that's a
lot easier to manage.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 0/2] can: mcp251xfd: Add ACPI device ID table for MCP251XFD

[guoqi0226]

While fixing the mcp251xfd driver, I found a latent flaw in spi_get_device_match_data(), 
so I sent both fixes as one patch series for unified review.
There is no hard build or runtime dependency between the two patches. 
They can be merged separately into the SPI and CAN subsystem trees respectively.

Thanks,
guoqi

Re: [PATCH v2 0/1] can: mcp251x: Add ACPI device ID table for MCP2515

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On 16.06.2026 16:53:58, guoqi0226 wrote:
> Changes in v2:
> 1. MCP2515 is verified, but MCP251XFD has not been tested.

There was a previous attempt to bring ACPI support for the mcp251xfd
mainline, see
https://lore.kernel.org/all/20241210-mcp251xfd-acpi-v2-1-d6694f590d00@pengutronix.de/

It was not successful. I think it's best to only add ACPI support for
devices you have actually tested.

>    When testing MCP2515, MCP251XFD was found missing ACPI support, hence this addition.
>    Reference the validated MCP2515 ACPI node as template:
>    Device(CAN0){
>          Name(_HID, "MCP2515")
>          Name(_UID, 0)
>          Name (_DSD, Package() {
> 2. The original 9-character HID "MCP251XFD" violates ACPI format rules;
>    redefine it to standard compliant ACPI ID MCHP15FD.

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   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]