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

[Tudor Ambarus]

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.

Cheers,
ta