Received-SPF: pass (google.com: domain of linux-kernel+bounces-16476-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <4d4d5d3b-cdb2-484a-8297-4b8bb0817986@quicinc.com>
Date: Thu, 4 Jan 2024 15:29:54 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/4] interconnect: qcom: x1e80100: Remove bogus per-RSC
 BCMs and nodes
To: Konrad Dybcio <konrad.dybcio@linaro.org>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Georgi Djakov <djakov@kernel.org>, Rob Herring
	<robh+dt@kernel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        "Sibi Sankar" <quic_sibis@quicinc.com>,
        Abel Vesa <abel.vesa@linaro.org>
CC: Marijn Suijten <marijn.suijten@somainline.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-pm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>
References: <20240102-topic-x1e_fixes-v1-0-70723e08d5f6@linaro.org>
 <20240102-topic-x1e_fixes-v1-1-70723e08d5f6@linaro.org>
Content-Language: en-US
From: Rajendra Nayak <quic_rjendra@quicinc.com>
In-Reply-To: <20240102-topic-x1e_fixes-v1-1-70723e08d5f6@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/2/2024 11:59 PM, Konrad Dybcio wrote:
> The downstream kernel has infrastructure for passing votes from different
> interconnect nodes onto different RPMh RSCs. This neither implemented, not
> is going to be implemented upstream (in favor of a different solution
> using ICC tags through the same node).
> 
> Unfortunately, as it happens, meaningless (in the upstream context) parts
> of the vendor driver were copied, ending up causing havoc - since all
> "per-RSC" (in quotes because they all point to the main APPS one) BCMs
> defined within the driver overwrite the value in RPMh on every
> aggregation.
> 
> To both avoid keeping bogus code around and possibly introducing
> impossible-to-track-down bugs (busses shutting down for no reason), get
> rid of the duplicated BCMs and their associated ICC nodes.

Thanks Konrad for catching this, I do see these nodes in other Qualcomm 
SoCs upstream (atleast sm8350/sm8450 and sm8550), perhaps they need to 
be cleaned up as well?

Reviewed-by: Rajendra Nayak <quic_rjendra@quicinc.com>

> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   drivers/interconnect/qcom/x1e80100.c | 315 -----------------------------------
>   1 file changed, 315 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/x1e80100.c b/drivers/interconnect/qcom/x1e80100.c
> index d19501d913b3..5b2de9c3a1d6 100644
> --- a/drivers/interconnect/qcom/x1e80100.c
> +++ b/drivers/interconnect/qcom/x1e80100.c
> @@ -670,150 +670,6 @@ static struct qcom_icc_node xm_usb4_2 = {
>   	.links = { X1E80100_SLAVE_AGGRE_USB_SOUTH },
>   };
>   
> -static struct qcom_icc_node qnm_mnoc_hf_disp = {
> -	.name = "qnm_mnoc_hf_disp",
> -	.id = X1E80100_MASTER_MNOC_HF_MEM_NOC_DISP,
> -	.channels = 2,
> -	.buswidth = 32,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_LLCC_DISP },
> -};
> -
> -static struct qcom_icc_node qnm_pcie_disp = {
> -	.name = "qnm_pcie_disp",
> -	.id = X1E80100_MASTER_ANOC_PCIE_GEM_NOC_DISP,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_LLCC_DISP },
> -};
> -
> -static struct qcom_icc_node llcc_mc_disp = {
> -	.name = "llcc_mc_disp",
> -	.id = X1E80100_MASTER_LLCC_DISP,
> -	.channels = 8,
> -	.buswidth = 4,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_EBI1_DISP },
> -};
> -
> -static struct qcom_icc_node qnm_mdp_disp = {
> -	.name = "qnm_mdp_disp",
> -	.id = X1E80100_MASTER_MDP_DISP,
> -	.channels = 2,
> -	.buswidth = 32,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_MNOC_HF_MEM_NOC_DISP },
> -};
> -
> -static struct qcom_icc_node qnm_pcie_pcie = {
> -	.name = "qnm_pcie_pcie",
> -	.id = X1E80100_MASTER_ANOC_PCIE_GEM_NOC_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_LLCC_PCIE },
> -};
> -
> -static struct qcom_icc_node llcc_mc_pcie = {
> -	.name = "llcc_mc_pcie",
> -	.id = X1E80100_MASTER_LLCC_PCIE,
> -	.channels = 8,
> -	.buswidth = 4,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_EBI1_PCIE },
> -};
> -
> -static struct qcom_icc_node qnm_pcie_north_gem_noc_pcie = {
> -	.name = "qnm_pcie_north_gem_noc_pcie",
> -	.id = X1E80100_MASTER_PCIE_NORTH_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_ANOC_PCIE_GEM_NOC_PCIE },
> -};
> -
> -static struct qcom_icc_node qnm_pcie_south_gem_noc_pcie = {
> -	.name = "qnm_pcie_south_gem_noc_pcie",
> -	.id = X1E80100_MASTER_PCIE_SOUTH_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_ANOC_PCIE_GEM_NOC_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_3_pcie = {
> -	.name = "xm_pcie_3_pcie",
> -	.id = X1E80100_MASTER_PCIE_3_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_NORTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_4_pcie = {
> -	.name = "xm_pcie_4_pcie",
> -	.id = X1E80100_MASTER_PCIE_4_PCIE,
> -	.channels = 1,
> -	.buswidth = 8,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_NORTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_5_pcie = {
> -	.name = "xm_pcie_5_pcie",
> -	.id = X1E80100_MASTER_PCIE_5_PCIE,
> -	.channels = 1,
> -	.buswidth = 8,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_NORTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_0_pcie = {
> -	.name = "xm_pcie_0_pcie",
> -	.id = X1E80100_MASTER_PCIE_0_PCIE,
> -	.channels = 1,
> -	.buswidth = 16,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_SOUTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_1_pcie = {
> -	.name = "xm_pcie_1_pcie",
> -	.id = X1E80100_MASTER_PCIE_1_PCIE,
> -	.channels = 1,
> -	.buswidth = 16,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_SOUTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_2_pcie = {
> -	.name = "xm_pcie_2_pcie",
> -	.id = X1E80100_MASTER_PCIE_2_PCIE,
> -	.channels = 1,
> -	.buswidth = 16,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_SOUTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_6a_pcie = {
> -	.name = "xm_pcie_6a_pcie",
> -	.id = X1E80100_MASTER_PCIE_6A_PCIE,
> -	.channels = 1,
> -	.buswidth = 32,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_SOUTH_PCIE },
> -};
> -
> -static struct qcom_icc_node xm_pcie_6b_pcie = {
> -	.name = "xm_pcie_6b_pcie",
> -	.id = X1E80100_MASTER_PCIE_6B_PCIE,
> -	.channels = 1,
> -	.buswidth = 16,
> -	.num_links = 1,
> -	.links = { X1E80100_SLAVE_PCIE_SOUTH_PCIE },
> -};
> -
>   static struct qcom_icc_node qns_a1noc_snoc = {
>   	.name = "qns_a1noc_snoc",
>   	.id = X1E80100_SLAVE_A1NOC_SNOC,
> @@ -1514,76 +1370,6 @@ static struct qcom_icc_node qns_aggre_usb_south_snoc = {
>   	.links = { X1E80100_MASTER_AGGRE_USB_SOUTH },
>   };
>   
> -static struct qcom_icc_node qns_llcc_disp = {
> -	.name = "qns_llcc_disp",
> -	.id = X1E80100_SLAVE_LLCC_DISP,
> -	.channels = 8,
> -	.buswidth = 16,
> -	.num_links = 1,
> -	.links = { X1E80100_MASTER_LLCC_DISP },
> -};
> -
> -static struct qcom_icc_node ebi_disp = {
> -	.name = "ebi_disp",
> -	.id = X1E80100_SLAVE_EBI1_DISP,
> -	.channels = 8,
> -	.buswidth = 4,
> -	.num_links = 0,
> -};
> -
> -static struct qcom_icc_node qns_mem_noc_hf_disp = {
> -	.name = "qns_mem_noc_hf_disp",
> -	.id = X1E80100_SLAVE_MNOC_HF_MEM_NOC_DISP,
> -	.channels = 2,
> -	.buswidth = 32,
> -	.num_links = 1,
> -	.links = { X1E80100_MASTER_MNOC_HF_MEM_NOC_DISP },
> -};
> -
> -static struct qcom_icc_node qns_llcc_pcie = {
> -	.name = "qns_llcc_pcie",
> -	.id = X1E80100_SLAVE_LLCC_PCIE,
> -	.channels = 8,
> -	.buswidth = 16,
> -	.num_links = 1,
> -	.links = { X1E80100_MASTER_LLCC_PCIE },
> -};
> -
> -static struct qcom_icc_node ebi_pcie = {
> -	.name = "ebi_pcie",
> -	.id = X1E80100_SLAVE_EBI1_PCIE,
> -	.channels = 8,
> -	.buswidth = 4,
> -	.num_links = 0,
> -};
> -
> -static struct qcom_icc_node qns_pcie_mem_noc_pcie = {
> -	.name = "qns_pcie_mem_noc_pcie",
> -	.id = X1E80100_SLAVE_ANOC_PCIE_GEM_NOC_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_MASTER_ANOC_PCIE_GEM_NOC_PCIE },
> -};
> -
> -static struct qcom_icc_node qns_pcie_north_gem_noc_pcie = {
> -	.name = "qns_pcie_north_gem_noc_pcie",
> -	.id = X1E80100_SLAVE_PCIE_NORTH_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_MASTER_PCIE_NORTH_PCIE },
> -};
> -
> -static struct qcom_icc_node qns_pcie_south_gem_noc_pcie = {
> -	.name = "qns_pcie_south_gem_noc_pcie",
> -	.id = X1E80100_SLAVE_PCIE_SOUTH_PCIE,
> -	.channels = 1,
> -	.buswidth = 64,
> -	.num_links = 1,
> -	.links = { X1E80100_MASTER_PCIE_SOUTH_PCIE },
> -};
> -
>   static struct qcom_icc_bcm bcm_acv = {
>   	.name = "ACV",
>   	.num_nodes = 1,
> @@ -1755,72 +1541,6 @@ static struct qcom_icc_bcm bcm_sn4 = {
>   	.nodes = { &qnm_usb_anoc },
>   };
>   
> -static struct qcom_icc_bcm bcm_acv_disp = {
> -	.name = "ACV",
> -	.num_nodes = 1,
> -	.nodes = { &ebi_disp },
> -};
> -
> -static struct qcom_icc_bcm bcm_mc0_disp = {
> -	.name = "MC0",
> -	.num_nodes = 1,
> -	.nodes = { &ebi_disp },
> -};
> -
> -static struct qcom_icc_bcm bcm_mm0_disp = {
> -	.name = "MM0",
> -	.num_nodes = 1,
> -	.nodes = { &qns_mem_noc_hf_disp },
> -};
> -
> -static struct qcom_icc_bcm bcm_mm1_disp = {
> -	.name = "MM1",
> -	.num_nodes = 1,
> -	.nodes = { &qnm_mdp_disp },
> -};
> -
> -static struct qcom_icc_bcm bcm_sh0_disp = {
> -	.name = "SH0",
> -	.num_nodes = 1,
> -	.nodes = { &qns_llcc_disp },
> -};
> -
> -static struct qcom_icc_bcm bcm_sh1_disp = {
> -	.name = "SH1",
> -	.num_nodes = 2,
> -	.nodes = { &qnm_mnoc_hf_disp, &qnm_pcie_disp },
> -};
> -
> -static struct qcom_icc_bcm bcm_acv_pcie = {
> -	.name = "ACV",
> -	.num_nodes = 1,
> -	.nodes = { &ebi_pcie },
> -};
> -
> -static struct qcom_icc_bcm bcm_mc0_pcie = {
> -	.name = "MC0",
> -	.num_nodes = 1,
> -	.nodes = { &ebi_pcie },
> -};
> -
> -static struct qcom_icc_bcm bcm_pc0_pcie = {
> -	.name = "PC0",
> -	.num_nodes = 1,
> -	.nodes = { &qns_pcie_mem_noc_pcie },
> -};
> -
> -static struct qcom_icc_bcm bcm_sh0_pcie = {
> -	.name = "SH0",
> -	.num_nodes = 1,
> -	.nodes = { &qns_llcc_pcie },
> -};
> -
> -static struct qcom_icc_bcm bcm_sh1_pcie = {
> -	.name = "SH1",
> -	.num_nodes = 1,
> -	.nodes = { &qnm_pcie_pcie },
> -};
> -
>   static struct qcom_icc_bcm *aggre1_noc_bcms[] = {
>   };
>   
> @@ -1982,10 +1702,6 @@ static const struct qcom_icc_desc x1e80100_cnoc_main = {
>   static struct qcom_icc_bcm * const gem_noc_bcms[] = {
>   	&bcm_sh0,
>   	&bcm_sh1,
> -	&bcm_sh0_disp,
> -	&bcm_sh1_disp,
> -	&bcm_sh0_pcie,
> -	&bcm_sh1_pcie,
>   };
>   
>   static struct qcom_icc_node * const gem_noc_nodes[] = {
> @@ -2004,11 +1720,6 @@ static struct qcom_icc_node * const gem_noc_nodes[] = {
>   	[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_ANOC_PCIE_GEM_NOC_PCIE] = &qnm_pcie_pcie,
> -	[SLAVE_LLCC_PCIE] = &qns_llcc_pcie,
>   };
>   
>   static const struct qcom_icc_desc x1e80100_gem_noc = {
> @@ -2067,19 +1778,11 @@ static const struct qcom_icc_desc x1e80100_lpass_lpicx_noc = {
>   static struct qcom_icc_bcm * const mc_virt_bcms[] = {
>   	&bcm_acv,
>   	&bcm_mc0,
> -	&bcm_acv_disp,
> -	&bcm_mc0_disp,
> -	&bcm_acv_pcie,
> -	&bcm_mc0_pcie,
>   };
>   
>   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_PCIE] = &llcc_mc_pcie,
> -	[SLAVE_EBI1_PCIE] = &ebi_pcie,
>   };
>   
>   static const struct qcom_icc_desc x1e80100_mc_virt = {
> @@ -2092,8 +1795,6 @@ static const struct qcom_icc_desc x1e80100_mc_virt = {
>   static struct qcom_icc_bcm * const mmss_noc_bcms[] = {
>   	&bcm_mm0,
>   	&bcm_mm1,
> -	&bcm_mm0_disp,
> -	&bcm_mm1_disp,
>   };
>   
>   static struct qcom_icc_node * const mmss_noc_nodes[] = {
> @@ -2110,8 +1811,6 @@ static struct qcom_icc_node * const mmss_noc_nodes[] = {
>   	[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,
>   };
>   
>   static const struct qcom_icc_desc x1e80100_mmss_noc = {
> @@ -2139,16 +1838,12 @@ static const struct qcom_icc_desc x1e80100_nsp_noc = {
>   
>   static struct qcom_icc_bcm * const pcie_center_anoc_bcms[] = {
>   	&bcm_pc0,
> -	&bcm_pc0_pcie,
>   };
>   
>   static struct qcom_icc_node * const pcie_center_anoc_nodes[] = {
>   	[MASTER_PCIE_NORTH] = &qnm_pcie_north_gem_noc,
>   	[MASTER_PCIE_SOUTH] = &qnm_pcie_south_gem_noc,
>   	[SLAVE_ANOC_PCIE_GEM_NOC] = &qns_pcie_mem_noc,
> -	[MASTER_PCIE_NORTH_PCIE] = &qnm_pcie_north_gem_noc_pcie,
> -	[MASTER_PCIE_SOUTH_PCIE] = &qnm_pcie_south_gem_noc_pcie,
> -	[SLAVE_ANOC_PCIE_GEM_NOC_PCIE] = &qns_pcie_mem_noc_pcie,
>   };
>   
>   static const struct qcom_icc_desc x1e80100_pcie_center_anoc = {
> @@ -2166,10 +1861,6 @@ static struct qcom_icc_node * const pcie_north_anoc_nodes[] = {
>   	[MASTER_PCIE_4] = &xm_pcie_4,
>   	[MASTER_PCIE_5] = &xm_pcie_5,
>   	[SLAVE_PCIE_NORTH] = &qns_pcie_north_gem_noc,
> -	[MASTER_PCIE_3_PCIE] = &xm_pcie_3_pcie,
> -	[MASTER_PCIE_4_PCIE] = &xm_pcie_4_pcie,
> -	[MASTER_PCIE_5_PCIE] = &xm_pcie_5_pcie,
> -	[SLAVE_PCIE_NORTH_PCIE] = &qns_pcie_north_gem_noc_pcie,
>   };
>   
>   static const struct qcom_icc_desc x1e80100_pcie_north_anoc = {
> @@ -2189,12 +1880,6 @@ static struct qcom_icc_node * const pcie_south_anoc_nodes[] = {
>   	[MASTER_PCIE_6A] = &xm_pcie_6a,
>   	[MASTER_PCIE_6B] = &xm_pcie_6b,
>   	[SLAVE_PCIE_SOUTH] = &qns_pcie_south_gem_noc,
> -	[MASTER_PCIE_0_PCIE] = &xm_pcie_0_pcie,
> -	[MASTER_PCIE_1_PCIE] = &xm_pcie_1_pcie,
> -	[MASTER_PCIE_2_PCIE] = &xm_pcie_2_pcie,
> -	[MASTER_PCIE_6A_PCIE] = &xm_pcie_6a_pcie,
> -	[MASTER_PCIE_6B_PCIE] = &xm_pcie_6b_pcie,
> -	[SLAVE_PCIE_SOUTH_PCIE] = &qns_pcie_south_gem_noc_pcie,
>   };
>   
>   static const struct qcom_icc_desc x1e80100_pcie_south_anoc = {
>