Re: [PATCH v2 15/16] clk: Detect failure to set defaults
Hi,
On Wed, 20 Oct 2021 at 01:17, Rasmus Villemoes
wrote:
>
> On 20/08/2021 20.18, Simon Glass wrote:
> > Hi Harm,
> >
> > On Wed, 18 Aug 2021 at 08:09, Harm Berntsen wrote:
> >>
> >> On Thu, 2021-05-13 at 19:39 -0600, Simon Glass wrote:
> >>> int clk_uclass_post_probe(struct udevice *dev)
> >>> {
> >>> + int ret;
> >>> +
> >>> /*
> >>> * when a clock provider is probed. Call clk_set_defaults()
> >>> * also after the device is probed. This takes care of cases
> >>> * where the DT is used to setup default parents and rates
> >>> * using assigned-clocks
> >>> */
> >>> - clk_set_defaults(dev, 1);
> >>> + ret = clk_set_defaults(dev, 1);
> >>> + if (ret)
> >>> + return log_ret(ret);
> >>>
> >>> return 0;
> >>> }
> >>
> >> Note that this patch broke booting my imx8mn based board on U-Boot
> >> v2021.10-rc2.
>
> I just ran into the same issue with v2021.10 being broken for
> imx8mp_evk, and git bisect pointing at this commit, symptoms being
>
> U-Boot 2021.10 (Oct 20 2021 - 08:45:51 +0200)
>
> CPU: Freescale i.MX8MP[8] rev1.1 at 1200 MHz
> ...
> MMC: mmc@30b5 - probe failed: -2
> mmc@30b6 - probe failed: -2
>
> Reverting 92f1e9a4b on top of v2021.10 yields
>
> U-Boot 2021.10-1-gac2520a138 (Oct 20 2021 - 09:05:48 +0200)
>
> CPU: Freescale i.MX8MP[8] rev1.1 at 1200 MHz
> ...
> MMC: FSL_SDHC: 1, FSL_SDHC: 2
>
> cc += imx maintainers, this should not still be broken 2 months (and a
> release) after it was reported.
I see a patch to explicitly make this optional, using the devicetree.
Regards,
Simon
Re: [PATCH v2 15/16] clk: Detect failure to set defaults
On 20/08/2021 20.18, Simon Glass wrote:
> Hi Harm,
>
> On Wed, 18 Aug 2021 at 08:09, Harm Berntsen wrote:
>>
>> On Thu, 2021-05-13 at 19:39 -0600, Simon Glass wrote:
>>> int clk_uclass_post_probe(struct udevice *dev)
>>> {
>>> + int ret;
>>> +
>>> /*
>>> * when a clock provider is probed. Call clk_set_defaults()
>>> * also after the device is probed. This takes care of cases
>>> * where the DT is used to setup default parents and rates
>>> * using assigned-clocks
>>> */
>>> - clk_set_defaults(dev, 1);
>>> + ret = clk_set_defaults(dev, 1);
>>> + if (ret)
>>> + return log_ret(ret);
>>>
>>> return 0;
>>> }
>>
>> Note that this patch broke booting my imx8mn based board on U-Boot
>> v2021.10-rc2.
I just ran into the same issue with v2021.10 being broken for
imx8mp_evk, and git bisect pointing at this commit, symptoms being
U-Boot 2021.10 (Oct 20 2021 - 08:45:51 +0200)
CPU: Freescale i.MX8MP[8] rev1.1 at 1200 MHz
...
MMC: mmc@30b5 - probe failed: -2
mmc@30b6 - probe failed: -2
Reverting 92f1e9a4b on top of v2021.10 yields
U-Boot 2021.10-1-gac2520a138 (Oct 20 2021 - 09:05:48 +0200)
CPU: Freescale i.MX8MP[8] rev1.1 at 1200 MHz
...
MMC: FSL_SDHC: 1, FSL_SDHC: 2
cc += imx maintainers, this should not still be broken 2 months (and a
release) after it was reported.
Rasmus
Fwd: Re: [PATCH v2 15/16] clk: Detect failure to set defaults
Hi Stefano and Peng,
There is an issue that prevents the imx8mn to boot in 2021.10-rc2. See
the conversation below. Could you help with this?
-- Harm
Forwarded Message
From: Simon Glass
To: Harm Berntsen
Cc: u-boot@lists.denx.de , tr...@konsulko.com
Subject: Re: [PATCH v2 15/16] clk: Detect failure to set defaults
Date: Fri, 20 Aug 2021 12:18:07 -0600
> Hi Harm,
>
> On Wed, 18 Aug 2021 at 08:09, Harm Berntsen
> wrote:
> >
> > On Thu, 2021-05-13 at 19:39 -0600, Simon Glass wrote:
> > > When the default clocks cannot be set, the clock is silently
> > > probed and
> > > the error is ignored. This is incorrect, since having the clocks
> > > at the
> > > correct speed may be important for operation of the system.
> > >
> > > Fix it by checking the return code.
> > >
> > > Signed-off-by: Simon Glass
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > drivers/clk/clk-uclass.c | 6 +-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > > index 4ab3c402ed8..2a2e1cfbd61 100644
> > > --- a/drivers/clk/clk-uclass.c
> > > +++ b/drivers/clk/clk-uclass.c
> > > @@ -796,13 +796,17 @@ void devm_clk_put(struct udevice *dev,
> > > struct clk
> > > *clk)
> > >
> > > int clk_uclass_post_probe(struct udevice *dev)
> > > {
> > > + int ret;
> > > +
> > > /*
> > > * when a clock provider is probed. Call
> > > clk_set_defaults()
> > > * also after the device is probed. This takes care of
> > > cases
> > > * where the DT is used to setup default parents and
> > > rates
> > > * using assigned-clocks
> > > */
> > > - clk_set_defaults(dev, 1);
> > > + ret = clk_set_defaults(dev, 1);
> > > + if (ret)
> > > + return log_ret(ret);
> > >
> > > return 0;
> > > }
> >
> > Note that this patch broke booting my imx8mn based board on U-Boot
> > v2021.10-rc2. The failure is due to the clock-controller@3038
> > configuration in the imx8mn.dtsi file. I had to remove the
> > following
> > clocks from the device tree to get my device to boot again (all
> > from
> > the assigned-clocks of clock-controller@3038):
> >
> > < IMX8MN_CLK_A53_CORE>,
> > < IMX8MN_CLK_NOC>,
> > < IMX8MN_CLK_AUDIO_AHB>,
> > < IMX8MN_CLK_IPG_AUDIO_ROOT>,
> > < IMX8MN_SYS_PLL3>,
> > < IMX8MN_AUDIO_PLL1>,
> > < IMX8MN_AUDIO_PLL2>;
> >
> > I looked into the clk-imx8mn.c code and I see that we indeed miss
> > clocks there. Unfortunately I could not port code from the Linux
> > kernel: we are missing the imx_clk_hw_mux2 function for the
> > IMX8MN_CLK_A53_CORE clock. I did not look into the other clocks.
>
>
> Perhaps the iMX maintainer could help with this? It does sound like a
> bug.
>
> Regards,
> SImon
>
> >
> >
> > -- Harm
Re: [PATCH v2 15/16] clk: Detect failure to set defaults
Hi Harm,
On Wed, 18 Aug 2021 at 08:09, Harm Berntsen wrote:
>
> On Thu, 2021-05-13 at 19:39 -0600, Simon Glass wrote:
> > When the default clocks cannot be set, the clock is silently probed and
> > the error is ignored. This is incorrect, since having the clocks at the
> > correct speed may be important for operation of the system.
> >
> > Fix it by checking the return code.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > (no changes since v1)
> >
> > drivers/clk/clk-uclass.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > index 4ab3c402ed8..2a2e1cfbd61 100644
> > --- a/drivers/clk/clk-uclass.c
> > +++ b/drivers/clk/clk-uclass.c
> > @@ -796,13 +796,17 @@ void devm_clk_put(struct udevice *dev, struct clk
> > *clk)
> >
> > int clk_uclass_post_probe(struct udevice *dev)
> > {
> > + int ret;
> > +
> > /*
> > * when a clock provider is probed. Call clk_set_defaults()
> > * also after the device is probed. This takes care of cases
> > * where the DT is used to setup default parents and rates
> > * using assigned-clocks
> > */
> > - clk_set_defaults(dev, 1);
> > + ret = clk_set_defaults(dev, 1);
> > + if (ret)
> > + return log_ret(ret);
> >
> > return 0;
> > }
>
> Note that this patch broke booting my imx8mn based board on U-Boot
> v2021.10-rc2. The failure is due to the clock-controller@3038
> configuration in the imx8mn.dtsi file. I had to remove the following
> clocks from the device tree to get my device to boot again (all from
> the assigned-clocks of clock-controller@3038):
>
> < IMX8MN_CLK_A53_CORE>,
> < IMX8MN_CLK_NOC>,
> < IMX8MN_CLK_AUDIO_AHB>,
> < IMX8MN_CLK_IPG_AUDIO_ROOT>,
> < IMX8MN_SYS_PLL3>,
> < IMX8MN_AUDIO_PLL1>,
> < IMX8MN_AUDIO_PLL2>;
>
> I looked into the clk-imx8mn.c code and I see that we indeed miss
> clocks there. Unfortunately I could not port code from the Linux
> kernel: we are missing the imx_clk_hw_mux2 function for the
> IMX8MN_CLK_A53_CORE clock. I did not look into the other clocks.
Perhaps the iMX maintainer could help with this? It does sound like a bug.
Regards,
SImon
>
>
> -- Harm
Re: [PATCH v2 15/16] clk: Detect failure to set defaults
On Thu, 2021-05-13 at 19:39 -0600, Simon Glass wrote:
> When the default clocks cannot be set, the clock is silently probed and
> the error is ignored. This is incorrect, since having the clocks at the
> correct speed may be important for operation of the system.
>
> Fix it by checking the return code.
>
> Signed-off-by: Simon Glass
> ---
>
> (no changes since v1)
>
> drivers/clk/clk-uclass.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 4ab3c402ed8..2a2e1cfbd61 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -796,13 +796,17 @@ void devm_clk_put(struct udevice *dev, struct clk
> *clk)
>
> int clk_uclass_post_probe(struct udevice *dev)
> {
> + int ret;
> +
> /*
> * when a clock provider is probed. Call clk_set_defaults()
> * also after the device is probed. This takes care of cases
> * where the DT is used to setup default parents and rates
> * using assigned-clocks
> */
> - clk_set_defaults(dev, 1);
> + ret = clk_set_defaults(dev, 1);
> + if (ret)
> + return log_ret(ret);
>
> return 0;
> }
Note that this patch broke booting my imx8mn based board on U-Boot
v2021.10-rc2. The failure is due to the clock-controller@3038
configuration in the imx8mn.dtsi file. I had to remove the following
clocks from the device tree to get my device to boot again (all from
the assigned-clocks of clock-controller@3038):
< IMX8MN_CLK_A53_CORE>,
< IMX8MN_CLK_NOC>,
< IMX8MN_CLK_AUDIO_AHB>,
< IMX8MN_CLK_IPG_AUDIO_ROOT>,
< IMX8MN_SYS_PLL3>,
< IMX8MN_AUDIO_PLL1>,
< IMX8MN_AUDIO_PLL2>;
I looked into the clk-imx8mn.c code and I see that we indeed miss
clocks there. Unfortunately I could not port code from the Linux
kernel: we are missing the imx_clk_hw_mux2 function for the
IMX8MN_CLK_A53_CORE clock. I did not look into the other clocks.
-- Harm
Re: [PATCH v2 15/16] clk: Detect failure to set defaults
On Thu, May 13, 2021 at 07:39:31PM -0600, Simon Glass wrote: > When the default clocks cannot be set, the clock is silently probed and > the error is ignored. This is incorrect, since having the clocks at the > correct speed may be important for operation of the system. > > Fix it by checking the return code. > > Signed-off-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH v2 15/16] clk: Detect failure to set defaults
When the default clocks cannot be set, the clock is silently probed and
the error is ignored. This is incorrect, since having the clocks at the
correct speed may be important for operation of the system.
Fix it by checking the return code.
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/clk/clk-uclass.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 4ab3c402ed8..2a2e1cfbd61 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -796,13 +796,17 @@ void devm_clk_put(struct udevice *dev, struct clk *clk)
int clk_uclass_post_probe(struct udevice *dev)
{
+ int ret;
+
/*
* when a clock provider is probed. Call clk_set_defaults()
* also after the device is probed. This takes care of cases
* where the DT is used to setup default parents and rates
* using assigned-clocks
*/
- clk_set_defaults(dev, 1);
+ ret = clk_set_defaults(dev, 1);
+ if (ret)
+ return log_ret(ret);
return 0;
}
--
2.31.1.751.gd2f1c929bd-goog
