2023-06-07 11:51:26

by Rohit Agarwal

[permalink] [raw]
Subject: [PATCH v3 4/5] dt-bindings: cpufreq: cpufreq-qcom-hw: Add SDX75 compatible

Add compatible for EPSS CPUFREQ-HW on SDX75.

Signed-off-by: Rohit Agarwal <[email protected]>
Acked-by: Viresh Kumar <[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 a6b3bb8..866ed2d 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
@@ -36,6 +36,7 @@ properties:
- qcom,sa8775p-cpufreq-epss
- qcom,sc7280-cpufreq-epss
- qcom,sc8280xp-cpufreq-epss
+ - qcom,sdx75-cpufreq-epss
- qcom,sm6375-cpufreq-epss
- qcom,sm8250-cpufreq-epss
- qcom,sm8350-cpufreq-epss
--
2.7.4



2023-06-07 12:36:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: cpufreq: cpufreq-qcom-hw: Add SDX75 compatible

On 07/06/2023 13:47, Rohit Agarwal wrote:
> Add compatible for EPSS CPUFREQ-HW on SDX75.
>
> Signed-off-by: Rohit Agarwal <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>
> ---

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions. However, there's no need to repost patches *only* to add the
tags. The upstream maintainer will do that for acks received on the
version they apply.

https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof


2023-06-09 05:57:37

by Pavan Kondeti

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: cpufreq: cpufreq-qcom-hw: Add SDX75 compatible

On Wed, Jun 07, 2023 at 05:17:48PM +0530, Rohit Agarwal wrote:
> Add compatible for EPSS CPUFREQ-HW on SDX75.
>
> Signed-off-by: Rohit Agarwal <[email protected]>
> Acked-by: Viresh Kumar <[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 a6b3bb8..866ed2d 100644
> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> @@ -36,6 +36,7 @@ properties:
> - qcom,sa8775p-cpufreq-epss
> - qcom,sc7280-cpufreq-epss
> - qcom,sc8280xp-cpufreq-epss
> + - qcom,sdx75-cpufreq-epss
> - qcom,sm6375-cpufreq-epss
> - qcom,sm8250-cpufreq-epss
> - qcom,sm8350-cpufreq-epss

This is a very basic question, not completely related to this patch.
Apologies in advance.

What is the rationale for adding a new soc string under compatible and
using it in the new soc device tree? Is it meant for documentation purpose?
i.e one know what all SoCs / boards supported by this device node.

I ask this because, we don't add these compatible strings in the driver
[1] which means there is not SoC specific handling and there is no
module load assist (module alias matching by user space based on device
presence).

Thanks,
Pavan

2023-06-09 09:33:25

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: cpufreq: cpufreq-qcom-hw: Add SDX75 compatible



On 9.06.2023 07:00, Pavan Kondeti wrote:
> On Wed, Jun 07, 2023 at 05:17:48PM +0530, Rohit Agarwal wrote:
>> Add compatible for EPSS CPUFREQ-HW on SDX75.
>>
>> Signed-off-by: Rohit Agarwal <[email protected]>
>> Acked-by: Viresh Kumar <[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 a6b3bb8..866ed2d 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
>> @@ -36,6 +36,7 @@ properties:
>> - qcom,sa8775p-cpufreq-epss
>> - qcom,sc7280-cpufreq-epss
>> - qcom,sc8280xp-cpufreq-epss
>> + - qcom,sdx75-cpufreq-epss
>> - qcom,sm6375-cpufreq-epss
>> - qcom,sm8250-cpufreq-epss
>> - qcom,sm8350-cpufreq-epss
>
> This is a very basic question, not completely related to this patch.
> Apologies in advance.
>
> What is the rationale for adding a new soc string under compatible and
> using it in the new soc device tree? Is it meant for documentation purpose?
> i.e one know what all SoCs / boards supported by this device node.
It's two-fold:

1. The device tree describes the hardware, and for lack of better terms (e.g.
an SoC-specific version number of the block that is identical to all other
implementations of that revision on all SoCs that use it), we tend to
associate it with the SoC it's been (first) found on.

2. In case we ever needed to introduce a SoC-specific quirk, we can just add
an of_is_compatible-sorta check to the driver and not have to update the
device trees. This is very important for keeping backwards compatibility,
as it's assumed that not everybody may be running the latest one. This
means we have to avoid ABI breaks (unless we have *very* good reasons, like
"this would have never worked anyway" or "it was not described properly
and worked on this occasion by pure luck")

Konrad
>
> I ask this because, we don't add these compatible strings in the driver
> [1] which means there is not SoC specific handling and there is no
> module load assist (module alias matching by user space based on device
> presence).
>
> Thanks,
> Pavan

2023-06-09 10:02:37

by Pavan Kondeti

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] dt-bindings: cpufreq: cpufreq-qcom-hw: Add SDX75 compatible

On Fri, Jun 09, 2023 at 11:17:08AM +0200, Konrad Dybcio wrote:
>
>
> On 9.06.2023 07:00, Pavan Kondeti wrote:
> > On Wed, Jun 07, 2023 at 05:17:48PM +0530, Rohit Agarwal wrote:
> >> Add compatible for EPSS CPUFREQ-HW on SDX75.
> >>
> >> Signed-off-by: Rohit Agarwal <[email protected]>
> >> Acked-by: Viresh Kumar <[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 a6b3bb8..866ed2d 100644
> >> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> >> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml
> >> @@ -36,6 +36,7 @@ properties:
> >> - qcom,sa8775p-cpufreq-epss
> >> - qcom,sc7280-cpufreq-epss
> >> - qcom,sc8280xp-cpufreq-epss
> >> + - qcom,sdx75-cpufreq-epss
> >> - qcom,sm6375-cpufreq-epss
> >> - qcom,sm8250-cpufreq-epss
> >> - qcom,sm8350-cpufreq-epss
> >
> > This is a very basic question, not completely related to this patch.
> > Apologies in advance.
> >
> > What is the rationale for adding a new soc string under compatible and
> > using it in the new soc device tree? Is it meant for documentation purpose?
> > i.e one know what all SoCs / boards supported by this device node.
> It's two-fold:
>
> 1. The device tree describes the hardware, and for lack of better terms (e.g.
> an SoC-specific version number of the block that is identical to all other
> implementations of that revision on all SoCs that use it), we tend to
> associate it with the SoC it's been (first) found on.
>
> 2. In case we ever needed to introduce a SoC-specific quirk, we can just add
> an of_is_compatible-sorta check to the driver and not have to update the
> device trees. This is very important for keeping backwards compatibility,
> as it's assumed that not everybody may be running the latest one. This
> means we have to avoid ABI breaks (unless we have *very* good reasons, like
> "this would have never worked anyway" or "it was not described properly
> and worked on this occasion by pure luck")
>

Thanks Konrad for the explanation. The #2 is a clear winner here. It
makes complete sense. In devices like USB, we have PID/VID through which
quirks can be implemented later. So I guess the same analogy applies here.
Like you said in (1), the devices are identified with SoC compatible
string.

Thanks,
Pavan