[PATCH 0/2] clk: si5351: Add phase offset for clock output

[PATCH 0/2] clk: si5351: Add phase offset for clock output

[Jens Renner]

This patch series adds a phase offset feature to the SI5351 clock
outputs. This is necessary to generate differential clocks or
arbitrary phase shifts with respect to other clock outputs.

Patch 1 adds the DT documentation to configure the phase offset.

Patch 2 implements the phase offset feature by adding *_set_phase()
functions to the multisynth and clockout stages.

Tested on a Xilinx Zynq (ARM Cortex-A9) board with Si5351A-B-GT for
several frequencies and phase offsets.

Jens Renner (2):
  clk: si5351: Add DT property for phase offset
  clk: si5351: Add clock output phase offset

 .../bindings/clock/silabs,si5351.txt          |  10 +-
 drivers/clk/clk-si5351.c                      | 234 +++++++++++++++++-
 drivers/clk/clk-si5351.h                      |   1 +
 include/linux/platform_data/si5351.h          |   2 +
 4 files changed, 243 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH 1/2] clk: si5351: Add DT property for phase offset

[Jens Renner]

Add optional output clock DT property "clock-phase" to configure the
phase offset in degrees with respect to other clock outputs.
Add missing description for related optional output clock DT property
"clock-frequency".

Signed-off-by: Jens Renner <renner@efe-gmbh.de>
---
 .../devicetree/bindings/clock/silabs,si5351.txt        | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
index 8fe6f80afade..62adf0d0874b 100644
--- a/Documentation/devicetree/bindings/clock/silabs,si5351.txt
+++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
@@ -50,11 +50,17 @@ Optional child node properties:
   divider.
 - silabs,pll-master: boolean, multisynth can change pll frequency.
 - silabs,pll-reset: boolean, clock output can reset its pll.
-- silabs,disable-state : clock output disable state, shall be
+- silabs,disable-state: clock output disable state, shall be
   0 = clock output is driven LOW when disabled
   1 = clock output is driven HIGH when disabled
   2 = clock output is FLOATING (HIGH-Z) when disabled
   3 = clock output is NEVER disabled
+- clock-frequency: integer in Hz, output frequency to generate (2500-200000000)
+  This defines the output frequency set during boot. It can be reprogrammed
+  duing runtime through the common clock framework.
+- clock-phase: integer, phase shift in degrees (0-359), using the multisynth
+  initial phase offset register (depends on the clock source / output ratio)
+  and the clock output inverter (180 deg. only).
 
 ==Example==
 
@@ -111,7 +117,7 @@ i2c-master-node {
 			silabs,drive-strength = <4>;
 			silabs,multisynth-source = <1>;
 			silabs,clock-source = <0>;
-			pll-master;
+			silabs,pll-master;
 		};
 
 		/*
-- 
2.33.0

[PATCH 2/2] clk: si5351: Add clock output phase offset

[Jens Renner]

Add phase offsets for individual clock outputs with respect to other
clock outputs. This allows to generate differential signals (via simple
clock output inversion) as well as arbitrary phase shifts in steps of
T_vco/4. The achievable phase offset range and resolution at the clock
output depends on the ratios of f_vco / f_ms and f_ms / f_clk.

Because phase offsets are not getting propagated through the clock
subsystem, the implementation takes care of this and converts the clock
output phase offset to the parent multisynth and VCO frequencies.

Tested on a Xilinx Zynq board with a SI5351A-B-GT (MSOP-10) for various
frequencies (up to 128 MHz) and phase offsets (90, 180, 270 deg.).

Signed-off-by: Jens Renner <renner@efe-gmbh.de>
---
 drivers/clk/clk-si5351.c             | 234 ++++++++++++++++++++++++++-
 drivers/clk/clk-si5351.h             |   1 +
 include/linux/platform_data/si5351.h |   2 +
 3 files changed, 235 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index 57e4597cdf4c..573d8a6d4c72 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -771,6 +771,10 @@ static int si5351_msynth_set_rate(struct clk_hw *hw, unsigned long rate,
 		si5351_set_bits(hwdata->drvdata, reg + 2,
 				SI5351_OUTPUT_CLK_DIVBY4,
 				(divby4) ? SI5351_OUTPUT_CLK_DIVBY4 : 0);
+		/* clear existing phase offset */
+		reg = si5351_set_bits(hwdata->drvdata,
+				SI5351_CLK0_PHASE_OFFSET + hwdata->num,
+				SI5351_CLK_PHASE_OFFSET_MASK, 0);
 		si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
 			SI5351_CLK_INTEGER_MODE,
 			(hwdata->params.p2 == 0) ? SI5351_CLK_INTEGER_MODE : 0);
@@ -785,12 +789,107 @@ static int si5351_msynth_set_rate(struct clk_hw *hw, unsigned long rate,
 	return 0;
 }
 
+/*
+ * Initial phase offset for MS0..5 can be set in 127 steps of T_vco/4.
+ * With a minimum MS divider a = 6, a theoretical maximum phase shift of
+ * ph_ms,max = 127 * 90 deg. / 6 = 1905 deg. is possible at the MS output.
+ *
+ * For a maximum R divider of 128 though, CLKOUT could request a phase shift
+ * from the MS of up to 128 * 360 deg. = 46080 deg.
+ *
+ * (clk_set_phase() applies a limit of degrees %= 360 which means CLKOUT phase
+ * offsets will be restricted to ph_clk < 180,..,3 deg. for RDIV = 2,..,128
+ * when not calling si5351_msynth_set_phase() directly!)
+ */
+static int si5351_msynth_set_phase(struct clk_hw *hw, int degrees)
+{
+	struct si5351_hw_data *hwdata =
+		container_of(hw, struct si5351_hw_data, hw);
+	struct clk_hw *parent_hw;
+	unsigned long parent_rate, rate;
+	unsigned int phoff, act_degrees, tmp;
+	bool fractmode;
+
+	/*
+	 * To leverage the full phase offset range for high ratios of
+	 * f_vco/f_clk, set the theoretical limit to ph_ms,max < 1905 deg.
+	 * The later calculation could still fail for large phase-divider
+	 * products (i.e. ph_ms * a = ph_ms * f_vco/f_ms > ph_vco,max).
+	 */
+	if ((degrees < 0) || (degrees >= 127*90/SI5351_MULTISYNTH_A_MIN) ||
+	    (hwdata->num > 5)) {
+		dev_err(&hwdata->drvdata->client->dev,
+			"Phase offset of %d degrees is not allowed for %s.\n",
+			degrees, clk_hw_get_name(hw));
+		return -EINVAL;
+	}
+
+	/*
+	 * Requested phase offset refers to multisynth output (ph_ms).
+	 * CLKx_PHOFF[6:0] = round(t_off * 4 * f_vco)
+	 * Convert phase offset for f_ms to steps of 4 x f_vco:
+	 * ph_offs = ph_ms * (a + b/c) / 90 = ph_ms * (f_vco / f_ms) / 90
+	 */
+	parent_hw = clk_hw_get_parent(hw);
+	parent_rate = clk_get_rate(parent_hw->clk);
+	rate = clk_get_rate(hw->clk);
+
+	tmp = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(parent_rate, degrees), rate);
+	phoff = tmp/90;
+
+	if (phoff > 127) {
+		dev_err(&hwdata->drvdata->client->dev,
+			"Phase offset of %d degrees (%d steps) not possible for chosen rates of %s.\n",
+			degrees, phoff, clk_hw_get_name(hw));
+		return -EINVAL;
+	}
+
+	/*
+	 * Fractional mode should be used for phase offsets, but works only
+	 * for dividers a + b/c >= 8! (P1 = 128 * a + floor(128*b/c) - 512).
+	 * (For RDIVBY4 = 11b, P1 = 0, P2 = 0, P3 = 1.)
+	 * Hence, use fract. mode for P2 != 0, or P1 >= 512 with phase offset.
+	 */
+	fractmode = ((hwdata->params.p2 != 0) ||
+		     ((phoff > 0) && (hwdata->params.p1 >= 512)));
+
+	si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
+		SI5351_CLK_INTEGER_MODE,
+		(fractmode) ? 0 : SI5351_CLK_INTEGER_MODE);
+
+	/*
+	 * For a MS rate f_ms > 112.5 MHz, only dividers a < 8 are possible and
+	 * fractional mode cannot be used. Issue a warning and continue.
+	 * (Tested & works for f_ms = 128 MHz, a = 7, and ph_vco = 7 * 90 deg.)
+	 */
+	if ((phoff > 0) && !fractmode) {
+		dev_warn(&hwdata->drvdata->client->dev,
+			 "Fract. mode cannot be set for divider of %s. Continue with int. mode.\n",
+			 clk_hw_get_name(hw));
+	}
+
+	si5351_set_bits(hwdata->drvdata,
+			SI5351_CLK0_PHASE_OFFSET + hwdata->num,
+			SI5351_CLK_PHASE_OFFSET_MASK, phoff);
+
+	act_degrees = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(90 * phoff, rate),
+					    parent_rate);
+
+	dev_dbg(&hwdata->drvdata->client->dev,
+		"%s - %s: degrees = %d, actual = %d, phoff steps = %u, mode = %c. PLL needs reset!\n",
+		__func__, clk_hw_get_name(hw),
+		degrees, act_degrees, phoff, (fractmode) ? 'f' : 'i');
+
+	return 0;
+}
+
 static const struct clk_ops si5351_msynth_ops = {
 	.set_parent = si5351_msynth_set_parent,
 	.get_parent = si5351_msynth_get_parent,
 	.recalc_rate = si5351_msynth_recalc_rate,
 	.round_rate = si5351_msynth_round_rate,
 	.set_rate = si5351_msynth_set_rate,
+	.set_phase = si5351_msynth_set_phase,
 };
 
 /*
@@ -952,10 +1051,11 @@ static void si5351_clkout_unprepare(struct clk_hw *hw)
 	struct si5351_hw_data *hwdata =
 		container_of(hw, struct si5351_hw_data, hw);
 
-	si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
-			SI5351_CLK_POWERDOWN, SI5351_CLK_POWERDOWN);
+	/* disable output first, then power down output driver */
 	si5351_set_bits(hwdata->drvdata, SI5351_OUTPUT_ENABLE_CTRL,
 			(1 << hwdata->num), (1 << hwdata->num));
+	si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
+			SI5351_CLK_POWERDOWN, SI5351_CLK_POWERDOWN);
 }
 
 static u8 si5351_clkout_get_parent(struct clk_hw *hw)
@@ -1124,6 +1224,10 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
 				rdiv << SI5351_OUTPUT_CLK_DIV_SHIFT);
 	}
 
+	/* clear existing clock output inversion */
+	si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
+			SI5351_CLK_INVERT, 0);
+
 	/* powerup clkout */
 	si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
 			SI5351_CLK_POWERDOWN, 0);
@@ -1136,6 +1240,41 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
 	return 0;
 }
 
+/*
+ * CLKOUT0..7 can be inverted to create differential signals or aid phase
+ * shifting, so allow only phase offsets of multiples of 180 deg.
+ */
+static int si5351_clkout_set_phase(struct clk_hw *hw, int degrees)
+{
+	struct si5351_hw_data *hwdata =
+		container_of(hw, struct si5351_hw_data, hw);
+	bool invert;
+
+	/* sanity check */
+	degrees %= 360;
+	if (degrees < 0)
+		degrees += 360;
+
+	if ((degrees % 180) != 0) {
+		dev_err(&hwdata->drvdata->client->dev,
+			"Phase offset of %d deg. is not allowed for %s.\n",
+			degrees, clk_hw_get_name(hw));
+		return -EINVAL;
+	}
+
+	invert = (degrees == 180) ? true : false;
+
+	si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num,
+			SI5351_CLK_INVERT, (invert) ? SI5351_CLK_INVERT : 0);
+
+	dev_dbg(&hwdata->drvdata->client->dev,
+		"%s - %s: clock output inverter %s\n",
+		__func__, clk_hw_get_name(hw),
+		(invert) ? "on (180 deg.)" : "off (0 deg.)");
+
+	return 0;
+}
+
 static const struct clk_ops si5351_clkout_ops = {
 	.prepare = si5351_clkout_prepare,
 	.unprepare = si5351_clkout_unprepare,
@@ -1144,8 +1283,73 @@ static const struct clk_ops si5351_clkout_ops = {
 	.recalc_rate = si5351_clkout_recalc_rate,
 	.round_rate = si5351_clkout_round_rate,
 	.set_rate = si5351_clkout_set_rate,
+	.set_phase = si5351_clkout_set_phase,
 };
 
+/*
+ * Phase offset calculation except trivial 0 / 180 deg. shift (i.e. inverting
+ * CLKOUT) depends on the CLKOUT / MS / PLL frequency ratio.
+ *
+ * Maximum phase offset at the clock output:
+ *   ph_clk,max = ph_vco,max * f_clk / f_vco = 127 * 90 deg. * f_clk / f_vco
+ *
+ * According to the documentation of clk_set_phase(), phase shift changes won't
+ * get propagated through the clock tree hierarchy.
+ *
+ * This function can be called with the desired clock output phase offset (as
+ * specified in the DTS) and takes care of the synthesis stages and dividers.
+ */
+static int _si5351_clk_set_phase(struct clk_hw *hw, int degrees)
+{
+	struct si5351_hw_data *hwdata =
+		container_of(hw, struct si5351_hw_data, hw);
+	struct clk_hw *parent_hw;
+	unsigned long rate, parent_rate;
+	int parent_degrees, ret;
+	bool invert = false;
+
+	/* sanity check */
+	degrees %= 360;
+	if (degrees < 0)
+		degrees += 360;
+
+	if (degrees >= 180)
+		invert = true;
+
+	dev_dbg(&hwdata->drvdata->client->dev,
+		"%s - %s: requested phase shift %d deg. (use inverter: %s).\n",
+		__func__, clk_hw_get_name(hw), degrees, (invert) ? "y" : "n");
+
+	ret = si5351_clkout_set_phase(hw, (invert) ? 180 : 0);
+	if (ret)
+		return -EINVAL;
+
+	if (invert)
+		degrees -= 180;
+
+	/*
+	 * The remaining phase offset must be taken care of by the MS parent
+	 * (phase offset value, reg 165..170, requires fractional mode).
+	 * This phase offset is relative to f_vco, but make a conversion to the
+	 * parent rate f_ms (rdiv: 1,2,4,...,128) first.
+	 */
+	rate = clk_get_rate(hw->clk);
+	parent_hw = clk_hw_get_parent(hw);
+	parent_rate = clk_get_rate(parent_hw->clk);
+	parent_degrees = degrees * (parent_rate / rate);
+
+	dev_dbg(&hwdata->drvdata->client->dev,
+		"%s - %s: RDIV = %lu, request MS phase = %d deg.\n",
+		__func__, clk_hw_get_name(hw),
+		parent_rate/rate, parent_degrees);
+
+	ret = si5351_msynth_set_phase(parent_hw, parent_degrees);
+	if (ret)
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * Si5351 i2c probe and DT
  */
@@ -1327,6 +1531,9 @@ static int si5351_dt_parse(struct i2c_client *client,
 		if (!of_property_read_u32(child, "clock-frequency", &val))
 			pdata->clkout[num].rate = val;
 
+		if (!of_property_read_u32(child, "clock-phase", &val))
+			pdata->clkout[num].phase = val;
+
 		pdata->clkout[num].pll_master =
 			of_property_read_bool(child, "silabs,pll-master");
 
@@ -1620,6 +1827,15 @@ static int si5351_i2c_probe(struct i2c_client *client,
 			return ret;
 		}
 
+		/*
+		 * All clocks are powered & enabled by default and the clock
+		 * subsystem assumes an unprepared clock.
+		 * So call our implementation directly to put each clock into a
+		 * safe state and get a well-defined behavior also with respect
+		 * to other clocks.
+		 */
+		si5351_clkout_unprepare(&drvdata->clkout[n].hw);
+
 		/* set initial clkout rate */
 		if (pdata->clkout[n].rate != 0) {
 			int ret;
@@ -1629,6 +1845,20 @@ static int si5351_i2c_probe(struct i2c_client *client,
 				dev_err(&client->dev, "Cannot set rate : %d\n",
 					ret);
 			}
+
+			/*
+			 * phase doesn't get propagated through the clock
+			 * hierarchy, so call our implementation directly
+			 */
+			ret = _si5351_clk_set_phase(&drvdata->clkout[n].hw,
+					    pdata->clkout[n].phase);
+			if (ret < 0) {
+				dev_err(&client->dev,
+					"Cannot set phase : %d\n", ret);
+			}
+
+			clk_prepare(drvdata->clkout[n].hw.clk);
+			clk_enable(drvdata->clkout[n].hw.clk);
 		}
 	}
 
diff --git a/drivers/clk/clk-si5351.h b/drivers/clk/clk-si5351.h
index 73dc8effc519..b00dddcf835b 100644
--- a/drivers/clk/clk-si5351.h
+++ b/drivers/clk/clk-si5351.h
@@ -133,6 +133,7 @@
 #define SI5351_CLK3_PHASE_OFFSET		168
 #define SI5351_CLK4_PHASE_OFFSET		169
 #define SI5351_CLK5_PHASE_OFFSET		170
+#define  SI5351_CLK_PHASE_OFFSET_MASK		127
 
 #define SI5351_PLL_RESET			177
 #define  SI5351_PLL_RESET_B			(1<<7)
diff --git a/include/linux/platform_data/si5351.h b/include/linux/platform_data/si5351.h
index c71a2dd66143..4eb59bd424dc 100644
--- a/include/linux/platform_data/si5351.h
+++ b/include/linux/platform_data/si5351.h
@@ -89,6 +89,7 @@ enum si5351_disable_state {
  * @pll_reset: if true, clkout can reset its pll
  * @drive: output drive strength
  * @rate: initial clkout rate, or default if 0
+ * @phase: initial clkout phase offset (default 0)
  */
 struct si5351_clkout_config {
 	enum si5351_multisynth_src multisynth_src;
@@ -98,6 +99,7 @@ struct si5351_clkout_config {
 	bool pll_master;
 	bool pll_reset;
 	unsigned long rate;
+	unsigned long phase;
 };
 
 /**
-- 
2.33.0

Re: [PATCH 1/2] clk: si5351: Add DT property for phase offset

[Rob Herring]

On Mon, Sep 13, 2021 at 10:52:41AM +0200, Jens Renner wrote:
> Add optional output clock DT property "clock-phase" to configure the
> phase offset in degrees with respect to other clock outputs.
> Add missing description for related optional output clock DT property
> "clock-frequency".
> 
> Signed-off-by: Jens Renner <renner@efe-gmbh.de>
> ---
>  .../devicetree/bindings/clock/silabs,si5351.txt        | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
> index 8fe6f80afade..62adf0d0874b 100644
> --- a/Documentation/devicetree/bindings/clock/silabs,si5351.txt
> +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
> @@ -50,11 +50,17 @@ Optional child node properties:
>    divider.
>  - silabs,pll-master: boolean, multisynth can change pll frequency.
>  - silabs,pll-reset: boolean, clock output can reset its pll.
> -- silabs,disable-state : clock output disable state, shall be
> +- silabs,disable-state: clock output disable state, shall be
>    0 = clock output is driven LOW when disabled
>    1 = clock output is driven HIGH when disabled
>    2 = clock output is FLOATING (HIGH-Z) when disabled
>    3 = clock output is NEVER disabled
> +- clock-frequency: integer in Hz, output frequency to generate (2500-200000000)
> +  This defines the output frequency set during boot. It can be reprogrammed
> +  duing runtime through the common clock framework.
> +- clock-phase: integer, phase shift in degrees (0-359), using the multisynth
> +  initial phase offset register (depends on the clock source / output ratio)
> +  and the clock output inverter (180 deg. only).

Not a standard property, needs a vendor prefix.

>  
>  ==Example==
>  
> @@ -111,7 +117,7 @@ i2c-master-node {
>  			silabs,drive-strength = <4>;
>  			silabs,multisynth-source = <1>;
>  			silabs,clock-source = <0>;
> -			pll-master;
> +			silabs,pll-master;
>  		};
>  
>  		/*
> -- 
> 2.33.0
> 
>