[U-Boot] [PATCH v2 0/5] clk: support assigned-clock, assigned-clock-parents, assigned-clock-rates

[U-Boot] [PATCH v2 0/5] clk: support assigned-clock, assigned-clock-parents, assigned-clock-rates

[Philipp Tomsich]

For various peripherals on Rockchip SoCs (e.g. for the Ethernet GMAC),
the parent-clock needs to be set via the DTS.  This adds the required
plumbing and implements the GMAC case for the RK3399.

Changes in v2:
- Fixed David's email address.


Philipp Tomsich (5):
  clk: add clk_set_parent()
  clk: refactor clk_get_by_index() into clk_get_by_indexed_prop()
  rockchip: clk: rk3399: implement set_parent() operation
  clk: implement clk_set_defaults()
  rockchip: clk: rk3399: accept all assigned-clocks from the 'cru'-node

 drivers/clk/clk-uclass.c          | 140 +++++++++++++++++++++++++++++++++++++-
 drivers/clk/rockchip/clk_rk3399.c |  92 ++++++++++++++++++++++++-
 drivers/core/device.c             |   6 ++
 include/clk-uclass.h              |   8 +++
 include/clk.h                     |  28 ++++++++
 5 files changed, 270 insertions(+), 4 deletions(-)

-- 
2.1.4

[U-Boot] [PATCH v2 1/5] clk: add clk_set_parent()

[Philipp Tomsich]

Clocks may support multiple parents: this change introduces an
optional operation on the clk-uclass to set a clock's parent.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: David Wu <david.wu@rock-chips.com>

---

Changes in v2:
- Fixed David's email address.

 drivers/clk/clk-uclass.c | 12 ++++++++++++
 include/clk-uclass.h     |  8 ++++++++
 include/clk.h            | 11 +++++++++++
 3 files changed, 31 insertions(+)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index fbea720..20ee0c5 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -188,6 +188,18 @@ ulong clk_set_rate(struct clk *clk, ulong rate)
 	return ops->set_rate(clk, rate);
 }
 
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	const struct clk_ops *ops = clk_dev_ops(clk->dev);
+
+	debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent);
+
+	if (!ops->set_parent)
+		return -ENOSYS;
+
+	return ops->set_parent(clk, parent);
+}
+
 int clk_enable(struct clk *clk)
 {
 	const struct clk_ops *ops = clk_dev_ops(clk->dev);
diff --git a/include/clk-uclass.h b/include/clk-uclass.h
index e7ea334..75933eb 100644
--- a/include/clk-uclass.h
+++ b/include/clk-uclass.h
@@ -78,6 +78,14 @@ struct clk_ops {
 	 */
 	ulong (*set_rate)(struct clk *clk, ulong rate);
 	/**
+	 * set_parent() - Set current clock parent
+	 *
+	 * @clk:        The clock to manipulate.
+	 * @parent:     New clock parent.
+	 * @return zero on success, or -ve error code.
+	 */
+	int (*set_parent)(struct clk *clk, struct clk *parent);
+	/**
 	 * enable() - Enable a clock.
 	 *
 	 * @clk:	The clock to manipulate.
diff --git a/include/clk.h b/include/clk.h
index e7ce3e8..e463d8e 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -178,6 +178,17 @@ ulong clk_get_rate(struct clk *clk);
 ulong clk_set_rate(struct clk *clk, ulong rate);
 
 /**
+ * clk_set_parent() - Set current clock parent.
+ *
+ * @clk:	A clock struct that was previously successfully requested by
+ *		clk_request/get_by_*().
+ * @parent:	A clock struct that was previously successfully requested by
+ *		clk_request/get_by_*().
+ * @return new rate, or -ve error code.
+ */
+int clk_set_parent(struct clk *clk, struct clk *parent);
+
+/**
  * clk_enable() - Enable (turn on) a clock.
  *
  * @clk:	A clock struct that was previously successfully requested by
-- 
2.1.4

[U-Boot] [U-Boot,v2,1/5] clk: add clk_set_parent()

[Philipp Tomsich]

> Clocks may support multiple parents: this change introduces an
> optional operation on the clk-uclass to set a clock's parent.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
> 
> Changes in v2:
> - Fixed David's email address.
> 
>  drivers/clk/clk-uclass.c | 12 ++++++++++++
>  include/clk-uclass.h     |  8 ++++++++
>  include/clk.h            | 11 +++++++++++
>  3 files changed, 31 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH v2 2/5] clk: refactor clk_get_by_index() into clk_get_by_indexed_prop()

[Philipp Tomsich]

The logic in clk_get_by_index() may be useful for other properties
than 'clocks': e.g. 'assigned-clocks' and 'assigned-clock-parents'
follows the same model.

This commit refactors clk_get_by_index() by introducing an internal
function clk_get_by_indexed_prop() that allows to specify the name
of the property to process.  The original clk_get_by_index() call
is simply directed through this helper function with the property
name fixed to "clocks".

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: David Wu <david.wu@rock-chips.com>

---

Changes in v2:
- Fixed David's email address.

 drivers/clk/clk-uclass.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 20ee0c5..7fdf16d 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -53,7 +53,8 @@ static int clk_of_xlate_default(struct clk *clk,
 	return 0;
 }
 
-int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
+static int clk_get_by_indexed_prop(struct udevice *dev, const char *prop_name,
+				   int index, struct clk *clk)
 {
 	int ret;
 	struct ofnode_phandle_args args;
@@ -65,7 +66,7 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
 	assert(clk);
 	clk->dev = NULL;
 
-	ret = dev_read_phandle_with_args(dev, "clocks", "#clock-cells", 0,
+	ret = dev_read_phandle_with_args(dev, prop_name, "#clock-cells", 0,
 					 index, &args);
 	if (ret) {
 		debug("%s: fdtdec_parse_phandle_with_args failed: err=%d\n",
@@ -95,6 +96,11 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
 
 	return clk_request(dev_clk, clk);
 }
+
+int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
+{
+	return clk_get_by_indexed_prop(dev, "clocks", index, clk);
+}
 # endif /* OF_PLATDATA */
 
 int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk)
-- 
2.1.4

[U-Boot] [U-Boot, v2, 2/5] clk: refactor clk_get_by_index() into clk_get_by_indexed_prop()

[Philipp Tomsich]

> The logic in clk_get_by_index() may be useful for other properties
> than 'clocks': e.g. 'assigned-clocks' and 'assigned-clock-parents'
> follows the same model.
> 
> This commit refactors clk_get_by_index() by introducing an internal
> function clk_get_by_indexed_prop() that allows to specify the name
> of the property to process.  The original clk_get_by_index() call
> is simply directed through this helper function with the property
> name fixed to "clocks".
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
> 
> Changes in v2:
> - Fixed David's email address.
> 
>  drivers/clk/clk-uclass.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH v2 3/5] rockchip: clk: rk3399: implement set_parent() operation

[Philipp Tomsich]

This implements the (newly added) set_parent() operation for the
RK3399 with a focus on allowing the RGMII clock parent to be
configured via the assigned-clock-parents property of the GMAC node.

This implementation supports only the GMAC (in fact only the RGMII
clock parent) and allows to set this clock's parent either to the
internal SCLK_GMAC or to an external clock input (identifiable by it
providing a 'clock-output-name' of "gmac_clkin").

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: David Wu <david.wu@rock-chips.com>

---

Changes in v2:
- Fixed David's email address.

 drivers/clk/rockchip/clk_rk3399.c | 74 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk_rk3399.c b/drivers/clk/rockchip/clk_rk3399.c
index 2f4c4e3..e791936 100644
--- a/drivers/clk/rockchip/clk_rk3399.c
+++ b/drivers/clk/rockchip/clk_rk3399.c
@@ -742,6 +742,30 @@ static ulong rk3399_mmc_set_clk(struct rk3399_cru *cru,
 	return rk3399_mmc_get_clk(cru, clk_id);
 }
 
+static ulong rk3399_gmac_set_clk(struct rk3399_cru *cru, ulong rate)
+{
+	ulong ret;
+
+	/*
+	 * The RGMII CLK can be derived either from an external "clkin"
+	 * or can be generated from internally by a divider from SCLK_MAC.
+	 */
+	if (readl(&cru->clksel_con[19]) & BIT(4)) {
+		/* An external clock will always generate the right rate... */
+		ret = rate;
+	} else {
+		/*
+		 * No platform uses an internal clock to date.
+		 * Implement this once it becomes necessary and print an error
+		 * if someone tries to use it (while it remains unimplemented).
+		 */
+		pr_err("%s: internal clock is UNIMPLEMENTED\n", __func__);
+		ret = 0;
+	}
+
+	return ret;
+}
+
 #define PMUSGRF_DDR_RGN_CON16 0xff330040
 static ulong rk3399_ddr_set_clk(struct rk3399_cru *cru,
 				ulong set_rate)
@@ -865,8 +889,7 @@ static ulong rk3399_clk_set_rate(struct clk *clk, ulong rate)
 		ret = rk3399_mmc_set_clk(priv->cru, clk->id, rate);
 		break;
 	case SCLK_MAC:
-		/* nothing to do, as this is an external clock */
-		ret = rate;
+		ret = rk3399_gmac_set_clk(priv->cru, rate);
 		break;
 	case SCLK_I2C1:
 	case SCLK_I2C2:
@@ -902,6 +925,52 @@ static ulong rk3399_clk_set_rate(struct clk *clk, ulong rate)
 	return ret;
 }
 
+static int rk3399_gmac_set_parent(struct clk *clk, struct clk *parent)
+{
+	struct rk3399_clk_priv *priv = dev_get_priv(clk->dev);
+	const char *clock_output_name;
+	int ret;
+
+	/*
+	 * If the requested parent is in the same clock-controller and
+	 * the id is SCLK_MAC ("clk_gmac"), switch to the internal clock.
+	 */
+	if ((parent->dev == clk->dev) && (parent->id == SCLK_MAC)) {
+		debug("%s: switching RGMII to SCLK_MAC\n", __func__);
+		rk_clrreg(&priv->cru->clksel_con[19], BIT(4));
+		return 0;
+	}
+
+	/*
+	 * Otherwise, we need to check the clock-output-names of the
+	 * requested parent to see if the requested id is "clkin_gmac".
+	 */
+	ret = dev_read_string_index(parent->dev, "clock-output-names",
+				    parent->id, &clock_output_name);
+	if (ret < 0)
+		return -ENODATA;
+
+	/* If this is "clkin_gmac", switch to the external clock input */
+	if (!strcmp(clock_output_name, "clkin_gmac")) {
+		debug("%s: switching RGMII to CLKIN\n", __func__);
+		rk_setreg(&priv->cru->clksel_con[19], BIT(4));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int rk3399_clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	switch (clk->id) {
+	case SCLK_RMII_SRC:
+		return rk3399_gmac_set_parent(clk, parent);
+	}
+
+	debug("%s: unsupported clk %ld\n", __func__, clk->id);
+	return -ENOENT;
+}
+
 static int rk3399_clk_enable(struct clk *clk)
 {
 	switch (clk->id) {
@@ -919,6 +988,7 @@ static int rk3399_clk_enable(struct clk *clk)
 static struct clk_ops rk3399_clk_ops = {
 	.get_rate = rk3399_clk_get_rate,
 	.set_rate = rk3399_clk_set_rate,
+	.set_parent = rk3399_clk_set_parent,
 	.enable = rk3399_clk_enable,
 };
 
-- 
2.1.4

[U-Boot] [U-Boot, v2, 3/5] rockchip: clk: rk3399: implement set_parent() operation

[Philipp Tomsich]

> This implements the (newly added) set_parent() operation for the
> RK3399 with a focus on allowing the RGMII clock parent to be
> configured via the assigned-clock-parents property of the GMAC node.
> 
> This implementation supports only the GMAC (in fact only the RGMII
> clock parent) and allows to set this clock's parent either to the
> internal SCLK_GMAC or to an external clock input (identifiable by it
> providing a 'clock-output-name' of "gmac_clkin").
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
> 
> Changes in v2:
> - Fixed David's email address.
> 
>  drivers/clk/rockchip/clk_rk3399.c | 74 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 72 insertions(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [PATCH v2 4/5] clk: implement clk_set_defaults()

[Philipp Tomsich]

Linux uses the properties 'assigned-clocks', 'assigned-clock-parents'
and 'assigned-clock-rates' to configure the clock subsystem for use
with various peripheral nodes.

This implements clk_set_defaults() and hooks it up with the general
device probibin in drivers/core/device.c: when a new device is probed,
clk_set_defaults() will be called for it and will process the
properties mentioned above.

Note that this functionality is designed to fail gracefully (i.e. if a
clock-driver does not implement set_parent(), we simply accept this
and ignore the error) as not to break existing board-support.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: David Wu <david.wu@rock-chips.com>

---

Changes in v2:
- Fixed David's email address.

 drivers/clk/clk-uclass.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/core/device.c    |   6 +++
 include/clk.h            |  17 +++++++
 3 files changed, 141 insertions(+)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 7fdf16d..ad76379 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -2,6 +2,7 @@
  * Copyright (C) 2015 Google, Inc
  * Written by Simon Glass <sjg@chromium.org>
  * Copyright (c) 2016, NVIDIA CORPORATION.
+ * Copyright (c) 2018, Theobroma Systems Design und Consulting GmbH
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -10,6 +11,7 @@
 #include <clk.h>
 #include <clk-uclass.h>
 #include <dm.h>
+#include <dm/read.h>
 #include <dt-structs.h>
 #include <errno.h>
 
@@ -101,6 +103,122 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
 {
 	return clk_get_by_indexed_prop(dev, "clocks", index, clk);
 }
+
+static int clk_set_default_parents(struct udevice *dev)
+{
+	struct clk clk, parent_clk;
+	int index;
+	int num_parents;
+	int ret;
+
+	num_parents = dev_count_phandle_with_args(dev, "assigned-clock-parents",
+						  "#clock-cells");
+	if (num_parents < 0) {
+		debug("%s: could not read assigned-clock-parents for %p\n",
+		      __func__, dev);
+		return 0;
+	}
+
+	for (index = 0; index < num_parents; index++) {
+		ret = clk_get_by_indexed_prop(dev, "assigned-clock-parents",
+					      index, &parent_clk);
+		if (ret) {
+			debug("%s: could not get parent clock %d for %s\n",
+			      __func__, index, dev_read_name(dev));
+			return ret;
+		}
+
+		ret = clk_get_by_indexed_prop(dev, "assigned-clocks",
+					      index, &clk);
+		if (ret) {
+			debug("%s: could not get assigned clock %d for %s\n",
+			      __func__, index, dev_read_name(dev));
+			return ret;
+		}
+
+		ret = clk_set_parent(&clk, &parent_clk);
+
+		/*
+		 * Not all drivers may support clock-reparenting (as of now).
+		 * Ignore errors due to this.
+		 */
+		if (ret == -ENOSYS)
+			continue;
+
+		if (ret) {
+			debug("%s: failed to reparent clock %d for %s\n",
+			      __func__, index, dev_read_name(dev));
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int clk_set_default_rates(struct udevice *dev)
+{
+	struct clk clk;
+	int index;
+	int num_rates;
+	int size;
+	int ret = 0;
+	u32 *rates = NULL;
+
+	size = dev_read_size(dev, "assigned-clock-rates");
+	if (size < 0)
+		return 0;
+
+	num_rates = size / sizeof(u32);
+	rates = calloc(num_rates, sizeof(u32));
+	if (!rates)
+		return -ENOMEM;
+
+	ret = dev_read_u32_array(dev, "assigned-clock-rates", rates, num_rates);
+	if (ret)
+		goto fail;
+
+	for (index = 0; index < num_rates; index++) {
+		ret = clk_get_by_indexed_prop(dev, "assigned-clocks",
+					      index, &clk);
+		if (ret) {
+			debug("%s: could not get assigned clock %d for %s\n",
+			      __func__, index, dev_read_name(dev));
+			continue;
+		}
+
+		ret = clk_set_rate(&clk, rates[index]);
+		if (ret < 0) {
+			debug("%s: failed to set rate on clock %d for %s\n",
+			      __func__, index, dev_read_name(dev));
+			break;
+		}
+	}
+
+fail:
+	free(rates);
+	return ret;
+}
+
+int clk_set_defaults(struct udevice *dev)
+{
+	int ret;
+
+	/* If this is running pre-reloc state, don't take any action. */
+	if (!(gd->flags & GD_FLG_RELOC))
+		return 0;
+
+	debug("%s(%s)\n", __func__, dev_read_name(dev));
+
+	ret = clk_set_default_parents(dev);
+	if (ret)
+		return ret;
+
+	ret = clk_set_default_rates(dev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
 # endif /* OF_PLATDATA */
 
 int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk)
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 144ac2a..940a153 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -11,6 +11,7 @@
 
 #include <common.h>
 #include <asm/io.h>
+#include <clk.h>
 #include <fdtdec.h>
 #include <fdt_support.h>
 #include <malloc.h>
@@ -391,6 +392,11 @@ int device_probe(struct udevice *dev)
 			goto fail;
 	}
 
+	/* Process 'assigned-{clocks/clock-parents/clock-rates}' properties */
+	ret = clk_set_defaults(dev);
+	if (ret)
+		goto fail;
+
 	if (drv->probe) {
 		ret = drv->probe(dev);
 		if (ret) {
diff --git a/include/clk.h b/include/clk.h
index e463d8e..a7d95d3 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -133,6 +133,23 @@ static inline int clk_release_all(struct clk *clk, int count)
 
 #endif
 
+#if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) && \
+	CONFIG_IS_ENABLED(CLK)
+/**
+ * clk_set_defaults - Process 'assigned-{clocks/clock-parents/clock-rates}'
+ *                    properties to configure clocks
+ *
+ * @dev:        A device to process (the ofnode associated with this device
+ *              will be processed).
+ */
+int clk_set_defaults(struct udevice *dev);
+#else
+static inline int clk_set_defaults(struct udevice *dev)
+{
+	return 0;
+}
+#endif
+
 /**
  * clk_request - Request a clock by provider-specific ID.
  *
-- 
2.1.4

[U-Boot] [U-Boot,v2,4/5] clk: implement clk_set_defaults()

[Philipp Tomsich]

> Linux uses the properties 'assigned-clocks', 'assigned-clock-parents'
> and 'assigned-clock-rates' to configure the clock subsystem for use
> with various peripheral nodes.
> 
> This implements clk_set_defaults() and hooks it up with the general
> device probibin in drivers/core/device.c: when a new device is probed,
> clk_set_defaults() will be called for it and will process the
> properties mentioned above.
> 
> Note that this functionality is designed to fail gracefully (i.e. if a
> clock-driver does not implement set_parent(), we simply accept this
> and ignore the error) as not to break existing board-support.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
> 
> Changes in v2:
> - Fixed David's email address.
> 
>  drivers/clk/clk-uclass.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/core/device.c    |   6 +++
>  include/clk.h            |  17 +++++++
>  3 files changed, 141 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot,v2,4/5] clk: implement clk_set_defaults()

[Michael Nazzareno Trimarchi]

Hi



On 28 Jan. 2018 5:50 pm, "Philipp Tomsich" <
philipp.tomsich@theobroma-systems.com> wrote:

> Linux uses the properties 'assigned-clocks', 'assigned-clock-parents'
> and 'assigned-clock-rates' to configure the clock subsystem for use
> with various peripheral nodes.
>
> This implements clk_set_defaults() and hooks it up with the general
> device probibin in drivers/core/device.c: when a new device is probed,
> clk_set_defaults() will be called for it and will process the
> properties mentioned above.
>
> Note that this functionality is designed to fail gracefully (i.e. if a
> clock-driver does not implement set_parent(), we simply accept this
> and ignore the error) as not to break existing board-support.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
>
> Changes in v2:
> - Fixed David's email address.
>
>  drivers/clk/clk-uclass.c | 118 ++++++++++++++++++++++++++++++
+++++++++++++++++
>  drivers/core/device.c    |   6 +++
>  include/clk.h            |  17 +++++++
>  3 files changed, 141 insertions(+)
>

Applied to u-boot-rockchip, thanks!


Is the right thing to do to apply a general change without more review?

Michael

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [U-Boot,v2,4/5] clk: implement clk_set_defaults()

[Dr. Philipp Tomsich]

Michael,

> On 28 Jan 2018, at 17:54, Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
> 
> Hi
> 
> 
> 
> On 28 Jan. 2018 5:50 pm, "Philipp Tomsich" <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
> > Linux uses the properties 'assigned-clocks', 'assigned-clock-parents'
> > and 'assigned-clock-rates' to configure the clock subsystem for use
> > with various peripheral nodes.
> >
> > This implements clk_set_defaults() and hooks it up with the general
> > device probibin in drivers/core/device.c: when a new device is probed,
> > clk_set_defaults() will be called for it and will process the
> > properties mentioned above.
> >
> > Note that this functionality is designed to fail gracefully (i.e. if a
> > clock-driver does not implement set_parent(), we simply accept this
> > and ignore the error) as not to break existing board-support.
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>>
> > Tested-by: David Wu <david.wu at rock-chips.com <mailto:david.wu@rock-chips.com>>
> > ---
> >
> > Changes in v2:
> > - Fixed David's email address.
> >
> >  drivers/clk/clk-uclass.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/core/device.c    |   6 +++
> >  include/clk.h            |  17 +++++++
> >  3 files changed, 141 insertions(+)
> >
> 
> Applied to u-boot-rockchip, thanks!
> 
> Is the right thing to do to apply a general change without more review?

Generally not, but this one has been floating around for a while, received
testing and blocks a very lengthy series from David that has already been
been submitted for the previous iteration.

Plus: it simply mimics the Linux behaviour in U-Boot.

In other words: this feels like something that needs to go into rc1 and with
the window closing rapidly, I am between a rock and a hard place...

> 
> Michael
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de>
> https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>
> 

[U-Boot] [U-Boot,v2,4/5] clk: implement clk_set_defaults()

[Michael Nazzareno Trimarchi]

Hi



On 28 Jan. 2018 6:15 pm, "Dr. Philipp Tomsich" <
philipp.tomsich@theobroma-systems.com> wrote:

Michael,

On 28 Jan 2018, at 17:54, Michael Nazzareno Trimarchi <
michael@amarulasolutions.com> wrote:

Hi



On 28 Jan. 2018 5:50 pm, "Philipp Tomsich" <philipp.tomsich@theobroma-
systems.com> wrote:

> Linux uses the properties 'assigned-clocks', 'assigned-clock-parents'
> and 'assigned-clock-rates' to configure the clock subsystem for use
> with various peripheral nodes.
>
> This implements clk_set_defaults() and hooks it up with the general
> device probibin in drivers/core/device.c: when a new device is probed,
> clk_set_defaults() will be called for it and will process the
> properties mentioned above.
>
> Note that this functionality is designed to fail gracefully (i.e. if a
> clock-driver does not implement set_parent(), we simply accept this
> and ignore the error) as not to break existing board-support.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
>
> Changes in v2:
> - Fixed David's email address.
>
>  drivers/clk/clk-uclass.c | 118 ++++++++++++++++++++++++++++++
+++++++++++++++++
>  drivers/core/device.c    |   6 +++
>  include/clk.h            |  17 +++++++
>  3 files changed, 141 insertions(+)
>

Applied to u-boot-rockchip, thanks!


Is the right thing to do to apply a general change without more review?


Generally not, but this one has been floating around for a while, received
testing and blocks a very lengthy series from David that has already been
been submitted for the previous iteration.

Plus: it simply mimics the Linux behaviour in U-Boot.

In other words: this feels like something that needs to go into rc1 and with
the window closing rapidly, I am between a rock and a hard place...


Understand the point but changes that are bounded on some subsystem should
be picked up from the manteiner and accepted. I'm not against those changes
because I have done a quick check but this break a bit what should be the
correct flow.

Anyway David Wu is rockchip or theobroma because I have seen both emails ;)

Michael



Michael

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [U-Boot,v2,4/5] clk: implement clk_set_defaults()

[Dr. Philipp Tomsich]

> On 28 Jan 2018, at 18:21, Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
> 
> Hi
> 
> 
> 
> On 28 Jan. 2018 6:15 pm, "Dr. Philipp Tomsich" <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
> Michael,
> 
>> On 28 Jan 2018, at 17:54, Michael Nazzareno Trimarchi <michael at amarulasolutions.com <mailto:michael@amarulasolutions.com>> wrote:
>> 
>> Hi
>> 
>> 
>> 
>> On 28 Jan. 2018 5:50 pm, "Philipp Tomsich" <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
>> > Linux uses the properties 'assigned-clocks', 'assigned-clock-parents'
>> > and 'assigned-clock-rates' to configure the clock subsystem for use
>> > with various peripheral nodes.
>> >
>> > This implements clk_set_defaults() and hooks it up with the general
>> > device probibin in drivers/core/device.c: when a new device is probed,
>> > clk_set_defaults() will be called for it and will process the
>> > properties mentioned above.
>> >
>> > Note that this functionality is designed to fail gracefully (i.e. if a
>> > clock-driver does not implement set_parent(), we simply accept this
>> > and ignore the error) as not to break existing board-support.
>> >
>> > Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>>
>> > Tested-by: David Wu <david.wu at rock-chips.com <mailto:david.wu@rock-chips.com>>
>> > ---
>> >
>> > Changes in v2:
>> > - Fixed David's email address.
>> >
>> >  drivers/clk/clk-uclass.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++
>> >  drivers/core/device.c    |   6 +++
>> >  include/clk.h            |  17 +++++++
>> >  3 files changed, 141 insertions(+)
>> >
>> 
>> Applied to u-boot-rockchip, thanks!
>> 
>> Is the right thing to do to apply a general change without more review?
> 
> Generally not, but this one has been floating around for a while, received
> testing and blocks a very lengthy series from David that has already been
> been submitted for the previous iteration.
> 
> Plus: it simply mimics the Linux behaviour in U-Boot.
> 
> In other words: this feels like something that needs to go into rc1 and with
> the window closing rapidly, I am between a rock and a hard place...
> 
> Understand the point but changes that are bounded on some subsystem should be picked up from the manteiner and accepted. I'm not against those changes because I have done a quick check but this break a bit what should be the correct flow.
> 
> Anyway David Wu is rockchip or theobroma because I have seen both emails ;)

David is Rockchip and I screwed up when adding the “Tested-by”.
So you may have seen both emails, but one was just my reflexive typing on
my end (I realised that, when I received bounces from our own mail-server).

Cheers,
Phil.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 529 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180128/72992141/attachment.sig>

[U-Boot] [PATCH v2 5/5] rockchip: clk: rk3399: accept all assigned-clocks from the 'cru'-node

[Philipp Tomsich]

The RK3399 CRU-node assigns rates to a number of clocks that are not
implemented in the RK3399 clock-driver (but which have been
sufficiently initialised from rkclk_init()): for these clocks, we
simply ignore the set_rate() operation and return 0 to signal success.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: David Wu <david.wu@rock-chips.com>

---

Changes in v2:
- Fixed David's email address.

 drivers/clk/rockchip/clk_rk3399.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3399.c b/drivers/clk/rockchip/clk_rk3399.c
index e791936..e431ec8 100644
--- a/drivers/clk/rockchip/clk_rk3399.c
+++ b/drivers/clk/rockchip/clk_rk3399.c
@@ -883,6 +883,24 @@ static ulong rk3399_clk_set_rate(struct clk *clk, ulong rate)
 	switch (clk->id) {
 	case 0 ... 63:
 		return 0;
+
+	case ACLK_PERIHP:
+	case HCLK_PERIHP:
+	case PCLK_PERIHP:
+		return 0;
+
+	case ACLK_PERILP0:
+	case HCLK_PERILP0:
+	case PCLK_PERILP0:
+		return 0;
+
+	case ACLK_CCI:
+		return 0;
+
+	case HCLK_PERILP1:
+	case PCLK_PERILP1:
+		return 0;
+
 	case HCLK_SDMMC:
 	case SCLK_SDMMC:
 	case SCLK_EMMC:
-- 
2.1.4

[U-Boot] [U-Boot, v2, 5/5] rockchip: clk: rk3399: accept all assigned-clocks from the 'cru'-node

[Philipp Tomsich]

> The RK3399 CRU-node assigns rates to a number of clocks that are not
> implemented in the RK3399 clock-driver (but which have been
> sufficiently initialised from rkclk_init()): for these clocks, we
> simply ignore the set_rate() operation and return 0 to signal success.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: David Wu <david.wu@rock-chips.com>
> ---
> 
> Changes in v2:
> - Fixed David's email address.
> 
>  drivers/clk/rockchip/clk_rk3399.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 

Applied to u-boot-rockchip, thanks!