* [U-Boot] [PATCH v2 1/5] drivers: clk: Handle gracefully NULL pointers
2019-10-22 12:00 [U-Boot] [PATCH v2 0/5] clk: Add a managed API and fix clock self-assignment Jean-Jacques Hiblot
@ 2019-10-22 12:00 ` Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 2/5] drivers: clk: Add a managed API to get clocks from the device-tree Jean-Jacques Hiblot
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jean-Jacques Hiblot @ 2019-10-22 12:00 UTC (permalink / raw)
To: u-boot
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>
---
Changes in v2: None
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;
}
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH v2 2/5] drivers: clk: Add a managed API to get clocks from the device-tree
2019-10-22 12:00 [U-Boot] [PATCH v2 0/5] clk: Add a managed API and fix clock self-assignment Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 1/5] drivers: clk: Handle gracefully NULL pointers Jean-Jacques Hiblot
@ 2019-10-22 12:00 ` Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 3/5] test: clk: Update tests to also check the managed API Jean-Jacques Hiblot
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jean-Jacques Hiblot @ 2019-10-22 12:00 UTC (permalink / raw)
To: u-boot
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>
---
Changes in v2:
- Add clk_prepare_enable/clk_disable_unprepare
- remove conflicting implmentations from brcmnand_compat.{c,h}
drivers/clk/clk-uclass.c | 48 +++++++++++++++++++
.../mtd/nand/raw/brcmnand/brcmnand_compat.c | 30 ------------
.../mtd/nand/raw/brcmnand/brcmnand_compat.h | 4 --
include/clk.h | 47 ++++++++++++++++++
4 files changed, 95 insertions(+), 34 deletions(-)
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/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.c b/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.c
index 96b27e6e5a..883948355c 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.c
@@ -3,36 +3,6 @@
#include <common.h>
#include "brcmnand_compat.h"
-struct clk *devm_clk_get(struct udevice *dev, const char *id)
-{
- struct clk *clk;
- int ret;
-
- clk = devm_kzalloc(dev, sizeof(*clk), GFP_KERNEL);
- if (!clk) {
- debug("%s: can't allocate clock\n", __func__);
- return ERR_PTR(-ENOMEM);
- }
-
- ret = clk_get_by_name(dev, id, clk);
- if (ret < 0) {
- debug("%s: can't get clock (ret = %d)!\n", __func__, ret);
- return ERR_PTR(ret);
- }
-
- return clk;
-}
-
-int clk_prepare_enable(struct clk *clk)
-{
- return clk_enable(clk);
-}
-
-void clk_disable_unprepare(struct clk *clk)
-{
- clk_disable(clk);
-}
-
static char *devm_kvasprintf(struct udevice *dev, gfp_t gfp, const char *fmt,
va_list ap)
{
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.h b/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.h
index 02cab0f828..6f9bec7085 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.h
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand_compat.h
@@ -6,10 +6,6 @@
#include <clk.h>
#include <dm.h>
-struct clk *devm_clk_get(struct udevice *dev, const char *id);
-int clk_prepare_enable(struct clk *clk);
-void clk_disable_unprepare(struct clk *clk);
-
char *devm_kasprintf(struct udevice *dev, gfp_t gfp, const char *fmt, ...);
#endif /* __BRCMNAND_COMPAT_H */
diff --git a/include/clk.h b/include/clk.h
index 6568865d40..9be3264113 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)
@@ -379,3 +423,6 @@ int clk_get_by_id(ulong id, struct clk **clkp);
*/
bool clk_dev_binded(struct clk *clk);
#endif
+
+#define clk_prepare_enable(clk) clk_enable(clk)
+#define clk_disable_unprepare(clk) clk_disable(clk)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH v2 3/5] test: clk: Update tests to also check the managed API
2019-10-22 12:00 [U-Boot] [PATCH v2 0/5] clk: Add a managed API and fix clock self-assignment Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 1/5] drivers: clk: Handle gracefully NULL pointers Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 2/5] drivers: clk: Add a managed API to get clocks from the device-tree Jean-Jacques Hiblot
@ 2019-10-22 12:00 ` Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 4/5] drivers: clk: Fix using assigned-clocks in the node of the clock it sets up Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 5/5] test: clk: test clock self assignment Jean-Jacques Hiblot
4 siblings, 0 replies; 6+ messages in thread
From: Jean-Jacques Hiblot @ 2019-10-22 12:00 UTC (permalink / raw)
To: u-boot
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>
---
Changes in v2: None
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 5d9ab3724f..0ee86a74ed 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -232,8 +232,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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH v2 4/5] drivers: clk: Fix using assigned-clocks in the node of the clock it sets up
2019-10-22 12:00 [U-Boot] [PATCH v2 0/5] clk: Add a managed API and fix clock self-assignment Jean-Jacques Hiblot
` (2 preceding siblings ...)
2019-10-22 12:00 ` [U-Boot] [PATCH v2 3/5] test: clk: Update tests to also check the managed API Jean-Jacques Hiblot
@ 2019-10-22 12:00 ` Jean-Jacques Hiblot
2019-10-22 12:00 ` [U-Boot] [PATCH v2 5/5] test: clk: test clock self assignment Jean-Jacques Hiblot
4 siblings, 0 replies; 6+ messages in thread
From: Jean-Jacques Hiblot @ 2019-10-22 12:00 UTC (permalink / raw)
To: u-boot
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>
---
Changes in v2: None
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 ce66c72e5e..854aa91bbf 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -417,7 +417,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 9be3264113..a5ee53d94a 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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH v2 5/5] test: clk: test clock self assignment
2019-10-22 12:00 [U-Boot] [PATCH v2 0/5] clk: Add a managed API and fix clock self-assignment Jean-Jacques Hiblot
` (3 preceding siblings ...)
2019-10-22 12:00 ` [U-Boot] [PATCH v2 4/5] drivers: clk: Fix using assigned-clocks in the node of the clock it sets up Jean-Jacques Hiblot
@ 2019-10-22 12:00 ` Jean-Jacques Hiblot
4 siblings, 0 replies; 6+ messages in thread
From: Jean-Jacques Hiblot @ 2019-10-22 12:00 UTC (permalink / raw)
To: u-boot
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>
---
Changes in v2: None
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 0ee86a74ed..c118125169 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -226,6 +226,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));
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread