Re: [PATCH v4 2/5] dt-bindings: soc: qcom: eud: Add SM6115 / SM4250 support

[Krzysztof Kozlowski]

On 05/05/2023 18:31, Bhupesh Sharma wrote:
> HI Krzysztof,
> 
> On Fri, 5 May 2023 at 21:54, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxx>
>>> ---
>>>  .../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