Message-ID: <cd42b8c9-a79a-068c-a967-810c05e818bd@samsung.com>
Date:   Thu, 26 Jan 2023 10:47:46 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0)
        Gecko/20100101 Thunderbird/102.6.1
Subject: Re: [PATCH 2/9] ARM: dts: exynos: move exynos-bus nodes out of soc
 in Exynos5420
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        Kukjin Kim <kgene@kernel.org>, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     replicant@osuosl.org, phone-devel@vger.kernel.org,
        ~postmarketos/upstreaming@lists.sr.ht,
        =?UTF-8?Q?Martin_J=c3=bccker?= <martin.juecker@gmail.com>,
        Henrik Grimler <henrik@grimler.se>,
        Chanwoo Choi <cw00.choi@samsung.com>
From:   Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <20230125094513.155063-2-krzysztof.kozlowski@linaro.org>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20230125094513.155063-1-krzysztof.kozlowski@linaro.org>
        <CGME20230125094529eucas1p29b5f25af2c658abef13a93c49eca975f@eucas1p2.samsung.com>
        <20230125094513.155063-2-krzysztof.kozlowski@linaro.org>
Precedence: bulk

Hi Krzysztof,

On 25.01.2023 10:45, Krzysztof Kozlowski wrote:
> The soc node is supposed to have only device nodes with MMIO addresses,
> as reported by dtc W=1:
>
>    arch/arm/boot/dts/exynos5420.dtsi:1070.24-1075.5:
>      Warning (simple_bus_reg): /soc/bus-wcore: missing or empty reg/ranges property
>
> and dtbs_check:
>
>    exynos5420-arndale-octa.dtb: soc: bus-wcore:
>      {'compatible': ['samsung,exynos-bus'], 'clocks': [[2, 769]], 'clock-names': ['bus'], 'status': ['disabled']} should not be valid under {'type': 'object'}
>
> Move the bus nodes and their OPP tables out of SoC to fix this.
> Re-order them alphabetically while moving and put some of the OPP tables
> in device nodes (if they are not shared).
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Frankly speaking I'm not very keen on moving those bus nodes out of 
/soc. Technically speaking this is definitely a part of soc and doesn't 
make much sense outside of it. IMHO they describe SoC hardware details 
and they might be moved somehow under clock controller device(s), 
although this would require some changes in the bindings and drivers.

> ---
>   arch/arm/boot/dts/exynos5420.dtsi | 223 +++++++++++++++---------------
>   1 file changed, 111 insertions(+), 112 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 7efe72879dc4..cea92f175d46 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -37,6 +37,117 @@ aliases {
>   		spi2 = &spi_2;
>   	};
>   
> +	bus_disp1: bus-disp1 {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_disp1_fimd: bus-disp1-fimd {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_fsys: bus-fsys {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_fsys2: bus-fsys2 {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_fsys_apb: bus-fsys-apb {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_g2d: bus-g2d {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK333_G2D>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_g2d_acp: bus-g2d-acp {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK266_G2D>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +	bus_gen: bus-gen {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK266>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_gscl_scaler: bus-gscl-scaler {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_jpeg: bus-jpeg {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_jpeg_apb: bus-jpeg-apb {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK166>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_mfc: bus-mfc {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK333>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_mscl: bus-mscl {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK400_MSCL>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_noc: bus-noc {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK100_NOC>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_peri: bus-peri {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK66>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
> +	bus_wcore: bus-wcore {
> +		compatible = "samsung,exynos-bus";
> +		clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
> +		clock-names = "bus";
> +		status = "disabled";
> +	};
> +
>   	/*
>   	 * The 'cpus' node is not present here but instead it is provided
>   	 * by exynos5420-cpus.dtsi or exynos5422-cpus.dtsi.
> @@ -1066,118 +1177,6 @@ sysmmu_fimd1_1: sysmmu@14680000 {
>   			power-domains = <&disp_pd>;
>   			#iommu-cells = <0>;
>   		};
> -
> -		bus_wcore: bus-wcore {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK400_WCORE>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_noc: bus-noc {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK100_NOC>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_fsys_apb: bus-fsys-apb {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_PCLK200_FSYS>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_fsys: bus-fsys {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK200_FSYS>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_fsys2: bus-fsys2 {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK200_FSYS2>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_mfc: bus-mfc {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK333>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_gen: bus-gen {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK266>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_peri: bus-peri {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK66>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_g2d: bus-g2d {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK333_G2D>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_g2d_acp: bus-g2d-acp {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK266_G2D>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_jpeg: bus-jpeg {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK300_JPEG>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_jpeg_apb: bus-jpeg-apb {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK166>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_disp1_fimd: bus-disp1-fimd {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK300_DISP1>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_disp1: bus-disp1 {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK400_DISP1>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_gscl_scaler: bus-gscl-scaler {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK300_GSCL>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
> -
> -		bus_mscl: bus-mscl {
> -			compatible = "samsung,exynos-bus";
> -			clocks = <&clock CLK_DOUT_ACLK400_MSCL>;
> -			clock-names = "bus";
> -			status = "disabled";
> -		};
>   	};
>   
>   	thermal-zones {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland