On 4/19/25 10:40 PM, Adam Ford wrote:
On Sat, Apr 19, 2025 at 2:12 PM Fabio Estevam <feste...@gmail.com> wrote:
Hi Marek,
On Fri, Apr 18, 2025 at 11:53 AM Marek Vasut <ma...@denx.de> wrote:
The "clock-output-names" is NEVER used for any clock look up.
In Linux, "clock-output-names" is used to register the name of the
fixed-rate clock.
From Linux drivers/clk/clk-fixed-rate.c:
```
static struct clk_hw *_of_fixed_clk_setup(struct device_node *node)
{
....
of_property_read_string(node, "clock-output-names", &clk_name);
hw = clk_hw_register_fixed_rate_with_accuracy(NULL, clk_name, NULL,
0, rate, accuracy);
```
In U-Boot, we should register a fixed-rate clock with the same name as in Linux.
This patch aims to make the U-Boot fixed-rate clock name conform to
Linux standards.
I am fine with the first half of the description, but the second half is
wrong and this actually does not fix anything. Simply remove
clock-output-names from the osc_24m and the code will be broken again.
In Linux, if "clock-output-names" is removed from the osc_24m node,
the console becomes corrupted
on an imx8mp-evk.
The clk_resolve_parent_clk() is meant to perform the aforementioned
resolution(), that is what "[PATCH v2 00/24] clk: Add
clk_resolve_parent_clk() and fix up iMX clock drivers" actually fixed.
It seems there is something still missing and clk_resolve_parent_clk()
does not do the resolution properly for some clock, but that is what
needs to be understood and fixed, not worked around this way.
Please try to reproduce the problem on your end.
On an imx8mp-based board, run 'usb start' in U-Boot, and this command will fail.
From what I can tell, on the imx8mp, the usb_phy_ref parent->dev->name
value s "clock-osc-24m" which never matches "osc_24" and I put some
debug code into clk_mux_fetch_parent_index.
I think clk_mux_fetch_parent_index needs some additional logic to go
up the chain to the root node and look for 'clock-names' to match in
addition to what it already does. I am just not sure of the best
method to go up the tree to find the root node that defines it.
[...]
Try something like this (also attached), maybe this needs to be made
generic ?
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 9e3b5191767..207224b1320 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -155,6 +155,8 @@ struct clk *clk_register_composite(struct udevice
*dev, const char *name,
goto err;
}
+ composite->dev = dev;
+
if (composite->mux)
composite->mux->dev = clk->dev;
if (composite->rate)
diff --git a/drivers/clk/imx/clk-composite-8m.c
b/drivers/clk/imx/clk-composite-8m.c
index 14c5b92939c..e1a3c0af308 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -116,14 +116,42 @@ static const struct clk_ops
imx8m_clk_composite_divider_ops = {
.set_rate = imx8m_clk_composite_divider_set_rate,
};
+static int imx8m_clk_mux_fetch_parent_index(struct udevice *cdev,
struct clk *clk, struct clk *parent)
+{
+ struct clk_mux *mux = to_clk_mux(clk);
+ struct clk cclk;
+ int ret;
+ int i;
+
+ if (!parent)
+ return -EINVAL;
+
+ for (i = 0; i < mux->num_parents; i++) {
+ ret = clk_get_by_name(cdev, mux->parent_names[i], &cclk);
+ if (!ret && ofnode_equal(dev_ofnode(parent->dev),
dev_ofnode(cclk.dev)))
+ return i;
+
+ if (!strcmp(parent->dev->name, mux->parent_names[i]))
+ return i;
+ if (!strcmp(parent->dev->name,
+ clk_resolve_parent_clk(clk->dev,
+ mux->parent_names[i])))
+ return i;
+ }
+
+ return -EINVAL;
+}
+
+
static int imx8m_clk_mux_set_parent(struct clk *clk, struct clk *parent)
{
struct clk_mux *mux = to_clk_mux(clk);
+ struct clk_composite *composite = (struct clk_composite *)clk->data;
int index;
u32 val;
u32 reg;
- index = clk_mux_fetch_parent_index(clk, parent);
+ index = imx8m_clk_mux_fetch_parent_index(composite->dev, clk, parent);
if (index < 0) {
log_err("Could not fetch index\n");
return index;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5ea2171492e..267757939e0 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -219,6 +219,8 @@ struct clk_composite {
const struct clk_ops *mux_ops;
const struct clk_ops *rate_ops;
const struct clk_ops *gate_ops;
+
+ struct udevice *dev;
};
#define to_clk_composite(_clk) container_of(_clk, struct
clk_composite, clk)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 9e3b5191767..207224b1320 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -155,6 +155,8 @@ struct clk *clk_register_composite(struct udevice *dev, const char *name,
goto err;
}
+ composite->dev = dev;
+
if (composite->mux)
composite->mux->dev = clk->dev;
if (composite->rate)
diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index 14c5b92939c..e1a3c0af308 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -116,14 +116,42 @@ static const struct clk_ops imx8m_clk_composite_divider_ops = {
.set_rate = imx8m_clk_composite_divider_set_rate,
};
+static int imx8m_clk_mux_fetch_parent_index(struct udevice *cdev, struct clk *clk, struct clk *parent)
+{
+ struct clk_mux *mux = to_clk_mux(clk);
+ struct clk cclk;
+ int ret;
+ int i;
+
+ if (!parent)
+ return -EINVAL;
+
+ for (i = 0; i < mux->num_parents; i++) {
+ ret = clk_get_by_name(cdev, mux->parent_names[i], &cclk);
+ if (!ret && ofnode_equal(dev_ofnode(parent->dev), dev_ofnode(cclk.dev)))
+ return i;
+
+ if (!strcmp(parent->dev->name, mux->parent_names[i]))
+ return i;
+ if (!strcmp(parent->dev->name,
+ clk_resolve_parent_clk(clk->dev,
+ mux->parent_names[i])))
+ return i;
+ }
+
+ return -EINVAL;
+}
+
+
static int imx8m_clk_mux_set_parent(struct clk *clk, struct clk *parent)
{
struct clk_mux *mux = to_clk_mux(clk);
+ struct clk_composite *composite = (struct clk_composite *)clk->data;
int index;
u32 val;
u32 reg;
- index = clk_mux_fetch_parent_index(clk, parent);
+ index = imx8m_clk_mux_fetch_parent_index(composite->dev, clk, parent);
if (index < 0) {
log_err("Could not fetch index\n");
return index;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5ea2171492e..267757939e0 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -219,6 +219,8 @@ struct clk_composite {
const struct clk_ops *mux_ops;
const struct clk_ops *rate_ops;
const struct clk_ops *gate_ops;
+
+ struct udevice *dev;
};
#define to_clk_composite(_clk) container_of(_clk, struct clk_composite, clk)
