| Message ID | 20210918135437.36667-4-kettenis@openbsd.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Apple M1 Support | expand |
Hi Mark, On Sat, 18 Sept 2021 at 07:55, Mark Kettenis <kettenis@openbsd.org> wrote: > > The DART is an IOMMU that is used on Apple's M1 SoC. This driver > supports the DART in bypass mode as well as in a mode where it > creates a 1:1 mapping of a subset of RAM as not all DARTs support > bypass mode. The USB3 ports integrated on the SoC use a DART > that supports bypass mode. The 1:1 mapping will be used in the > future to support other devices such as the PCIe host bridge > of the M1 SoC. > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > --- > drivers/misc/Kconfig | 7 ++ > drivers/misc/Makefile | 1 + > drivers/misc/apple_dart.c | 171 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 179 insertions(+) > create mode 100644 drivers/misc/apple_dart.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 997b713221..d70b060e74 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -38,6 +38,13 @@ config ALTERA_SYSID > Select this to enable a sysid for Altera devices. Please find > details on the "Embedded Peripherals IP User Guide" of Altera. > > +config APPLE_DART > + bool "Apple DART support" > + depends on MISC && ARCH_APPLE > + default y > + help > + Enable support for the DART on Apple SoCs. Should have at least 3 lines. E.g. what does DART stand for, what does it do and what does the driver support? > + > config ATSHA204A > bool "Support for Atmel ATSHA204A module" > depends on MISC > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index b64cd2a4de..f666cd392d 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_APPLE_DART) += apple_dart.o > obj-$(CONFIG_ATSHA204A) += atsha204a-i2c.o > obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o > obj-$(CONFIG_DS4510) += ds4510.o > diff --git a/drivers/misc/apple_dart.c b/drivers/misc/apple_dart.c > new file mode 100644 > index 0000000000..f619a624d0 > --- /dev/null > +++ b/drivers/misc/apple_dart.c > @@ -0,0 +1,171 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> > + */ > + > +#include <common.h> > +#include <clk.h> > +#include <cpu_func.h> > +#include <dm.h> > +#include <mapmem.h> > +#include <asm/io.h> > + > +#define DART_PARAMS2 0x0004 > +#define DART_PARAMS2_BYPASS_SUPPORT BIT(0) > +#define DART_TLB_OP 0x0020 > +#define DART_TLB_OP_OPMASK (0xfff << 20) > +#define DART_TLB_OP_FLUSH (0x001 << 20) > +#define DART_TLB_OP_BUSY BIT(2) > +#define DART_TLB_OP_SIDMASK 0x0034 > +#define DART_ERROR_STATUS 0x0040 > +#define DART_TCR(sid) (0x0100 + 4 * (sid)) > +#define DART_TCR_TRANSLATE_ENABLE BIT(7) > +#define DART_TCR_BYPASS_DART BIT(8) > +#define DART_TCR_BYPASS_DAPF BIT(12) > +#define DART_TTBR(sid, idx) (0x0200 + 16 * (sid) + 4 * (idx)) > +#define DART_TTBR_VALID BIT(31) > +#define DART_TTBR_SHIFT 12 > + > +struct apple_dart_priv { How about s/apple_dart/dart/ ? It makes the code easier to read. > + struct clk_bulk clks; > + void *base; > +}; > + > +dma_addr_t apple_dart_bus_start; > +phys_addr_t apple_dart_phys_start; > +phys_size_t apple_dart_size = SZ_512M; Try to avoid variables in drivers. Can these go in a priv struct? > + > +static void apple_dart_flush_tlb(struct apple_dart_priv *priv) comments on these functions > +{ > + u32 status; > + > + writel(0xffffffff, priv->base + DART_TLB_OP_SIDMASK); > + writel(DART_TLB_OP_FLUSH, priv->base + DART_TLB_OP); > + > + for (;;) { > + status = readl(priv->base + DART_TLB_OP); > + if ((status & DART_TLB_OP_OPMASK) == 0) > + break; > + if ((status & DART_TLB_OP_BUSY) == 0) > + break; > + } > +} > + > +static int apple_dart_clk_init(struct udevice *dev, > + struct apple_dart_priv *priv) > +{ > + int ret; > + > + ret = clk_get_bulk(dev, &priv->clks); > + if (ret == -ENOSYS || ret == -ENOENT) Does -ENOSYS not indicate an error? If it doesn't, I think a comment would help here. > + return 0; > + if (ret) > + return ret; > + > + ret = clk_enable_bulk(&priv->clks); > + if (ret) { > + clk_release_bulk(&priv->clks); > + return ret; > + } > + > + return 0; > +} > + > +static int apple_dart_bind(struct udevice *dev) > +{ > + void *base; > + int sid, i; > + > + base = dev_read_addr_ptr(dev); > + if (!base) > + return -EINVAL; > + > + u32 params2 = readl(base + DART_PARAMS2); > + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { > + for (sid = 0; sid < 16; sid++) { > + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, > + base + DART_TCR(sid)); > + for (i = 0; i < 4; i++) > + writel(0, base + DART_TTBR(sid, i)); > + } > + } Not allowed hardware access in bind(). Can this more to probe() ? > + > + return 0; > +} > + > +static int apple_dart_probe(struct udevice *dev) > +{ > + struct apple_dart_priv *priv = dev_get_priv(dev); > + phys_addr_t phys; > + u64 *l1, *l2; > + int sid, i, j; > + int ret; > + > + apple_dart_phys_start = gd->ram_top - apple_dart_size; > + > + priv->base = dev_read_addr_ptr(dev); > + if (!priv->base) > + return -EINVAL; > + > + ret = apple_dart_clk_init(dev, priv); > + if (ret) > + return ret; > + > + l1 = memalign(SZ_64K, SZ_64K); > + memset(l1, 0, SZ_64K); > + > + i = 0; > + phys = apple_dart_phys_start; > + while (phys < apple_dart_phys_start + apple_dart_size) { > + l2 = memalign(SZ_16K, SZ_16K); check for error > + memset(l2, 0, SZ_16K); > + > + for (j = 0; j < 2048; j++) { > + l2[j] = phys | 0x3; > + phys += SZ_16K; > + } > + flush_dcache_range((unsigned long)l2, > + (unsigned long)l2 + SZ_16K); > + > + l1[i++] = (phys_addr_t)l2 | 0x8 | 0x3; Do you need the cast? What are the magic numbers here? Can you use an enum/#define ? > + } > + > + flush_dcache_range((unsigned long)l1, (unsigned long)l1 + SZ_64K); > + > + for (sid = 0; sid < 16; sid++) { comment,...what is this doing? > + for (i = 0; i < 4; i++) > + writel(0, priv->base + DART_TTBR(sid, i)); > + } > + > + apple_dart_flush_tlb(priv); > + > + for (sid = 0; sid < 16; sid++) { > + phys = (phys_addr_t)l1; > + for (i = 0; i < 4; i++) { > + writel((phys >> DART_TTBR_SHIFT) | DART_TTBR_VALID, > + priv->base + DART_TTBR(sid, i)); > + phys += SZ_16K; > + } > + } > + > + apple_dart_flush_tlb(priv); > + > + for (sid = 0; sid < 16; sid++) > + writel(DART_TCR_TRANSLATE_ENABLE, priv->base + DART_TCR(sid)); > + > + return 0; > +} > + > +static const struct udevice_id apple_dart_ids[] = { > + { .compatible = "apple,t8103-dart" }, > + { /* sentinel */ } > +}; > + > +U_BOOT_DRIVER(apple_dart) = { > + .name = "apple_dart", > + .id = UCLASS_MISC, > + .of_match = apple_dart_ids, > + .priv_auto = sizeof(struct apple_dart_priv), > + .bind = apple_dart_bind, > + .probe = apple_dart_probe > +}; > -- > 2.33.0 > Regards, Simon
> From: Simon Glass <sjg@chromium.org> > Date: Sun, 19 Sep 2021 21:16:00 -0600 > > Hi Mark, > > On Sat, 18 Sept 2021 at 07:55, Mark Kettenis <kettenis@openbsd.org> wrote: > > > > The DART is an IOMMU that is used on Apple's M1 SoC. This driver > > supports the DART in bypass mode as well as in a mode where it > > creates a 1:1 mapping of a subset of RAM as not all DARTs support > > bypass mode. The USB3 ports integrated on the SoC use a DART > > that supports bypass mode. The 1:1 mapping will be used in the > > future to support other devices such as the PCIe host bridge > > of the M1 SoC. > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > --- > > drivers/misc/Kconfig | 7 ++ > > drivers/misc/Makefile | 1 + > > drivers/misc/apple_dart.c | 171 ++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 179 insertions(+) > > create mode 100644 drivers/misc/apple_dart.c > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > index 997b713221..d70b060e74 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -38,6 +38,13 @@ config ALTERA_SYSID > > Select this to enable a sysid for Altera devices. Please find > > details on the "Embedded Peripherals IP User Guide" of Altera. > > > > +config APPLE_DART > > + bool "Apple DART support" > > + depends on MISC && ARCH_APPLE > > + default y > > + help > > + Enable support for the DART on Apple SoCs. > > Should have at least 3 lines. E.g. what does DART stand for, what does > it do and what does the driver support? Sure. The DART is what we usually call an IOMMU. > > + > > config ATSHA204A > > bool "Support for Atmel ATSHA204A module" > > depends on MISC > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > index b64cd2a4de..f666cd392d 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_APPLE_DART) += apple_dart.o > > obj-$(CONFIG_ATSHA204A) += atsha204a-i2c.o > > obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o > > obj-$(CONFIG_DS4510) += ds4510.o > > diff --git a/drivers/misc/apple_dart.c b/drivers/misc/apple_dart.c > > new file mode 100644 > > index 0000000000..f619a624d0 > > --- /dev/null > > +++ b/drivers/misc/apple_dart.c > > @@ -0,0 +1,171 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> > > + */ > > + > > +#include <common.h> > > +#include <clk.h> > > +#include <cpu_func.h> > > +#include <dm.h> > > +#include <mapmem.h> > > +#include <asm/io.h> > > + > > +#define DART_PARAMS2 0x0004 > > +#define DART_PARAMS2_BYPASS_SUPPORT BIT(0) > > +#define DART_TLB_OP 0x0020 > > +#define DART_TLB_OP_OPMASK (0xfff << 20) > > +#define DART_TLB_OP_FLUSH (0x001 << 20) > > +#define DART_TLB_OP_BUSY BIT(2) > > +#define DART_TLB_OP_SIDMASK 0x0034 > > +#define DART_ERROR_STATUS 0x0040 > > +#define DART_TCR(sid) (0x0100 + 4 * (sid)) > > +#define DART_TCR_TRANSLATE_ENABLE BIT(7) > > +#define DART_TCR_BYPASS_DART BIT(8) > > +#define DART_TCR_BYPASS_DAPF BIT(12) > > +#define DART_TTBR(sid, idx) (0x0200 + 16 * (sid) + 4 * (idx)) > > +#define DART_TTBR_VALID BIT(31) > > +#define DART_TTBR_SHIFT 12 > > + > > +struct apple_dart_priv { > > How about s/apple_dart/dart/ ? > > It makes the code easier to read. I think using apple_dart_ consistently as a prefix makes more sense. > > + struct clk_bulk clks; > > + void *base; > > +}; > > + > > +dma_addr_t apple_dart_bus_start; > > +phys_addr_t apple_dart_phys_start; > > +phys_size_t apple_dart_size = SZ_512M; > > Try to avoid variables in drivers. Can these go in a priv struct? Not really since the intent is that these variables specify a global "window" that is mapped 1:1 into all the DARTs. > > + > > +static void apple_dart_flush_tlb(struct apple_dart_priv *priv) > > comments on these functions > > > +{ > > + u32 status; > > + > > + writel(0xffffffff, priv->base + DART_TLB_OP_SIDMASK); > > + writel(DART_TLB_OP_FLUSH, priv->base + DART_TLB_OP); > > + > > + for (;;) { > > + status = readl(priv->base + DART_TLB_OP); > > + if ((status & DART_TLB_OP_OPMASK) == 0) > > + break; > > + if ((status & DART_TLB_OP_BUSY) == 0) > > + break; > > + } > > +} > > + > > +static int apple_dart_clk_init(struct udevice *dev, > > + struct apple_dart_priv *priv) > > +{ > > + int ret; > > + > > + ret = clk_get_bulk(dev, &priv->clks); > > + if (ret == -ENOSYS || ret == -ENOENT) > > Does -ENOSYS not indicate an error? If it doesn't, I think a comment > would help here. So we just learned that what we previously considered to be clocks are really better modelled as power domains. So this code will go away. > > + return 0; > > + if (ret) > > + return ret; > > + > > + ret = clk_enable_bulk(&priv->clks); > > + if (ret) { > > + clk_release_bulk(&priv->clks); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int apple_dart_bind(struct udevice *dev) > > +{ > > + void *base; > > + int sid, i; > > + > > + base = dev_read_addr_ptr(dev); > > + if (!base) > > + return -EINVAL; > > + > > + u32 params2 = readl(base + DART_PARAMS2); > > + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { > > + for (sid = 0; sid < 16; sid++) { > > + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, > > + base + DART_TCR(sid)); > > + for (i = 0; i < 4; i++) > > + writel(0, base + DART_TTBR(sid, i)); > > + } > > + } > > Not allowed hardware access in bind(). Can this more to probe() ? Well, I need to make sure that this happens before other drivers get probed (in particular the xhci-dwc3 driver). Is there a better mechanism to achieve that? > > + > > + return 0; > > +} > > + > > +static int apple_dart_probe(struct udevice *dev) > > +{ > > + struct apple_dart_priv *priv = dev_get_priv(dev); > > + phys_addr_t phys; > > + u64 *l1, *l2; > > + int sid, i, j; > > + int ret; > > + > > + apple_dart_phys_start = gd->ram_top - apple_dart_size; > > + > > + priv->base = dev_read_addr_ptr(dev); > > + if (!priv->base) > > + return -EINVAL; > > + > > + ret = apple_dart_clk_init(dev, priv); > > + if (ret) > > + return ret; > > + > > + l1 = memalign(SZ_64K, SZ_64K); > > + memset(l1, 0, SZ_64K); > > + > > + i = 0; > > + phys = apple_dart_phys_start; > > + while (phys < apple_dart_phys_start + apple_dart_size) { > > + l2 = memalign(SZ_16K, SZ_16K); > > check for error > > > + memset(l2, 0, SZ_16K); > > + > > + for (j = 0; j < 2048; j++) { > > + l2[j] = phys | 0x3; > > + phys += SZ_16K; > > + } > > + flush_dcache_range((unsigned long)l2, > > + (unsigned long)l2 + SZ_16K); > > + > > + l1[i++] = (phys_addr_t)l2 | 0x8 | 0x3; > > Do you need the cast? What are the magic numbers here? Can you use an > enum/#define ? Not sure if we know the exact meaning of those bits yet. But there is a Linux driver now, so maybe I need to look at it again. > > + } > > + > > + flush_dcache_range((unsigned long)l1, (unsigned long)l1 + SZ_64K); > > + > > + for (sid = 0; sid < 16; sid++) { > > comment,...what is this doing? > > > + for (i = 0; i < 4; i++) > > + writel(0, priv->base + DART_TTBR(sid, i)); > > + } > > + > > + apple_dart_flush_tlb(priv); > > + > > + for (sid = 0; sid < 16; sid++) { > > + phys = (phys_addr_t)l1; > > + for (i = 0; i < 4; i++) { > > + writel((phys >> DART_TTBR_SHIFT) | DART_TTBR_VALID, > > + priv->base + DART_TTBR(sid, i)); > > + phys += SZ_16K; > > + } > > + } > > + > > + apple_dart_flush_tlb(priv); > > + > > + for (sid = 0; sid < 16; sid++) > > + writel(DART_TCR_TRANSLATE_ENABLE, priv->base + DART_TCR(sid)); > > + > > + return 0; > > +} > > + > > +static const struct udevice_id apple_dart_ids[] = { > > + { .compatible = "apple,t8103-dart" }, > > + { /* sentinel */ } > > +}; > > + > > +U_BOOT_DRIVER(apple_dart) = { > > + .name = "apple_dart", > > + .id = UCLASS_MISC, > > + .of_match = apple_dart_ids, > > + .priv_auto = sizeof(struct apple_dart_priv), > > + .bind = apple_dart_bind, > > + .probe = apple_dart_probe > > +}; > > -- > > 2.33.0 > > > > Regards, > Simon >
Hi Mark, On Mon, 20 Sept 2021 at 02:33, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Simon Glass <sjg@chromium.org> > > Date: Sun, 19 Sep 2021 21:16:00 -0600 > > > > Hi Mark, > > > > On Sat, 18 Sept 2021 at 07:55, Mark Kettenis <kettenis@openbsd.org> wrote: > > > > > > The DART is an IOMMU that is used on Apple's M1 SoC. This driver > > > supports the DART in bypass mode as well as in a mode where it > > > creates a 1:1 mapping of a subset of RAM as not all DARTs support > > > bypass mode. The USB3 ports integrated on the SoC use a DART > > > that supports bypass mode. The 1:1 mapping will be used in the > > > future to support other devices such as the PCIe host bridge > > > of the M1 SoC. > > > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > > --- > > > drivers/misc/Kconfig | 7 ++ > > > drivers/misc/Makefile | 1 + > > > drivers/misc/apple_dart.c | 171 ++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 179 insertions(+) > > > create mode 100644 drivers/misc/apple_dart.c > > > [..] > > > + > > > +struct apple_dart_priv { > > > > How about s/apple_dart/dart/ ? > > > > It makes the code easier to read. > > I think using apple_dart_ consistently as a prefix makes more sense. OK I don't mind too much. I just prefer shorter code and sometimes people use globally unique things in drivers when it really doesn't help anything. > > > > + struct clk_bulk clks; > > > + void *base; > > > +}; > > > + > > > +dma_addr_t apple_dart_bus_start; > > > +phys_addr_t apple_dart_phys_start; > > > +phys_size_t apple_dart_size = SZ_512M; > > > > Try to avoid variables in drivers. Can these go in a priv struct? > > Not really since the intent is that these variables specify a global > "window" that is mapped 1:1 into all the DARTs. So there are multiple DART devices? In that can you could store this info in a priv struct attached to the uclass. [..] > > > +static int apple_dart_bind(struct udevice *dev) > > > +{ > > > + void *base; > > > + int sid, i; > > > + > > > + base = dev_read_addr_ptr(dev); > > > + if (!base) > > > + return -EINVAL; > > > + > > > + u32 params2 = readl(base + DART_PARAMS2); > > > + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { > > > + for (sid = 0; sid < 16; sid++) { > > > + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, > > > + base + DART_TCR(sid)); > > > + for (i = 0; i < 4; i++) > > > + writel(0, base + DART_TTBR(sid, i)); > > > + } > > > + } > > > > Not allowed hardware access in bind(). Can this more to probe() ? > > Well, I need to make sure that this happens before other drivers get > probed (in particular the xhci-dwc3 driver). Is there a better > mechanism to achieve that? If those drivers have something in the DT indicating that they need this, then you can add a uclass_get_device_by_phandle() in those drivers. If not, then you can probe all the DART devices with uclass_probe_all(). Having said that, I see you are using UCLASS_MISC. I suspect this should have its own UCLASS_IOMMU. [..] > > > > > + memset(l2, 0, SZ_16K); > > > + > > > + for (j = 0; j < 2048; j++) { > > > + l2[j] = phys | 0x3; > > > + phys += SZ_16K; > > > + } > > > + flush_dcache_range((unsigned long)l2, > > > + (unsigned long)l2 + SZ_16K); > > > + > > > + l1[i++] = (phys_addr_t)l2 | 0x8 | 0x3; > > > > Do you need the cast? What are the magic numbers here? Can you use an > > enum/#define ? > > Not sure if we know the exact meaning of those bits yet. But there is > a Linux driver now, so maybe I need to look at it again. Oh well, a comment that we don't know is fine, too. People can fix it later if it becomes known. [..] Regards, Simon
On Mon, 20 Sept 2021 at 19:11, Simon Glass <sjg@chromium.org> wrote: > > Hi Mark, > > On Mon, 20 Sept 2021 at 02:33, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > > > From: Simon Glass <sjg@chromium.org> > > > Date: Sun, 19 Sep 2021 21:16:00 -0600 > > > > > > Hi Mark, > > > > > > On Sat, 18 Sept 2021 at 07:55, Mark Kettenis <kettenis@openbsd.org> wrote: > > > > > > > > The DART is an IOMMU that is used on Apple's M1 SoC. This driver > > > > supports the DART in bypass mode as well as in a mode where it > > > > creates a 1:1 mapping of a subset of RAM as not all DARTs support > > > > bypass mode. The USB3 ports integrated on the SoC use a DART > > > > that supports bypass mode. The 1:1 mapping will be used in the > > > > future to support other devices such as the PCIe host bridge > > > > of the M1 SoC. > > > > > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org> > > > > --- > > > > drivers/misc/Kconfig | 7 ++ > > > > drivers/misc/Makefile | 1 + > > > > drivers/misc/apple_dart.c | 171 ++++++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 179 insertions(+) > > > > create mode 100644 drivers/misc/apple_dart.c > > > > > Tested on: Macbook Air M1 Tested-by: Simon Glass <sjg@chromium.org>
> From: Simon Glass <sjg@chromium.org> > Date: Mon, 20 Sep 2021 19:11:25 -0600 Hi Simon, > [..] > > > > > +static int apple_dart_bind(struct udevice *dev) > > > > +{ > > > > + void *base; > > > > + int sid, i; > > > > + > > > > + base = dev_read_addr_ptr(dev); > > > > + if (!base) > > > > + return -EINVAL; > > > > + > > > > + u32 params2 = readl(base + DART_PARAMS2); > > > > + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { > > > > + for (sid = 0; sid < 16; sid++) { > > > > + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, > > > > + base + DART_TCR(sid)); > > > > + for (i = 0; i < 4; i++) > > > > + writel(0, base + DART_TTBR(sid, i)); > > > > + } > > > > + } > > > > > > Not allowed hardware access in bind(). Can this more to probe() ? > > > > Well, I need to make sure that this happens before other drivers get > > probed (in particular the xhci-dwc3 driver). Is there a better > > mechanism to achieve that? > > If those drivers have something in the DT indicating that they need > this, then you can add a uclass_get_device_by_phandle() in those > drivers. Yes, there is an "iommus" property that points at the IOMMU. This is a standard property and I think it would make sense to have the core device probing code handle it in a way similar to how "power-domains" is handled. That way we don't have to add IOMMU hooks into each and every driver. > If not, then you can probe all the DART devices with uclass_probe_all(). > > Having said that, I see you are using UCLASS_MISC. I suspect this > should have its own UCLASS_IOMMU. So here is a proof-of-concept diff to add UCLASS_IOMMU. This deliberately does not yet introduce driver ops yet; coming up with something sensible there needs a bit more thought. For USB support all I need is for the probe function to put the IOMMU in bypass mode, so that is what's implemented here. Does this look reasonable? I'll split this up eventually into a commit that adds UCLASS_IOMMU and a commit that adds the apple_dart driver. commit 606979a32d8e0d19ee45df97ad6855c24e27e5ad Author: Mark Kettenis <kettenis@openbsd.org> Date: Sun Sep 26 22:35:30 2021 +0200 iommu: Proof of concept diff --git a/drivers/Kconfig b/drivers/Kconfig index 417d6f88c2..b26ca8cf70 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -50,6 +50,8 @@ source "drivers/i2c/Kconfig" source "drivers/input/Kconfig" +source "drivers/iommu/Kconfig" + source "drivers/led/Kconfig" source "drivers/mailbox/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index fd218c9056..166aeb9817 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -109,6 +109,7 @@ obj-y += mtd/ obj-y += pwm/ obj-y += reset/ obj-y += input/ +obj-y += iommu/ # SOC specific infrastructure drivers. obj-y += smem/ obj-y += thermal/ diff --git a/drivers/core/device.c b/drivers/core/device.c index 29668f6fb3..5f480ad443 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -28,6 +28,7 @@ #include <dm/uclass.h> #include <dm/uclass-internal.h> #include <dm/util.h> +#include <iommu.h> #include <linux/err.h> #include <linux/list.h> #include <power-domain.h> @@ -543,6 +544,13 @@ int device_probe(struct udevice *dev) goto fail; } + if (CONFIG_IS_ENABLED(IOMMU) && dev->parent && + (device_get_uclass_id(dev) != UCLASS_IOMMU)) { + ret = dev_iommu_probe(dev); + if (ret) + goto fail; + } + ret = device_get_dma_constraints(dev); if (ret) goto fail; diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig new file mode 100644 index 0000000000..d0638140af --- /dev/null +++ b/drivers/iommu/Kconfig @@ -0,0 +1,20 @@ +# +# IOMMU devices +# + +menu "IOMMU device drivers" + +config IOMMU + bool "Enable Driver Model for IOMMU drivers" + depends on DM + help + Enable driver model for IOMMU devices. + +config APPLE_DART + bool "Apple DART support" + depends on IOMMU && ARCH_APPLE + default y + help + Enable support for the DART on Apple SoCs. + +endmenu diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile new file mode 100644 index 0000000000..8a50e3aba8 --- /dev/null +++ b/drivers/iommu/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_IOMMU) += iommu-uclass.o + +obj-$(CONFIG_APPLE_DART) += apple_dart.o diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c new file mode 100644 index 0000000000..ff8c5fa62c --- /dev/null +++ b/drivers/iommu/apple_dart.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> + */ + +#include <common.h> +#include <cpu_func.h> +#include <dm.h> +#include <asm/io.h> + +#define DART_PARAMS2 0x0004 +#define DART_PARAMS2_BYPASS_SUPPORT BIT(0) +#define DART_TLB_OP 0x0020 +#define DART_TLB_OP_OPMASK (0xfff << 20) +#define DART_TLB_OP_FLUSH (0x001 << 20) +#define DART_TLB_OP_BUSY BIT(2) +#define DART_TLB_OP_SIDMASK 0x0034 +#define DART_ERROR_STATUS 0x0040 +#define DART_TCR(sid) (0x0100 + 4 * (sid)) +#define DART_TCR_TRANSLATE_ENABLE BIT(7) +#define DART_TCR_BYPASS_DART BIT(8) +#define DART_TCR_BYPASS_DAPF BIT(12) +#define DART_TTBR(sid, idx) (0x0200 + 16 * (sid) + 4 * (idx)) +#define DART_TTBR_VALID BIT(31) +#define DART_TTBR_SHIFT 12 + +static int apple_dart_probe(struct udevice *dev) +{ + void *base; + int sid, i; + + base = dev_read_addr_ptr(dev); + if (!base) + return -EINVAL; + + u32 params2 = readl(base + DART_PARAMS2); + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { + for (sid = 0; sid < 16; sid++) { + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, + base + DART_TCR(sid)); + for (i = 0; i < 4; i++) + writel(0, base + DART_TTBR(sid, i)); + } + } + + return 0; +} + +static const struct udevice_id apple_dart_ids[] = { + { .compatible = "apple,t8103-dart" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(apple_dart) = { + .name = "apple_dart", + .id = UCLASS_IOMMU, + .of_match = apple_dart_ids, + .probe = apple_dart_probe +}; diff --git a/drivers/iommu/iommu-uclass.c b/drivers/iommu/iommu-uclass.c new file mode 100644 index 0000000000..5c55df3066 --- /dev/null +++ b/drivers/iommu/iommu-uclass.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> + */ + +#define LOG_CATEGORY UCLASS_IOMMU + +#include <common.h> +#include <dm.h> + +#if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) +int dev_iommu_probe(struct udevice *dev) +{ + struct ofnode_phandle_args args; + struct udevice *dev_iommu; + int i, count, ret = 0; + + count = dev_count_phandle_with_args(dev, "iommus", + "#iommu-cells", 0); + for (i = 0; i < count; i++) { + ret = dev_read_phandle_with_args(dev, "iommus", + "#iommu-cells", 0, i, &args); + if (ret) { + debug("%s: dev_read_phandle_with_args failed: %d\n", + __func__, ret); + return ret; + } + + ret = uclass_get_device_by_ofnode(UCLASS_IOMMU, args.node, + &dev_iommu); + if (ret) { + debug("%s: uclass_get_device_by_ofnode failed: %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} +#endif + +UCLASS_DRIVER(iommu) = { + .id = UCLASS_IOMMU, + .name = "iommu", +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index e7edd409f3..56aa981613 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -61,6 +61,7 @@ enum uclass_id { UCLASS_I2C_MUX, /* I2C multiplexer */ UCLASS_I2S, /* I2S bus */ UCLASS_IDE, /* IDE device */ + UCLASS_IOMMU, /* IOMMU */ UCLASS_IRQ, /* Interrupt controller */ UCLASS_KEYBOARD, /* Keyboard input device */ UCLASS_LED, /* Light-emitting diode (LED) */
Hi Mark, On Sun, 26 Sept 2021 at 14:53, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Simon Glass <sjg@chromium.org> > > Date: Mon, 20 Sep 2021 19:11:25 -0600 > > Hi Simon, > > > [..] > > > > > > > +static int apple_dart_bind(struct udevice *dev) > > > > > +{ > > > > > + void *base; > > > > > + int sid, i; > > > > > + > > > > > + base = dev_read_addr_ptr(dev); > > > > > + if (!base) > > > > > + return -EINVAL; > > > > > + > > > > > + u32 params2 = readl(base + DART_PARAMS2); > > > > > + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { > > > > > + for (sid = 0; sid < 16; sid++) { > > > > > + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, > > > > > + base + DART_TCR(sid)); > > > > > + for (i = 0; i < 4; i++) > > > > > + writel(0, base + DART_TTBR(sid, i)); > > > > > + } > > > > > + } > > > > > > > > Not allowed hardware access in bind(). Can this more to probe() ? > > > > > > Well, I need to make sure that this happens before other drivers get > > > probed (in particular the xhci-dwc3 driver). Is there a better > > > mechanism to achieve that? > > > > If those drivers have something in the DT indicating that they need > > this, then you can add a uclass_get_device_by_phandle() in those > > drivers. > > Yes, there is an "iommus" property that points at the IOMMU. This is > a standard property and I think it would make sense to have the core > device probing code handle it in a way similar to how "power-domains" > is handled. That way we don't have to add IOMMU hooks into each and > every driver. OK. > > > If not, then you can probe all the DART devices with uclass_probe_all(). > > > > Having said that, I see you are using UCLASS_MISC. I suspect this > > should have its own UCLASS_IOMMU. > > So here is a proof-of-concept diff to add UCLASS_IOMMU. This > deliberately does not yet introduce driver ops yet; coming up with > something sensible there needs a bit more thought. For USB support > all I need is for the probe function to put the IOMMU in bypass mode, > so that is what's implemented here. > > Does this look reasonable? I'll split this up eventually into a > commit that adds UCLASS_IOMMU and a commit that adds the apple_dart > driver. Yes from a quick look. Remember for the IOMMU uclass to add a sandbox test, even though for now it will only probe the driver since there are no ops. Also you don't need the #if in iommu-uclass.c as the linked will garbage-collect it. Regards, Simon
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 997b713221..d70b060e74 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -38,6 +38,13 @@ config ALTERA_SYSID Select this to enable a sysid for Altera devices. Please find details on the "Embedded Peripherals IP User Guide" of Altera. +config APPLE_DART + bool "Apple DART support" + depends on MISC && ARCH_APPLE + default y + help + Enable support for the DART on Apple SoCs. + config ATSHA204A bool "Support for Atmel ATSHA204A module" depends on MISC diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index b64cd2a4de..f666cd392d 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_APPLE_DART) += apple_dart.o obj-$(CONFIG_ATSHA204A) += atsha204a-i2c.o obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o obj-$(CONFIG_DS4510) += ds4510.o diff --git a/drivers/misc/apple_dart.c b/drivers/misc/apple_dart.c new file mode 100644 index 0000000000..f619a624d0 --- /dev/null +++ b/drivers/misc/apple_dart.c @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> + */ + +#include <common.h> +#include <clk.h> +#include <cpu_func.h> +#include <dm.h> +#include <mapmem.h> +#include <asm/io.h> + +#define DART_PARAMS2 0x0004 +#define DART_PARAMS2_BYPASS_SUPPORT BIT(0) +#define DART_TLB_OP 0x0020 +#define DART_TLB_OP_OPMASK (0xfff << 20) +#define DART_TLB_OP_FLUSH (0x001 << 20) +#define DART_TLB_OP_BUSY BIT(2) +#define DART_TLB_OP_SIDMASK 0x0034 +#define DART_ERROR_STATUS 0x0040 +#define DART_TCR(sid) (0x0100 + 4 * (sid)) +#define DART_TCR_TRANSLATE_ENABLE BIT(7) +#define DART_TCR_BYPASS_DART BIT(8) +#define DART_TCR_BYPASS_DAPF BIT(12) +#define DART_TTBR(sid, idx) (0x0200 + 16 * (sid) + 4 * (idx)) +#define DART_TTBR_VALID BIT(31) +#define DART_TTBR_SHIFT 12 + +struct apple_dart_priv { + struct clk_bulk clks; + void *base; +}; + +dma_addr_t apple_dart_bus_start; +phys_addr_t apple_dart_phys_start; +phys_size_t apple_dart_size = SZ_512M; + +static void apple_dart_flush_tlb(struct apple_dart_priv *priv) +{ + u32 status; + + writel(0xffffffff, priv->base + DART_TLB_OP_SIDMASK); + writel(DART_TLB_OP_FLUSH, priv->base + DART_TLB_OP); + + for (;;) { + status = readl(priv->base + DART_TLB_OP); + if ((status & DART_TLB_OP_OPMASK) == 0) + break; + if ((status & DART_TLB_OP_BUSY) == 0) + break; + } +} + +static int apple_dart_clk_init(struct udevice *dev, + struct apple_dart_priv *priv) +{ + int ret; + + ret = clk_get_bulk(dev, &priv->clks); + if (ret == -ENOSYS || ret == -ENOENT) + return 0; + if (ret) + return ret; + + ret = clk_enable_bulk(&priv->clks); + if (ret) { + clk_release_bulk(&priv->clks); + return ret; + } + + return 0; +} + +static int apple_dart_bind(struct udevice *dev) +{ + void *base; + int sid, i; + + base = dev_read_addr_ptr(dev); + if (!base) + return -EINVAL; + + u32 params2 = readl(base + DART_PARAMS2); + if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { + for (sid = 0; sid < 16; sid++) { + writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, + base + DART_TCR(sid)); + for (i = 0; i < 4; i++) + writel(0, base + DART_TTBR(sid, i)); + } + } + + return 0; +} + +static int apple_dart_probe(struct udevice *dev) +{ + struct apple_dart_priv *priv = dev_get_priv(dev); + phys_addr_t phys; + u64 *l1, *l2; + int sid, i, j; + int ret; + + apple_dart_phys_start = gd->ram_top - apple_dart_size; + + priv->base = dev_read_addr_ptr(dev); + if (!priv->base) + return -EINVAL; + + ret = apple_dart_clk_init(dev, priv); + if (ret) + return ret; + + l1 = memalign(SZ_64K, SZ_64K); + memset(l1, 0, SZ_64K); + + i = 0; + phys = apple_dart_phys_start; + while (phys < apple_dart_phys_start + apple_dart_size) { + l2 = memalign(SZ_16K, SZ_16K); + memset(l2, 0, SZ_16K); + + for (j = 0; j < 2048; j++) { + l2[j] = phys | 0x3; + phys += SZ_16K; + } + flush_dcache_range((unsigned long)l2, + (unsigned long)l2 + SZ_16K); + + l1[i++] = (phys_addr_t)l2 | 0x8 | 0x3; + } + + flush_dcache_range((unsigned long)l1, (unsigned long)l1 + SZ_64K); + + for (sid = 0; sid < 16; sid++) { + for (i = 0; i < 4; i++) + writel(0, priv->base + DART_TTBR(sid, i)); + } + + apple_dart_flush_tlb(priv); + + for (sid = 0; sid < 16; sid++) { + phys = (phys_addr_t)l1; + for (i = 0; i < 4; i++) { + writel((phys >> DART_TTBR_SHIFT) | DART_TTBR_VALID, + priv->base + DART_TTBR(sid, i)); + phys += SZ_16K; + } + } + + apple_dart_flush_tlb(priv); + + for (sid = 0; sid < 16; sid++) + writel(DART_TCR_TRANSLATE_ENABLE, priv->base + DART_TCR(sid)); + + return 0; +} + +static const struct udevice_id apple_dart_ids[] = { + { .compatible = "apple,t8103-dart" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(apple_dart) = { + .name = "apple_dart", + .id = UCLASS_MISC, + .of_match = apple_dart_ids, + .priv_auto = sizeof(struct apple_dart_priv), + .bind = apple_dart_bind, + .probe = apple_dart_probe +};
The DART is an IOMMU that is used on Apple's M1 SoC. This driver supports the DART in bypass mode as well as in a mode where it creates a 1:1 mapping of a subset of RAM as not all DARTs support bypass mode. The USB3 ports integrated on the SoC use a DART that supports bypass mode. The 1:1 mapping will be used in the future to support other devices such as the PCIe host bridge of the M1 SoC. Signed-off-by: Mark Kettenis <kettenis@openbsd.org> --- drivers/misc/Kconfig | 7 ++ drivers/misc/Makefile | 1 + drivers/misc/apple_dart.c | 171 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 drivers/misc/apple_dart.c