Re: [PATCH 6/7] arm64: dts: exynos: Add initial Exynos850 SoC support

From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
To: Alim Akhtar <alim.akhtar@gmail.com>,
	Sam Protsenko <semen.protsenko@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Jaewon Kim <jaewon02.kim@samsung.com>,
	Chanho Park <chanho61.park@samsung.com>,
	David Virag <virag.david003@gmail.com>,
	Youngmin Nam <youngmin.nam@samsung.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Daniel Palmer <daniel@0x0f.com>, Hao Fang <fanghao11@huawei.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
	open list <linux-kernel@vger.kernel.org>,
	linux-clk@vger.kernel.org
Subject: Re: [PATCH 6/7] arm64: dts: exynos: Add initial Exynos850 SoC support
Date: Sat, 18 Dec 2021 11:37:45 +0100	[thread overview]
Message-ID: <cd97d5f8-42ff-98a4-2dfe-7d8076cf5e53@canonical.com> (raw)
In-Reply-To: <CAGOxZ52h-PL7ii-qDVy0tn51gmvgU3uhZC6NYH3hgxWiHrRJEA@mail.gmail.com>

On 17/12/2021 17:46, Alim Akhtar wrote:
> Hi Sam,
> 
> On Thu, Dec 16, 2021 at 1:36 AM Sam Protsenko
> <semen.protsenko@linaro.org> wrote:
>>
>> Samsung Exynos850 is ARMv8-based mobile-oriented SoC. This patch adds
>> initial SoC support. It's not comprehensive yet, some more devices will
>> be added later. Right now only crucial system components and most needed
>> platform devices are defined.
>>
>> Crucial features (needed to boot Linux up to shell with serial console):
>>
>>   * Octa cores (Cortex-A55), supporting PSCI v1.0
>>   * ARM architected timer (armv8-timer)
>>   * Interrupt controller (GIC-400)
>>   * Pinctrl nodes for GPIO
>>   * Serial node
>>
>> Basic platform features:
>>
>>   * Clock controller CMUs
>>   * OSCCLK clock
>>   * RTC clock
>>   * MCT timer
>>   * ARM PMU (Performance Monitor Unit)
>>   * Chip-id
>>   * RTC
>>   * Reset
>>   * Watchdog timers
>>   * eMMC
>>   * I2C
>>   * HSI2C
>>   * USI
>>
>> All those features were already enabled and tested on E850-96 board with
>> minimal BusyBox rootfs.
>>
>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>> ---
>>  .../boot/dts/exynos/exynos850-pinctrl.dtsi    | 755 ++++++++++++++++++
>>  arch/arm64/boot/dts/exynos/exynos850.dtsi     | 755 ++++++++++++++++++
> Instead of such a large patch, it is good to logically divide the
> patches as per IP for easy of review
> e.g.
> Put everything in one patch which is good enough to get you a Linux
> prompt, followed
> by one or a couple of IPs dtsi, dts entries.

The patch is not that big and splitting it into several addons does not
bring benefits. You still add new DTSI - either in one or two patches,
there is going to be the same amount of code to review. One still has to
review everything.

It would be different if DTSI was already applied - then incremental
updates make sense. Another reason for splitting is for different
topics, when doing multiple separate actions, like fix + add, change + add.

Best regards,
Krzysztof