Re: [PATCH v2] firmware: exynos-acpm: Drop fake 'const' on handle pointer

[Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>]

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.

Best regards,
Krzysztof