Re: [PATCH v2, 1/3] dt-bindings: soc: Add MT8183 emi dt-bindings

[Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 24/05/2019 05:54, Xi Chen wrote:
> Add emi dt-bindings of MT8183 in binding document.
> 
> Signed-off-by: Xi Chen <xixi.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  .../bindings/memory-controllers/mediatek,emi.txt      | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,emi.txt
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,emi.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,emi.txt
> new file mode 100644
> index 0000000..a19e3b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,emi.txt
> @@ -0,0 +1,19 @@
> +EMI (External Memory Interface)
> +
> +Required properties:
> +- compatible : must be one of :
> +	"mediatek,mt8183-emi"
> +- reg : the register and size of the EMI block.

Please name the registers explicitly.

> +- interrupts : includes MPU, CGM, ELM.
> +
> +Example:
> +	emi@10219000 {
> +	compatible = "mediatek,mt8183-emi";

Please do the indention right.

> +	reg = <0 0x10219000 0 0x1000>, /* CEN EMI */
> +		  <0 0x10226000 0 0x1000>, /* EMI MPU */
> +		  <0 0x1022d000 0 0x1000>, /* CHA EMI */
> +		  <0 0x10235000 0 0x1000>; /* CHB EMI */

This looks quite spread out over the IO space. Is this really one HW block or
did you add various blocks into one driver?

> +	interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_LOW>, /* MPU */
> +			 <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>, /* CGM */
> +			 <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>; /* ELM */
> +};
>