Message ID | 20250111161429.51-4-quic_rlaggysh@quicinc.com |
---|---|
State | New |
Headers | show |
Series | Add EPSS L3 provider support on SA8775P SoC | expand |
On Sat, Jan 11, 2025 at 04:14:27PM +0000, Raviteja Laggyshetty wrote: > Add Epoch Subsystem (EPSS) L3 interconnect provider binding on > SA8775P SoCs. 1. And why is this not compatible with sm8250? There was lengthy discussion and no outcome of it managed to get to commit msg. Really, so we are going to repeat everything again and you will not get any acks. You have entire commit msg to explain things but instead you repeat what the patch does. We can read the diff for that. 2. Binding *ALWAYS* comes before the user. > > Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> > --- > Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml > index 21dae0b92819..94f7f283787a 100644 > --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml > +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml > @@ -33,6 +33,7 @@ properties: > - qcom,sm6375-cpucp-l3 > - qcom,sm8250-epss-l3 > - qcom,sm8350-epss-l3 > + - qcom,sa8775p-epss-l3 > - const: qcom,epss-l3 Your driver suggests this is not really true - it is not compatible with qcom,epss-l3. Maybe it is, maybe not, no clue, commit explains nothing. Best regards, Krzysztof
On 1/12/2025 3:01 PM, Krzysztof Kozlowski wrote: > On Sat, Jan 11, 2025 at 04:14:27PM +0000, Raviteja Laggyshetty wrote: >> Add Epoch Subsystem (EPSS) L3 interconnect provider binding on >> SA8775P SoCs. > > 1. And why is this not compatible with sm8250? There was lengthy > discussion and no outcome of it managed to get to commit msg. Really, so > we are going to repeat everything again and you will not get any acks. > sa8775p is compatible with sm8250, but only difference is sa8775p has two instances of L3 EPSS. Initial patches were posted with two compatibles for supporting two instances. Later there was a comment to reuse the existing and avoid growing the compatibles in the driver. Initially we thought to have separate generic compatible "qcom,epss-l3-perf" for SoCs which support L3 voting through EPSS_L3_PERF state register. But actually, it is already supported in the driver for sm8250 and sc7280. We can reuse sm8250 or sc7280 compatible for sa8775p. I will update the commit text with details in the next patch revision. > You have entire commit msg to explain things but instead you repeat what > the patch does. We can read the diff for that. > > 2. Binding *ALWAYS* comes before the user. Sure, I will update the patch order to bindings, driver and dt in next patch revision and will follow the same for future patchsets. > >> >> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> >> --- >> Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >> index 21dae0b92819..94f7f283787a 100644 >> --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >> +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml >> @@ -33,6 +33,7 @@ properties: >> - qcom,sm6375-cpucp-l3 >> - qcom,sm8250-epss-l3 >> - qcom,sm8350-epss-l3 >> + - qcom,sa8775p-epss-l3 >> - const: qcom,epss-l3 > > Your driver suggests this is not really true - it is not compatible with > qcom,epss-l3. Maybe it is, maybe not, no clue, commit explains nothing. "qcom,epss-l3" is generic compatible introduced for EPSS H/W from sm8250 SoC. sm8250, sa8775p and sc7280 SoCs have same EPSS H/W and use EPSS_L3_PERF register for configuring the perf level. We thought to add generic compatible "qcom,epss-l3-perf" for configuring perf level through EPSS_L3_PERF register. Later, as suggested by reviewers, looks like using a different register cannot be a reason to have different generic compatible as the EPSS H/W is still same on all the SoCs. > > 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..94f7f283787a 100644 --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml @@ -33,6 +33,7 @@ properties: - qcom,sm6375-cpucp-l3 - qcom,sm8250-epss-l3 - qcom,sm8350-epss-l3 + - qcom,sa8775p-epss-l3 - const: qcom,epss-l3 reg:
Add Epoch Subsystem (EPSS) L3 interconnect provider binding on SA8775P SoCs. Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com> --- Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml | 1 + 1 file changed, 1 insertion(+)