[PATCH v5 0/4] clk: zynqmp: Add firmware specific clock flags

[PATCH v5 0/4] clk: zynqmp: Add firmware specific clock flags

[Rajan Vaja]

Currently firmware is maintaining CCF specific flags and provides to
CCF as it is. But CCF flag numbers may change and that shouldn't mean
that the firmware needs to change. The firmware should have its own
'flag number space' that is distinct from the common clk framework's
'flag number space'. So use firmware specific clock flags in ZynqMP
clock driver instead of CCF flags.

Changes in v5:
 - Added base commit
 - Added patch #4 to handle divider specific read only flag

Changes in v4:
 - Use if condition instead of ternary operator.

Changes in v3:
 - Modify helper function signature to map zynqmp (common)flags with CCF
 - Add helper function to map zynqmp (mux & divider)flags with CCF flags

Changes in v2:
 - Add helper function to map zynqmp (common)flags with CCF flags.
 - Mapped zynqmp clock flags with CCF flags from
   zynqmp_clk_register_*() functions instead of
   __zynqmp_clock_get_topology() which is changing the flags to struct
   clk_init_data instead of the struct clock_topology.

Rajan Vaja (4):
  clk: zynqmp: Use firmware specific common clock flags
  clk: zynqmp: Use firmware specific divider clock flags
  clk: zynqmp: Use firmware specific mux clock flags
  clk: zynqmp: Handle divider specific read only flag

 drivers/clk/zynqmp/clk-gate-zynqmp.c |  4 ++-
 drivers/clk/zynqmp/clk-mux-zynqmp.c  | 27 ++++++++++++++++--
 drivers/clk/zynqmp/clk-zynqmp.h      | 41 ++++++++++++++++++++++++++++
 drivers/clk/zynqmp/clkc.c            | 33 +++++++++++++++++++++-
 drivers/clk/zynqmp/divider.c         | 40 ++++++++++++++++++++++++---
 drivers/clk/zynqmp/pll.c             |  4 ++-
 6 files changed, 140 insertions(+), 9 deletions(-)


base-commit: 6efb943b8616ec53a5e444193dccf1af9ad627b5
-- 
2.32.0.93.g670b81a

[PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock flags

[Rajan Vaja]

Currently firmware passes CCF specific flags to ZynqMP clock driver.
So firmware needs to be updated if CCF flags are changed. The firmware
should have its own 'flag number space' that is distinct from the
common clk framework's 'flag number space'. So define and use ZynqMP
specific common clock flags instead of using CCF flags.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
---
 drivers/clk/zynqmp/clk-gate-zynqmp.c |  4 +++-
 drivers/clk/zynqmp/clk-mux-zynqmp.c  |  4 +++-
 drivers/clk/zynqmp/clk-zynqmp.h      | 24 ++++++++++++++++++++
 drivers/clk/zynqmp/clkc.c            | 33 +++++++++++++++++++++++++++-
 drivers/clk/zynqmp/divider.c         |  5 +++--
 drivers/clk/zynqmp/pll.c             |  4 +++-
 6 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
index 10c9b889324f..695feaa82da5 100644
--- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
@@ -121,7 +121,9 @@ struct clk_hw *zynqmp_clk_register_gate(const char *name, u32 clk_id,
 
 	init.name = name;
 	init.ops = &zynqmp_clk_gate_ops;
-	init.flags = nodes->flag;
+
+	init.flags = zynqmp_clk_map_common_ccf_flags(nodes->flag);
+
 	init.parent_names = parents;
 	init.num_parents = 1;
 
diff --git a/drivers/clk/zynqmp/clk-mux-zynqmp.c b/drivers/clk/zynqmp/clk-mux-zynqmp.c
index 06194149be83..a49b1c586d5e 100644
--- a/drivers/clk/zynqmp/clk-mux-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-mux-zynqmp.c
@@ -120,7 +120,9 @@ struct clk_hw *zynqmp_clk_register_mux(const char *name, u32 clk_id,
 		init.ops = &zynqmp_clk_mux_ro_ops;
 	else
 		init.ops = &zynqmp_clk_mux_ops;
-	init.flags = nodes->flag;
+
+	init.flags = zynqmp_clk_map_common_ccf_flags(nodes->flag);
+
 	init.parent_names = parents;
 	init.num_parents = num_parents;
 	mux->flags = nodes->type_flag;
diff --git a/drivers/clk/zynqmp/clk-zynqmp.h b/drivers/clk/zynqmp/clk-zynqmp.h
index 5beeb41b29fa..974d3dae35a7 100644
--- a/drivers/clk/zynqmp/clk-zynqmp.h
+++ b/drivers/clk/zynqmp/clk-zynqmp.h
@@ -10,6 +10,28 @@
 
 #include <linux/firmware/xlnx-zynqmp.h>
 
+/* Common Flags */
+/* must be gated across rate change */
+#define ZYNQMP_CLK_SET_RATE_GATE	BIT(0)
+/* must be gated across re-parent */
+#define ZYNQMP_CLK_SET_PARENT_GATE	BIT(1)
+/* propagate rate change up one level */
+#define ZYNQMP_CLK_SET_RATE_PARENT	BIT(2)
+/* do not gate even if unused */
+#define ZYNQMP_CLK_IGNORE_UNUSED	BIT(3)
+/* do not use the cached clk rate */
+#define ZYNQMP_CLK_GET_RATE_NOCACHE	BIT(6)
+/* don't re-parent on rate change */
+#define ZYNQMP_CLK_SET_RATE_NO_REPARENT	BIT(7)
+/* do not use the cached clk accuracy */
+#define ZYNQMP_CLK_GET_ACCURACY_NOCACHE	BIT(8)
+/* recalc rates after notifications */
+#define ZYNQMP_CLK_RECALC_NEW_RATES	BIT(9)
+/* clock needs to run to set rate */
+#define ZYNQMP_CLK_SET_RATE_UNGATE	BIT(10)
+/* do not gate, ever */
+#define ZYNQMP_CLK_IS_CRITICAL		BIT(11)
+
 enum topology_type {
 	TYPE_INVALID,
 	TYPE_MUX,
@@ -33,6 +55,8 @@ struct clock_topology {
 	u8 custom_type_flag;
 };
 
+unsigned long zynqmp_clk_map_common_ccf_flags(const u32 zynqmp_flag);
+
 struct clk_hw *zynqmp_clk_register_pll(const char *name, u32 clk_id,
 				       const char * const *parents,
 				       u8 num_parents,
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,
 	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;
+	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;
+	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;
+
+	return ccf_flag;
+}
+
 /**
  * zynqmp_clk_register_fixed_factor() - Register fixed factor with the
  *					clock framework
@@ -292,6 +320,7 @@ struct clk_hw *zynqmp_clk_register_fixed_factor(const char *name, u32 clk_id,
 	struct zynqmp_pm_query_data qdata = {0};
 	u32 ret_payload[PAYLOAD_ARG_CNT];
 	int ret;
+	unsigned long flag;
 
 	qdata.qid = PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS;
 	qdata.arg1 = clk_id;
@@ -303,9 +332,11 @@ struct clk_hw *zynqmp_clk_register_fixed_factor(const char *name, u32 clk_id,
 	mult = ret_payload[1];
 	div = ret_payload[2];
 
+	flag = zynqmp_clk_map_common_ccf_flags(nodes->flag);
+
 	hw = clk_hw_register_fixed_factor(NULL, name,
 					  parents[0],
-					  nodes->flag, mult,
+					  flag, mult,
 					  div);
 
 	return hw;
diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
index e9bf7958b821..0becdc0a8bff 100644
--- a/drivers/clk/zynqmp/divider.c
+++ b/drivers/clk/zynqmp/divider.c
@@ -312,8 +312,9 @@ struct clk_hw *zynqmp_clk_register_divider(const char *name,
 
 	init.name = name;
 	init.ops = &zynqmp_clk_divider_ops;
-	/* CLK_FRAC is not defined in the common clk framework */
-	init.flags = nodes->flag & ~CLK_FRAC;
+
+	init.flags = zynqmp_clk_map_common_ccf_flags(nodes->flag);
+
 	init.parent_names = parents;
 	init.num_parents = 1;
 
diff --git a/drivers/clk/zynqmp/pll.c b/drivers/clk/zynqmp/pll.c
index abe6afbf3407..e0bceb07740f 100644
--- a/drivers/clk/zynqmp/pll.c
+++ b/drivers/clk/zynqmp/pll.c
@@ -312,7 +312,9 @@ struct clk_hw *zynqmp_clk_register_pll(const char *name, u32 clk_id,
 
 	init.name = name;
 	init.ops = &zynqmp_pll_ops;
-	init.flags = nodes->flag;
+
+	init.flags = zynqmp_clk_map_common_ccf_flags(nodes->flag);
+
 	init.parent_names = parents;
 	init.num_parents = 1;
 
-- 
2.32.0.93.g670b81a

[PATCH v5 2/4] clk: zynqmp: Use firmware specific divider clock flags

[Rajan Vaja]

Use ZynqMP specific divider clock flags instead of using CCF flags.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
---
 drivers/clk/zynqmp/clk-zynqmp.h |  9 +++++++++
 drivers/clk/zynqmp/divider.c    | 25 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/zynqmp/clk-zynqmp.h b/drivers/clk/zynqmp/clk-zynqmp.h
index 974d3dae35a7..9b2ff35ee136 100644
--- a/drivers/clk/zynqmp/clk-zynqmp.h
+++ b/drivers/clk/zynqmp/clk-zynqmp.h
@@ -32,6 +32,15 @@
 /* do not gate, ever */
 #define ZYNQMP_CLK_IS_CRITICAL		BIT(11)
 
+/* Type Flags for divider clock */
+#define ZYNQMP_CLK_DIVIDER_ONE_BASED		BIT(0)
+#define ZYNQMP_CLK_DIVIDER_POWER_OF_TWO		BIT(1)
+#define ZYNQMP_CLK_DIVIDER_ALLOW_ZERO		BIT(2)
+#define ZYNQMP_CLK_DIVIDER_HIWORD_MASK		BIT(3)
+#define ZYNQMP_CLK_DIVIDER_ROUND_CLOSEST	BIT(4)
+#define ZYNQMP_CLK_DIVIDER_READ_ONLY		BIT(5)
+#define ZYNQMP_CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
+
 enum topology_type {
 	TYPE_INVALID,
 	TYPE_MUX,
diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
index 0becdc0a8bff..c07423e03bc8 100644
--- a/drivers/clk/zynqmp/divider.c
+++ b/drivers/clk/zynqmp/divider.c
@@ -284,6 +284,29 @@ static u32 zynqmp_clk_get_max_divisor(u32 clk_id, u32 type)
 	return ret_payload[1];
 }
 
+static inline unsigned long zynqmp_clk_map_divider_ccf_flags(
+					       const u32 zynqmp_type_flag)
+{
+	unsigned long ccf_flag = 0;
+
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_ONE_BASED)
+		ccf_flag |= CLK_DIVIDER_ONE_BASED;
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_POWER_OF_TWO)
+		ccf_flag |= CLK_DIVIDER_POWER_OF_TWO;
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_ALLOW_ZERO)
+		ccf_flag |= CLK_DIVIDER_ALLOW_ZERO;
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_POWER_OF_TWO)
+		ccf_flag |= CLK_DIVIDER_HIWORD_MASK;
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_ROUND_CLOSEST)
+		ccf_flag |= CLK_DIVIDER_ROUND_CLOSEST;
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_READ_ONLY)
+		ccf_flag |= CLK_DIVIDER_READ_ONLY;
+	if (zynqmp_type_flag & ZYNQMP_CLK_DIVIDER_MAX_AT_ZERO)
+		ccf_flag |= CLK_DIVIDER_MAX_AT_ZERO;
+
+	return ccf_flag;
+}
+
 /**
  * zynqmp_clk_register_divider() - Register a divider clock
  * @name:		Name of this clock
@@ -321,7 +344,7 @@ struct clk_hw *zynqmp_clk_register_divider(const char *name,
 	/* struct clk_divider assignments */
 	div->is_frac = !!((nodes->flag & CLK_FRAC) |
 			  (nodes->custom_type_flag & CUSTOM_FLAG_CLK_FRAC));
-	div->flags = nodes->type_flag;
+	div->flags = zynqmp_clk_map_divider_ccf_flags(nodes->type_flag);
 	div->hw.init = &init;
 	div->clk_id = clk_id;
 	div->div_type = nodes->type;
-- 
2.32.0.93.g670b81a

[PATCH v5 3/4] clk: zynqmp: Use firmware specific mux clock flags

[Rajan Vaja]

Use ZynqMP specific mux clock flags instead of using CCF flags.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
---
 drivers/clk/zynqmp/clk-mux-zynqmp.c | 23 ++++++++++++++++++++++-
 drivers/clk/zynqmp/clk-zynqmp.h     |  8 ++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/zynqmp/clk-mux-zynqmp.c b/drivers/clk/zynqmp/clk-mux-zynqmp.c
index a49b1c586d5e..4c28b4d8d122 100644
--- a/drivers/clk/zynqmp/clk-mux-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-mux-zynqmp.c
@@ -90,6 +90,27 @@ static const struct clk_ops zynqmp_clk_mux_ro_ops = {
 	.get_parent = zynqmp_clk_mux_get_parent,
 };
 
+static inline unsigned long zynqmp_clk_map_mux_ccf_flags(
+				       const u32 zynqmp_type_flag)
+{
+	unsigned long ccf_flag = 0;
+
+	if (zynqmp_type_flag & ZYNQMP_CLK_MUX_INDEX_ONE)
+		ccf_flag |= CLK_MUX_INDEX_ONE;
+	if (zynqmp_type_flag & ZYNQMP_CLK_MUX_INDEX_BIT)
+		ccf_flag |= CLK_MUX_INDEX_BIT;
+	if (zynqmp_type_flag & ZYNQMP_CLK_MUX_HIWORD_MASK)
+		ccf_flag |= CLK_MUX_HIWORD_MASK;
+	if (zynqmp_type_flag & ZYNQMP_CLK_MUX_READ_ONLY)
+		ccf_flag |= CLK_MUX_READ_ONLY;
+	if (zynqmp_type_flag & ZYNQMP_CLK_MUX_ROUND_CLOSEST)
+		ccf_flag |= CLK_MUX_ROUND_CLOSEST;
+	if (zynqmp_type_flag & ZYNQMP_CLK_MUX_BIG_ENDIAN)
+		ccf_flag |= CLK_MUX_BIG_ENDIAN;
+
+	return ccf_flag;
+}
+
 /**
  * zynqmp_clk_register_mux() - Register a mux table with the clock
  *			       framework
@@ -125,7 +146,7 @@ struct clk_hw *zynqmp_clk_register_mux(const char *name, u32 clk_id,
 
 	init.parent_names = parents;
 	init.num_parents = num_parents;
-	mux->flags = nodes->type_flag;
+	mux->flags = zynqmp_clk_map_mux_ccf_flags(nodes->type_flag);
 	mux->hw.init = &init;
 	mux->clk_id = clk_id;
 
diff --git a/drivers/clk/zynqmp/clk-zynqmp.h b/drivers/clk/zynqmp/clk-zynqmp.h
index 9b2ff35ee136..87a2e1298be8 100644
--- a/drivers/clk/zynqmp/clk-zynqmp.h
+++ b/drivers/clk/zynqmp/clk-zynqmp.h
@@ -41,6 +41,14 @@
 #define ZYNQMP_CLK_DIVIDER_READ_ONLY		BIT(5)
 #define ZYNQMP_CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
 
+/* Type Flags for mux clock */
+#define ZYNQMP_CLK_MUX_INDEX_ONE		BIT(0)
+#define ZYNQMP_CLK_MUX_INDEX_BIT		BIT(1)
+#define ZYNQMP_CLK_MUX_HIWORD_MASK		BIT(2)
+#define ZYNQMP_CLK_MUX_READ_ONLY		BIT(3)
+#define ZYNQMP_CLK_MUX_ROUND_CLOSEST		BIT(4)
+#define ZYNQMP_CLK_MUX_BIG_ENDIAN		BIT(5)
+
 enum topology_type {
 	TYPE_INVALID,
 	TYPE_MUX,
-- 
2.32.0.93.g670b81a

[PATCH v5 4/4] clk: zynqmp: Handle divider specific read only flag

[Rajan Vaja]

Add support for divider specific read only CCF flag
(CLK_DIVIDER_READ_ONLY).

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
---
 drivers/clk/zynqmp/divider.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
index c07423e03bc8..cb49281f9cf9 100644
--- a/drivers/clk/zynqmp/divider.c
+++ b/drivers/clk/zynqmp/divider.c
@@ -256,6 +256,11 @@ static const struct clk_ops zynqmp_clk_divider_ops = {
 	.set_rate = zynqmp_clk_divider_set_rate,
 };
 
+static const struct clk_ops zynqmp_clk_divider_ro_ops = {
+	.recalc_rate = zynqmp_clk_divider_recalc_rate,
+	.round_rate = zynqmp_clk_divider_round_rate,
+};
+
 /**
  * zynqmp_clk_get_max_divisor() - Get maximum supported divisor from firmware.
  * @clk_id:		Id of clock
@@ -334,7 +339,10 @@ struct clk_hw *zynqmp_clk_register_divider(const char *name,
 		return ERR_PTR(-ENOMEM);
 
 	init.name = name;
-	init.ops = &zynqmp_clk_divider_ops;
+	if (nodes->type_flag & CLK_DIVIDER_READ_ONLY)
+		init.ops = &zynqmp_clk_divider_ro_ops;
+	else
+		init.ops = &zynqmp_clk_divider_ops;
 
 	init.flags = zynqmp_clk_map_common_ccf_flags(nodes->flag);
 
-- 
2.32.0.93.g670b81a

Re: [PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock flags

[Stephen Boyd]

Quoting Rajan Vaja (2021-06-24 05:16:30)
> 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,
>         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.

> +       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.

> +       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.

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

RE: [PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock flags

[Rajan Vaja]

Hi Stephen,

> -----Original Message-----
> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Friday, June 25, 2021 4:22 PM
> To: Rajan Vaja <RAJANV@xilinx.com>; kristo@kernel.org; lee.jones@linaro.org;
> Michal Simek <michals@xilinx.com>; 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 <RAJANV@xilinx.com>
> Subject: Re: [PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock
> flags
> 
> Quoting Rajan Vaja (2021-06-24 05:16:30)
> > 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,
> >         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.
> 
> > +       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.
> 
> > +       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
> 
> > +
> > +       return ccf_flag;
> > +}
> > +
> >  /**
> >   * zynqmp_clk_register_fixed_factor() - Register fixed factor with the
> >   *                                     clock framework