Re: [PATCH v3 1/4] clk: qcom: common: Register reset controller only when resets are present

[Imran Shaik <imran.shaik@oss.qualcomm.com>]

On 24-07-2026 04:19 pm, Philipp Zabel wrote:
> On Do, 2026-07-23 at 21:15 +0530, Imran Shaik wrote:
>> Some clock controller descriptors do not define resets. Avoid registering
>> a reset controller in such cases by checking desc->num_resets.
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   drivers/clk/qcom/common.c | 24 +++++++++++++-----------
>>   1 file changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index 2c09abaf1d2a15b7fbbbfeb67c03075381185a00..d6ff83045da8f308dcb9c5836af48090323248de 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -359,17 +359,19 @@ int qcom_cc_really_probe(struct device *dev,
>>   		qcom_cc_clk_regs_configure(dev, desc->driver_data, regmap);
>>   	}
>>   
>> -	reset = &cc->reset;
>> -	reset->rcdev.of_node = dev->of_node;
>> -	reset->rcdev.ops = &qcom_reset_ops;
>> -	reset->rcdev.owner = dev->driver->owner;
>> -	reset->rcdev.nr_resets = desc->num_resets;
>> -	reset->regmap = regmap;
>> -	reset->reset_map = desc->resets;
>> -
>> -	ret = devm_reset_controller_register(dev, &reset->rcdev);
>> -	if (ret)
>> -		goto put_rpm;
>> +	if (desc->num_resets) {
>> +		reset = &cc->reset;
>> +		reset->rcdev.of_node = dev->of_node;
>> +		reset->rcdev.ops = &qcom_reset_ops;
>> +		reset->rcdev.owner = dev->driver->owner;
>> +		reset->rcdev.nr_resets = desc->num_resets;
>> +		reset->regmap = regmap;
>> +		reset->reset_map = desc->resets;
>> +
>> +		ret = devm_reset_controller_register(dev, &reset->rcdev);
>> +		if (ret)
>> +			goto put_rpm;
>> +	}
>>   
>>   	if (desc->gdscs && desc->num_gdscs) {
>>   		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
> 
> Is it possible to have num_resets == 0 but num_gdscs > 0?
> If so, the now uninitialized reset variable will be dereferenced and
> passed into gdsc_register() a few lines below:
> 
> 		ret = gdsc_register(scd, &reset->rcdev, regmap);
> 
> The whole gdsc reset handling looks very spooky, with
> gdsc_(de)assert_reset() calling directly into the rcdev->ops with no
> regard for reset control state.
> 

Yes, sashiko also pointed [1] the same issue. I will check further on
this and submit the updated change in a separate patch. For now, I'll
drop this patch from this series, as it is no longer required after
separating AudioCoreCC clocks and resets into different devices.

[1] https://lore.kernel.org/all/20260723155515.68A0E1F00A3A@smtp.kernel.org/

Thanks,
Imran