Am 10.03.2020 um 17:42 schrieb Ang, Chee Hong:
-----Original Message----- From: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Sent: Wednesday, March 11, 2020 12:17 AM To: Ang, Chee Hong chee.hong.ang@intel.com Cc: u-boot@lists.denx.de; Marek Vasut marex@denx.de; See, Chin Liang chin.liang.see@intel.com; Tan, Ley Foon ley.foon.tan@intel.com; Westergreen, Dalon dalon.westergreen@intel.com; Gong, Richard richard.gong@intel.com Subject: Re: [PATCH v4 11/21] misc: altera_sysmgr: Add Altera System Manager driver
Am 09.03.2020 um 10:07 schrieb chee.hong.ang@intel.com:
From: Chee Hong Ang chee.hong.ang@intel.com
This driver (misc uclass) handle the read/write access to System Manager. For 64 bits platforms, processor needs to be in secure mode to has write access to most of the System Manager's registers (except boot scratch registers). When the processor is running in EL2 (non-secure), this driver will invoke the SMC call to ATF to perform write access to the System Manager's registers. All other drivers that require access to System Manager should go through this driver.
Signed-off-by: Chee Hong Ang chee.hong.ang@intel.com
drivers/misc/Makefile | 1 + drivers/misc/altera_sysmgr.c | 115 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 drivers/misc/altera_sysmgr.c
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 2b843de..9fa2411 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -29,6 +29,7 @@ endif endif obj-$(CONFIG_ALI152X) += ali512x.o obj-$(CONFIG_ALTERA_SYSID) += altera_sysid.o +obj-$(CONFIG_ARCH_SOCFPGA) += altera_sysmgr.o obj-$(CONFIG_ATSHA204A) += atsha204a-i2c.o obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o obj-$(CONFIG_DS4510) += ds4510.o diff --git a/drivers/misc/altera_sysmgr.c b/drivers/misc/altera_sysmgr.c new file mode 100644 index 0000000..b36ecae --- /dev/null +++ b/drivers/misc/altera_sysmgr.c
I think this file should have something in the name specifying it is for
s10/agilex.
I will post a misc/sysmgr for gen5 that needs a specific name, too
Gen5/A10/S10/Agilex are using same DW MMC/MAC drivers and these drivers
access system manager.
Therefore, this driver is enabled for all platforms. Gen5/A10, S10/Agilex all are
using it.
Can I know what does your gen5 sysmgr driver do ? I can change the name to avoid conflict but Gen5 will have 2 sysmgr drivers for
different purposes.
Are you OK with that ?
@@ -0,0 +1,115 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2020 Intel Corporation <www.intel.com> */
+#include <common.h> +#include <command.h> +#include <dm.h> +#include <errno.h> +#include <misc.h> +#include <asm/io.h> +#include <asm/arch/misc.h> +#include <linux/intel-smc.h>
+#define IS_OUT_OF_SYSMGR(addr, range) ((addr) > (range))
+struct altera_sysmgr_priv {
- fdt_addr_t base_addr;
- fdt_addr_t base_size;
+};
+#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF) static +int +secure_write32(u32 val, fdt_addr_t addr) {
- int ret;
- u64 args[2];
- args[0] = (u64)addr;
- args[1] = val;
- ret = invoke_smc(INTEL_SIP_SMC_REG_WRITE, args, 2, NULL, 0);
Hmm, so you're just using misc_ops to still issue generic writes. From the discussion with Marek in the last version, I would have thought you wanted to create a higher level API instead of still tunnelling
reads and writes?
Any response to this?
Sorry, I missed this one Actually I have created higher level API in ATF but I switch back to generic writes because the higher level API in ATF doesn't apply to Gen5/A10 platforms. Here is what I will do in my revision in system manager driver: 1) drop misc_read/misc_write and use misc_ioctl instead in system manager 2) misc_ioctl() will support configuring EMAC/SDMMC 3) For SoC64 running at EL2 (non-secure), misc_iotctl() will invoke the ATF's 'high level' API 4) For Gen/A10 and SoC64 running at EL3 (secure), the driver just configure the EMAC/SDMMC registers in misc_iotcl() Is this better ?
Thanks, Simon
In my gen5 series to abstract the gen5 sysmgr, I have used 'ioctl' and 'call' from misc_ops to have an API.
Regards, Simon
- if (ret)
return -EIO;- return 0;
+} +#endif
+static int write32(u32 val, fdt_addr_t addr) { #if +!defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF)
- return secure_write32(val, addr);
+#else
- writel(val, addr);
- return 0;
+#endif +}
+static int altera_sysmgr_read(struct udevice *dev,
int offset, void *buf, int size) {- struct altera_sysmgr_priv *priv = dev_get_priv(dev);
- fdt_addr_t addr = priv->base_addr + offset;
- if (IS_OUT_OF_SYSMGR(addr, priv->base_addr + priv->base_size - size))
return -EINVAL;- if (size != sizeof(u32))
return -EIO;- *(u32 *)buf = readl(addr);
- return 0;
+}
+static int altera_sysmgr_write(struct udevice *dev, int offset,
const void *buf, int size)+{
- struct altera_sysmgr_priv *priv = dev_get_priv(dev);
- fdt_addr_t addr = priv->base_addr + offset;
- if (IS_OUT_OF_SYSMGR(addr, priv->base_addr + priv->base_size - size))
return -EINVAL;- if (size != sizeof(u32))
return -EIO;- return write32(*(u32 *)buf, addr); }
+static int altera_sysmgr_probe(struct udevice *dev) {
- struct altera_sysmgr_priv *priv = dev_get_priv(dev);
- fdt_addr_t addr;
- fdt_size_t size;
- addr = ofnode_get_addr_size_index(dev_ofnode(dev), 0, &size);
- if (addr == FDT_ADDR_T_NONE)
return -EINVAL;- priv->base_addr = addr;
- priv->base_size = size;
- return 0;
+}
+static const struct misc_ops altera_sysmgr_ops = {
- .read = altera_sysmgr_read,
- .write = altera_sysmgr_write,
+};
+static const struct udevice_id altera_sysmgr_ids[] = {
- { .compatible = "altr,sys-mgr" },
- {}
+};
+U_BOOT_DRIVER(altera_sysmgr) = {
- .name = "altr,sys-mgr",
- .id = UCLASS_MISC,
- .of_match = altera_sysmgr_ids,
- .priv_auto_alloc_size = sizeof(struct altera_sysmgr_priv),
- .probe = altera_sysmgr_probe,
- .ops = &altera_sysmgr_ops,
+};