From: Simon Glass <sjg@chromium.org>
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: Mark Kettenis <kettenis@openbsd.org>,
U-Boot Mailing List <u-boot@lists.denx.de>,
Bin Meng <bmeng.cn@gmail.com>,
Heinrich Schuchardt <xypron.glpk@gmx.de>,
Nicolas Saenz Julienne <nsaenz@kernel.org>
Subject: Re: [PATCH v2 1/7] iommu: Add IOMMU uclass
Date: Thu, 14 Oct 2021 18:40:20 -0600 [thread overview]
Message-ID: <CAPnjgZ3dmQr-SJ7j71pEAw0Mfk-KnbK1qCJXATnwqgWwkYXzVA@mail.gmail.com> (raw)
In-Reply-To: <d3ca5fccfc8db834@bloch.sibelius.xs4all.nl>
Hi Mark,
On Thu, 14 Oct 2021 at 15:11, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Simon Glass <sjg@chromium.org>
> > Date: Thu, 14 Oct 2021 14:55:19 -0600
> >
> > Hi Mark,
> >
> > On Thu, 14 Oct 2021 at 14:51, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > >
> > > > From: Simon Glass <sjg@chromium.org>
> > > > Date: Thu, 14 Oct 2021 14:20:21 -0600
> > > >
> > > > Hi Mark,
> > > >
> > > > On Thu, 14 Oct 2021 at 13:35, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > > > >
> > > > > > From: Simon Glass <sjg@chromium.org>
> > > > > > Date: Mon, 11 Oct 2021 11:00:34 -0600
> > > > > >
> > > > > > Hi Mark,
> > > > > >
> > > > > > On Sun, 3 Oct 2021 at 12:31, Mark Kettenis <kettenis@openbsd.org> wrote:
> > > > > > >
> > > > > > > This uclass is intended to manage IOMMUs on systems where the
> > > > > > > IOMMUs are not in bypass mode by default. In that case U-Boot
> > > > > > > cannot ignore the IOMMUs if it wants to use devices that need
> > > > > > > to do DMA and sit behind such an IOMMU.
> > > > > > >
> > > > > > > This initial IOMMU uclass implementation does not implement and
> > > > > > > device ops and is intended for IOMMUs that have a bypass mode
> > > > > > > that does not require address translation. Support for IOMMUs
> > > > > > > that do require address translation is planned and device ops
> > > > > > > will be defined when support for such IOMMUs will be added.
> > > > > > >
> > > > > > > Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> > > > > > > ---
> > > > > > > drivers/Kconfig | 2 ++
> > > > > > > drivers/Makefile | 1 +
> > > > > > > drivers/core/device.c | 8 +++++++
> > > > > > > drivers/iommu/Kconfig | 13 +++++++++++
> > > > > > > drivers/iommu/Makefile | 3 +++
> > > > > > > drivers/iommu/iommu-uclass.c | 45 ++++++++++++++++++++++++++++++++++++
> > > > > > > include/dm/uclass-id.h | 1 +
> > > > > > > include/iommu.h | 16 +++++++++++++
> > > > > > > 8 files changed, 89 insertions(+)
> > > > > > > create mode 100644 drivers/iommu/Kconfig
> > > > > > > create mode 100644 drivers/iommu/Makefile
> > > > > > > create mode 100644 drivers/iommu/iommu-uclass.c
> > > > > > > create mode 100644 include/iommu.h
> > > > > > >
> > > > > > > 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..8cb377560e
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/iommu/Kconfig
> > > > > > > @@ -0,0 +1,13 @@
> > > > > > > +#
> > > > > > > +# 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.
> > > > > >
> > > > > > Need at least three lines. What is an IOMMU? How does it relate to
> > > > > > other devices?
> > > > >
> > > > > Not sure if I can describe what an IOMMU is in just a few lines, but
> > > > > I'll try.
> > > > >
> > > > > > > +
> > > > > > > +endmenu
> > > > > > > diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> > > > > > > new file mode 100644
> > > > > > > index 0000000000..f1ceb10150
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/iommu/Makefile
> > > > > > > @@ -0,0 +1,3 @@
> > > > > > > +# SPDX-License-Identifier: GPL-2.0+
> > > > > > > +
> > > > > > > +obj-$(CONFIG_IOMMU) += iommu-uclass.o
> > > > > > > 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)
> > > > > >
> > > > > > Can we rename to dev_iommus_enable()? I like to keep probe for use by
> > > > > > DM itself so it is confusing to talk about probing in a context other
> > > > > > than probing a single device.
> > > > >
> > > > > Sure. That was one of the names I considered myself when writing the
> > > > > code.
> > > > >
> > > > > > > +{
> > > > > > > + 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);
> > > > > >
> > > > > > Do you have a binding file you can add to docs/device-tree-bindings ?
> > > > >
> > > > > The binding is available in Documentation/bindings/iommu/iommu.txt in
> > > > > the Linux tree. But I don't think copying it into the u-boot tree
> > > > > makes sense. It will just get out of date. Especially because this
> > > > > will hopefully be converted into a proper DT schema in the near
> > > > > future.
> > > >
> > > > We do plan to sync these at some point, so don't worry about that. We
> > > > have bigger problems than the bindings.
> > >
> > > Well, at this point they're so much out of date that I don't look
> > > there unless I'm looking for someting that I know to be u-boot
> > > specific.
> > >
> > > > But without this it is not discoverable in U-Boot so no one knows what
> > > > the driver is referring to.
> > >
> > > Really? The canonical place for device tree bindings that aren't part
> > > of the device tree specification itself *is* the Linux kernel tree.
> >
> > Yes please, these should be in the U-Boot tree at this point. If they
> > move out of the kernel into some other repo then we can revisit it. I
> > don't really understand your reluctance. I hope that what you say will
> > be true at some point, but as you probably know there are some U-Boot
> > bindings which are not in that repo.
>
> Should I still add my Signed-off-by on files copied verbatim from the
> Linux kernel sources?
Yes that's my understanding..."The Signed-off-by: tag indicates that
the signer was involved in the development of the patch, or that
he/she was in the patch's delivery path"
That's why I sometimes add a tag when applying a patch, e.g. if I make
a minor tweak.
Regards,
Simon
next prev parent reply other threads:[~2021-10-15 0:41 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-03 18:30 [PATCH v2 0/7] Apple M1 Support Mark Kettenis
2021-10-03 18:30 ` [PATCH v2 1/7] iommu: Add IOMMU uclass Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-14 19:34 ` Mark Kettenis
2021-10-14 20:20 ` Simon Glass
2021-10-14 20:51 ` Mark Kettenis
2021-10-14 20:55 ` Simon Glass
2021-10-14 21:11 ` Mark Kettenis
2021-10-15 0:40 ` Simon Glass [this message]
2021-10-03 18:30 ` [PATCH v2 2/7] test: Add tests for " Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-14 19:50 ` Mark Kettenis
2021-10-14 20:43 ` Simon Glass
2021-10-03 18:30 ` [PATCH v2 3/7] arm: apple: Add initial support for Apple's M1 SoC Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-03 18:30 ` [PATCH v2 4/7] serial: s5p: Add Apple M1 support Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-14 19:57 ` Mark Kettenis
2021-10-03 18:30 ` [PATCH v2 5/7] iommu: Add Apple DART driver Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-03 18:30 ` [PATCH v2 6/7] arm: dts: apple: Add preliminary device trees Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-11 18:36 ` Rob Herring
2021-10-11 18:55 ` Simon Glass
2021-10-11 19:00 ` Mark Kettenis
2021-10-11 19:48 ` Rob Herring
2021-10-11 20:00 ` Simon Glass
2021-10-11 20:30 ` Tom Rini
2021-10-14 20:33 ` Mark Kettenis
2021-10-03 18:30 ` [PATCH v2 7/7] doc: board: apple: Add Apple M1 documentation Mark Kettenis
2021-10-11 17:00 ` Simon Glass
2021-10-11 17:00 ` [PATCH v2 0/7] Apple M1 Support Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPnjgZ3dmQr-SJ7j71pEAw0Mfk-KnbK1qCJXATnwqgWwkYXzVA@mail.gmail.com \
--to=sjg@chromium.org \
--cc=bmeng.cn@gmail.com \
--cc=kettenis@openbsd.org \
--cc=mark.kettenis@xs4all.nl \
--cc=nsaenz@kernel.org \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).