From: "Alexey Klimov" <alexey.klimov@linaro.org>
To: "Tudor Ambarus" <tudor.ambarus@linaro.org>
Cc: "Krzysztof Kozlowski" <krzk@kernel.org>,
"Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@kernel.org>, "Lee Jones" <lee@kernel.org>,
"Alim Akhtar" <alim.akhtar@samsung.com>,
"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
"Chanwoo Choi" <cw00.choi@samsung.com>,
"André Draszik" <andre.draszik@linaro.org>,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
peter.griffin@linaro.org, jyescas@google.com,
kernel-team@android.com,
"Krzysztof Kozlowski" <krzysztof.kozlowski@oss.qualcomm.com>
Subject: Re: [PATCH 5/6] firmware: samsung: acpm: Add TMU protocol support
Date: Wed, 20 May 2026 22:01:19 +0100 [thread overview]
Message-ID: <DINT2DFDVWFX.BGTG7LOC91RH@linaro.org> (raw)
In-Reply-To: <73f18a2c-e729-45d7-9376-7c0e60ed35c7@linaro.org>
Hi Tudor,
On Tue May 19, 2026 at 4:46 PM BST, Tudor Ambarus wrote:
> Hi, Alexey,
>
> On 5/18/26 2:24 PM, Alexey Klimov wrote:
>> 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.
>
> I did that for the pmic helpers. I don't need any debug prints for
> gs101 TMU as I have clear instructions from firmware: 0 for success,
> -1 for error.
This doesn't look like a right approach for upstreaming a ACPM TMU
framework.
You are trying to submit a gs101-specific implementation masquerading
it as a generic ACPM TMU framework, while explicitly pushing the
refactoring work onto the next developer to add support for other
SoCs in this generic ACPM code.
The ACPM TMU protocol implementation on Exynos850 is different: it uses
different error codes, and half of the calls in this 'generic' driver
are not even implemented in the Exynos850 firmware. Relying on a
hardcoded if (fw_err == -1) in a driver named generic ACPM is broken
by design and may silently swallow critical firmware errors on other
SoCs.
What about such options below?
- rename the driver to reflect reality: rename this specifically to
gs101-acpm-tmu-something to reflect that it is tailored for gs101-s;
or
- abstract the firmware error handling paths through driver_data or
a dedicated ops structure now, so that other SoCs can cleanly hook into
it without having to rewrite the logic later.
Maybe we can utilise build info date from SRAM initdata base to
distinguish between different version of ACPM firmware implementations.
Sadly I was told that specs doesn't provide any version info.
Or probe for implemented calls but that may break ACPM firmware
potentially.
You also mentioned that there are capabilities returned by TMU init
call. Maybe that one can be used somehow.
Thanks,
Alexey
next prev parent reply other threads:[~2026-05-20 21:01 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-06 11:39 [PATCH 0/6] firmware: samsung: acpm: TMU support and cleanups Tudor Ambarus
2026-05-06 11:39 ` [PATCH 1/6] firmware: samsung: acpm: Consolidate transfer initialization helper Tudor Ambarus
2026-05-08 21:06 ` Peter Griffin
2026-05-06 11:39 ` [PATCH 2/6] firmware: samsung: acpm: Annotate rx_data->cmd with __counted_by_ptr Tudor Ambarus
2026-05-08 21:15 ` Peter Griffin
2026-05-06 11:39 ` [PATCH 3/6] firmware: samsung: acpm: Drop redundant _ops suffix in acpm_ops members Tudor Ambarus
2026-05-08 21:23 ` Peter Griffin
2026-05-06 11:39 ` [PATCH 4/6] firmware: samsung: acpm: Make acpm_ops const and access via pointer Tudor Ambarus
2026-05-08 21:27 ` Peter Griffin
2026-05-06 11:39 ` [PATCH 5/6] firmware: samsung: acpm: Add TMU protocol support Tudor Ambarus
2026-05-06 15:13 ` Alexey Klimov
2026-05-07 8:31 ` Tudor Ambarus
2026-05-11 13:17 ` Alexey Klimov
2026-05-15 7:56 ` Tudor Ambarus
2026-05-18 11:24 ` Alexey Klimov
2026-05-19 15:46 ` Tudor Ambarus
2026-05-20 21:01 ` Alexey Klimov [this message]
2026-05-21 8:25 ` Peter Griffin
2026-05-08 21:47 ` Peter Griffin
2026-05-06 11:39 ` [PATCH 6/6] firmware: samsung: acpm: Add devm_acpm_get_by_phandle helper Tudor Ambarus
2026-05-08 21:36 ` Peter Griffin
2026-05-15 8:07 ` [PATCH 0/6] firmware: samsung: acpm: TMU support and cleanups Tudor Ambarus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DINT2DFDVWFX.BGTG7LOC91RH@linaro.org \
--to=alexey.klimov@linaro.org \
--cc=alim.akhtar@samsung.com \
--cc=andre.draszik@linaro.org \
--cc=cw00.choi@samsung.com \
--cc=jyescas@google.com \
--cc=kernel-team@android.com \
--cc=krzk@kernel.org \
--cc=krzysztof.kozlowski@oss.qualcomm.com \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=peter.griffin@linaro.org \
--cc=s.nawrocki@samsung.com \
--cc=sboyd@kernel.org \
--cc=tudor.ambarus@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox