Re: [PATCH net-next v5 5/5] ARM: dts: qcom: ipq4019: Add description for the IPQESS Ethernet controller

[Vladimir Oltean]

On Fri, Oct 21, 2022 at 02:45:56PM +0200, Maxime Chevallier wrote:
> @@ -591,6 +592,51 @@ wifi1: wifi@a800000 {
>  			status = "disabled";
>  		};
>  
> +		gmac: ethernet@c080000 {

Pretty random ordering in this dts, you'd expect nodes are sorted by
address...

> +			compatible = "qcom,ipq4019-ess-edma";
> +			reg = <0xc080000 0x8000>;
> +			resets = <&gcc ESS_RESET>;
> +			reset-names = "ess";
> +			clocks = <&gcc GCC_ESS_CLK>;
> +			clock-names = "ess";
> +			interrupts = <GIC_SPI  65 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  66 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  67 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  68 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  69 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  70 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  71 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  72 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  73 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  74 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  75 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  76 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  77 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  78 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  79 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI  80 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 240 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 241 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 242 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 243 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 244 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 245 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 246 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 247 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 248 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 249 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 251 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 252 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 253 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 254 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 255 IRQ_TYPE_EDGE_RISING>;

32 interrupts, and no interrupt-names? :)

> +
> +			status = "disabled";
> +

Could you drop these 2 blank lines? They aren't generally added between
properties.

> +			phy-mode = "internal";

And the fixed-link from the schema example no?

> +		};
> +
>  		mdio: mdio@90000 {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -- 
> 2.37.3
>