| Message ID | 20241121113006.28520-2-quic_rlaggysh@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add EPSS L3 provider support on SA8775P SoC | expand |
On 21/11/2024 12:30, Raviteja Laggyshetty wrote: > Add Epoch Subsystem (EPSS) L3 interconnect provider binding on > SA8775P SoCs. This we see from the diff. Explain the hardware, why adding epps-l3-perf. > > Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> > --- > .../devicetree/bindings/interconnect/qcom,osm-l3.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml > index 21dae0b92819..042ca44c32ec 100644 > --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml > +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml > @@ -34,6 +34,10 @@ properties: > - qcom,sm8250-epss-l3 > - qcom,sm8350-epss-l3 > - const: qcom,epss-l3 > + - items: > + - enum: > + - qcom,sa8775p-epss-l3 > + - const: qcom,epss-l3-perf I don't understand this change in context of driver. These are the same. Isn't this compatible with sm8250? Sorry, this is all (binding plus driver) quite confusing. Best regards, Krzysztof
On 11/21/2024 5:23 PM, Krzysztof Kozlowski wrote: > On 21/11/2024 12:30, Raviteja Laggyshetty wrote: >> Add Epoch Subsystem (EPSS) L3 interconnect provider binding on >> SA8775P SoCs. > > This we see from the diff. Explain the hardware, why adding epps-l3-perf. > The EPSS instance in SA8775P uses PERF_STATE register instead of REG_L3_VOTE to scale L3 clocks.Along with SoC specific compatible, add new generic compatible "qcom,epss-l3-perf" for PERF_STATE register based L3 scaling. >> >> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> >> --- >> .../devicetree/bindings/interconnect/qcom,osm-l3.yaml | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >> index 21dae0b92819..042ca44c32ec 100644 >> --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >> +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >> @@ -34,6 +34,10 @@ properties: >> - qcom,sm8250-epss-l3 >> - qcom,sm8350-epss-l3 >> - const: qcom,epss-l3 >> + - items: >> + - enum: >> + - qcom,sa8775p-epss-l3 >> + - const: qcom,epss-l3-perf > > I don't understand this change in context of driver. These are the same. > Isn't this compatible with sm8250? > The intention for adding "qcom,epss-l3-perf" generic compatible is to use it for the chipsets which use perf state register for l3 scaling. Using generic compatible avoids the need for adding chipset specific compatible in match table. But received comment from konrad to add both SoC-specific and generic compatibles. Dmitry has suggested to update generic comaptibles for sc7280 and sm8250 SoCs, which makes use of perf state registers. It will be done as separate patch series. > Sorry, this is all (binding plus driver) quite confusing. > > Best regards, > Krzysztof
On 21/11/2024 18:43, Raviteja Laggyshetty wrote: > > > On 11/21/2024 5:23 PM, Krzysztof Kozlowski wrote: >> On 21/11/2024 12:30, Raviteja Laggyshetty wrote: >>> Add Epoch Subsystem (EPSS) L3 interconnect provider binding on >>> SA8775P SoCs. >> >> This we see from the diff. Explain the hardware, why adding epps-l3-perf. >> > The EPSS instance in SA8775P uses PERF_STATE register instead of REG_L3_VOTE to scale L3 clocks.Along with SoC specific compatible, add new generic compatible "qcom,epss-l3-perf" for PERF_STATE register based L3 scaling. Pasting the same replies as you pasted to others won't solve the problem. Solve the problem - fix the commit msg. > >>> >>> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> >>> --- >>> .../devicetree/bindings/interconnect/qcom,osm-l3.yaml | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >>> index 21dae0b92819..042ca44c32ec 100644 >>> --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >>> @@ -34,6 +34,10 @@ properties: >>> - qcom,sm8250-epss-l3 >>> - qcom,sm8350-epss-l3 >>> - const: qcom,epss-l3 >>> + - items: >>> + - enum: >>> + - qcom,sa8775p-epss-l3 >>> + - const: qcom,epss-l3-perf >> >> I don't understand this change in context of driver. These are the same. >> Isn't this compatible with sm8250? >> > > The intention for adding "qcom,epss-l3-perf" generic compatible is to use it for the chipsets which use perf state register for l3 scaling. > Using generic compatible avoids the need for adding chipset specific compatible in match table. Not true, specific compatibles used as fallback do the same and is a preferred way. > But received comment from konrad to add both SoC-specific and generic compatibles. I went through the history and don't see anything like that. Point to the specific email please, if you disagree. > Dmitry has suggested to update generic comaptibles for sc7280 and sm8250 SoCs, which makes use of perf state registers. OK > It will be done as separate patch series. No. I expect to see full, correct picture, not half baked patches which contradict what is in current code. Best regards, Krzysztof
On 11/21/2024 11:20 PM, Krzysztof Kozlowski wrote: > On 21/11/2024 18:43, Raviteja Laggyshetty wrote: >> >> >> On 11/21/2024 5:23 PM, Krzysztof Kozlowski wrote: >>> On 21/11/2024 12:30, Raviteja Laggyshetty wrote: >>>> Add Epoch Subsystem (EPSS) L3 interconnect provider binding on >>>> SA8775P SoCs. >>> >>> This we see from the diff. Explain the hardware, why adding epps-l3-perf. >>> >> The EPSS instance in SA8775P uses PERF_STATE register instead of REG_L3_VOTE to scale L3 clocks.Along with SoC specific compatible, add new generic compatible "qcom,epss-l3-perf" for PERF_STATE register based L3 scaling. > > Pasting the same replies as you pasted to others won't solve the > problem. Solve the problem - fix the commit msg. > >> >>>> >>>> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> >>>> --- >>>> .../devicetree/bindings/interconnect/qcom,osm-l3.yaml | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >>>> index 21dae0b92819..042ca44c32ec 100644 >>>> --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >>>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >>>> @@ -34,6 +34,10 @@ properties: >>>> - qcom,sm8250-epss-l3 >>>> - qcom,sm8350-epss-l3 >>>> - const: qcom,epss-l3 >>>> + - items: >>>> + - enum: >>>> + - qcom,sa8775p-epss-l3 >>>> + - const: qcom,epss-l3-perf >>> >>> I don't understand this change in context of driver. These are the same. >>> Isn't this compatible with sm8250? >>> >> >> The intention for adding "qcom,epss-l3-perf" generic compatible is to use it for the chipsets which use perf state register for l3 scaling. >> Using generic compatible avoids the need for adding chipset specific compatible in match table. > > > Not true, specific compatibles used as fallback do the same and is a > preferred way. > Will make use of sm8250 compatibles for sa8775p in the next patch revision. > >> But received comment from konrad to add both SoC-specific and generic compatibles. > > I went through the history and don't see anything like that. Point to > the specific email please, if you disagree. > https://patchwork.kernel.org/project/linux-pm/patch/20241026123058.28258-2-quic_rlaggysh@quicinc.com/#26104591 >> Dmitry has suggested to update generic comaptibles for sc7280 and sm8250 SoCs, which makes use of perf state registers. > > OK > >> It will be done as separate patch series. > > No. I expect to see full, correct picture, not half baked patches which > contradict what is in current code. > > Ok, I will update the generic compatibles for sm8250, sc7280 SoCs in the next patch revision and will include them along with sa8775p dt patch. > Best regards, > Krzysztof
On Fri, Nov 22, 2024 at 12:46:37PM +0530, Raviteja Laggyshetty wrote: > > > >> But received comment from konrad to add both SoC-specific and generic compatibles. > > > > I went through the history and don't see anything like that. Point to > > the specific email please, if you disagree. > > > https://patchwork.kernel.org/project/linux-pm/patch/20241026123058.28258-2-quic_rlaggysh@quicinc.com/#26104591 I read this message and it does not say to use generic compatible. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml index 21dae0b92819..042ca44c32ec 100644 --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml @@ -34,6 +34,10 @@ properties: - qcom,sm8250-epss-l3 - qcom,sm8350-epss-l3 - const: qcom,epss-l3 + - items: + - enum: + - qcom,sa8775p-epss-l3 + - const: qcom,epss-l3-perf reg: maxItems: 1
Add Epoch Subsystem (EPSS) L3 interconnect provider binding on SA8775P SoCs. Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> --- .../devicetree/bindings/interconnect/qcom,osm-l3.yaml | 4 ++++ 1 file changed, 4 insertions(+)