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

[Philipp Zabel]

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@xxxxxxxxxxxxxxxx>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
> Signed-off-by: Imran Shaik <imran.shaik@xxxxxxxxxxxxxxxx>
> ---
>  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.

regards
Philipp