[U-Boot] [PATCH v2 0/5] clk: support assigned-clock, assigned-clock-parents, assigned-clock-rates
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(-)
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
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!
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)
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!
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, };
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!
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. *
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!
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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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 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@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> Tested-by: David Wu <david.wu@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@lists.denx.de mailto:U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot https://lists.denx.de/listinfo/u-boot
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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> wrote: Michael,
On 28 Jan 2018, at 17:54, Michael Nazzareno Trimarchi <michael@amarulasolutions.com mailto:michael@amarulasolutions.com> wrote:
Hi
On 28 Jan. 2018 5:50 pm, "Philipp Tomsich" <philipp.tomsich@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@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> Tested-by: David Wu <david.wu@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.
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:
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!
participants (3)
-
Dr. Philipp Tomsich -
Michael Nazzareno Trimarchi -
Philipp Tomsich