From: Tudor Ambarus <tudor.ambarus@linaro.org>
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@oss.qualcomm.com>,
"Krzysztof Kozlowski" <krzk@kernel.org>,
"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
"Chanwoo Choi" <cw00.choi@samsung.com>,
"Alim Akhtar" <alim.akhtar@samsung.com>,
"Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@kernel.org>,
"André Draszik" <andre.draszik@linaro.org>,
"Lee Jones" <lee@kernel.org>,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: stable@vger.kernel.org
Subject: Re: [PATCH v2] firmware: exynos-acpm: Drop fake 'const' on handle pointer
Date: Wed, 25 Feb 2026 16:00:43 +0200 [thread overview]
Message-ID: <279baf9e-ef05-4217-9357-94d21bd93978@linaro.org> (raw)
In-Reply-To: <3e5001a6-ea3c-4304-8db3-bbe616eb4015@oss.qualcomm.com>
On 2/25/26 12:48 PM, Krzysztof Kozlowski wrote:
> On 24/02/2026 13:57, Tudor Ambarus wrote:
>> Hi Krzysztof,
>>
>> On 2/24/26 12:42 PM, Krzysztof Kozlowski wrote:
>>> All the functions operating on the 'handle' pointer are claiming it is a
>>> pointer to const thus they should not modify the handle. In fact that's
>>> a false statement, because first thing these functions do is drop the
>>> cast to const with container_of:
>>>
>>> struct acpm_info *acpm = handle_to_acpm_info(handle);
>>>
>>> And with such cast the handle is easily writable with simple:
>>>
>>> acpm->handle.ops.pmic_ops.read_reg = NULL;
>>>> The code is not correct logically, either, because functions like
>>> acpm_get_by_node() and acpm_handle_put() are meant to modify the handle
>>> reference counting, thus they must modify the handle. Modification here
>>
>> You are right that casting away const via container_of to modify the
>> parent's reference count is incorrect, so dropping the const from the
>> handle argument makes sense.
>>
>> However, to address the underlying issue of the operations being
>> writable (e.g., acpm->handle.ops.pmic_ops.read_reg = NULL), I think we
>> should also decouple the ops from the handle struct and keep them strictly
>> constant in .rodata.
>>
>> How about we apply your fix for the signatures, and I follow up with
>> (or we include) a patch to do the following:
>>
>> struct acpm_handle {
>> const struct acpm_ops *ops; // Changed from embedded struct to pointer
>> };
>>
>> static const struct acpm_ops exynos_acpm_driver_ops = {
>> .dvfs_ops = {
>> .set_rate = acpm_dvfs_set_rate,
>> .get_rate = acpm_dvfs_get_rate,
>> },
>> .pmic_ops = {
>> .read_reg = acpm_pmic_read_reg,
>> .write_reg = acpm_pmic_write_reg,
>> // ... other ops
>> },
>> };
>>
>> and in probe:
>> acpm->handle.ops = &exynos_acpm_driver_ops;
>>
>> This way, the handle safely reflects the mutability of its container,
>> but our function pointers remain fully protected.
>
> Yes, this makes sense.
>
Will you come with a follow up patch or do you want me to ACK this and do
the follow up patch myself? Both options are fine by mine.
Thanks!
ta
next prev parent reply other threads:[~2026-02-25 14:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-24 10:42 [PATCH v2] firmware: exynos-acpm: Drop fake 'const' on handle pointer Krzysztof Kozlowski
2026-02-24 12:57 ` Tudor Ambarus
2026-02-25 10:48 ` Krzysztof Kozlowski
2026-02-25 14:00 ` Tudor Ambarus [this message]
2026-03-01 12:09 ` Krzysztof Kozlowski
2026-02-28 14:58 ` Krzysztof Kozlowski
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=279baf9e-ef05-4217-9357-94d21bd93978@linaro.org \
--to=tudor.ambarus@linaro.org \
--cc=alim.akhtar@samsung.com \
--cc=andre.draszik@linaro.org \
--cc=cw00.choi@samsung.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=s.nawrocki@samsung.com \
--cc=sboyd@kernel.org \
--cc=stable@vger.kernel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).