Re: [PATCH v9 32/38] ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms

From: "Arnd Bergmann" <arnd@arndb.de>
To: "Nikita Shubin" <nikita.shubin@maquefel.me>,
	"Russell King" <linux@armlinux.org.uk>,
	"Hartley Sweeten" <hsweeten@visionengravers.com>,
	"Alexander Sverdlin" <alexander.sverdlin@gmail.com>,
	"Andrew Davis" <afd@ti.com>, "Nicolas Schier" <nicolas@fjasle.eu>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Masahiro Yamada" <masahiroy@kernel.org>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Dmitry Baryshkov" <dmitry.baryshkov@linaro.org>
Cc: "Bjorn Andersson" <andersson@kernel.org>,
	"Russell King" <rmk+kernel@armlinux.org.uk>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 32/38] ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
Date: Tue, 26 Mar 2024 10:43:22 +0100	[thread overview]
Message-ID: <b7b0b2fa-b092-45dd-b93e-0d59e8d40442@app.fastmail.com> (raw)
In-Reply-To: <20240326-ep93xx-v9-32-156e2ae5dfc8@maquefel.me>

On Tue, Mar 26, 2024, at 10:18, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@maquefel.me>
>
> Add compulsory device tree support to the Cirrus ep93xx ARMv4 platform.
>
> - select PINCTRL_EP93xx
> - select COMMON_CLK_EP93XX, as clock driver moved out of platform code
> - select ARCH_HAS_RESET_CONTROLLER
>
> Select ARM_ATAG_DTB_COMPAT to update device tree with information
> about memory passed from bootloader.
>
> We have to leave all MACH options as they are used for board checking
> before decomp, to turn off watchdog and ethernet DMA.
>
> Tested-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  	select ARCH_SPARSEMEM_ENABLE
>  	select ARM_AMBA
>  	select ARM_VIC
> +	select ARM_APPENDED_DTB # Old Redboot bootloaders deployed
> +	select ARM_ATAG_DTB_COMPAT # we need this to update dt memory node

I would drop these two 'select' statements: while it is
a reasonable assumption that the boot loaders are never going
to be fixed, this is not something that's up to the kernel.

We have other platforms that are in the same situation, and they
just enable those options through the defconfig, as they are
user-visible.

> +	select COMMON_CLK_EP93XX
> +	select EP93XX_TIMER
>  	select CLKSRC_MMIO
>  	select CPU_ARM920T
>  	select GPIOLIB
> +	select PINCTRL
> +	select PINCTRL_EP93XX

If these options are user visible, I would prefer to leave
these up to the defconfig as well, but that's not as important
and something we do in other platforms as well. In general,
I'd prefer to keep 'select' statements to a minimum though.

> -obj-y			:= core.o clock.o timer-ep93xx.o
> -
> -obj-$(CONFIG_EP93XX_DMA)	+= dma.o
> -
> -obj-$(CONFIG_MACH_EDB93XX)	+= edb93xx.o
> -obj-$(CONFIG_MACH_TS72XX)	+= ts72xx.o
> -obj-$(CONFIG_MACH_VISION_EP9307)+= vision_ep9307.o

The Makefile changes should be grouped together with
whichever patch removes the individual files.

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel