Re: [PATCH 2/2] interconnect: qcom: Add SM8550 interconnect provider driver

From: Krzysztof Kozlowski
Date: Wed Nov 16 2022 - 12:32:52 EST

Next message: Lee Jones: "Re: [PATCH 01/18] mfd: 88pm800: Replace irqchip mask_invert with unmask_base"
Previous message: Andy Shevchenko: "Re: [PATCH v4 0/7] pinctrl: intel: Enable PWM optional feature"
In reply to: Abel Vesa: "[PATCH 2/2] interconnect: qcom: Add SM8550 interconnect provider driver"
Next in thread: Krzysztof Kozlowski: "Re: [PATCH 1/2] dt-bindings: interconnect: Add Qualcomm SM8550 DT bindings"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 16/11/2022 12:45, Abel Vesa wrote:
> Add driver for the Qualcomm interconnect buses found in SM8550 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.
>
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>

Thank you for your patch. There is something to discuss/improve.

> +static struct qcom_icc_node * const config_noc_nodes[] = {
> + [MASTER_CNOC_CFG] = &qsm_cfg,
> + [SLAVE_AHB2PHY_SOUTH] = &qhs_ahb2phy0,
> + [SLAVE_AHB2PHY_NORTH] = &qhs_ahb2phy1,
> + [SLAVE_APPSS] = &qhs_apss,
> + [SLAVE_CAMERA_CFG] = &qhs_camera_cfg,
> + [SLAVE_CLK_CTL] = &qhs_clk_ctl,
> + [SLAVE_RBCPR_CX_CFG] = &qhs_cpr_cx,
> + [SLAVE_RBCPR_MMCX_CFG] = &qhs_cpr_mmcx,
> + [SLAVE_RBCPR_MXA_CFG] = &qhs_cpr_mxa,
> + [SLAVE_RBCPR_MXC_CFG] = &qhs_cpr_mxc,
> + [SLAVE_CPR_NSPCX] = &qhs_cpr_nspcx,
> + [SLAVE_CRYPTO_0_CFG] = &qhs_crypto0_cfg,
> + [SLAVE_CX_RDPM] = &qhs_cx_rdpm,
> + [SLAVE_DISPLAY_CFG] = &qhs_display_cfg,
> + [SLAVE_GFX3D_CFG] = &qhs_gpuss_cfg,
> + [SLAVE_I2C] = &qhs_i2c,
> + [SLAVE_IMEM_CFG] = &qhs_imem_cfg,
> + [SLAVE_IPA_CFG] = &qhs_ipa,
> + [SLAVE_IPC_ROUTER_CFG] = &qhs_ipc_router,
> + [SLAVE_CNOC_MSS] = &qhs_mss_cfg,
> + [SLAVE_MX_RDPM] = &qhs_mx_rdpm,
> + [SLAVE_PCIE_0_CFG] = &qhs_pcie0_cfg,
> + [SLAVE_PCIE_1_CFG] = &qhs_pcie1_cfg,
> + [SLAVE_PDM] = &qhs_pdm,
> + [SLAVE_PIMEM_CFG] = &qhs_pimem_cfg,
> + [SLAVE_PRNG] = &qhs_prng,
> + [SLAVE_QDSS_CFG] = &qhs_qdss_cfg,
> + [SLAVE_QSPI_0] = &qhs_qspi,
> + [SLAVE_QUP_1] = &qhs_qup1,
> + [SLAVE_QUP_2] = &qhs_qup2,
> + [SLAVE_SDCC_2] = &qhs_sdc2,
> + [SLAVE_SDCC_4] = &qhs_sdc4,
> + [SLAVE_SPSS_CFG] = &qhs_spss_cfg,
> + [SLAVE_TCSR] = &qhs_tcsr,
> + [SLAVE_TLMM] = &qhs_tlmm,
> + [SLAVE_UFS_MEM_CFG] = &qhs_ufs_mem_cfg,
> + [SLAVE_USB3_0] = &qhs_usb3_0,
> + [SLAVE_VENUS_CFG] = &qhs_venus_cfg,
> + [SLAVE_VSENSE_CTRL_CFG] = &qhs_vsense_ctrl_cfg,
> + [SLAVE_LPASS_QTB_CFG] = &qss_lpass_qtb_cfg,
> + [SLAVE_CNOC_MNOC_CFG] = &qss_mnoc_cfg,
> + [SLAVE_NSP_QTB_CFG] = &qss_nsp_qtb_cfg,
> + [SLAVE_PCIE_ANOC_CFG] = &qss_pcie_anoc_cfg,
> + [SLAVE_QDSS_STM] = &xs_qdss_stm,
> + [SLAVE_TCU] = &xs_sys_tcu_cfg,
> +};
> +
> +static struct qcom_icc_desc sm8550_config_noc = {
> + .nodes = config_noc_nodes,
> + .num_nodes = ARRAY_SIZE(config_noc_nodes),
> + .bcms = config_noc_bcms,
> + .num_bcms = ARRAY_SIZE(config_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm *cnoc_main_bcms[] = {

Const pointers. Also in several other places as well.

> + &bcm_cn0,
> +};
> +
> +static struct qcom_icc_node *cnoc_main_nodes[] = {
> + [MASTER_GEM_NOC_CNOC] = &qnm_gemnoc_cnoc,
> + [MASTER_GEM_NOC_PCIE_SNOC] = &qnm_gemnoc_pcie,
> + [SLAVE_AOSS] = &qhs_aoss,
> + [SLAVE_TME_CFG] = &qhs_tme_cfg,
> + [SLAVE_CNOC_CFG] = &qss_cfg,
> + [SLAVE_DDRSS_CFG] = &qss_ddrss_cfg,
> + [SLAVE_BOOT_IMEM] = &qxs_boot_imem,
> + [SLAVE_IMEM] = &qxs_imem,
> + [SLAVE_PCIE_0] = &xs_pcie_0,
> + [SLAVE_PCIE_1] = &xs_pcie_1,
> +};
> +
> +static struct qcom_icc_desc sm8550_cnoc_main = {

This should be const.

> + .nodes = cnoc_main_nodes,
> + .num_nodes = ARRAY_SIZE(cnoc_main_nodes),
> + .bcms = cnoc_main_bcms,
> + .num_bcms = ARRAY_SIZE(cnoc_main_bcms),
> +};
> +
> +static struct qcom_icc_bcm *gem_noc_bcms[] = {
> + &bcm_sh0,
> + &bcm_sh1,
> + &bcm_sh0_disp,
> + &bcm_sh1_disp,
> + &bcm_sh0_cam_ife_0,
> + &bcm_sh1_cam_ife_0,
> + &bcm_sh0_cam_ife_1,
> + &bcm_sh1_cam_ife_1,
> + &bcm_sh0_cam_ife_2,
> + &bcm_sh1_cam_ife_2,
> +};
> +
> +static struct qcom_icc_node *gem_noc_nodes[] = {
> + [MASTER_GPU_TCU] = &alm_gpu_tcu,
> + [MASTER_SYS_TCU] = &alm_sys_tcu,
> + [MASTER_APPSS_PROC] = &chm_apps,
> + [MASTER_GFX3D] = &qnm_gpu,
> + [MASTER_LPASS_GEM_NOC] = &qnm_lpass_gemnoc,
> + [MASTER_MSS_PROC] = &qnm_mdsp,
> + [MASTER_MNOC_HF_MEM_NOC] = &qnm_mnoc_hf,
> + [MASTER_MNOC_SF_MEM_NOC] = &qnm_mnoc_sf,
> + [MASTER_COMPUTE_NOC] = &qnm_nsp_gemnoc,
> + [MASTER_ANOC_PCIE_GEM_NOC] = &qnm_pcie,
> + [MASTER_SNOC_GC_MEM_NOC] = &qnm_snoc_gc,
> + [MASTER_SNOC_SF_MEM_NOC] = &qnm_snoc_sf,
> + [SLAVE_GEM_NOC_CNOC] = &qns_gem_noc_cnoc,
> + [SLAVE_LLCC] = &qns_llcc,
> + [SLAVE_MEM_NOC_PCIE_SNOC] = &qns_pcie,
> + [MASTER_MNOC_HF_MEM_NOC_DISP] = &qnm_mnoc_hf_disp,
> + [MASTER_ANOC_PCIE_GEM_NOC_DISP] = &qnm_pcie_disp,
> + [SLAVE_LLCC_DISP] = &qns_llcc_disp,
> + [MASTER_MNOC_HF_MEM_NOC_CAM_IFE_0] = &qnm_mnoc_hf_cam_ife_0,
> + [MASTER_MNOC_SF_MEM_NOC_CAM_IFE_0] = &qnm_mnoc_sf_cam_ife_0,
> + [MASTER_ANOC_PCIE_GEM_NOC_CAM_IFE_0] = &qnm_pcie_cam_ife_0,
> + [SLAVE_LLCC_CAM_IFE_0] = &qns_llcc_cam_ife_0,
> + [MASTER_MNOC_HF_MEM_NOC_CAM_IFE_1] = &qnm_mnoc_hf_cam_ife_1,
> + [MASTER_MNOC_SF_MEM_NOC_CAM_IFE_1] = &qnm_mnoc_sf_cam_ife_1,
> + [MASTER_ANOC_PCIE_GEM_NOC_CAM_IFE_1] = &qnm_pcie_cam_ife_1,
> + [SLAVE_LLCC_CAM_IFE_1] = &qns_llcc_cam_ife_1,
> + [MASTER_MNOC_HF_MEM_NOC_CAM_IFE_2] = &qnm_mnoc_hf_cam_ife_2,
> + [MASTER_MNOC_SF_MEM_NOC_CAM_IFE_2] = &qnm_mnoc_sf_cam_ife_2,
> + [MASTER_ANOC_PCIE_GEM_NOC_CAM_IFE_2] = &qnm_pcie_cam_ife_2,
> + [SLAVE_LLCC_CAM_IFE_2] = &qns_llcc_cam_ife_2,
> +};
> +
> +static struct qcom_icc_desc sm8550_gem_noc = {

Const... and so on - multiple places.

> + .nodes = gem_noc_nodes,
> + .num_nodes = ARRAY_SIZE(gem_noc_nodes),
> + .bcms = gem_noc_bcms,
> + .num_bcms = ARRAY_SIZE(gem_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const lpass_ag_noc_bcms[] = {
> +};
> +
> +static struct qcom_icc_node * const lpass_ag_noc_nodes[] = {
> + [MASTER_LPIAON_NOC] = &qnm_lpiaon_noc,
> + [SLAVE_LPASS_GEM_NOC] = &qns_lpass_ag_noc_gemnoc,
> +};
> +
> +static const struct qcom_icc_desc sm8550_lpass_ag_noc = {
> + .nodes = lpass_ag_noc_nodes,
> + .num_nodes = ARRAY_SIZE(lpass_ag_noc_nodes),
> + .bcms = lpass_ag_noc_bcms,
> + .num_bcms = ARRAY_SIZE(lpass_ag_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const lpass_lpiaon_noc_bcms[] = {
> + &bcm_lp0,
> +};
> +
> +static struct qcom_icc_node * const lpass_lpiaon_noc_nodes[] = {
> + [MASTER_LPASS_LPINOC] = &qnm_lpass_lpinoc,
> + [SLAVE_LPIAON_NOC_LPASS_AG_NOC] = &qns_lpass_aggnoc,
> +};
> +
> +static const struct qcom_icc_desc sm8550_lpass_lpiaon_noc = {
> + .nodes = lpass_lpiaon_noc_nodes,
> + .num_nodes = ARRAY_SIZE(lpass_lpiaon_noc_nodes),
> + .bcms = lpass_lpiaon_noc_bcms,
> + .num_bcms = ARRAY_SIZE(lpass_lpiaon_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const lpass_lpicx_noc_bcms[] = {
> +};
> +
> +static struct qcom_icc_node * const lpass_lpicx_noc_nodes[] = {
> + [MASTER_LPASS_PROC] = &qxm_lpinoc_dsp_axim,
> + [SLAVE_LPICX_NOC_LPIAON_NOC] = &qns_lpi_aon_noc,
> +};
> +
> +static const struct qcom_icc_desc sm8550_lpass_lpicx_noc = {
> + .nodes = lpass_lpicx_noc_nodes,
> + .num_nodes = ARRAY_SIZE(lpass_lpicx_noc_nodes),
> + .bcms = lpass_lpicx_noc_bcms,
> + .num_bcms = ARRAY_SIZE(lpass_lpicx_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const mc_virt_bcms[] = {
> + &bcm_acv,
> + &bcm_mc0,
> + &bcm_acv_disp,
> + &bcm_mc0_disp,
> + &bcm_acv_cam_ife_0,
> + &bcm_mc0_cam_ife_0,
> + &bcm_acv_cam_ife_1,
> + &bcm_mc0_cam_ife_1,
> + &bcm_acv_cam_ife_2,
> + &bcm_mc0_cam_ife_2,
> +};
> +
> +static struct qcom_icc_node * const mc_virt_nodes[] = {
> + [MASTER_LLCC] = &llcc_mc,
> + [SLAVE_EBI1] = &ebi,
> + [MASTER_LLCC_DISP] = &llcc_mc_disp,
> + [SLAVE_EBI1_DISP] = &ebi_disp,
> + [MASTER_LLCC_CAM_IFE_0] = &llcc_mc_cam_ife_0,
> + [SLAVE_EBI1_CAM_IFE_0] = &ebi_cam_ife_0,
> + [MASTER_LLCC_CAM_IFE_1] = &llcc_mc_cam_ife_1,
> + [SLAVE_EBI1_CAM_IFE_1] = &ebi_cam_ife_1,
> + [MASTER_LLCC_CAM_IFE_2] = &llcc_mc_cam_ife_2,
> + [SLAVE_EBI1_CAM_IFE_2] = &ebi_cam_ife_2,
> +};
> +
> +static const struct qcom_icc_desc sm8550_mc_virt = {
> + .nodes = mc_virt_nodes,
> + .num_nodes = ARRAY_SIZE(mc_virt_nodes),
> + .bcms = mc_virt_bcms,
> + .num_bcms = ARRAY_SIZE(mc_virt_bcms),
> +};
> +
> +static struct qcom_icc_bcm *mmss_noc_bcms[] = {
> + &bcm_mm0,
> + &bcm_mm1,
> + &bcm_mm0_disp,
> + &bcm_mm0_cam_ife_0,
> + &bcm_mm1_cam_ife_0,
> + &bcm_mm0_cam_ife_1,
> + &bcm_mm1_cam_ife_1,
> + &bcm_mm0_cam_ife_2,
> + &bcm_mm1_cam_ife_2,
> +};
> +
> +static struct qcom_icc_node * const mmss_noc_nodes[] = {
> + [MASTER_CAMNOC_HF] = &qnm_camnoc_hf,
> + [MASTER_CAMNOC_ICP] = &qnm_camnoc_icp,
> + [MASTER_CAMNOC_SF] = &qnm_camnoc_sf,
> + [MASTER_MDP] = &qnm_mdp,
> + [MASTER_CDSP_HCP] = &qnm_vapss_hcp,
> + [MASTER_VIDEO] = &qnm_video,
> + [MASTER_VIDEO_CV_PROC] = &qnm_video_cv_cpu,
> + [MASTER_VIDEO_PROC] = &qnm_video_cvp,
> + [MASTER_VIDEO_V_PROC] = &qnm_video_v_cpu,
> + [MASTER_CNOC_MNOC_CFG] = &qsm_mnoc_cfg,
> + [SLAVE_MNOC_HF_MEM_NOC] = &qns_mem_noc_hf,
> + [SLAVE_MNOC_SF_MEM_NOC] = &qns_mem_noc_sf,
> + [SLAVE_SERVICE_MNOC] = &srvc_mnoc,
> + [MASTER_MDP_DISP] = &qnm_mdp_disp,
> + [SLAVE_MNOC_HF_MEM_NOC_DISP] = &qns_mem_noc_hf_disp,
> + [MASTER_CAMNOC_HF_CAM_IFE_0] = &qnm_camnoc_hf_cam_ife_0,
> + [MASTER_CAMNOC_ICP_CAM_IFE_0] = &qnm_camnoc_icp_cam_ife_0,
> + [MASTER_CAMNOC_SF_CAM_IFE_0] = &qnm_camnoc_sf_cam_ife_0,
> + [SLAVE_MNOC_HF_MEM_NOC_CAM_IFE_0] = &qns_mem_noc_hf_cam_ife_0,
> + [SLAVE_MNOC_SF_MEM_NOC_CAM_IFE_0] = &qns_mem_noc_sf_cam_ife_0,
> + [MASTER_CAMNOC_HF_CAM_IFE_1] = &qnm_camnoc_hf_cam_ife_1,
> + [MASTER_CAMNOC_ICP_CAM_IFE_1] = &qnm_camnoc_icp_cam_ife_1,
> + [MASTER_CAMNOC_SF_CAM_IFE_1] = &qnm_camnoc_sf_cam_ife_1,
> + [SLAVE_MNOC_HF_MEM_NOC_CAM_IFE_1] = &qns_mem_noc_hf_cam_ife_1,
> + [SLAVE_MNOC_SF_MEM_NOC_CAM_IFE_1] = &qns_mem_noc_sf_cam_ife_1,
> + [MASTER_CAMNOC_HF_CAM_IFE_2] = &qnm_camnoc_hf_cam_ife_2,
> + [MASTER_CAMNOC_ICP_CAM_IFE_2] = &qnm_camnoc_icp_cam_ife_2,
> + [MASTER_CAMNOC_SF_CAM_IFE_2] = &qnm_camnoc_sf_cam_ife_2,
> + [SLAVE_MNOC_HF_MEM_NOC_CAM_IFE_2] = &qns_mem_noc_hf_cam_ife_2,
> + [SLAVE_MNOC_SF_MEM_NOC_CAM_IFE_2] = &qns_mem_noc_sf_cam_ife_2,
> +};
> +
> +static const struct qcom_icc_desc sm8550_mmss_noc = {
> + .nodes = mmss_noc_nodes,
> + .num_nodes = ARRAY_SIZE(mmss_noc_nodes),
> + .bcms = mmss_noc_bcms,
> + .num_bcms = ARRAY_SIZE(mmss_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const nsp_noc_bcms[] = {
> + &bcm_co0,
> +};
> +
> +static struct qcom_icc_node * const nsp_noc_nodes[] = {
> + [MASTER_CDSP_PROC] = &qxm_nsp,
> + [SLAVE_CDSP_MEM_NOC] = &qns_nsp_gemnoc,
> +};
> +
> +static const struct qcom_icc_desc sm8550_nsp_noc = {
> + .nodes = nsp_noc_nodes,
> + .num_nodes = ARRAY_SIZE(nsp_noc_nodes),
> + .bcms = nsp_noc_bcms,
> + .num_bcms = ARRAY_SIZE(nsp_noc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const pcie_anoc_bcms[] = {
> + &bcm_sn7,
> +};
> +
> +static struct qcom_icc_node * const pcie_anoc_nodes[] = {
> + [MASTER_PCIE_ANOC_CFG] = &qsm_pcie_anoc_cfg,
> + [MASTER_PCIE_0] = &xm_pcie3_0,
> + [MASTER_PCIE_1] = &xm_pcie3_1,
> + [SLAVE_ANOC_PCIE_GEM_NOC] = &qns_pcie_mem_noc,
> + [SLAVE_SERVICE_PCIE_ANOC] = &srvc_pcie_aggre_noc,
> +};
> +
> +static const struct qcom_icc_desc sm8550_pcie_anoc = {
> + .nodes = pcie_anoc_nodes,
> + .num_nodes = ARRAY_SIZE(pcie_anoc_nodes),
> + .bcms = pcie_anoc_bcms,
> + .num_bcms = ARRAY_SIZE(pcie_anoc_bcms),
> +};
> +
> +static struct qcom_icc_bcm * const system_noc_bcms[] = {
> + &bcm_sn0,
> + &bcm_sn1,
> + &bcm_sn2,
> + &bcm_sn3,
> +};
> +
> +static struct qcom_icc_node * const system_noc_nodes[] = {
> + [MASTER_GIC_AHB] = &qhm_gic,
> + [MASTER_A1NOC_SNOC] = &qnm_aggre1_noc,
> + [MASTER_A2NOC_SNOC] = &qnm_aggre2_noc,
> + [MASTER_GIC] = &xm_gic,
> + [SLAVE_SNOC_GEM_NOC_GC] = &qns_gemnoc_gc,
> + [SLAVE_SNOC_GEM_NOC_SF] = &qns_gemnoc_sf,
> +};
> +
> +static const struct qcom_icc_desc sm8550_system_noc = {
> + .nodes = system_noc_nodes,
> + .num_nodes = ARRAY_SIZE(system_noc_nodes),
> + .bcms = system_noc_bcms,
> + .num_bcms = ARRAY_SIZE(system_noc_bcms),
> +};
> +
> +static int qnoc_probe(struct platform_device *pdev)
> +{
> + const struct qcom_icc_desc *desc;
> + struct icc_onecell_data *data;
> + struct icc_provider *provider;
> + struct qcom_icc_node * const *qnodes;
> + struct qcom_icc_provider *qp;
> + struct icc_node *node;
> + size_t num_nodes, i;
> + int ret;
> +
> + desc = device_get_match_data(&pdev->dev);
> + if (!desc)
> + return -EINVAL;
> +
> + qnodes = desc->nodes;
> + num_nodes = desc->num_nodes;
> +
> + qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> + if (!qp)
> + return -ENOMEM;
> +
> + data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + provider = &qp->provider;
> + provider->dev = &pdev->dev;
> + provider->set = qcom_icc_set;
> + provider->pre_aggregate = qcom_icc_pre_aggregate;
> + provider->aggregate = qcom_icc_aggregate;
> + provider->xlate_extended = qcom_icc_xlate_extended;
> + INIT_LIST_HEAD(&provider->nodes);
> + provider->data = data;
> +
> + qp->dev = &pdev->dev;
> + qp->bcms = desc->bcms;
> + qp->num_bcms = desc->num_bcms;
> +
> + qp->voter = of_bcm_voter_get(qp->dev, NULL);
> + if (IS_ERR(qp->voter))
> + return PTR_ERR(qp->voter);
> +
> + ret = icc_provider_add(provider);
> + if (ret) {
> + dev_err(&pdev->dev, "error adding interconnect provider\n");

return dev_err_probe().

> + return ret;
> + }
> +

Best regards,
Krzysztof

Next message: Lee Jones: "Re: [PATCH 01/18] mfd: 88pm800: Replace irqchip mask_invert with unmask_base"
Previous message: Andy Shevchenko: "Re: [PATCH v4 0/7] pinctrl: intel: Enable PWM optional feature"
In reply to: Abel Vesa: "[PATCH 2/2] interconnect: qcom: Add SM8550 interconnect provider driver"
Next in thread: Krzysztof Kozlowski: "Re: [PATCH 1/2] dt-bindings: interconnect: Add Qualcomm SM8550 DT bindings"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]