The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] interconnect: qcom: Add interconnect provider driver for Nord SoC
       [not found] ` <20260420021351.1239355-3-shengchao.guo@oss.qualcomm.com>
@ 2026-05-08 20:41   ` Georgi Djakov
  2026-05-09  4:28     ` Shawn Guo
  0 siblings, 1 reply; 2+ messages in thread
From: Georgi Djakov @ 2026-05-08 20:41 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Baryshkov,
	Odelu Kukatla, Konrad Dybcio, Bartosz Golaszewski, Deepti Jaggi,
	linux-arm-msm, linux-pm, devicetree, linux-kernel

On 4/20/26 5:13 AM, Shawn Guo wrote:
> From: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> 
> Add driver for the Qualcomm interconnect buses found on Nord SoC.
> The topology consists of several NoCs that are controlled by
> a remote processor that collects the aggregated bandwidth for each
> master-slave pair.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
>   drivers/interconnect/qcom/Kconfig  |   11 +
>   drivers/interconnect/qcom/Makefile |    2 +
>   drivers/interconnect/qcom/nord.c   | 2682 ++++++++++++++++++++++++++++
>   3 files changed, 2695 insertions(+)
>   create mode 100644 drivers/interconnect/qcom/nord.c
> 
[..]
> +static struct qcom_icc_node * const aggre1_noc_nodes[] = {
> +	[MASTER_QSPI_0] = &qhm_qspi,
> +	[MASTER_SAILSS_MD1] = &qnm_sailss_md1,
> +	[MASTER_QUP_3] = &qxm_qup02,

Maybe this should be qxm_qup3 ?

> +	[SLAVE_A1NOC_SNOC] = &qns_a1noc_snoc,
> +};
[..]
> +static struct qcom_icc_node * const cnoc_cfg_nodes[] = {
> +	[MASTER_CNOC_CFG] = &qsm_cfg,
> +	[SLAVE_PS_ETH_0] = &ps_eth_0,
> +	[SLAVE_PS_ETH_1] = &ps_eth_1,
> +	[SLAVE_SHS_SERVER] = &ps_shs_server,
> +	[SLAVE_AHB2PHY_0] = &qhs_ahb2phy0,
> +	[SLAVE_AHB2PHY_1] = &qhs_ahb2phy1,
> +	[SLAVE_AHB2PHY_2] = &qhs_ahb2phy2,
> +	[SLAVE_AHB2PHY_3] = &qhs_ahb2phy3,
> +	[SLAVE_AHB2PHY_ETH_0] = &qhs_ahb2phy_eth_0,
> +	[SLAVE_AHB2PHY_ETH_1] = &qhs_ahb2phy_eth_1,
> +	[SLAVE_CAMERA_CFG] = &qhs_camera_cfg,
> +	[SLAVE_CLK_CTL] = &qhs_clk_ctl,
> +	[SLAVE_CRYPTO_0_CFG] = &qhs_crypto0_cfg,
> +	[SLAVE_CRYPTO_1_CFG] = &qhs_crypto1_cfg,
> +	[SLAVE_CRYPTO_2_CFG] = &qhs_crypto2_cfg,
> +	[SLAVE_DISPLAY_1_CFG] = &qhs_display_1_cfg,
> +	[SLAVE_DISPLAY_CFG] = &qhs_display_cfg,
> +	[SLAVE_DPRX0] = &qhs_dprx0,
> +	[SLAVE_DPRX1] = &qhs_dprx1,
> +	[SLAVE_EVA_CFG] = &qhs_eva_cfg,
> +	[SLAVE_GFX3D_CFG] = &qhs_gpuss_0_cfg,
> +	[SLAVE_GFX3D_1_CFG] = &qhs_gpuss_1_cfg,
> +	[SLAVE_I2C] = &qhs_i2c,
> +	[SLAVE_IMEM_CFG] = &qhs_imem_cfg,
> +	[SLAVE_MCW_PCIE] = &qhs_mcw_pcie,
> +	[SLAVE_MM_RSCC] = &qhs_mm_rscc,
> +	[SLAVE_NE_CLK_CTL] = &qhs_ne_clk_ctl,
> +	[SLAVE_NSPSS0_CFG] = &qhs_nspss0_cfg,
> +	[SLAVE_NSPSS1_CFG] = &qhs_nspss1_cfg,
> +	[SLAVE_NSPSS2_CFG] = &qhs_nspss2_cfg,
> +	[SLAVE_NSPSS3_CFG] = &qhs_nspss3_cfg,
> +	[SLAVE_NW_CLK_CTL] = &qhs_nw_clk_ctl,
> +	[SLAVE_PRNG] = &qhs_prng,
> +	[SLAVE_QDSS_CFG] = &qhs_qdss_cfg,
> +	[SLAVE_QSPI_0] = &qhs_qspi,
> +	[SLAVE_QUP_0] = &qhs_qup0,
> +	[SLAVE_QUP_3] = &qhs_qup02,

qhs_qup3 maybe?

> +	[SLAVE_QUP_1] = &qhs_qup1,
> +	[SLAVE_QUP_2] = &qhs_qup2,
> +	[SLAVE_SAFEDMA_CFG] = &qhs_safedma_cfg,
> +	[SLAVE_SDCC_4] = &qhs_sdc4,
> +	[SLAVE_SE_CLK_CTL] = &qhs_se_clk_ctl,
> +	[SLAVE_TCSR] = &qhs_tcsr,
> +	[SLAVE_TLMM] = &qhs_tlmm,
> +	[SLAVE_TSC_CFG] = &qhs_tsc_cfg,
> +	[SLAVE_UFS_MEM_CFG] = &qhs_ufs_mem_cfg,
> +	[SLAVE_USB2] = &qhs_usb2,
> +	[SLAVE_USB3_0] = &qhs_usb3_0,
> +	[SLAVE_USB3_1] = &qhs_usb3_1,
> +	[SLAVE_VENUS_CFG] = &qhs_venus_cfg,
> +	[SLAVE_COMPUTENOC_CFG] = &qss_computenoc_cfg,
> +	[SLAVE_PCIE_NOC_CFG] = &qss_pcie_noc_cfg,
> +	[SLAVE_QTC_CFG] = &qss_qtc_cfg,
> +	[SLAVE_QDSS_STM] = &xs_qdss_stm,
> +	[SLAVE_SYS_TCU0_CFG] = &xs_sys_tcu0_cfg,
> +	[SLAVE_SYS_TCU1_CFG] = &xs_sys_tcu1_cfg,
> +	[SLAVE_SYS_TCU2_CFG] = &xs_sys_tcu2_cfg,
> +};
[..]
> +static const struct regmap_config nord_hscnoc_regmap_config = {
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.max_register = 0x45080,

Is this value correct? The qosbox offsets for some nodes go beyond this.

> +	.fast_io = true,
> +};
> +

The rest looks good!

Thanks,
Georgi

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH 2/2] interconnect: qcom: Add interconnect provider driver for Nord SoC
  2026-05-08 20:41   ` [PATCH 2/2] interconnect: qcom: Add interconnect provider driver for Nord SoC Georgi Djakov
@ 2026-05-09  4:28     ` Shawn Guo
  0 siblings, 0 replies; 2+ messages in thread
From: Shawn Guo @ 2026-05-09  4:28 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Baryshkov,
	Odelu Kukatla, Konrad Dybcio, Bartosz Golaszewski, Deepti Jaggi,
	linux-arm-msm, linux-pm, devicetree, linux-kernel

On Fri, May 08, 2026 at 11:41:01PM +0300, Georgi Djakov wrote:
> On 4/20/26 5:13 AM, Shawn Guo wrote:
> > From: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> > 
> > Add driver for the Qualcomm interconnect buses found on Nord SoC.
> > The topology consists of several NoCs that are controlled by
> > a remote processor that collects the aggregated bandwidth for each
> > master-slave pair.
> > 
> > Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > ---
> >   drivers/interconnect/qcom/Kconfig  |   11 +
> >   drivers/interconnect/qcom/Makefile |    2 +
> >   drivers/interconnect/qcom/nord.c   | 2682 ++++++++++++++++++++++++++++
> >   3 files changed, 2695 insertions(+)
> >   create mode 100644 drivers/interconnect/qcom/nord.c
> > 
> [..]
> > +static struct qcom_icc_node * const aggre1_noc_nodes[] = {
> > +	[MASTER_QSPI_0] = &qhm_qspi,
> > +	[MASTER_SAILSS_MD1] = &qnm_sailss_md1,
> > +	[MASTER_QUP_3] = &qxm_qup02,
> 
> Maybe this should be qxm_qup3 ?

Yes, it makes more sense considering QUP_0/1/2 naming.

  [MASTER_QUP_0] = &qhm_qup0,
  [MASTER_QUP_1] = &qhm_qup1,
  [MASTER_QUP_2] = &qhm_qup2,

> 
> > +	[SLAVE_A1NOC_SNOC] = &qns_a1noc_snoc,
> > +};
> [..]
> > +static struct qcom_icc_node * const cnoc_cfg_nodes[] = {
> > +	[MASTER_CNOC_CFG] = &qsm_cfg,
> > +	[SLAVE_PS_ETH_0] = &ps_eth_0,
> > +	[SLAVE_PS_ETH_1] = &ps_eth_1,
> > +	[SLAVE_SHS_SERVER] = &ps_shs_server,
> > +	[SLAVE_AHB2PHY_0] = &qhs_ahb2phy0,
> > +	[SLAVE_AHB2PHY_1] = &qhs_ahb2phy1,
> > +	[SLAVE_AHB2PHY_2] = &qhs_ahb2phy2,
> > +	[SLAVE_AHB2PHY_3] = &qhs_ahb2phy3,
> > +	[SLAVE_AHB2PHY_ETH_0] = &qhs_ahb2phy_eth_0,
> > +	[SLAVE_AHB2PHY_ETH_1] = &qhs_ahb2phy_eth_1,
> > +	[SLAVE_CAMERA_CFG] = &qhs_camera_cfg,
> > +	[SLAVE_CLK_CTL] = &qhs_clk_ctl,
> > +	[SLAVE_CRYPTO_0_CFG] = &qhs_crypto0_cfg,
> > +	[SLAVE_CRYPTO_1_CFG] = &qhs_crypto1_cfg,
> > +	[SLAVE_CRYPTO_2_CFG] = &qhs_crypto2_cfg,
> > +	[SLAVE_DISPLAY_1_CFG] = &qhs_display_1_cfg,
> > +	[SLAVE_DISPLAY_CFG] = &qhs_display_cfg,
> > +	[SLAVE_DPRX0] = &qhs_dprx0,
> > +	[SLAVE_DPRX1] = &qhs_dprx1,
> > +	[SLAVE_EVA_CFG] = &qhs_eva_cfg,
> > +	[SLAVE_GFX3D_CFG] = &qhs_gpuss_0_cfg,
> > +	[SLAVE_GFX3D_1_CFG] = &qhs_gpuss_1_cfg,
> > +	[SLAVE_I2C] = &qhs_i2c,
> > +	[SLAVE_IMEM_CFG] = &qhs_imem_cfg,
> > +	[SLAVE_MCW_PCIE] = &qhs_mcw_pcie,
> > +	[SLAVE_MM_RSCC] = &qhs_mm_rscc,
> > +	[SLAVE_NE_CLK_CTL] = &qhs_ne_clk_ctl,
> > +	[SLAVE_NSPSS0_CFG] = &qhs_nspss0_cfg,
> > +	[SLAVE_NSPSS1_CFG] = &qhs_nspss1_cfg,
> > +	[SLAVE_NSPSS2_CFG] = &qhs_nspss2_cfg,
> > +	[SLAVE_NSPSS3_CFG] = &qhs_nspss3_cfg,
> > +	[SLAVE_NW_CLK_CTL] = &qhs_nw_clk_ctl,
> > +	[SLAVE_PRNG] = &qhs_prng,
> > +	[SLAVE_QDSS_CFG] = &qhs_qdss_cfg,
> > +	[SLAVE_QSPI_0] = &qhs_qspi,
> > +	[SLAVE_QUP_0] = &qhs_qup0,
> > +	[SLAVE_QUP_3] = &qhs_qup02,
> 
> qhs_qup3 maybe?

Indeed!

> > +	[SLAVE_QUP_1] = &qhs_qup1,
> > +	[SLAVE_QUP_2] = &qhs_qup2,
> > +	[SLAVE_SAFEDMA_CFG] = &qhs_safedma_cfg,
> > +	[SLAVE_SDCC_4] = &qhs_sdc4,
> > +	[SLAVE_SE_CLK_CTL] = &qhs_se_clk_ctl,
> > +	[SLAVE_TCSR] = &qhs_tcsr,
> > +	[SLAVE_TLMM] = &qhs_tlmm,
> > +	[SLAVE_TSC_CFG] = &qhs_tsc_cfg,
> > +	[SLAVE_UFS_MEM_CFG] = &qhs_ufs_mem_cfg,
> > +	[SLAVE_USB2] = &qhs_usb2,
> > +	[SLAVE_USB3_0] = &qhs_usb3_0,
> > +	[SLAVE_USB3_1] = &qhs_usb3_1,
> > +	[SLAVE_VENUS_CFG] = &qhs_venus_cfg,
> > +	[SLAVE_COMPUTENOC_CFG] = &qss_computenoc_cfg,
> > +	[SLAVE_PCIE_NOC_CFG] = &qss_pcie_noc_cfg,
> > +	[SLAVE_QTC_CFG] = &qss_qtc_cfg,
> > +	[SLAVE_QDSS_STM] = &xs_qdss_stm,
> > +	[SLAVE_SYS_TCU0_CFG] = &xs_sys_tcu0_cfg,
> > +	[SLAVE_SYS_TCU1_CFG] = &xs_sys_tcu1_cfg,
> > +	[SLAVE_SYS_TCU2_CFG] = &xs_sys_tcu2_cfg,
> > +};
> [..]
> > +static const struct regmap_config nord_hscnoc_regmap_config = {
> > +	.reg_bits = 32,
> > +	.reg_stride = 4,
> > +	.val_bits = 32,
> > +	.max_register = 0x45080,
> 
> Is this value correct? The qosbox offsets for some nodes go beyond this.

The HSCNOC max port_offset is 0xa45000, so the correct max_register
should be 0xa45080. I appreciate that you spot this error!

> > +	.fast_io = true,
> > +};
> > +
> 
> The rest looks good!

Thank you for the review, Georgi!

Shawn

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-09  4:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260420021351.1239355-1-shengchao.guo@oss.qualcomm.com>
     [not found] ` <20260420021351.1239355-3-shengchao.guo@oss.qualcomm.com>
2026-05-08 20:41   ` [PATCH 2/2] interconnect: qcom: Add interconnect provider driver for Nord SoC Georgi Djakov
2026-05-09  4:28     ` Shawn Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox