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

[Thierry Reding <thierry.reding@kernel.org>]

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

On Thu, Jul 23, 2026 at 02:10:23PM +0900, Mikko Perttunen wrote:
> On Wednesday, July 22, 2026 8:05 PM Aniruddha Rao wrote:
[...]
> > diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
[...]
> > @@ -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?

We've had some internal discussions about this and I think we can pull
in the message validity check into this function since it's common
between the ACPI and the non-ACPI implementation. Similarly, that __
prefix may be confusing. It's often used to refer to some internal
helper, mostly for unlocked operations, so it does make some sense here,
but using something like tegra_bpmp_transfer_channel() (I have a slight
preference for this, because technically you could use this on non-DT
systems as well, provided you get channel setup from some other firmware
interface) or tegra_bpmp_transfer_dt() would make it clearer that this
is the alternative to ACPI.

Thierry

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