Re: [PATCH v2 2/5] firmware: tegra: bpmp: Add ACPI support

[Mikko Perttunen <mperttunen@nvidia.com>]

On Wednesday, July 22, 2026 8:05 PM Aniruddha Rao wrote:
> Add required changes in the Tegra BPMP driver to make it compatible with
> ACPI based platforms.
> 
> On ACPI systems, IPC is handled through the AML method instead of
> the core kernel framework using mailboxes and IVC.
> 
> Bypass clock, reset and powergate init calls as these are not
> controlled by the Linux drivers on ACPI based systems.
> 
> Signed-off-by: Aniruddha Rao <anrao@nvidia.com>
> ---
> Changes since v1:
> - Use acpi_extract_package() and a common cleanup path for BMRQ replies.
> - Treat short BMRQ responses as errors.
> - Rename the mailbox transport helper to __tegra_bpmp_transfer().
> - Avoid a dummy ACPI ops table and call SoC ops only when present.
> - Keep OF resource setup guarded by the OF node in probe.
> 
>  drivers/firmware/tegra/bpmp.c | 221 ++++++++++++++++++++++++++++------
>  1 file changed, 182 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
> index 0eb99b1c5068..2dcb74a45b59 100644
> --- a/drivers/firmware/tegra/bpmp.c
> +++ b/drivers/firmware/tegra/bpmp.c
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/clk/tegra.h>
>  #include <linux/genalloc.h>
>  #include <linux/mailbox_client.h>
> @@ -11,8 +12,10 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
> +#include <linux/property.h>
>  #include <linux/semaphore.h>
>  #include <linux/sched/clock.h>
> +#include <linux/slab.h>
>  
>  #include <soc/tegra/bpmp.h>
>  #include <soc/tegra/bpmp-abi.h>
> @@ -23,6 +26,12 @@
>  #define MSG_ACK		BIT(0)
>  #define MSG_RING	BIT(1)
>  #define TAG_SZ		32
> +#define TEGRA_BPMP_ACPI_BMRQ_DATA_SZ	3960U
> +
> +struct tegra_bpmp_acpi_message {
> +	u64 status;
> +	u8 *data;
> +};
>  
>  static inline const struct tegra_bpmp_ops *
>  channel_to_ops(struct tegra_bpmp_channel *channel)
> @@ -343,12 +352,99 @@ static ssize_t tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,
>  
>  static int __maybe_unused tegra_bpmp_resume(struct device *dev);
>  
> +#ifdef CONFIG_ACPI
> +static int tegra_bpmp_transfer_acpi(struct tegra_bpmp *bpmp,
> +				    struct tegra_bpmp_message *msg)
> +{
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer format = { sizeof("NB"), "NB" };
> +	struct acpi_object_list param_list;
> +	union acpi_object params[2];
> +	struct tegra_bpmp_acpi_message *reply;
> +	acpi_status status;
> +	size_t data_len;
> +	int err = 0;
> +
> +	if (!tegra_bpmp_message_valid(msg))
> +		return -EINVAL;
> +
> +	params[0].type = ACPI_TYPE_INTEGER;
> +	params[0].integer.value = msg->mrq;
> +
> +	params[1].type = ACPI_TYPE_BUFFER;
> +	params[1].buffer.length = msg->tx.size;
> +	params[1].buffer.pointer = (u8 *)msg->tx.data;
> +
> +	param_list.count = 2;
> +	param_list.pointer = params;
> +
> +	status = acpi_evaluate_object(ACPI_HANDLE(bpmp->dev), "BMRQ",
> +				      &param_list, &output);
> +	if (ACPI_FAILURE(status)) {
> +		acpi_evaluation_failure_warn(ACPI_HANDLE(bpmp->dev), "BMRQ",
> +					     status);
> +		return -ENODEV;
> +	}
> +
> +	status = acpi_extract_package(output.pointer, &format, &response);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(bpmp->dev, "BMRQ: invalid response package: %s\n",
> +			acpi_format_exception(status));
> +		err = -ENODATA;
> +		goto out;
> +	}
> +
> +	if (response.length < sizeof(*reply)) {
> +		dev_err(bpmp->dev, "BMRQ: response too short\n");
> +		err = -ENODATA;
> +		goto out;
> +	}
> +
> +	reply = response.pointer;
> +	data_len = response.length - sizeof(*reply);
> +	if (data_len > TEGRA_BPMP_ACPI_BMRQ_DATA_SZ) {
> +		dev_err(bpmp->dev, "BMRQ: reply buffer too large (%zu)\n",
> +			data_len);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	msg->rx.ret = (int)reply->status;
> +
> +	if (msg->rx.data && msg->rx.size) {
> +		if (data_len < msg->rx.size) {
> +			dev_err(bpmp->dev, "BMRQ: response data too short\n");
> +			err = -ENODATA;
> +			goto out;
> +		}
> +
> +		memcpy(msg->rx.data, reply->data, msg->rx.size);
> +	}
> +
> +out:
> +	kfree(response.pointer);
> +	kfree(output.pointer);
> +
> +	return err;
> +}
> +#else
> +static int tegra_bpmp_transfer_acpi(struct tegra_bpmp *bpmp,
> +				    struct tegra_bpmp_message *msg)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
> +
>  int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp,
>  			       struct tegra_bpmp_message *msg)
>  {
>  	struct tegra_bpmp_channel *channel;
>  	int err;
>  
> +	if (WARN_ON(ACPI_HANDLE(bpmp->dev)))
> +		return -EOPNOTSUPP;
> +
>  	if (WARN_ON(!irqs_disabled()))
>  		return -EPERM;
>  
> @@ -389,16 +485,13 @@ int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp,
>  }
>  EXPORT_SYMBOL_GPL(tegra_bpmp_transfer_atomic);
>  
> -int tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
> -			struct tegra_bpmp_message *msg)
> +static int __tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
> +				 struct tegra_bpmp_message *msg)
>  {
>  	struct tegra_bpmp_channel *channel;
>  	unsigned long timeout;
>  	int err;
>  
> -	if (WARN_ON(irqs_disabled()))
> -		return -EPERM;
> -
>  	if (!tegra_bpmp_message_valid(msg))
>  		return -EINVAL;
>  
> @@ -428,6 +521,18 @@ int tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
>  	return tegra_bpmp_channel_read(channel, msg->rx.data, msg->rx.size,
>  				       &msg->rx.ret);
>  }
> +
> +int tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
> +			struct tegra_bpmp_message *msg)
> +{
> +	if (WARN_ON(irqs_disabled()))
> +		return -EPERM;
> +
> +	if (ACPI_HANDLE(bpmp->dev))
> +		return tegra_bpmp_transfer_acpi(bpmp, msg);
> +
> +	return __tegra_bpmp_transfer(bpmp, msg);
> +}

Having separate __tegra_bpmp_transfer and tegra_bpmp_transfer seems 
unnecessary to me. I think we can just have tegra_bpmp_transfer call 
tegra_bpmp_transfer_acpi after the tegra_bpmp_message_valid check?

>  EXPORT_SYMBOL_GPL(tegra_bpmp_transfer);
>  
>  static struct tegra_bpmp_mrq *tegra_bpmp_find_mrq(struct tegra_bpmp *bpmp,
> @@ -606,11 +711,17 @@ static int tegra_bpmp_ping(struct tegra_bpmp *bpmp)
>  	msg.rx.data = &response;
>  	msg.rx.size = sizeof(response);
>  
> -	local_irq_save(flags);
>  	start = ktime_get();
> -	err = tegra_bpmp_transfer_atomic(bpmp, &msg);
> +
> +	if (ACPI_HANDLE(bpmp->dev)) {
> +		err = tegra_bpmp_transfer(bpmp, &msg);
> +	} else {
> +		local_irq_save(flags);
> +		err = tegra_bpmp_transfer_atomic(bpmp, &msg);
> +		local_irq_restore(flags);
> +	}
> +
>  	end = ktime_get();
> -	local_irq_restore(flags);
>  
>  	if (!err)
>  		dev_dbg(bpmp->dev,
> @@ -632,6 +743,9 @@ static int tegra_bpmp_get_firmware_tag_old(struct tegra_bpmp *bpmp, char *tag,
>  	void *virt;
>  	int err;
>  
> +	if (ACPI_HANDLE(bpmp->dev))
> +		return -EOPNOTSUPP;
> +
>  	if (size != TAG_SZ)
>  		return -EINVAL;
>  
> @@ -783,23 +897,32 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  	if (!bpmp)
>  		return -ENOMEM;
>  
> -	bpmp->soc = of_device_get_match_data(&pdev->dev);
> -	bpmp->dev = &pdev->dev;
> +	bpmp->soc = device_get_match_data(&pdev->dev);
> +	if (!bpmp->soc)
> +		return -EINVAL;
>  
> -	err = tegra_bpmp_init_channels(bpmp);
> -	if (err < 0)
> -		return err;
> +	bpmp->dev = &pdev->dev;
>  
>  	platform_set_drvdata(pdev, bpmp);
>  
> -	err = bpmp->soc->ops->init(bpmp);
> -	if (err < 0)
> -		return err;
> +	if (!ACPI_HANDLE(bpmp->dev)) {
> +		err = tegra_bpmp_init_channels(bpmp);
> +		if (err < 0)
> +			return err;
> +	}
>  
> -	err = tegra_bpmp_request_mrq(bpmp, MRQ_PING,
> -				     tegra_bpmp_mrq_handle_ping, bpmp);
> -	if (err < 0)
> -		goto deinit;
> +	if (bpmp->soc->ops && bpmp->soc->ops->init) {
> +		err = bpmp->soc->ops->init(bpmp);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	if (!ACPI_HANDLE(bpmp->dev)) {
> +		err = tegra_bpmp_request_mrq(bpmp, MRQ_PING,
> +					     tegra_bpmp_mrq_handle_ping, bpmp);
> +		if (err < 0)
> +			goto deinit;
> +	}
>  
>  	err = tegra_bpmp_ping(bpmp);
>  	if (err < 0) {
> @@ -815,26 +938,30 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  
>  	dev_info(&pdev->dev, "firmware: %.*s\n", (int)sizeof(tag), tag);
>  
> -	err = of_platform_default_populate(pdev->dev.of_node, NULL, &pdev->dev);
> -	if (err < 0)
> -		goto free_mrq;
> -
> -	if (of_property_present(pdev->dev.of_node, "#clock-cells")) {
> -		err = tegra_bpmp_init_clocks(bpmp);
> +	if (pdev->dev.of_node) {
> +		err = of_platform_default_populate(pdev->dev.of_node, NULL,
> +						   &pdev->dev);
>  		if (err < 0)
>  			goto free_mrq;
> -	}
>  
> -	if (of_property_present(pdev->dev.of_node, "#reset-cells")) {
> -		err = tegra_bpmp_init_resets(bpmp);
> -		if (err < 0)
> -			goto free_mrq;
> -	}
> +		if (of_property_present(pdev->dev.of_node, "#clock-cells")) {
> +			err = tegra_bpmp_init_clocks(bpmp);
> +			if (err < 0)
> +				goto free_mrq;
> +		}
>  
> -	if (of_property_present(pdev->dev.of_node, "#power-domain-cells")) {
> -		err = tegra_bpmp_init_powergates(bpmp);
> -		if (err < 0)
> -			goto free_mrq;
> +		if (of_property_present(pdev->dev.of_node, "#reset-cells")) {
> +			err = tegra_bpmp_init_resets(bpmp);
> +			if (err < 0)
> +				goto free_mrq;
> +		}
> +
> +		if (of_property_present(pdev->dev.of_node,
> +					"#power-domain-cells")) {
> +			err = tegra_bpmp_init_powergates(bpmp);
> +			if (err < 0)
> +				goto free_mrq;
> +		}
>  	}
>  
>  	err = tegra_bpmp_init_debugfs(bpmp);
> @@ -844,9 +971,10 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
>  	return 0;
>  
>  free_mrq:
> -	tegra_bpmp_free_mrq(bpmp, MRQ_PING, bpmp);
> +	if (!ACPI_HANDLE(bpmp->dev))
> +		tegra_bpmp_free_mrq(bpmp, MRQ_PING, bpmp);
>  deinit:
> -	if (bpmp->soc->ops->deinit)
> +	if (bpmp->soc->ops && bpmp->soc->ops->deinit)
>  		bpmp->soc->ops->deinit(bpmp);
>  
>  	return err;
> @@ -867,7 +995,7 @@ static int __maybe_unused tegra_bpmp_resume(struct device *dev)
>  
>  	bpmp->suspended = false;
>  
> -	if (bpmp->soc->ops->resume)
> +	if (bpmp->soc->ops && bpmp->soc->ops->resume)
>  		return bpmp->soc->ops->resume(bpmp);
>  	else
>  		return 0;
> @@ -939,10 +1067,25 @@ static const struct of_device_id tegra_bpmp_match[] = {
>  	{ }
>  };
>  
> +#ifdef CONFIG_ACPI
> +static const struct tegra_bpmp_soc tegra_bpmp_acpi_soc = { };
> +
> +static const struct acpi_device_id tegra_bpmp_acpi_match[] = {
> +	{
> +		.id = "NVDA3001",
> +		.driver_data = (kernel_ulong_t)&tegra_bpmp_acpi_soc,
> +	},
> +	{ }
> +};
> +#endif
> +
>  static struct platform_driver tegra_bpmp_driver = {
>  	.driver = {
>  		.name = "tegra-bpmp",
>  		.of_match_table = tegra_bpmp_match,
> +#ifdef CONFIG_ACPI
> +		.acpi_match_table = tegra_bpmp_acpi_match,
> +#endif
>  		.pm = &tegra_bpmp_pm_ops,
>  		.suppress_bind_attrs = true,
>  	},
> -- 
> 2.43.0
>