Re: [PATCH 1/2] clk: qcom: Constify qcom_cc_driver_data
From: Konrad Dybcio
Date: Tue Mar 31 2026 - 06:40:56 EST
On 3/31/26 12:20 PM, Krzysztof Kozlowski wrote:
> On 31/03/2026 12:13, Krzysztof Kozlowski wrote:
>> On 31/03/2026 12:10, Konrad Dybcio wrote:
>>> On 3/31/26 12:09 PM, Krzysztof Kozlowski wrote:
>>>> On 31/03/2026 11:33, Konrad Dybcio wrote:
>>>>> On 3/31/26 11:17 AM, Krzysztof Kozlowski wrote:
>>>>>> The static 'struct qcom_cc_driver_data' contains probe match-like data
>>>>>> and is not modified: neither by the driver defining it nor by common.c
>>>>>> code using it.
>>>>>>
>>>>>> Make it const for code safety and code readability.
>>>>>>
>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>
>>>>> [...]
>>>>>
>>>>>> --- a/drivers/clk/qcom/common.h
>>>>>> +++ b/drivers/clk/qcom/common.h
>>>>>> @@ -49,7 +49,7 @@ struct qcom_cc_desc {
>>>>>> size_t num_icc_hws;
>>>>>> unsigned int icc_first_node_id;
>>>>>> bool use_rpm;
>>>>>> - struct qcom_cc_driver_data *driver_data;
>>>>>> + const struct qcom_cc_driver_data *driver_data;
>>>>>
>>>>> This can be a const ptr to const data, even
>>>>
>>>> None of other elements in 'qcom_cc_desc' is const pointer, even though
>>>> they also could. If doing this change, let's make it consistent - so
>>>> shall all of them be const?
>>>
>>> I thought about it, but then it turns out that videocc-sm8550.c has:
>>>
>>> video_cc_sm8550_driver_data.clk_cbcrs = video_cc_sm8650_critical_cbcrs
>>>
>>> So we'd have to duplicate the entire struct
>>
>> No, that's not a problem. Pointer is not modified and we speak here
>> about const pointer.
>>
>
> So to clarify what the code is doing now: I constified the pointed data.
> Not the pointer. If you ask me to constify the pointer itself, it's
> fine, it will compile/work as well, but do you want it?
>
> It allows only definition with initialization, no further changes later.
> All existing drivers would be fine with it, so just confirm that's your
> preferred expression.
I'm actually a little on the verge. Maybe let's keep the current
iteration of this patch after all, as it'd be a mess to undo if it turned
out to be useful
Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
Konrad