[PATCH v11 0/6] mmc: sdhci: Add Tegra234 support

[PATCH v11 0/6] mmc: sdhci: Add Tegra234 support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Hi,

This is an updated version of v10 that Prathamesh had sent out a while
ago:

	https://lore.kernel.org/all/20221103043852.24718-1-pshete@nvidia.com/

The main changes in here are the first three patches, of which 2 are
minor preparatory work. The first patch adds a note to the kerneldoc
comment for struct iommu_fwspec to hopefully make it clearer that no
driver should reach into its internals. Patch 2 adds a custom Tegra-
specific helper to encapsulate the common pattern that Tegra drivers
use to access the stream IDs. Patch three then resorts the includes
to make it easier to add subsequent includes in the right place.

The rest of the series is mostly the same as what Prathamesh had sent
out. Patch 4 is slightly updated because it uses the new helper and it
turns out that that also addresses some of the other review comments.

I have a local patch to migrate the remaining Tegra drivers over to the
new helper, but the plan is to hold that back until the new helper has
been merged (hopefully for v6.2) to provide the dependency in mainline,
which makes the conversion patches much easier to apply through the
appropriate subsystem trees.

Thierry

Prathamesh Shete (3):
  mmc: sdhci-tegra: Separate Tegra194 and Tegra234 SoC data
  mmc: sdhci-tegra: Add support to program MC stream ID
  mmc: sdhci-tegra: Issue CMD and DAT resets together

Thierry Reding (3):
  iommu: Add note about struct iommu_fwspec usage
  iommu/tegra: Add tegra_dev_iommu_get_stream_id() helper
  mmc: sdhci-tegra: Sort includes alphabetically

 drivers/mmc/host/sdhci-tegra.c | 70 ++++++++++++++++++++++++++++------
 drivers/mmc/host/sdhci.c       |  5 +++
 drivers/mmc/host/sdhci.h       |  2 +
 include/linux/iommu.h          | 25 ++++++++++++
 4 files changed, 90 insertions(+), 12 deletions(-)

-- 
2.38.1

[PATCH v11 1/6] iommu: Add note about struct iommu_fwspec usage

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This structure is to be considered private to the IOMMU API. Except for
very few exceptions, IOMMU consumer drivers should treat this as opaque
data.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/linux/iommu.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 1690c334e516..dc5e6f5c14fc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -641,6 +641,10 @@ struct iommu_group *fsl_mc_device_group(struct device *dev);
  * @flags: IOMMU_FWSPEC_* flags
  * @num_ids: number of associated device IDs
  * @ids: IDs which this device may present to the IOMMU
+ *
+ * Note that the IDs (and any other information, really) stored in this structure should be
+ * considered private to the IOMMU device driver and are not to be used directly by IOMMU
+ * consumers.
  */
 struct iommu_fwspec {
 	const struct iommu_ops	*ops;
-- 
2.38.1

[PATCH v11 2/6] iommu/tegra: Add tegra_dev_iommu_get_stream_id() helper

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Access to the internals of struct iommu_fwspec by non-IOMMU drivers is
discouraged. Many drivers for Tegra SoCs, however, need access to their
IOMMU stream IDs so that they can be programmed into various hardware
registers.

Formalize this access into a common helper to make it easier to audit
and maintain.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/linux/iommu.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index dc5e6f5c14fc..f0d72acad7db 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1186,4 +1186,25 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
 }
 #endif /* CONFIG_IOMMU_SVA */
 
+/*
+ * Newer generations of Tegra SoCs require devices' stream IDs to be directly programmed into
+ * some registers. These are always paired with a Tegra SMMU or ARM SMMU, for which the contents
+ * of the struct iommu_fwspec are known. Use this helper to formalize access to these internals.
+ */
+#define TEGRA_STREAM_ID_BYPASS 0x7f
+
+static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream_id)
+{
+#ifdef CONFIG_IOMMU_API
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+	if (fwspec && fwspec->num_ids == 1) {
+		*stream_id = fwspec->ids[0] & 0xffff;
+		return true;
+	}
+#endif
+
+	return false;
+}
+
 #endif /* __LINUX_IOMMU_H */
-- 
2.38.1

[PATCH v11 3/6] mmc: sdhci-tegra: Sort includes alphabetically

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Sort includes alphabetically to make it easier to add new ones
subsequently.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index c71000a07656..e2a8488d4fa9 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -3,28 +3,29 @@
  * Copyright (C) 2010 Google, Inc.
  */
 
+#include <linux/bitfield.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
-#include <linux/module.h>
+#include <linux/gpio/consumer.h>
 #include <linux/init.h>
-#include <linux/iopoll.h>
-#include <linux/platform_device.h>
-#include <linux/clk.h>
 #include <linux/io.h>
-#include <linux/of.h>
+#include <linux/iopoll.h>
+#include <linux/ktime.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/slot-gpio.h>
+#include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
 #include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
-#include <linux/mmc/card.h>
-#include <linux/mmc/host.h>
-#include <linux/mmc/mmc.h>
-#include <linux/mmc/slot-gpio.h>
-#include <linux/gpio/consumer.h>
-#include <linux/ktime.h>
 
 #include <soc/tegra/common.h>
 
-- 
2.38.1

[PATCH v11 4/6] mmc: sdhci-tegra: Separate Tegra194 and Tegra234 SoC data

[Thierry Reding]

From: Prathamesh Shete <pshete@nvidia.com>

Create new SoC data structure for Tegra234 platforms.
Additional features, tap value configurations are added/
updated for Tegra234 platform hence separate Tegra194 and
Tegra234 SoC data.

Signed-off-by: Aniruddha Tvs Rao <anrao@nvidia.com>
Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index e2a8488d4fa9..bbe2bc238171 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -1558,7 +1558,21 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
 	.max_tap_delay = 139,
 };
 
+static const struct sdhci_tegra_soc_data soc_data_tegra234 = {
+	.pdata = &sdhci_tegra186_pdata,
+	.dma_mask = DMA_BIT_MASK(39),
+	.nvquirks = NVQUIRK_NEEDS_PAD_CONTROL |
+		    NVQUIRK_HAS_PADCALIB |
+		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
+		    NVQUIRK_ENABLE_SDR50 |
+		    NVQUIRK_ENABLE_SDR104 |
+		    NVQUIRK_HAS_TMCLK,
+	.min_tap_delay = 95,
+	.max_tap_delay = 111,
+};
+
 static const struct of_device_id sdhci_tegra_dt_match[] = {
+	{ .compatible = "nvidia,tegra234-sdhci", .data = &soc_data_tegra234 },
 	{ .compatible = "nvidia,tegra194-sdhci", .data = &soc_data_tegra194 },
 	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
 	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
-- 
2.38.1

[PATCH v11 5/6] mmc: sdhci-tegra: Add support to program MC stream ID

[Thierry Reding]

From: Prathamesh Shete <pshete@nvidia.com>

SMMU clients are supposed to program stream ID from
their respective address spaces instead of MC override.
Define NVQUIRK_PROGRAM_STREAMID and use it to program
SMMU stream ID from the SDMMC client address space.

Signed-off-by: Aniruddha TVS Rao <anrao@nvidia.com>
Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index bbe2bc238171..1cb7fd3afa7e 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -11,6 +12,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/iommu.h>
 #include <linux/iopoll.h>
 #include <linux/ktime.h>
 #include <linux/mmc/card.h>
@@ -96,6 +98,8 @@
 #define SDHCI_TEGRA_AUTO_CAL_STATUS			0x1ec
 #define SDHCI_TEGRA_AUTO_CAL_ACTIVE			BIT(31)
 
+#define SDHCI_TEGRA_CIF2AXI_CTRL_0			0x1fc
+
 #define NVQUIRK_FORCE_SDHCI_SPEC_200			BIT(0)
 #define NVQUIRK_ENABLE_BLOCK_GAP_DET			BIT(1)
 #define NVQUIRK_ENABLE_SDHCI_SPEC_300			BIT(2)
@@ -123,6 +127,7 @@
 #define NVQUIRK_HAS_TMCLK				BIT(10)
 
 #define NVQUIRK_HAS_ANDROID_GPT_SECTOR			BIT(11)
+#define NVQUIRK_PROGRAM_STREAMID			BIT(12)
 
 /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */
 #define SDHCI_TEGRA_CQE_BASE_ADDR			0xF000
@@ -179,6 +184,7 @@ struct sdhci_tegra {
 	bool enable_hwcq;
 	unsigned long curr_clk_rate;
 	u8 tuned_tap_delay;
+	u32 stream_id;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -1566,6 +1572,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra234 = {
 		    NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
 		    NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_SDR104 |
+		    NVQUIRK_PROGRAM_STREAMID |
 		    NVQUIRK_HAS_TMCLK,
 	.min_tap_delay = 95,
 	.max_tap_delay = 111,
@@ -1632,6 +1639,19 @@ static int sdhci_tegra_add_host(struct sdhci_host *host)
 	return ret;
 }
 
+/* Program MC streamID for DMA transfers */
+static void sdhci_tegra_program_stream_id(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
+
+	if (tegra_host->soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID) {
+		tegra_sdhci_writel(host, FIELD_PREP(GENMASK(15, 8), tegra_host->stream_id) |
+					 FIELD_PREP(GENMASK( 7, 0), tegra_host->stream_id),
+					 SDHCI_TEGRA_CIF2AXI_CTRL_0);
+	}
+}
+
 static int sdhci_tegra_probe(struct platform_device *pdev)
 {
 	const struct sdhci_tegra_soc_data *soc_data;
@@ -1692,6 +1712,12 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 
 	tegra_sdhci_parse_dt(host);
 
+	if (tegra_host->soc_data->nvquirks & NVQUIRK_PROGRAM_STREAMID &&
+	    !tegra_dev_iommu_get_stream_id(&pdev->dev, &tegra_host->stream_id)) {
+		dev_warn(mmc_dev(host->mmc), "missing IOMMU stream ID\n");
+		tegra_host->stream_id = 0x7f;
+	}
+
 	tegra_host->power_gpio = devm_gpiod_get_optional(&pdev->dev, "power",
 							 GPIOD_OUT_HIGH);
 	if (IS_ERR(tegra_host->power_gpio)) {
@@ -1777,6 +1803,8 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	if (rc)
 		goto err_add_host;
 
+	sdhci_tegra_program_stream_id(host);
+
 	return 0;
 
 err_add_host:
@@ -1873,6 +1901,8 @@ static int sdhci_tegra_resume(struct device *dev)
 	if (ret)
 		return ret;
 
+	sdhci_tegra_program_stream_id(host);
+
 	ret = sdhci_resume_host(host);
 	if (ret)
 		goto disable_clk;
-- 
2.38.1

[PATCH v11 6/6] mmc: sdhci-tegra: Issue CMD and DAT resets together

[Thierry Reding]

From: Prathamesh Shete <pshete@nvidia.com>

In case of error condition to avoid system crash
Tegra SDMMC controller requires CMD and DAT resets
issued together. SDHCI controller FSM goes into
bad state due to rapid SD card hot-plug event.
Issuing reset on the CMD FSM before DATA FSM results
in kernel panic, hence add support to issue CMD and
DAT resets together.
This is applicable to Tegra186 and later chips.

Signed-off-by: Aniruddha TVS Rao <anrao@nvidia.com>
Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 3 ++-
 drivers/mmc/host/sdhci.c       | 5 +++++
 drivers/mmc/host/sdhci.h       | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 1cb7fd3afa7e..3384bfd3585c 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -1533,7 +1533,8 @@ static const struct sdhci_pltfm_data sdhci_tegra186_pdata = {
 		  SDHCI_QUIRK_NO_HISPD_BIT |
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+		   SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER,
 	.ops  = &tegra186_sdhci_ops,
 };
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fb6e9a81f198..ed59bb3247a1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -270,6 +270,11 @@ enum sdhci_reset_reason {
 
 static void sdhci_reset_for_reason(struct sdhci_host *host, enum sdhci_reset_reason reason)
 {
+	if (host->quirks2 &
+		SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER) {
+		sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
+		return;
+	}
 	switch (reason) {
 	case SDHCI_RESET_FOR_INIT:
 		sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 908da47ac5ba..ca7ede721523 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -478,6 +478,8 @@ struct sdhci_host {
  * block count.
  */
 #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT			(1<<18)
+/* Issue CMD and DATA reset together */
+#define SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER	(1<<19)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
2.38.1

Re: [PATCH v11 3/6] mmc: sdhci-tegra: Sort includes alphabetically

[Adrian Hunter]

On 17/11/22 20:27, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Sort includes alphabetically to make it easier to add new ones
> subsequently.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)

12 insertions and 11 deletions because linux/bitfield.h
was added.

I am not super enthusiastic about reordering includes.
I would advise people to text search to find an include
instead.

Nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index c71000a07656..e2a8488d4fa9 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -3,28 +3,29 @@
>   * Copyright (C) 2010 Google, Inc.
>   */
>  
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
> -#include <linux/module.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/init.h>
> -#include <linux/iopoll.h>
> -#include <linux/platform_device.h>
> -#include <linux/clk.h>
>  #include <linux/io.h>
> -#include <linux/of.h>
> +#include <linux/iopoll.h>
> +#include <linux/ktime.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/slot-gpio.h>
> +#include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/of.h>
>  #include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
>  #include <linux/pm_opp.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
> -#include <linux/mmc/card.h>
> -#include <linux/mmc/host.h>
> -#include <linux/mmc/mmc.h>
> -#include <linux/mmc/slot-gpio.h>
> -#include <linux/gpio/consumer.h>
> -#include <linux/ktime.h>
>  
>  #include <soc/tegra/common.h>
>  

Re: [PATCH v11 3/6] mmc: sdhci-tegra: Sort includes alphabetically

[Ulf Hansson]

On Fri, 18 Nov 2022 at 08:33, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 17/11/22 20:27, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Sort includes alphabetically to make it easier to add new ones
> > subsequently.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/mmc/host/sdhci-tegra.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
>
> 12 insertions and 11 deletions because linux/bitfield.h
> was added.
>
> I am not super enthusiastic about reordering includes.
> I would advise people to text search to find an include
> instead.

Yes, I agree with you. I don't quite see the benefit.

On the other hand, I don't want to waste my time on discussing it (I
just did!?), as it's (currently) not that such a big deal, I think.

[...]

Kind regards
Uffe

Re: [PATCH v11 2/6] iommu/tegra: Add tegra_dev_iommu_get_stream_id() helper

[Ulf Hansson]

On Thu, 17 Nov 2022 at 19:27, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> From: Thierry Reding <treding@nvidia.com>
>
> Access to the internals of struct iommu_fwspec by non-IOMMU drivers is
> discouraged. Many drivers for Tegra SoCs, however, need access to their
> IOMMU stream IDs so that they can be programmed into various hardware
> registers.
>
> Formalize this access into a common helper to make it easier to audit
> and maintain.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/linux/iommu.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index dc5e6f5c14fc..f0d72acad7db 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -1186,4 +1186,25 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
>  }
>  #endif /* CONFIG_IOMMU_SVA */
>
> +/*
> + * Newer generations of Tegra SoCs require devices' stream IDs to be directly programmed into
> + * some registers. These are always paired with a Tegra SMMU or ARM SMMU, for which the contents
> + * of the struct iommu_fwspec are known. Use this helper to formalize access to these internals.
> + */
> +#define TEGRA_STREAM_ID_BYPASS 0x7f
> +
> +static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream_id)
> +{
> +#ifdef CONFIG_IOMMU_API
> +       struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +       if (fwspec && fwspec->num_ids == 1) {
> +               *stream_id = fwspec->ids[0] & 0xffff;
> +               return true;
> +       }
> +#endif

It's totally up to iommu maintainers to decide, but I personally would
prefer to replace the below part with a stub function, defined when
CONFIG_IOMMU_API is unset.

> +
> +       return false;
> +}
> +
>  #endif /* __LINUX_IOMMU_H */
> --
> 2.38.1
>

Other than that, both patch1 and patch2 look good to me. I can pick up
the series, after an ack from Robin or Joerg.

Kind regards
Uffe

Re: [PATCH v11 1/6] iommu: Add note about struct iommu_fwspec usage

[Joerg Roedel]

On Thu, Nov 17, 2022 at 07:27:15PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> This structure is to be considered private to the IOMMU API. Except for
> very few exceptions, IOMMU consumer drivers should treat this as opaque
> data.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/linux/iommu.h | 4 ++++
>  1 file changed, 4 insertions(+)

We should probably hiding the structure outside the IOMMU world then.
But that's another path to go, so

Acked-by: Joerg Roedel <jroedel@suse.de>

Re: [PATCH v11 2/6] iommu/tegra: Add tegra_dev_iommu_get_stream_id() helper

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

On Fri, Nov 18, 2022 at 09:08:17AM +0100, Ulf Hansson wrote:
> On Thu, 17 Nov 2022 at 19:27, Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Access to the internals of struct iommu_fwspec by non-IOMMU drivers is
> > discouraged. Many drivers for Tegra SoCs, however, need access to their
> > IOMMU stream IDs so that they can be programmed into various hardware
> > registers.
> >
> > Formalize this access into a common helper to make it easier to audit
> > and maintain.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  include/linux/iommu.h | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index dc5e6f5c14fc..f0d72acad7db 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -1186,4 +1186,25 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
> >  }
> >  #endif /* CONFIG_IOMMU_SVA */
> >
> > +/*
> > + * Newer generations of Tegra SoCs require devices' stream IDs to be directly programmed into
> > + * some registers. These are always paired with a Tegra SMMU or ARM SMMU, for which the contents
> > + * of the struct iommu_fwspec are known. Use this helper to formalize access to these internals.
> > + */
> > +#define TEGRA_STREAM_ID_BYPASS 0x7f
> > +
> > +static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream_id)
> > +{
> > +#ifdef CONFIG_IOMMU_API
> > +       struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +
> > +       if (fwspec && fwspec->num_ids == 1) {
> > +               *stream_id = fwspec->ids[0] & 0xffff;
> > +               return true;
> > +       }
> > +#endif
> 
> It's totally up to iommu maintainers to decide, but I personally would
> prefer to replace the below part with a stub function, defined when
> CONFIG_IOMMU_API is unset.

Yeah, that's what I had done originally, but then it felt a bit at odds
with Robin's request to move this into a central location. Ultimately
the result is the same and this has the benefit of standing out a bit
more as unusual.

I don't feel strongly, though, so if Joerg and/or Robin want to see this
split up into an implementation and a stub, I can do that.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]