[PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[Mogens Lauridsen]

PLL3 was first set to 400MHz and then some peripheral was switched
to PLL3. Finally PLL3 was set to 216MHz. This could make some
i.MX538 hang in a dead loop in the boot process.

Signed-off-by: Mogens Lauridsen <mlauridsen171@gmail.com>
---
 arch/arm/mach-imx/imx53.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/imx53.c b/arch/arm/mach-imx/imx53.c
index 56f1bda75..3fdb3b91a 100644
--- a/arch/arm/mach-imx/imx53.c
+++ b/arch/arm/mach-imx/imx53.c
@@ -119,7 +119,7 @@ void imx53_init_lowlevel_early(unsigned int cpufreq_mhz)
 	else
 		imx5_setup_pll_800((void __iomem *)MX53_PLL1_BASE_ADDR);
 
-	imx5_setup_pll_400((void __iomem *)MX53_PLL3_BASE_ADDR);
+	imx5_setup_pll_216((void __iomem *)MX53_PLL3_BASE_ADDR);
 
         /* Switch peripheral to PLL3 */
 	writel(0x00015154, ccm + MX5_CCM_CBCMR);
@@ -154,7 +154,6 @@ void imx53_init_lowlevel_early(unsigned int cpufreq_mhz)
 	/* make sure change is effective */
 	while (readl(ccm + MX5_CCM_CDHIPR));
 
-	imx5_setup_pll_216((void __iomem *)MX53_PLL3_BASE_ADDR);
 	imx5_setup_pll_455((void __iomem *)MX53_PLL4_BASE_ADDR);
 
 	/* Set the platform clock dividers */
-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[Sascha Hauer]

On Wed, Jun 27, 2018 at 04:07:11PM +0200, Mogens Lauridsen wrote:
> PLL3 was first set to 400MHz and then some peripheral was switched
> to PLL3. Finally PLL3 was set to 216MHz. This could make some
> i.MX538 hang in a dead loop in the boot process.

Let's see what the code currently does:

By reset default the clock path to the DDR is:

PLL2 (192MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/1 = 192MHz) -> ddr_clk_root

PLL2 is running at 192MHz. The code now tries to switch PLL2 from 192MHz
to 400MHz. This requires that the RAM is driven by some other clock
during
the PLL reconfiguration. The code switches the clock path to:

PLL3 (400MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/2 = 200MHz) -> ddr_clk_root

Then PLL2 is reconfigured to 400MHz and the DDR clock path is switched
back to the original path, this time with the PLL runnning at 400MHz, so
RAM is then running at the desired speed.

The code configures PLL3 to 400MHz probably to keep the DDR frequency
nearly constant during the transition.

I have no idea why your change helps you. When I understand correctly
then you are running nearly at half the frequency during the transition
(400/216).

I'm afraid to merge such change as long as we do not fully understand
what we are doing and why it helps.

BTW the current code is the same as on U-Boot which is derived from the
original Freescale code, so this is not exactly new or barebox specific.

Do you have anything special in your dcd table that influences the
clocks in an unexpected way?

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[Mogens Lauridsen]

On Thu, Jun 28, 2018 at 10:30 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Jun 27, 2018 at 04:07:11PM +0200, Mogens Lauridsen wrote:
>> PLL3 was first set to 400MHz and then some peripheral was switched
>> to PLL3. Finally PLL3 was set to 216MHz. This could make some
>> i.MX538 hang in a dead loop in the boot process.
>
> Let's see what the code currently does:
>
> By reset default the clock path to the DDR is:
>
> PLL2 (192MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/1 = 192MHz) -> ddr_clk_root
>
> PLL2 is running at 192MHz. The code now tries to switch PLL2 from 192MHz
> to 400MHz. This requires that the RAM is driven by some other clock
> during
> the PLL reconfiguration. The code switches the clock path to:
>
> PLL3 (400MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/2 = 200MHz) -> ddr_clk_root
>
> Then PLL2 is reconfigured to 400MHz and the DDR clock path is switched
> back to the original path, this time with the PLL runnning at 400MHz, so
> RAM is then running at the desired speed.
>
> The code configures PLL3 to 400MHz probably to keep the DDR frequency
> nearly constant during the transition.
>
> I have no idea why your change helps you. When I understand correctly
> then you are running nearly at half the frequency during the transition
> (400/216).
>
> I'm afraid to merge such change as long as we do not fully understand
> what we are doing and why it helps.
I see your point.
I don't know exactly why it helps. But to me it looks suspicious to
set a clock to 400MHz
which in the end only is suppose to run at 216 MHz. I fear that there are small
differences in the individual iMX538 and in some of them the pll might
not always be able
to get a lock at the high frequency. The problem seem to have some
relationship with the temperature of the chip. (Higher temperature also seem
to solve the problem).
>
> BTW the current code is the same as on U-Boot which is derived from the
> original Freescale code, so this is not exactly new or barebox specific.
Yes, I did see that the code has been in barebox since 2011. I am also
puzzled why we see this, and nobody else has seen/reported it. We
are using the i.MX538 which is in a pop package with the DDR mounted
on the top of the i.MX. This chip might not be widely used.
>
> Do you have anything special in your dcd table that influences the
> clocks in an unexpected way?
Not as far as I know. Could you give me a hint of what I should look for?

Best regards,
Mogens

>
> Sascha
>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[Sascha Hauer]

On Thu, Jun 28, 2018 at 03:56:55PM +0200, Mogens Lauridsen wrote:
> On Thu, Jun 28, 2018 at 10:30 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, Jun 27, 2018 at 04:07:11PM +0200, Mogens Lauridsen wrote:
> >> PLL3 was first set to 400MHz and then some peripheral was switched
> >> to PLL3. Finally PLL3 was set to 216MHz. This could make some
> >> i.MX538 hang in a dead loop in the boot process.
> >
> > Let's see what the code currently does:
> >
> > By reset default the clock path to the DDR is:
> >
> > PLL2 (192MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/1 = 192MHz) -> ddr_clk_root
> >
> > PLL2 is running at 192MHz. The code now tries to switch PLL2 from 192MHz
> > to 400MHz. This requires that the RAM is driven by some other clock
> > during
> > the PLL reconfiguration. The code switches the clock path to:
> >
> > PLL3 (400MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/2 = 200MHz) -> ddr_clk_root
> >
> > Then PLL2 is reconfigured to 400MHz and the DDR clock path is switched
> > back to the original path, this time with the PLL runnning at 400MHz, so
> > RAM is then running at the desired speed.
> >
> > The code configures PLL3 to 400MHz probably to keep the DDR frequency
> > nearly constant during the transition.
> >
> > I have no idea why your change helps you. When I understand correctly
> > then you are running nearly at half the frequency during the transition
> > (400/216).
> >
> > I'm afraid to merge such change as long as we do not fully understand
> > what we are doing and why it helps.
> I see your point.
> I don't know exactly why it helps. But to me it looks suspicious to
> set a clock to 400MHz
> which in the end only is suppose to run at 216 MHz. I fear that there are small
> differences in the individual iMX538 and in some of them the pll might
> not always be able
> to get a lock at the high frequency.

You should be able to test this. In this case the code should hang in
the loop waiting for the PLL lock. Is that the case?

> The problem seem to have some
> relationship with the temperature of the chip. (Higher temperature also seem
> to solve the problem).
> >
> > BTW the current code is the same as on U-Boot which is derived from the
> > original Freescale code, so this is not exactly new or barebox specific.
> Yes, I did see that the code has been in barebox since 2011. I am also
> puzzled why we see this, and nobody else has seen/reported it. We
> are using the i.MX538 which is in a pop package with the DDR mounted
> on the top of the i.MX. This chip might not be widely used.
> >
> > Do you have anything special in your dcd table that influences the
> > clocks in an unexpected way?
> Not as far as I know. Could you give me a hint of what I should look for?

Writes to the CCM register space (0x53fd4xxx) in the imxcfg file for
your board.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[Mogens Lauridsen]

On Fri, Jun 29, 2018 at 10:10 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jun 28, 2018 at 03:56:55PM +0200, Mogens Lauridsen wrote:
>> On Thu, Jun 28, 2018 at 10:30 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Wed, Jun 27, 2018 at 04:07:11PM +0200, Mogens Lauridsen wrote:
>> >> PLL3 was first set to 400MHz and then some peripheral was switched
>> >> to PLL3. Finally PLL3 was set to 216MHz. This could make some
>> >> i.MX538 hang in a dead loop in the boot process.
>> >
>> > Let's see what the code currently does:
>> >
>> > By reset default the clock path to the DDR is:
>> >
>> > PLL2 (192MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/1 = 192MHz) -> ddr_clk_root
>> >
>> > PLL2 is running at 192MHz. The code now tries to switch PLL2 from 192MHz
>> > to 400MHz. This requires that the RAM is driven by some other clock
>> > during
>> > the PLL reconfiguration. The code switches the clock path to:
>> >
>> > PLL3 (400MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/2 = 200MHz) -> ddr_clk_root
>> >
>> > Then PLL2 is reconfigured to 400MHz and the DDR clock path is switched
>> > back to the original path, this time with the PLL runnning at 400MHz, so
>> > RAM is then running at the desired speed.
>> >
>> > The code configures PLL3 to 400MHz probably to keep the DDR frequency
>> > nearly constant during the transition.
>> >
>> > I have no idea why your change helps you. When I understand correctly
>> > then you are running nearly at half the frequency during the transition
>> > (400/216).
>> >
>> > I'm afraid to merge such change as long as we do not fully understand
>> > what we are doing and why it helps.
>> I see your point.
>> I don't know exactly why it helps. But to me it looks suspicious to
>> set a clock to 400MHz
>> which in the end only is suppose to run at 216 MHz. I fear that there are small
>> differences in the individual iMX538 and in some of them the pll might
>> not always be able
>> to get a lock at the high frequency.
>
> You should be able to test this. In this case the code should hang in
> the loop waiting for the PLL lock. Is that the case?
Before changing the pll setup code I did some testing (toggle LEDs on gpio)
to find where the cpu was hanging. I have verified this again, and it is stuck
in the second "while (readl(ccm + MX5_CCM_CDHIPR));" in
"imx53_init_lowlevel_early".

>> The problem seem to have some
>> relationship with the temperature of the chip. (Higher temperature also seem
>> to solve the problem).
>> >
>> > BTW the current code is the same as on U-Boot which is derived from the
>> > original Freescale code, so this is not exactly new or barebox specific.
>> Yes, I did see that the code has been in barebox since 2011. I am also
>> puzzled why we see this, and nobody else has seen/reported it. We
>> are using the i.MX538 which is in a pop package with the DDR mounted
>> on the top of the i.MX. This chip might not be widely used.
>> >
>> > Do you have anything special in your dcd table that influences the
>> > clocks in an unexpected way?
>> Not as far as I know. Could you give me a hint of what I should look for?
>
> Writes to the CCM register space (0x53fd4xxx) in the imxcfg file for
> your board.
Our imxcfg file doesn't contain any write 0x53fd4xxx.

Best regards
Mogens

>
> Sascha
>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX53: Set pll3 directly to 216MHz.

[Sascha Hauer]

On Mon, Jul 02, 2018 at 11:56:54AM +0200, Mogens Lauridsen wrote:
> On Fri, Jun 29, 2018 at 10:10 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Thu, Jun 28, 2018 at 03:56:55PM +0200, Mogens Lauridsen wrote:
> >> On Thu, Jun 28, 2018 at 10:30 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >> > On Wed, Jun 27, 2018 at 04:07:11PM +0200, Mogens Lauridsen wrote:
> >> >> PLL3 was first set to 400MHz and then some peripheral was switched
> >> >> to PLL3. Finally PLL3 was set to 216MHz. This could make some
> >> >> i.MX538 hang in a dead loop in the boot process.
> >> >
> >> > Let's see what the code currently does:
> >> >
> >> > By reset default the clock path to the DDR is:
> >> >
> >> > PLL2 (192MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/1 = 192MHz) -> ddr_clk_root
> >> >
> >> > PLL2 is running at 192MHz. The code now tries to switch PLL2 from 192MHz
> >> > to 400MHz. This requires that the RAM is driven by some other clock
> >> > during
> >> > the PLL reconfiguration. The code switches the clock path to:
> >> >
> >> > PLL3 (400MHz) -> periph_clk -> main_bus_clk -> axi_a_podf -> axi_a (/2 = 200MHz) -> ddr_clk_root
> >> >
> >> > Then PLL2 is reconfigured to 400MHz and the DDR clock path is switched
> >> > back to the original path, this time with the PLL runnning at 400MHz, so
> >> > RAM is then running at the desired speed.
> >> >
> >> > The code configures PLL3 to 400MHz probably to keep the DDR frequency
> >> > nearly constant during the transition.
> >> >
> >> > I have no idea why your change helps you. When I understand correctly
> >> > then you are running nearly at half the frequency during the transition
> >> > (400/216).
> >> >
> >> > I'm afraid to merge such change as long as we do not fully understand
> >> > what we are doing and why it helps.
> >> I see your point.
> >> I don't know exactly why it helps. But to me it looks suspicious to
> >> set a clock to 400MHz
> >> which in the end only is suppose to run at 216 MHz. I fear that there are small
> >> differences in the individual iMX538 and in some of them the pll might
> >> not always be able
> >> to get a lock at the high frequency.
> >
> > You should be able to test this. In this case the code should hang in
> > the loop waiting for the PLL lock. Is that the case?
> Before changing the pll setup code I did some testing (toggle LEDs on gpio)
> to find where the cpu was hanging. I have verified this again, and it is stuck
> in the second "while (readl(ccm + MX5_CCM_CDHIPR));" in
> "imx53_init_lowlevel_early".

This indeed suggests that the PLL doesn't get a lock :(

I wonder why the current code switches PLL3 to 400MHz and at the same
time increases the axi_a divider from 1 to 2. Instead, we could program
PLL3 to 192MHz, so the same frequency as PLL2 which we already knows the
system runs with, without increasing the divider.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox