Hi Robin,
> Am 08.10.2024 um 16:00 schrieb Robin Murphy <robin.murphy@xxxxxxx>:
>
> On 08/10/2024 2:32 pm, H. Nikolaus Schaller wrote:
>> Hi Robin,
>>> Am 08.10.2024 um 15:17 schrieb Robin Murphy <robin.murphy@xxxxxxx>:
>>>
>>> The OMAP driver still has its own traditional firmware parsing and
>>> instance handling in omap_iommu_probe_device(), rather than using the
>>> generic fwnode-based paths. However, it also passes a hwdev to
>>> iommu_device_register(), thus registering a fwnode for each ops
>>> instance, wherein __iommu_probe_device() then fails to find matching ops
>>> for a client device with no fwspec and thus a NULL iommu_fwnode.
>>>
>>> Since omap-iommu is not known to coexist with any other IOMMU hardware
>>> and shares the same ops between all instances, we can reasonably remove
>>> the hwdev/fwnode registration to put it back into "legacy" mode where
>>> the ops are effectively global and ->probe_device remains responsible
>>> for filtering individual clients.
>>>
>>> Reported-by: Beleswar Padhi <b-padhi@xxxxxx>
>>> Reported-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
>>> Fixes: 17de3f5fdd35 ("iommu: Retire bus ops")
>>> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
>>> ---
>>> drivers/iommu/omap-iommu.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
>>> index c9528065a59a..425ae8e551dc 100644
>>> --- a/drivers/iommu/omap-iommu.c
>>> +++ b/drivers/iommu/omap-iommu.c
>>> @@ -1230,7 +1230,7 @@ static int omap_iommu_probe(struct platform_device *pdev)
>>> if (err)
>>> return err;
>>>
>>> - err = iommu_device_register(&obj->iommu, &omap_iommu_ops, &pdev->dev);
>>> + err = iommu_device_register(&obj->iommu, &omap_iommu_ops, NULL);
>> Thanks for this proposal, but this prevents my OMAP3 system completely
>> from booting (at least with v6.8-rc1):
>> ## Booting kernel from Legacy Image at 82000000 ...
>> Image Name: Linux-6.8.0-rc1-letux+
>> Image Type: ARM Linux Kernel Image (uncompressed)
>> Data Size: 6491520 Bytes = 6.2 MiB
>> Load Address: 80008000
>> Entry Point: 80008000
>> Verifying Checksum ... OK
>> ## Flattened Device Tree blob at 81c00000
>> Booting using the fdt blob at 0x81c00000
>> Loading Kernel Image ... OK
>> Using Device Tree in place at 81c00000, end 81c1fe8e
>> Starting kernel ...
>> --- no further reaction --
>
> Urgh... is it possible to get earlycon/ealyprintk output on this platform?
Ah, my mistake. earlycon did reveal a NULL pointer dereference coming from that
I have added some printk() to iommu_device_register() and friends. And one
assumed that hwdev is not a NULL pointer and we can print hwdev->kobj.name...
>
> As a stab in the dark by inspection, there might potentially be some interaction with "iommu: Move bus setup to IOMMU device registration" as well, for which the additional diff below might help...
>
> Thanks,
> Robin.
>
> ---->8----
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 425ae8e551dc..9112178e22d8 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1691,7 +1691,8 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
> if (!oiommu) {
> of_node_put(np);
> kfree(arch_data);
> - return ERR_PTR(-EINVAL);
> + /* Not fatal, will re-probe once the right instance registers itself */
> + return ERR_PTR(-ENODEV);
> }
>
> tmp->iommu_dev = oiommu;
Now I can boot with both patches applied (and Jason's patch and my printk removed),
but there is still (exactly the same as with Jason's patch):
root@letux:~# uname -a
Linux letux 6.8.0-rc1-letux+ #19517 SMP PREEMPT Tue Oct 8 17:23:11 CEST 2024 armv7l GNU/Linux
root@letux:~# dmesg|fgrep iommu
[ 0.700195] iommu: Default domain type: Translated
[ 0.705078] iommu: DMA domain TLB invalidation policy: strict mode
[ 0.728393] platform 480bc000.isp: Adding to iommu group 0
[ 0.734161] omap-iommu 480bd400.mmu: 480bd400.mmu registered
[ 23.565216] omap3isp 480bc000.isp: iommu configuration for device failed with -ETIMEDOUT
[ 23.605102] omap-iommu 480bd400.mmu: 480bd400.mmu: version 1.1
root@letux:~# dmesg|fgrep isp
[ 0.000000] OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk)
[ 0.728393] platform 480bc000.isp: Adding to iommu group 0
[ 11.472045] omapdss_dss 48050000.dss: bound 48050400.dispc (ops dsi_vc_flush_receive_data [omapdrm]) <-- false positive of fgrep
[ 23.557586] omap3isp 480bc000.isp: deferred probe timeout, ignoring dependency
[ 23.565216] omap3isp 480bc000.isp: iommu configuration for device failed with -ETIMEDOUT <-- duplicate with fgrep iommu
[ 23.625183] omap3isp 480bc000.isp: supply vdd-csiphy1 not found, using dummy regulator
[ 23.657135] omap3isp 480bc000.isp: supply vdd-csiphy2 not found, using dummy regulator
[ 23.665832] omap3isp 480bc000.isp: Revision 15.0 found
[ 23.700073] omap3isp 480bc000.isp: failed to attach device to VA mapping
[ 23.724182] omap3isp 480bc000.isp: unable to attach to IOMMU
[ 23.731262] omap3isp: probe of 480bc000.isp failed with error -16
root@letux:~#
The -ETIMEDOUT seems to come from of_iommu_configure().
I also did now run the same with v6.7 to compare timing and message sequence:
root@letux:~# uname -a
Linux letux 6.7.0-letux+ #19518 SMP PREEMPT Tue Oct 8 17:48:27 CEST 2024 armv7l GNU/Linux
root@letux:~# dmesg|fgrep iommu
[ 0.412078] iommu: Default domain type: Translated
[ 0.412109] iommu: DMA domain TLB invalidation policy: strict mode
[ 0.415008] platform 480bc000.isp: Adding to iommu group 0
[ 0.415191] omap-iommu 480bd400.mmu: 480bd400.mmu registered
[ 11.046630] omap-iommu 480bd400.mmu: 480bd400.mmu: version 1.1
root@letux:~# dmesg|fgrep isp
[ 0.000000] OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk)
[ 0.415008] platform 480bc000.isp: Adding to iommu group 0
[ 10.885711] omap3isp 480bc000.isp: supply vdd-csiphy1 not found, using dummy regulator
[ 10.953338] omap3isp 480bc000.isp: supply vdd-csiphy2 not found, using dummy regulator
[ 11.025482] omap3isp 480bc000.isp: Revision 15.0 found
[ 11.084014] omap3isp 480bc000.isp: hist: using DMA channel dma0chan15
[ 11.150695] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP CCP2 was not initialized!
[ 11.162109] omap3isp 480bc000.isp: isp_xclk_set_rate: cam_xclka set to 24685714 Hz (div 7)
[ 11.199523] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP CSI2a was not initialized!
[ 11.291931] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP CCDC was not initialized!
[ 11.321807] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP preview was not initialized!
[ 11.393188] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP resizer was not initialized!
[ 11.425109] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP AEWB was not initialized!
[ 11.434082] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP AF was not initialized!
[ 11.534820] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP histogram was not initialized!
[ 11.565155] omap3isp 480bc000.isp: parsing parallel interface
[ 12.589965] omapdss_dss 48050000.dss: bound 48050400.dispc (ops dsi_vc_flush_receive_data [omapdrm])
root@letux:~#
Interestingly, there is no -ETIMEOUT and initialization start much earlier.
BR and thanks,
Nikolaus
