[PATCH 0/5] clk: bcm2835: add flags for mash and parent clocks

[PATCH 0/5] clk: bcm2835: add flags for mash and parent clocks

[kernel at martin.sperl.org]

From: Martin Sperl <kernel@martin.sperl.org>

Allow flags to be set on a per clock index basis, which control:
* the parent clocks selected when not setting clocks explicitly
* the clock mode with regards to integer only or higher order mash

These flags can also get set via the device tree.

Default flags:
* avoid the use of testdebug clocks for all clocks
* avoid pllc_* for per clocks
* allow fractional clocks, but not higher order mash

Other flags can easily get added if the need arrises.

Martin Sperl (5):
  dt: bindings: add means to control flags of specific clocks
  clk: bcm2835: expose the parent clocks via include/dt-bindings
  clk: bcm2835: enable default filtering for parent clocks
  clk: bcm2835: allow setting clocks flags via the dt
  clk: bcm2835: add support for BCM2835_CLOCK_FLAG_USE_MASH/INTEGER

 .../bindings/clock/brcm,bcm2835-cprman.txt         |  14 ++
 drivers/clk/bcm/clk-bcm2835.c                      | 202 +++++++++++++++++----
 include/dt-bindings/clock/bcm2835.h                |  33 ++++
 3 files changed, 216 insertions(+), 33 deletions(-)

--
2.1.4

[PATCH 1/5] dt: bindings: add means to control flags of specific clocks

[kernel at martin.sperl.org]

From: Martin Sperl <kernel@martin.sperl.org>

Add documentation for "brcm,clock-index" and "brcm,clock-flags"
which allows to set flags to enable the use of parent clocks
that may be different from the default.

In the future this may also contain flags that would allow
enabling higher order mash dividers.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 .../devicetree/bindings/clock/brcm,bcm2835-cprman.txt      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
index e56a1df..cbcca56 100644
--- a/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
@@ -18,6 +18,15 @@ Required properties:
 - reg:		Specifies base physical address and size of the registers
 - clocks:	The external oscillator clock phandle

+Optional properties:
+- brcm,clock-index: clock id to which the corresponding brcm,clock-flags
+		      value applies
+- brcm,clock-flags: flags for the specific clock id given in
+		      brcm,clock-index - the bits are defined in
+		      include/dt-bindings/clock/bcm2835.h
+		      of which the bottom 16 bits are reserved for
+		      parent clock selection.
+
 Example:

 	clk_osc: clock at 3 {
@@ -33,7 +42,11 @@ Example:
 		#clock-cells = <1>;
 		reg = <0x7e101000 0x2000>;
 		clocks = <&clk_osc>;
+		/* the pcm clock should use the non-standard pllc-per parent */
+		brcm,clock-index = <BCM2835_CLOCK_PCM>;
+		brcm,clock-flags = <((1 <<BCM2835_PER_PARENT_GND)|
+				     (1 <<BCM2835_PER_PARENT_OSC)|
+				     (1 <<BCM2835_PER_PARENT_PLLC))>;
 	};

 	i2c0: i2c at 7e205000 {
--
2.1.4

[PATCH 1/5] dt: bindings: add means to control flags of specific clocks

[Rob Herring]

On Thu, May 05, 2016 at 03:53:24PM +0000, kernel at martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
> 
> Add documentation for "brcm,clock-index" and "brcm,clock-flags"
> which allows to set flags to enable the use of parent clocks
> that may be different from the default.

assigned-clocks provides a way to do this already though that does mean 
exposing all the parents to DT. Does that not work for you?

> In the future this may also contain flags that would allow
> enabling higher order mash dividers.
> 
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> ---
>  .../devicetree/bindings/clock/brcm,bcm2835-cprman.txt      | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
> index e56a1df..cbcca56 100644
> --- a/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
> +++ b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
> @@ -18,6 +18,15 @@ Required properties:
>  - reg:		Specifies base physical address and size of the registers
>  - clocks:	The external oscillator clock phandle
> 
> +Optional properties:
> +- brcm,clock-index: clock id to which the corresponding brcm,clock-flags
> +		      value applies
> +- brcm,clock-flags: flags for the specific clock id given in
> +		      brcm,clock-index - the bits are defined in
> +		      include/dt-bindings/clock/bcm2835.h
> +		      of which the bottom 16 bits are reserved for
> +		      parent clock selection.

These are an array or single value?

I tend to think these should be generic properties or not at all. I'd 
like to hear Mike's and/or Stephen's opinion on it.

Rob

[PATCH 2/5] clk: bcm2835: expose the parent clocks via include/dt-bindings

[kernel at martin.sperl.org]

From: Martin Sperl <kernel@martin.sperl.org>

Expose the parent clocks via include/dt-bindings,
so that they can easily get referred to in the device tree.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c       | 45 ++++++++++++++++++-------------------
 include/dt-bindings/clock/bcm2835.h | 26 +++++++++++++++++++++
 2 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index c19b99c9..1b1657d 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1303,10 +1303,10 @@ struct bcm2835_clk_desc {
 
 /* main oscillator parent mux */
 static const char *const bcm2835_clock_osc_parents[] = {
-	"gnd",
-	"xosc",
-	"testdebug0",
-	"testdebug1"
+	[BCM2835_OSC_PARENT_GND]	= "gnd",
+	[BCM2835_OSC_PARENT_OSC]	= "xosc",
+	[BCM2835_OSC_PARENT_TESTDEBUG0]	= "testdebug0",
+	[BCM2835_OSC_PARENT_TESTDEBUG1]	= "testdebug1",
 };
 
 #define REGISTER_OSC_CLK(...)	REGISTER_CLK(				\
@@ -1316,14 +1316,14 @@ static const char *const bcm2835_clock_osc_parents[] = {
 
 /* main peripherial parent mux */
 static const char *const bcm2835_clock_per_parents[] = {
-	"gnd",
-	"xosc",
-	"testdebug0",
-	"testdebug1",
-	"plla_per",
-	"pllc_per",
-	"plld_per",
-	"pllh_aux",
+	[BCM2835_PER_PARENT_GND]	= "gnd",
+	[BCM2835_PER_PARENT_OSC]	= "xosc",
+	[BCM2835_PER_PARENT_TESTDEBUG0]	= "testdebug0",
+	[BCM2835_PER_PARENT_TESTDEBUG1]	= "testdebug1",
+	[BCM2835_PER_PARENT_PLLA_PER]	= "plla_per",
+	[BCM2835_PER_PARENT_PLLC_PER]	= "pllc_per",
+	[BCM2835_PER_PARENT_PLLD_PER]	= "plld_per",
+	[BCM2835_PER_PARENT_PLLH_AUX]	= "pllh_aux",
 };
 
 #define REGISTER_PER_CLK(...)	REGISTER_CLK(				\
@@ -1333,18 +1333,17 @@ static const char *const bcm2835_clock_per_parents[] = {
 
 /* main vpu parent mux */
 static const char *const bcm2835_clock_vpu_parents[] = {
-	"gnd",
-	"xosc",
-	"testdebug0",
-	"testdebug1",
-	"plla_core",
-	"pllc_core0",
-	"plld_core",
-	"pllh_aux",
-	"pllc_core1",
-	"pllc_core2",
+	[BCM2835_VPU_PARENT_GND]	= "gnd",
+	[BCM2835_VPU_PARENT_OSC]	= "xosc",
+	[BCM2835_VPU_PARENT_TESTDEBUG0]	= "testdebug0",
+	[BCM2835_VPU_PARENT_TESTDEBUG1]	= "testdebug1",
+	[BCM2835_VPU_PARENT_PLLA_CORE]	= "plla_core",
+	[BCM2835_VPU_PARENT_PLLC_CORE0]	= "pllc_core0",
+	[BCM2835_VPU_PARENT_PLLD_CORE]	= "plld_core",
+	[BCM2835_VPU_PARENT_PLLH_AUX]	= "pllh_aux",
+	[BCM2835_VPU_PARENT_PLLC_CORE1]	= "pllc_core1",
+	[BCM2835_VPU_PARENT_PLLC_CORE2]	= "pllc_core2",
 };
-
 #define REGISTER_VPU_CLK(...)	REGISTER_CLK(				\
 	.num_mux_parents = ARRAY_SIZE(bcm2835_clock_vpu_parents),	\
 	.parents = bcm2835_clock_vpu_parents,				\
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index 360e00c..e5396fe 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -64,3 +64,29 @@
 #define BCM2835_CLOCK_CAM1		46
 #define BCM2835_CLOCK_DSI0E		47
 #define BCM2835_CLOCK_DSI1E		48
+
+/* the parent clock for different clock types */
+#define BCM2835_OSC_PARENT_GND		0
+#define BCM2835_OSC_PARENT_OSC		1
+#define BCM2835_OSC_PARENT_TESTDEBUG0	2
+#define BCM2835_OSC_PARENT_TESTDEBUG1	3
+
+#define BCM2835_PER_PARENT_GND		0
+#define BCM2835_PER_PARENT_OSC		1
+#define BCM2835_PER_PARENT_TESTDEBUG0	2
+#define BCM2835_PER_PARENT_TESTDEBUG1	3
+#define BCM2835_PER_PARENT_PLLA_PER	4
+#define BCM2835_PER_PARENT_PLLC_PER	5
+#define BCM2835_PER_PARENT_PLLD_PER	6
+#define BCM2835_PER_PARENT_PLLH_AUX	7
+
+#define BCM2835_VPU_PARENT_GND		0
+#define BCM2835_VPU_PARENT_OSC		1
+#define BCM2835_VPU_PARENT_TESTDEBUG0	2
+#define BCM2835_VPU_PARENT_TESTDEBUG1	3
+#define BCM2835_VPU_PARENT_PLLA_CORE	4
+#define BCM2835_VPU_PARENT_PLLC_CORE0	5
+#define BCM2835_VPU_PARENT_PLLD_CORE	6
+#define BCM2835_VPU_PARENT_PLLH_AUX	7
+#define BCM2835_VPU_PARENT_PLLC_CORE1	8
+#define BCM2835_VPU_PARENT_PLLC_CORE2	9
-- 
2.1.4

[PATCH 3/5] clk: bcm2835: enable default filtering for parent clocks

[kernel at martin.sperl.org]

From: Martin Sperl <kernel@martin.sperl.org>

Depending on the type of clock define a default set of
parent clocks that are uses during parent clock selection.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c | 47 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 1b1657d..8a44ebf 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -836,6 +836,7 @@ struct bcm2835_clock {
 	struct clk_hw hw;
 	struct bcm2835_cprman *cprman;
 	const struct bcm2835_clock_data *data;
+	u32 flags;
 };
 
 static struct bcm2835_clock *bcm2835_clock_from_hw(struct clk_hw *hw)
@@ -1023,6 +1024,8 @@ static int bcm2835_clock_determine_rate(struct clk_hw *hw,
 		parent = clk_hw_get_parent_by_index(hw, i);
 		if (!parent)
 			continue;
+		if (!(clock->flags & BIT(i)))
+			continue;
 		prate = clk_hw_get_rate(parent);
 		div = bcm2835_clock_choose_div(hw, req->rate, prate, true);
 		rate = bcm2835_clock_rate_from_divisor(clock, prate, div);
@@ -1216,6 +1219,10 @@ bcm2835_register_pll_divider(struct bcm2835_cprman *cprman,
 	return clk;
 }
 
+static u32 bcm2835_register_clock_default_parents(
+	struct device *dev,
+	const struct bcm2835_clock_data *data);
+
 static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
 					  const struct bcm2835_clock_data *data)
 {
@@ -1263,6 +1270,8 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
 	clock->cprman = cprman;
 	clock->data = data;
 	clock->hw.init = &init;
+	clock->flags = bcm2835_register_clock_default_parents(
+		cprman->dev, data);
 
 	return devm_clk_register(cprman->dev, &clock->hw);
 }
@@ -1349,6 +1358,44 @@ static const char *const bcm2835_clock_vpu_parents[] = {
 	.parents = bcm2835_clock_vpu_parents,				\
 	__VA_ARGS__)
 
+/* calc the default flags for different clocks based on the parent array */
+static u32 bcm2835_register_clock_default_parents(
+	struct device *dev,
+	const struct bcm2835_clock_data *data)
+{
+	/* by default we disable the testdebug clocks for all known types */
+	if (data->parents == bcm2835_clock_osc_parents)
+		return
+			BIT(BCM2835_OSC_PARENT_GND) |
+			BIT(BCM2835_OSC_PARENT_OSC);
+	if (data->parents == bcm2835_clock_vpu_parents)
+		return
+			BIT(BCM2835_VPU_PARENT_GND) |
+			BIT(BCM2835_VPU_PARENT_OSC) |
+			BIT(BCM2835_VPU_PARENT_PLLA_CORE) |
+			BIT(BCM2835_VPU_PARENT_PLLC_CORE0) |
+			BIT(BCM2835_VPU_PARENT_PLLD_CORE) |
+			BIT(BCM2835_VPU_PARENT_PLLH_AUX) |
+			BIT(BCM2835_VPU_PARENT_PLLC_CORE1) |
+			BIT(BCM2835_VPU_PARENT_PLLC_CORE2);
+	if (data->parents == bcm2835_clock_per_parents)
+		return
+			BIT(BCM2835_PER_PARENT_GND) |
+			BIT(BCM2835_PER_PARENT_OSC) |
+			BIT(BCM2835_PER_PARENT_PLLA_PER) |
+			/* by default do not use PLLC_PER */
+			BIT(BCM2835_PER_PARENT_PLLD_PER) |
+			BIT(BCM2835_PER_PARENT_PLLH_AUX);
+
+	/* warn about unknown parents */
+	dev_warn(dev,
+		 "unknown clock parents for clock %s - enabling all %d parents\n",
+		 data->name, data->num_mux_parents);
+
+	/* return enable all clocks */
+	return BIT(data->num_mux_parents) - 1;
+}
+
 /*
  * the real definition of all the pll, pll_dividers and clocks
  * these make use of the above REGISTER_* macros
-- 
2.1.4

[PATCH 4/5] clk: bcm2835: allow setting clocks flags via the dt

[kernel at martin.sperl.org]

From: Martin Sperl <kernel@martin.sperl.org>

Allow setting flags for each clock via the device tree.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c | 57 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 8a44ebf..c80487e 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1125,7 +1125,8 @@ static const struct clk_ops bcm2835_vpu_clock_clk_ops = {
 };
 
 static struct clk *bcm2835_register_pll(struct bcm2835_cprman *cprman,
-					const struct bcm2835_pll_data *data)
+					const struct bcm2835_pll_data *data,
+					u32 flags)
 {
 	struct bcm2835_pll *pll;
 	struct clk_init_data init;
@@ -1152,7 +1153,8 @@ static struct clk *bcm2835_register_pll(struct bcm2835_cprman *cprman,
 
 static struct clk *
 bcm2835_register_pll_divider(struct bcm2835_cprman *cprman,
-			     const struct bcm2835_pll_divider_data *data)
+			     const struct bcm2835_pll_divider_data *data,
+			     u32 flags)
 {
 	struct bcm2835_pll_divider *divider;
 	struct clk_init_data init;
@@ -1223,8 +1225,10 @@ static u32 bcm2835_register_clock_default_parents(
 	struct device *dev,
 	const struct bcm2835_clock_data *data);
 
-static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
-					  const struct bcm2835_clock_data *data)
+static struct clk *
+bcm2835_register_clock(struct bcm2835_cprman *cprman,
+		       const struct bcm2835_clock_data *data,
+		       u32 flags)
 {
 	struct bcm2835_clock *clock;
 	struct clk_init_data init;
@@ -1270,14 +1274,19 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
 	clock->cprman = cprman;
 	clock->data = data;
 	clock->hw.init = &init;
-	clock->flags = bcm2835_register_clock_default_parents(
-		cprman->dev, data);
+
+	clock->flags = flags;
+	if ((clock->flags & (BIT(data->num_mux_parents) - 1)) == 0)
+		clock->flags |= bcm2835_register_clock_default_parents(
+			cprman->dev, data);
 
 	return devm_clk_register(cprman->dev, &clock->hw);
 }
 
-static struct clk *bcm2835_register_gate(struct bcm2835_cprman *cprman,
-					 const struct bcm2835_gate_data *data)
+static struct clk *
+bcm2835_register_gate(struct bcm2835_cprman *cprman,
+		      const struct bcm2835_gate_data *data,
+		      u32 flags)
 {
 	return clk_register_gate(cprman->dev, data->name, data->parent,
 				 CLK_IGNORE_UNUSED | CLK_SET_RATE_GATE,
@@ -1286,7 +1295,7 @@ static struct clk *bcm2835_register_gate(struct bcm2835_cprman *cprman,
 }
 
 typedef struct clk *(*bcm2835_clk_register)(struct bcm2835_cprman *cprman,
-					    const void *data);
+					    const void *data, int flags);
 struct bcm2835_clk_desc {
 	bcm2835_clk_register clk_register;
 	const void *data;
@@ -1861,6 +1870,10 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
 	struct resource *res;
 	const struct bcm2835_clk_desc *desc;
 	const size_t asize = ARRAY_SIZE(clk_desc_array);
+	u32 flags[asize], index;
+	struct property *prop;
+	const __be32 *cur;
+	int ret;
 	size_t i;
 
 	cprman = devm_kzalloc(dev,
@@ -1869,6 +1882,29 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
 	if (!cprman)
 		return -ENOMEM;
 
+	/* fill in flags from dt */
+	memset(flags, 0, sizeof(flags));
+	i = 0;
+	of_property_for_each_u32(dev->of_node, "brcm,clock-index",
+				 prop, cur, index) {
+		if (index >= asize) {
+			dev_err(dev,
+				"clock-index %d in \"brcm,clock-index\" is out of bounds at position %d\n",
+				index, i);
+		} else {
+			ret = of_property_read_u32_index(dev->of_node,
+							 "brcm,clock-flags",
+							 i, &flags[index]);
+			if (ret) {
+				dev_err(dev,
+					"no value at position %d in \"brcm,clock-flags\" - %d\n",
+					i, ret);
+			}
+		}
+		/* increment index */
+		i++;
+	}
+
 	spin_lock_init(&cprman->regs_lock);
 	cprman->dev = dev;
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1889,7 +1925,8 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
 	for (i = 0; i < asize; i++) {
 		desc = &clk_desc_array[i];
 		if (desc->clk_register && desc->data)
-			clks[i] = desc->clk_register(cprman, desc->data);
+			clks[i] = desc->clk_register(cprman, desc->data,
+				flags[i]);
 	}
 
 	return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
-- 
2.1.4

[PATCH 5/5] clk: bcm2835: add support for BCM2835_CLOCK_FLAG_USE_MASH/INTEGER

[kernel at martin.sperl.org]

From: Martin Sperl <kernel@martin.sperl.org>

Allow the use of higher order mash fractional divider modes
and also allow to force the use of integer dividers.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/clk/bcm/clk-bcm2835.c       | 57 +++++++++++++++++++++++++++++++++++--
 include/dt-bindings/clock/bcm2835.h |  7 +++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index c80487e..08140ad 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -130,6 +130,7 @@
 # define CM_BUSY			BIT(7)
 # define CM_BUSYD			BIT(8)
 # define CM_FRAC			BIT(9)
+# define CM_MASH			BIT(10)
 # define CM_SRC_SHIFT			0
 # define CM_SRC_BITS			4
 # define CM_SRC_MASK			0xf
@@ -888,6 +889,24 @@ static u32 bcm2835_clock_choose_div(struct clk_hw *hw,
 				 CM_DIV_FRAC_BITS - data->frac_bits);
 	}
 
+	/* based on flags modify settings */
+	switch (clock->flags & BCM2835_CLOCK_FLAG_USE_MASK) {
+	case BCM2835_CLOCK_FLAG_USE_FRAC:
+			break;
+	case BCM2835_CLOCK_FLAG_USE_INTEGER:
+		div &= GENMASK(data->int_bits + CM_DIV_FRAC_BITS - 1,
+			       CM_DIV_FRAC_BITS);
+		break;
+	case BCM2835_CLOCK_FLAG_USE_MASH2:
+		mindiv += 2 << CM_DIV_FRAC_BITS;
+		maxdiv -= 1 << CM_DIV_FRAC_BITS;
+		break;
+	case BCM2835_CLOCK_FLAG_USE_MASH3:
+		mindiv += 4 << CM_DIV_FRAC_BITS;
+		maxdiv -= 3 << CM_DIV_FRAC_BITS;
+		break;
+	}
+
 	/* apply the clamping  limits */
 	div = max_t(u32, div, mindiv);
 	div = min_t(u32, div, maxdiv);
@@ -996,8 +1015,26 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
 	 * clock this requirement should be take care of by the
 	 * clk-framework.
 	 */
-	ctl = cprman_read(cprman, data->ctl_reg) & ~CM_FRAC;
-	ctl |= (div & CM_DIV_FRAC_MASK) ? CM_FRAC : 0;
+	ctl = cprman_read(cprman, data->ctl_reg) & ~(CM_FRAC | CM_MASH);
+	if (div & CM_DIV_FRAC_MASK) {
+		/*
+		 * we never get to *MASH* for non-mash clock,
+		 * as it is filtered in bcm2835_register_clock
+		 * USE_INTEGER forces already choose_clock_div
+		 * to apply the mask so that we do not get here
+		 */
+		switch (clock->flags & BCM2835_CLOCK_FLAG_USE_MASK) {
+		case BCM2835_CLOCK_FLAG_USE_FRAC:
+			ctl |= CM_FRAC;
+			break;
+		case BCM2835_CLOCK_FLAG_USE_MASH2:
+			ctl |= CM_MASH;
+			break;
+		case BCM2835_CLOCK_FLAG_USE_MASH3:
+			ctl |= CM_MASH | CM_FRAC;
+			break;
+		}
+	}
 	cprman_write(cprman, data->ctl_reg, ctl);
 
 	cprman_write(cprman, data->div_reg, div);
@@ -1280,6 +1317,22 @@ bcm2835_register_clock(struct bcm2835_cprman *cprman,
 		clock->flags |= bcm2835_register_clock_default_parents(
 			cprman->dev, data);
 
+	/* check for MASH flags when the clock is not a mash clock */
+	if (!data->is_mash_clock) {
+		switch (clock->flags & BCM2835_CLOCK_FLAG_USE_MASK) {
+		case BCM2835_CLOCK_FLAG_USE_MASH2:
+		case BCM2835_CLOCK_FLAG_USE_MASH3:
+			dev_warn(cprman->dev,
+				 "found mash2/mash3 flag for clock %s, which does not support mash\n",
+				 data->name);
+			clock->flags &= BCM2835_CLOCK_FLAG_USE_MASK;
+			clock->flags |= BCM2835_CLOCK_FLAG_USE_FRAC;
+			break;
+		default:
+			break;
+		}
+	}
+
 	return devm_clk_register(cprman->dev, &clock->hw);
 }
 
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
index e5396fe..5591bd1 100644
--- a/include/dt-bindings/clock/bcm2835.h
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -90,3 +90,10 @@
 #define BCM2835_VPU_PARENT_PLLH_AUX	7
 #define BCM2835_VPU_PARENT_PLLC_CORE1	8
 #define BCM2835_VPU_PARENT_PLLC_CORE2	9
+
+/* additional clock flags */
+#define BCM2835_CLOCK_FLAG_USE_FRAC	((0 << 31) | (0 << 30))
+#define BCM2835_CLOCK_FLAG_USE_INTEGER	((0 << 31) | (1 << 30))
+#define BCM2835_CLOCK_FLAG_USE_MASH2	((1 << 31) | (0 << 30))
+#define BCM2835_CLOCK_FLAG_USE_MASH3	((1 << 31) | (1 << 30))
+#define BCM2835_CLOCK_FLAG_USE_MASK	((1 << 31) | (1 << 30))
-- 
2.1.4

[PATCH 0/5] clk: bcm2835: add flags for mash and parent clocks

[Eric Anholt]

kernel at martin.sperl.org writes:

> From: Martin Sperl <kernel@martin.sperl.org>
>
> Allow flags to be set on a per clock index basis, which control:
> * the parent clocks selected when not setting clocks explicitly

I don't think we need this other than avoiding PLLC.  What else do you
want to filter, and why?

> * the clock mode with regards to integer only or higher order mash

Please provide justification in the commit message explaining why there
is no obvious mash setting to just implement in clk-bcm2835.c, but that
that a better policy can be encoded in the DT.

Overall, I'm skeptical of this series.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160509/cfd0dcbe/attachment.sig>

[PATCH 0/5] clk: bcm2835: add flags for mash and parent clocks

[Martin Sperl]

On 10.05.2016 03:05, Eric Anholt wrote:
> kernel at martin.sperl.org writes:
>
>> From: Martin Sperl <kernel@martin.sperl.org>
>>
>> Allow flags to be set on a per clock index basis, which control:
>> * the parent clocks selected when not setting clocks explicitly
>
> I don't think we need this other than avoiding PLLC.  What else do you
> want to filter, and why?
What about potentially disabling PLLA_per or PLLH_aux?
Under some circumstances it may make sense to avoid those clocks.
(or testdebug for that matter).

One of the reasons can be to avoid low dividers - see also mash below

>
>> * the clock mode with regards to integer only or higher order mash
>
> Please provide justification in the commit message explaining why there
> is no obvious mash setting to just implement in clk-bcm2835.c, but that
> that a better policy can be encoded in the DT.

The normal fractional divider will vary the effective period length
between int(div)/Fparent and (int(div)+1)/Fparent.

So the device (say DAC or other) connected needs to support in
principle Fparent/int(div) as a clock.

But for higher order mash the HW will spread the period length between:
(int(div)-3)/Fparent and (int(div)+4)/Fparent.

This value may be outside of the frequencies supported by the attached
HW. That is why mash is not enabled by default for mash clocks - only
div.

That is also why we need to have this configurable.

Also obviously higher parent clock frequencies result in higher dividers
for the same target-frequency. So the above effect is less dramatic
when using plld_per (or pllc for that matter) compared to the main
19.2MHz oscillator, as for 32bit stereo audio at 96kHz (equivalent to
6144000Hz) in the first case we have a divider of 81.38 while for the
second (osc) case we have a divider of 3.125.

The use of mash3 is impossible for osc but the use of mash 2 would
spread the frequencies between 9.6MHz (div=2) and 3.84MHz (div=5).

For plld_per as parent using mash3 instead we get the range:
6410256.4Hz (div=78) and 5882352.9Hz (div=85), which is a much more
acceptable range and should not impact the device that much.

So enabling mash 3 by default is not in the best interest hence it is
not done.

This also shows why with some devices it may be preferable to configure
that only pllc is used and then with mash 3.

So this patchset tries to accomodate that - even if not in a
perfect way - I would have preferred to have this in the dt-node that
is claiming that clock, but I did not want to go that far (yet) adding
another method to clock_op.

To answer your comments on the patches themselves:
I had also been investigating the use of "assigned-clock-parents".
Something like this will not work:
i2s at ... {
   clock = <&preman PCM>;
   assigned-clocks = <&cprman PCM>, <&pcrman PCM>
   assigned-clock-parents = <&osc>, <&cprman PLLD_PER>;
}
because what it does it:
* call clk_set_parent(pcm, osc);
* call clk_set_parent(pcm, plld_per);

(if you add "assigned-clock-rates" into the mix then
clk_set_rate(pcm, rate) is also called separately).

So there is no way to "keep" track of the parent clocks we wanted
assigned.

As far as I can see the only thing that would work is to implement
an extension to the dt that would define "assigned-clock-flags"
and have a set_flags(struct clk_hw *, u32 flags) method in
clk_ops.

So maybe someone has some better ideas of how to solve this.

Martin

As a side note: selection of parent oscillator and corresponding mash
may have distinct noise behavior on the same device, where for
32bit*2 at 48k it is the main oscillator with mash3 while the second best
match would be pllc with mash2 - not mash3, which has the worsted SN
from all tested!

This behavior obviously changes with different "target frequencies"
so it may even be necessary to account for the different target
frequencies.

But that is for a different discussion.

[PATCH 0/5] clk: bcm2835: add flags for mash and parent clocks

[Eric Anholt]

Martin Sperl <kernel@martin.sperl.org> writes:

> On 10.05.2016 03:05, Eric Anholt wrote:
>> kernel at martin.sperl.org writes:
>>
>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>
>>> Allow flags to be set on a per clock index basis, which control:
>>> * the parent clocks selected when not setting clocks explicitly
>>
>> I don't think we need this other than avoiding PLLC.  What else do you
>> want to filter, and why?
> What about potentially disabling PLLA_per or PLLH_aux?
> Under some circumstances it may make sense to avoid those clocks.
> (or testdebug for that matter).

PLLH and its descendents are only used by vc4, and the parent choice
done by this driver for pllh is correct.

I don't know what you mean by "potentially disabling PLLA_PER".

We don't expose the testdebug parents, because they're of no use to us.

>>> * the clock mode with regards to integer only or higher order mash
>>
>> Please provide justification in the commit message explaining why there
>> is no obvious mash setting to just implement in clk-bcm2835.c, but that
>> that a better policy can be encoded in the DT.
>
> The normal fractional divider will vary the effective period length
> between int(div)/Fparent and (int(div)+1)/Fparent.
>
> So the device (say DAC or other) connected needs to support in
> principle Fparent/int(div) as a clock.
>
> But for higher order mash the HW will spread the period length between:
> (int(div)-3)/Fparent and (int(div)+4)/Fparent.
>
> This value may be outside of the frequencies supported by the attached
> HW. That is why mash is not enabled by default for mash clocks - only
> div.
>
> That is also why we need to have this configurable.
>
> Also obviously higher parent clock frequencies result in higher dividers
> for the same target-frequency. So the above effect is less dramatic
> when using plld_per (or pllc for that matter) compared to the main
> 19.2MHz oscillator, as for 32bit stereo audio at 96kHz (equivalent to
> 6144000Hz) in the first case we have a divider of 81.38 while for the
> second (osc) case we have a divider of 3.125.
>
> The use of mash3 is impossible for osc but the use of mash 2 would
> spread the frequencies between 9.6MHz (div=2) and 3.84MHz (div=5).
>
> For plld_per as parent using mash3 instead we get the range:
> 6410256.4Hz (div=78) and 5882352.9Hz (div=85), which is a much more
> acceptable range and should not impact the device that much.
>
> So enabling mash 3 by default is not in the best interest hence it is
> not done.

It sounds like you're optimizing for reducing frequency spread.  That
sounds fairly easy to do in the parent-choice function.

> As a side note: selection of parent oscillator and corresponding mash
> may have distinct noise behavior on the same device, where for
> 32bit*2 at 48k it is the main oscillator with mash3 while the second best
> match would be pllc with mash2 - not mash3, which has the worsted SN
> from all tested!

But PLLC continues to be unusable, since it's changed at runtime, and
evaluating its noise characteristics based on a snapshot of its
frequency doesn't make any sense.

> This behavior obviously changes with different "target frequencies"
> so it may even be necessary to account for the different target
> frequencies.

This is why encoding mash in the DT doesn't make any sense to me.  You
need a runtime policy for choosing the mash setting based on the
frequency requested, so let's figure out what policy we actually want.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160510/a3970398/attachment.sig>

[PATCH 0/5] clk: bcm2835: add flags for mash and parent clocks

[Martin Sperl]

On 10.05.2016 19:45, Eric Anholt wrote:
> Martin Sperl <kernel@martin.sperl.org> writes:
>
>> On 10.05.2016 03:05, Eric Anholt wrote:
>>> kernel at martin.sperl.org writes:
>>>
>>>> From: Martin Sperl <kernel@martin.sperl.org>
>>>>
>>>> Allow flags to be set on a per clock index basis, which control:
>>>> * the parent clocks selected when not setting clocks explicitly
>>>
>>> I don't think we need this other than avoiding PLLC.  What else do you
>>> want to filter, and why?
>> What about potentially disabling PLLA_per or PLLH_aux?
>> Under some circumstances it may make sense to avoid those clocks.
>> (or testdebug for that matter).
>
> PLLH and its descendents are only used by vc4, and the parent choice
> done by this driver for pllh is correct.
>
> I don't know what you mean by "potentially disabling PLLA_PER".
>
> We don't expose the testdebug parents, because they're of no use to us.
For all practical purposes we still have all the clocks as potential
parents during automatic clock selection - and that includes:
PLLA_PER, PLLH_AUX and TESTDEBUGX.

Not all of them are enabled, but some are - so If I select an i2s 
frequency of "3903125" then plla_per will get used with a divider of 2.

>>>> * the clock mode with regards to integer only or higher order mash
>>>
>>> Please provide justification in the commit message explaining why there
>>> is no obvious mash setting to just implement in clk-bcm2835.c, but that
>>> that a better policy can be encoded in the DT.
>>
>> The normal fractional divider will vary the effective period length
>> between int(div)/Fparent and (int(div)+1)/Fparent.
>>
>> So the device (say DAC or other) connected needs to support in
>> principle Fparent/int(div) as a clock.
>>
>> But for higher order mash the HW will spread the period length between:
>> (int(div)-3)/Fparent and (int(div)+4)/Fparent.
>>
>> This value may be outside of the frequencies supported by the attached
>> HW. That is why mash is not enabled by default for mash clocks - only
>> div.
>>
>> That is also why we need to have this configurable.
>>
>> Also obviously higher parent clock frequencies result in higher dividers
>> for the same target-frequency. So the above effect is less dramatic
>> when using plld_per (or pllc for that matter) compared to the main
>> 19.2MHz oscillator, as for 32bit stereo audio at 96kHz (equivalent to
>> 6144000Hz) in the first case we have a divider of 81.38 while for the
>> second (osc) case we have a divider of 3.125.
>>
>> The use of mash3 is impossible for osc but the use of mash 2 would
>> spread the frequencies between 9.6MHz (div=2) and 3.84MHz (div=5).
>>
>> For plld_per as parent using mash3 instead we get the range:
>> 6410256.4Hz (div=78) and 5882352.9Hz (div=85), which is a much more
>> acceptable range and should not impact the device that much.
>>
>> So enabling mash 3 by default is not in the best interest hence it is
>> not done.
>
> It sounds like you're optimizing for reducing frequency spread.  That
> sounds fairly easy to do in the parent-choice function.

You misunderstand my argument here!

I know that it may be automated, but you may have different devices
connected to: i2s/pcm, pwm, gp and slim.

Some of them need the audio spreading, while others need a stable
waveform - as mentioned mash3 varies the period between:
(divi-2)/fparent and (divi+4)/fparent in discrete steps
For this the worsted case is a requested frequency that results
in a divider between 4 and 5.

Also some DAC require a STABLE clock waveform on i2s with
specific requirements for ton/(ton+toff) between 40% and 60%,
as they have their own internal PLL, while others do not need
this kind of behavior and can handle variable period-length.
But then these DAC still have a minimum period length, which we
may not exceed.

So making an automatic selection is not possible hence
we need a means to configure the supported mash level
to get the best results in all cases.

>
>> As a side note: selection of parent oscillator and corresponding mash
>> may have distinct noise behavior on the same device, where for
>> 32bit*2 at 48k it is the main oscillator with mash3 while the second best
>> match would be pllc with mash2 - not mash3, which has the worsted SN
>> from all tested!
>
> But PLLC continues to be unusable, since it's changed at runtime, and
> evaluating its noise characteristics based on a snapshot of its
> frequency doesn't make any sense.
That was a general side note not necessarily related to PLLC.
But this may be better handled by a generic clock driver
similar to clk-fixed-factor.

>
>> This behavior obviously changes with different "target frequencies"
>> so it may even be necessary to account for the different target
>> frequencies.
>
> This is why encoding mash in the DT doesn't make any sense to me.  You
> need a runtime policy for choosing the mash setting based on the
> frequency requested, so let's figure out what policy we actually want.
See the argument of different devices/DAC with different requirements.
There is no way for any of this to work automatically and make every
client happy.