Re: [PATCH 1/4] dt-bindings: soc: qcom: eud: add the QCM2290 EUD

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dt-bindings: soc: qcom: eud: TCSR register modeled as EUD reg
  entry
--

commit 37264f054b9e512075c1f86ca8168f643e3b2fd7
Author: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>

dt-bindings: soc: qcom: eud: add the QCM2290 EUD

This commit adds the qcom,qcm2290-eud compatible string to the Qualcomm EUD
device tree binding schema and defines a third reg entry for the TCSR
register required on QCM2290.

> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> index 84218636c0d8d..14503527cdf2e 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> @@ -17,13 +17,17 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - qcom,qcm2290-eud
>            - qcom,sc7280-eud
>        - const: qcom,eud
>  
>    reg:
> +    minItems: 2
>      items:
>        - description: EUD Base Register Region
>        - description: EUD Mode Manager Register
> +      - description: TCSR register that has to be set before the mode
> +          manager accepts the enable
>  
>    interrupts:
>      description: EUD interrupt
> @@ -50,6 +54,21 @@ required:
>    - reg
>    - ports
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,qcm2290-eud
> +    then:
> +      properties:
> +        reg:
> +          minItems: 3
> +    else:
> +      properties:
> +        reg:
> +          maxItems: 2

[Severity: Medium]
Should the TCSR register be referenced via a phandle and offset to the TCSR
syscon node rather than modeled as a third reg entry on the EUD node?

TCSR_APSS_SPARE_REG1 (0x003e5018) belongs to the separate TCSR hardware
block, which is already described on QCM2290 as a syscon node covering that
address range:

arch/arm64/boot/dts/qcom/agatti.dtsi {
    ...
		tcsr_regs: syscon@3c0000 {
			compatible = "qcom,qcm2290-tcsr", "syscon";
			reg = <0x0 0x003c0000 0x0 0x40000>;
		};
    ...
}

Putting this register into the EUD node's reg property represents a TCSR
register as part of the EUD address space and creates overlapping reg ranges
between sibling nodes on the SoC bus.  Other Qualcomm bindings that access
TCSR registers (including via qcom_scm_io_writel(), such as qcom,dload-mode
in Documentation/devicetree/bindings/firmware/qcom,scm.yaml) use a
phandle-and-offset property pointing to tcsr_regs instead.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-qcm2290-eud-v1-0-b6157b817377@oss.qualcomm.com?part=1