Re: [PATCH] ARM: dts: exynos5420: add mali dt bindings and enable mali on Odroid XU3/4

[Krzysztof Kozlowski <krzk@kernel.org>]

Hi,

Thanks for the patch.

Use scripts/get_maintainer.pl to get list of maintainer and lists to Cc.

On Tue, Oct 29, 2019 at 11:43:12AM +1030, Marian Mihailescu wrote:
> Add device tree bindings for Mali GPU for Exynos 542x SoC.

"Add device tree nodes", not bindings.

> GPU is disabled by default, and is enabled for each board after the regulator
> is defined. Tested on Odroid-XU4.
> 
> Signed-off-by: Marian Mihailescu <mihailescu2m@gmail.com>
> ---
>  arch/arm/boot/dts/exynos5420.dtsi             | 49 +++++++++++++++++++++++++++
>  arch/arm/boot/dts/exynos5422-odroid-core.dtsi |  5 +++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 7d51e0f4ab79..5d60e0f1b09a 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -670,6 +670,55 @@
>  			iommus = <&sysmmu_gscl1>;
>  		};
>  
> +		mali: mali@11800000 {

The node should be generic, so use gpu. Also for the label.

> +			compatible = "samsung,exynos5420-mali", "arm,malit628";

First compatible is not documented - please run scripts/checkpatch.pl.
See: https://patchwork.kernel.org/patch/10996479/

Second compatible looks wrong.

> +			reg = <0x11800000 0x5000>;
> +			interrupts = <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> +						 <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>,
> +						 <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;

Broken indentation. Align it with <GIC_SPI before.

> +			interrupt-names = "JOB", "MMU", "GPU";

Does not match bindings.

> +
> +			clocks = <&clock CLK_G3D>;
> +			clock-names = "core";
> +			power-domains = <&g3d_pd>;
> +			operating-points-v2 = <&gpu_opp_table>;

This should be also cooling device so at least cooling cells are needed.
The best would be to connect it with TMU GPU but this could be in next
patches.

> +
> +			status = "disabled";
> +
> +			gpu_opp_table: gpu-opp-table {

The node name: just opp-table, to be generic.

> +				compatible = "operating-points-v2";
> +
> +				opp@177000000 {
> +					opp-hz = /bits/ 64 <177000000>;
> +					opp-microvolt = <812500>;
> +				};
> +				opp@266000000 {
> +					opp-hz = /bits/ 64 <266000000>;
> +					opp-microvolt = <862500>;
> +				};
> +				opp@350000000 {
> +					opp-hz = /bits/ 64 <350000000>;
> +					opp-microvolt = <912500>;
> +				};
> +				opp-420000000 {
> +					opp-hz = /bits/ 64 <420000000>;
> +					opp-microvolt = <962500>;
> +				};
> +				opp-480000000 {
> +					opp-hz = /bits/ 64 <480000000>;
> +					opp-microvolt = <1000000>;
> +				};
> +				opp-543000000 {
> +					opp-hz = /bits/ 64 <543000000>;
> +					opp-microvolt = <1037500>;
> +				};
> +				opp-600000000 {
> +					opp-hz = /bits/ 64 <600000000>;
> +					opp-microvolt = <1150000>;
> +				};
> +			};
> +		};
> +
>  		scaler_0: scaler@12800000 {
>  			compatible = "samsung,exynos5420-scaler";
>  			reg = <0x12800000 0x1294>;
> diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> index 422692b20c46..210c305a0ce8 100644
> --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> @@ -659,6 +659,11 @@
>  	status = "okay";
>  };
>  
> +&mali {
> +	mali-supply = <&buck4_reg>;

Remove the always-on from buck4.

Best regards,
Krzysztof


> +	status = "okay";
> +};
> +
>  &rtc {
>  	status = "okay";
>  	clocks = <&clock CLK_RTC>, <&s2mps11_osc S2MPS11_CLK_AP>;
> -- 
> 2.14.1
>