Re: [PATCH 3/3] arm: zynqmp: add boot source support

[Michael Tretter <m.tretter@pengutronix.de>]

On Fri, 10 Sep 2021 15:43:23 +0200, Michael Riesch wrote:
> The ZynqMP reports the mode pins sampled at POR via the register
> ZYNQMP_CRL_APB_BOOT_MODE_USER. This commit adds a function that reads
> the register and populates the boot source.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  arch/arm/mach-zynqmp/zynqmp.c | 43 +++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/arch/arm/mach-zynqmp/zynqmp.c b/arch/arm/mach-zynqmp/zynqmp.c
> index 5871c145b..262bc0dd4 100644
> --- a/arch/arm/mach-zynqmp/zynqmp.c
> +++ b/arch/arm/mach-zynqmp/zynqmp.c
> @@ -6,9 +6,11 @@
>  #include <common.h>
>  #include <init.h>
>  #include <linux/types.h>
> +#include <bootsource.h>
>  #include <reset_source.h>
>  
>  #define ZYNQMP_CRL_APB_BASE		0xff5e0000
> +#define ZYNQMP_CRL_APB_BOOT_MODE_USER	(ZYNQMP_CRL_APB_BASE + 0x200)
>  #define ZYNQMP_CRL_APB_RESET_REASON	(ZYNQMP_CRL_APB_BASE + 0x220)
>  
>  /* External POR: The PS_POR_B reset signal pin was asserted. */
> @@ -26,6 +28,46 @@
>  /* Software debugger reset: Write to BLOCKONLY_RST [debug_only]. */
>  #define ZYNQMP_CRL_APB_RESET_REASON_DEBUG_SYS  BIT(6)
>  
> +struct zynqmp_bootsource {
> +	enum bootsource src;
> +	int instance;
> +};
> +
> +/* cf. Table 11-1 "Boot Modes" in UG1085 Zynq UltraScale+ Device TRM */
> +static struct zynqmp_bootsource boot_modes[] = {
> +	{ .src = BOOTSOURCE_JTAG, .instance = 0 },
> +	{ .src = BOOTSOURCE_SPI, .instance = 0 },
> +	{ .src = BOOTSOURCE_SPI, .instance = 0 },
> +	{ .src = BOOTSOURCE_MMC, .instance = 0 },
> +	{ .src = BOOTSOURCE_SPI_NAND, .instance = 0 },
> +	{ .src = BOOTSOURCE_MMC, .instance = 1 },
> +	{ .src = BOOTSOURCE_MMC, .instance = 0 },
> +	{ .src = BOOTSOURCE_USB, .instance = 0 },
> +	{ .src = BOOTSOURCE_JTAG, .instance = 0 },
> +	{ .src = BOOTSOURCE_JTAG, .instance = 0 },
> +	{ .src = BOOTSOURCE_UNKNOWN, .instance = 0 },
> +	{ .src = BOOTSOURCE_UNKNOWN, .instance = 0 },
> +	{ .src = BOOTSOURCE_UNKNOWN, .instance = 0 },
> +	{ .src = BOOTSOURCE_UNKNOWN, .instance = 0 },
> +	{ .src = BOOTSOURCE_MMC, .instance = 1 },
> +};

Thanks for the patch.

Please make the mapping of the Boot Mode value to the BOOTSOURCE explicit
instead of hiding in as the index into this array.

> +
> +static enum bootsource zynqmp_bootsource(void)
> +{
> +	u32 v;
> +
> +	v = readl(ZYNQMP_CRL_APB_BOOT_MODE_USER);
> +	v &= 0x0F;
> +
> +	if (v >= ARRAY_SIZE(boot_modes))
> +		return BOOTSOURCE_UNKNOWN;
> +
> +	bootsource_set(boot_modes[v].src);
> +	bootsource_set_instance(boot_modes[v].instance);

Don't set the bootsource as a side effect of this function. This function
should only lookup of the boot mode and zynqmp_init should actually set it.

Michael

> +
> +	return boot_modes[v].src;
> +}
> +
>  struct zynqmp_reset_reason {
>  	u32 mask;
>  	enum reset_src_type type;
> @@ -65,6 +107,7 @@ static enum reset_src_type zynqmp_get_reset_src(void)
>  
>  static int zynqmp_init(void)
>  {
> +	zynqmp_bootsource();
>  	reset_source_set(zynqmp_get_reset_src());
>  
>  	return 0;
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox