Re: [PATCH v13 7/9] arm64: dts: qcom: agatti: Add OPE node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: agatti: duplicate OPE SMMU stream IDs in camss and isp_ope
--

commit 0728079779703fd918786c156a61313ff87b0a37
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

arm64: dts: qcom: agatti: Add OPE node

This commit adds the Offline Processing Engine (OPE) device tree node for the
Agatti platform (QCM2290), describing its register regions, clocks,
interrupt, interconnects, IOMMU mappings, and OPP table.

> diff --git a/arch/arm64/boot/dts/qcom/agatti.dtsi b/arch/arm64/boot/dts/qcom/agatti.dtsi
> index d9f4cbfd2861c..8f2b451b8f6e8 100644
> --- a/arch/arm64/boot/dts/qcom/agatti.dtsi
> +++ b/arch/arm64/boot/dts/qcom/agatti.dtsi
> @@ -1990,6 +1990,83 @@ port@1 {
>  			};
>  		};
>  
> +		isp_ope: isp@5c42000 {
> +			compatible = "qcom,qcm2290-camss-ope";
> +
> +			reg = <0x0 0x5c42000 0x0 0x400>,
> +			      <0x0 0x5c42400 0x0 0x200>,
> +			      <0x0 0x5c42600 0x0 0x200>,
> +			      <0x0 0x5c42800 0x0 0x4400>,
> +			      <0x0 0x5c46c00 0x0 0x190>,
> +			      <0x0 0x5c46d90 0x0 0xa00>;
> +			reg-names = "cdm",
> +				    "top",
> +				    "qos",
> +				    "pipeline",
> +				    "bus_read",
> +				    "bus_write";
> +
> +			clocks = <&gcc GCC_CAMSS_AXI_CLK>,
> +				 <&gcc GCC_CAMSS_OPE_CLK>,
> +				 <&gcc GCC_CAMSS_OPE_AHB_CLK>,
> +				 <&gcc GCC_CAMSS_NRT_AXI_CLK>,
> +				 <&gcc GCC_CAMSS_TOP_AHB_CLK>;
> +			clock-names = "axi",
> +				      "core",
> +				      "iface",
> +				      "nrt",
> +				      "top";
> +
> +			interrupts = <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>;
> +
> +			interconnects = <&bimc MASTER_APPSS_PROC RPM_ACTIVE_TAG
> +					 &config_noc SLAVE_CAMERA_CFG RPM_ACTIVE_TAG>,
> +					<&mmnrt_virt MASTER_CAMNOC_SF RPM_ALWAYS_TAG
> +					 &bimc SLAVE_EBI1 RPM_ALWAYS_TAG>;
> +			interconnect-names = "config",
> +					     "data";
> +
> +			iommus = <&apps_smmu 0x800 0x0>,
> +				 <&apps_smmu 0x820 0x0>,
> +				 <&apps_smmu 0x840 0x0>;

[Severity: Medium]
As reported by Sashiko on v12 [1], should the OPE stream IDs (0x800, 0x820,
and 0x840) be removed from the camss node (camss@5c11000) in
arch/arm64/boot/dts/qcom/agatti.dtsi now that isp_ope (isp@5c42000) claims
them?

In arch/arm64/boot/dts/qcom/agatti.dtsi, camss@5c11000 still lists the three
OPE stream IDs alongside VFE's 0x400 stream ID:

		camss: camss@5c11000 {
			...
			iommus = <&apps_smmu 0x400 0x0>,
				 <&apps_smmu 0x800 0x0>,
				 <&apps_smmu 0x820 0x0>,
				 <&apps_smmu 0x840 0x0>;

When both camss and isp_ope are enabled (for example via
arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso),
arm_smmu_master_alloc_smes() and arm_smmu_device_group() in
drivers/iommu/arm/arm-smmu/arm-smmu.c match the shared SMEs for 0x800, 0x820,
and 0x840 and merge camss and isp_ope into a single iommu_group and shared
DMA iommu_domain:

drivers/iommu/arm/arm-smmu/arm-smmu.c:arm_smmu_device_group() {
	...
	for_each_cfg_sme(cfg, fwspec, i, idx) {
		if (group && smmu->s2crs[idx].group &&
		    group != smmu->s2crs[idx].group) {
			mutex_unlock(&smmu->stream_map_mutex);
			return ERR_PTR(-EINVAL);
		}

		group = smmu->s2crs[idx].group;
	}

	if (group) {
		mutex_unlock(&smmu->stream_map_mutex);
		return iommu_group_ref_get(group);
	}
	...
}

Furthermore, if isp_ope is probed by the IOMMU core before camss (such as
when camss is enabled via an overlay), arm_smmu_device_group(camss) matches
isp_ope's group on SMEs 0x800, 0x820, and 0x840 and returns early without
populating smmu->s2crs[idx].group for camss's 0x400 SME.

[1] https://lore.kernel.org/r/20261102-camss-isp-ope-v12-7-702254689feb@oss.qualcomm.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-camss-isp-ope-v13-0-c2cbf15a39b1@oss.qualcomm.com?part=7