| Message ID | 20230505064039.1630025-3-bhupesh.sharma@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add Qualcomm SM6115 / SM4250 EUD dt-bindings & driver support | expand |
On 05/05/2023 08:40, Bhupesh Sharma wrote: > Add dt-bindings for EUD found on Qualcomm SM6115 / SM4250 SoC. > > On this SoC (and derivatives) the enable bit inside 'tcsr_check_reg' > needs to be set first to 'enable' the eud module. > > So, update the dt-bindings to accommodate the third register > property (TCSR Base) required by the driver on these SoCs. > > Also for these SoCs, introduce a new bool property > 'qcom,secure-mode-enable', which indicates that the mode manager > needs to be accessed only via the secure world. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml > index f2c5ec7e6437..3b92cdf4e306 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml > @@ -18,17 +18,33 @@ properties: > items: > - enum: > - qcom,sc7280-eud > + - qcom,sm6115-eud > - const: qcom,eud > > reg: > + minItems: 2 > items: > - description: EUD Base Register Region > - description: EUD Mode Manager Register > + - description: TCSR Base Register Region > + > + reg-names: > + minItems: 2 > + items: > + - const: eud-base > + - const: eud-mode-mgr > + - const: tcsr-base > > interrupts: > description: EUD interrupt > maxItems: 1 > > + qcom,secure-mode-enable: > + type: boolean > + description: > + Indicates that the mode manager needs to be accessed only via the secure > + world (through 'scm' calls). I understood tcsr-base aplies only to SM6115, so this should be further constrained in allOf:if:then:. Best regards, Krzysztof
Hi Krzysztof, On Fri, 5 May 2023 at 21:54, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 05/05/2023 08:40, Bhupesh Sharma wrote: > > Add dt-bindings for EUD found on Qualcomm SM6115 / SM4250 SoC. > > > > On this SoC (and derivatives) the enable bit inside 'tcsr_check_reg' > > needs to be set first to 'enable' the eud module. > > > > So, update the dt-bindings to accommodate the third register > > property (TCSR Base) required by the driver on these SoCs. > > > > Also for these SoCs, introduce a new bool property > > 'qcom,secure-mode-enable', which indicates that the mode manager > > needs to be accessed only via the secure world. > > Cannot it be implied by compatible? I can see this will be used by future SoCs as well from the available EUD documentation. So let's keep a dedicated dt property as suggested by Bjorn in earlier reviews, as otherwise the compatible checks would start getting bigger / messier in the driver code, in my opinion, when we add EUD support for other SoCs + boards. Regards, Bhupesh
On 05/05/2023 18:31, Bhupesh Sharma wrote: > HI Krzysztof, > > On Fri, 5 May 2023 at 21:54, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 05/05/2023 08:40, Bhupesh Sharma wrote: >>> Add dt-bindings for EUD found on Qualcomm SM6115 / SM4250 SoC. >>> >>> On this SoC (and derivatives) the enable bit inside 'tcsr_check_reg' >>> needs to be set first to 'enable' the eud module. >>> >>> So, update the dt-bindings to accommodate the third register >>> property (TCSR Base) required by the driver on these SoCs. >>> >>> Also for these SoCs, introduce a new bool property >>> 'qcom,secure-mode-enable', which indicates that the mode manager >>> needs to be accessed only via the secure world. >>> >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >>> --- >>> .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml >>> index f2c5ec7e6437..3b92cdf4e306 100644 >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml >>> @@ -18,17 +18,33 @@ properties: >>> items: >>> - enum: >>> - qcom,sc7280-eud >>> + - qcom,sm6115-eud >>> - const: qcom,eud >>> >>> reg: >>> + minItems: 2 >>> items: >>> - description: EUD Base Register Region >>> - description: EUD Mode Manager Register >>> + - description: TCSR Base Register Region >>> + >>> + reg-names: >>> + minItems: 2 >>> + items: >>> + - const: eud-base >>> + - const: eud-mode-mgr >>> + - const: tcsr-base >>> >>> interrupts: >>> description: EUD interrupt >>> maxItems: 1 >>> >>> + qcom,secure-mode-enable: >>> + type: boolean >>> + description: >>> + Indicates that the mode manager needs to be accessed only via the secure >>> + world (through 'scm' calls). >> >> I understood tcsr-base aplies only to SM6115, so this should be further >> constrained in allOf:if:then:. > > Please refer to my reply to your query in another review. > I can see that secure access to mode_mgr register via TCSR will be > exposed by other Qualcomm SoCs as well (from the available EUD > documentation). So, maybe keeping it generic instead of limiting it to > sm6115 only would be more useful, for future changes. Your reply does not say that qcom,sc7280-eud uses it, so I don't understand. This is not valid to qcom,sc7280-eud, so allowing it there is not correct. Best regards, Krzysztof
Hi Krzysztof, On Fri, 5 May 2023 at 23:27, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 05/05/2023 18:31, Bhupesh Sharma wrote: > > HI Krzysztof, > > > > On Fri, 5 May 2023 at 21:54, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 05/05/2023 08:40, Bhupesh Sharma wrote: > >>> Add dt-bindings for EUD found on Qualcomm SM6115 / SM4250 SoC. > >>> > >>> On this SoC (and derivatives) the enable bit inside 'tcsr_check_reg' > >>> needs to be set first to 'enable' the eud module. > >>> > >>> So, update the dt-bindings to accommodate the third register > >>> property (TCSR Base) required by the driver on these SoCs. > >>> > >>> Also for these SoCs, introduce a new bool property > >>> 'qcom,secure-mode-enable', which indicates that the mode manager > >>> needs to be accessed only via the secure world. > >>> > >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > >>> --- > >>> .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 16 ++++++++++++++++ > >>> 1 file changed, 16 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml > >>> index f2c5ec7e6437..3b92cdf4e306 100644 > >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml > >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml > >>> @@ -18,17 +18,33 @@ properties: > >>> items: > >>> - enum: > >>> - qcom,sc7280-eud > >>> + - qcom,sm6115-eud > >>> - const: qcom,eud > >>> > >>> reg: > >>> + minItems: 2 > >>> items: > >>> - description: EUD Base Register Region > >>> - description: EUD Mode Manager Register > >>> + - description: TCSR Base Register Region > >>> + > >>> + reg-names: > >>> + minItems: 2 > >>> + items: > >>> + - const: eud-base > >>> + - const: eud-mode-mgr > >>> + - const: tcsr-base > >>> > >>> interrupts: > >>> description: EUD interrupt > >>> maxItems: 1 > >>> > >>> + qcom,secure-mode-enable: > >>> + type: boolean > >>> + description: > >>> + Indicates that the mode manager needs to be accessed only via the secure > >>> + world (through 'scm' calls). > >> > >> I understood tcsr-base aplies only to SM6115, so this should be further > >> constrained in allOf:if:then:. > > > > Please refer to my reply to your query in another review. > > I can see that secure access to mode_mgr register via TCSR will be > > exposed by other Qualcomm SoCs as well (from the available EUD > > documentation). So, maybe keeping it generic instead of limiting it to > > sm6115 only would be more useful, for future changes. > > Your reply does not say that qcom,sc7280-eud uses it, so I don't > understand. This is not valid to qcom,sc7280-eud, so allowing it there > is not correct. Ok, I got your concern now. I will send a v5 with the change shortly. Thanks.
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml index f2c5ec7e6437..3b92cdf4e306 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml @@ -18,17 +18,33 @@ properties: items: - enum: - qcom,sc7280-eud + - qcom,sm6115-eud - const: qcom,eud reg: + minItems: 2 items: - description: EUD Base Register Region - description: EUD Mode Manager Register + - description: TCSR Base Register Region + + reg-names: + minItems: 2 + items: + - const: eud-base + - const: eud-mode-mgr + - const: tcsr-base interrupts: description: EUD interrupt maxItems: 1 + qcom,secure-mode-enable: + type: boolean + description: + Indicates that the mode manager needs to be accessed only via the secure + world (through 'scm' calls). + ports: $ref: /schemas/graph.yaml#/properties/ports description:
Add dt-bindings for EUD found on Qualcomm SM6115 / SM4250 SoC. On this SoC (and derivatives) the enable bit inside 'tcsr_check_reg' needs to be set first to 'enable' the eud module. So, update the dt-bindings to accommodate the third register property (TCSR Base) required by the driver on these SoCs. Also for these SoCs, introduce a new bool property 'qcom,secure-mode-enable', which indicates that the mode manager needs to be accessed only via the secure world. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> --- .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)