From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8551EC433EF for ; Thu, 14 Oct 2021 20:55:41 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8FF0E60F70 for ; Thu, 14 Oct 2021 20:55:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8FF0E60F70 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 36D3783693; Thu, 14 Oct 2021 22:55:38 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="mjyaJkCQ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 73B9A836A6; Thu, 14 Oct 2021 22:55:36 +0200 (CEST) Received: from mail-vk1-xa2b.google.com (mail-vk1-xa2b.google.com [IPv6:2607:f8b0:4864:20::a2b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D669D8366C for ; Thu, 14 Oct 2021 22:55:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-vk1-xa2b.google.com with SMTP id l39so4133332vkd.7 for ; Thu, 14 Oct 2021 13:55:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ytbVx7BYtdOyfgp+zNN/Ll7gcP8OylaGtxrzrC6ljxo=; b=mjyaJkCQcqG9CgI5jLhLkhylBe5jW0gG4dmtO+CxQsq++msGEXioY91b6WVXI3PeAN zVUKZ7F2C2NIjH41y2KBdcebLKnBr/02juMhBd9qP4V12HpoQj/reXFplKZlMb0f1Yi6 UqFaBeT1OsJ+Vg4rg6jtXYYnanRPP9OZAt9mQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ytbVx7BYtdOyfgp+zNN/Ll7gcP8OylaGtxrzrC6ljxo=; b=N0Zvqh/1zlK+KKjnTmoOtE8fKrXKeeI4/sy1YF+kufKz8mwUeJOTOkWNe6JS03srV6 MHXvd5fLpoAKeODsAMLPFyxaaEpK72+0QNxS4WMIGBMX5LZC3o8q+Mtr5wE8p1hDzHKI UUDh3HQMDunmtWn/m4PXanVbDNJ9KaQzCjIn0LN0eek5NYnq1Fa8DyV8SUSdtxidrCT7 HASRkiuN+N4auiziUKjAvRqYDPtYBvJJUEmn7k8mvcS7WhHKIV2y08IOEm9fGBlmPHpD FXfJmRY24gStsNbQPhbYO8CQKGoHzxjW+G0sOuKUj2NRNr5DwNxC/CsQwhUGztwGDOx9 2thA== X-Gm-Message-State: AOAM530cvN7ibCv+hDjy6dpUkz4O+/gCvVpGjK7vDa14f3G7n0CXUGsu FgW5DwJukCZc2I/P52XeLHY7sx3dndZQKjq9flq/ExjF/3U= X-Google-Smtp-Source: ABdhPJynDy+DSptzpU1QkZnceCSTLR05BKpBLr8dElpe/PKlh/dHsCdHVAPndhVypS+BKPiWOhDK3PXSevbU/tSFSaI= X-Received: by 2002:a1f:c6:: with SMTP id 189mr9073545vka.16.1634244931436; Thu, 14 Oct 2021 13:55:31 -0700 (PDT) MIME-Version: 1.0 References: <20211003183050.67925-1-kettenis@openbsd.org> <20211003183050.67925-2-kettenis@openbsd.org> In-Reply-To: From: Simon Glass Date: Thu, 14 Oct 2021 14:55:19 -0600 Message-ID: Subject: Re: [PATCH v2 1/7] iommu: Add IOMMU uclass To: Mark Kettenis Cc: Mark Kettenis , U-Boot Mailing List , Bin Meng , Heinrich Schuchardt , Nicolas Saenz Julienne Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Mark, On Thu, 14 Oct 2021 at 14:51, Mark Kettenis wrote: > > > From: Simon Glass > > Date: Thu, 14 Oct 2021 14:20:21 -0600 > > > > Hi Mark, > > > > On Thu, 14 Oct 2021 at 13:35, Mark Kettenis wrote: > > > > > > > From: Simon Glass > > > > Date: Mon, 11 Oct 2021 11:00:34 -0600 > > > > > > > > Hi Mark, > > > > > > > > On Sun, 3 Oct 2021 at 12:31, Mark Kettenis 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 > > > > > --- > > > > > 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 > > > > > #include > > > > > #include > > > > > +#include > > > > > #include > > > > > #include > > > > > #include > > > > > @@ -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 > > > > > + */ > > > > > + > > > > > +#define LOG_CATEGORY UCLASS_IOMMU > > > > > + > > > > > +#include > > > > > +#include > > > > > + > > > > > +#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. Regards, Simon