Re: [PATCH v2] firmware: exynos-acpm: Drop fake 'const' on handle pointer
From: Krzysztof Kozlowski <hidden>
Date: 2026-02-25 10:48:38
Also in:
linux-clk, linux-samsung-soc, lkml, stable
On 24/02/2026 13:57, Tudor Ambarus wrote:
Hi Krzysztof, On 2/24/26 12:42 PM, Krzysztof Kozlowski wrote:quoted
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;quoted
The code is not correct logically, either, because functions likeacpm_get_by_node() and acpm_handle_put() are meant to modify the handle reference counting, thus they must modify the handle. Modification hereYou 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. Best regards, Krzysztof