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? Regards, Simon > --Sean