[U-Boot] [PATCH v1 0/5] clk: Add a managed API and fix clock self-assignment
This is the second of a few series, the goal of which is to facilitate porting drivers from the linux kernel. Most of the series will be about adding managed API to existing infrastructure (GPIO, reset, phy,...)
This particular series is about clocks. It adds a managed API, using the same API as linux. It also fix a use-case where the clock is initialized (rate and/or mux) when it is probed using the assigned-clocks-* properties.
JJ
Jean-Jacques Hiblot (5): drivers: clk: Handle gracefully NULL pointers drivers: clk: Add a managed API to get clocks from the device-tree test: clk: Update tests to also check the managed API drivers: clk: Fix using assigned-clocks in the node of the clock it sets up test: clk: test clock self assignment
arch/sandbox/dts/test.dts | 8 +- arch/sandbox/include/asm/clk.h | 33 ++++++++ drivers/clk/clk-uclass.c | 139 ++++++++++++++++++++++++++++++--- drivers/clk/clk_sandbox.c | 56 +++++++++++++ drivers/clk/clk_sandbox_test.c | 66 +++++++++++++--- drivers/core/device.c | 2 +- include/clk.h | 53 ++++++++++++- test/dm/clk.c | 36 ++++++++- 8 files changed, 364 insertions(+), 29 deletions(-)
Prepare the way for a managed CLK API by handling NULL pointers without crashing nor failing.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
drivers/clk/clk-uclass.c | 43 +++++++++++++++++++++++++++++++++------- include/clk.h | 2 +- 2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 64c181f4ad..dff395fedb 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -349,9 +349,12 @@ int clk_release_all(struct clk *clk, int count)
int clk_request(struct udevice *dev, struct clk *clk) { - const struct clk_ops *ops = clk_dev_ops(dev); + const struct clk_ops *ops;
debug("%s(dev=%p, clk=%p)\n", __func__, dev, clk); + if (!clk) + return 0; + ops = clk_dev_ops(dev);
clk->dev = dev;
@@ -363,9 +366,12 @@ int clk_request(struct udevice *dev, struct clk *clk)
int clk_free(struct clk *clk) { - const struct clk_ops *ops = clk_dev_ops(clk->dev); + const struct clk_ops *ops;
debug("%s(clk=%p)\n", __func__, clk); + if (!clk) + return 0; + ops = clk_dev_ops(clk->dev);
if (!ops->free) return 0; @@ -375,9 +381,12 @@ int clk_free(struct clk *clk)
ulong clk_get_rate(struct clk *clk) { - const struct clk_ops *ops = clk_dev_ops(clk->dev); + const struct clk_ops *ops;
debug("%s(clk=%p)\n", __func__, clk); + if (!clk) + return 0; + ops = clk_dev_ops(clk->dev);
if (!ops->get_rate) return -ENOSYS; @@ -391,6 +400,8 @@ struct clk *clk_get_parent(struct clk *clk) struct clk *pclk;
debug("%s(clk=%p)\n", __func__, clk); + if (!clk) + return NULL;
pdev = dev_get_parent(clk->dev); pclk = dev_get_clk_ptr(pdev); @@ -406,6 +417,8 @@ long long clk_get_parent_rate(struct clk *clk) struct clk *pclk;
debug("%s(clk=%p)\n", __func__, clk); + if (!clk) + return 0;
pclk = clk_get_parent(clk); if (IS_ERR(pclk)) @@ -424,9 +437,12 @@ long long clk_get_parent_rate(struct clk *clk)
ulong clk_set_rate(struct clk *clk, ulong rate) { - const struct clk_ops *ops = clk_dev_ops(clk->dev); + const struct clk_ops *ops;
debug("%s(clk=%p, rate=%lu)\n", __func__, clk, rate); + if (!clk) + return 0; + ops = clk_dev_ops(clk->dev);
if (!ops->set_rate) return -ENOSYS; @@ -436,9 +452,12 @@ ulong clk_set_rate(struct clk *clk, ulong rate)
int clk_set_parent(struct clk *clk, struct clk *parent) { - const struct clk_ops *ops = clk_dev_ops(clk->dev); + const struct clk_ops *ops;
debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent); + if (!clk) + return 0; + ops = clk_dev_ops(clk->dev);
if (!ops->set_parent) return -ENOSYS; @@ -448,11 +467,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
int clk_enable(struct clk *clk) { - const struct clk_ops *ops = clk_dev_ops(clk->dev); + const struct clk_ops *ops; struct clk *clkp = NULL; int ret;
debug("%s(clk=%p)\n", __func__, clk); + if (!clk) + return 0; + ops = clk_dev_ops(clk->dev);
if (CONFIG_IS_ENABLED(CLK_CCF)) { /* Take id 0 as a non-valid clk, such as dummy */ @@ -505,11 +527,14 @@ int clk_enable_bulk(struct clk_bulk *bulk)
int clk_disable(struct clk *clk) { - const struct clk_ops *ops = clk_dev_ops(clk->dev); + const struct clk_ops *ops; struct clk *clkp = NULL; int ret;
debug("%s(clk=%p)\n", __func__, clk); + if (!clk) + return 0; + ops = clk_dev_ops(clk->dev);
if (CONFIG_IS_ENABLED(CLK_CCF)) { if (clk->id && !clk_get_by_id(clk->id, &clkp)) { @@ -589,6 +614,10 @@ bool clk_is_match(const struct clk *p, const struct clk *q) if (p == q) return true;
+ /* trivial case #2: on the clk pointer is NULL */ + if (!p || !q) + return false; + /* same device, id and data */ if (p->dev == q->dev && p->id == q->id && p->data == q->data) return true; diff --git a/include/clk.h b/include/clk.h index 18b2e3ca54..6568865d40 100644 --- a/include/clk.h +++ b/include/clk.h @@ -356,7 +356,7 @@ int soc_clk_dump(void); */ static inline bool clk_valid(struct clk *clk) { - return !!clk->dev; + return clk && !!clk->dev; }
/**
Hi Jean-Jacques,
On Mon, 30 Sep 2019 at 06:29, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Prepare the way for a managed CLK API by handling NULL pointers without crashing nor failing.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/clk/clk-uclass.c | 43 +++++++++++++++++++++++++++++++++------- include/clk.h | 2 +- 2 files changed, 37 insertions(+), 8 deletions(-)
This increases code size so can we make it a Kconfig option?
Why is someone calling these functions with NULL?
Regards, Simon
Add devm_clk_get(), devm_clk_get_optional() to get clocks from the device-tree. The clocks is automatically released and the data structure freed when the device is unbound. Also add devm_clk_put() to release the clock and free the data structure manually.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
drivers/clk/clk-uclass.c | 48 ++++++++++++++++++++++++++++++++++++++++ include/clk.h | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index dff395fedb..e7ec6347de 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -625,6 +625,54 @@ bool clk_is_match(const struct clk *p, const struct clk *q) return false; }
+static void devm_clk_release(struct udevice *dev, void *res) +{ + clk_free(res); +} + +static int devm_clk_match(struct udevice *dev, void *res, void *data) +{ + return res == data; +} + +struct clk *devm_clk_get(struct udevice *dev, const char *id) +{ + int rc; + struct clk *clk; + + clk = devres_alloc(devm_clk_release, sizeof(struct clk), __GFP_ZERO); + if (unlikely(!clk)) + return ERR_PTR(-ENOMEM); + + rc = clk_get_by_name(dev, id, clk); + if (rc) + return ERR_PTR(rc); + + devres_add(dev, clk); + return clk; +} + +struct clk *devm_clk_get_optional(struct udevice *dev, const char *id) +{ + struct clk *clk = devm_clk_get(dev, id); + + if (IS_ERR(clk)) + return NULL; + + return clk; +} + +void devm_clk_put(struct udevice *dev, struct clk *clk) +{ + int rc; + + if (!clk) + return; + + rc = devres_release(dev, devm_clk_release, devm_clk_match, clk); + WARN_ON(rc); +} + UCLASS_DRIVER(clk) = { .id = UCLASS_CLK, .name = "clk", diff --git a/include/clk.h b/include/clk.h index 6568865d40..6c5bf8ad67 100644 --- a/include/clk.h +++ b/include/clk.h @@ -154,6 +154,37 @@ int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk); */ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
+/** + * devm_clk_get - lookup and obtain a managed reference to a clock producer. + * @dev: device for clock "consumer" + * @id: clock consumer ID + * + * Returns a struct clk corresponding to the clock producer, or + * valid IS_ERR() condition containing errno. The implementation + * uses @dev and @id to determine the clock consumer, and thereby + * the clock producer. (IOW, @id may be identical strings, but + * clk_get may return different clock producers depending on @dev.) + * + * Drivers must assume that the clock source is not enabled. + * + * devm_clk_get should not be called from within interrupt context. + * + * The clock will automatically be freed when the device is unbound + * from the bus. + */ +struct clk *devm_clk_get(struct udevice *dev, const char *id); + +/** + * devm_clk_get_optional - lookup and obtain a managed reference to an optional + * clock producer. + * @dev: device for clock "consumer" + * @id: clock consumer ID + * + * Behaves the same as devm_clk_get() except where there is no clock producer. + * In this case, instead of returning -ENOENT, the function returns NULL. + */ +struct clk *devm_clk_get_optional(struct udevice *dev, const char *id); + /** * clk_release_all() - Disable (turn off)/Free an array of previously * requested clocks. @@ -168,6 +199,19 @@ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk); */ int clk_release_all(struct clk *clk, int count);
+/** + * devm_clk_put - "free" a managed clock source + * @dev: device used to acquire the clock + * @clk: clock source acquired with devm_clk_get() + * + * Note: drivers must ensure that all clk_enable calls made on this + * clock source are balanced by clk_disable calls prior to calling + * this function. + * + * clk_put should not be called from within interrupt context. + */ +void devm_clk_put(struct udevice *dev, struct clk *clk); + #else static inline int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
On Mon, 30 Sep 2019 at 06:29, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Add devm_clk_get(), devm_clk_get_optional() to get clocks from the device-tree. The clocks is automatically released and the data structure freed when the device is unbound. Also add devm_clk_put() to release the clock and free the data structure manually.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/clk/clk-uclass.c | 48 ++++++++++++++++++++++++++++++++++++++++ include/clk.h | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
+Stephen Warren
Add a few more clocks the clk_sandbox clock provider and get them using the managed API. Make sure they are released when the device is removed.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
arch/sandbox/dts/test.dts | 6 ++-- arch/sandbox/include/asm/clk.h | 33 +++++++++++++++++ drivers/clk/clk_sandbox.c | 34 ++++++++++++++++++ drivers/clk/clk_sandbox_test.c | 66 +++++++++++++++++++++++++++++----- test/dm/clk.c | 36 ++++++++++++++++++- 5 files changed, 163 insertions(+), 12 deletions(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 27b0baab27..25c238d8e5 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -228,8 +228,10 @@ compatible = "sandbox,clk-test"; clocks = <&clk_fixed>, <&clk_sandbox 1>, - <&clk_sandbox 0>; - clock-names = "fixed", "i2c", "spi"; + <&clk_sandbox 0>, + <&clk_sandbox 3>, + <&clk_sandbox 2>; + clock-names = "fixed", "i2c", "spi", "uart2", "uart1"; };
ccf: clk-ccf { diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h index 2b1c49f783..1573e4a134 100644 --- a/arch/sandbox/include/asm/clk.h +++ b/arch/sandbox/include/asm/clk.h @@ -19,6 +19,8 @@ struct udevice; enum sandbox_clk_id { SANDBOX_CLK_ID_SPI, SANDBOX_CLK_ID_I2C, + SANDBOX_CLK_ID_UART1, + SANDBOX_CLK_ID_UART2,
SANDBOX_CLK_ID_COUNT, }; @@ -33,10 +35,15 @@ enum sandbox_clk_test_id { SANDBOX_CLK_TEST_ID_FIXED, SANDBOX_CLK_TEST_ID_SPI, SANDBOX_CLK_TEST_ID_I2C, + SANDBOX_CLK_TEST_ID_DEVM1, + SANDBOX_CLK_TEST_ID_DEVM2, + SANDBOX_CLK_TEST_ID_DEVM_NULL,
SANDBOX_CLK_TEST_ID_COUNT, };
+#define SANDBOX_CLK_TEST_NON_DEVM_COUNT SANDBOX_CLK_TEST_ID_DEVM1 + /** * sandbox_clk_query_rate - Query the current rate of a sandbox clock. * @@ -53,6 +60,14 @@ ulong sandbox_clk_query_rate(struct udevice *dev, int id); * @return: The rate of the clock. */ int sandbox_clk_query_enable(struct udevice *dev, int id); +/** + * sandbox_clk_query_requested - Query the requested state of a sandbox clock. + * + * @dev: The sandbox clock provider device. + * @id: The clock to query. + * @return: The rate of the clock. + */ +int sandbox_clk_query_requested(struct udevice *dev, int id);
/** * sandbox_clk_test_get - Ask the sandbox clock test device to request its @@ -62,6 +77,16 @@ int sandbox_clk_query_enable(struct udevice *dev, int id); * @return: 0 if OK, or a negative error code. */ int sandbox_clk_test_get(struct udevice *dev); + +/** + * sandbox_clk_test_devm_get - Ask the sandbox clock test device to request its + * clocks using the managed API. + * + * @dev: The sandbox clock test (client) devivce. + * @return: 0 if OK, or a negative error code. + */ +int sandbox_clk_test_devm_get(struct udevice *dev); + /** * sandbox_clk_test_get_bulk - Ask the sandbox clock test device to request its * clocks with the bulk clk API. @@ -146,5 +171,13 @@ int sandbox_clk_test_release_bulk(struct udevice *dev); * @return: 0 if OK, or a negative error code. */ int sandbox_clk_test_valid(struct udevice *dev); +/** + * sandbox_clk_test_valid - Ask the sandbox clock test device to check its + * clocks are valid. + * + * @dev: The sandbox clock test (client) devivce. + * @return: 0 if OK, or a negative error code. + */ +struct clk *sandbox_clk_test_get_devm_clk(struct udevice *dev, int id);
#endif diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c index 1d5cbb589a..d152fd7e5b 100644 --- a/drivers/clk/clk_sandbox.c +++ b/drivers/clk/clk_sandbox.c @@ -12,6 +12,7 @@ struct sandbox_clk_priv { ulong rate[SANDBOX_CLK_ID_COUNT]; bool enabled[SANDBOX_CLK_ID_COUNT]; + bool requested[SANDBOX_CLK_ID_COUNT]; };
static ulong sandbox_clk_get_rate(struct clk *clk) @@ -65,11 +66,35 @@ static int sandbox_clk_disable(struct clk *clk) return 0; }
+static int sandbox_clk_request(struct clk *clk) +{ + struct sandbox_clk_priv *priv = dev_get_priv(clk->dev); + + if (clk->id >= SANDBOX_CLK_ID_COUNT) + return -EINVAL; + + priv->requested[clk->id] = true; + return 0; +} + +static int sandbox_clk_free(struct clk *clk) +{ + struct sandbox_clk_priv *priv = dev_get_priv(clk->dev); + + if (clk->id >= SANDBOX_CLK_ID_COUNT) + return -EINVAL; + + priv->requested[clk->id] = false; + return 0; +} + static struct clk_ops sandbox_clk_ops = { .get_rate = sandbox_clk_get_rate, .set_rate = sandbox_clk_set_rate, .enable = sandbox_clk_enable, .disable = sandbox_clk_disable, + .request = sandbox_clk_request, + .free = sandbox_clk_free, };
static const struct udevice_id sandbox_clk_ids[] = { @@ -104,3 +129,12 @@ int sandbox_clk_query_enable(struct udevice *dev, int id)
return priv->enabled[id]; } + +int sandbox_clk_query_requested(struct udevice *dev, int id) +{ + struct sandbox_clk_priv *priv = dev_get_priv(dev); + + if (id < 0 || id >= SANDBOX_CLK_ID_COUNT) + return -EINVAL; + return priv->requested[id]; +} diff --git a/drivers/clk/clk_sandbox_test.c b/drivers/clk/clk_sandbox_test.c index e8465dbfad..41954660ea 100644 --- a/drivers/clk/clk_sandbox_test.c +++ b/drivers/clk/clk_sandbox_test.c @@ -9,7 +9,8 @@ #include <asm/clk.h>
struct sandbox_clk_test { - struct clk clks[SANDBOX_CLK_TEST_ID_COUNT]; + struct clk clks[SANDBOX_CLK_TEST_NON_DEVM_COUNT]; + struct clk *clkps[SANDBOX_CLK_TEST_ID_COUNT]; struct clk_bulk bulk; };
@@ -24,7 +25,7 @@ int sandbox_clk_test_get(struct udevice *dev) struct sandbox_clk_test *sbct = dev_get_priv(dev); int i, ret;
- for (i = 0; i < SANDBOX_CLK_TEST_ID_COUNT; i++) { + for (i = 0; i < SANDBOX_CLK_TEST_NON_DEVM_COUNT; i++) { ret = clk_get_by_name(dev, sandbox_clk_test_names[i], &sbct->clks[i]); if (ret) @@ -34,6 +35,37 @@ int sandbox_clk_test_get(struct udevice *dev) return 0; }
+int sandbox_clk_test_devm_get(struct udevice *dev) +{ + struct sandbox_clk_test *sbct = dev_get_priv(dev); + struct clk *clk; + + clk = devm_clk_get(dev, "no-an-existing-clock"); + if (!IS_ERR(clk)) { + dev_err(dev, "devm_clk_get() should have failed\n"); + return -EINVAL; + } + + clk = devm_clk_get(dev, "uart2"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + sbct->clkps[SANDBOX_CLK_TEST_ID_DEVM1] = clk; + + clk = devm_clk_get_optional(dev, "uart1"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + sbct->clkps[SANDBOX_CLK_TEST_ID_DEVM2] = clk; + + sbct->clkps[SANDBOX_CLK_TEST_ID_DEVM_NULL] = NULL; + clk = devm_clk_get_optional(dev, "not_an_existing_clock"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + if (clk) + return -EINVAL; + + return 0; +} + int sandbox_clk_test_get_bulk(struct udevice *dev) { struct sandbox_clk_test *sbct = dev_get_priv(dev); @@ -48,7 +80,7 @@ ulong sandbox_clk_test_get_rate(struct udevice *dev, int id) if (id < 0 || id >= SANDBOX_CLK_TEST_ID_COUNT) return -EINVAL;
- return clk_get_rate(&sbct->clks[id]); + return clk_get_rate(sbct->clkps[id]); }
ulong sandbox_clk_test_set_rate(struct udevice *dev, int id, ulong rate) @@ -58,7 +90,7 @@ ulong sandbox_clk_test_set_rate(struct udevice *dev, int id, ulong rate) if (id < 0 || id >= SANDBOX_CLK_TEST_ID_COUNT) return -EINVAL;
- return clk_set_rate(&sbct->clks[id], rate); + return clk_set_rate(sbct->clkps[id], rate); }
int sandbox_clk_test_enable(struct udevice *dev, int id) @@ -68,7 +100,7 @@ int sandbox_clk_test_enable(struct udevice *dev, int id) if (id < 0 || id >= SANDBOX_CLK_TEST_ID_COUNT) return -EINVAL;
- return clk_enable(&sbct->clks[id]); + return clk_enable(sbct->clkps[id]); }
int sandbox_clk_test_enable_bulk(struct udevice *dev) @@ -85,7 +117,7 @@ int sandbox_clk_test_disable(struct udevice *dev, int id) if (id < 0 || id >= SANDBOX_CLK_TEST_ID_COUNT) return -EINVAL;
- return clk_disable(&sbct->clks[id]); + return clk_disable(sbct->clkps[id]); }
int sandbox_clk_test_disable_bulk(struct udevice *dev) @@ -100,7 +132,8 @@ int sandbox_clk_test_free(struct udevice *dev) struct sandbox_clk_test *sbct = dev_get_priv(dev); int i, ret;
- for (i = 0; i < SANDBOX_CLK_TEST_ID_COUNT; i++) { + devm_clk_put(dev, sbct->clkps[SANDBOX_CLK_TEST_ID_DEVM1]); + for (i = 0; i < SANDBOX_CLK_TEST_NON_DEVM_COUNT; i++) { ret = clk_free(&sbct->clks[i]); if (ret) return ret; @@ -122,13 +155,27 @@ int sandbox_clk_test_valid(struct udevice *dev) int i;
for (i = 0; i < SANDBOX_CLK_TEST_ID_COUNT; i++) { - if (!clk_valid(&sbct->clks[i])) - return -EINVAL; + if (!clk_valid(sbct->clkps[i])) + if (i != SANDBOX_CLK_TEST_ID_DEVM_NULL) + return -EINVAL; }
return 0; }
+static int sandbox_clk_test_probe(struct udevice *dev) +{ + struct sandbox_clk_test *sbct = dev_get_priv(dev); + int i; + + for (i = 0; i < SANDBOX_CLK_TEST_ID_DEVM1; i++) + sbct->clkps[i] = &sbct->clks[i]; + for (i = SANDBOX_CLK_TEST_ID_DEVM1; i < SANDBOX_CLK_TEST_ID_COUNT; i++) + sbct->clkps[i] = NULL; + + return 0; +} + static const struct udevice_id sandbox_clk_test_ids[] = { { .compatible = "sandbox,clk-test" }, { } @@ -138,5 +185,6 @@ U_BOOT_DRIVER(sandbox_clk_test) = { .name = "sandbox_clk_test", .id = UCLASS_MISC, .of_match = sandbox_clk_test_ids, + .probe = sandbox_clk_test_probe, .priv_auto_alloc_size = sizeof(struct sandbox_clk_test), }; diff --git a/test/dm/clk.c b/test/dm/clk.c index 676ef217f0..3ad0ad8ca3 100644 --- a/test/dm/clk.c +++ b/test/dm/clk.c @@ -8,6 +8,7 @@ #include <dm.h> #include <asm/clk.h> #include <dm/test.h> +#include <dm/device-internal.h> #include <linux/err.h> #include <test/ut.h>
@@ -53,8 +54,19 @@ static int dm_test_clk(struct unit_test_state *uts) ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "clk-test", &dev_test)); ut_assertok(sandbox_clk_test_get(dev_test)); + ut_assertok(sandbox_clk_test_devm_get(dev_test)); ut_assertok(sandbox_clk_test_valid(dev_test));
+ ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, + SANDBOX_CLK_TEST_ID_DEVM_NULL)); + ut_asserteq(0, sandbox_clk_test_set_rate(dev_test, + SANDBOX_CLK_TEST_ID_DEVM_NULL, + 0)); + ut_asserteq(0, sandbox_clk_test_enable(dev_test, + SANDBOX_CLK_TEST_ID_DEVM_NULL)); + ut_asserteq(0, sandbox_clk_test_disable(dev_test, + SANDBOX_CLK_TEST_ID_DEVM_NULL)); + ut_asserteq(1234, sandbox_clk_test_get_rate(dev_test, SANDBOX_CLK_TEST_ID_FIXED)); @@ -62,6 +74,10 @@ static int dm_test_clk(struct unit_test_state *uts) SANDBOX_CLK_TEST_ID_SPI)); ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, SANDBOX_CLK_TEST_ID_I2C)); + ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, + SANDBOX_CLK_TEST_ID_DEVM1)); + ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, + SANDBOX_CLK_TEST_ID_DEVM2));
rate = sandbox_clk_test_set_rate(dev_test, SANDBOX_CLK_TEST_ID_FIXED, 12345); @@ -121,8 +137,25 @@ static int dm_test_clk(struct unit_test_state *uts) ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_SPI)); ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_I2C));
+ ut_asserteq(1, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_SPI)); + ut_asserteq(1, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_I2C)); + ut_asserteq(1, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_UART2)); ut_assertok(sandbox_clk_test_free(dev_test)); - + ut_asserteq(0, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_SPI)); + ut_asserteq(0, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_I2C)); + ut_asserteq(0, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_UART2)); + + ut_asserteq(1, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_UART1)); + ut_assertok(device_remove(dev_test, DM_REMOVE_NORMAL)); + ut_asserteq(0, sandbox_clk_query_requested(dev_clk, + SANDBOX_CLK_ID_UART1)); return 0; } DM_TEST(dm_test_clk, DM_TESTF_SCAN_FDT); @@ -159,6 +192,7 @@ static int dm_test_clk_bulk(struct unit_test_state *uts) ut_assertok(sandbox_clk_test_release_bulk(dev_test)); ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_SPI)); ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_I2C)); + ut_assertok(device_remove(dev_test, DM_REMOVE_NORMAL));
return 0; }
On Mon, 30 Sep 2019 at 06:29, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Add a few more clocks the clk_sandbox clock provider and get them using the managed API. Make sure they are released when the device is removed.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/sandbox/dts/test.dts | 6 ++-- arch/sandbox/include/asm/clk.h | 33 +++++++++++++++++ drivers/clk/clk_sandbox.c | 34 ++++++++++++++++++ drivers/clk/clk_sandbox_test.c | 66 +++++++++++++++++++++++++++++----- test/dm/clk.c | 36 ++++++++++++++++++- 5 files changed, 163 insertions(+), 12 deletions(-)
+Stephen Warren
Reviewed-by: Simon Glass sjg@chromium.org
This fixes the case where assigned-clocks is used to define a clock defaults inside this same clock's node. This is used sometimes to setup a default parents and/or rate for a clock.
example: muxed_clock: muxed_clock { clocks = <&clk_provider 0>, <&clk_provider 1>; #clock-cells = <0>; assigned-clocks = <&muxed_clock>; assigned-clock-parents = <&clk_provider 1>; };
It doesn't work in u-boot because the assigned-clocks are setup *before* the clock is probed. (clk_set_parent() will likely crash or fail if called before the device probe function) Making it work by handling "assigned-clocks" in 2 steps: first before the clk device is probed, and then after the clk device is probed.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
drivers/clk/clk-uclass.c | 48 +++++++++++++++++++++++++++++++++++----- drivers/core/device.c | 2 +- include/clk.h | 7 ++++-- 3 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index e7ec6347de..75d618a47b 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -178,7 +178,7 @@ bulk_get_err: return ret; }
-static int clk_set_default_parents(struct udevice *dev) +static int clk_set_default_parents(struct udevice *dev, int stage) { struct clk clk, parent_clk; int index; @@ -214,8 +214,18 @@ static int clk_set_default_parents(struct udevice *dev) return ret; }
- ret = clk_set_parent(&clk, &parent_clk); + /* This is clk provider device trying to reparent itself + * It cannot be done right now but need to wait after the + * device is probed + */ + if (stage == 0 && clk.dev == dev) + continue; + + if (stage > 0 && clk.dev != dev) + /* do not setup twice the parent clocks */ + continue;
+ ret = clk_set_parent(&clk, &parent_clk); /* * Not all drivers may support clock-reparenting (as of now). * Ignore errors due to this. @@ -233,7 +243,7 @@ static int clk_set_default_parents(struct udevice *dev) return 0; }
-static int clk_set_default_rates(struct udevice *dev) +static int clk_set_default_rates(struct udevice *dev, int stage) { struct clk clk; int index; @@ -268,7 +278,19 @@ static int clk_set_default_rates(struct udevice *dev) continue; }
+ /* This is clk provider device trying to program itself + * It cannot be done right now but need to wait after the + * device is probed + */ + if (stage == 0 && clk.dev == dev) + continue; + + if (stage > 0 && clk.dev != dev) + /* do not setup twice the parent clocks */ + continue; + ret = clk_set_rate(&clk, rates[index]); + if (ret < 0) { debug("%s: failed to set rate on clock index %d (%ld) for %s\n", __func__, index, clk.id, dev_read_name(dev)); @@ -281,7 +303,7 @@ fail: return ret; }
-int clk_set_defaults(struct udevice *dev) +int clk_set_defaults(struct udevice *dev, int stage) { int ret;
@@ -294,11 +316,11 @@ int clk_set_defaults(struct udevice *dev)
debug("%s(%s)\n", __func__, dev_read_name(dev));
- ret = clk_set_default_parents(dev); + ret = clk_set_default_parents(dev, stage); if (ret) return ret;
- ret = clk_set_default_rates(dev); + ret = clk_set_default_rates(dev, stage); if (ret < 0) return ret;
@@ -673,7 +695,21 @@ void devm_clk_put(struct udevice *dev, struct clk *clk) WARN_ON(rc); }
+int clk_uclass_post_probe(struct udevice *dev) +{ + /* + * when a clock provider is probed. Call clk_set_defaults() + * also after the device is probed. This takes care of cases + * where the DT is used to setup default parents and rates + * using assigned-clocks + */ + clk_set_defaults(dev, 1); + + return 0; +} + UCLASS_DRIVER(clk) = { .id = UCLASS_CLK, .name = "clk", + .post_probe = clk_uclass_post_probe, }; diff --git a/drivers/core/device.c b/drivers/core/device.c index 05dadf98f9..cbcbb1b15d 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -416,7 +416,7 @@ int device_probe(struct udevice *dev) * Process 'assigned-{clocks/clock-parents/clock-rates}' * properties */ - ret = clk_set_defaults(dev); + ret = clk_set_defaults(dev, 0); if (ret) goto fail; } diff --git a/include/clk.h b/include/clk.h index 6c5bf8ad67..f6739f7149 100644 --- a/include/clk.h +++ b/include/clk.h @@ -244,10 +244,13 @@ static inline int clk_release_all(struct clk *clk, int count) * * @dev: A device to process (the ofnode associated with this device * will be processed). + * @stage: A integer. 0 indicates that this is called before the device + * is probed. 1 indicates that this is called just after the + * device has been probed */ -int clk_set_defaults(struct udevice *dev); +int clk_set_defaults(struct udevice *dev, int stage); #else -static inline int clk_set_defaults(struct udevice *dev) +static inline int clk_set_defaults(struct udevice *dev, int stage) { return 0; }
Hi Jean-Jacques,
On Mon, 30 Sep 2019 at 06:29, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This fixes the case where assigned-clocks is used to define a clock defaults inside this same clock's node. This is used sometimes to setup a default parents and/or rate for a clock.
example: muxed_clock: muxed_clock { clocks = <&clk_provider 0>, <&clk_provider 1>; #clock-cells = <0>; assigned-clocks = <&muxed_clock>; assigned-clock-parents = <&clk_provider 1>; };
It doesn't work in u-boot because the assigned-clocks are setup *before*
U-Boot
the clock is probed. (clk_set_parent() will likely crash or fail if called before the device probe function) Making it work by handling "assigned-clocks" in 2 steps: first before the clk device is probed, and then after the clk device is probed.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/clk/clk-uclass.c | 48 +++++++++++++++++++++++++++++++++++----- drivers/core/device.c | 2 +- include/clk.h | 7 ++++-- 3 files changed, 48 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
+Stephen Warren who is the expert here
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index e7ec6347de..75d618a47b 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -178,7 +178,7 @@ bulk_get_err: return ret; }
-static int clk_set_default_parents(struct udevice *dev) +static int clk_set_default_parents(struct udevice *dev, int stage) { struct clk clk, parent_clk; int index; @@ -214,8 +214,18 @@ static int clk_set_default_parents(struct udevice *dev) return ret; }
ret = clk_set_parent(&clk, &parent_clk);
/* This is clk provider device trying to reparent itself
/* * This ...
* It cannot be done right now but need to wait after the* device is probed*/if (stage == 0 && clk.dev == dev)continue;if (stage > 0 && clk.dev != dev)/* do not setup twice the parent clocks */continue;ret = clk_set_parent(&clk, &parent_clk); /* * Not all drivers may support clock-reparenting (as of now). * Ignore errors due to this.@@ -233,7 +243,7 @@ static int clk_set_default_parents(struct udevice *dev) return 0; }
-static int clk_set_default_rates(struct udevice *dev) +static int clk_set_default_rates(struct udevice *dev, int stage) { struct clk clk; int index; @@ -268,7 +278,19 @@ static int clk_set_default_rates(struct udevice *dev) continue; }
/* This is clk provider device trying to program itself* It cannot be done right now but need to wait after the* device is probed*/
wait until after?
Regards, Simon
Make sure that the clock self-assignment works by having a clock of clk-sbox be configured automatically when clk-sbox is probed.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
arch/sandbox/dts/test.dts | 2 ++ drivers/clk/clk_sandbox.c | 22 ++++++++++++++++++++++ test/dm/clk.c | 4 ++-- 3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 25c238d8e5..0917255476 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -222,6 +222,8 @@ clk_sandbox: clk-sbox { compatible = "sandbox,clk"; #clock-cells = <1>; + assigned-clocks = <&clk_sandbox 3>; + assigned-clock-rates = <321>; };
clk-test { diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c index d152fd7e5b..de6b2f7c82 100644 --- a/drivers/clk/clk_sandbox.c +++ b/drivers/clk/clk_sandbox.c @@ -10,6 +10,7 @@ #include <asm/clk.h>
struct sandbox_clk_priv { + bool probed; ulong rate[SANDBOX_CLK_ID_COUNT]; bool enabled[SANDBOX_CLK_ID_COUNT]; bool requested[SANDBOX_CLK_ID_COUNT]; @@ -19,6 +20,9 @@ static ulong sandbox_clk_get_rate(struct clk *clk) { struct sandbox_clk_priv *priv = dev_get_priv(clk->dev);
+ if (!priv->probed) + return -ENODEV; + if (clk->id >= SANDBOX_CLK_ID_COUNT) return -EINVAL;
@@ -30,6 +34,9 @@ static ulong sandbox_clk_set_rate(struct clk *clk, ulong rate) struct sandbox_clk_priv *priv = dev_get_priv(clk->dev); ulong old_rate;
+ if (!priv->probed) + return -ENODEV; + if (clk->id >= SANDBOX_CLK_ID_COUNT) return -EINVAL;
@@ -46,6 +53,9 @@ static int sandbox_clk_enable(struct clk *clk) { struct sandbox_clk_priv *priv = dev_get_priv(clk->dev);
+ if (!priv->probed) + return -ENODEV; + if (clk->id >= SANDBOX_CLK_ID_COUNT) return -EINVAL;
@@ -58,6 +68,9 @@ static int sandbox_clk_disable(struct clk *clk) { struct sandbox_clk_priv *priv = dev_get_priv(clk->dev);
+ if (!priv->probed) + return -ENODEV; + if (clk->id >= SANDBOX_CLK_ID_COUNT) return -EINVAL;
@@ -97,6 +110,14 @@ static struct clk_ops sandbox_clk_ops = { .free = sandbox_clk_free, };
+static int sandbox_clk_probe(struct udevice *dev) +{ + struct sandbox_clk_priv *priv = dev_get_priv(dev); + + priv->probed = true; + return 0; +} + static const struct udevice_id sandbox_clk_ids[] = { { .compatible = "sandbox,clk" }, { } @@ -107,6 +128,7 @@ U_BOOT_DRIVER(clk_sandbox) = { .id = UCLASS_CLK, .of_match = sandbox_clk_ids, .ops = &sandbox_clk_ops, + .probe = sandbox_clk_probe, .priv_auto_alloc_size = sizeof(struct sandbox_clk_priv), };
diff --git a/test/dm/clk.c b/test/dm/clk.c index 3ad0ad8ca3..31335a543f 100644 --- a/test/dm/clk.c +++ b/test/dm/clk.c @@ -74,8 +74,8 @@ static int dm_test_clk(struct unit_test_state *uts) SANDBOX_CLK_TEST_ID_SPI)); ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, SANDBOX_CLK_TEST_ID_I2C)); - ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, - SANDBOX_CLK_TEST_ID_DEVM1)); + ut_asserteq(321, sandbox_clk_test_get_rate(dev_test, + SANDBOX_CLK_TEST_ID_DEVM1)); ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, SANDBOX_CLK_TEST_ID_DEVM2));
On Mon, 30 Sep 2019 at 06:29, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Make sure that the clock self-assignment works by having a clock of clk-sbox be configured automatically when clk-sbox is probed.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/sandbox/dts/test.dts | 2 ++ drivers/clk/clk_sandbox.c | 22 ++++++++++++++++++++++ test/dm/clk.c | 4 ++-- 3 files changed, 26 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Hi Jean-Jacques,
This is the second of a few series, the goal of which is to facilitate porting drivers from the linux kernel. Most of the series will be about adding managed API to existing infrastructure (GPIO, reset, phy,...)
This particular series is about clocks. It adds a managed API, using the same API as linux. It also fix a use-case where the clock is initialized (rate and/or mux) when it is probed using the assigned-clocks-* properties.
This series when applied on newest -master branch causes build breaks (on for instance mips, bcm):
mips: + bcm968380gerg_ram
+drivers/mtd/nand/raw/built-in.o: In function `devm_clk_get':
+drivers/mtd/nand/raw/brcmnand/brcmnand_compat.c:7: multiple definition of `devm_clk_get'
Travis-CI result: https://travis-ci.org/lmajewski/u-boot-dfu/jobs/600367698
The branch to reproduce: https://github.com/lmajewski/u-boot-dfu/commits/testing
JJ
Jean-Jacques Hiblot (5): drivers: clk: Handle gracefully NULL pointers drivers: clk: Add a managed API to get clocks from the device-tree test: clk: Update tests to also check the managed API drivers: clk: Fix using assigned-clocks in the node of the clock it sets up test: clk: test clock self assignment
arch/sandbox/dts/test.dts | 8 +- arch/sandbox/include/asm/clk.h | 33 ++++++++ drivers/clk/clk-uclass.c | 139 ++++++++++++++++++++++++++++++--- drivers/clk/clk_sandbox.c | 56 +++++++++++++ drivers/clk/clk_sandbox_test.c | 66 +++++++++++++--- drivers/core/device.c | 2 +- include/clk.h | 53 ++++++++++++- test/dm/clk.c | 36 ++++++++- 8 files changed, 364 insertions(+), 29 deletions(-)
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
participants (3)
-
Jean-Jacques Hiblot -
Lukasz Majewski -
Simon Glass