Re: [PATCH 2/8] firmware: arm_scmi: Extend transport driver macro to support ACPI

Jonathan Cameron
2025-10-20T17:11:36Z

On Fri, 17 Oct 2025 14:23:45 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

> Extend the SCMI transport driver helper to also support ACPI-based
> systems. Introduce an internal helper macro that accepts both OF and
> ACPI match tables, and expose two wrappers:
>
> - DEFINE_SCMI_TRANSPORT_DRIVER(...) -> DT/OF wrapper (unchanged ABI)
> - DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(...) -> ACPI wrapper
>
> For ACPI, the generated platform_driver now sets .acpi_match_table via
> ACPI_PTR() so that builds without CONFIG_ACPI remain safe (becomes NULL).
In general it doesn't normally matter if ACPI_PTR() isn't used.

What specifically is this preventing? If CONFIG_ACPI isn't set then we'll never
use those paths anyway. Using ACPI_PTR() tends to need ifdef magic or
__maybe_unused markings that are rarely worth the effort for these tiny
tables.

> The spawned platform_device inherits the parent device’s ACPI companion
> to ensure proper firmware-node association (i.e.subsequent lookups or
> fwnode use see the correct firmware node).
>
> This keeps existing DT users unchanged and continues to function as
> expected while allowing transports to be probed using the structure
> `acpi_device_id` tables on ACPI platforms.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scmi/common.h | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 911941e6885d..d038fec72360 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -9,6 +9,7 @@
> #ifndef _SCMI_COMMON_H
> #define _SCMI_COMMON_H
>
> +#include <linux/acpi.h>
> #include <linux/bitfield.h>
> #include <linux/completion.h>
> #include <linux/device.h>
> @@ -453,7 +454,8 @@ struct scmi_transport {
> struct scmi_transport_core_operations **core_ops;
> };
>
> -#define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
> +#define __DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __of_match, \
> + __acpi_match, __core_ops) \
> static void __tag##_dev_free(void *data) \
> { \
> struct platform_device *spdev = data; \
> @@ -474,6 +476,7 @@ static int __tag##_probe(struct platform_device *pdev) \
> \
> device_set_of_node_from_dev(&spdev->dev, dev); \
> device_set_node(&spdev->dev, dev_fwnode(dev)); \
> + ACPI_COMPANION_SET(&spdev->dev, ACPI_COMPANION(dev)); \
> \
> strans.supplier = dev; \
> memcpy(&strans.desc, &(__desc), sizeof(strans.desc)); \
> @@ -498,11 +501,18 @@ err: \
> static struct platform_driver __drv = { \
> .driver = { \
> .name = #__tag "_transport", \
> - .of_match_table = __match, \
> + .of_match_table = __of_match, \
> + .acpi_match_table = ACPI_PTR(__acpi_match), \
> }, \
> .probe = __tag##_probe, \
> }
>
> +#define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
> + __DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, NULL, __core_ops)
> +
> +#define DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
> + __DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, NULL, __match, __core_ops)
> +
> void scmi_notification_instance_data_set(const struct scmi_handle *handle,
> void *priv);
> void *scmi_notification_instance_data_get(const struct scmi_handle *handle);
>