[PATCH v4 11/21] misc: altera_sysmgr: Add Altera System Manager driver

Ang, Chee Hong chee.hong.ang at intel.com
Wed Mar 11 07:35:04 CET 2020


> Am 10.03.2020 um 17:42 schrieb Ang, Chee Hong:
> >> -----Original Message-----
> >> From: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >> Sent: Wednesday, March 11, 2020 12:17 AM
> >> To: Ang, Chee Hong <chee.hong.ang at intel.com>
> >> Cc: u-boot at lists.denx.de; Marek Vasut <marex at denx.de>; See, Chin
> >> Liang <chin.liang.see at intel.com>; Tan, Ley Foon
> >> <ley.foon.tan at intel.com>; Westergreen, Dalon
> >> <dalon.westergreen at intel.com>; Gong, Richard <richard.gong at 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 at intel.com:
> >>> From: Chee Hong Ang <chee.hong.ang at 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 at 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,
> >>> +};
> >>>
> >



More information about the U-Boot mailing list