Re: [PATCH 3/9] ARM: socfpga: add Enclustra AA1 SoM support

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 6282 bytes --]

On Thu, Sep 12, 2024 at 06:06:43AM +0000, Lothar Rubusch wrote:

> Introduce initial support for the Enclustra SoMs:
> 
> - Mercury AA1
> 
> Cover general board files for SD/MMC and QSPI boot modes. Integrate the
> boards to kconfig. All build variants will depend on Quartus handoff
> files, thus they depend on the particular Quartus design. The approach is
> covered in the according documentation part.
> 
> Additionally add configuration for SD/MMC boot and QSPI bootmodes. Register
> additional targets in kconfig.
> 
> Signed-off-by: Andreas Buerkler <andreas.buerkler@enclustra.com>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
[snip]
> diff --git a/board/enclustra/mercury_aa1/mercury_aa1.c b/board/enclustra/mercury_aa1/mercury_aa1.c
> new file mode 100644
> index 0000000000..7de9b287d9
> --- /dev/null
> +++ b/board/enclustra/mercury_aa1/mercury_aa1.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2024 Enclustra GmbH
> + * <info@enclustra.com>
> + */
> +
> +#include <command.h>
> +#include <env.h>
> +#include <init.h>
> +#include <dm/uclass.h>
> +#include <asm-generic/gpio.h>
> +#include <asm/io.h>
> +
> +/* Pin muxing */
> +#if !defined(CONFIG_SPL_BUILD)
> +
> +#define ALTERA_NONE 0
> +#define ALTERA_MMC 1
> +#define ALTERA_QSPI 2
> +#define ALTERA_EMMC 3
> +#define MMC_CLK_DIV 0x9
> +#define QSPI_CLK_DIV 0x384
> +#define ALTERA_PINMUX_OFFS 0xffd07200
> +#define ALTERA_CLKMGR_MAINPLL_CNTR6CLK_BASE 0xFFD04078
> +
> +static int altera_current_storage = ALTERA_NONE;
> +
> +#endif
> +
> +#if !defined(CONFIG_SPL_BUILD)
> +
> +static void set_mux_mmc(void)
> +{
> +	u32 pinmux_arr[] = {0x0c, 0x8,  // IO4 connected to SDMMC
> +			0x10, 0x8,	// IO5
> +			0x14, 0x8,	// IO6
> +			0x18, 0x8,	// IO7
> +			0x1c, 0x8,	// IO8
> +			0x20, 0x8,	// IO9
> +			0x24, 0xf,	// IO10 connected to GPIO
> +			0x28, 0xf,	// IO11
> +			0x2c, 0xf,	// IO12
> +			0x30, 0xf,	// IO13
> +			0x34, 0xf,	// IO14
> +			0x38, 0xf};	// IO15
> +	u32 len, i, offset, value;
> +
> +	len = sizeof(pinmux_arr) / sizeof(u32);
> +	for (i = 0; i < len; i += 2) {
> +		offset = pinmux_arr[i];
> +		value = pinmux_arr[i + 1];
> +		writel(value, ALTERA_PINMUX_OFFS + offset);
> +	}
> +}
> +
> +static void set_mux_emmc(void)
> +{
> +	u32 pinmux_arr[] = {0x0c, 0x8,  // IO4
> +		0x10, 0x8,	// IO5
> +		0x14, 0x8,	// IO6
> +		0x18, 0x8,	// IO7
> +		0x1c, 0x8,	// IO8
> +		0x20, 0x8,	// IO9
> +		0x24, 0xf,	// IO10
> +		0x28, 0xf,	// IO11
> +		0x2c, 0x8,	// IO12
> +		0x30, 0x8,	// IO13
> +		0x34, 0x8,	// IO14
> +		0x38, 0x8};	// IO15
> +	u32 len, i, offset, value;
> +
> +	len = sizeof(pinmux_arr) / sizeof(u32);
> +	for (i = 0; i < len; i += 2) {
> +		offset = pinmux_arr[i];
> +		value = pinmux_arr[i + 1];
> +		writel(value, ALTERA_PINMUX_OFFS + offset);
> +	}
> +}
> +
> +static void set_mux_qspi(void)
> +{
> +	u32 pinmux_arr[] = {0x0c, 0x4,  // IO4 connected to QSPI
> +		0x10, 0x4,	// IO5
> +		0x14, 0x4,	// IO6
> +		0x18, 0x4,	// IO7
> +		0x1c, 0x4,	// IO8
> +		0x20, 0x4,	// IO9
> +		0x24, 0xf,	// IO10
> +		0x28, 0xf,	// IO11
> +		0x2c, 0xf,	// IO12
> +		0x30, 0xf,	// IO13
> +		0x34, 0xf,	// IO14
> +		0x38, 0xf};	// IO15
> +	u32 len, i, offset, value;
> +
> +	len = sizeof(pinmux_arr) / sizeof(u32);
> +	for (i = 0; i < len; i += 2) {
> +		offset = pinmux_arr[i];
> +		value = pinmux_arr[i + 1];
> +		writel(value, ALTERA_PINMUX_OFFS + offset);
> +	}
> +}
> +
> +void altera_set_storage(int store)

Should be static?

> +{
> +	unsigned int gpio_flash_sel;
> +	unsigned int gpio_flash_oe;
> +
> +	if (store == altera_current_storage)
> +		return;
> +
> +	if (gpio_lookup_name("portb5", NULL, NULL, &gpio_flash_oe)) {
> +		printf("ERROR: GPIO not found\n");
> +		return;
> +	}
> +
> +	if (gpio_request(gpio_flash_oe, "flash_oe")) {
> +		printf("ERROR: GPIO request failed\n");
> +		return;
> +	}
> +
> +	if (gpio_lookup_name("portc6", NULL, NULL, &gpio_flash_sel)) {
> +		printf("ERROR: GPIO not found\n");
> +		return;
> +	}
> +
> +	if (gpio_request(gpio_flash_sel, "flash_sel")) {
> +		printf("ERROR: GPIO request failed\n");
> +		return;
> +	}
> +
> +	switch (store) {
> +	case ALTERA_MMC:
> +		set_mux_mmc();
> +		gpio_direction_output(gpio_flash_sel, 0);
> +		gpio_direction_output(gpio_flash_oe, 0);
> +		altera_current_storage = ALTERA_MMC;
> +		writel(MMC_CLK_DIV, ALTERA_CLKMGR_MAINPLL_CNTR6CLK_BASE);
> +		break;
> +	case ALTERA_EMMC:
> +		set_mux_emmc();
> +		gpio_direction_output(gpio_flash_sel, 1);
> +		gpio_direction_output(gpio_flash_oe, 1);
> +		altera_current_storage = ALTERA_EMMC;
> +		writel(MMC_CLK_DIV, ALTERA_CLKMGR_MAINPLL_CNTR6CLK_BASE);
> +		break;
> +	case ALTERA_QSPI:
> +		set_mux_qspi();
> +		gpio_direction_output(gpio_flash_sel, 1);
> +		gpio_direction_output(gpio_flash_oe, 0);
> +		altera_current_storage = ALTERA_QSPI;
> +		writel(QSPI_CLK_DIV, ALTERA_CLKMGR_MAINPLL_CNTR6CLK_BASE);
> +		break;
> +	default:
> +		altera_current_storage = ALTERA_NONE;
> +		break;
> +	}
> +
> +	gpio_free(gpio_flash_sel);
> +	gpio_free(gpio_flash_oe);
> +}
> +
> +int altera_set_storage_cmd(struct cmd_tbl *cmdtp, int flag,
> +			   int argc, char * const argv[])

Same?

> +{
> +	if (argc != 2)
> +		return CMD_RET_USAGE;
> +
> +	if (!strcmp(argv[1], "MMC"))
> +		altera_set_storage(ALTERA_MMC);
> +	else if (!strcmp(argv[1], "QSPI"))
> +		altera_set_storage(ALTERA_QSPI);
> +	else if (!strcmp(argv[1], "EMMC"))
> +		altera_set_storage(ALTERA_EMMC);
> +	else
> +		return CMD_RET_USAGE;
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
> +U_BOOT_CMD(altera_set_storage, 2, 0, altera_set_storage_cmd,
> +	   "Set non volatile memory access",
> +	   "<MMC|QSPI|EMMC> - Set access for the selected memory device");
> +
> +#endif

The entire file seems to be for just adding the cmd. Perhaps rename this
file to reflect that and have the Makefile only even build this when
CONFIG_SPL_BUILD is not set. And then yes, the later patch will need to
be refactored slightly.

Also, the MAINTAINERS entry doesn't cover the documentation (which I'm
glad to see being added at the start!) nor the clock driver.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]