Re: [PATCH] ARM: dts: ls1021a: add QUICC Engine node

Alexander Stein
2024-05-31T06:32:50Z

Hi Esben,

thanks for the patch.

Would you consider current converting into YAML format?

Am Donnerstag, 30. Mai 2024, 16:22:54 CEST schrieb Esben Haabendal:
> The LS1021A contains a QUICC Engine Block, so add a node to device
> tree describing that.
>
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
> arch/arm/boot/dts/nxp/ls/ls1021a.dtsi | 51 +++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi b/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
> index e86998ca77d6..ff7be69acdd5 100644
> --- a/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
> +++ b/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
> @@ -460,6 +460,57 @@ gpio3: gpio@2330000 {
> #interrupt-cells = <2>;
> };
>
> + uqe: uqe@2400000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "qe";
> + compatible = "fsl,qe", "simple-bus";
> + ranges = <0x0 0x0 0x2400000 0x40000>;
> + reg = <0x0 0x2400000 0x0 0x480>;

Properties please in this order:
* compatible
* reg
* #address-cells
* #size-cells
* ranges
* device_type

> + brg-frequency = <150000000>;
> + bus-frequency = <300000000>;

Mh, aren't these values depending on your actual RCW configuration?

> + fsl,qe-num-riscs = <1>;
> + fsl,qe-num-snums = <28>;

Current bindings defines:
> fsl,qe-snums: This property has to be specified as '/bits/ 8' value,
> defining the array of serial number (SNUM) values for the virtual
> threads.

So '/bits/ 8' is missing.

> + qeic: qeic@80 {
> + compatible = "fsl,qe-ic";
> + reg = <0x80 0x80>;
> + #address-cells = <0>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH
> + GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + ucc@2000 {
> + cell-index = <1>;
> + reg = <0x2000 0x200>;
> + interrupts = <32>;
> + interrupt-parent = <&qeic>;

Move cell-index to last position.

> + };
> +
> + ucc@2200 {
> + cell-index = <3>;
> + reg = <0x2200 0x200>;
> + interrupts = <34>;
> + interrupt-parent = <&qeic>;

Same here.

> + };
> +
> + muram@10000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,qe-muram", "fsl,cpm-muram";
> + ranges = <0x0 0x10000 0x6000>;

Node address but no 'reg' property? I have no idea if this is okay.
Also compatible (and possibly reg) first.

Thanks and best regards.
Alexander

> + data-only@0 {
> + compatible = "fsl,qe-muram-data",
> + "fsl,cpm-muram-data";
> + reg = <0x0 0x6000>;
> + };
> + };
> + };
> +
> lpuart0: serial@2950000 {
> compatible = "fsl,ls1021a-lpuart";
> reg = <0x0 0x2950000 0x0 0x1000>;
>
> ---
> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
> change-id: 20240530-arm-ls1021a-qe-dts-093381110793
>
> Best regards,