> -----Original Message-----
> From: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
> Sent: Wednesday, March 11, 2020 12:17 AM
> To: Ang, Chee Hong <chee.hong....@intel.com>
> Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; See, Chin Liang
> <chin.liang....@intel.com>; Tan, Ley Foon <ley.foon....@intel.com>;
> Westergreen, Dalon <dalon.westergr...@intel.com>; Gong, Richard
> <richard.g...@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....@intel.com:
> > From: Chee Hong Ang <chee.hong....@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....@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?
>
> 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,
> > +};
> >
