Re: [PATCH v1] ARM: add Skov i'MX8MP based board support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Oleksij,

On 11.08.23 13:48, Oleksij Rempel wrote:
> Add support for the Skov i'MX8MP based system.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

with change request in last paragraph below adressed.

> +static int skov_imx8m_probe(struct device *dev)
> +{
> +	int emmc_bbu_flag = 0;
> +	int sd_bbu_flag = 0;
> +
> +	if (bootsource_get() == BOOTSOURCE_MMC && bootsource_get_instance() == 1) {
> +		of_device_enable_path("/chosen/environment-sd");
> +		sd_bbu_flag = BBU_HANDLER_FLAG_DEFAULT;
> +	} else {
> +		of_device_enable_path("/chosen/environment-emmc");
> +		emmc_bbu_flag = BBU_HANDLER_FLAG_DEFAULT;
> +	}
> +
> +	imx8m_bbu_internal_mmc_register_handler("SD", "/dev/mmc1.barebox", sd_bbu_flag);
> +	imx8m_bbu_internal_mmcboot_register_handler("eMMC", "/dev/mmc2", emmc_bbu_flag);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id skov_imx8m_of_match[] = {
> +	{ .compatible = "skov,imx8mp" },

Are you sure, you don't want to have a -someboard suffix in case you need
to differentiate future variants?


> +	pmic@25 {
> +		compatible = "nxp,pca9450c";

FYI, there is a driver here:
https://lore.barebox.org/barebox/20230724075258.1840671-1-h.assmann@pengutronix.de/

That would allow you to get $global.system.reset if that's something that's
intereting to you.

> +config MACH_SKOV_IMX8M
> +	bool "Skov IMX8Mx based boards"
> +	select ARCH_IMX8MP
> +	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> +	select FIRMWARE_IMX8MP_ATF
> +	select ARM_SMCCC
> +	select MCI_IMX_ESDHC_PBL
> +	select IMX8M_DRAM
> +	select I2C_IMX_EARLY
> +	select DSA
> +	select DRIVER_NET_KSZ9477

Please drop last two selects. You should only select what's actually required
for booting. At most, you could do an imply here. Also, please enable in defconfig
for compile coverage. There you can enable DSA and KSZ9477 support.

Cheers,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |