Re: [PATCH v4 02/11] firmware: samsung: acpm: Consolidate transfer initialization helper

Tudor Ambarus
2026-04-29T17:02:49Z

On 4/28/26 12:25 PM, Krzysztof Kozlowski wrote:
> On 23/04/2026 17:22, Tudor Ambarus wrote:
>> acpm_dvfs_init_get_rate_cmd(cmd, clk_id);
>> - acpm_dvfs_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id, true);
>> + acpm_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id, true);
>>
>> ret = acpm_do_xfer(handle, &xfer);
>> if (ret)
>> diff --git a/drivers/firmware/samsung/exynos-acpm-pmic.c b/drivers/firmware/samsung/exynos-acpm-pmic.c
>> index 0c50993cc9a8..f032f2c69685 100644
>> --- a/drivers/firmware/samsung/exynos-acpm-pmic.c
>> +++ b/drivers/firmware/samsung/exynos-acpm-pmic.c
>> @@ -58,16 +58,6 @@ static inline u32 acpm_pmic_get_bulk(u32 data, unsigned int i)
>> return (data >> (ACPM_PMIC_BULK_SHIFT * i)) & ACPM_PMIC_BULK_MASK;
>> }
>>
>> -static void acpm_pmic_set_xfer(struct acpm_xfer *xfer, u32 *cmd, size_t cmdlen,
>> - unsigned int acpm_chan_id)
>> -{
>> - xfer->txd = cmd;
>> - xfer->rxd = cmd;
>> - xfer->txcnt = cmdlen;
>> - xfer->rxcnt = cmdlen;
>> - xfer->acpm_chan_id = acpm_chan_id;
>
> The code is not equivalent. RX bits were always cleared. This should be
> explained in the commit msg which would also provide a proof that you
> actually analyzed if it has any impact.
>
Indeed, my commit message is inaccurate because I claimed the
acpm_pmic/dvfs_set_xfer() were identical.

There's no change in functionality: in the original
`acpm_pmic_set_xfer()`, the RX parameters were unconditionally assigned
(`xfer->rxd = cmd;` and `xfer->rxcnt = cmdlen;`). In this patch, all the
updated PMIC call sites now pass `true` for the `response` argument.
This ensures the unified helper takes the `if (response)` branch, which
performs those exact same assignments, preserving the original PMIC
behavior.

Will update the commit message. And revisit the other patches when
splitting the firmware patches into their own set.

Thanks,
ta