Hi Stephen,

-----Original Message-----
From: Stephen Boyd <sboyd@kernel.org>
Sent: Friday, June 25, 2021 4:22 PM
To: Rajan Vaja <redacted>; kristo@kernel.org; lee.jones@linaro.org;
Michal Simek [off-list ref]; mturquette@baylibre.com;
quanyang.wang@windriver.com
Cc: linux-clk@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; Rajan Vaja [off-list ref]
Subject: Re: [PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock
flags

Quoting Rajan Vaja (2021-06-24 05:16:30)

quoted

diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
index db8d0d7161ce..af06a195ec46 100644
--- a/drivers/clk/zynqmp/clkc.c
+++ b/drivers/clk/zynqmp/clkc.c

@@ -271,6 +271,34 @@ static int zynqmp_pm_clock_get_topology(u32

clock_id, u32 index,

quoted

        return ret;
 }

+unsigned long zynqmp_clk_map_common_ccf_flags(const u32 zynqmp_flag)
+{
+       unsigned long ccf_flag = 0;
+
+       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_GATE)
+               ccf_flag |= CLK_SET_RATE_GATE;
+       if (zynqmp_flag & ZYNQMP_CLK_SET_PARENT_GATE)
+               ccf_flag |= CLK_SET_PARENT_GATE;
+       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_PARENT)
+               ccf_flag |= CLK_SET_RATE_PARENT;
+       if (zynqmp_flag & ZYNQMP_CLK_IGNORE_UNUSED)
+               ccf_flag |= CLK_IGNORE_UNUSED;
+       if (zynqmp_flag & ZYNQMP_CLK_GET_RATE_NOCACHE)
+               ccf_flag |= CLK_GET_RATE_NOCACHE;

Does the firmware really use all these flags? Ideally we get rid of the
above two.

quoted

+       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_NO_REPARENT)
+               ccf_flag |= CLK_SET_RATE_NO_REPARENT;
+       if (zynqmp_flag & ZYNQMP_CLK_GET_ACCURACY_NOCACHE)
+               ccf_flag |= CLK_GET_ACCURACY_NOCACHE;
+       if (zynqmp_flag & ZYNQMP_CLK_RECALC_NEW_RATES)
+               ccf_flag |= CLK_RECALC_NEW_RATES;

And this one.

quoted

+       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_UNGATE)
+               ccf_flag |= CLK_SET_RATE_UNGATE;
+       if (zynqmp_flag & ZYNQMP_CLK_IS_CRITICAL)
+               ccf_flag |= CLK_IS_CRITICAL;

And this one.

I worry that supporting all these flags will mean we can never get rid
of them. And we currently don't support setting critical via DT, which
is essentially another firmware interface like this one.

[Rajan] firmware is using below flags:
ZYNQMP_CLK_SET_RATE_GATE
ZYNQMP_CLK_SET_PARENT_GATE
ZYNQMP_CLK_SET_RATE_PARENT
ZYNQMP_CLK_IGNORE_UNUSED
ZYNQMP_CLK_SET_RATE_NO_REPARENT
ZYNQMP_CLK_IS_CRITICAL

Other flags are unused. I will remove unused flags in next version.

Thanks,
Rajan

quoted

+
+       return ccf_flag;
+}
+
 /**
  * zynqmp_clk_register_fixed_factor() - Register fixed factor with the
  *                                     clock framework