* [PATCH] clk: introduce u-boot,ignore-clk-defaults @ 2021-10-13 9:37 Peng Fan (OSS) 2021-10-14 15:11 ` Simon Glass 2021-10-15 1:10 ` Sean Anderson 0 siblings, 2 replies; 12+ messages in thread From: Peng Fan (OSS) @ 2021-10-13 9:37 UTC (permalink / raw) To: lukma, sjg, trini, seanga2; +Cc: u-boot, Peng Fan From: Peng Fan <peng.fan@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.fan@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 -- 2.30.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-13 9:37 [PATCH] clk: introduce u-boot,ignore-clk-defaults Peng Fan (OSS) @ 2021-10-14 15:11 ` Simon Glass 2021-10-14 15:14 ` Tom Rini 2021-10-15 8:58 ` Peng Fan (OSS) 2021-10-15 1:10 ` Sean Anderson 1 sibling, 2 replies; 12+ messages in thread From: Simon Glass @ 2021-10-14 15:11 UTC (permalink / raw) To: Peng Fan (OSS) Cc: Lukasz Majewski, Tom Rini, Sean Anderson, U-Boot Mailing List, Peng Fan On Wed, 13 Oct 2021 at 03:00, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > From: Peng Fan <peng.fan@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.fan@nxp.com> > --- > doc/device-tree-bindings/device.txt | 3 +++ > drivers/clk/clk-uclass.c | 3 +++ > 2 files changed, 6 insertions(+) > Reviewed-by: Simon Glass <sjg@chromium.org> I wonder if this binding could be made standard, instead of 'u-boot,'? > 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 > -- > 2.30.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-14 15:11 ` Simon Glass @ 2021-10-14 15:14 ` Tom Rini 2021-10-15 8:58 ` Peng Fan (OSS) 1 sibling, 0 replies; 12+ messages in thread From: Tom Rini @ 2021-10-14 15:14 UTC (permalink / raw) To: Simon Glass Cc: Peng Fan (OSS), Lukasz Majewski, Sean Anderson, U-Boot Mailing List, Peng Fan [-- Attachment #1: Type: text/plain, Size: 1001 bytes --] On Thu, Oct 14, 2021 at 09:11:26AM -0600, Simon Glass wrote: > On Wed, 13 Oct 2021 at 03:00, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > > > From: Peng Fan <peng.fan@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.fan@nxp.com> > > --- > > doc/device-tree-bindings/device.txt | 3 +++ > > drivers/clk/clk-uclass.c | 3 +++ > > 2 files changed, 6 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > I wonder if this binding could be made standard, instead of 'u-boot,'? Feels like something we should see about, yes. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-14 15:11 ` Simon Glass 2021-10-14 15:14 ` Tom Rini @ 2021-10-15 8:58 ` Peng Fan (OSS) 2021-10-24 19:53 ` Simon Glass 1 sibling, 1 reply; 12+ messages in thread From: Peng Fan (OSS) @ 2021-10-15 8:58 UTC (permalink / raw) To: Simon Glass Cc: Lukasz Majewski, Tom Rini, Sean Anderson, U-Boot Mailing List, Peng Fan Simon, On 2021/10/14 23:11, Simon Glass wrote: > On Wed, 13 Oct 2021 at 03:00, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: >> >> From: Peng Fan <peng.fan@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.fan@nxp.com> >> --- >> doc/device-tree-bindings/device.txt | 3 +++ >> drivers/clk/clk-uclass.c | 3 +++ >> 2 files changed, 6 insertions(+) >> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > I wonder if this binding could be made standard, instead of 'u-boot,'? Sorry, could you explain more? You mean put it in linux device tree in linux kernel repo? Thanks, Peng. > > >> 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 >> -- >> 2.30.0 >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-15 8:58 ` Peng Fan (OSS) @ 2021-10-24 19:53 ` Simon Glass 0 siblings, 0 replies; 12+ messages in thread From: Simon Glass @ 2021-10-24 19:53 UTC (permalink / raw) To: Peng Fan (OSS) Cc: Lukasz Majewski, Tom Rini, Sean Anderson, U-Boot Mailing List, Peng Fan Hi Peng, On Fri, 15 Oct 2021 at 02:58, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > Simon, > > On 2021/10/14 23:11, Simon Glass wrote: > > On Wed, 13 Oct 2021 at 03:00, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > >> > >> From: Peng Fan <peng.fan@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.fan@nxp.com> > >> --- > >> doc/device-tree-bindings/device.txt | 3 +++ > >> drivers/clk/clk-uclass.c | 3 +++ > >> 2 files changed, 6 insertions(+) > >> > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > I wonder if this binding could be made standard, instead of 'u-boot,'? > > Sorry, could you explain more? You mean put it in linux device tree in > linux kernel repo? > Yes, if you think it would be useful there. Just a suggestion. Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-13 9:37 [PATCH] clk: introduce u-boot,ignore-clk-defaults Peng Fan (OSS) 2021-10-14 15:11 ` Simon Glass @ 2021-10-15 1:10 ` Sean Anderson 2021-10-15 1:17 ` Peng Fan 1 sibling, 1 reply; 12+ messages in thread From: Sean Anderson @ 2021-10-15 1:10 UTC (permalink / raw) To: Peng Fan (OSS), lukma, sjg, trini; +Cc: u-boot, Peng Fan On 10/13/21 5:37 AM, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@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.fan@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? 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). --Sean ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-15 1:10 ` Sean Anderson @ 2021-10-15 1:17 ` Peng Fan 2021-10-15 2:19 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Peng Fan @ 2021-10-15 1:17 UTC (permalink / raw) To: Sean Anderson, Peng Fan (OSS), lukma, sjg, trini; +Cc: u-boot > 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.fan@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.fan@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. Regards, Peng. > > --Sean ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-15 1:17 ` Peng Fan @ 2021-10-15 2:19 ` Simon Glass 2021-10-25 0:13 ` Sean Anderson 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2021-10-15 2:19 UTC (permalink / raw) To: Peng Fan; +Cc: Sean Anderson, Peng Fan (OSS), lukma, trini, u-boot Hi Peng, Sean, On Thu, 14 Oct 2021 at 19:17, Peng Fan <peng.fan@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.fan@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.fan@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. Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-15 2:19 ` Simon Glass @ 2021-10-25 0:13 ` Sean Anderson 2021-10-25 15:18 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Sean Anderson @ 2021-10-25 0:13 UTC (permalink / raw) To: Simon Glass, Peng Fan; +Cc: Peng Fan (OSS), lukma, trini, u-boot On 10/14/21 10:19 PM, Simon Glass wrote: > Hi Peng, Sean, > > On Thu, 14 Oct 2021 at 19:17, Peng Fan <peng.fan@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.fan@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.fan@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? --Sean ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-25 0:13 ` Sean Anderson @ 2021-10-25 15:18 ` Simon Glass 2021-10-26 1:23 ` Sean Anderson 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2021-10-25 15:18 UTC (permalink / raw) To: Sean Anderson; +Cc: Peng Fan, Peng Fan (OSS), lukma, trini, u-boot Hi Sean, On Sun, 24 Oct 2021 at 18:13, Sean Anderson <seanga2@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.fan@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.fan@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.fan@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-25 15:18 ` Simon Glass @ 2021-10-26 1:23 ` Sean Anderson 2021-10-26 1:59 ` Sean Anderson 0 siblings, 1 reply; 12+ messages in thread From: Sean Anderson @ 2021-10-26 1:23 UTC (permalink / raw) To: Simon Glass; +Cc: Peng Fan, Peng Fan (OSS), lukma, trini, u-boot On 10/25/21 11:18 AM, Simon Glass wrote: > Hi Sean, > > On Sun, 24 Oct 2021 at 18:13, Sean Anderson <seanga2@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.fan@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.fan@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.fan@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: introduce u-boot,ignore-clk-defaults 2021-10-26 1:23 ` Sean Anderson @ 2021-10-26 1:59 ` Sean Anderson 0 siblings, 0 replies; 12+ messages in thread From: Sean Anderson @ 2021-10-26 1:59 UTC (permalink / raw) To: Simon Glass; +Cc: Peng Fan, Peng Fan (OSS), lukma, trini, u-boot On 10/25/21 9:23 PM, Sean Anderson wrote: > On 10/25/21 11:18 AM, Simon Glass wrote: >> Hi Sean, >> >> On Sun, 24 Oct 2021 at 18:13, Sean Anderson <seanga2@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.fan@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.fan@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.fan@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"; Actually, I think that was Peng's idea in the first place :) --Sean ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-10-26 2:00 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-13 9:37 [PATCH] clk: introduce u-boot,ignore-clk-defaults Peng Fan (OSS) 2021-10-14 15:11 ` Simon Glass 2021-10-14 15:14 ` Tom Rini 2021-10-15 8:58 ` Peng Fan (OSS) 2021-10-24 19:53 ` Simon Glass 2021-10-15 1:10 ` Sean Anderson 2021-10-15 1:17 ` Peng Fan 2021-10-15 2:19 ` Simon Glass 2021-10-25 0:13 ` Sean Anderson 2021-10-25 15:18 ` Simon Glass 2021-10-26 1:23 ` Sean Anderson 2021-10-26 1:59 ` Sean Anderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.