On 10/25/21 11:18 AM, Simon Glass wrote:
Hi Sean,

On Sun, 24 Oct 2021 at 18:13, Sean Anderson <sean...@gmail.com> wrote:

On 10/14/21 10:19 PM, Simon Glass wrote:
Hi Peng, Sean,

On Thu, 14 Oct 2021 at 19:17, Peng Fan <peng....@nxp.com> wrote:

Subject: Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults


On 10/13/21 5:37 AM, Peng Fan (OSS) wrote:
From: Peng Fan <peng....@nxp.com>

Current code has a force clk_set_defaults in multiple stages, U-Boot
reuse the same device tree and Linux Kernel device tree, but we not
register all the clks as Linux Kernel, so clk_set_defaults will fail
and cause the clk provider registeration fail.

So introduce a new property to ignore the default settings which could
be used by any node that wanna ignore default settings.

Signed-off-by: Peng Fan <peng....@nxp.com>
---
    doc/device-tree-bindings/device.txt | 3 +++
    drivers/clk/clk-uclass.c            | 3 +++
    2 files changed, 6 insertions(+)

diff --git a/doc/device-tree-bindings/device.txt
b/doc/device-tree-bindings/device.txt
index 73ce2a3b5b..fe34ced268 100644
--- a/doc/device-tree-bindings/device.txt
+++ b/doc/device-tree-bindings/device.txt
@@ -28,6 +28,9 @@ the acpi,compatible property.
        Linux will only load the driver if the device can be detected (e.g. on
I2C
        bus). Note that this is an out-of-tree Linux feature.

+Common device bindings that could be shared listed below:
+ - u-boot,ignore-clk-defaults : ignore the assigned-clock-parents
+   and assigned-clock-rates for a device that has the property.

    Example
    -------
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index
493018b33e..6bf3179e7b 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -376,6 +376,9 @@ int clk_set_defaults(struct udevice *dev, enum
clk_defaults_stage stage)
      if (!dev_has_ofnode(dev))
              return 0;

+   if (ofnode_get_property(dev_ofnode(dev), "u-boot,ignore-clk-defaults",
NULL))
+           return 0;
+
      /*
       * To avoid setting defaults twice, don't set them before relocation.
       * However, still set them for SPL. And still set them if
explicitly


Why not just have the property ignore errors?

I think the force err return was done by Simon?


In the long term, it may be better to standardize that e.g. ENOENT means that
the clock doesn't exist. That way we can skip setting the defaults.
ENOSYS should probably be treated the same way (warn, but don't fail).

I am not sure whether people expect force error for ENOENT/ENOSYS in U-Boot.
For i.MX, I not expect force error.

Yes that is me, indeed. It's just that we should not silently ignore
errors. If we know the clock is optional, then the driver that knows
that can handle it. But if we start having things quietly fail,
debugging becomes a pain.

Can't we have them loudly fail instead?


That is how it works today, as I understand it. But some boards want
the defaults to be there but not to implement them in U-Boot. This
seems fair enough to me. Perhaps we could add something to each node
instead, to disable it?

u-boot,assigned-clock-status = "disabled";

?

--Sean

Reply via email to