mbox series

[v1,0/3] Add interconnect driver for IPQ5332 SoC

Message ID 20240709063949.4127310-1-quic_varada@quicinc.com
Headers show
Series Add interconnect driver for IPQ5332 SoC | expand

Message

Varadarajan Narayanan July 9, 2024, 6:39 a.m. UTC 
Enable icc-clk based interconnect driver for IPQ5332. This is
similar to IPQ9574's icc-clk based driver.

dt_bindings_check and dtbs_check passed.

Ensured that icc_sync_state is called and relevant clocks are
disabled.

Dependency:
[1] https://lore.kernel.org/linux-arm-msm/20240430064214.2030013-1-quic_varada@quicinc.com/

Varadarajan Narayanan (3):
  dt-bindings: interconnect: Add Qualcomm IPQ5332 support
  clk: qcom: ipq5332: Use icc-clk for enabling NoC related clocks
  arm64: dts: qcom: ipq5332: Add icc provider ability to gcc

 .../bindings/clock/qcom,ipq5332-gcc.yaml      |  2 +
 arch/arm64/boot/dts/qcom/ipq5332.dtsi         |  2 +
 drivers/clk/qcom/gcc-ipq5332.c                | 36 +++++++++++++--
 .../dt-bindings/interconnect/qcom,ipq5332.h   | 46 +++++++++++++++++++
 4 files changed, 81 insertions(+), 5 deletions(-)
 create mode 100644 include/dt-bindings/interconnect/qcom,ipq5332.h

Comments

Konrad Dybcio July 9, 2024, 9:53 a.m. UTC | #1 
On 9.07.2024 8:39 AM, Varadarajan Narayanan wrote:
> IPQ SoCs dont involve RPM in managing NoC related clocks and
> there is no NoC scaling. Linux itself handles these clocks.
> However, these should not be exposed as just clocks and align
> with other Qualcomm SoCs that handle these clocks from a
> interconnect provider.
> 
> Hence include icc provider capability to the gcc node so that
> peripherals can use the interconnect facility to enable these
> clocks.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---

Doesn't the USB host need to have its path described to keep working?

Konrad
Varadarajan Narayanan July 10, 2024, 10:42 a.m. UTC | #2 
On Tue, Jul 09, 2024 at 11:53:41AM +0200, Konrad Dybcio wrote:
> On 9.07.2024 8:39 AM, Varadarajan Narayanan wrote:
> > IPQ SoCs dont involve RPM in managing NoC related clocks and
> > there is no NoC scaling. Linux itself handles these clocks.
> > However, these should not be exposed as just clocks and align
> > with other Qualcomm SoCs that handle these clocks from a
> > interconnect provider.
> >
> > Hence include icc provider capability to the gcc node so that
> > peripherals can use the interconnect facility to enable these
> > clocks.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
>
> Doesn't the USB host need to have its path described to keep working?

Presently, USB host enables GCC_SNOC_USB_CLK directly using
the clocks/clock-name entries. So it is not dependent on ICC.

Shall I update the USB DT node to use interconnects now itself,
or wait until this IPQ5332 ICC enablement series is approved?
Please let me know.

Thanks
Varada
Konrad Dybcio July 10, 2024, 10:49 a.m. UTC | #3 
On 10.07.2024 12:42 PM, Varadarajan Narayanan wrote:
> On Tue, Jul 09, 2024 at 11:53:41AM +0200, Konrad Dybcio wrote:
>> On 9.07.2024 8:39 AM, Varadarajan Narayanan wrote:
>>> IPQ SoCs dont involve RPM in managing NoC related clocks and
>>> there is no NoC scaling. Linux itself handles these clocks.
>>> However, these should not be exposed as just clocks and align
>>> with other Qualcomm SoCs that handle these clocks from a
>>> interconnect provider.
>>>
>>> Hence include icc provider capability to the gcc node so that
>>> peripherals can use the interconnect facility to enable these
>>> clocks.
>>>
>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>>> ---
>>
>> Doesn't the USB host need to have its path described to keep working?
> 
> Presently, USB host enables GCC_SNOC_USB_CLK directly using
> the clocks/clock-name entries. So it is not dependent on ICC.
> 
> Shall I update the USB DT node to use interconnects now itself,
> or wait until this IPQ5332 ICC enablement series is approved?
> Please let me know.

Definitely so. Now that you registered that clock with the
interconnect framework, the current usage is essentially
circumventing it..

Say some consumers casted an ICC vote on that node, and then
the USB driver called set_rate on the clock.. The data from
icc-clk would be discarded

Konrad
Varadarajan Narayanan July 10, 2024, 11:12 a.m. UTC | #4 
On Wed, Jul 10, 2024 at 12:49:13PM +0200, Konrad Dybcio wrote:
> On 10.07.2024 12:42 PM, Varadarajan Narayanan wrote:
> > On Tue, Jul 09, 2024 at 11:53:41AM +0200, Konrad Dybcio wrote:
> >> On 9.07.2024 8:39 AM, Varadarajan Narayanan wrote:
> >>> IPQ SoCs dont involve RPM in managing NoC related clocks and
> >>> there is no NoC scaling. Linux itself handles these clocks.
> >>> However, these should not be exposed as just clocks and align
> >>> with other Qualcomm SoCs that handle these clocks from a
> >>> interconnect provider.
> >>>
> >>> Hence include icc provider capability to the gcc node so that
> >>> peripherals can use the interconnect facility to enable these
> >>> clocks.
> >>>
> >>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >>> ---
> >>
> >> Doesn't the USB host need to have its path described to keep working?
> >
> > Presently, USB host enables GCC_SNOC_USB_CLK directly using
> > the clocks/clock-name entries. So it is not dependent on ICC.
> >
> > Shall I update the USB DT node to use interconnects now itself,
> > or wait until this IPQ5332 ICC enablement series is approved?
> > Please let me know.
>
> Definitely so. Now that you registered that clock with the
> interconnect framework, the current usage is essentially
> circumventing it..
>
> Say some consumers casted an ICC vote on that node, and then
> the USB driver called set_rate on the clock.. The data from
> icc-clk would be discarded

Will update, test and post a new version.

Thanks
Varada
Dmitry Baryshkov July 13, 2024, 3:12 p.m. UTC | #5 
On Tue, Jul 09, 2024 at 12:09:47PM GMT, Varadarajan Narayanan wrote:
> 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>
> ---
>  drivers/clk/qcom/gcc-ipq5332.c | 36 +++++++++++++++++++++++++++++-----
>  1 file changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c
> index f98591148a97..6d7672cae0f7 100644
> --- a/drivers/clk/qcom/gcc-ipq5332.c
> +++ b/drivers/clk/qcom/gcc-ipq5332.c
> @@ -4,12 +4,14 @@
>   */
>  
>  #include <linux/clk-provider.h>
> +#include <linux/interconnect-provider.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  
>  #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +#include <dt-bindings/interconnect/qcom,ipq5332.h>
>  
>  #include "clk-alpha-pll.h"
>  #include "clk-branch.h"
> @@ -131,12 +133,14 @@ static struct clk_alpha_pll gpll4_main = {
>  			 * (will be added soon), so the clock framework
>  			 * disables this source. But some of the clocks
>  			 * initialized by boot loaders uses this source. So we
> -			 * need to keep this clock ON. Add the
> -			 * CLK_IGNORE_UNUSED flag so the clock will not be
> -			 * disabled. Once the consumer in kernel is added, we
> -			 * can get rid of this flag.
> +			 * need to keep this clock ON.
> +			 *
> +			 * After initial bootup, when the ICC framework turns
> +			 * off unused paths, as part of the icc-clk dependencies
> +			 * this clock gets disabled resulting in a hang. Marking
> +			 * it as critical to ensure it is not turned off.

Are there any other users for this clocks? What exactly causes the hang?

>  			 */
> -			.flags = CLK_IGNORE_UNUSED,
> +			.flags = CLK_IS_CRITICAL,
>  		},
>  	},
>  };
> @@ -3628,6 +3632,24 @@ static const struct qcom_reset_map gcc_ipq5332_resets[] = {
>  	[GCC_UNIPHY1_XPCS_ARES] = { 0x16060 },
>  };
>  
> +#define IPQ_APPS_ID			5332	/* some unique value */
> +
> +static struct qcom_icc_hws_data icc_ipq5332_hws[] = {
> +	{ MASTER_SNOC_PCIE3_1_M, SLAVE_SNOC_PCIE3_1_M, GCC_SNOC_PCIE3_1LANE_M_CLK },
> +	{ MASTER_ANOC_PCIE3_1_S, SLAVE_ANOC_PCIE3_1_S, GCC_SNOC_PCIE3_1LANE_S_CLK },
> +	{ MASTER_SNOC_PCIE3_2_M, SLAVE_SNOC_PCIE3_2_M, GCC_SNOC_PCIE3_2LANE_M_CLK },
> +	{ MASTER_ANOC_PCIE3_2_S, SLAVE_ANOC_PCIE3_2_S, GCC_SNOC_PCIE3_2LANE_S_CLK },
> +	{ MASTER_SNOC_USB, SLAVE_SNOC_USB, GCC_SNOC_USB_CLK },
> +	{ MASTER_NSSNOC_NSSCC, SLAVE_NSSNOC_NSSCC, GCC_NSSNOC_NSSCC_CLK },
> +	{ MASTER_NSSNOC_SNOC_0, SLAVE_NSSNOC_SNOC_0, GCC_NSSNOC_SNOC_CLK },
> +	{ MASTER_NSSNOC_SNOC_1, SLAVE_NSSNOC_SNOC_1, GCC_NSSNOC_SNOC_1_CLK },
> +	{ MASTER_NSSNOC_ATB, SLAVE_NSSNOC_ATB, GCC_NSSNOC_ATB_CLK },
> +	{ MASTER_NSSNOC_PCNOC_1, SLAVE_NSSNOC_PCNOC_1, GCC_NSSNOC_PCNOC_1_CLK },
> +	{ MASTER_NSSNOC_QOSGEN_REF, SLAVE_NSSNOC_QOSGEN_REF, GCC_NSSNOC_QOSGEN_REF_CLK },
> +	{ MASTER_NSSNOC_TIMEOUT_REF, SLAVE_NSSNOC_TIMEOUT_REF, GCC_NSSNOC_TIMEOUT_REF_CLK },
> +	{ MASTER_NSSNOC_XO_DCD, SLAVE_NSSNOC_XO_DCD, GCC_NSSNOC_XO_DCD_CLK },
> +};
> +
>  static const struct regmap_config gcc_ipq5332_regmap_config = {
>  	.reg_bits = 32,
>  	.reg_stride = 4,
> @@ -3656,6 +3678,9 @@ static const struct qcom_cc_desc gcc_ipq5332_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_ipq5332_resets),
>  	.clk_hws = gcc_ipq5332_hws,
>  	.num_clk_hws = ARRAY_SIZE(gcc_ipq5332_hws),
> +	.icc_hws = icc_ipq5332_hws,
> +	.num_icc_hws = ARRAY_SIZE(icc_ipq5332_hws),
> +	.icc_first_node_id = IPQ_APPS_ID,
>  };
>  
>  static int gcc_ipq5332_probe(struct platform_device *pdev)
> @@ -3674,6 +3699,7 @@ static struct platform_driver gcc_ipq5332_driver = {
>  	.driver = {
>  		.name = "gcc-ipq5332",
>  		.of_match_table = gcc_ipq5332_match_table,
> +		.sync_state = icc_sync_state,
>  	},
>  };
>  
> -- 
> 2.34.1
>