Re: [PATCH] ti: sci: Drop fake 'const' on handle pointer

[Andrew Davis <afd@ti.com>]

On 3/5/26 8:59 AM, Krzysztof Kozlowski wrote:
> On 02/03/2026 20:12, Andrew Davis wrote:
>> On 2/23/26 2:24 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 ti_sci_info *info = handle_to_ti_sci_info(handle);
>>>
>>> And with such cast the handle is easily writable with simple:
>>>
>>>     info->handle.version.abi_major = 0;
>>>
>>
>> The const is for all the consumers drivers of the handle. Those
>> consumers cannot do the above becouse both handle_to_ti_sci_info()
>> and struct ti_sci_info itself are only defined inside ti_sci.c.
>>
>>> The code is not correct logically, either, because functions like
>>> ti_sci_get_handle() and ti_sci_put_handle() are meant to modify the
>>> handle reference counting, thus they must modify the handle.
>>
>> The reference counting is handled outside of the ti_sci_handle struct,
>> the contents of the handle are never modified after it is created.
>>
>> The const is only added by functions return a handle to consumers.
>> We cannot return non-const to consumer drivers or then they would
>> be able to modify the content without a compiler warning, which would
>> be a real problem.
> 
> This is the same argument as making pointer to const the pointer freed
> via kfree() (or free() in userspace). kfree() does not modify the
> contents of the pointer, right? The same as getting putting handle does
> not modify the handle...
> 

In that argument, if we wanted the consumer of the pointer to not free()
it we would return a const pointer, free()'ing that would result in the
warning we want (discards const qualifier).

If you could somehow malloc() from a const area in memory then free()
doesn't modify the pointed to values, only the non-const record keeping
which would be stored outside of the const memory. So even in this analogy
there isn't a problem.

> The point is that storing the reference counter outside of handle does
> not make the argument correct. Logically when you get a reference, you
> increase the counter, so it is not a pointer to const. And the code
> agrees, because you must drop the const.
> 

The record keeping memory is not const and can be modified.

And where do we drop the const? The outer "struct ti_sci_info" was never
const to begin with, so no dropped const.

If the issue is that the handle is not const inside that outer struct
we could fix that,

struct ti_sci_info {
...
-	struct ti_sci_handle handle;
+	const struct ti_sci_handle handle;
...
};

And with that change even your original commit message example issue
goes away,

struct ti_sci_info *info = handle_to_ti_sci_info(handle);
info->handle.version.abi_major = 0;

would now fail to work to compile.

Andrew

> 
>>
>> Andrew
>>
>>> Modification here happens anyway, even if the reference counting is
>>> stored in the container which the handle is part of.
>>>
>>> The code does not have actual visible bug, but incorrect 'const'
>>> annotations could lead to incorrect compiler decisions.
>>>
> 
> 
> Please kindly trim the replies from unnecessary context. It makes it
> much easier to find new content.
> 
> 
> Best regards,
> Krzysztof