Re: [PATCH v6 11/12] arm64: dts: qcom: sm8650: Add Iris context bank subnodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: iris: missing IOMMU readiness check in iris_create_cb_dev()
--

commit b47d404291c07cfcdeb8c7a25de5057ab9c274a2
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

arm64: dts: qcom: sm8650: Add Iris context bank subnodes

This commit moves the iommus property on the SM8650 video-codec node into
dedicated non-pixel and pixel child subnodes so that the non-pixel stream can
specify its own restricted iommu-ranges. It also adds address-cells,
size-cells, and dma-ranges to the parent video-codec node.

> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 86fbb0fd6472d..fd82b83c228ef 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -5274,11 +5274,12 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
>  				      "xo",
>  				      "core";
>  
> -			iommus = <&apps_smmu 0x1940 0>,
> -				 <&apps_smmu 0x1947 0>;
> -
>  			dma-coherent;
>  
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			dma-ranges = <0x0 0x0 0x0 0xe0000000>;
> +
>  			/*
>  			 * IRIS firmware is signed by vendors, only
>  			 * enable on boards where the proper signed firmware
> @@ -5286,6 +5287,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
>  			 */
>  			status = "disabled";
>  
> +			non-pixel {
> +				iommus = <&apps_smmu 0x1940 0x0>;
> +				iommu-ranges = <0x25800000 0xba800000>;
> +			};
> +
> +			pixel {
> +				iommus = <&apps_smmu 0x1947 0x0>;
> +			};

[Severity: High]
When moving the iommus property from the parent video-codec node to the
driverless non-pixel and pixel child subnodes, could iris_probe() succeed
before apps_smmu has probed, leaving the context bank devices without DMA or
IOMMU configuration?

With iommus on the child subnodes rather than the parent node,
__fw_devlink_link_to_suppliers() only creates DL_FLAG_SYNC_STATE_ONLY proxy
links on the parent video-codec device, and parse_iommus in
drivers/of/property.c is marked optional unless fw_devlink.strict=1. As a
result, really_probe() on the parent video-codec device no longer defers on
apps_smmu in device_links_check_suppliers() or platform_dma_configure().

When iris_probe() subsequently creates the child devices via
iris_init_cb_devs() -> iris_create_cb_dev(), platform_device_register_full()
triggers the bus notifier path:

  platform_device_register_full()
    device_add()
      bus_notify(BUS_NOTIFY_ADD_DEVICE)
        iommu_bus_notifier()
          iommu_probe_device()
            iommu_init_device()
              platform_dma_configure()
                of_dma_configure_id()
                  of_iommu_configure()

If apps_smmu has not registered yet (for instance when CONFIG_ARM_SMMU=m or
during asynchronous probing), of_iommu_configure() returns -EPROBE_DEFER,
which causes of_dma_configure_id() to free the range map, clear
dev->dma_range_map, skip arch_setup_dma_ops(), and return -EPROBE_DEFER.
However, iommu_init_device() and iommu_bus_notifier() ignore that return
value, so platform_device_register_full() still succeeds:

  drivers/media/platform/qcom/iris/iris_probe.c:iris_create_cb_dev() {
      ...
      pdev = platform_device_register_full(&plat_dev_info);
      of_node_put(child_of_node);
      if (IS_ERR(pdev))
          return ERR_CAST(pdev);

      dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
      dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));

      return &pdev->dev;
  }

Because no driver binds to the non-pixel or pixel platform devices,
really_probe() -> platform_dma_configure() never runs for them, and
iris_probe() registers the V4L2 video nodes while core->np_dev and
core->p_dev have no IOMMU group, dma_range_map, or DMA ops configured.

Should iris_create_cb_dev() check device_iommu_mapped(&pdev->dev) (or verify
that IOMMU/DMA setup completed) and unregister pdev while returning
-EPROBE_DEFER if the SMMU is not ready yet?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-vpu_iommu_iova_handling-v6-0-e4b6ad864f8f@oss.qualcomm.com?part=11