2023-01-11 20:59:48

by Konrad Dybcio

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

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 <[email protected]>
---
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


2023-01-11 21:12:23

by Konrad Dybcio

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

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 <[email protected]>
---
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

2023-01-12 16:00:34

by Bjorn Andersson

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

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 <[email protected]>
> ---
> 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
>

2023-01-12 16:03:37

by Konrad Dybcio

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



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 <[email protected]>
>> ---
>> 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
>>

2023-01-13 20:33:08

by Rob Herring (Arm)

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

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 <[email protected]>
> >> ---
> >> 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

2023-01-14 20:59:30

by Konrad Dybcio

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



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 <[email protected]>
>>>> ---
>>>> 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/[email protected]/
>
> Rob

2023-01-15 04:45:10

by Manivannan Sadhasivam

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

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 <[email protected]>
> ---
> 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
>

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

2023-01-15 05:25:10

by Manivannan Sadhasivam

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

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 <[email protected]>
> ---
> 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
>

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