Re: [PATCH v4 2/5] arm64: dts: exynos: make tm2 and tm2e independent from each other

[Javier Martinez Canillas]

Hello Andi,

On 01/06/2017 08:41 AM, Andi Shyti wrote:
> Currently tm2e dts includes tm2 but there are some differences
> between the two boards and tm2 has some properties that tm2e
> doesn't have.
> 
> That's why it's important to keep the two dts files independent
> and put all the commonalities in a tm2-common.dtsi file.
> 
> At the current status the only two differences between the two
> dts files (besides the board name) are ldo31 and ldo38.
> 
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxx>
> ---

Patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>

I've some comments though:

> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> dissimilarity index 98%
> index e8971f4a5977..d30b45a9c0d4 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -1,1120 +1,33 @@

[snip]

> -
> -/ {
> -	model = "Samsung TM2 board";
> -	compatible = "samsung,tm2", "samsung,exynos5433";
> -

[snip]

> +
> +/ {
> +	model = "Samsung TM2E board";
> +	compatible = "samsung,tm2e", "samsung,exynos5433";
> +};
> +

You ended with the wrong model and compatible for TM2.

Speaking about these, I noticed that the common .dtsi has the TM2 model and
compatible. I think those don't belong to .dtsi files and instead should be
defined on each .dts file.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America