[PATCH 5/6] firmware: samsung: acpm: Add TMU protocol support
Peter Griffin
peter.griffin at linaro.org
Thu May 21 01:25:34 PDT 2026
Hi Alexey,
On Wed, 20 May 2026 at 22:01, Alexey Klimov <alexey.klimov at linaro.org> wrote:
>
> 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.
AFAIK it's pretty normal not to add new hooks before they are
required. I think the approach taken in this series makes sense, as
it's the developer adding support for SoC #2 who best knows what the
differences are on their platform versus what exists upstream.
Similarly, the developer adding support for SoC #3 may have different
requirements to e850 and gs101 and that developer is best placed to
refactor and add hooks or quirks that are required for that platform.
Let's not try to boil the ocean with this series, it's targeting GS101
support. We can evolve it for future SoCs as those requirements and
differences become clear.
Peter
Peter
More information about the linux-arm-kernel
mailing list