Re: [PATCH 2/2] interconnect: qcom: add Maili interconnect provider driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Overlapping array indices in `mmss_noc_nodes` due to unsequential DT binding macros.
- [Low] INTERCONNECT_QCOM_MAILI Kconfig entry deviates from standard subsystem patterns by missing architecture dependencies and improperly adding `default ARCH_QCOM`.
--

commit 8dd65bdb718ff52c61f92fb6eb9e65eee995529e
Author: Raviteja Laggyshetty <raviteja.laggyshetty@oss.qualcomm.com>

interconnect: qcom: add Maili interconnect provider driver

Add driver for the Qualcomm interconnect buses found in Maili based platforms.
The topology consists of several NoCs that are controlled by a remote processor
that collects the aggregated bandwidth for each master-slave pairs.

> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index 5b8a094ca4ede..5b57f111c63c0 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -47,6 +47,16 @@ config INTERCONNECT_QCOM_KAANAPALI
>  	  This is a driver for the Qualcomm Network-on-Chip on kaanapali-based
>  	  platforms.
>  
> +config INTERCONNECT_QCOM_MAILI
> +	tristate "Qualcomm MAILI interconnect driver"
> +	depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
> +	select INTERCONNECT_QCOM_RPMH
> +	select INTERCONNECT_QCOM_BCM_VOTER
> +	default ARCH_QCOM

[Severity: Low]
Does this Kconfig definition miss the standard "depends on ARM64 || COMPILE_TEST"
constraint found in other Qualcomm interconnect drivers?

Also, by specifying "default ARCH_QCOM", does this unconditionally force the
driver to build on all Qualcomm targets rather than relying on defconfigs,
potentially causing unnecessary bloat?

> +	help
> +	  This is a driver for the Qualcomm Network-on-Chip on maili-based
> +	  platforms.
> +
>  config INTERCONNECT_QCOM_MSM8909
>  	tristate "Qualcomm MSM8909 interconnect driver"
>  	depends on INTERCONNECT_QCOM

[ ... ]

> diff --git a/drivers/interconnect/qcom/maili.c b/drivers/interconnect/qcom/maili.c
> new file mode 100644
> index 0000000000000..e5eed7f998f92
> --- /dev/null
> +++ b/drivers/interconnect/qcom/maili.c

[ ... ]

> +static struct qcom_icc_node * const mmss_noc_nodes[] = {
> +	[MASTER_CAMNOC_HF] = &qnm_camnoc_hf,
> +	[MASTER_CAMNOC_NRT_ICP_SF] = &qnm_camnoc_nrt_icp_sf,
> +	[MASTER_CAMNOC_RT_CDM_SF] = &qnm_camnoc_rt_cdm_sf,
> +	[MASTER_CAMNOC_SF] = &qnm_camnoc_sf,
> +	[MASTER_MDP] = &qnm_mdp,
> +	[MASTER_MDSS_DCP] = &qnm_mdss_dcp,

[Severity: High]
Does mmss_noc_nodes[] have overlapping array indices due to how the
dt-bindings are defined?

Looking at dt-bindings/interconnect/qcom,maili-rpmh.h:

#define MASTER_CAMNOC_HF                0
#define MASTER_MDP                      1
...
#define MASTER_CAMNOC_NRT_ICP_SF        0
#define MASTER_CAMNOC_RT_CDM_SF         1
#define MASTER_CAMNOC_SF                2

Because these macros share the same values, the later designated initializers
in mmss_noc_nodes[] will silently overwrite the earlier ones.

Does this unintentionally drop qnm_camnoc_hf, qnm_mdp, and qns_mem_noc_hf from
the provider array, breaking interconnect requests that target them?

> +	[MASTER_CDSP_HCP] = &qnm_vapss_hcp,
> +	[MASTER_VIDEO_CV_PROC] = &qnm_video_cv_cpu,
> +	[MASTER_VIDEO_EVA] = &qnm_video_eva,
> +	[MASTER_VIDEO_MVP] = &qnm_video_mvp,
> +	[MASTER_VIDEO_V_PROC] = &qnm_video_v_cpu,
> +	[SLAVE_MNOC_HF_MEM_NOC] = &qns_mem_noc_hf,
> +	[SLAVE_MNOC_SF_MEM_NOC] = &qns_mem_noc_sf,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-maili_icc-v1-0-0aa8286e6748@oss.qualcomm.com?part=2