[PATCH 1/2] dt-bindings: cpufreq: Make reg-names a required property

[PATCH 1/2] dt-bindings: cpufreq: Make reg-names a required property

[Konrad Dybcio]

In preparation for determining the number of frequency domains based
on reg-names entries, rather than performing calculations based on
reg entries and parent #{address,size}-cells, make reg-names a
required property.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
index 99e159bc5fb1..460a7ab7b87c 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
@@ -74,6 +74,7 @@ properties:
 required:
   - compatible
   - reg
+  - reg-names
   - clocks
   - clock-names
   - '#freq-domain-cells'
-- 
2.39.0

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

[Konrad Dybcio]

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(-)

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

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

[Bjorn Andersson]

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
> 

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

[Konrad Dybcio]

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
>>

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

[Rob Herring]

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

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

[Konrad Dybcio]

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

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

[Manivannan Sadhasivam]

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
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/2] dt-bindings: cpufreq: Make reg-names a required property

[Manivannan Sadhasivam]

On Wed, Jan 11, 2023 at 09:51:24PM +0100, Konrad Dybcio wrote:
> In preparation for determining the number of frequency domains based
> on reg-names entries, rather than performing calculations based on
> reg entries and parent #{address,size}-cells, make reg-names a
> required property.
> 

As Bjorn mentioned in the driver patch, this would break backwards
compatibility.

Thanks,
Mani

> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> index 99e159bc5fb1..460a7ab7b87c 100644
> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> @@ -74,6 +74,7 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - reg-names
>    - clocks
>    - clock-names
>    - '#freq-domain-cells'
> -- 
> 2.39.0
> 

-- 
மணிவண்ணன் சதாசிவம்