diff mbox series

[2/2] cpufreq: qcom-hw: Ensure only freq-domain regs are counted in num_domains

Message ID 20230111205125.1860858-2-konrad.dybcio@linaro.org
State New
Headers show
Series [1/2] dt-bindings: cpufreq: Make reg-names a required property | expand

Commit Message

Konrad Dybcio Jan. 11, 2023, 8:51 p.m. UTC
In preparation for CPRh-aware OSM programming, change the probe
function so that we determine the number of frequency domains by
counting the number of reg-names entries that begin with
"freq-domain", as the aforementioned changes require introduction
of non-freq-domain register spaces.

Fixes: 1a6a8b0080b0 ("cpufreq: qcom-hw: Fix reading "reg" with address/size-cells != 2")
Fixes: 054a3ef683a1 ("cpufreq: qcom-hw: Allocate qcom_cpufreq_data during probe")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 10 deletions(-)

Comments

Bjorn Andersson Jan. 12, 2023, 3:37 p.m. UTC | #1
On Wed, Jan 11, 2023 at 09:51:25PM +0100, Konrad Dybcio wrote:
> In preparation for CPRh-aware OSM programming, change the probe
> function so that we determine the number of frequency domains by
> counting the number of reg-names entries that begin with
> "freq-domain", as the aforementioned changes require introduction
> of non-freq-domain register spaces.
> 

Requiring reg-names would break backwards compatibility with at least
sc7280 and sm6115.

Regards,
Bjorn

> Fixes: 1a6a8b0080b0 ("cpufreq: qcom-hw: Fix reading "reg" with address/size-cells != 2")
> Fixes: 054a3ef683a1 ("cpufreq: qcom-hw: Allocate qcom_cpufreq_data during probe")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 9505a812d6a1..89d5ed267399 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -651,8 +651,9 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct device_node *soc_node;
>  	struct device *cpu_dev;
> +	const char *reg_name;
>  	struct clk *clk;
> -	int ret, i, num_domains, reg_sz;
> +	int ret, i, num_reg_names, num_domains = 0;
>  
>  	clk = clk_get(dev, "xo");
>  	if (IS_ERR(clk))
> @@ -684,19 +685,32 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>  	if (!soc_node)
>  		return -EINVAL;
>  
> -	ret = of_property_read_u32(soc_node, "#address-cells", &reg_sz);
> -	if (ret)
> +	num_reg_names = of_property_count_strings(dev->of_node, "reg-names");
> +	if (num_reg_names <= 0) {
> +		ret = num_reg_names ? num_reg_names : -ENODATA;
>  		goto of_exit;
> +	}
>  
> -	ret = of_property_read_u32(soc_node, "#size-cells", &i);
> -	if (ret)
> -		goto of_exit;
> +	for (i = 0; i < num_reg_names; i++) {
> +		ret = of_property_read_string_index(dev->of_node, "reg-names", i, &reg_name);
> +		if (ret < 0)
> +			goto of_exit;
>  
> -	reg_sz += i;
> +		/*
> +		 * Check if the i-th reg is a freq-domain base, no need to add 1
> +		 * more byte for idx, as sizeof counts \0 whereas strlen does not.
> +		 */
> +		if (strlen(reg_name) == sizeof("freq-domain")) {
> +			/* Check if this reg-name begins with "freq-domain" */
> +			if (!strncmp(reg_name, "freq-domain", sizeof("freq-domain") - 1))
> +				num_domains++;
> +		}
> +	}
>  
> -	num_domains = of_property_count_elems_of_size(dev->of_node, "reg", sizeof(u32) * reg_sz);
> -	if (num_domains <= 0)
> -		return num_domains;
> +	if (num_domains <= 0) {
> +		ret = -EINVAL;
> +		goto of_exit;
> +	}
>  
>  	qcom_cpufreq.data = devm_kzalloc(dev, sizeof(struct qcom_cpufreq_data) * num_domains,
>  					 GFP_KERNEL);
> -- 
> 2.39.0
>
Konrad Dybcio Jan. 12, 2023, 3:41 p.m. UTC | #2
On 12.01.2023 16:37, Bjorn Andersson wrote:
> On Wed, Jan 11, 2023 at 09:51:25PM +0100, Konrad Dybcio wrote:
>> In preparation for CPRh-aware OSM programming, change the probe
>> function so that we determine the number of frequency domains by
>> counting the number of reg-names entries that begin with
>> "freq-domain", as the aforementioned changes require introduction
>> of non-freq-domain register spaces.
>>
> 
> Requiring reg-names would break backwards compatibility with at least
> sc7280 and sm6115.
Ouch, you're correct..

Does checking for reg-names and applying the code flow proposed in this
patch if found and the existing one if not sound good?

Konrad
> 
> Regards,
> Bjorn
> 
>> Fixes: 1a6a8b0080b0 ("cpufreq: qcom-hw: Fix reading "reg" with address/size-cells != 2")
>> Fixes: 054a3ef683a1 ("cpufreq: qcom-hw: Allocate qcom_cpufreq_data during probe")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++---------
>>  1 file changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index 9505a812d6a1..89d5ed267399 100644
>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -651,8 +651,9 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>>  	struct device *dev = &pdev->dev;
>>  	struct device_node *soc_node;
>>  	struct device *cpu_dev;
>> +	const char *reg_name;
>>  	struct clk *clk;
>> -	int ret, i, num_domains, reg_sz;
>> +	int ret, i, num_reg_names, num_domains = 0;
>>  
>>  	clk = clk_get(dev, "xo");
>>  	if (IS_ERR(clk))
>> @@ -684,19 +685,32 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>>  	if (!soc_node)
>>  		return -EINVAL;
>>  
>> -	ret = of_property_read_u32(soc_node, "#address-cells", &reg_sz);
>> -	if (ret)
>> +	num_reg_names = of_property_count_strings(dev->of_node, "reg-names");
>> +	if (num_reg_names <= 0) {
>> +		ret = num_reg_names ? num_reg_names : -ENODATA;
>>  		goto of_exit;
>> +	}
>>  
>> -	ret = of_property_read_u32(soc_node, "#size-cells", &i);
>> -	if (ret)
>> -		goto of_exit;
>> +	for (i = 0; i < num_reg_names; i++) {
>> +		ret = of_property_read_string_index(dev->of_node, "reg-names", i, &reg_name);
>> +		if (ret < 0)
>> +			goto of_exit;
>>  
>> -	reg_sz += i;
>> +		/*
>> +		 * Check if the i-th reg is a freq-domain base, no need to add 1
>> +		 * more byte for idx, as sizeof counts \0 whereas strlen does not.
>> +		 */
>> +		if (strlen(reg_name) == sizeof("freq-domain")) {
>> +			/* Check if this reg-name begins with "freq-domain" */
>> +			if (!strncmp(reg_name, "freq-domain", sizeof("freq-domain") - 1))
>> +				num_domains++;
>> +		}
>> +	}
>>  
>> -	num_domains = of_property_count_elems_of_size(dev->of_node, "reg", sizeof(u32) * reg_sz);
>> -	if (num_domains <= 0)
>> -		return num_domains;
>> +	if (num_domains <= 0) {
>> +		ret = -EINVAL;
>> +		goto of_exit;
>> +	}
>>  
>>  	qcom_cpufreq.data = devm_kzalloc(dev, sizeof(struct qcom_cpufreq_data) * num_domains,
>>  					 GFP_KERNEL);
>> -- 
>> 2.39.0
>>
Rob Herring (Arm) Jan. 13, 2023, 7:41 p.m. UTC | #3
On Thu, Jan 12, 2023 at 04:41:50PM +0100, Konrad Dybcio wrote:
> 
> 
> On 12.01.2023 16:37, Bjorn Andersson wrote:
> > On Wed, Jan 11, 2023 at 09:51:25PM +0100, Konrad Dybcio wrote:
> >> In preparation for CPRh-aware OSM programming, change the probe
> >> function so that we determine the number of frequency domains by
> >> counting the number of reg-names entries that begin with
> >> "freq-domain", as the aforementioned changes require introduction
> >> of non-freq-domain register spaces.
> >>
> > 
> > Requiring reg-names would break backwards compatibility with at least
> > sc7280 and sm6115.
> Ouch, you're correct..
> 
> Does checking for reg-names and applying the code flow proposed in this
> patch if found and the existing one if not sound good?

Why support 2 ways?


> Konrad
> > 
> > Regards,
> > Bjorn
> > 
> >> Fixes: 1a6a8b0080b0 ("cpufreq: qcom-hw: Fix reading "reg" with address/size-cells != 2")
> >> Fixes: 054a3ef683a1 ("cpufreq: qcom-hw: Allocate qcom_cpufreq_data during probe")
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >> ---
> >>  drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++---------
> >>  1 file changed, 24 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> >> index 9505a812d6a1..89d5ed267399 100644
> >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> >> @@ -651,8 +651,9 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
> >>  	struct device *dev = &pdev->dev;
> >>  	struct device_node *soc_node;
> >>  	struct device *cpu_dev;
> >> +	const char *reg_name;
> >>  	struct clk *clk;
> >> -	int ret, i, num_domains, reg_sz;
> >> +	int ret, i, num_reg_names, num_domains = 0;
> >>  
> >>  	clk = clk_get(dev, "xo");
> >>  	if (IS_ERR(clk))
> >> @@ -684,19 +685,32 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
> >>  	if (!soc_node)
> >>  		return -EINVAL;
> >>  
> >> -	ret = of_property_read_u32(soc_node, "#address-cells", &reg_sz);
> >> -	if (ret)
> >> +	num_reg_names = of_property_count_strings(dev->of_node, "reg-names");
> >> +	if (num_reg_names <= 0) {
> >> +		ret = num_reg_names ? num_reg_names : -ENODATA;
> >>  		goto of_exit;
> >> +	}
> >>  
> >> -	ret = of_property_read_u32(soc_node, "#size-cells", &i);
> >> -	if (ret)
> >> -		goto of_exit;
> >> +	for (i = 0; i < num_reg_names; i++) {
> >> +		ret = of_property_read_string_index(dev->of_node, "reg-names", i, &reg_name);
> >> +		if (ret < 0)
> >> +			goto of_exit;
> >>  
> >> -	reg_sz += i;
> >> +		/*
> >> +		 * Check if the i-th reg is a freq-domain base, no need to add 1
> >> +		 * more byte for idx, as sizeof counts \0 whereas strlen does not.
> >> +		 */
> >> +		if (strlen(reg_name) == sizeof("freq-domain")) {
> >> +			/* Check if this reg-name begins with "freq-domain" */
> >> +			if (!strncmp(reg_name, "freq-domain", sizeof("freq-domain") - 1))
> >> +				num_domains++;
> >> +		}
> >> +	}
> >>  
> >> -	num_domains = of_property_count_elems_of_size(dev->of_node, "reg", sizeof(u32) * reg_sz);

This code was not great to begin with. Any code parsing 'reg' on it's 
own is suspect IMO. It's a standard property and all parsing of it 
should be in drivers/of/address.c. (Yes, I know there are other cases.)

The reg entries are already available as platform_device resources? Why 
don't you use that? There's also of_address_count(), but I prefer if 
there's a platform device equivalent like we have for interrupts.

Rob
Konrad Dybcio Jan. 14, 2023, 8:42 p.m. UTC | #4
On 13.01.2023 20:41, Rob Herring wrote:
> On Thu, Jan 12, 2023 at 04:41:50PM +0100, Konrad Dybcio wrote:
>>
>>
>> On 12.01.2023 16:37, Bjorn Andersson wrote:
>>> On Wed, Jan 11, 2023 at 09:51:25PM +0100, Konrad Dybcio wrote:
>>>> In preparation for CPRh-aware OSM programming, change the probe
>>>> function so that we determine the number of frequency domains by
>>>> counting the number of reg-names entries that begin with
>>>> "freq-domain", as the aforementioned changes require introduction
>>>> of non-freq-domain register spaces.
>>>>
>>>
>>> Requiring reg-names would break backwards compatibility with at least
>>> sc7280 and sm6115.
>> Ouch, you're correct..
>>
>> Does checking for reg-names and applying the code flow proposed in this
>> patch if found and the existing one if not sound good?
> 
> Why support 2 ways?
Targets that are supported by the current revision of this driver
(which only specify frequency-domain-N MMIO spaces as reg
entries) assume that ARRAY_SIZE(reg) == the number of frequency
domains. These usually range from 1 to 3.

We can either hardcode the number of frequency domains on targets
that require more, different register spaces (for manual hardware
programming, which also happens on currently supported hardware,
just that the secure firmware does it for us.. see [1])
or check with reg-names. Requiring reg-names would break backwards
compatibility with older DTs for at least two SoCs.


> 
> 
>> Konrad
>>>
>>> Regards,
>>> Bjorn
>>>
>>>> Fixes: 1a6a8b0080b0 ("cpufreq: qcom-hw: Fix reading "reg" with address/size-cells != 2")
>>>> Fixes: 054a3ef683a1 ("cpufreq: qcom-hw: Allocate qcom_cpufreq_data during probe")
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>>  drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++---------
>>>>  1 file changed, 24 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>>>> index 9505a812d6a1..89d5ed267399 100644
>>>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>>>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>>>> @@ -651,8 +651,9 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>>>>  	struct device *dev = &pdev->dev;
>>>>  	struct device_node *soc_node;
>>>>  	struct device *cpu_dev;
>>>> +	const char *reg_name;
>>>>  	struct clk *clk;
>>>> -	int ret, i, num_domains, reg_sz;
>>>> +	int ret, i, num_reg_names, num_domains = 0;
>>>>  
>>>>  	clk = clk_get(dev, "xo");
>>>>  	if (IS_ERR(clk))
>>>> @@ -684,19 +685,32 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>>>>  	if (!soc_node)
>>>>  		return -EINVAL;
>>>>  
>>>> -	ret = of_property_read_u32(soc_node, "#address-cells", &reg_sz);
>>>> -	if (ret)
>>>> +	num_reg_names = of_property_count_strings(dev->of_node, "reg-names");
>>>> +	if (num_reg_names <= 0) {
>>>> +		ret = num_reg_names ? num_reg_names : -ENODATA;
>>>>  		goto of_exit;
>>>> +	}
>>>>  
>>>> -	ret = of_property_read_u32(soc_node, "#size-cells", &i);
>>>> -	if (ret)
>>>> -		goto of_exit;
>>>> +	for (i = 0; i < num_reg_names; i++) {
>>>> +		ret = of_property_read_string_index(dev->of_node, "reg-names", i, &reg_name);
>>>> +		if (ret < 0)
>>>> +			goto of_exit;
>>>>  
>>>> -	reg_sz += i;
>>>> +		/*
>>>> +		 * Check if the i-th reg is a freq-domain base, no need to add 1
>>>> +		 * more byte for idx, as sizeof counts \0 whereas strlen does not.
>>>> +		 */
>>>> +		if (strlen(reg_name) == sizeof("freq-domain")) {
>>>> +			/* Check if this reg-name begins with "freq-domain" */
>>>> +			if (!strncmp(reg_name, "freq-domain", sizeof("freq-domain") - 1))
>>>> +				num_domains++;
>>>> +		}
>>>> +	}
>>>>  
>>>> -	num_domains = of_property_count_elems_of_size(dev->of_node, "reg", sizeof(u32) * reg_sz);
> 
> This code was not great to begin with. Any code parsing 'reg' on it's 
> own is suspect IMO. It's a standard property and all parsing of it 
> should be in drivers/of/address.c. (Yes, I know there are other cases.)
> 
> The reg entries are already available as platform_device resources? Why 
> don't you use that? There's also of_address_count(), but I prefer if 
> there's a platform device equivalent like we have for interrupts.
Hm.. I knew this was suspiciously bare-dt-operation, but never quite
connected the dots.. perhaps that's a good idea to pursue..

Konrad

[1] https://patchwork.kernel.org/project/linux-pm/patch/20210701105730.322718-7-angelogioacchino.delregno@somainline.org/
> 
> Rob
Manivannan Sadhasivam Jan. 15, 2023, 4:36 a.m. UTC | #5
On Wed, Jan 11, 2023 at 09:51:25PM +0100, Konrad Dybcio wrote:
> In preparation for CPRh-aware OSM programming, change the probe
> function so that we determine the number of frequency domains by
> counting the number of reg-names entries that begin with
> "freq-domain", as the aforementioned changes require introduction
> of non-freq-domain register spaces.
> 
> Fixes: 1a6a8b0080b0 ("cpufreq: qcom-hw: Fix reading "reg" with address/size-cells != 2")
> Fixes: 054a3ef683a1 ("cpufreq: qcom-hw: Allocate qcom_cpufreq_data during probe")

Why do you need these fixes tags?

Thanks,
Mani

> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 9505a812d6a1..89d5ed267399 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -651,8 +651,9 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct device_node *soc_node;
>  	struct device *cpu_dev;
> +	const char *reg_name;
>  	struct clk *clk;
> -	int ret, i, num_domains, reg_sz;
> +	int ret, i, num_reg_names, num_domains = 0;
>  
>  	clk = clk_get(dev, "xo");
>  	if (IS_ERR(clk))
> @@ -684,19 +685,32 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>  	if (!soc_node)
>  		return -EINVAL;
>  
> -	ret = of_property_read_u32(soc_node, "#address-cells", &reg_sz);
> -	if (ret)
> +	num_reg_names = of_property_count_strings(dev->of_node, "reg-names");
> +	if (num_reg_names <= 0) {
> +		ret = num_reg_names ? num_reg_names : -ENODATA;
>  		goto of_exit;
> +	}
>  
> -	ret = of_property_read_u32(soc_node, "#size-cells", &i);
> -	if (ret)
> -		goto of_exit;
> +	for (i = 0; i < num_reg_names; i++) {
> +		ret = of_property_read_string_index(dev->of_node, "reg-names", i, &reg_name);
> +		if (ret < 0)
> +			goto of_exit;
>  
> -	reg_sz += i;
> +		/*
> +		 * Check if the i-th reg is a freq-domain base, no need to add 1
> +		 * more byte for idx, as sizeof counts \0 whereas strlen does not.
> +		 */
> +		if (strlen(reg_name) == sizeof("freq-domain")) {
> +			/* Check if this reg-name begins with "freq-domain" */
> +			if (!strncmp(reg_name, "freq-domain", sizeof("freq-domain") - 1))
> +				num_domains++;
> +		}
> +	}
>  
> -	num_domains = of_property_count_elems_of_size(dev->of_node, "reg", sizeof(u32) * reg_sz);
> -	if (num_domains <= 0)
> -		return num_domains;
> +	if (num_domains <= 0) {
> +		ret = -EINVAL;
> +		goto of_exit;
> +	}
>  
>  	qcom_cpufreq.data = devm_kzalloc(dev, sizeof(struct qcom_cpufreq_data) * num_domains,
>  					 GFP_KERNEL);
> -- 
> 2.39.0
>
diff mbox series

Patch

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 9505a812d6a1..89d5ed267399 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -651,8 +651,9 @@  static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *soc_node;
 	struct device *cpu_dev;
+	const char *reg_name;
 	struct clk *clk;
-	int ret, i, num_domains, reg_sz;
+	int ret, i, num_reg_names, num_domains = 0;
 
 	clk = clk_get(dev, "xo");
 	if (IS_ERR(clk))
@@ -684,19 +685,32 @@  static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
 	if (!soc_node)
 		return -EINVAL;
 
-	ret = of_property_read_u32(soc_node, "#address-cells", &reg_sz);
-	if (ret)
+	num_reg_names = of_property_count_strings(dev->of_node, "reg-names");
+	if (num_reg_names <= 0) {
+		ret = num_reg_names ? num_reg_names : -ENODATA;
 		goto of_exit;
+	}
 
-	ret = of_property_read_u32(soc_node, "#size-cells", &i);
-	if (ret)
-		goto of_exit;
+	for (i = 0; i < num_reg_names; i++) {
+		ret = of_property_read_string_index(dev->of_node, "reg-names", i, &reg_name);
+		if (ret < 0)
+			goto of_exit;
 
-	reg_sz += i;
+		/*
+		 * Check if the i-th reg is a freq-domain base, no need to add 1
+		 * more byte for idx, as sizeof counts \0 whereas strlen does not.
+		 */
+		if (strlen(reg_name) == sizeof("freq-domain")) {
+			/* Check if this reg-name begins with "freq-domain" */
+			if (!strncmp(reg_name, "freq-domain", sizeof("freq-domain") - 1))
+				num_domains++;
+		}
+	}
 
-	num_domains = of_property_count_elems_of_size(dev->of_node, "reg", sizeof(u32) * reg_sz);
-	if (num_domains <= 0)
-		return num_domains;
+	if (num_domains <= 0) {
+		ret = -EINVAL;
+		goto of_exit;
+	}
 
 	qcom_cpufreq.data = devm_kzalloc(dev, sizeof(struct qcom_cpufreq_data) * num_domains,
 					 GFP_KERNEL);