devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] clk: add support for TI CDCE6214
@ 2025-04-08 12:00 Sascha Hauer
  2025-04-08 12:00 ` [PATCH 1/3] clk: make determine_rate optional for non reparenting clocks Sascha Hauer
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Sascha Hauer @ 2025-04-08 12:00 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-clk, linux-kernel, devicetree, kernel, Alvin Šipraga,
	Sascha Hauer

The CDCE6214 is a Ultra-Low Power Clock Generator With One PLL, Four
Differential Outputs, Two Inputs, and Internal EEPROM.

This series adds a common clk framework driver for this chip along with
the dt-bindings document and a small fix needed for the common clk
framework.

Sascha

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (3):
      clk: make determine_rate optional for non reparenting clocks
      clk: add TI CDCE6214 clock driver
      dt-bindings: clock: add TI CDCE6214 binding

 .../devicetree/bindings/clock/ti,cdce6214.yaml     |  157 +++
 drivers/clk/Kconfig                                |    7 +
 drivers/clk/Makefile                               |    1 +
 drivers/clk/clk-cdce6214.c                         | 1105 ++++++++++++++++++++
 drivers/clk/clk.c                                  |    3 +-
 include/dt-bindings/clock/ti,cdce6214.h            |   24 +
 6 files changed, 1296 insertions(+), 1 deletion(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250408-clk-cdce6214-0c74043dc267

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>


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

* [PATCH 1/3] clk: make determine_rate optional for non reparenting clocks
  2025-04-08 12:00 [PATCH 0/3] clk: add support for TI CDCE6214 Sascha Hauer
@ 2025-04-08 12:00 ` Sascha Hauer
  2025-04-08 12:00 ` [PATCH 2/3] clk: add TI CDCE6214 clock driver Sascha Hauer
  2025-04-08 12:00 ` [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding Sascha Hauer
  2 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2025-04-08 12:00 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-clk, linux-kernel, devicetree, kernel, Alvin Šipraga,
	Sascha Hauer

With 326cc42f9fdc ("clk: Forbid to register a mux without determine_rate")
it became mandatory to provide a determine_rate hook once a set_parent
hook is provided. The determine_rate hook is only needed though when the
clock reparents to set its rate. Clocks which do not reparent during
set_rate do not need a determine_rate hook, so make the hook optional
for clocks with the CLK_SET_RATE_NO_REPARENT flag.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/clk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0565c87656cf5..07ae3652df6c1 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3937,7 +3937,8 @@ static int __clk_core_init(struct clk_core *core)
 		goto out;
 	}
 
-	if (core->ops->set_parent && !core->ops->determine_rate) {
+	if (!(core->flags & CLK_SET_RATE_NO_REPARENT) &&
+	    core->ops->set_parent && !core->ops->determine_rate) {
 		pr_err("%s: %s must implement .set_parent & .determine_rate\n",
 			__func__, core->name);
 		ret = -EINVAL;

-- 
2.39.5


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

* [PATCH 2/3] clk: add TI CDCE6214 clock driver
  2025-04-08 12:00 [PATCH 0/3] clk: add support for TI CDCE6214 Sascha Hauer
  2025-04-08 12:00 ` [PATCH 1/3] clk: make determine_rate optional for non reparenting clocks Sascha Hauer
@ 2025-04-08 12:00 ` Sascha Hauer
  2025-04-08 13:43   ` Sascha Hauer
  2025-04-08 12:00 ` [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding Sascha Hauer
  2 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2025-04-08 12:00 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-clk, linux-kernel, devicetree, kernel, Alvin Šipraga,
	Sascha Hauer

The CDCE6214 is a Ultra-Low Power Clock Generator With One PLL, Four
Differential Outputs, Two Inputs, and Internal EEPROM. This patch adds
a common clk framework driver for this chip.

- Two inputs (PRIREF and SECREF)
- Programmable 8bit divider or x2 multiplier between input and PLL
- 16b integer / 24bit fractional PLL
- Two programmable /4, /5, /6 dividers after PLL (PSA/PSB)
- Four outputs (OUT1-OUT4) with programmable 14b dividers,
  muxable between PSA, PSB and PLL input
- One output (OUT0) fed from PLL input

- PRIREF can be configured as LVCMOS or differential input
- SECREF can be configured as LVCMOS, differential or oscillator input
- OUT0 is a LVCMOS output
- OUT1 and OUT4 can be configured as LVDS, LP-HCSL or LVCMOS outputs
- OUT2 and OUT3 can be configured as LVDS or LP-HCSL outputs

All clocks are registered without parent rate propagation, so each of
the clocks must be configured separately via device tree or consumer.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/Kconfig                     |    7 +
 drivers/clk/Makefile                    |    1 +
 drivers/clk/clk-cdce6214.c              | 1105 +++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ti,cdce6214.h |   24 +
 4 files changed, 1137 insertions(+)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 713573b6c86c7..499fd610c0467 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -170,6 +170,13 @@ config COMMON_CLK_BM1880
 	help
 	  This driver supports the clocks on Bitmain BM1880 SoC.
 
+config COMMON_CLK_CDCE6214
+	tristate "Clock driver for TI CDCE6214 clock synthesizer"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  This driver supports TI CDCE6214 programmable 1-PLL clock synthesizer.
+
 config COMMON_CLK_CDCE706
 	tristate "Clock driver for TI CDCE706 clock synthesizer"
 	depends on I2C
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index bf4bd45adc3a0..0f87b13b137b5 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
 obj-$(CONFIG_COMMON_CLK_BD718XX)	+= clk-bd718x7.o
 obj-$(CONFIG_COMMON_CLK_BM1880)		+= clk-bm1880.o
+obj-$(CONFIG_COMMON_CLK_CDCE6214)	+= clk-cdce6214.o
 obj-$(CONFIG_COMMON_CLK_CDCE706)	+= clk-cdce706.o
 obj-$(CONFIG_COMMON_CLK_CDCE925)	+= clk-cdce925.o
 obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
diff --git a/drivers/clk/clk-cdce6214.c b/drivers/clk/clk-cdce6214.c
new file mode 100644
index 0000000000000..a825cd71bb11b
--- /dev/null
+++ b/drivers/clk/clk-cdce6214.c
@@ -0,0 +1,1105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for the TI CDCE6214 clock generator
+ *
+ * Copyright (c) 2023 Alvin Šipraga <alsi@bang-olufsen.dk>
+ * Copyright (c) 2025 Sascha Hauer <s.hauer@pengutronix.de>
+ */
+
+#include <linux/i2c.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <dt-bindings/clock/ti,cdce6214.h>
+
+#define RO_I2C_A0			BIT(15)
+#define RO_PDN_INPUT_SEL		BIT(14)
+#define RO_GPIO4_DIR_SEL		BIT(13)
+#define RO_GPIO1_DIR_SEL		BIT(12)
+#define RO_ZDM_CLOCKSEL			BIT(10)
+#define RO_ZDM_EN			BIT(8)
+#define RO_SYNC				BIT(5)
+#define RO_RECAL			BIT(4)
+#define RO_RESETN_SOFT			BIT(3)
+#define RO_SWRST			BIT(2)
+#define RO_POWERDOWN			BIT(1)
+#define RO_MODE				BIT(0)
+
+#define R1_GPIO4_INPUT_SEL		GENMASK(15, 12)
+#define R1_GPIO3_INPUT_SEL		GENMASK(11, 8)
+#define R1_GPIO2_INPUT_SEL		GENMASK(7, 4)
+#define R1_GPIO1_INPUT_SEL		GENMASK(3, 0)
+
+#define R2_GPIO4_OUTPUT_SEL		GENMASK(9, 6)
+#define R2_GPIO1_OUTPUT_SEL		GENMASK(5, 2)
+#define R2_REFSEL_SW			GENMASK(1, 0)
+
+#define R3_DISABLE_CRC			BIT(13)
+#define R3_UPDATE_CRC			BIT(12)
+#define R3_NVMCOMMIT			BIT(11)
+#define R3_REGCOMMIT			BIT(10)
+#define R3_REGCOMMIT_PAGE		BIT(9)
+#define R3_FREQ_DEC_REG			BIT(6)
+#define R3_FREQ_INC_REG			BIT(5)
+#define R3_FREQ_INC_DEC_REG_MODE	BIT(4)
+#define R3_FREQ_INC_DEC_EN		BIT(3)
+
+#define R4_CH4_PD			BIT(7)
+#define R4_CH3_PD			BIT(6)
+#define R4_CH2_PD			BIT(5)
+#define R4_CH1_PD			BIT(4)
+#define R4_POST_EE_DLY			GENMASK(3, 0)
+
+#define R5_PLL_VCOBUFF_LDO_PD		BIT(8)
+#define R5_PLL_VCO_LDO_PD		BIT(7)
+#define R5_PLL_VCO_BUFF_PD		BIT(6)
+#define R5_PLL_CP_LDO_PD		BIT(5)
+#define R5_PLL_LOCKDET_PD		BIT(4)
+#define R5_PLL_PSB_PD			BIT(3)
+#define R5_PLL_PSA_PD			BIT(2)
+#define R5_PLL_PFD_PD			BIT(1)
+
+#define R7_NVMCRCERR			BIT(5)
+#define R7_LOCK_DET_S			BIT(1)
+#define R7_LOCK_DET			BIT(0)
+
+#define R9_NVMLCRC			GENMASK(15, 0)
+
+#define R10_NVMSCRC			GENMASK(15, 0)
+
+#define R11_NVM_RD_ADDR			GENMASK(5, 0)
+
+#define R12_NVM_RD_DATA			GENMASK(15, 0)
+
+#define R13_NVM_WR_ADDR			GENMASK(5, 0)
+
+#define R14_NVM_WR_DATA			GENMASK(15, 0)
+
+#define R15_EE_LOCK			GENMASK(15, 12)
+#define R15_CAL_MUTE			BIT(5)
+
+#define R24_IP_PRIREF_BUF_SEL		BIT(15)
+#define R24_IP_XO_CLOAD			GENMASK(12, 8)
+#define R24_IP_BIAS_SEL_XO		GENMASK(5, 2)
+#define R24_IP_SECREF_BUF_SEL		GENMASK(1, 0)
+#define R24_IP_SECREF_BUF_SEL_XTAL	0
+#define R24_IP_SECREF_BUF_SEL_LVCMOS	1
+#define R24_IP_SECREF_BUF_SEL_DIFF	2
+
+#define R25_IP_REF_TO_OUT4_EN		BIT(14)
+#define R25_IP_REF_TO_OUT3_EN		BIT(13)
+#define R25_IP_REF_TO_OUT2_EN		BIT(12)
+#define R25_IP_REF_TO_OUT1_EN		BIT(11)
+#define R25_IP_BYP_OUT0_EN		BIT(10)
+#define R25_REF_CH_MUX			BIT(9)
+#define R25_IP_RDIV			GENMASK(7, 0)
+
+#define R27_MASH_ORDER			GENMASK(1, 0)
+
+#define R30_PLL_NDIV			GENMASK(14, 0)
+
+#define R31_PLL_NUM_15_0		GENMASK(15, 0)
+
+#define R32_PLL_NUM_23_16		GENMASK(7, 0)
+
+#define R33_PLL_DEN_15_0		GENMASK(15, 0)
+
+#define R34_PLL_DEN_23_16		GENMASK(7, 0)
+
+#define R41_SSC_EN			BIT(15)
+
+#define R42_SSC_TYPE			BIT(5)
+#define R42_SSC_SEL			GENMASK(3, 1)
+
+#define R43_FREQ_INC_DEC_DELTA		GENMASK(15, 0)
+
+#define R47_PLL_CP_DN			GENMASK(12, 7)
+#define R47_PLL_PSB			GENMASK(6, 5)
+#define R47_PLL_PSA			GENMASK(4, 3)
+
+#define R48_PLL_LF_RES			GENMASK(14, 11)
+#define R48_PLL_CP_UP			GENMASK(5, 0)
+
+#define R49_PLL_LF_ZCAP			GENMASK(4, 0)
+
+#define R50_PLL_LOCKDET_WINDOW		GENMASK(10, 8)
+
+#define R51_PLL_PFD_DLY_EN		BIT(10)
+#define R51_PLL_PFD_CTRL		BIT(6)
+
+#define R52_PLL_NCTRL_EN		BIT(6)
+#define R52_PLL_CP_EN			BIT(3)
+
+#define R55_PLL_LF_3_PCTRIM		GENMASK(9, 8)
+#define R55_PLL_LF_3_PRTRIM		GENMASK(7, 6)
+
+#define R56_CH1_MUX			GENMASK(15, 14)
+#define R56_CH1_DIV			GENMASK(13, 0)
+
+#define R57_CH1_LPHCSL_EN		BIT(14)
+#define R57_CH1_1P8VDET			BIT(12)
+#define R57_CH1_GLITCHLESS_EN		BIT(9)
+#define R57_CH1_SYNC_DELAY		GENMASK(8, 4)
+#define R57_CH1_SYNC_EN			BIT(3)
+#define R57_CH1_MUTE_SEL		BIT(1)
+#define R57_CH1_MUTE			BIT(0)
+
+#define R59_CH1_LVDS_EN			BIT(15)
+#define R59_CH1_CMOSN_EN		BIT(14)
+#define R59_CH1_CMOSP_EN		BIT(13)
+#define R59_CH1_CMOSN_POL		BIT(12)
+#define R59_CH1_CMOSP_POL		BIT(11)
+
+#define R60_CH1_DIFFBUF_IBIAS_TRIM	GENMASK(15, 12)
+#define R60_CH1_LVDS_CMTRIM_INC		GENMASK(11, 10)
+#define R60_CH1_LVDS_CMTRIM_DEC		GENMASK(5, 4)
+#define R60_CH1_CMOS_SLEW_RATE_CTRL	GENMASK(3, 0)
+
+#define R62_CH2_MUX			GENMASK(15, 14)
+#define R62_CH2_DIV			GENMASK(13, 0)
+
+#define R63_CH2_LPHCSL_EN		BIT(13)
+#define R63_CH2_1P8VDET			BIT(12)
+#define R63_CH2_GLITCHLESS_EN		BIT(9)
+#define R63_CH2_SYNC_DELAY		GENMASK(8, 4)
+#define R63_CH2_SYNC_EN			BIT(3)
+#define R63_CH2_MUTE_SEL		BIT(1)
+#define R63_CH2_MUTE			BIT(0)
+
+#define R65_CH2_LVDS_CMTRIM_DEC		GENMASK(14, 13)
+#define R65_CH2_LVDS_EN			BIT(11)
+
+#define R66_CH2_LVDS_CMTRIM_IN		GENMASK(5, 4)
+#define R66_CH2_DIFFBUF_IBIAS_TRIM	GENMASK(3, 0)
+
+#define R67_CH3_MUX			GENMASK(15, 14)
+#define R67_CH3_DIV			GENMASK(13, 0)
+
+#define R68_CH3_LPHCSL_EN		BIT(13)
+#define R68_CH3_1P8VDET			BIT(12)
+#define R68_CH3_GLITCHLESS_EN		BIT(9)
+#define R68_CH3_SYNC_DELAY		GENMASK(8, 4)
+#define R68_CH3_SYNC_EN			BIT(3)
+#define R68_CH3_MUTE_SEL		BIT(1)
+#define R68_CH3_MUTE			BIT(0)
+
+#define R70_CH3_LVDS_EN			BIT(11)
+
+#define R71_CH3_LVDS_CMTRIM_DEC		GENMASK(10, 9)
+#define R71_CH3_LVDS_CMTRIM_INC		GENMASK(5, 4)
+#define R71_CH3_DIFFBUF_IBIAS_TR	GENMASK(3, 0)
+
+#define R72_CH4_MUX			GENMASK(15, 14)
+#define R72_CH4_DIV			GENMASK(13, 0)
+
+#define R73_CH4_LPHCSL_EN		BIT(13)
+#define R73_CH4_1P8VDET			BIT(12)
+#define R73_CH4_GLITCHLESS_EN		BIT(9)
+#define R73_CH4_SYNC_DELAY		GENMASK(8, 4)
+#define R73_CH4_SYNC_EN			BIT(3)
+#define R73_CH4_MUTE_SEL		BIT(1)
+#define R73_CH4_MUTE			BIT(0)
+
+#define R75_CH4_LVDS_EN			BIT(15)
+#define R75_CH4_CMOSP_EN		BIT(14)
+#define R75_CH4_CMOSN_EN		BIT(13)
+#define R75_CH4_CMOSP_POL		BIT(12)
+#define R75_CH4_CMOSN_POL		BIT(11)
+
+#define R76_CH4_DIFFBUF_IBIAS_TRIM	GENMASK(9, 6)
+#define R76_CH4_LVDS_CMTRIM_IN		GENMASK(5, 4)
+#define R76_CH4_CMOS_SLEW_RATE_CTRL	GENMASK(3, 0)
+
+#define R77_CH4_LVDS_CMTRIM_DEC		GENMASK(1, 0)
+
+#define R78_CH0_EN			BIT(12)
+
+#define R79_SAFETY_1P8V_MODE		BIT(9)
+#define R79_CH0_CMOS_SLEW_RATE_CTRL	GENMASK(3, 0)
+
+#define R81_PLL_LOCK_MASK		BIT(3)
+
+#define CDCE6214_VCO_MIN 2335000000
+#define CDCE6214_VCO_MAX 2625000000
+#define CDCE6214_DENOM_DEFAULT (1 << 24)
+
+static char *clk_names[] = {
+	[CDCE6214_CLK_PRIREF] = "priref",
+	[CDCE6214_CLK_SECREF] = "secref",
+	[CDCE6214_CLK_OUT0] = "out0",
+	[CDCE6214_CLK_OUT1] = "out1",
+	[CDCE6214_CLK_OUT2] = "out2",
+	[CDCE6214_CLK_OUT3] = "out3",
+	[CDCE6214_CLK_OUT4] = "out4",
+	[CDCE6214_CLK_PLL] = "pll",
+	[CDCE6214_CLK_PSA] = "psa",
+	[CDCE6214_CLK_PSB] = "psb",
+};
+
+#define CDCE6214_NUM_CLOCKS	ARRAY_SIZE(clk_names)
+
+struct cdce6214;
+
+struct cdce6214_clock {
+	struct clk_hw hw;
+	struct cdce6214 *priv;
+	int index;
+};
+
+struct cdce6214_config {
+	const struct reg_default *reg_default;
+	int reg_default_size;
+};
+
+struct cdce6214 {
+	struct i2c_client *client;
+	struct device *dev;
+	struct regmap *regmap;
+	struct gpio_desc *reset_gpio;
+	const struct cdce6214_config *config;
+	struct cdce6214_clock clk[CDCE6214_NUM_CLOCKS];
+};
+
+static inline struct cdce6214_clock *hw_to_cdce6214_clk(struct clk_hw *hw)
+{
+	return container_of(hw, struct cdce6214_clock, hw);
+}
+
+static struct clk_hw *cdce6214_of_clk_get(struct of_phandle_args *clkspec,
+					  void *data)
+{
+	struct cdce6214 *priv = data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx >= CDCE6214_NUM_CLOCKS)
+		return ERR_PTR(-EINVAL);
+	if (idx <= CDCE6214_CLK_SECREF)
+		return ERR_PTR(-EINVAL);
+
+	return &priv->clk[idx].hw;
+}
+
+static const struct regmap_config cdce6214_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 16,
+	.reg_stride = 1,
+	.max_register = 0x0055,
+};
+
+static int cdce6214_configure(struct cdce6214 *priv)
+{
+	regmap_update_bits(priv->regmap, 2, R2_REFSEL_SW,
+			   FIELD_PREP(R2_REFSEL_SW, 2));
+
+	return 0;
+}
+
+static unsigned long cdce6214_clk_out0_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int val, div;
+
+	regmap_read(priv->regmap, 25, &val);
+
+	div = FIELD_GET(R25_IP_RDIV, val);
+
+	if (!div)
+		return parent_rate * 2;
+
+	return DIV_ROUND_UP_ULL((u64)parent_rate, div);
+}
+
+static long cdce6214_clk_out0_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long *best_parent_rate)
+{
+	unsigned int div;
+
+	if (rate >= *best_parent_rate)
+		return *best_parent_rate * 2;
+
+	div = DIV_ROUND_CLOSEST(*best_parent_rate, rate);
+
+	return DIV_ROUND_UP_ULL((u64)*best_parent_rate, div);
+}
+
+static int cdce6214_clk_out0_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long parent_rate)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int div;
+
+	if (rate >= parent_rate) {
+		regmap_update_bits(priv->regmap, 25, R25_IP_RDIV, FIELD_PREP(R25_IP_RDIV, 0));
+		return 0;
+	}
+
+	div = DIV_ROUND_CLOSEST(parent_rate, rate);
+	if (div > R25_IP_RDIV)
+		div = R25_IP_RDIV;
+
+	regmap_update_bits(priv->regmap, 25, R25_IP_RDIV, FIELD_PREP(R25_IP_RDIV, div));
+
+	return 0;
+}
+
+static u8 cdce6214_clk_out0_get_parent(struct clk_hw *hw)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int val, idx;
+
+	regmap_read(priv->regmap, 2, &val);
+
+	idx = FIELD_GET(R2_REFSEL_SW, val);
+
+	switch (idx) {
+		case 0:
+		case 1:
+			idx = 0;
+			break;
+		case 2:
+			idx = 1;
+			break;
+		case 3:
+			idx = 0;
+			break;
+	};
+
+	return idx;
+}
+
+static int cdce6214_clk_out0_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	regmap_update_bits(priv->regmap, 25, R25_REF_CH_MUX, FIELD_PREP(R25_REF_CH_MUX, index));
+
+	return 0;
+}
+
+static const struct clk_ops cdce6214_clk_out0_ops = {
+	.recalc_rate = cdce6214_clk_out0_recalc_rate,
+	.round_rate = cdce6214_clk_out0_round_rate,
+	.set_rate = cdce6214_clk_out0_set_rate,
+	.get_parent = cdce6214_clk_out0_get_parent,
+	.set_parent = cdce6214_clk_out0_set_parent,
+};
+
+static int cdce6214_clk_out_ldo(struct clk_hw *hw, int enable)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int mask, val;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_OUT1:
+		mask = R4_CH1_PD;
+		break;
+	case CDCE6214_CLK_OUT2:
+		mask = R4_CH2_PD;
+		break;
+	case CDCE6214_CLK_OUT3:
+		mask = R4_CH3_PD;
+		break;
+	case CDCE6214_CLK_OUT4:
+		mask = R4_CH4_PD;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	if (enable > 0) {
+		regmap_clear_bits(priv->regmap, 4, mask);
+	} else if (!enable) {
+		regmap_set_bits(priv->regmap, 4, mask);
+	} else {
+		regmap_read(priv->regmap, 4, &val);
+		return !(val & mask);
+	}
+
+	return 0;
+}
+
+static int cdce6214_clk_out_prepare(struct clk_hw *hw)
+{
+	return cdce6214_clk_out_ldo(hw, 1);
+}
+
+static void cdce6214_clk_out_unprepare(struct clk_hw *hw)
+{
+	cdce6214_clk_out_ldo(hw, 0);
+}
+
+static int cdce6214_clk_out_is_prepared(struct clk_hw *hw)
+{
+	return cdce6214_clk_out_ldo(hw, -1);
+}
+
+static unsigned long cdce6214_clk_out_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int val, div;
+	unsigned long r;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_OUT1:
+		regmap_read(priv->regmap, 56, &val);
+		div = FIELD_GET(R56_CH1_DIV, val);
+		break;
+	case CDCE6214_CLK_OUT2:
+		regmap_read(priv->regmap, 62, &val);
+		div = FIELD_GET(R62_CH2_DIV, val);
+		break;
+	case CDCE6214_CLK_OUT3:
+		regmap_read(priv->regmap, 67, &val);
+		div = FIELD_GET(R67_CH3_DIV, val);
+		break;
+	case CDCE6214_CLK_OUT4:
+		regmap_read(priv->regmap, 72, &val);
+		div = FIELD_GET(R72_CH4_DIV, val);
+		break;
+	};
+
+	if (!div)
+		div = 1;
+
+	r = DIV_ROUND_UP_ULL((u64)parent_rate, div);
+
+	return r;
+}
+
+static int cdce6214_get_out_div(unsigned long rate, unsigned long parent_rate)
+{
+	unsigned int div = DIV_ROUND_CLOSEST(parent_rate, rate);
+
+	if (div < 1)
+		div = 1;
+
+	if (div > R72_CH4_DIV)
+		div = R72_CH4_DIV;
+
+	return div;
+}
+
+static long cdce6214_clk_out_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long *best_parent_rate)
+{
+	unsigned int div = cdce6214_get_out_div(rate, *best_parent_rate);
+
+	return DIV_ROUND_UP_ULL((u64)*best_parent_rate, div);
+}
+
+static int cdce6214_clk_out_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long parent_rate)
+{
+	unsigned int div = cdce6214_get_out_div(rate, parent_rate);
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_OUT1:
+		regmap_update_bits(priv->regmap, 56, R56_CH1_DIV,
+				   FIELD_PREP(R56_CH1_DIV, div));
+		break;
+	case CDCE6214_CLK_OUT2:
+		regmap_update_bits(priv->regmap, 62, R62_CH2_DIV,
+				   FIELD_PREP(R62_CH2_DIV, div));
+		break;
+	case CDCE6214_CLK_OUT3:
+		regmap_update_bits(priv->regmap, 67, R67_CH3_DIV,
+				   FIELD_PREP(R67_CH3_DIV, div));
+		break;
+	case CDCE6214_CLK_OUT4:
+		regmap_update_bits(priv->regmap, 72, R72_CH4_DIV,
+				   FIELD_PREP(R72_CH4_DIV, div));
+		break;
+	};
+
+	return 0;
+}
+
+static u8 cdce6214_clk_out_get_parent(struct clk_hw *hw)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int val, idx;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_OUT1:
+		regmap_read(priv->regmap, 56, &val);
+		idx = FIELD_GET(R56_CH1_MUX, val);
+		break;
+	case CDCE6214_CLK_OUT2:
+		regmap_read(priv->regmap, 62, &val);
+		idx = FIELD_GET(R62_CH2_MUX, val);
+		break;
+	case CDCE6214_CLK_OUT3:
+		regmap_read(priv->regmap, 67, &val);
+		idx = FIELD_GET(R67_CH3_MUX, val);
+		break;
+	case CDCE6214_CLK_OUT4:
+		regmap_read(priv->regmap, 72, &val);
+		idx = FIELD_GET(R72_CH4_MUX, val);
+		break;
+	};
+
+	return idx;
+}
+
+static int cdce6214_clk_out_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_OUT1:
+		regmap_update_bits(priv->regmap, 56, R56_CH1_MUX, FIELD_PREP(R56_CH1_MUX, index));
+		break;
+	case CDCE6214_CLK_OUT2:
+		regmap_update_bits(priv->regmap, 62, R62_CH2_MUX, FIELD_PREP(R62_CH2_MUX, index));
+		break;
+	case CDCE6214_CLK_OUT3:
+		regmap_update_bits(priv->regmap, 67, R67_CH3_MUX, FIELD_PREP(R67_CH3_MUX, index));
+		break;
+	case CDCE6214_CLK_OUT4:
+		regmap_update_bits(priv->regmap, 72, R72_CH4_MUX, FIELD_PREP(R72_CH4_MUX, index));
+		break;
+	};
+
+	return 0;
+}
+
+static const struct clk_ops cdce6214_clk_out_ops = {
+	.prepare = cdce6214_clk_out_prepare,
+	.unprepare = cdce6214_clk_out_unprepare,
+	.is_prepared = cdce6214_clk_out_is_prepared,
+	.recalc_rate = cdce6214_clk_out_recalc_rate,
+	.round_rate = cdce6214_clk_out_round_rate,
+	.set_rate = cdce6214_clk_out_set_rate,
+	.get_parent = cdce6214_clk_out_get_parent,
+	.set_parent = cdce6214_clk_out_set_parent,
+};
+
+static int pll_calc_values(unsigned long parent_rate, unsigned long out,
+			   unsigned long *ndiv, unsigned long *num, unsigned long *den)
+{
+	u64 a;
+
+	if (out < CDCE6214_VCO_MIN || out > CDCE6214_VCO_MAX)
+		return -EINVAL;
+
+	*den = 10000000;
+	*ndiv = out / parent_rate;
+	a = (out % parent_rate);
+	a *= *den;
+	do_div(a, parent_rate);
+	*num = a;
+
+	return 0;
+}
+
+static unsigned long cdce6214_clk_pll_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned long ndiv, num, den;
+	unsigned int val;
+
+	regmap_read(priv->regmap, 30, &val);
+	ndiv = FIELD_GET(R30_PLL_NDIV, val);
+
+	regmap_read(priv->regmap, 31, &val);
+	num = FIELD_GET(R31_PLL_NUM_15_0, val);
+
+	regmap_read(priv->regmap, 32, &val);
+	num |= FIELD_GET(R32_PLL_NUM_23_16, val) << 16;
+
+	regmap_read(priv->regmap, 33, &val);
+	den = FIELD_GET(R33_PLL_DEN_15_0, val);
+
+	regmap_read(priv->regmap, 34, &val);
+	den |= FIELD_GET(R34_PLL_DEN_23_16, val) << 16;
+
+	if (!den)
+		den = CDCE6214_DENOM_DEFAULT;
+
+	return parent_rate * ndiv + DIV_ROUND_CLOSEST(parent_rate * num, den);
+}
+
+static long cdce6214_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long *best_parent_rate)
+{
+	if (rate < CDCE6214_VCO_MIN)
+		rate = CDCE6214_VCO_MIN;
+	if (rate > CDCE6214_VCO_MAX)
+		rate = CDCE6214_VCO_MAX;
+	if (rate < *best_parent_rate * 24)
+		return -EINVAL;
+
+	return rate;
+}
+
+static bool cdce6214_pll_locked(struct cdce6214 *priv)
+{
+	unsigned int val;
+
+	regmap_read(priv->regmap, 7, &val);
+
+	return val & R7_LOCK_DET;
+}
+
+static int cdce6214_wait_pll_lock(struct cdce6214 *priv)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(priv->regmap, 7, val,
+				       val & R7_LOCK_DET, 0, 1000);
+	if (ret)
+		dev_err(priv->dev, "Timeout waiting for PLL lock\n");
+
+	return ret;
+}
+
+#define R5_PLL_POWER_BITS (R5_PLL_VCOBUFF_LDO_PD | \
+			   R5_PLL_VCO_LDO_PD | \
+			   R5_PLL_VCO_BUFF_PD)
+
+static int cdce6214_clk_pll_prepare(struct clk_hw *hw)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	regmap_clear_bits(priv->regmap, 5, R5_PLL_POWER_BITS);
+
+	regmap_set_bits(priv->regmap, 0, RO_RECAL);
+
+	return cdce6214_wait_pll_lock(priv);
+}
+
+static void cdce6214_clk_pll_unprepare(struct clk_hw *hw)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	regmap_set_bits(priv->regmap, 5, R5_PLL_POWER_BITS);
+}
+
+static bool cdce6214_clk_pll_powered(struct cdce6214 *priv)
+{
+	unsigned int val;
+
+	regmap_read(priv->regmap, 5, &val);
+
+	return (val & R5_PLL_POWER_BITS) == 0;
+}
+
+static int cdce6214_clk_pll_is_prepared(struct clk_hw *hw)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	return cdce6214_pll_locked(priv);
+}
+
+static int cdce6214_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long parent_rate)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned long ndiv, num, den;
+	int ret;
+
+	ret = pll_calc_values(parent_rate, rate, &ndiv, &num, &den);
+	if (ret < 0)
+		return ret;
+
+	if (den == CDCE6214_DENOM_DEFAULT)
+		den = 0;
+
+	regmap_update_bits(priv->regmap, 34, R34_PLL_DEN_23_16,
+			   FIELD_PREP(R34_PLL_DEN_23_16, den >> 16));
+	regmap_update_bits(priv->regmap, 33, R33_PLL_DEN_15_0,
+			   FIELD_PREP(R33_PLL_DEN_15_0, den & 0xffff));
+	regmap_update_bits(priv->regmap, 32, R32_PLL_NUM_23_16,
+			   FIELD_PREP(R32_PLL_NUM_23_16, num >> 16));
+	regmap_update_bits(priv->regmap, 31, R31_PLL_NUM_15_0,
+			   FIELD_PREP(R31_PLL_NUM_15_0, num & 0xffff));
+	regmap_update_bits(priv->regmap, 30, R30_PLL_NDIV,
+			   FIELD_PREP(R30_PLL_NDIV, ndiv));
+
+	regmap_update_bits(priv->regmap, 3, R3_FREQ_INC_DEC_REG_MODE | R3_FREQ_INC_DEC_EN,
+			   R3_FREQ_INC_DEC_REG_MODE | R3_FREQ_INC_DEC_EN);
+
+	if (cdce6214_clk_pll_powered(priv)) {
+		regmap_set_bits(priv->regmap, 0, RO_RECAL);
+		ret = cdce6214_wait_pll_lock(priv);
+	}
+
+	return ret;
+}
+
+static const struct clk_ops cdce6214_clk_pll_ops = {
+	.prepare = cdce6214_clk_pll_prepare,
+	.unprepare = cdce6214_clk_pll_unprepare,
+	.is_prepared = cdce6214_clk_pll_is_prepared,
+	.recalc_rate = cdce6214_clk_pll_recalc_rate,
+	.round_rate = cdce6214_clk_pll_round_rate,
+	.set_rate = cdce6214_clk_pll_set_rate,
+};
+
+static int cdce6214_clk_psx_ldo(struct clk_hw *hw, int enable)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int mask, val;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_PSA:
+		mask = R5_PLL_PSA_PD;
+		break;
+	case CDCE6214_CLK_PSB:
+		mask = R5_PLL_PSB_PD;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	if (enable > 0 ) {
+		regmap_clear_bits(priv->regmap, 5, mask);
+	} else if (!enable) {
+		regmap_set_bits(priv->regmap, 5, mask);
+	} else {
+		regmap_read(priv->regmap, 5, &val);
+
+		return !(val & mask);
+	}
+
+	return 0;
+}
+
+static int cdce6214_clk_psx_prepare(struct clk_hw *hw)
+{
+	return cdce6214_clk_psx_ldo(hw, 1);
+}
+
+static void cdce6214_clk_psx_unprepare(struct clk_hw *hw)
+{
+	cdce6214_clk_psx_ldo(hw, 0);
+}
+
+static int cdce6214_clk_psx_is_prepared(struct clk_hw *hw)
+{
+	return cdce6214_clk_psx_ldo(hw, -1);
+}
+
+static unsigned long cdce6214_clk_psx_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+	unsigned int psx[] = { 4, 5, 6, 6 };
+	unsigned int val, div;
+
+	regmap_read(priv->regmap, 47, &val);
+
+	switch (clock->index) {
+	case CDCE6214_CLK_PSA:
+		div = psx[FIELD_GET(R47_PLL_PSA, val)];
+		break;
+	case CDCE6214_CLK_PSB:
+		div = psx[FIELD_GET(R47_PLL_PSB, val)];
+		break;
+	};
+
+	return DIV_ROUND_UP_ULL((u64)parent_rate, div);
+}
+
+static int cdce6214_get_psx_div(unsigned long rate, unsigned long parent_rate)
+{
+	unsigned int div = DIV_ROUND_CLOSEST(parent_rate, rate);
+
+	if (div < 4)
+		div = 4;
+
+	if (div > 6)
+		div = 6;
+
+	return div;
+}
+
+static long cdce6214_clk_psx_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long *best_parent_rate)
+{
+	unsigned int div = cdce6214_get_psx_div(rate, *best_parent_rate);
+
+	return DIV_ROUND_UP_ULL((u64)*best_parent_rate, div);
+}
+
+static int cdce6214_clk_psx_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long parent_rate)
+{
+	unsigned int div = cdce6214_get_psx_div(rate, parent_rate);
+	struct cdce6214_clock *clock = hw_to_cdce6214_clk(hw);
+	struct cdce6214 *priv = clock->priv;
+
+	switch (clock->index) {
+	case CDCE6214_CLK_PSA:
+		regmap_update_bits(priv->regmap, 47, R47_PLL_PSA,
+				   FIELD_PREP(R47_PLL_PSA, div));
+		break;
+	case CDCE6214_CLK_PSB:
+		regmap_update_bits(priv->regmap, 47, R47_PLL_PSB,
+				   FIELD_PREP(R47_PLL_PSB, div));
+		break;
+	};
+	
+	return 0;
+}
+
+static const struct clk_ops cdce6214_clk_psx_ops = {
+	.prepare = cdce6214_clk_psx_prepare,
+	.unprepare = cdce6214_clk_psx_unprepare,
+	.is_prepared = cdce6214_clk_psx_is_prepared,
+	.recalc_rate = cdce6214_clk_psx_recalc_rate,
+	.round_rate = cdce6214_clk_psx_round_rate,
+	.set_rate = cdce6214_clk_psx_set_rate,
+};
+
+static int cdce6214_clk_register(struct cdce6214 *priv)
+{
+	struct clk_init_data init[CDCE6214_NUM_CLOCKS] = { 0 };
+	struct clk_parent_data pdata_out0[2] = {};
+	struct clk_parent_data pdata_out[4] = {};
+	struct clk_parent_data pdata_pll = {};
+	struct clk_parent_data pdata_psx = {};
+	int i, ret;
+
+	pdata_out0[0].fw_name = "priref";
+	pdata_out0[1].fw_name = "secref";
+
+	init[CDCE6214_CLK_OUT0].ops = &cdce6214_clk_out0_ops;
+	init[CDCE6214_CLK_OUT0].num_parents = 2;
+	init[CDCE6214_CLK_OUT0].parent_data = pdata_out0;
+	init[CDCE6214_CLK_OUT0].flags = CLK_SET_RATE_NO_REPARENT;
+
+	pdata_out[0].hw = &priv->clk[CDCE6214_CLK_PSA].hw;
+	pdata_out[1].hw = &priv->clk[CDCE6214_CLK_PSB].hw;
+	pdata_out[3].hw = &priv->clk[CDCE6214_CLK_OUT0].hw;
+
+	for (i = CDCE6214_CLK_OUT1; i <= CDCE6214_CLK_OUT4; i++) {
+		init[i].ops = &cdce6214_clk_out_ops;
+		init[i].num_parents = 4;
+		init[i].parent_data = pdata_out;
+		init[i].flags = CLK_SET_RATE_NO_REPARENT;
+	}
+
+	init[CDCE6214_CLK_PLL].ops = &cdce6214_clk_pll_ops;
+	init[CDCE6214_CLK_PLL].num_parents = 1;
+	pdata_pll.hw = &priv->clk[CDCE6214_CLK_OUT0].hw;
+	init[CDCE6214_CLK_PLL].parent_data = &pdata_pll;
+
+	pdata_psx.hw = &priv->clk[CDCE6214_CLK_PLL].hw;
+	for (i = CDCE6214_CLK_PSA; i <= CDCE6214_CLK_PSB; i++) {
+		init[i].ops = &cdce6214_clk_psx_ops;
+		init[i].num_parents = 1;
+		init[i].parent_data = &pdata_psx;
+	}
+
+	for (i = 0; i < CDCE6214_NUM_CLOCKS; i++) {
+		struct cdce6214_clock *clk = &priv->clk[i];
+		char name[128];
+
+		if (!init[i].ops)
+			continue;
+
+		snprintf(name, sizeof(name), "%s_%s", dev_name(priv->dev), clk_names[i]);
+		init[i].name = name;
+		clk->hw.init = &init[i];
+		clk->priv = priv;
+		clk->index = i;
+		ret = devm_clk_hw_register(priv->dev, &clk->hw);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static inline int regmap_clrset_bit(struct cdce6214 *priv, unsigned int reg,
+				    unsigned int bit, bool set)
+{
+	return regmap_update_bits(priv->regmap, reg, bit, set ? bit : 0);
+}
+
+static int cdce6214_parse_subnode(struct cdce6214 *priv, struct device_node *np)
+{
+	unsigned int idx, mode;
+	struct cdce6214_clock *clk;
+	bool lphcsl, lvds, cmosp, cmosn, ref_xtal, ref_lvcmos, ref_diff;
+	int ret;
+
+	ret = of_property_read_u32(np, "reg", &idx);
+	if (ret) {
+		dev_err(priv->dev, "missing reg property in child: %s\n",
+			np->full_name);
+		return ret;
+	}
+
+	if (idx >= CDCE6214_NUM_CLOCKS)
+		return -EINVAL;
+
+	clk = &priv->clk[idx];
+
+	lphcsl = of_property_read_bool(np, "ti,lphcsl");
+	lvds = of_property_read_bool(np, "ti,lvds");
+	cmosp = of_property_read_bool(np, "ti,cmosp");
+	cmosn = of_property_read_bool(np, "ti,cmosn");
+
+	ref_xtal = of_property_read_bool(np, "ti,ref-xtal");
+	ref_lvcmos = of_property_read_bool(np, "ti,ref-lvcmos");
+	ref_diff = of_property_read_bool(np, "ti,ref-diff");
+
+	switch (idx) {
+	case CDCE6214_CLK_OUT1:
+		regmap_clrset_bit(priv, 57, R57_CH1_LPHCSL_EN, lphcsl);
+		regmap_clrset_bit(priv, 59, R59_CH1_LVDS_EN, lvds);
+		regmap_clrset_bit(priv, 59, R59_CH1_CMOSP_EN, cmosp);
+		regmap_clrset_bit(priv, 59, R59_CH1_CMOSN_EN, cmosn);
+		lphcsl = lvds = cmosp = cmosn = false;
+		break;
+	case CDCE6214_CLK_OUT2:
+		regmap_clrset_bit(priv, 63, R63_CH2_LPHCSL_EN, lphcsl);
+		regmap_clrset_bit(priv, 65, R65_CH2_LVDS_EN, lvds);
+		lphcsl = lvds = false;
+		break;
+	case CDCE6214_CLK_OUT3:
+		regmap_clrset_bit(priv, 68, R68_CH3_LPHCSL_EN, lphcsl);
+		regmap_clrset_bit(priv, 70, R70_CH3_LVDS_EN, lvds);
+		lphcsl = lvds = false;
+		break;
+	case CDCE6214_CLK_OUT4:
+		regmap_clrset_bit(priv, 73, R73_CH4_LPHCSL_EN, lphcsl);
+		regmap_clrset_bit(priv, 75, R75_CH4_LVDS_EN, lvds);
+		regmap_clrset_bit(priv, 75, R75_CH4_CMOSP_EN, cmosp);
+		regmap_clrset_bit(priv, 75, R75_CH4_CMOSN_EN, cmosn);
+		lphcsl = lvds = cmosp = cmosn = false;
+		break;
+	case CDCE6214_CLK_PRIREF:
+		if (ref_lvcmos + ref_diff != 1) {
+			dev_err(priv->dev, "Multiple or no input modes in %pOF\n", np);
+			return -EINVAL;
+		}
+
+		if (ref_lvcmos)
+			regmap_clear_bits(priv->regmap, 24, R24_IP_PRIREF_BUF_SEL);
+		if (ref_diff)
+			regmap_set_bits(priv->regmap, 24, R24_IP_PRIREF_BUF_SEL);
+
+		ref_lvcmos = ref_diff = false;
+		break;
+	case CDCE6214_CLK_SECREF:
+		if (ref_xtal + ref_lvcmos + ref_diff != 1) {
+			dev_err(priv->dev, "Multiple or no input modes in %pOF\n", np);
+			return -EINVAL;
+		}
+
+		if (ref_xtal)
+			mode = R24_IP_SECREF_BUF_SEL_XTAL;
+		else if (ref_lvcmos)
+			mode = R24_IP_SECREF_BUF_SEL_LVCMOS;
+		else
+			mode = R24_IP_SECREF_BUF_SEL_DIFF;
+
+		regmap_update_bits(priv->regmap, 24, R24_IP_SECREF_BUF_SEL, mode);
+		ref_xtal = ref_lvcmos = ref_diff = false;
+		break;
+	}
+
+	if (lphcsl || lvds || cmosp || cmosn || ref_xtal || ref_lvcmos || ref_diff)
+		dev_warn(priv->dev, "%pOF contains unhandled properties\n", np);
+
+	return 0;
+}
+
+static int cdce6214_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct device_node *child;
+	struct cdce6214 *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->config = device_get_match_data(dev);
+	priv->client = client;
+	priv->dev = dev;
+	i2c_set_clientdata(client, priv);
+	dev_set_drvdata(dev, priv);
+
+	priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->reset_gpio)) {
+		return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
+				     "failed to get reset gpio\n");
+	}
+
+	priv->regmap = devm_regmap_init_i2c(client, &cdce6214_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	ret = cdce6214_configure(priv);
+	if (ret)
+		return ret;
+
+	for_each_child_of_node(dev->of_node, child) {
+		ret = cdce6214_parse_subnode(priv, child);
+		if (ret)
+			return ret;
+	}
+
+	ret = cdce6214_clk_register(priv);
+	if (ret)
+		return ret;
+
+	ret = devm_of_clk_add_hw_provider(dev, cdce6214_of_clk_get, priv);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id cdce6214_ids[] = {
+	{
+		.compatible = "ti,cdce6214-24mhz",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, cdce6214_ids);
+
+static struct i2c_driver cdce6214_driver = {
+	.driver = {
+		.name = "cdce6214",
+		.of_match_table = cdce6214_ids,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.probe = cdce6214_probe,
+};
+module_i2c_driver(cdce6214_driver);
+
+MODULE_AUTHOR("Alvin Šipraga <alsi@bang-olufsen.dk>");
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
+MODULE_DESCRIPTION("TI CDCE6214 driver");
+MODULE_LICENSE("GPL");
diff --git a/include/dt-bindings/clock/ti,cdce6214.h b/include/dt-bindings/clock/ti,cdce6214.h
new file mode 100644
index 0000000000000..1b41060896cc3
--- /dev/null
+++ b/include/dt-bindings/clock/ti,cdce6214.h
@@ -0,0 +1,24 @@
+#ifndef _DT_BINDINGS_CLK_TI_CDCE6214_H
+#define _DT_BINDINGS_CLK_TI_CDCE6214_H
+
+/*
+ * primary/secondary inputs. Not registered as clocks, but used
+ * as reg properties for the subnodes specifying the input properties
+ */
+#define CDCE6214_CLK_PRIREF	0
+#define CDCE6214_CLK_SECREF	1
+
+/*
+ * Clock indices for the clocks provided by the CDCE6214. Also used
+ * as reg properties for the subnodes specifying the output properties
+ */
+#define CDCE6214_CLK_OUT0	2
+#define CDCE6214_CLK_OUT1	3
+#define CDCE6214_CLK_OUT2	4
+#define CDCE6214_CLK_OUT3	5
+#define CDCE6214_CLK_OUT4	6
+#define CDCE6214_CLK_PLL	7
+#define CDCE6214_CLK_PSA	8
+#define CDCE6214_CLK_PSB	9
+
+#endif /* _DT_BINDINGS_CLK_TI_CDCE6214_H */

-- 
2.39.5


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

* [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding
  2025-04-08 12:00 [PATCH 0/3] clk: add support for TI CDCE6214 Sascha Hauer
  2025-04-08 12:00 ` [PATCH 1/3] clk: make determine_rate optional for non reparenting clocks Sascha Hauer
  2025-04-08 12:00 ` [PATCH 2/3] clk: add TI CDCE6214 clock driver Sascha Hauer
@ 2025-04-08 12:00 ` Sascha Hauer
  2025-04-08 14:27   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2025-04-08 12:00 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-clk, linux-kernel, devicetree, kernel, Alvin Šipraga,
	Sascha Hauer

The CDCE6214 is a Ultra-Low Power Clock Generator With One PLL, Four
Differential Outputs, Two Inputs, and Internal EEPROM. This patch adds
the device tree binding for this chip.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/clock/ti,cdce6214.yaml     | 157 +++++++++++++++++++++
 1 file changed, 157 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml b/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml
new file mode 100644
index 0000000000000..63e6c9d9b1771
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml
@@ -0,0 +1,157 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti,cdce6214.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI CDCE6214 programmable clock generator with PLL
+
+maintainers:
+  - Sascha Hauer <s.hauer@pengutronix.de>
+
+description: |
+  Ultra-Low Power Clock Generator With One PLL, Four Differential Outputs,
+  Two Inputs, and Internal EEPROM
+
+  - CDCE6214: https://www.ti.com/product/CDCE6214
+
+properties:
+  compatible:
+    enum:
+      - ti,cdce6214
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: priref
+      - const: secref
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  "#clock-cells":
+    const: 1
+
+patternProperties:
+  "^clk@[0-1]$":
+    type: object
+    description: |
+      optional child node that can be used to specify input pin parameters. The reg
+      properties match the CDCE6214_CLK_* defines.
+
+    additionalProperties: false
+
+    properties:
+      reg:
+        description:
+          clock input identifier.
+        minimum: 0
+        maximum: 1
+
+      ti,ref-xtal:
+        type: boolean
+        description: |
+          If true use input as XTAL input
+
+      ti,ref-lvcmos:
+        type: boolean
+        description: |
+          If true use input as LVCMOS input
+
+      ti,ref-diff:
+        type: boolean
+        description: |
+          If true use input as differential input
+
+  "^clk@[2-9]$":
+    type: object
+    description: |
+      optional child node that can be used to specify output pin parameters.  The reg
+      properties match the CDCE6214_CLK_* defines.
+
+    additionalProperties: false
+
+    properties:
+      reg:
+        description:
+          clock output identifier.
+        minimum: 2
+        maximum: 9
+
+      ti,lphcsl:
+        type: boolean
+        description: |
+          If true enable LP-HCSL output mode for this clock
+
+      ti,lvds:
+        type: boolean
+        description: |
+          If true enable LVDS output mode for this clock
+
+      ti,cmosp:
+        type: boolean
+        description: |
+          If true enable CMOSP output for this clock
+
+      ti,cmosn:
+        type: boolean
+        description: |
+          If true enable CMOSN output for this clock
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/ti,cdce6214.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        clock-generator@67 {
+            compatible = "ti,cdce6214";
+            reg = <0x67>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            #clock-cells = <1>;
+            clocks = <&clock_ref25m>;
+            clock-names = "priref";
+
+            clk@CDCE6214_CLK_SECREF {
+                reg = <CDCE6214_CLK_SECREF>;
+                ti,ref-xtal;
+            };
+
+            clk@CDCE6214_CLK_OUT1 {
+                reg = <CDCE6214_CLK_OUT1>;
+                ti,cmosp;
+                ti,cmosn;
+            };
+
+            clk@CDCE6214_CLK_OUT2 {
+                reg = <CDCE6214_CLK_OUT2>;
+                ti,lvds;
+            };
+
+            clk@CDCE6214_CLK_OUT4 {
+                reg = <CDCE6214_CLK_OUT4>;
+                ti,cmosp;
+                ti,cmosn;
+            };
+        };
+    };

-- 
2.39.5


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

* Re: [PATCH 2/3] clk: add TI CDCE6214 clock driver
  2025-04-08 12:00 ` [PATCH 2/3] clk: add TI CDCE6214 clock driver Sascha Hauer
@ 2025-04-08 13:43   ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2025-04-08 13:43 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: devicetree, linux-kernel, kernel, Alvin Šipraga, linux-clk

On Tue, Apr 08, 2025 at 02:00:23PM +0200, Sascha Hauer wrote:
> The CDCE6214 is a Ultra-Low Power Clock Generator With One PLL, Four
> Differential Outputs, Two Inputs, and Internal EEPROM. This patch adds
> a common clk framework driver for this chip.
> 
> - Two inputs (PRIREF and SECREF)
> - Programmable 8bit divider or x2 multiplier between input and PLL
> - 16b integer / 24bit fractional PLL
> - Two programmable /4, /5, /6 dividers after PLL (PSA/PSB)
> - Four outputs (OUT1-OUT4) with programmable 14b dividers,
>   muxable between PSA, PSB and PLL input
> - One output (OUT0) fed from PLL input
> 
> - PRIREF can be configured as LVCMOS or differential input
> - SECREF can be configured as LVCMOS, differential or oscillator input
> - OUT0 is a LVCMOS output
> - OUT1 and OUT4 can be configured as LVDS, LP-HCSL or LVCMOS outputs
> - OUT2 and OUT3 can be configured as LVDS or LP-HCSL outputs
> 
> All clocks are registered without parent rate propagation, so each of
> the clocks must be configured separately via device tree or consumer.
> 
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/clk/Kconfig                     |    7 +
>  drivers/clk/Makefile                    |    1 +
>  drivers/clk/clk-cdce6214.c              | 1105 +++++++++++++++++++++++++++++++
>  include/dt-bindings/clock/ti,cdce6214.h |   24 +
>  4 files changed, 1137 insertions(+)
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 713573b6c86c7..499fd610c0467 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -170,6 +170,13 @@ config COMMON_CLK_BM1880
>  	help
>  	  This driver supports the clocks on Bitmain BM1880 SoC.
>  
> +config COMMON_CLK_CDCE6214
> +	tristate "Clock driver for TI CDCE6214 clock synthesizer"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  This driver supports TI CDCE6214 programmable 1-PLL clock synthesizer.
> +
>  config COMMON_CLK_CDCE706
>  	tristate "Clock driver for TI CDCE706 clock synthesizer"
>  	depends on I2C
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index bf4bd45adc3a0..0f87b13b137b5 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
>  obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
>  obj-$(CONFIG_COMMON_CLK_BD718XX)	+= clk-bd718x7.o
>  obj-$(CONFIG_COMMON_CLK_BM1880)		+= clk-bm1880.o
> +obj-$(CONFIG_COMMON_CLK_CDCE6214)	+= clk-cdce6214.o
>  obj-$(CONFIG_COMMON_CLK_CDCE706)	+= clk-cdce706.o
>  obj-$(CONFIG_COMMON_CLK_CDCE925)	+= clk-cdce925.o
>  obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
> diff --git a/drivers/clk/clk-cdce6214.c b/drivers/clk/clk-cdce6214.c
> new file mode 100644
> index 0000000000000..a825cd71bb11b
> --- /dev/null
> +++ b/drivers/clk/clk-cdce6214.c
> @@ -0,0 +1,1105 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for the TI CDCE6214 clock generator
> + *
> + * Copyright (c) 2023 Alvin Šipraga <alsi@bang-olufsen.dk>
> + * Copyright (c) 2025 Sascha Hauer <s.hauer@pengutronix.de>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <dt-bindings/clock/ti,cdce6214.h>
> +
> +#define RO_I2C_A0			BIT(15)
> +#define RO_PDN_INPUT_SEL		BIT(14)
> +#define RO_GPIO4_DIR_SEL		BIT(13)
> +#define RO_GPIO1_DIR_SEL		BIT(12)
> +#define RO_ZDM_CLOCKSEL			BIT(10)
> +#define RO_ZDM_EN			BIT(8)
> +#define RO_SYNC				BIT(5)
> +#define RO_RECAL			BIT(4)
> +#define RO_RESETN_SOFT			BIT(3)
> +#define RO_SWRST			BIT(2)
> +#define RO_POWERDOWN			BIT(1)
> +#define RO_MODE				BIT(0)
> +
> +#define R1_GPIO4_INPUT_SEL		GENMASK(15, 12)
> +#define R1_GPIO3_INPUT_SEL		GENMASK(11, 8)
> +#define R1_GPIO2_INPUT_SEL		GENMASK(7, 4)
> +#define R1_GPIO1_INPUT_SEL		GENMASK(3, 0)
> +
> +#define R2_GPIO4_OUTPUT_SEL		GENMASK(9, 6)
> +#define R2_GPIO1_OUTPUT_SEL		GENMASK(5, 2)
> +#define R2_REFSEL_SW			GENMASK(1, 0)
> +
> +#define R3_DISABLE_CRC			BIT(13)
> +#define R3_UPDATE_CRC			BIT(12)
> +#define R3_NVMCOMMIT			BIT(11)
> +#define R3_REGCOMMIT			BIT(10)
> +#define R3_REGCOMMIT_PAGE		BIT(9)
> +#define R3_FREQ_DEC_REG			BIT(6)
> +#define R3_FREQ_INC_REG			BIT(5)
> +#define R3_FREQ_INC_DEC_REG_MODE	BIT(4)
> +#define R3_FREQ_INC_DEC_EN		BIT(3)
> +
> +#define R4_CH4_PD			BIT(7)
> +#define R4_CH3_PD			BIT(6)
> +#define R4_CH2_PD			BIT(5)
> +#define R4_CH1_PD			BIT(4)
> +#define R4_POST_EE_DLY			GENMASK(3, 0)
> +
> +#define R5_PLL_VCOBUFF_LDO_PD		BIT(8)
> +#define R5_PLL_VCO_LDO_PD		BIT(7)
> +#define R5_PLL_VCO_BUFF_PD		BIT(6)
> +#define R5_PLL_CP_LDO_PD		BIT(5)
> +#define R5_PLL_LOCKDET_PD		BIT(4)
> +#define R5_PLL_PSB_PD			BIT(3)
> +#define R5_PLL_PSA_PD			BIT(2)
> +#define R5_PLL_PFD_PD			BIT(1)
> +
> +#define R7_NVMCRCERR			BIT(5)
> +#define R7_LOCK_DET_S			BIT(1)
> +#define R7_LOCK_DET			BIT(0)
> +
> +#define R9_NVMLCRC			GENMASK(15, 0)
> +
> +#define R10_NVMSCRC			GENMASK(15, 0)
> +
> +#define R11_NVM_RD_ADDR			GENMASK(5, 0)
> +
> +#define R12_NVM_RD_DATA			GENMASK(15, 0)
> +
> +#define R13_NVM_WR_ADDR			GENMASK(5, 0)
> +
> +#define R14_NVM_WR_DATA			GENMASK(15, 0)
> +
> +#define R15_EE_LOCK			GENMASK(15, 12)
> +#define R15_CAL_MUTE			BIT(5)
> +
> +#define R24_IP_PRIREF_BUF_SEL		BIT(15)
> +#define R24_IP_XO_CLOAD			GENMASK(12, 8)
> +#define R24_IP_BIAS_SEL_XO		GENMASK(5, 2)
> +#define R24_IP_SECREF_BUF_SEL		GENMASK(1, 0)
> +#define R24_IP_SECREF_BUF_SEL_XTAL	0
> +#define R24_IP_SECREF_BUF_SEL_LVCMOS	1
> +#define R24_IP_SECREF_BUF_SEL_DIFF	2
> +
> +#define R25_IP_REF_TO_OUT4_EN		BIT(14)
> +#define R25_IP_REF_TO_OUT3_EN		BIT(13)
> +#define R25_IP_REF_TO_OUT2_EN		BIT(12)
> +#define R25_IP_REF_TO_OUT1_EN		BIT(11)
> +#define R25_IP_BYP_OUT0_EN		BIT(10)
> +#define R25_REF_CH_MUX			BIT(9)
> +#define R25_IP_RDIV			GENMASK(7, 0)
> +
> +#define R27_MASH_ORDER			GENMASK(1, 0)
> +
> +#define R30_PLL_NDIV			GENMASK(14, 0)
> +
> +#define R31_PLL_NUM_15_0		GENMASK(15, 0)
> +
> +#define R32_PLL_NUM_23_16		GENMASK(7, 0)
> +
> +#define R33_PLL_DEN_15_0		GENMASK(15, 0)
> +
> +#define R34_PLL_DEN_23_16		GENMASK(7, 0)
> +
> +#define R41_SSC_EN			BIT(15)
> +
> +#define R42_SSC_TYPE			BIT(5)
> +#define R42_SSC_SEL			GENMASK(3, 1)
> +
> +#define R43_FREQ_INC_DEC_DELTA		GENMASK(15, 0)
> +
> +#define R47_PLL_CP_DN			GENMASK(12, 7)
> +#define R47_PLL_PSB			GENMASK(6, 5)
> +#define R47_PLL_PSA			GENMASK(4, 3)
> +
> +#define R48_PLL_LF_RES			GENMASK(14, 11)
> +#define R48_PLL_CP_UP			GENMASK(5, 0)
> +
> +#define R49_PLL_LF_ZCAP			GENMASK(4, 0)
> +
> +#define R50_PLL_LOCKDET_WINDOW		GENMASK(10, 8)
> +
> +#define R51_PLL_PFD_DLY_EN		BIT(10)
> +#define R51_PLL_PFD_CTRL		BIT(6)
> +
> +#define R52_PLL_NCTRL_EN		BIT(6)
> +#define R52_PLL_CP_EN			BIT(3)
> +
> +#define R55_PLL_LF_3_PCTRIM		GENMASK(9, 8)
> +#define R55_PLL_LF_3_PRTRIM		GENMASK(7, 6)
> +
> +#define R56_CH1_MUX			GENMASK(15, 14)
> +#define R56_CH1_DIV			GENMASK(13, 0)
> +
> +#define R57_CH1_LPHCSL_EN		BIT(14)
> +#define R57_CH1_1P8VDET			BIT(12)
> +#define R57_CH1_GLITCHLESS_EN		BIT(9)
> +#define R57_CH1_SYNC_DELAY		GENMASK(8, 4)
> +#define R57_CH1_SYNC_EN			BIT(3)
> +#define R57_CH1_MUTE_SEL		BIT(1)
> +#define R57_CH1_MUTE			BIT(0)
> +
> +#define R59_CH1_LVDS_EN			BIT(15)
> +#define R59_CH1_CMOSN_EN		BIT(14)
> +#define R59_CH1_CMOSP_EN		BIT(13)
> +#define R59_CH1_CMOSN_POL		BIT(12)
> +#define R59_CH1_CMOSP_POL		BIT(11)
> +
> +#define R60_CH1_DIFFBUF_IBIAS_TRIM	GENMASK(15, 12)
> +#define R60_CH1_LVDS_CMTRIM_INC		GENMASK(11, 10)
> +#define R60_CH1_LVDS_CMTRIM_DEC		GENMASK(5, 4)
> +#define R60_CH1_CMOS_SLEW_RATE_CTRL	GENMASK(3, 0)
> +
> +#define R62_CH2_MUX			GENMASK(15, 14)
> +#define R62_CH2_DIV			GENMASK(13, 0)
> +
> +#define R63_CH2_LPHCSL_EN		BIT(13)
> +#define R63_CH2_1P8VDET			BIT(12)
> +#define R63_CH2_GLITCHLESS_EN		BIT(9)
> +#define R63_CH2_SYNC_DELAY		GENMASK(8, 4)
> +#define R63_CH2_SYNC_EN			BIT(3)
> +#define R63_CH2_MUTE_SEL		BIT(1)
> +#define R63_CH2_MUTE			BIT(0)
> +
> +#define R65_CH2_LVDS_CMTRIM_DEC		GENMASK(14, 13)
> +#define R65_CH2_LVDS_EN			BIT(11)
> +
> +#define R66_CH2_LVDS_CMTRIM_IN		GENMASK(5, 4)
> +#define R66_CH2_DIFFBUF_IBIAS_TRIM	GENMASK(3, 0)
> +
> +#define R67_CH3_MUX			GENMASK(15, 14)
> +#define R67_CH3_DIV			GENMASK(13, 0)
> +
> +#define R68_CH3_LPHCSL_EN		BIT(13)
> +#define R68_CH3_1P8VDET			BIT(12)
> +#define R68_CH3_GLITCHLESS_EN		BIT(9)
> +#define R68_CH3_SYNC_DELAY		GENMASK(8, 4)
> +#define R68_CH3_SYNC_EN			BIT(3)
> +#define R68_CH3_MUTE_SEL		BIT(1)
> +#define R68_CH3_MUTE			BIT(0)
> +
> +#define R70_CH3_LVDS_EN			BIT(11)
> +
> +#define R71_CH3_LVDS_CMTRIM_DEC		GENMASK(10, 9)
> +#define R71_CH3_LVDS_CMTRIM_INC		GENMASK(5, 4)
> +#define R71_CH3_DIFFBUF_IBIAS_TR	GENMASK(3, 0)
> +
> +#define R72_CH4_MUX			GENMASK(15, 14)
> +#define R72_CH4_DIV			GENMASK(13, 0)
> +
> +#define R73_CH4_LPHCSL_EN		BIT(13)
> +#define R73_CH4_1P8VDET			BIT(12)
> +#define R73_CH4_GLITCHLESS_EN		BIT(9)
> +#define R73_CH4_SYNC_DELAY		GENMASK(8, 4)
> +#define R73_CH4_SYNC_EN			BIT(3)
> +#define R73_CH4_MUTE_SEL		BIT(1)
> +#define R73_CH4_MUTE			BIT(0)
> +
> +#define R75_CH4_LVDS_EN			BIT(15)
> +#define R75_CH4_CMOSP_EN		BIT(14)
> +#define R75_CH4_CMOSN_EN		BIT(13)
> +#define R75_CH4_CMOSP_POL		BIT(12)
> +#define R75_CH4_CMOSN_POL		BIT(11)
> +
> +#define R76_CH4_DIFFBUF_IBIAS_TRIM	GENMASK(9, 6)
> +#define R76_CH4_LVDS_CMTRIM_IN		GENMASK(5, 4)
> +#define R76_CH4_CMOS_SLEW_RATE_CTRL	GENMASK(3, 0)
> +
> +#define R77_CH4_LVDS_CMTRIM_DEC		GENMASK(1, 0)
> +
> +#define R78_CH0_EN			BIT(12)
> +
> +#define R79_SAFETY_1P8V_MODE		BIT(9)
> +#define R79_CH0_CMOS_SLEW_RATE_CTRL	GENMASK(3, 0)
> +
> +#define R81_PLL_LOCK_MASK		BIT(3)
> +
> +#define CDCE6214_VCO_MIN 2335000000
> +#define CDCE6214_VCO_MAX 2625000000
> +#define CDCE6214_DENOM_DEFAULT (1 << 24)
> +
> +static char *clk_names[] = {
> +	[CDCE6214_CLK_PRIREF] = "priref",
> +	[CDCE6214_CLK_SECREF] = "secref",
> +	[CDCE6214_CLK_OUT0] = "out0",
> +	[CDCE6214_CLK_OUT1] = "out1",
> +	[CDCE6214_CLK_OUT2] = "out2",
> +	[CDCE6214_CLK_OUT3] = "out3",
> +	[CDCE6214_CLK_OUT4] = "out4",
> +	[CDCE6214_CLK_PLL] = "pll",
> +	[CDCE6214_CLK_PSA] = "psa",
> +	[CDCE6214_CLK_PSB] = "psb",
> +};
> +
> +#define CDCE6214_NUM_CLOCKS	ARRAY_SIZE(clk_names)
> +
> +struct cdce6214;
> +
> +struct cdce6214_clock {
> +	struct clk_hw hw;
> +	struct cdce6214 *priv;
> +	int index;
> +};
> +
> +struct cdce6214_config {
> +	const struct reg_default *reg_default;
> +	int reg_default_size;
> +};

This is unused. I'll remove it next round.

> +static const struct of_device_id cdce6214_ids[] = {
> +	{
> +		.compatible = "ti,cdce6214-24mhz",

Should be ti,cdce6214. Will fix next round.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding
  2025-04-08 12:00 ` [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding Sascha Hauer
@ 2025-04-08 14:27   ` Krzysztof Kozlowski
  2025-04-08 15:02     ` Sascha Hauer
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-08 14:27 UTC (permalink / raw)
  To: Sascha Hauer, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-clk, linux-kernel, devicetree, kernel, Alvin Šipraga

On 08/04/2025 14:00, Sascha Hauer wrote:
> +

A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> +properties:
> +  compatible:
> +    enum:
> +      - ti,cdce6214
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: priref
> +      - const: secref

So one input is optional?

> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  "#clock-cells":

Use consistent quotes, either ' or "

> +    const: 1
> +
> +patternProperties:
> +  "^clk@[0-1]$":
> +    type: object
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      optional child node that can be used to specify input pin parameters. The reg
> +      properties match the CDCE6214_CLK_* defines.
> +
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          clock input identifier.
> +        minimum: 0
> +        maximum: 1
> +
> +      ti,ref-xtal:
> +        type: boolean
> +        description: |
> +          If true use input as XTAL input
> +
> +      ti,ref-lvcmos:
> +        type: boolean
> +        description: |
> +          If true use input as LVCMOS input
> +
> +      ti,ref-diff:
> +        type: boolean
> +        description: |
> +          If true use input as differential input

So these three are an enum string.

> +
> +  "^clk@[2-9]$":
> +    type: object
> +    description: |
> +      optional child node that can be used to specify output pin parameters.  The reg
> +      properties match the CDCE6214_CLK_* defines.
> +
> +    additionalProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          clock output identifier.
> +        minimum: 2
> +        maximum: 9
> +
> +      ti,lphcsl:
> +        type: boolean
> +        description: |
> +          If true enable LP-HCSL output mode for this clock
> +
> +      ti,lvds:
> +        type: boolean
> +        description: |
> +          If true enable LVDS output mode for this clock
> +
> +      ti,cmosp:
> +        type: boolean
> +        description: |
> +          If true enable CMOSP output for this clock
> +
> +      ti,cmosn:
> +        type: boolean
> +        description: |
> +          If true enable CMOSN output for this clock

Looks the same here. Anyway having these as subnodes is too much. You
have fixed number of clocks, so you need one or two array properties in
top-level.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - "#clock-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/ti,cdce6214.h>

This file does not exist. Something is odd in this example.

> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        clock-generator@67 {
> +            compatible = "ti,cdce6214";
> +            reg = <0x67>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            #clock-cells = <1>;
> +            clocks = <&clock_ref25m>;
> +            clock-names = "priref";
> +
> +            clk@CDCE6214_CLK_SECREF {

That's not a valid unit address. Use simple numbers, see DT spec and DTS
coding style.


Best regards,
Krzysztof

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

* Re: [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding
  2025-04-08 14:27   ` Krzysztof Kozlowski
@ 2025-04-08 15:02     ` Sascha Hauer
  2025-04-08 15:11       ` Krzysztof Kozlowski
  2025-04-11 15:15       ` Rob Herring
  0 siblings, 2 replies; 10+ messages in thread
From: Sascha Hauer @ 2025-04-08 15:02 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-kernel, devicetree, kernel,
	Alvin Šipraga

On Tue, Apr 08, 2025 at 04:27:23PM +0200, Krzysztof Kozlowski wrote:
> On 08/04/2025 14:00, Sascha Hauer wrote:
> > +
> 
> A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 
> 
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,cdce6214
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    items:
> > +      - const: priref
> > +      - const: secref
> 
> So one input is optional?

The chip has two clock inputs and to be operational it needs at least
one clock, could be priref or secref or both.

Is there a proper way to express this situation?


> > +  "^clk@[2-9]$":
> > +    type: object
> > +    description: |
> > +      optional child node that can be used to specify output pin parameters.  The reg
> > +      properties match the CDCE6214_CLK_* defines.
> > +
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        description:
> > +          clock output identifier.
> > +        minimum: 2
> > +        maximum: 9
> > +
> > +      ti,lphcsl:
> > +        type: boolean
> > +        description: |
> > +          If true enable LP-HCSL output mode for this clock
> > +
> > +      ti,lvds:
> > +        type: boolean
> > +        description: |
> > +          If true enable LVDS output mode for this clock
> > +
> > +      ti,cmosp:
> > +        type: boolean
> > +        description: |
> > +          If true enable CMOSP output for this clock
> > +
> > +      ti,cmosn:
> > +        type: boolean
> > +        description: |
> > +          If true enable CMOSN output for this clock
> 
> Looks the same here. Anyway having these as subnodes is too much. You
> have fixed number of clocks, so you need one or two array properties in
> top-level.

There are several properties I haven't yet modeled, like

- 1.8V / 2.5V output
- sync_delay
- LVDS common-mode trim increment/decrement
- differential buffer BIAS trim
- slew rate
- BIAS current setting for XTAL mode
- load capacity for XTAL mode

I don't know which of them will ever be supported, but I thought having a
node per pin would add a natural place to add these properties. Do you
still think arrays would be more appropriate?

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - "#clock-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/ti,cdce6214.h>
> 
> This file does not exist. Something is odd in this example.

It is added in the driver patch. Should it come with the binding patch
instead?

> 
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        clock-generator@67 {
> > +            compatible = "ti,cdce6214";
> > +            reg = <0x67>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            #clock-cells = <1>;
> > +            clocks = <&clock_ref25m>;
> > +            clock-names = "priref";
> > +
> > +            clk@CDCE6214_CLK_SECREF {
> 
> That's not a valid unit address. Use simple numbers, see DT spec and DTS
> coding style.

CDCE6214_CLK_SECREF is a macro added in dt-bindings/clock/ti,cdce6214.h
and it expands to a simple number (1 in this case). While I haven't
found any examples of someone using macros for the unit address / reg
property I thought I'd give it a try as it nicely shows how it is used.

I can switch to plain numbers if you prefer that though.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding
  2025-04-08 15:02     ` Sascha Hauer
@ 2025-04-08 15:11       ` Krzysztof Kozlowski
  2025-04-11 15:15       ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-08 15:11 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-clk, linux-kernel, devicetree, kernel,
	Alvin Šipraga

On 08/04/2025 17:02, Sascha Hauer wrote:
>>> +  clock-names:
>>> +    minItems: 1
>>> +    items:
>>> +      - const: priref
>>> +      - const: secref
>>
>> So one input is optional?
> 
> The chip has two clock inputs and to be operational it needs at least
> one clock, could be priref or secref or both.
> 
> Is there a proper way to express this situation?

No, this is fine, I just wanted to be sure that's the intention.

> 
> 
>>> +  "^clk@[2-9]$":
>>> +    type: object
>>> +    description: |
>>> +      optional child node that can be used to specify output pin parameters.  The reg
>>> +      properties match the CDCE6214_CLK_* defines.
>>> +
>>> +    additionalProperties: false
>>> +
>>> +    properties:
>>> +      reg:
>>> +        description:
>>> +          clock output identifier.
>>> +        minimum: 2
>>> +        maximum: 9
>>> +
>>> +      ti,lphcsl:
>>> +        type: boolean
>>> +        description: |
>>> +          If true enable LP-HCSL output mode for this clock
>>> +
>>> +      ti,lvds:
>>> +        type: boolean
>>> +        description: |
>>> +          If true enable LVDS output mode for this clock
>>> +
>>> +      ti,cmosp:
>>> +        type: boolean
>>> +        description: |
>>> +          If true enable CMOSP output for this clock
>>> +
>>> +      ti,cmosn:
>>> +        type: boolean
>>> +        description: |
>>> +          If true enable CMOSN output for this clock
>>
>> Looks the same here. Anyway having these as subnodes is too much. You
>> have fixed number of clocks, so you need one or two array properties in
>> top-level.
> 
> There are several properties I haven't yet modeled, like
> 
> - 1.8V / 2.5V output
> - sync_delay
> - LVDS common-mode trim increment/decrement
> - differential buffer BIAS trim
> - slew rate
> - BIAS current setting for XTAL mode
> - load capacity for XTAL mode
> 
> I don't know which of them will ever be supported, but I thought having a
> node per pin would add a natural place to add these properties. Do you
> still think arrays would be more appropriate?

Binding is supposed to be complete. If you send incomplete, you get
review like that.

Several of these look like pinctrl thus maybe this should be pin
controller as well. It's not exactly GPIO, but still configuring
specific functions and pin characteristics is the exact job of pinctrl.

> 
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - "#clock-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/ti,cdce6214.h>
>>
>> This file does not exist. Something is odd in this example.
> 
> It is added in the driver patch. Should it come with the binding patch
> instead?

Yes, because it is a binding.

> 
>>
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        clock-generator@67 {
>>> +            compatible = "ti,cdce6214";
>>> +            reg = <0x67>;
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +            #clock-cells = <1>;
>>> +            clocks = <&clock_ref25m>;
>>> +            clock-names = "priref";
>>> +
>>> +            clk@CDCE6214_CLK_SECREF {
>>
>> That's not a valid unit address. Use simple numbers, see DT spec and DTS
>> coding style.
> 
> CDCE6214_CLK_SECREF is a macro added in dt-bindings/clock/ti,cdce6214.h
> and it expands to a simple number (1 in this case). While I haven't
> found any examples of someone using macros for the unit address / reg
> property I thought I'd give it a try as it nicely shows how it is used.
> 
> I can switch to plain numbers if you prefer that though.

We don't encode addresses as headers in DTS (with few exception), so
definitely not as a binding. It just does not bind anything (in the ABI).

Best regards,
Krzysztof

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

* Re: [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding
  2025-04-08 15:02     ` Sascha Hauer
  2025-04-08 15:11       ` Krzysztof Kozlowski
@ 2025-04-11 15:15       ` Rob Herring
  2025-04-14 15:07         ` Sascha Hauer
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2025-04-11 15:15 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Krzysztof Kozlowski, Michael Turquette, Stephen Boyd,
	Krzysztof Kozlowski, Conor Dooley, linux-clk, linux-kernel,
	devicetree, kernel, Alvin Šipraga

On Tue, Apr 08, 2025 at 05:02:21PM +0200, Sascha Hauer wrote:
> On Tue, Apr 08, 2025 at 04:27:23PM +0200, Krzysztof Kozlowski wrote:
> > On 08/04/2025 14:00, Sascha Hauer wrote:
> > > +
> > 
> > A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> > prefix is already stating that these are bindings.
> > See also:
> > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> > 
> > 
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - ti,cdce6214
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    minItems: 1
> > > +    items:
> > > +      - const: priref
> > > +      - const: secref
> > 
> > So one input is optional?
> 
> The chip has two clock inputs and to be operational it needs at least
> one clock, could be priref or secref or both.
> 
> Is there a proper way to express this situation?

If I understand correctly that only 'secref' is possible then you want:

items:
  - enum: [ priref, secref ]
  - const: secref

(By default, entries have to be unique, so that eliminates 'secref' in 
both)

> 
> 
> > > +  "^clk@[2-9]$":
> > > +    type: object
> > > +    description: |
> > > +      optional child node that can be used to specify output pin parameters.  The reg
> > > +      properties match the CDCE6214_CLK_* defines.
> > > +
> > > +    additionalProperties: false
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description:
> > > +          clock output identifier.
> > > +        minimum: 2
> > > +        maximum: 9
> > > +
> > > +      ti,lphcsl:
> > > +        type: boolean
> > > +        description: |
> > > +          If true enable LP-HCSL output mode for this clock
> > > +
> > > +      ti,lvds:
> > > +        type: boolean
> > > +        description: |
> > > +          If true enable LVDS output mode for this clock
> > > +
> > > +      ti,cmosp:
> > > +        type: boolean
> > > +        description: |
> > > +          If true enable CMOSP output for this clock
> > > +
> > > +      ti,cmosn:
> > > +        type: boolean
> > > +        description: |
> > > +          If true enable CMOSN output for this clock
> > 
> > Looks the same here. Anyway having these as subnodes is too much. You
> > have fixed number of clocks, so you need one or two array properties in
> > top-level.
> 
> There are several properties I haven't yet modeled, like
> 
> - 1.8V / 2.5V output
> - sync_delay
> - LVDS common-mode trim increment/decrement
> - differential buffer BIAS trim
> - slew rate
> - BIAS current setting for XTAL mode
> - load capacity for XTAL mode
> 
> I don't know which of them will ever be supported, but I thought having a
> node per pin would add a natural place to add these properties. Do you
> still think arrays would be more appropriate?

Assuming they are connected to something in DT (if not, why care), you 
could add a flags cell so the consumer side can define what they need.

> 
> > 
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - "#clock-cells"
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/ti,cdce6214.h>
> > 
> > This file does not exist. Something is odd in this example.
> 
> It is added in the driver patch. Should it come with the binding patch
> instead?

Yes.

Rob

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

* Re: [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding
  2025-04-11 15:15       ` Rob Herring
@ 2025-04-14 15:07         ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2025-04-14 15:07 UTC (permalink / raw)
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Michael Turquette, Stephen Boyd,
	Krzysztof Kozlowski, Conor Dooley, linux-clk, linux-kernel,
	devicetree, kernel, Alvin Šipraga

On Fri, Apr 11, 2025 at 10:15:52AM -0500, Rob Herring wrote:
> On Tue, Apr 08, 2025 at 05:02:21PM +0200, Sascha Hauer wrote:
> > On Tue, Apr 08, 2025 at 04:27:23PM +0200, Krzysztof Kozlowski wrote:
> > > On 08/04/2025 14:00, Sascha Hauer wrote:
> > > > +
> > > 
> > > A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> > > prefix is already stating that these are bindings.
> > > See also:
> > > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> > > 
> > > 
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - ti,cdce6214
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +
> > > > +  clock-names:
> > > > +    minItems: 1
> > > > +    items:
> > > > +      - const: priref
> > > > +      - const: secref
> > > 
> > > So one input is optional?
> > 
> > The chip has two clock inputs and to be operational it needs at least
> > one clock, could be priref or secref or both.
> > 
> > Is there a proper way to express this situation?
> 
> If I understand correctly that only 'secref' is possible then you want:
> 
> items:
>   - enum: [ priref, secref ]
>   - const: secref
> 
> (By default, entries have to be unique, so that eliminates 'secref' in 
> both)

Valid combinations shall be:

- only secref
- only priref
- priref and secref.

Background is: The CDCE6214 has a mux on these inputs, so you can select
one of both in software. In our case only one input is actually used,
but which one it is depends on the board designer. There are likely
usecases in which both inputs are connected to different clocks so you
want to decide during runtime depending on usecase.

> > > Looks the same here. Anyway having these as subnodes is too much. You
> > > have fixed number of clocks, so you need one or two array properties in
> > > top-level.
> > 
> > There are several properties I haven't yet modeled, like
> > 
> > - 1.8V / 2.5V output
> > - sync_delay
> > - LVDS common-mode trim increment/decrement
> > - differential buffer BIAS trim
> > - slew rate
> > - BIAS current setting for XTAL mode
> > - load capacity for XTAL mode
> > 
> > I don't know which of them will ever be supported, but I thought having a
> > node per pin would add a natural place to add these properties. Do you
> > still think arrays would be more appropriate?
> 
> Assuming they are connected to something in DT (if not, why care), you 
> could add a flags cell so the consumer side can define what they need.

Let's see if I got that right. Do you mean something like:

	cdce6214: clock-generator@67 {
		compatible = "ti,cdce6214";
		#clock-cells = <2>;
		...
	};

	some-consumer {
		clocks = <&cdce6214 4 CDCE6214_SLEW_RATE_MAX | CDCE6214_BIAS(14)>;
	};

That would work for the outputs. What about the inputs?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

end of thread, other threads:[~2025-04-14 15:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-08 12:00 [PATCH 0/3] clk: add support for TI CDCE6214 Sascha Hauer
2025-04-08 12:00 ` [PATCH 1/3] clk: make determine_rate optional for non reparenting clocks Sascha Hauer
2025-04-08 12:00 ` [PATCH 2/3] clk: add TI CDCE6214 clock driver Sascha Hauer
2025-04-08 13:43   ` Sascha Hauer
2025-04-08 12:00 ` [PATCH 3/3] dt-bindings: clock: add TI CDCE6214 binding Sascha Hauer
2025-04-08 14:27   ` Krzysztof Kozlowski
2025-04-08 15:02     ` Sascha Hauer
2025-04-08 15:11       ` Krzysztof Kozlowski
2025-04-11 15:15       ` Rob Herring
2025-04-14 15:07         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).