| Message ID | 1583744842-24632-12-git-send-email-chee.hong.ang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enable ARM Trusted Firmware for U-Boot | expand |
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 > @@ -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, > +}; >
> -----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? > > 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, > > +}; > >
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. Ah, I missed that part of the series. I'm still reading it. Making gen5 use misc_read/misc_write seems a bit strange, but I can't think of a better way right now, either... > Can I know what does your gen5 sysmgr driver do ? I moved "pin init", "freezereq" and "get fpga ID" there to have less ad-hoc code in the main SPL file... The series where it's in targets moving as much as I can to DM drivers. Sadly, I still haven't found a way to make it fit into the gen5 SRAM, which is why I haven't posted it, yet... > I can change the name to avoid conflict but Gen5 will have 2 sysmgr drivers for different purposes. > Are you OK with that ? Hmm, I don't think that will work. That would mean binding 2 drivers to one ofnode. I can split the gen5 driver later and implement read/write like it's needed if this one gets applied as is. >> >>> @@ -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, >>> +}; >>> >
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? 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, >>> +}; >>> >
> 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, > >>> +}; > >>> > >
On 3/11/20 7:35 AM, Ang, Chee Hong wrote: [...] >>>> 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. ATF doesn't apply to Gen5/A10 either though ? > 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 ? Can't you configure everything in secure-mode and just not configure anything anymore in non-secure mode ?
> On 3/11/20 7:35 AM, Ang, Chee Hong wrote: > [...] > > >>>> 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. > > ATF doesn't apply to Gen5/A10 either though ? > > > 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 ? > Can't you configure everything in secure-mode and just not configure anything > anymore in non-secure mode ? Yes. I can move all these configurations to SPL(secure mode) and remove them from EMAC/SDMMC drivers. This will affect all platforms even Gen5/A10 even they don?t have the secure access problems. All Gen5/A10/S10/Agilex share the same EMAC/SDMMC drivers. For EMAC driver such as 'drivers/net/dwmac_socfpga.c', moving the PHY settings into SPL will leave this EMAC driver just asserting reset to EMAC controller and nothing else. EMAC node has to be enabled for SPL device tree as well for MAC PHY configuration. If you think it is OK to split the SDMMC clock and EMAC PHY configuration from SDMMC and EMAC drivers and put them in SPL, we can go this way. I can just drop the 'system manager' driver and all those high level APIs in ATF. > > -- > Best regards, > Marek Vasut
On 3/11/20 8:03 AM, Ang, Chee Hong wrote: >> On 3/11/20 7:35 AM, Ang, Chee Hong wrote: >> [...] >> >>>>>> 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. >> >> ATF doesn't apply to Gen5/A10 either though ? >> >>> 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 ? >> Can't you configure everything in secure-mode and just not configure anything >> anymore in non-secure mode ? > Yes. I can move all these configurations to SPL(secure mode) and remove them from EMAC/SDMMC drivers. > This will affect all platforms even Gen5/A10 even they don?t have the secure access problems. Gen5/A10 are always in "secure" mode. > All Gen5/A10/S10/Agilex share the same EMAC/SDMMC drivers. Surely you can abstract this away somehow, e.g. with some function which is compiled-out on Gen5/A10, while it's compiled-in on Agilex and tells you whether you're in EL2/EL3 mode. > For EMAC driver such as 'drivers/net/dwmac_socfpga.c', moving the PHY settings into SPL > will leave this EMAC driver just asserting reset to EMAC controller and nothing else. > EMAC node has to be enabled for SPL device tree as well for MAC PHY configuration. > If you think it is OK to split the SDMMC clock and EMAC PHY configuration from SDMMC and EMAC drivers > and put them in SPL, we can go this way. > I can just drop the 'system manager' driver and all those high level APIs in ATF. If this is only about clock/PHY configuration, can't the clock/PHY driver for agilex just handle the EL2/EL3 stuff transparently ?
> On 3/11/20 8:03 AM, Ang, Chee Hong wrote: > >> On 3/11/20 7:35 AM, Ang, Chee Hong wrote: > >> [...] > >> > >>>>>> 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. > >> > >> ATF doesn't apply to Gen5/A10 either though ? > >> > >>> 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 ? > >> Can't you configure everything in secure-mode and just not configure > >> anything anymore in non-secure mode ? > > Yes. I can move all these configurations to SPL(secure mode) and remove them > from EMAC/SDMMC drivers. > > This will affect all platforms even Gen5/A10 even they don?t have the secure > access problems. > > Gen5/A10 are always in "secure" mode. > > > All Gen5/A10/S10/Agilex share the same EMAC/SDMMC drivers. > > Surely you can abstract this away somehow, e.g. with some function which is > compiled-out on Gen5/A10, while it's compiled-in on Agilex and tells you > whether you're in EL2/EL3 mode. That's right. We have Gen5/A10 always in "secure" mode and S10/Agilex can be in either "secure" or "non-secure" mode. All of them share the same DW MMC/MAC drivers. In all platforms, EMAC driver only active in U-Boot proper but not SPL. This is not an issue for Gen5/A10 as SPL/U-Boot proper all run in "secure" mode. But this is not the case for S10/Agilex, EMAC driver is active only in U-Boot proper which can be EL2 or EL3 depending whether you include ATF support. So the MAC driver has to somehow handle this PHY configuration in EL2 and EL3. > > > For EMAC driver such as 'drivers/net/dwmac_socfpga.c', moving the PHY > > settings into SPL will leave this EMAC driver just asserting reset to EMAC > controller and nothing else. > > EMAC node has to be enabled for SPL device tree as well for MAC PHY > configuration. > > If you think it is OK to split the SDMMC clock and EMAC PHY > > configuration from SDMMC and EMAC drivers and put them in SPL, we can go > this way. > > I can just drop the 'system manager' driver and all those high level APIs in ATF. > > If this is only about clock/PHY configuration, can't the clock/PHY driver for > agilex just handle the EL2/EL3 stuff transparently ? This is the clock phase settings specific to SDMMC controller. That's why it is being configured in SDMCC driver instead of clock driver. There is no PHY driver for EMAC PHY. It's current being handled in DW MAC driver 'drivers/net/dwmac_socfpga.c'. > -- > Best regards, > Marek Vasut
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 @@ -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); + 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, +};