[PATCH v2] clk: fixed-rate: Use "clock-output-names" to name fixed clocks

[Marek Vasut]

On 4/19/25 10:40 PM, Adam Ford wrote:
> On Sat, Apr 19, 2025 at 2:12 PM Fabio Estevam <festevam at gmail.com> wrote:
>>
>> Hi Marek,
>>
>> On Fri, Apr 18, 2025 at 11:53 AM Marek Vasut <marex at 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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clk.diff
Type: text/x-patch
Size: 2250 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250420/5515a450/attachment.bin>