Re: [PATCH] clk: Dont return error when assigned-clocks is empty or missing

[Michal Simek]

On 8/25/23 16:39, Tom Rini wrote:
On Fri, Aug 25, 2023 at 09:15:09AM +0200, Michal Simek wrote:
Hi Tom,

On 7/11/23 11:51, Ashok Reddy Soma wrote:
There is a chance that assigned-clock-rates is given and assigned-clocks
could be empty. Dont return error in that case, because the probe of the
corresponding driver will not be called at all if this fails.
Better to continue to look for it and return 0.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.s...@amd.com>
---

   drivers/clk/clk-uclass.c | 8 +++++++-
   1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index dc3e9d6a26..f186fcbcdb 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -329,7 +329,13 @@ static int clk_set_default_rates(struct udevice *dev,
                        dev_dbg(dev,
                                "could not get assigned clock %d (err = %d)\n",
                                index, ret);
-                       continue;
+                       /* Skip if it is empty */
+                       if (ret == -ENOENT) {
+                               ret = 0;
+                               continue;
+                       }
+
+                       return ret;
                }
                /* This is clk provider device trying to program itself

What's your take on this one?  I didn't get reply from Sean.

I guess, what's the validated upstream dts where this is the case?


It was found by incorrect DT. It means I don't think there is any DT which 
contains this issue.
But that being said we can extend current clock tests to cover this case.
Please look below.

Thanks,
Michal

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index ff9f9222e6f9..0f9bedc0065c 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -589,6 +589,16 @@
                clock-names = "fixed", "i2c", "spi", "uart2", "uart1";
        };

+       clk-test2 {
+               compatible = "sandbox,clk-test";
+               assigned-clock-rates = <321>;
+       };
+
+       clk-test3 {
+               compatible = "sandbox,clk-test";
+               assigned-clocks = <&clk_sandbox 1>;
+       };
+
        ccf: clk-ccf {
                compatible = "sandbox,clk-ccf";
        };
diff --git a/test/dm/clk.c b/test/dm/clk.c
index f48de05436bf..71ae32eaff50 100644
--- a/test/dm/clk.c
+++ b/test/dm/clk.c
@@ -36,6 +36,12 @@ static int dm_test_clk_base(struct unit_test_state *uts)
        ut_asserteq(clk_is_match(&clk_method1, &clk_method2), true);
        ut_asserteq(clk_method1.id, clk_method2.id);

+       ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "clk-test2", &dev));
+       ut_assertok(clk_set_defaults(dev, CLK_DEFAULTS_PRE));
+
+       ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "clk-test3", &dev));
+       ut_assertok(clk_set_defaults(dev, CLK_DEFAULTS_PRE));
+
        return 0;
 }