* [PATCH v4 0/4] Add device links to clocks
@ 2019-01-08 16:19 Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 1/4] clk: core: link consumer with clock driver Miquel Raynal
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Miquel Raynal @ 2019-01-08 16:19 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: linux-clk, linux-kernel, linux-arm-kernel, Thomas Petazzoni,
Antoine Tenart, Maxime Chevallier, Gregory Clement, Nadav Haklai,
Miquel Raynal
Hello,
While working on suspend to RAM feature, I ran into troubles multiple
times when clocks where not suspending/resuming at the desired time. I
had a look at the core and I think the same logic as in the
regulator's core may be applied here to (very easily) fix this issue:
using device links.
The only additional change I had to do was to always (when available)
populate the device entry of the core clock structure so that it could
be used later. This is the purpose of patch 1. Patch 2 actually adds
support for device links.
Here is a step-by-step explanation of how links are managed, following
Maxime Ripard's suggestion.
The order of probe has no importance because the framework already
handles orphaned clocks so let's be simple and say there are two root
clocks, not depending on anything, that are probed first: xtal0 and
xtal1. None of these clocks have a parent, there is no device link in
the game, yet.
+----------------+ +----------------+
| | | |
| | | |
| xtal0 core | | xtal1 core |
| | | |
| | | |
+-------^^-------+ +-------^^-------+
|| ||
|| ||
+----------------+ +----------------+
| | | |
| xtal0 clk | | xtal1 clk |
| | | |
+----------------+ +----------------+
Then, a peripheral clock periph0 is probed. His parent is xtal1. The
clock_register_*() call will run __clk_init_parent() and a link between
periph0's core and xtal1's core will be created and stored in
periph0's core->parent_clk_link entry.
+----------------+ +----------------+
| | | |
| | | |
| xtal0 core | | xtal1 core |
| | | |
| | | |
+-------^^-------+ +-------^^-------+
|| ||
|| ||
+----------------+ +----------------+
| | | |
| xtal0 clk | | xtal1 clk |
| | | |
+----------------+ +-------^--------+
|
|
+--------------+
| ->parent_clk_link
|
+----------------+
| |
| |
| periph0 core |
| |
| |
+-------^^-------+
||
||
+----------------+
| |
| periph0 clk 0 |
| |
+----------------+
Then, device0 is probed and "get" the periph0 clock. clk_get() will be
called and a struct clk will be instantiated for device0 (called in
the figure clk 1). A link between device0 and the new clk 1 instance of
periph0 will be created and stored in the clk->consumer_link entry.
+----------------+ +----------------+
| | | |
| | | |
| xtal0 core | | xtal1 core |
| | | |
| | | |
+-------^^-------+ +-------^^-------+
|| ||
|| ||
+----------------+ +----------------+
| | | |
| xtal0 clk | | xtal1 clk |
| | | |
+----------------+ +-------^--------+
|
|
+--------------+
| ->parent_clk_link
|
+----------------+
| |
| |
| periph0 core |
| <-------------+
| <-------------|
+-------^^-------+ ||
|| ||
|| ||
+----------------+ +----------------+
| | | |
| periph0 clk 0 | | periph0 clk 1 |
| | | |
+----------------+ +----------------+
|
| ->consumer_link
|
|
|
+-------v--------+
| device0 |
+----------------+
Right now, device0 is linked to periph0, itself linked to xtal1 so
everything is fine.
Now let's get some fun: the new parent of periph0 is xtal1. The process
will call clk_reparent(), periph0's core->parent_clk_link will be
destroyed and a new link to xtal1 will be setup and stored. The
situation is now that device0 is linked to periph0 and periph0 is
linked to xtal1, so the dependency between device0 and xtal1 is still
clear.
+----------------+ +----------------+
| | | |
| | | |
| xtal0 core | | xtal1 core |
| | | |
| | | |
+-------^^-------+ +-------^^-------+
|| ||
|| ||
+----------------+ +----------------+
| | | |
| xtal0 clk | | xtal1 clk |
| | | |
+-------^--------+ +----------------+
|
| \ /
+----------------------------x
->parent_clk_link | / \
|
+----------------+
| |
| |
| periph0 core |
| <-------------+
| <-------------|
+-------^^-------+ ||
|| ||
|| ||
+----------------+ +----------------+
| | | |
| periph0 clk 0 | | periph0 clk 1 |
| | | |
+----------------+ +----------------+
|
| ->consumer_link
|
|
|
+-------v--------+
| device0 |
+----------------+
I assume periph0 cannot be removed while there are devices using it,
same for xtal0.
What can happen is that device0 'put' the clock periph0. The relevant
link is deleted and the clk instance dropped.
+----------------+ +----------------+
| | | |
| | | |
| xtal0 core | | xtal1 core |
| | | |
| | | |
+-------^^-------+ +-------^^-------+
|| ||
|| ||
+----------------+ +----------------+
| | | |
| xtal0 clk | | xtal1 clk |
| | | |
+-------^--------+ +----------------+
|
| \ /
+----------------------------x
->parent_clk_link | / \
|
+----------------+
| |
| |
| periph0 core |
| |
| |
+-------^^-------+
||
||
+----------------+
| |
| periph0 clk 0 |
| |
+----------------+
Now we can unregister periph0: link with the parent will be destroyed
and the clock may be safely removed.
+----------------+ +----------------+
| | | |
| | | |
| xtal0 core | | xtal1 core |
| | | |
| | | |
+-------^^-------+ +-------^^-------+
|| ||
|| ||
+----------------+ +----------------+
| | | |
| xtal0 clk | | xtal1 clk |
| | | |
+----------------+ +----------------+
This is my understanding of the common clock framework and how links
can be added to it.
As a result, here are the links created during the boot of an
ESPRESSObin:
----->8-----
marvell-armada-3700-tbg-clock d0013200.tbg: Linked as a consumer to d0013800.pinctrl:xtal-clk
marvell-armada-3700-tbg-clock d0013200.tbg: Dropping the link to d0013800.pinctrl:xtal-clk
marvell-armada-3700-tbg-clock d0013200.tbg: Linked as a consumer to d0013800.pinctrl:xtal-clk
marvell-armada-3700-periph-clock d0013000.nb-periph-clk: Linked as a consumer to d0013200.tbg
marvell-armada-3700-periph-clock d0013000.nb-periph-clk: Linked as a consumer to d0013800.pinctrl:xtal-clk
marvell-armada-3700-periph-clock d0018000.sb-periph-clk: Linked as a consumer to d0013200.tbg
mvneta d0030000.ethernet: Linked as a consumer to d0018000.sb-periph-clk
xhci-hcd d0058000.usb: Linked as a consumer to d0018000.sb-periph-clk
xenon-sdhci d00d0000.sdhci: Linked as a consumer to d0013000.nb-periph-clk
xenon-sdhci d00d0000.sdhci: Dropping the link to d0013000.nb-periph-clk
mvebu-uart d0012000.serial: Linked as a consumer to d0013800.pinctrl:xtal-clk
advk-pcie d0070000.pcie: Linked as a consumer to d0018000.sb-periph-clk
xenon-sdhci d00d0000.sdhci: Linked as a consumer to d0013000.nb-periph-clk
xenon-sdhci d00d0000.sdhci: Linked as a consumer to regulator.1
cpu cpu0: Linked as a consumer to d0013000.nb-periph-clk
cpu cpu0: Dropping the link to d0013000.nb-periph-clk
cpu cpu0: Linked as a consumer to d0013000.nb-periph-clk
-----8<-----
Thanks,
Miquèl
Changes since v3:
=================
* Rebased on top of Stephen's 'clk-parent-rewrite' branch. Stephen
already updated and took the patch 'clk: core: clarify the check for
runtime PM' so it is not present in this series anymore.
* Updated the code to fit with the new core. Kept the helpers that
were added to clk/clk.c (turning them static) for more readability.
* While working on clocks, I discovered a typo in an a3700-tbg driver
error message. A patch has been added to correct this typo.
Changes since v2:
=================
* Fixed compilation issue when not using the common clock framework:
removed the static keyword in front of clk_link/unlink_consumer()
dummy definitions in clkdev.c.
Changes since v1:
=================
* Add clock->clock links, not only device->clock links.
* Helpers renamed:
> clk_{link,unlink}_hierarchy()
> clk_{link,unlink}_consumer()
* Add two patches to pass a "struct device" to the clock registration
helper. This way device links may work between clocks themselves
(otherwise the link is not created).
Miquel Raynal (4):
clk: core: link consumer with clock driver
clk: mvebu: armada-37xx-tbg: fix error message
clk: mvebu: armada-37xx-tbg: fill the device entry when registering
the clocks
clk: mvebu: armada-37xx-xtal: fill the device entry when registering
the clock
drivers/clk/clk.c | 50 +++++++++++++++++++++++++++-
drivers/clk/mvebu/armada-37xx-tbg.c | 8 +++--
drivers/clk/mvebu/armada-37xx-xtal.c | 3 +-
3 files changed, 56 insertions(+), 5 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/4] clk: core: link consumer with clock driver
2019-01-08 16:19 [PATCH v4 0/4] Add device links to clocks Miquel Raynal
@ 2019-01-08 16:19 ` Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 2/4] clk: mvebu: armada-37xx-tbg: fix error message Miquel Raynal
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Miquel Raynal @ 2019-01-08 16:19 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: linux-clk, linux-kernel, linux-arm-kernel, Thomas Petazzoni,
Antoine Tenart, Maxime Chevallier, Gregory Clement, Nadav Haklai,
Miquel Raynal
One major concern when, for instance, suspending/resuming a platform
is to never access registers before the underlying clock has been
resumed, otherwise most of the time the kernel will just crash. One
solution is to use syscore operations when registering clock drivers
suspend/resume callbacks. One problem of using syscore_ops is that the
suspend/resume scheduling will depend on the order of the
registrations, which brings (unacceptable) randomness in the process.
A feature called device links has been introduced to handle such
situation. It creates dependencies between consumers and providers,
enforcing e.g. the suspend/resume order when needed. Such feature is
already in use for regulators.
Add device links support in the clock subsystem by creating/deleting
the links at get/put time.
Example of a boot (ESPRESSObin, A3700 SoC) with devices linked to clocks:
marvell-armada-3700-tbg-clock d0013200.tbg: Linked as a consumer to d0013800.pinctrl:xtal-clk
marvell-armada-3700-tbg-clock d0013200.tbg: Dropping the link to d0013800.pinctrl:xtal-clk
marvell-armada-3700-tbg-clock d0013200.tbg: Linked as a consumer to d0013800.pinctrl:xtal-clk
marvell-armada-3700-periph-clock d0013000.nb-periph-clk: Linked as a consumer to d0013200.tbg
marvell-armada-3700-periph-clock d0013000.nb-periph-clk: Linked as a consumer to d0013800.pinctrl:xtal-clk
marvell-armada-3700-periph-clock d0018000.sb-periph-clk: Linked as a consumer to d0013200.tbg
mvneta d0030000.ethernet: Linked as a consumer to d0018000.sb-periph-clk
xhci-hcd d0058000.usb: Linked as a consumer to d0018000.sb-periph-clk
xenon-sdhci d00d0000.sdhci: Linked as a consumer to d0013000.nb-periph-clk
xenon-sdhci d00d0000.sdhci: Dropping the link to d0013000.nb-periph-clk
mvebu-uart d0012000.serial: Linked as a consumer to d0013800.pinctrl:xtal-clk
advk-pcie d0070000.pcie: Linked as a consumer to d0018000.sb-periph-clk
xenon-sdhci d00d0000.sdhci: Linked as a consumer to d0013000.nb-periph-clk
xenon-sdhci d00d0000.sdhci: Linked as a consumer to regulator.1
cpu cpu0: Linked as a consumer to d0013000.nb-periph-clk
cpu cpu0: Dropping the link to d0013000.nb-periph-clk
cpu cpu0: Linked as a consumer to d0013000.nb-periph-clk
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/clk/clk.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e1449db544c7..5c36ff52cb72 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -78,6 +78,7 @@ struct clk_core {
struct hlist_node debug_node;
#endif
struct kref ref;
+ struct device_link *parent_clk_link;
};
#define CREATE_TRACE_POINTS
@@ -92,6 +93,7 @@ struct clk {
unsigned long max_rate;
unsigned int exclusive_count;
struct hlist_node clks_node;
+ struct device_link *consumer_link;
};
/*** runtime pm ***/
@@ -276,6 +278,35 @@ struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw)
}
EXPORT_SYMBOL_GPL(clk_hw_get_parent);
+static void clk_link_consumer(struct device *consumer, struct clk *clk)
+{
+ if (consumer && clk)
+ clk->consumer_link = device_link_add(consumer, clk->core->dev,
+ DL_FLAG_STATELESS);
+}
+
+static void clk_unlink_consumer(struct clk *clk)
+{
+ if (clk && clk->consumer_link)
+ device_link_del(clk->consumer_link);
+}
+
+static void clk_link_hierarchy(struct clk_core *consumer,
+ struct clk_core *provider)
+{
+ if (consumer && provider)
+ consumer->parent_clk_link = device_link_add(consumer->dev,
+ provider->dev,
+ DL_FLAG_STATELESS);
+}
+
+static void clk_unlink_hierarchy(struct clk_core *consumer,
+ struct clk_core *provider)
+{
+ if (consumer && provider && consumer->parent_clk_link)
+ device_link_del(consumer->parent_clk_link);
+}
+
static struct clk_core *__clk_lookup_subtree(const char *name,
struct clk_core *core)
{
@@ -1599,6 +1630,9 @@ static void clk_reparent(struct clk_core *core, struct clk_core *new_parent)
hlist_del(&core->child_node);
+ if (core->parent)
+ clk_unlink_hierarchy(core, core->parent);
+
if (new_parent) {
bool becomes_orphan = new_parent->orphan;
@@ -1610,6 +1644,8 @@ static void clk_reparent(struct clk_core *core, struct clk_core *new_parent)
if (was_orphan != becomes_orphan)
clk_core_update_orphan_status(core, becomes_orphan);
+
+ clk_link_hierarchy(core, new_parent);
} else {
hlist_add_head(&core->child_node, &clk_orphan_list);
if (!was_orphan)
@@ -2336,6 +2372,8 @@ __clk_init_parent(struct clk_core *core, bool update_orphan)
if (!parent_hw)
return NULL;
+ clk_link_hierarchy(core, parent_hw->core);
+
return parent_hw->core;
}
@@ -3411,6 +3449,7 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
}
kref_get(&core->ref);
+ clk_link_consumer(dev, clk);
clk_core_link_consumer(core, clk);
return clk;
@@ -3633,11 +3672,18 @@ void clk_unregister(struct clk *clk)
clk->core->ops = &clk_nodrv_ops;
clk_enable_unlock(flags);
+ clk_unlink_hierarchy(clk->core, clk->core->parent);
+
if (!hlist_empty(&clk->core->children)) {
struct clk_core *child;
struct hlist_node *t;
- /* Reparent all children to the orphan list. */
+ /*
+ * Reparent all children to the orphan list.
+ *
+ * No need to unlink the child clock manually, this will be
+ * handled by clk_reparent().
+ */
hlist_for_each_entry_safe(child, t, &clk->core->children,
child_node)
clk_core_set_parent_nolock(child, NULL);
@@ -3799,6 +3845,8 @@ void __clk_put(struct clk *clk)
clk_prepare_lock();
+ clk_unlink_consumer(clk);
+
/*
* Before calling clk_put, all calls to clk_rate_exclusive_get() from a
* given user should be balanced with calls to clk_rate_exclusive_put()
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 2/4] clk: mvebu: armada-37xx-tbg: fix error message
2019-01-08 16:19 [PATCH v4 0/4] Add device links to clocks Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 1/4] clk: core: link consumer with clock driver Miquel Raynal
@ 2019-01-08 16:19 ` Miquel Raynal
2019-01-18 14:16 ` Gregory CLEMENT
2019-01-08 16:19 ` [PATCH v4 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks Miquel Raynal
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2019-01-08 16:19 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: linux-clk, linux-kernel, linux-arm-kernel, Thomas Petazzoni,
Antoine Tenart, Maxime Chevallier, Gregory Clement, Nadav Haklai,
Miquel Raynal
The error message should state that the driver failed to get the
parent clock, not the opposite.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/clk/mvebu/armada-37xx-tbg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
index ee272d4d8c24..77b978c55ef6 100644
--- a/drivers/clk/mvebu/armada-37xx-tbg.c
+++ b/drivers/clk/mvebu/armada-37xx-tbg.c
@@ -98,7 +98,7 @@ static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
parent = clk_get(dev, NULL);
if (IS_ERR(parent)) {
- dev_err(dev, "Could get the clock parent\n");
+ dev_err(dev, "Could not get the clock parent\n");
return -EINVAL;
}
parent_name = __clk_get_name(parent);
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks
2019-01-08 16:19 [PATCH v4 0/4] Add device links to clocks Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 1/4] clk: core: link consumer with clock driver Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 2/4] clk: mvebu: armada-37xx-tbg: fix error message Miquel Raynal
@ 2019-01-08 16:19 ` Miquel Raynal
2019-01-18 14:16 ` Gregory CLEMENT
2019-01-08 16:19 ` [PATCH v4 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock Miquel Raynal
2019-04-11 23:34 ` [PATCH v4 0/4] Add device links to clocks Stephen Boyd
4 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2019-01-08 16:19 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: linux-clk, linux-kernel, linux-arm-kernel, Thomas Petazzoni,
Antoine Tenart, Maxime Chevallier, Gregory Clement, Nadav Haklai,
Miquel Raynal
So far the clk_hw_register_fixed_factor() calls are not providing any
device structure. While doing so is harmless for regular use, the
missing device structure may be a problem for suspend to RAM support.
Since, device links have been added to clocks, links created during
probe will enforce the suspend/resume orders. When the device is
missing during the registration, no link can be established, hence the
order between parent and child clocks are not enforced.
Adding the device structure here will create a link between the 4 TBG
clocks (registered by this driver) and:
* their parent clock: XTAL,
* their child clocks: several 'periph' clock.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/clk/mvebu/armada-37xx-tbg.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
index 77b978c55ef6..da3a08e419d6 100644
--- a/drivers/clk/mvebu/armada-37xx-tbg.c
+++ b/drivers/clk/mvebu/armada-37xx-tbg.c
@@ -116,8 +116,10 @@ static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
name = tbg[i].name;
mult = tbg_get_mult(reg, &tbg[i]);
div = tbg_get_div(reg, &tbg[i]);
- hw_tbg_data->hws[i] = clk_hw_register_fixed_factor(NULL, name,
- parent_name, 0, mult, div);
+ hw_tbg_data->hws[i] = clk_hw_register_fixed_factor(dev, name,
+ parent_name,
+ 0, mult,
+ div);
if (IS_ERR(hw_tbg_data->hws[i]))
dev_err(dev, "Can't register TBG clock %s\n", name);
}
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock
2019-01-08 16:19 [PATCH v4 0/4] Add device links to clocks Miquel Raynal
` (2 preceding siblings ...)
2019-01-08 16:19 ` [PATCH v4 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks Miquel Raynal
@ 2019-01-08 16:19 ` Miquel Raynal
2019-01-18 14:16 ` Gregory CLEMENT
2019-04-11 23:34 ` [PATCH v4 0/4] Add device links to clocks Stephen Boyd
4 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2019-01-08 16:19 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: linux-clk, linux-kernel, linux-arm-kernel, Thomas Petazzoni,
Antoine Tenart, Maxime Chevallier, Gregory Clement, Nadav Haklai,
Miquel Raynal
So far the clk_hw_register_fixed_factor() call was not providing any
device structure. While doing so is harmless for regular use, the
missing device structure may be a problem for suspend to RAM support.
Since, device links have been added to clocks, links created during
probe will enforce the suspend/resume orders. When the device is
missing during the registration, no link can be established, hence the
order between parent and child clocks are not enforced.
Adding the device structure here will create a link between the XTAL
clock (this one) and the four TBG clocks that are derived from it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/clk/mvebu/armada-37xx-xtal.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/mvebu/armada-37xx-xtal.c b/drivers/clk/mvebu/armada-37xx-xtal.c
index e9e306d4e9af..0e74bcd83d1a 100644
--- a/drivers/clk/mvebu/armada-37xx-xtal.c
+++ b/drivers/clk/mvebu/armada-37xx-xtal.c
@@ -57,7 +57,8 @@ static int armada_3700_xtal_clock_probe(struct platform_device *pdev)
rate = 25000000;
of_property_read_string_index(np, "clock-output-names", 0, &xtal_name);
- xtal_hw = clk_hw_register_fixed_rate(NULL, xtal_name, NULL, 0, rate);
+ xtal_hw = clk_hw_register_fixed_rate(&pdev->dev, xtal_name, NULL, 0,
+ rate);
if (IS_ERR(xtal_hw))
return PTR_ERR(xtal_hw);
ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, xtal_hw);
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/4] clk: mvebu: armada-37xx-tbg: fix error message
2019-01-08 16:19 ` [PATCH v4 2/4] clk: mvebu: armada-37xx-tbg: fix error message Miquel Raynal
@ 2019-01-18 14:16 ` Gregory CLEMENT
0 siblings, 0 replies; 13+ messages in thread
From: Gregory CLEMENT @ 2019-01-18 14:16 UTC (permalink / raw)
To: Miquel Raynal
Cc: Michael Turquette, Stephen Boyd, Russell King, linux-clk,
linux-kernel, linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Nadav Haklai
Hi Miquel,
On mar., janv. 08 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> The error message should state that the driver failed to get the
> parent clock, not the opposite.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Thanks,
Gregory
> ---
> drivers/clk/mvebu/armada-37xx-tbg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
> index ee272d4d8c24..77b978c55ef6 100644
> --- a/drivers/clk/mvebu/armada-37xx-tbg.c
> +++ b/drivers/clk/mvebu/armada-37xx-tbg.c
> @@ -98,7 +98,7 @@ static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
>
> parent = clk_get(dev, NULL);
> if (IS_ERR(parent)) {
> - dev_err(dev, "Could get the clock parent\n");
> + dev_err(dev, "Could not get the clock parent\n");
> return -EINVAL;
> }
> parent_name = __clk_get_name(parent);
> --
> 2.19.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks
2019-01-08 16:19 ` [PATCH v4 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks Miquel Raynal
@ 2019-01-18 14:16 ` Gregory CLEMENT
0 siblings, 0 replies; 13+ messages in thread
From: Gregory CLEMENT @ 2019-01-18 14:16 UTC (permalink / raw)
To: Miquel Raynal
Cc: Michael Turquette, Stephen Boyd, Russell King, linux-clk,
linux-kernel, linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Nadav Haklai
Hi Miquel,
On mar., janv. 08 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> So far the clk_hw_register_fixed_factor() calls are not providing any
> device structure. While doing so is harmless for regular use, the
> missing device structure may be a problem for suspend to RAM support.
>
> Since, device links have been added to clocks, links created during
> probe will enforce the suspend/resume orders. When the device is
> missing during the registration, no link can be established, hence the
> order between parent and child clocks are not enforced.
>
> Adding the device structure here will create a link between the 4 TBG
> clocks (registered by this driver) and:
> * their parent clock: XTAL,
> * their child clocks: several 'periph' clock.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Thanks,
Gregory
> ---
> drivers/clk/mvebu/armada-37xx-tbg.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
> index 77b978c55ef6..da3a08e419d6 100644
> --- a/drivers/clk/mvebu/armada-37xx-tbg.c
> +++ b/drivers/clk/mvebu/armada-37xx-tbg.c
> @@ -116,8 +116,10 @@ static int armada_3700_tbg_clock_probe(struct platform_device *pdev)
> name = tbg[i].name;
> mult = tbg_get_mult(reg, &tbg[i]);
> div = tbg_get_div(reg, &tbg[i]);
> - hw_tbg_data->hws[i] = clk_hw_register_fixed_factor(NULL, name,
> - parent_name, 0, mult, div);
> + hw_tbg_data->hws[i] = clk_hw_register_fixed_factor(dev, name,
> + parent_name,
> + 0, mult,
> + div);
> if (IS_ERR(hw_tbg_data->hws[i]))
> dev_err(dev, "Can't register TBG clock %s\n", name);
> }
> --
> 2.19.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock
2019-01-08 16:19 ` [PATCH v4 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock Miquel Raynal
@ 2019-01-18 14:16 ` Gregory CLEMENT
0 siblings, 0 replies; 13+ messages in thread
From: Gregory CLEMENT @ 2019-01-18 14:16 UTC (permalink / raw)
To: Miquel Raynal
Cc: Michael Turquette, Stephen Boyd, Russell King, linux-clk,
linux-kernel, linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Nadav Haklai
Hi Miquel,
On mar., janv. 08 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> So far the clk_hw_register_fixed_factor() call was not providing any
> device structure. While doing so is harmless for regular use, the
> missing device structure may be a problem for suspend to RAM support.
>
> Since, device links have been added to clocks, links created during
> probe will enforce the suspend/resume orders. When the device is
> missing during the registration, no link can be established, hence the
> order between parent and child clocks are not enforced.
>
> Adding the device structure here will create a link between the XTAL
> clock (this one) and the four TBG clocks that are derived from it.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Thanks,
Gregory
> ---
> drivers/clk/mvebu/armada-37xx-xtal.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/mvebu/armada-37xx-xtal.c b/drivers/clk/mvebu/armada-37xx-xtal.c
> index e9e306d4e9af..0e74bcd83d1a 100644
> --- a/drivers/clk/mvebu/armada-37xx-xtal.c
> +++ b/drivers/clk/mvebu/armada-37xx-xtal.c
> @@ -57,7 +57,8 @@ static int armada_3700_xtal_clock_probe(struct platform_device *pdev)
> rate = 25000000;
>
> of_property_read_string_index(np, "clock-output-names", 0, &xtal_name);
> - xtal_hw = clk_hw_register_fixed_rate(NULL, xtal_name, NULL, 0, rate);
> + xtal_hw = clk_hw_register_fixed_rate(&pdev->dev, xtal_name, NULL, 0,
> + rate);
> if (IS_ERR(xtal_hw))
> return PTR_ERR(xtal_hw);
> ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, xtal_hw);
> --
> 2.19.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/4] Add device links to clocks
2019-01-08 16:19 [PATCH v4 0/4] Add device links to clocks Miquel Raynal
` (3 preceding siblings ...)
2019-01-08 16:19 ` [PATCH v4 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock Miquel Raynal
@ 2019-04-11 23:34 ` Stephen Boyd
2019-05-21 9:46 ` Miquel Raynal
4 siblings, 1 reply; 13+ messages in thread
From: Stephen Boyd @ 2019-04-11 23:34 UTC (permalink / raw)
To: Michael Turquette, Miquel Raynal, Russell King
Cc: linux-clk, linux-kernel, linux-arm-kernel, Thomas Petazzoni,
Antoine Tenart, Maxime Chevallier, Gregory Clement, Nadav Haklai,
Miquel Raynal
Quoting Miquel Raynal (2019-01-08 08:19:36)
> Hello,
>
> While working on suspend to RAM feature, I ran into troubles multiple
> times when clocks where not suspending/resuming at the desired time. I
> had a look at the core and I think the same logic as in the
> regulator's core may be applied here to (very easily) fix this issue:
> using device links.
>
> The only additional change I had to do was to always (when available)
> populate the device entry of the core clock structure so that it could
> be used later. This is the purpose of patch 1. Patch 2 actually adds
> support for device links.
>
> Here is a step-by-step explanation of how links are managed, following
> Maxime Ripard's suggestion.
>
>
> The order of probe has no importance because the framework already
> handles orphaned clocks so let's be simple and say there are two root
> clocks, not depending on anything, that are probed first: xtal0 and
> xtal1. None of these clocks have a parent, there is no device link in
> the game, yet.
>
> +----------------+ +----------------+
> | | | |
> | | | |
> | xtal0 core | | xtal1 core |
> | | | |
> | | | |
> +-------^^-------+ +-------^^-------+
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | xtal0 clk | | xtal1 clk |
> | | | |
> +----------------+ +----------------+
>
> Then, a peripheral clock periph0 is probed. His parent is xtal1. The
> clock_register_*() call will run __clk_init_parent() and a link between
> periph0's core and xtal1's core will be created and stored in
> periph0's core->parent_clk_link entry.
>
> +----------------+ +----------------+
> | | | |
> | | | |
> | xtal0 core | | xtal1 core |
> | | | |
> | | | |
> +-------^^-------+ +-------^^-------+
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | xtal0 clk | | xtal1 clk |
> | | | |
> +----------------+ +-------^--------+
> |
> |
> +--------------+
> | ->parent_clk_link
> |
> +----------------+
> | |
> | |
> | periph0 core |
> | |
> | |
> +-------^^-------+
> ||
> ||
> +----------------+
> | |
> | periph0 clk 0 |
> | |
> +----------------+
>
> Then, device0 is probed and "get" the periph0 clock. clk_get() will be
> called and a struct clk will be instantiated for device0 (called in
> the figure clk 1). A link between device0 and the new clk 1 instance of
> periph0 will be created and stored in the clk->consumer_link entry.
>
> +----------------+ +----------------+
> | | | |
> | | | |
> | xtal0 core | | xtal1 core |
> | | | |
> | | | |
> +-------^^-------+ +-------^^-------+
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | xtal0 clk | | xtal1 clk |
> | | | |
> +----------------+ +-------^--------+
> |
> |
> +--------------+
> | ->parent_clk_link
> |
> +----------------+
> | |
> | |
> | periph0 core |
> | <-------------+
> | <-------------|
> +-------^^-------+ ||
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | periph0 clk 0 | | periph0 clk 1 |
> | | | |
> +----------------+ +----------------+
> |
> | ->consumer_link
> |
> |
> |
> +-------v--------+
> | device0 |
> +----------------+
>
> Right now, device0 is linked to periph0, itself linked to xtal1 so
> everything is fine.
>
> Now let's get some fun: the new parent of periph0 is xtal1. The process
> will call clk_reparent(), periph0's core->parent_clk_link will be
> destroyed and a new link to xtal1 will be setup and stored. The
> situation is now that device0 is linked to periph0 and periph0 is
> linked to xtal1, so the dependency between device0 and xtal1 is still
> clear.
>
> +----------------+ +----------------+
> | | | |
> | | | |
> | xtal0 core | | xtal1 core |
> | | | |
> | | | |
> +-------^^-------+ +-------^^-------+
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | xtal0 clk | | xtal1 clk |
> | | | |
> +-------^--------+ +----------------+
> |
> | \ /
> +----------------------------x
> ->parent_clk_link | / \
> |
> +----------------+
> | |
> | |
> | periph0 core |
> | <-------------+
> | <-------------|
> +-------^^-------+ ||
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | periph0 clk 0 | | periph0 clk 1 |
> | | | |
> +----------------+ +----------------+
> |
> | ->consumer_link
> |
> |
> |
> +-------v--------+
> | device0 |
> +----------------+
>
> I assume periph0 cannot be removed while there are devices using it,
> same for xtal0.
>
> What can happen is that device0 'put' the clock periph0. The relevant
> link is deleted and the clk instance dropped.
>
> +----------------+ +----------------+
> | | | |
> | | | |
> | xtal0 core | | xtal1 core |
> | | | |
> | | | |
> +-------^^-------+ +-------^^-------+
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | xtal0 clk | | xtal1 clk |
> | | | |
> +-------^--------+ +----------------+
> |
> | \ /
> +----------------------------x
> ->parent_clk_link | / \
> |
> +----------------+
> | |
> | |
> | periph0 core |
> | |
> | |
> +-------^^-------+
> ||
> ||
> +----------------+
> | |
> | periph0 clk 0 |
> | |
> +----------------+
>
> Now we can unregister periph0: link with the parent will be destroyed
> and the clock may be safely removed.
>
> +----------------+ +----------------+
> | | | |
> | | | |
> | xtal0 core | | xtal1 core |
> | | | |
> | | | |
> +-------^^-------+ +-------^^-------+
> || ||
> || ||
> +----------------+ +----------------+
> | | | |
> | xtal0 clk | | xtal1 clk |
> | | | |
> +----------------+ +----------------+
>
>
> This is my understanding of the common clock framework and how links
> can be added to it.
>
> As a result, here are the links created during the boot of an
> ESPRESSObin:
>
Sorry this patch series is taking way too long to get merged. It's
already mid-April!
So I still have some of the original questions I had from before, mostly
around circular parent chains between clk providers. For example, there
are clk providers that both provide clks to other providers and consume
clks from those providers. Does device links work gracefully here?
Just speaking from my own qcom experience, I can point to the PCIe PHY
that's a provider of a clk to GCC and a consumer of a clk in GCC. In
block diagram form this is:
PCIE PHY GCC
+--------------+ +-------------------------+
| | | |
| PHY clk ->----------+---- gcc_pipe_clk ---+ |
| | | | |
| | | | |
| pci_pipe_clk <----------|---------------------+ |
| | | |
+--------------+ +-------------------------+
The end result is that the PCIe PHY is a clk controller that provides
the PHY clk to GCC's gcc_pipe_clk and then it gets the same clk signal
back from GCC and uses it on the PCIe PHY's pci_pipe_clk input.
So is this is a problem?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/4] Add device links to clocks
2019-04-11 23:34 ` [PATCH v4 0/4] Add device links to clocks Stephen Boyd
@ 2019-05-21 9:46 ` Miquel Raynal
2019-06-17 9:57 ` Miquel Raynal
0 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2019-05-21 9:46 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Russell King, linux-clk, linux-kernel,
linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Gregory Clement, Nadav Haklai
Hi Stephen,
Stephen Boyd <sboyd@kernel.org> wrote on Thu, 11 Apr 2019 16:34:16
-0700:
> Quoting Miquel Raynal (2019-01-08 08:19:36)
> > Hello,
> >
> > While working on suspend to RAM feature, I ran into troubles multiple
> > times when clocks where not suspending/resuming at the desired time. I
> > had a look at the core and I think the same logic as in the
> > regulator's core may be applied here to (very easily) fix this issue:
> > using device links.
> >
> > The only additional change I had to do was to always (when available)
> > populate the device entry of the core clock structure so that it could
> > be used later. This is the purpose of patch 1. Patch 2 actually adds
> > support for device links.
> >
> > Here is a step-by-step explanation of how links are managed, following
> > Maxime Ripard's suggestion.
> >
> >
> > The order of probe has no importance because the framework already
> > handles orphaned clocks so let's be simple and say there are two root
> > clocks, not depending on anything, that are probed first: xtal0 and
> > xtal1. None of these clocks have a parent, there is no device link in
> > the game, yet.
> >
> > +----------------+ +----------------+
> > | | | |
> > | | | |
> > | xtal0 core | | xtal1 core |
> > | | | |
> > | | | |
> > +-------^^-------+ +-------^^-------+
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | xtal0 clk | | xtal1 clk |
> > | | | |
> > +----------------+ +----------------+
> >
> > Then, a peripheral clock periph0 is probed. His parent is xtal1. The
> > clock_register_*() call will run __clk_init_parent() and a link between
> > periph0's core and xtal1's core will be created and stored in
> > periph0's core->parent_clk_link entry.
> >
> > +----------------+ +----------------+
> > | | | |
> > | | | |
> > | xtal0 core | | xtal1 core |
> > | | | |
> > | | | |
> > +-------^^-------+ +-------^^-------+
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | xtal0 clk | | xtal1 clk |
> > | | | |
> > +----------------+ +-------^--------+
> > |
> > |
> > +--------------+
> > | ->parent_clk_link
> > |
> > +----------------+
> > | |
> > | |
> > | periph0 core |
> > | |
> > | |
> > +-------^^-------+
> > ||
> > ||
> > +----------------+
> > | |
> > | periph0 clk 0 |
> > | |
> > +----------------+
> >
> > Then, device0 is probed and "get" the periph0 clock. clk_get() will be
> > called and a struct clk will be instantiated for device0 (called in
> > the figure clk 1). A link between device0 and the new clk 1 instance of
> > periph0 will be created and stored in the clk->consumer_link entry.
> >
> > +----------------+ +----------------+
> > | | | |
> > | | | |
> > | xtal0 core | | xtal1 core |
> > | | | |
> > | | | |
> > +-------^^-------+ +-------^^-------+
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | xtal0 clk | | xtal1 clk |
> > | | | |
> > +----------------+ +-------^--------+
> > |
> > |
> > +--------------+
> > | ->parent_clk_link
> > |
> > +----------------+
> > | |
> > | |
> > | periph0 core |
> > | <-------------+
> > | <-------------|
> > +-------^^-------+ ||
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | periph0 clk 0 | | periph0 clk 1 |
> > | | | |
> > +----------------+ +----------------+
> > |
> > | ->consumer_link
> > |
> > |
> > |
> > +-------v--------+
> > | device0 |
> > +----------------+
> >
> > Right now, device0 is linked to periph0, itself linked to xtal1 so
> > everything is fine.
> >
> > Now let's get some fun: the new parent of periph0 is xtal1. The process
> > will call clk_reparent(), periph0's core->parent_clk_link will be
> > destroyed and a new link to xtal1 will be setup and stored. The
> > situation is now that device0 is linked to periph0 and periph0 is
> > linked to xtal1, so the dependency between device0 and xtal1 is still
> > clear.
> >
> > +----------------+ +----------------+
> > | | | |
> > | | | |
> > | xtal0 core | | xtal1 core |
> > | | | |
> > | | | |
> > +-------^^-------+ +-------^^-------+
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | xtal0 clk | | xtal1 clk |
> > | | | |
> > +-------^--------+ +----------------+
> > |
> > | \ /
> > +----------------------------x
> > ->parent_clk_link | / \
> > |
> > +----------------+
> > | |
> > | |
> > | periph0 core |
> > | <-------------+
> > | <-------------|
> > +-------^^-------+ ||
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | periph0 clk 0 | | periph0 clk 1 |
> > | | | |
> > +----------------+ +----------------+
> > |
> > | ->consumer_link
> > |
> > |
> > |
> > +-------v--------+
> > | device0 |
> > +----------------+
> >
> > I assume periph0 cannot be removed while there are devices using it,
> > same for xtal0.
> >
> > What can happen is that device0 'put' the clock periph0. The relevant
> > link is deleted and the clk instance dropped.
> >
> > +----------------+ +----------------+
> > | | | |
> > | | | |
> > | xtal0 core | | xtal1 core |
> > | | | |
> > | | | |
> > +-------^^-------+ +-------^^-------+
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | xtal0 clk | | xtal1 clk |
> > | | | |
> > +-------^--------+ +----------------+
> > |
> > | \ /
> > +----------------------------x
> > ->parent_clk_link | / \
> > |
> > +----------------+
> > | |
> > | |
> > | periph0 core |
> > | |
> > | |
> > +-------^^-------+
> > ||
> > ||
> > +----------------+
> > | |
> > | periph0 clk 0 |
> > | |
> > +----------------+
> >
> > Now we can unregister periph0: link with the parent will be destroyed
> > and the clock may be safely removed.
> >
> > +----------------+ +----------------+
> > | | | |
> > | | | |
> > | xtal0 core | | xtal1 core |
> > | | | |
> > | | | |
> > +-------^^-------+ +-------^^-------+
> > || ||
> > || ||
> > +----------------+ +----------------+
> > | | | |
> > | xtal0 clk | | xtal1 clk |
> > | | | |
> > +----------------+ +----------------+
> >
> >
> > This is my understanding of the common clock framework and how links
> > can be added to it.
> >
> > As a result, here are the links created during the boot of an
> > ESPRESSObin:
> >
>
> Sorry this patch series is taking way too long to get merged. It's
> already mid-April!
>
> So I still have some of the original questions I had from before, mostly
> around circular parent chains between clk providers. For example, there
> are clk providers that both provide clks to other providers and consume
> clks from those providers. Does device links work gracefully here?
>
> Just speaking from my own qcom experience, I can point to the PCIe PHY
> that's a provider of a clk to GCC and a consumer of a clk in GCC. In
> block diagram form this is:
>
>
> PCIE PHY GCC
> +--------------+ +-------------------------+
> | | | |
> | PHY clk ->----------+---- gcc_pipe_clk ---+ |
> | | | | |
> | | | | |
> | pci_pipe_clk <----------|---------------------+ |
> | | | |
> +--------------+ +-------------------------+
>
> The end result is that the PCIe PHY is a clk controller that provides
> the PHY clk to GCC's gcc_pipe_clk and then it gets the same clk signal
> back from GCC and uses it on the PCIe PHY's pci_pipe_clk input.
>
> So is this is a problem?
>
It's now my turn to get back on this topic.
I just put my noise back into this and for what I understand of the
clk subsystem, I think the situation you describe could be pictured
like this:
+---------------+
| |
| |
| PCIe PHY |
| |
| |
+-----^^--------+
||
||
+---------------+
| |
| pcie_pipe_clk |
| |
+------^--------+
|
| ->parent_clk_link
|
|
+---------------+
| |
| |
| GCC |
| |
| |
+------^^-------+
||
||
+---------------+
| |
| gcc_pipe_clk |
| |
+------^--------+
|
| ->parent_clk_link
|
|
+---------------+
| |
| |
| PCIe PHY |
| |
| |
+------^^-------+
||
||
+---------------+
| |
| phy_clk |
| |
+---------------+
IMHO the fact that the first and third blocks are the same does not
interfere with device links.
Honestly, I cannot be 100% sure it won't break on qcom designs, maybe
the best would be to have someone to test. I don't have the relevant
hardware. Do you? It would be really helpful!
There is an entire PCIe series blocked, waiting for these device links
to be merged so it would help a lot if someone could test.
Thank you very much,
Miquèl
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/4] Add device links to clocks
2019-05-21 9:46 ` Miquel Raynal
@ 2019-06-17 9:57 ` Miquel Raynal
2019-07-27 8:53 ` Miquel Raynal
0 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2019-06-17 9:57 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Russell King, linux-clk, linux-kernel,
linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Gregory Clement, Nadav Haklai
Hi Stephen,
Miquel Raynal <miquel.raynal@bootlin.com> wrote on Tue, 21 May 2019
11:46:44 +0200:
> Hi Stephen,
>
> Stephen Boyd <sboyd@kernel.org> wrote on Thu, 11 Apr 2019 16:34:16
> -0700:
>
> > Quoting Miquel Raynal (2019-01-08 08:19:36)
> > > Hello,
> > >
> > > While working on suspend to RAM feature, I ran into troubles multiple
> > > times when clocks where not suspending/resuming at the desired time. I
> > > had a look at the core and I think the same logic as in the
> > > regulator's core may be applied here to (very easily) fix this issue:
> > > using device links.
> > >
> > > The only additional change I had to do was to always (when available)
> > > populate the device entry of the core clock structure so that it could
> > > be used later. This is the purpose of patch 1. Patch 2 actually adds
> > > support for device links.
> > >
> > > Here is a step-by-step explanation of how links are managed, following
> > > Maxime Ripard's suggestion.
> > >
> > >
> > > The order of probe has no importance because the framework already
> > > handles orphaned clocks so let's be simple and say there are two root
> > > clocks, not depending on anything, that are probed first: xtal0 and
> > > xtal1. None of these clocks have a parent, there is no device link in
> > > the game, yet.
> > >
> > > +----------------+ +----------------+
> > > | | | |
> > > | | | |
> > > | xtal0 core | | xtal1 core |
> > > | | | |
> > > | | | |
> > > +-------^^-------+ +-------^^-------+
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | xtal0 clk | | xtal1 clk |
> > > | | | |
> > > +----------------+ +----------------+
> > >
> > > Then, a peripheral clock periph0 is probed. His parent is xtal1. The
> > > clock_register_*() call will run __clk_init_parent() and a link between
> > > periph0's core and xtal1's core will be created and stored in
> > > periph0's core->parent_clk_link entry.
> > >
> > > +----------------+ +----------------+
> > > | | | |
> > > | | | |
> > > | xtal0 core | | xtal1 core |
> > > | | | |
> > > | | | |
> > > +-------^^-------+ +-------^^-------+
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | xtal0 clk | | xtal1 clk |
> > > | | | |
> > > +----------------+ +-------^--------+
> > > |
> > > |
> > > +--------------+
> > > | ->parent_clk_link
> > > |
> > > +----------------+
> > > | |
> > > | |
> > > | periph0 core |
> > > | |
> > > | |
> > > +-------^^-------+
> > > ||
> > > ||
> > > +----------------+
> > > | |
> > > | periph0 clk 0 |
> > > | |
> > > +----------------+
> > >
> > > Then, device0 is probed and "get" the periph0 clock. clk_get() will be
> > > called and a struct clk will be instantiated for device0 (called in
> > > the figure clk 1). A link between device0 and the new clk 1 instance of
> > > periph0 will be created and stored in the clk->consumer_link entry.
> > >
> > > +----------------+ +----------------+
> > > | | | |
> > > | | | |
> > > | xtal0 core | | xtal1 core |
> > > | | | |
> > > | | | |
> > > +-------^^-------+ +-------^^-------+
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | xtal0 clk | | xtal1 clk |
> > > | | | |
> > > +----------------+ +-------^--------+
> > > |
> > > |
> > > +--------------+
> > > | ->parent_clk_link
> > > |
> > > +----------------+
> > > | |
> > > | |
> > > | periph0 core |
> > > | <-------------+
> > > | <-------------|
> > > +-------^^-------+ ||
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | periph0 clk 0 | | periph0 clk 1 |
> > > | | | |
> > > +----------------+ +----------------+
> > > |
> > > | ->consumer_link
> > > |
> > > |
> > > |
> > > +-------v--------+
> > > | device0 |
> > > +----------------+
> > >
> > > Right now, device0 is linked to periph0, itself linked to xtal1 so
> > > everything is fine.
> > >
> > > Now let's get some fun: the new parent of periph0 is xtal1. The process
> > > will call clk_reparent(), periph0's core->parent_clk_link will be
> > > destroyed and a new link to xtal1 will be setup and stored. The
> > > situation is now that device0 is linked to periph0 and periph0 is
> > > linked to xtal1, so the dependency between device0 and xtal1 is still
> > > clear.
> > >
> > > +----------------+ +----------------+
> > > | | | |
> > > | | | |
> > > | xtal0 core | | xtal1 core |
> > > | | | |
> > > | | | |
> > > +-------^^-------+ +-------^^-------+
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | xtal0 clk | | xtal1 clk |
> > > | | | |
> > > +-------^--------+ +----------------+
> > > |
> > > | \ /
> > > +----------------------------x
> > > ->parent_clk_link | / \
> > > |
> > > +----------------+
> > > | |
> > > | |
> > > | periph0 core |
> > > | <-------------+
> > > | <-------------|
> > > +-------^^-------+ ||
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | periph0 clk 0 | | periph0 clk 1 |
> > > | | | |
> > > +----------------+ +----------------+
> > > |
> > > | ->consumer_link
> > > |
> > > |
> > > |
> > > +-------v--------+
> > > | device0 |
> > > +----------------+
> > >
> > > I assume periph0 cannot be removed while there are devices using it,
> > > same for xtal0.
> > >
> > > What can happen is that device0 'put' the clock periph0. The relevant
> > > link is deleted and the clk instance dropped.
> > >
> > > +----------------+ +----------------+
> > > | | | |
> > > | | | |
> > > | xtal0 core | | xtal1 core |
> > > | | | |
> > > | | | |
> > > +-------^^-------+ +-------^^-------+
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | xtal0 clk | | xtal1 clk |
> > > | | | |
> > > +-------^--------+ +----------------+
> > > |
> > > | \ /
> > > +----------------------------x
> > > ->parent_clk_link | / \
> > > |
> > > +----------------+
> > > | |
> > > | |
> > > | periph0 core |
> > > | |
> > > | |
> > > +-------^^-------+
> > > ||
> > > ||
> > > +----------------+
> > > | |
> > > | periph0 clk 0 |
> > > | |
> > > +----------------+
> > >
> > > Now we can unregister periph0: link with the parent will be destroyed
> > > and the clock may be safely removed.
> > >
> > > +----------------+ +----------------+
> > > | | | |
> > > | | | |
> > > | xtal0 core | | xtal1 core |
> > > | | | |
> > > | | | |
> > > +-------^^-------+ +-------^^-------+
> > > || ||
> > > || ||
> > > +----------------+ +----------------+
> > > | | | |
> > > | xtal0 clk | | xtal1 clk |
> > > | | | |
> > > +----------------+ +----------------+
> > >
> > >
> > > This is my understanding of the common clock framework and how links
> > > can be added to it.
> > >
> > > As a result, here are the links created during the boot of an
> > > ESPRESSObin:
> > >
> >
> > Sorry this patch series is taking way too long to get merged. It's
> > already mid-April!
> >
> > So I still have some of the original questions I had from before, mostly
> > around circular parent chains between clk providers. For example, there
> > are clk providers that both provide clks to other providers and consume
> > clks from those providers. Does device links work gracefully here?
> >
> > Just speaking from my own qcom experience, I can point to the PCIe PHY
> > that's a provider of a clk to GCC and a consumer of a clk in GCC. In
> > block diagram form this is:
> >
> >
> > PCIE PHY GCC
> > +--------------+ +-------------------------+
> > | | | |
> > | PHY clk ->----------+---- gcc_pipe_clk ---+ |
> > | | | | |
> > | | | | |
> > | pci_pipe_clk <----------|---------------------+ |
> > | | | |
> > +--------------+ +-------------------------+
> >
> > The end result is that the PCIe PHY is a clk controller that provides
> > the PHY clk to GCC's gcc_pipe_clk and then it gets the same clk signal
> > back from GCC and uses it on the PCIe PHY's pci_pipe_clk input.
> >
> > So is this is a problem?
> >
>
> It's now my turn to get back on this topic.
>
> I just put my noise back into this and for what I understand of the
> clk subsystem, I think the situation you describe could be pictured
> like this:
>
>
> +---------------+
> | |
> | |
> | PCIe PHY |
> | |
> | |
> +-----^^--------+
> ||
> ||
> +---------------+
> | |
> | pcie_pipe_clk |
> | |
> +------^--------+
> |
> | ->parent_clk_link
> |
> |
> +---------------+
> | |
> | |
> | GCC |
> | |
> | |
> +------^^-------+
> ||
> ||
> +---------------+
> | |
> | gcc_pipe_clk |
> | |
> +------^--------+
> |
> | ->parent_clk_link
> |
> |
> +---------------+
> | |
> | |
> | PCIe PHY |
> | |
> | |
> +------^^-------+
> ||
> ||
> +---------------+
> | |
> | phy_clk |
> | |
> +---------------+
>
>
> IMHO the fact that the first and third blocks are the same does not
> interfere with device links.
>
> Honestly, I cannot be 100% sure it won't break on qcom designs, maybe
> the best would be to have someone to test. I don't have the relevant
> hardware. Do you? It would be really helpful!
>
> There is an entire PCIe series blocked, waiting for these device links
> to be merged so it would help a lot if someone could test.
>
Could you share the status of this series? Will it be applied for the
next merge window? I would really like to see this moving forward.
> Thank you very much,
> Miquèl
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/4] Add device links to clocks
2019-06-17 9:57 ` Miquel Raynal
@ 2019-07-27 8:53 ` Miquel Raynal
2019-08-14 18:41 ` Stephen Boyd
0 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2019-07-27 8:53 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Russell King, linux-clk, linux-kernel,
linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Gregory Clement, Nadav Haklai
Hi Stephen,
Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 17 Jun 2019
11:57:03 +0200:
> Hi Stephen,
>
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Tue, 21 May 2019
> 11:46:44 +0200:
>
> > Hi Stephen,
> >
> > Stephen Boyd <sboyd@kernel.org> wrote on Thu, 11 Apr 2019 16:34:16
> > -0700:
> >
> > > Quoting Miquel Raynal (2019-01-08 08:19:36)
> > > > Hello,
> > > >
> > > > While working on suspend to RAM feature, I ran into troubles multiple
> > > > times when clocks where not suspending/resuming at the desired time. I
> > > > had a look at the core and I think the same logic as in the
> > > > regulator's core may be applied here to (very easily) fix this issue:
> > > > using device links.
> > > >
> > > > The only additional change I had to do was to always (when available)
> > > > populate the device entry of the core clock structure so that it could
> > > > be used later. This is the purpose of patch 1. Patch 2 actually adds
> > > > support for device links.
> > > >
> > > > Here is a step-by-step explanation of how links are managed, following
> > > > Maxime Ripard's suggestion.
> > > >
> > > >
> > > > The order of probe has no importance because the framework already
> > > > handles orphaned clocks so let's be simple and say there are two root
> > > > clocks, not depending on anything, that are probed first: xtal0 and
> > > > xtal1. None of these clocks have a parent, there is no device link in
> > > > the game, yet.
> > > >
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | | | |
> > > > | xtal0 core | | xtal1 core |
> > > > | | | |
> > > > | | | |
> > > > +-------^^-------+ +-------^^-------+
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | xtal0 clk | | xtal1 clk |
> > > > | | | |
> > > > +----------------+ +----------------+
> > > >
> > > > Then, a peripheral clock periph0 is probed. His parent is xtal1. The
> > > > clock_register_*() call will run __clk_init_parent() and a link between
> > > > periph0's core and xtal1's core will be created and stored in
> > > > periph0's core->parent_clk_link entry.
> > > >
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | | | |
> > > > | xtal0 core | | xtal1 core |
> > > > | | | |
> > > > | | | |
> > > > +-------^^-------+ +-------^^-------+
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | xtal0 clk | | xtal1 clk |
> > > > | | | |
> > > > +----------------+ +-------^--------+
> > > > |
> > > > |
> > > > +--------------+
> > > > | ->parent_clk_link
> > > > |
> > > > +----------------+
> > > > | |
> > > > | |
> > > > | periph0 core |
> > > > | |
> > > > | |
> > > > +-------^^-------+
> > > > ||
> > > > ||
> > > > +----------------+
> > > > | |
> > > > | periph0 clk 0 |
> > > > | |
> > > > +----------------+
> > > >
> > > > Then, device0 is probed and "get" the periph0 clock. clk_get() will be
> > > > called and a struct clk will be instantiated for device0 (called in
> > > > the figure clk 1). A link between device0 and the new clk 1 instance of
> > > > periph0 will be created and stored in the clk->consumer_link entry.
> > > >
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | | | |
> > > > | xtal0 core | | xtal1 core |
> > > > | | | |
> > > > | | | |
> > > > +-------^^-------+ +-------^^-------+
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | xtal0 clk | | xtal1 clk |
> > > > | | | |
> > > > +----------------+ +-------^--------+
> > > > |
> > > > |
> > > > +--------------+
> > > > | ->parent_clk_link
> > > > |
> > > > +----------------+
> > > > | |
> > > > | |
> > > > | periph0 core |
> > > > | <-------------+
> > > > | <-------------|
> > > > +-------^^-------+ ||
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | periph0 clk 0 | | periph0 clk 1 |
> > > > | | | |
> > > > +----------------+ +----------------+
> > > > |
> > > > | ->consumer_link
> > > > |
> > > > |
> > > > |
> > > > +-------v--------+
> > > > | device0 |
> > > > +----------------+
> > > >
> > > > Right now, device0 is linked to periph0, itself linked to xtal1 so
> > > > everything is fine.
> > > >
> > > > Now let's get some fun: the new parent of periph0 is xtal1. The process
> > > > will call clk_reparent(), periph0's core->parent_clk_link will be
> > > > destroyed and a new link to xtal1 will be setup and stored. The
> > > > situation is now that device0 is linked to periph0 and periph0 is
> > > > linked to xtal1, so the dependency between device0 and xtal1 is still
> > > > clear.
> > > >
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | | | |
> > > > | xtal0 core | | xtal1 core |
> > > > | | | |
> > > > | | | |
> > > > +-------^^-------+ +-------^^-------+
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | xtal0 clk | | xtal1 clk |
> > > > | | | |
> > > > +-------^--------+ +----------------+
> > > > |
> > > > | \ /
> > > > +----------------------------x
> > > > ->parent_clk_link | / \
> > > > |
> > > > +----------------+
> > > > | |
> > > > | |
> > > > | periph0 core |
> > > > | <-------------+
> > > > | <-------------|
> > > > +-------^^-------+ ||
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | periph0 clk 0 | | periph0 clk 1 |
> > > > | | | |
> > > > +----------------+ +----------------+
> > > > |
> > > > | ->consumer_link
> > > > |
> > > > |
> > > > |
> > > > +-------v--------+
> > > > | device0 |
> > > > +----------------+
> > > >
> > > > I assume periph0 cannot be removed while there are devices using it,
> > > > same for xtal0.
> > > >
> > > > What can happen is that device0 'put' the clock periph0. The relevant
> > > > link is deleted and the clk instance dropped.
> > > >
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | | | |
> > > > | xtal0 core | | xtal1 core |
> > > > | | | |
> > > > | | | |
> > > > +-------^^-------+ +-------^^-------+
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | xtal0 clk | | xtal1 clk |
> > > > | | | |
> > > > +-------^--------+ +----------------+
> > > > |
> > > > | \ /
> > > > +----------------------------x
> > > > ->parent_clk_link | / \
> > > > |
> > > > +----------------+
> > > > | |
> > > > | |
> > > > | periph0 core |
> > > > | |
> > > > | |
> > > > +-------^^-------+
> > > > ||
> > > > ||
> > > > +----------------+
> > > > | |
> > > > | periph0 clk 0 |
> > > > | |
> > > > +----------------+
> > > >
> > > > Now we can unregister periph0: link with the parent will be destroyed
> > > > and the clock may be safely removed.
> > > >
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | | | |
> > > > | xtal0 core | | xtal1 core |
> > > > | | | |
> > > > | | | |
> > > > +-------^^-------+ +-------^^-------+
> > > > || ||
> > > > || ||
> > > > +----------------+ +----------------+
> > > > | | | |
> > > > | xtal0 clk | | xtal1 clk |
> > > > | | | |
> > > > +----------------+ +----------------+
> > > >
> > > >
> > > > This is my understanding of the common clock framework and how links
> > > > can be added to it.
> > > >
> > > > As a result, here are the links created during the boot of an
> > > > ESPRESSObin:
> > > >
> > >
> > > Sorry this patch series is taking way too long to get merged. It's
> > > already mid-April!
> > >
> > > So I still have some of the original questions I had from before, mostly
> > > around circular parent chains between clk providers. For example, there
> > > are clk providers that both provide clks to other providers and consume
> > > clks from those providers. Does device links work gracefully here?
> > >
> > > Just speaking from my own qcom experience, I can point to the PCIe PHY
> > > that's a provider of a clk to GCC and a consumer of a clk in GCC. In
> > > block diagram form this is:
> > >
> > >
> > > PCIE PHY GCC
> > > +--------------+ +-------------------------+
> > > | | | |
> > > | PHY clk ->----------+---- gcc_pipe_clk ---+ |
> > > | | | | |
> > > | | | | |
> > > | pci_pipe_clk <----------|---------------------+ |
> > > | | | |
> > > +--------------+ +-------------------------+
> > >
> > > The end result is that the PCIe PHY is a clk controller that provides
> > > the PHY clk to GCC's gcc_pipe_clk and then it gets the same clk signal
> > > back from GCC and uses it on the PCIe PHY's pci_pipe_clk input.
> > >
> > > So is this is a problem?
> > >
> >
> > It's now my turn to get back on this topic.
> >
> > I just put my noise back into this and for what I understand of the
> > clk subsystem, I think the situation you describe could be pictured
> > like this:
> >
> >
> > +---------------+
> > | |
> > | |
> > | PCIe PHY |
> > | |
> > | |
> > +-----^^--------+
> > ||
> > ||
> > +---------------+
> > | |
> > | pcie_pipe_clk |
> > | |
> > +------^--------+
> > |
> > | ->parent_clk_link
> > |
> > |
> > +---------------+
> > | |
> > | |
> > | GCC |
> > | |
> > | |
> > +------^^-------+
> > ||
> > ||
> > +---------------+
> > | |
> > | gcc_pipe_clk |
> > | |
> > +------^--------+
> > |
> > | ->parent_clk_link
> > |
> > |
> > +---------------+
> > | |
> > | |
> > | PCIe PHY |
> > | |
> > | |
> > +------^^-------+
> > ||
> > ||
> > +---------------+
> > | |
> > | phy_clk |
> > | |
> > +---------------+
> >
> >
> > IMHO the fact that the first and third blocks are the same does not
> > interfere with device links.
> >
> > Honestly, I cannot be 100% sure it won't break on qcom designs, maybe
> > the best would be to have someone to test. I don't have the relevant
> > hardware. Do you? It would be really helpful!
> >
> > There is an entire PCIe series blocked, waiting for these device links
> > to be merged so it would help a lot if someone could test.
> >
>
> Could you share the status of this series? Will it be applied for the
> next merge window? I would really like to see this moving forward.
I know this series might have side effects despite the consequent
amount of time spent to write and test it, but I also think the
clk subsystem would really benefit from such change and handling
suspend to RAM support would be greatly enhanced. You seemed
interested at first and now not anymore, could I know why? I got
inspired by the regulators subsystem. It is not an idea of mine
that device links should be bring to clocks. Regulators are almost
as used as clocks so I really understand your fears but why not
applying this to -next very early during the -rc cycles and see
what happens? You'll have plenty of time to ask me to fix things
or even drop it off.
Kind regards,
Miquèl
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/4] Add device links to clocks
2019-07-27 8:53 ` Miquel Raynal
@ 2019-08-14 18:41 ` Stephen Boyd
0 siblings, 0 replies; 13+ messages in thread
From: Stephen Boyd @ 2019-08-14 18:41 UTC (permalink / raw)
To: Miquel Raynal
Cc: Michael Turquette, Russell King, linux-clk, linux-kernel,
linux-arm-kernel, Thomas Petazzoni, Antoine Tenart,
Maxime Chevallier, Gregory Clement, Nadav Haklai
Quoting Miquel Raynal (2019-07-27 01:53:30)
>
> I know this series might have side effects despite the consequent
> amount of time spent to write and test it, but I also think the
> clk subsystem would really benefit from such change and handling
> suspend to RAM support would be greatly enhanced. You seemed
> interested at first and now not anymore, could I know why? I got
> inspired by the regulators subsystem. It is not an idea of mine
> that device links should be bring to clocks. Regulators are almost
> as used as clocks so I really understand your fears but why not
> applying this to -next very early during the -rc cycles and see
> what happens? You'll have plenty of time to ask me to fix things
> or even drop it off.
>
Ok, I'm back on this topic. Let me look at the latest code and see how
it works on a qcom platform I have in hand. If the device links look OK
then it should be good. I also want to make sure we're not holding a
nested pile of locks when we're adding the device links so that we don't
get some weird lockdep problems.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-14 18:41 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08 16:19 [PATCH v4 0/4] Add device links to clocks Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 1/4] clk: core: link consumer with clock driver Miquel Raynal
2019-01-08 16:19 ` [PATCH v4 2/4] clk: mvebu: armada-37xx-tbg: fix error message Miquel Raynal
2019-01-18 14:16 ` Gregory CLEMENT
2019-01-08 16:19 ` [PATCH v4 3/4] clk: mvebu: armada-37xx-tbg: fill the device entry when registering the clocks Miquel Raynal
2019-01-18 14:16 ` Gregory CLEMENT
2019-01-08 16:19 ` [PATCH v4 4/4] clk: mvebu: armada-37xx-xtal: fill the device entry when registering the clock Miquel Raynal
2019-01-18 14:16 ` Gregory CLEMENT
2019-04-11 23:34 ` [PATCH v4 0/4] Add device links to clocks Stephen Boyd
2019-05-21 9:46 ` Miquel Raynal
2019-06-17 9:57 ` Miquel Raynal
2019-07-27 8:53 ` Miquel Raynal
2019-08-14 18:41 ` Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).