Fix omap-iommu bitrot

Am 28.10.2024 um 18:58 schrieb Robin Murphy <[email protected]>:

Hi all,

It seems omap-iommu hasn't had enough mainline users to avoid bitrotting through the more recent evolution of the IOMMU API internals. These patches attempt to bring it and its consumers sufficiently up-to-date to work again, in a manner that's hopefully backportable. This is largely all written by inspection, but I have managed to lightly boot test patch #3 on an OMAP4 Pandaboard to confirm iommu_probe_device() working again.

This supersedes my previous patch[1]. Patches #1 and #2 are functionally independent, and can be applied directly to their respective trees if preferred.

Hi Robin,

Quoted 12 lines

Am 28.10.2024 um 18:58 schrieb Robin Murphy <[email protected]>:

Hi all,

It seems omap-iommu hasn't had enough mainline users to avoid bitrotting through the more recent evolution of the IOMMU API internals. These patches attempt to bring it and its consumers sufficiently up-to-date to work again, in a manner that's hopefully backportable. This is largely all written by inspection, but I have managed to lightly boot test patch #3 on an OMAP4 Pandaboard to confirm iommu_probe_device() working again.

Quoted 3 lines

This supersedes my previous patch[1]. Patches #1 and #2 are functionally independent, and can be applied directly to their respective trees if preferred.

I can confirm that this series works on omap3 with v6.12-rc5 and Camera is back to the GTA04.

So you can add:

Tested-by H. Nikolaus Schaller <[email protected]>

BR and thanks,
Nikolaus

root@letux:~# dmesg|fgrep iommu
[    0.522613] iommu: Default domain type: Translated
[    0.522644] iommu: DMA domain TLB invalidation policy: strict mode
[    0.525177] omap-iommu 480bd400.mmu: 480bd400.mmu registered
[   10.752563] omap3isp 480bc000.isp: Adding to iommu group 0
[   10.811218] omap-iommu 480bd400.mmu: 480bd400.mmu: version 1.1
[   11.039489] omap-iommu 480bd400.mmu: 480bd400.mmu: version 1.1
root@letux:~# dmesg|fgrep .isp
[   10.752563] omap3isp 480bc000.isp: Adding to iommu group 0
[   10.841522] omap3isp 480bc000.isp: supply vdd-csiphy1 not found, using dummy regulator
[   10.948577] omap3isp 480bc000.isp: supply vdd-csiphy2 not found, using dummy regulator
[   10.990814] omap3isp 480bc000.isp: Revision 15.0 found
[   11.089324] omap3isp 480bc000.isp: hist: using DMA channel dma0chan15
[   11.115905] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP CCP2 was not initialized!
[   11.168792] omap3isp 480bc000.isp: isp_xclk_set_rate: cam_xclka set to 24685714 Hz (div 7)
[   11.220062] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP CSI2a was not initialized!
[   11.291748] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP CCDC was not initialized!
[   11.362152] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP preview was not initialized!
[   11.404266] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP resizer was not initialized!
[   11.485687] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP AEWB was not initialized!
[   11.520874] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP AF was not initialized!
[   11.574981] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP histogram was not initialized!
[   11.594268] omap3isp 480bc000.isp: parsing parallel interface
[  106.102905] omap3isp 480bc000.isp: -------------CCDC Register dump-------------
[  106.110595] omap3isp 480bc000.isp: ###CCDC PCR=0x00000000
[  106.116973] omap3isp 480bc000.isp: ###CCDC SYN_MODE=0x00031704
[  106.123657] omap3isp 480bc000.isp: ###CCDC HD_VD_WID=0x00000000
[  106.129974] omap3isp 480bc000.isp: ###CCDC PIX_LINES=0x00000000
[  106.136810] omap3isp 480bc000.isp: ###CCDC HORZ_INFO=0x000004ff
[  106.143615] omap3isp 480bc000.isp: ###CCDC VERT_START=0x00000000
[  106.149932] omap3isp 480bc000.isp: ###CCDC VERT_LINES=0x000003ff
[  106.156799] omap3isp 480bc000.isp: ###CCDC CULLING=0xffff00ff
[  106.163299] omap3isp 480bc000.isp: ###CCDC HSIZE_OFF=0x00000a00
[  106.169616] omap3isp 480bc000.isp: ###CCDC SDOFST=0x00000000
[  106.176086] omap3isp 480bc000.isp: ###CCDC SDR_ADDR=0x40000000
[  106.182708] omap3isp 480bc000.isp: ###CCDC CLAMP=0x00000010
[  106.188598] omap3isp 480bc000.isp: ###CCDC DCSUB=0x00000000
[  106.195068] omap3isp 480bc000.isp: ###CCDC COLPTN=0xbb11bb11
[  106.201507] omap3isp 480bc000.isp: ###CCDC BLKCMP=0x00000000
[  106.207550] omap3isp 480bc000.isp: ###CCDC FPC=0x00000000
[  106.213684] omap3isp 480bc000.isp: ###CCDC FPC_ADDR=0x00000000
[  106.219909] omap3isp 480bc000.isp: ###CCDC VDINT=0x03fe02aa
[  106.226409] omap3isp 480bc000.isp: ###CCDC ALAW=0x00000006
[  106.232696] omap3isp 480bc000.isp: ###CCDC REC656IF=0x00000000
[  106.238830] omap3isp 480bc000.isp: ###CCDC CFG=0x00008800
[  106.244964] omap3isp 480bc000.isp: ###CCDC FMTCFG=0x00000000
[  106.251434] omap3isp 480bc000.isp: ###CCDC FMT_HORZ=0x00000000
[  106.257568] omap3isp 480bc000.isp: ###CCDC FMT_VERT=0x00000000
[  106.264251] omap3isp 480bc000.isp: ###CCDC PRGEVEN0=0x00000000
[  106.271606] omap3isp 480bc000.isp: ###CCDC PRGEVEN1=0x00000000
[  106.285400] omap3isp 480bc000.isp: ###CCDC PRGODD0=0x00000000
[  106.301147] omap3isp 480bc000.isp: ###CCDC PRGODD1=0x00000000
[  106.307220] omap3isp 480bc000.isp: ###CCDC VP_OUT=0x00000000
[  106.326019] omap3isp 480bc000.isp: ###CCDC LSC_CONFIG=0x00006600
[  106.340087] omap3isp 480bc000.isp: ###CCDC LSC_INITIAL=0x00000000
[  106.358001] omap3isp 480bc000.isp: ###CCDC LSC_TABLE_BASE=0x00000000
[  106.372985] omap3isp 480bc000.isp: ###CCDC LSC_TABLE_OFFSET=0x00000000
[  106.379882] omap3isp 480bc000.isp: --------------------------------------------
[  118.617980] omap3isp 480bc000.isp: OMAP3 ISP AEWB: user wants to disable module.
[  118.626068] omap3isp 480bc000.isp: OMAP3 ISP AEWB: module is being disabled
[  118.633392] omap3isp 480bc000.isp: OMAP3 ISP AF: user wants to disable module.
[  118.641937] omap3isp 480bc000.isp: OMAP3 ISP AF: module is being disabled
[  118.649627] omap3isp 480bc000.isp: OMAP3 ISP histogram: user wants to disable module.
[  118.658508] omap3isp 480bc000.isp: OMAP3 ISP histogram: module is being disabled
root@letux:~#

It seems omap-iommu hasn't had enough mainline users to avoid bitrotting through the more recent evolution of the IOMMU API internals. These patches attempt to bring it and its consumers sufficiently up-to-date to work again, in a manner that's hopefully backportable. This is largely all written by inspection, but I have managed to lightly boot test patch #3 on an OMAP4 Pandaboard to confirm iommu_probe_device() working again.

This supersedes my previous patch[1]. Patches #1 and #2 are functionally independent, and can be applied directly to their respective trees if preferred.

Hi all,

It seems omap-iommu hasn't had enough mainline users to avoid bitrotting through the more recent evolution of the IOMMU API internals. These patches attempt to bring it and its consumers sufficiently up-to-date to work again, in a manner that's hopefully backportable. This is largely all written by inspection, but I have managed to lightly boot test patch #3 on an OMAP4 Pandaboard to confirm iommu_probe_device() working again.

This supersedes my previous patch[1]. Patches #1 and #2 are functionally independent, and can be applied directly to their respective trees if preferred.

Thanks,
Robin.

[1] https://lore.kernel.org/linux-iommu/c44545c6d07c65d89daa297298c27bb0f15c8b84.1728393458.git.robin.murphy@arm.com/

Robin Murphy (4):
remoteproc/omap: Handle ARM dma_iommu_mapping
media: omap3isp: Handle ARM dma_iommu_mapping
iommu/omap: Add minimal fwnode support
iommu: Make bus_iommu_probe() static

  drivers/iommu/iommu.c                    |  3 ++-
  drivers/iommu/omap-iommu.c               | 26 +++++++++++++++---------
  drivers/media/platform/ti/omap3isp/isp.c |  7 +++++++
  drivers/remoteproc/omap_remoteproc.c     | 17 ++++++++++++++++
  include/linux/iommu.h                    |  1 -
  5 files changed, 42 insertions(+), 12 deletions(-)

It seems omap-iommu hasn't had enough mainline users to avoid bitrotting through the more recent evolution of the IOMMU API internals. These patches attempt to bring it and its consumers sufficiently up-to-date to work again, in a manner that's hopefully backportable. This is largely all written by inspection, but I have managed to lightly boot test patch #3 on an OMAP4 Pandaboard to confirm iommu_probe_device() working again.

This supersedes my previous patch[1]. Patches #1 and #2 are functionally independent, and can be applied directly to their respective trees if preferred.

Am 30.10.2024 um 10:55 schrieb Joerg Roedel <[email protected]>:

Quoted 7 lines · Robin Murphy

It seems omap-iommu hasn't had enough mainline users to avoid bitrotting through the more recent evolution of the IOMMU API internals. These patches attempt to bring it and its consumers sufficiently up-to-date to work again, in a manner that's hopefully backportable. This is largely all written by inspection, but I have managed to lightly boot test patch #3 on an OMAP4 Pandaboard to confirm iommu_probe_device() working again.

My initial reflex would have been to just wipe the omap drivers,

hardware is 10+ years out of production, no? So who is still using this hardware with recent kernels for other purposes than kernel testing?

Quoted 4 lines

This supersedes my previous patch[1]. Patches #1 and #2 are functionally independent, and can be applied directly to their respective trees if preferred.

I applied patches 3 and 4 to the ti/omap branch.

Why that? There was a discussion and everyone agreed to remove omap2, but not omap3 and later.

There are some devices besides the PandaBoard. I am aware of these where this is relevant: Epson BT200, Samsung Galaxy Tab 2, Pyra Handheld (in production) and we are currently thinking about producing a tiny series of the DM3730 based GTA04A5 with spare parts.

And of course we want to participate from the latest and greatest upstream changes.

Quoted 2 lines · H. Nikolaus Schaller

Why that? There was a discussion and everyone agreed to remove omap2, but not omap3 and later.

I raised this question to make sure the things we maintain are still relevant. Developer and maintainers time is limited and we should not spend it on stuff that nobody uses.

Quoted 5 lines

There are some devices besides the PandaBoard. I am aware of these where this is relevant: Epson BT200, Samsung Galaxy Tab 2, Pyra Handheld (in production) and we are currently thinking about producing a tiny series of the DM3730 based GTA04A5 with spare parts.

Quoted 1 line

And of course we want to participate from the latest and greatest upstream changes.

Okay, if there are still real users for latest mainline kernels on this hardware, then the effort is justified.

Regards,

Joerg

Hi

Quoted 18 lines · Joerg Roedel

Quoted 2 lines · H. Nikolaus Schaller

Why that? There was a discussion and everyone agreed to remove omap2, but not omap3 and later.

I raised this question to make sure the things we maintain are still relevant. Developer and maintainers time is limited and we should not spend it on stuff that nobody uses.

Quoted 6 lines

There are some devices besides the PandaBoard. I am aware of these where this is relevant: Epson BT200, Samsung Galaxy Tab 2, Pyra Handheld (in production) and we are currently thinking about producing a tiny series of the DM3730 based GTA04A5 with spare parts.

And of course we want to participate from the latest and greatest upstream changes.

Okay, if there are still real users for latest mainline kernels on this hardware, then the effort is justified.

Quoted 3 lines

Regards,

Joerg

There is also the Nokia N900 phone (OMAP3) still seeing mainline activity, as well as the Motorola Droid 4 (OMAP4), to name a few. I will also be testing on the N900 around the weekend.

Thanks to everyone for the amazing work.

Sincerely
Sicelo

It's no longer practical for the OMAP IOMMU driver to trick arm_setup_iommu_dma_ops() into ignoring its presence, so let's use the same tactic as other IOMMU API users on 32-bit ARM and explicitly kick the arch code's dma_iommu_mapping out of the way to avoid problems.

Fixes 4720287c7bf7 ("iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()")

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/remoteproc/omap_remoteproc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/remoteproc/omap_remoteproc.c +7 -0

index 9ae2e831456d..3260dd512491 100644
@@ -37,6 +37,10 @@
 
 #include <linux/platform_data/dmtimer-omap.h>
 
+#ifdef CONFIG_ARM_DMA_USE_IOMMU
+#include <asm/dma-iommu.h>
+#endif
+
 #include "omap_remoteproc.h"
 #include "remoteproc_internal.h"
 

@@ -1323,6 +1327,19 @@ static int omap_rproc_probe(struct platform_device *pdev)
 	/* All existing OMAP IPU and DSP processors have an MMU */
 	rproc->has_iommu = true;
 
+#ifdef CONFIG_ARM_DMA_USE_IOMMU
+	/*
+	 * Throw away the ARM DMA mapping that we'll never use, so it doesn't

drivers/remoteproc/omap_remoteproc.c +0 -0

 	ret = omap_rproc_of_get_internal_memories(pdev, rproc);
 	if (ret)
 		return ret;

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

It's no longer practical for the OMAP IOMMU driver to trick arm_setup_iommu_dma_ops() into ignoring its presence, so let's use the same tactic as other IOMMU API users on 32-bit ARM and explicitly kick the arch code's dma_iommu_mapping out of the way to avoid problems.

Fixes 4720287c7bf7 ("iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()")

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/media/platform/ti/omap3isp/isp.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/media/platform/ti/omap3isp/isp.c +7 -0

index 91101ba88ef0..b2210841a320 100644
@@ -1961,6 +1961,13 @@ static int isp_attach_iommu(struct isp_device *isp)
 	struct dma_iommu_mapping *mapping;
 	int ret;
 
+	/* We always want to replace any default mapping from the arch code */
+	mapping = to_dma_iommu_mapping(isp->dev);
+	if (mapping) {
+		arm_iommu_detach_device(isp->dev);
+		arm_iommu_release_mapping(mapping);
+	}
+
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
 	 * VAs. This will allocate a corresponding IOMMU domain.

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

Quoted 32 lines · Robin Murphy

It's no longer practical for the OMAP IOMMU driver to trick arm_setup_iommu_dma_ops() into ignoring its presence, so let's use the same tactic as other IOMMU API users on 32-bit ARM and explicitly kick the arch code's dma_iommu_mapping out of the way to avoid problems.

Fixes 4720287c7bf7 ("iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()")

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/media/platform/ti/omap3isp/isp.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/media/platform/ti/omap3isp/isp.c +7 -0

index 91101ba88ef0..b2210841a320 100644
@@ -1961,6 +1961,13 @@ static int isp_attach_iommu(struct isp_device *isp)
 	struct dma_iommu_mapping *mapping;
 	int ret;
 
+	/* We always want to replace any default mapping from the arch code */
+	mapping = to_dma_iommu_mapping(isp->dev);
+	if (mapping) {
+		arm_iommu_detach_device(isp->dev);
+		arm_iommu_release_mapping(mapping);
+	}
+
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
 	 * VAs. This will allocate a corresponding IOMMU domain.

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

I have finally found time to test this patch on the Nokia N900 and can confirm it is working fine.

I was wondering - is there a reason that it is not merged yet? I tested on 6.14-rc2, which did not have it, and notice it is also not in linux-next.

If it helps:

Tested-by Sicelo A. Mhlongo <[email protected]>

CC'ing Sakari.

Sakari, would you pick this patch ?

Quoted 44 lines · Robin Murphy

Quoted 32 lines

It's no longer practical for the OMAP IOMMU driver to trick arm_setup_iommu_dma_ops() into ignoring its presence, so let's use the same tactic as other IOMMU API users on 32-bit ARM and explicitly kick the arch code's dma_iommu_mapping out of the way to avoid problems.

Fixes 4720287c7bf7 ("iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()")

Signed-off-by Robin Murphy <[email protected]>

---
 drivers/media/platform/ti/omap3isp/isp.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/media/platform/ti/omap3isp/isp.c +7 -0

index 91101ba88ef0..b2210841a320 100644
@@ -1961,6 +1961,13 @@ static int isp_attach_iommu(struct isp_device *isp)
 	struct dma_iommu_mapping *mapping;
 	int ret;
 
+	/* We always want to replace any default mapping from the arch code */
+	mapping = to_dma_iommu_mapping(isp->dev);
+	if (mapping) {
+		arm_iommu_detach_device(isp->dev);
+		arm_iommu_release_mapping(mapping);
+	}
+
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
 	 * VAs. This will allocate a corresponding IOMMU domain.

Signature · 1 line

2.39.2.101.g768bb238c484.dirty

I have finally found time to test this patch on the Nokia N900 and can confirm it is working fine.

I was wondering - is there a reason that it is not merged yet? I tested on 6.14-rc2, which did not have it, and notice it is also not in linux-next.

If it helps:

Tested-by Sicelo A. Mhlongo <[email protected]>