[PATCH v1 1/1] drivers: bootcount: Add ZynqMP specific bootcount support

[Heiko Schocher]

Hello Vasileios,

On 29.10.24 19:58, Vasileios Amoiridis wrote:
> From: Vasileios Amoiridis <vasileios.amoiridis at cern.ch>
> 
> Add native support of the bootcount mechanism in the ZynqMP by utilising internal
> PMU registers. The Persistent Global Storage Registers of the Platform Management
> Unit can keep their value during reboot cycles unless there is a POR reset, making
> them appropriate for the bootcount mechanism.
> 
> Signed-off-by: Vasileios Amoiridis <vasileios.amoiridis at cern.ch>
> ---
>   drivers/bootcount/Kconfig            |  4 ++++
>   drivers/bootcount/Makefile           |  1 +
>   drivers/bootcount/bootcount_zynqmp.c | 28 ++++++++++++++++++++++++++++
>   3 files changed, 33 insertions(+)
>   create mode 100644 drivers/bootcount/bootcount_zynqmp.c
> 
> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
> index fa6d8e7128..95b6a9541a 100644
> --- a/drivers/bootcount/Kconfig
> +++ b/drivers/bootcount/Kconfig
> @@ -82,6 +82,10 @@ config BOOTCOUNT_AT91
>   	bool "Boot counter for Atmel AT91SAM9XE"
>   	depends on AT91SAM9XE
>   
> +config BOOTCOUNT_ZYNQMP
> +	bool "Boot counter for Zynq UltraScale+ MPSoC"
> +	depends on ARCH_ZYNQMP
> +
>   config DM_BOOTCOUNT
>           bool "Boot counter in a device-model device"
>   	help
> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
> index 245f879633..487adc1212 100644
> --- a/drivers/bootcount/Makefile
> +++ b/drivers/bootcount/Makefile
> @@ -3,6 +3,7 @@
>   obj-$(CONFIG_BOOTCOUNT_GENERIC)	+= bootcount.o
>   obj-$(CONFIG_BOOTCOUNT_MEM)	+= bootcount.o
>   obj-$(CONFIG_BOOTCOUNT_AT91)	+= bootcount_at91.o
> +obj-$(CONFIG_BOOTCOUNT_ZYNQMP)  += bootcount_zynqmp.o
>   obj-$(CONFIG_BOOTCOUNT_AM33XX)	+= bootcount_davinci.o
>   obj-$(CONFIG_BOOTCOUNT_RAM)	+= bootcount_ram.o
>   obj-$(CONFIG_BOOTCOUNT_ENV)	+= bootcount_env.o
> diff --git a/drivers/bootcount/bootcount_zynqmp.c b/drivers/bootcount/bootcount_zynqmp.c
> new file mode 100644
> index 0000000000..9bb801e188
> --- /dev/null
> +++ b/drivers/bootcount/bootcount_zynqmp.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2024 CERN (home.cern)
> +
> +#include <bootcount.h>
> +#include <stdio.h>
> +#include <zynqmp_firmware.h>
> +
> +#define PMU_PERS_GLOB_GEN_STORAGE0 0x00FFD80050
> +
> +void bootcount_store(ulong a)
> +{
> +	int ret;
> +
> +	ret = zynqmp_mmio_write(PMU_PERS_GLOB_GEN_STORAGE0, 0xFF, a);
> +	if (ret)
> +		printf("Failed: mmio write\n");
> +}
> +
> +ulong bootcount_load(void)
> +{
> +	int ret;
> +	u32 val;
> +
> +	ret = zynqmp_mmio_read(PMU_PERS_GLOB_GEN_STORAGE0, &val);
> +	if (ret)
> +		printf("Failed: mmio read\n");
> +	return val;

Makes it sense to return val, when ret != 0 ? Maybe val is uninitialzed
in this case... at least please return ret.


just Nitpick: Also may at least add the function name to the printf,
that you have a chance to see on console output, that bootcount read
gone wrong... may the zynqmp_mmio_write/read functions have already an
error message on error?

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de