On 10/30/24 17:04, Vasileios Amoiridis wrote:
On Wed, Oct 30, 2024 at 04:49:10PM +0100, Michal Simek wrote:


On 10/30/24 15:38, Vasileios Amoiridis wrote:
On Wed, Oct 30, 2024 at 02:13:43PM +0100, Michal Simek wrote:


On 10/29/24 19:58, Vasileios Amoiridis wrote:
From: Vasileios Amoiridis <vasileios.amoiri...@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.amoiri...@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


help would help to also described where that count is stored.


Hi Michal,

thanks for the review. Indeed I could add it.

And why not to have it under DM/U_BOOT_DRIVER?
You don't need to create compatible string for it just instantiate it.

Look at:

U_BOOT_DRVINFO(soc_xilinx_zynqmp) = {
           .name = "soc_xilinx_zynqmp",
};


I was not fully aware of this, I could try it. Just out of curiosity, is
this the new/prefered way of adding new drivers?


+
    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

please put it to the end of CONFIG_BOOTCOUNT to have it at least in correct 
location

    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

In arch/arm/mach-zynqmp/include/mach/hardware.h
there is already structure defined and gen_storage0 is there too.
Please use it.

Regarding this location. Exception in PMUFW is using this reg.
It means at the end of day it is up to you if you want to use it or not.


In master branch (commit 5cca0e3f6e0ff17db92476235ea1bb9cd8cbc9eb) there
is no gen_storage0 in the structure pmu_regs, only gen_storage4 and
gen_storage6.

you are right but easy to extend that structure to have it there.


But in any case though, I don't use gen_storage0 which is the
Global General Storage 0 but the Persistent Global General Storage 0
which is in the in the offset 0x50 and is defined in [1]. According to
this document, only registers {4:7} are used by FSBL and other Xilinx
software products so from what I understand registers {0:3} are free to
use. If that's not the case, which one of them is free for use?

[1]: 
https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/PERS_GLOB_GEN_STORAGE0-PMU_GLOBAL-Register

in embedded sw you can find this
lib/sw_apps/zynqmp_pmufw/src/xpfw_start.S:77:           addik   r3,     r0,
0xffd80050
lib/sw_apps/zynqmp_pmufw/src/xpfw_start.S:81:           addik   r3,     r0,
0xffd80054

And there are 2 pages which describe their usage

https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841724/PMU+Firmware#PMUFirmware-RegistersreservedforPMUFirmware

https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18842019/Zynq+UltraScale+FSBL#ZynqUltraScale+FSBL-WhatarethememoryregionsandregistersreservedforFSBL?

But as I said I am fine if you choose one and you will use it for bootcount.

Thanks
M


Ah okay I see. It was much easier to look on the UG1087 document, I
didn't think of looking somewhere else. Thanks!

In that case I would prefer to use maybe use one of the 2 registers
that are not used, like pers_gen_storage{2,3}. Would that still be
okay for you?

No problem from my side.

M



Reply via email to