Re: [PATCH 5/6] firmware: samsung: acpm: Add TMU protocol support

["Alexey Klimov" <alexey.klimov@linaro.org>]

On Fri May 15, 2026 at 8:56 AM BST, Tudor Ambarus wrote:
>
>
> On 5/11/26 4:17 PM, Alexey Klimov wrote:
>> On Thu May 7, 2026 at 9:31 AM BST, Tudor Ambarus wrote:
>>> On 5/6/26 6:13 PM, Alexey Klimov wrote:
>>>> On Wed May 6, 2026 at 12:39 PM BST, Tudor Ambarus wrote:
>> 
>> [..]
>> 
>>>>> new file mode 100644
>>>>> index 000000000000..c68d60b4c0b3
>>>>> --- /dev/null
>>>>> +++ b/drivers/firmware/samsung/exynos-acpm-tmu.c
>>>>
>>>> [..]
>>>>
>>>>> +static int acpm_tmu_to_linux_err(s8 fw_err)
>>>>> +{
>>>>> +	/*
>>>>> +	 * ACPM_TMU_INIT uses BIT(0) and BIT(1) of msg.rx.ret to flag APM
>>>>> +	 * capabilities. Treat zero and all positive values as success.
>>>>
>>>> ACPM_TMU_INIT returns capabilities inside designated error field?
>>>
>>> yes
>> 
>> Heh. Okay.
>> 
>>>> What about other messages/commands? They just return error code there?
>>>
>>> all the other commands either return -1 for errors, regardless of the error
>>> type, or 0 for success.
>>>>
>>>>> +	 */
>>>>> +	if (fw_err >= 0)
>>>>> +		return 0;
>>>>> +
>>>>> +	if (fw_err == -1)
>>>>> +		return -EACCES;
>>>>> +
>>>>> +	return -EIO;
>>>>> +}
>>>>
>>>> Could we map these return values with better granularity instead of
>>>> returning -EIO for everything else that is not minus one?
>>>
>>> I think we're good as we are now. The firmware returns either -1 for errors,
>>> zero for success, or BIT(0) and BIT(1) for TMU_INIT to flag some capabilities.
>>> I can't tell if there are other commands that return capabilities as well,
>>> or if there are other capabilities for TMU_INIT, I don't have access to the
>>> firmware code.
>> 
>> On Exynos850 I see more than just one returned error codes. I definitely
>> see 0xfe and 0xfd at least. I don't have any data to confirm that ff
>> maps to -1 and fd-s, fe-s to -2,-3 though and what they mean. From my
>
> for these error codes we will return -EIO which is alright. We can have
> a more granular approach depending on the SoC if you want.
>
> For GS101 above is alright, it matches the info I got from the firmware guys.

*sigh*

>> experiments I suspect that 0xfe means that call/msg type is not
>> implemented or not accessible and 0xfd means that passed parameter is
>> wrong or incorrect or not found.
>> 
>> I am also not sure that I saw 0xff-s but, well, maybe that needs more
>> experimenting.
>> 
> fe and ff will be covered as well by -EIO.

Of course. But it is about propagating the correct error (meaning
exactly "what went wrong") to the other levels and to a user.
My debugging of tmu for e850 would be a bit easier when sending messages
to acpm if I knew that:
-- something went completely bad;
-- specific acpm call is not implemented/not allowed
   (but acpm machinery is working);
-- passed argument for acpm call is not correct/not found
   (but acpm machinery is working);

Instead all of that I get only -EIO. Couldn't say if it will be helpful
for any other platforms.

> Let's keep this as it is for now, and if you need a more granular approach
> we can differentiate that for e850.

It's quite sad that even on that level ACPM on gs101 differs from ACPM
on E850.

Thinking further about this I'd humbly suggest that even

	if (fw_err >= 0)
		return 0;

	pr_debug_ratelimited("ACPM tmu call returned: %x\n", fw_err);
	or pr_debug(...);

	if (fw_err == -1)
		return -EACCES;

some debug message would do.
Perhaps we need some convertation, for instance as it is done in scmi
code (scmi_to_linux_errno(), scmi_linux_errmap[]). But I don't have any
data for mapping acpm errors to some human meanings.

Up to you.

Thanks,
Alexey