| Message ID | 20240328075936.223461-5-quic_varada@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add interconnect driver for IPQ9574 SoC | expand |
On Thu, Mar 28, 2024 at 02:51:09PM -0700, Stephen Boyd wrote: > Quoting Varadarajan Narayanan (2024-03-28 00:59:35) > > diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c > > index 0a3f846695b8..187fd9dcdf49 100644 > > --- a/drivers/clk/qcom/gcc-ipq9574.c > > +++ b/drivers/clk/qcom/gcc-ipq9574.c > > @@ -4301,6 +4302,56 @@ static const struct qcom_reset_map gcc_ipq9574_resets[] = { > > [GCC_WCSS_Q6_TBU_BCR] = { 0x12054, 0 }, > > }; > > > > +#define IPQ_APPS_ID 9574 /* some unique value */ > > How is this supposed to stay unique? The icc-clk driver expects some number that wouldn't interfere with the existing nodes. So just used the SoC id itself. > I don't understand icc_node_create() API quite honestly. Why > can't icc_clk_register() maintain some ida of allocated > numbers? Or is there some global number space that we can > "reserve" from? I'm quite amazed this is how things are > connected in interconnect framework. > > > + > > +enum { > > + ICC_ANOC_PCIE0, > > + ICC_SNOC_PCIE0, > > + ICC_ANOC_PCIE1, > > + ICC_SNOC_PCIE1, > > + ICC_ANOC_PCIE2, > > + ICC_SNOC_PCIE2, > > + ICC_ANOC_PCIE3, > > + ICC_SNOC_PCIE3, > > + ICC_SNOC_USB, > > + ICC_ANOC_USB_AXI, > > + ICC_NSSNOC_NSSCC, > > + ICC_NSSNOC_SNOC_0, > > + ICC_NSSNOC_SNOC_1, > > + ICC_NSSNOC_PCNOC_1, > > + ICC_NSSNOC_QOSGEN_REF, > > + ICC_NSSNOC_TIMEOUT_REF, > > + ICC_NSSNOC_XO_DCD, > > + ICC_NSSNOC_ATB, > > + ICC_MEM_NOC_NSSNOC, > > + ICC_NSSNOC_MEMNOC, > > + ICC_NSSNOC_MEM_NOC_1, > > +}; > > Are these supposed to be in a dt-binding header? Since these don't directly relate to the ids in the dt-bindings not sure if they will be permitted there. Will move and post a new version and get feedback. Thanks Varada > > + > > +static struct clk_hw *icc_ipq9574_hws[] = { > > + [ICC_ANOC_PCIE0] = &gcc_anoc_pcie0_1lane_m_clk.clkr.hw, > > + [ICC_SNOC_PCIE0] = &gcc_anoc_pcie1_1lane_m_clk.clkr.hw, > > + [ICC_ANOC_PCIE1] = &gcc_anoc_pcie2_2lane_m_clk.clkr.hw, > > + [ICC_SNOC_PCIE1] = &gcc_anoc_pcie3_2lane_m_clk.clkr.hw, > > + [ICC_ANOC_PCIE2] = &gcc_snoc_pcie0_1lane_s_clk.clkr.hw, > > + [ICC_SNOC_PCIE2] = &gcc_snoc_pcie1_1lane_s_clk.clkr.hw,
On 29/03/2024 11:55, Varadarajan Narayanan wrote: >>> + >>> +enum { >>> + ICC_ANOC_PCIE0, >>> + ICC_SNOC_PCIE0, >>> + ICC_ANOC_PCIE1, >>> + ICC_SNOC_PCIE1, >>> + ICC_ANOC_PCIE2, >>> + ICC_SNOC_PCIE2, >>> + ICC_ANOC_PCIE3, >>> + ICC_SNOC_PCIE3, >>> + ICC_SNOC_USB, >>> + ICC_ANOC_USB_AXI, >>> + ICC_NSSNOC_NSSCC, >>> + ICC_NSSNOC_SNOC_0, >>> + ICC_NSSNOC_SNOC_1, >>> + ICC_NSSNOC_PCNOC_1, >>> + ICC_NSSNOC_QOSGEN_REF, >>> + ICC_NSSNOC_TIMEOUT_REF, >>> + ICC_NSSNOC_XO_DCD, >>> + ICC_NSSNOC_ATB, >>> + ICC_MEM_NOC_NSSNOC, >>> + ICC_NSSNOC_MEMNOC, >>> + ICC_NSSNOC_MEM_NOC_1, >>> +}; >> >> Are these supposed to be in a dt-binding header? > > Since these don't directly relate to the ids in the dt-bindings > not sure if they will be permitted there. Will move and post a > new version and get feedback. You can answer this by yourself by looking at your DTS. Do you use them as well in the DTS? It's a pity we see here only parts of DTS, instead of full interconnect usage. Best regards, Krzysztof
On Fri, Mar 29, 2024 at 01:10:03PM +0100, Krzysztof Kozlowski wrote: > On 29/03/2024 11:55, Varadarajan Narayanan wrote: > >>> + > >>> +enum { > >>> + ICC_ANOC_PCIE0, > >>> + ICC_SNOC_PCIE0, > >>> + ICC_ANOC_PCIE1, > >>> + ICC_SNOC_PCIE1, > >>> + ICC_ANOC_PCIE2, > >>> + ICC_SNOC_PCIE2, > >>> + ICC_ANOC_PCIE3, > >>> + ICC_SNOC_PCIE3, > >>> + ICC_SNOC_USB, > >>> + ICC_ANOC_USB_AXI, > >>> + ICC_NSSNOC_NSSCC, > >>> + ICC_NSSNOC_SNOC_0, > >>> + ICC_NSSNOC_SNOC_1, > >>> + ICC_NSSNOC_PCNOC_1, > >>> + ICC_NSSNOC_QOSGEN_REF, > >>> + ICC_NSSNOC_TIMEOUT_REF, > >>> + ICC_NSSNOC_XO_DCD, > >>> + ICC_NSSNOC_ATB, > >>> + ICC_MEM_NOC_NSSNOC, > >>> + ICC_NSSNOC_MEMNOC, > >>> + ICC_NSSNOC_MEM_NOC_1, > >>> +}; > >> > >> Are these supposed to be in a dt-binding header? > > > > Since these don't directly relate to the ids in the dt-bindings > > not sure if they will be permitted there. Will move and post a > > new version and get feedback. > > You can answer this by yourself by looking at your DTS. Do you use them > as well in the DTS? I can use them in the DTS. The icc-clk framework automatically creates master and slave nodes as 'n' and 'n+1'. Hence I can have something like this in the dt-bindings include file #define ICC_ANOC_PCIE0 0 #define ICC_SNOC_PCIE0 1 . . . #define ICC_NSSNOC_MEM_NOC_1 20 #define MASTER(x) ((ICC_ ## x) * 2) #define SLAVE(x) (MASTER(x) + 1) > It's a pity we see here only parts of DTS, instead of full interconnect > usage. Unfortunately cannot include the pcie dts changes with this patch, but you can refer to them at https://lore.kernel.org/linux-arm-msm/20230519090219.15925-5-quic_devipriy@quicinc.com/ The above macros will be used in the pcie node as follows pcie0: pci@28000000 { compatible = "qcom,pcie-ipq9574"; . . . interconnects = <&gcc MASTER(ANOC_PCIE0) &gcc SLAVE(ANOC_PCIE0)>, <&gcc MASTER(SNOC_PCIE0) &gcc SLAVE(SNOC_PCIE0)>; interconnect-names = "pcie-mem", "cpu-pcie"; . . . }; Hope this is acceptable. Thanks Varada
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig index 8ab08e7b5b6c..af73a0b396eb 100644 --- a/drivers/clk/qcom/Kconfig +++ b/drivers/clk/qcom/Kconfig @@ -243,6 +243,8 @@ config IPQ_GCC_8074 config IPQ_GCC_9574 tristate "IPQ9574 Global Clock Controller" + select INTERCONNECT + select INTERCONNECT_CLK help Support for global clock controller on ipq9574 devices. Say Y if you want to use peripheral devices such as UART, SPI, diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c index 0a3f846695b8..187fd9dcdf49 100644 --- a/drivers/clk/qcom/gcc-ipq9574.c +++ b/drivers/clk/qcom/gcc-ipq9574.c @@ -12,6 +12,7 @@ #include <dt-bindings/clock/qcom,ipq9574-gcc.h> #include <dt-bindings/reset/qcom,ipq9574-gcc.h> +#include <dt-bindings/interconnect/qcom,ipq9574.h> #include "clk-alpha-pll.h" #include "clk-branch.h" @@ -4301,6 +4302,56 @@ static const struct qcom_reset_map gcc_ipq9574_resets[] = { [GCC_WCSS_Q6_TBU_BCR] = { 0x12054, 0 }, }; +#define IPQ_APPS_ID 9574 /* some unique value */ + +enum { + ICC_ANOC_PCIE0, + ICC_SNOC_PCIE0, + ICC_ANOC_PCIE1, + ICC_SNOC_PCIE1, + ICC_ANOC_PCIE2, + ICC_SNOC_PCIE2, + ICC_ANOC_PCIE3, + ICC_SNOC_PCIE3, + ICC_SNOC_USB, + ICC_ANOC_USB_AXI, + ICC_NSSNOC_NSSCC, + ICC_NSSNOC_SNOC_0, + ICC_NSSNOC_SNOC_1, + ICC_NSSNOC_PCNOC_1, + ICC_NSSNOC_QOSGEN_REF, + ICC_NSSNOC_TIMEOUT_REF, + ICC_NSSNOC_XO_DCD, + ICC_NSSNOC_ATB, + ICC_MEM_NOC_NSSNOC, + ICC_NSSNOC_MEMNOC, + ICC_NSSNOC_MEM_NOC_1, +}; + +static struct clk_hw *icc_ipq9574_hws[] = { + [ICC_ANOC_PCIE0] = &gcc_anoc_pcie0_1lane_m_clk.clkr.hw, + [ICC_SNOC_PCIE0] = &gcc_anoc_pcie1_1lane_m_clk.clkr.hw, + [ICC_ANOC_PCIE1] = &gcc_anoc_pcie2_2lane_m_clk.clkr.hw, + [ICC_SNOC_PCIE1] = &gcc_anoc_pcie3_2lane_m_clk.clkr.hw, + [ICC_ANOC_PCIE2] = &gcc_snoc_pcie0_1lane_s_clk.clkr.hw, + [ICC_SNOC_PCIE2] = &gcc_snoc_pcie1_1lane_s_clk.clkr.hw, + [ICC_ANOC_PCIE3] = &gcc_snoc_pcie2_2lane_s_clk.clkr.hw, + [ICC_SNOC_PCIE3] = &gcc_snoc_pcie3_2lane_s_clk.clkr.hw, + [ICC_SNOC_USB] = &gcc_snoc_usb_clk.clkr.hw, + [ICC_ANOC_USB_AXI] = &gcc_anoc_usb_axi_clk.clkr.hw, + [ICC_NSSNOC_NSSCC] = &gcc_nssnoc_nsscc_clk.clkr.hw, + [ICC_NSSNOC_SNOC_0] = &gcc_nssnoc_snoc_clk.clkr.hw, + [ICC_NSSNOC_SNOC_1] = &gcc_nssnoc_snoc_1_clk.clkr.hw, + [ICC_NSSNOC_PCNOC_1] = &gcc_nssnoc_pcnoc_1_clk.clkr.hw, + [ICC_NSSNOC_QOSGEN_REF] = &gcc_nssnoc_qosgen_ref_clk.clkr.hw, + [ICC_NSSNOC_TIMEOUT_REF] = &gcc_nssnoc_timeout_ref_clk.clkr.hw, + [ICC_NSSNOC_XO_DCD] = &gcc_nssnoc_xo_dcd_clk.clkr.hw, + [ICC_NSSNOC_ATB] = &gcc_nssnoc_atb_clk.clkr.hw, + [ICC_MEM_NOC_NSSNOC] = &gcc_mem_noc_nssnoc_clk.clkr.hw, + [ICC_NSSNOC_MEMNOC] = &gcc_nssnoc_memnoc_clk.clkr.hw, + [ICC_NSSNOC_MEM_NOC_1] = &gcc_nssnoc_mem_noc_1_clk.clkr.hw, +}; + static const struct of_device_id gcc_ipq9574_match_table[] = { { .compatible = "qcom,ipq9574-gcc" }, { } @@ -4323,6 +4374,9 @@ static const struct qcom_cc_desc gcc_ipq9574_desc = { .num_resets = ARRAY_SIZE(gcc_ipq9574_resets), .clk_hws = gcc_ipq9574_hws, .num_clk_hws = ARRAY_SIZE(gcc_ipq9574_hws), + .icc_hws = icc_ipq9574_hws, + .num_icc_hws = ARRAY_SIZE(icc_ipq9574_hws), + .first_id = IPQ_APPS_ID, }; static int gcc_ipq9574_probe(struct platform_device *pdev)
Use the icc-clk framework to enable few clocks to be able to create paths and use the peripherals connected on those NoCs. Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> --- v5: Split from common.c changes into separate patch No functional changes --- drivers/clk/qcom/Kconfig | 2 ++ drivers/clk/qcom/gcc-ipq9574.c | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+)