Hello Miquel,

On 01.10.24 10:29, Miquel Raynal wrote:
Hi Heiko,


Hmm.. unfortunately ... I had applied your 2 clock patches, which
fixed a problem with enabling parent clocks ... but they broke booting
on a carrier which has fec ethernet! After "Net: " output the board hang...

I reverted your 2 clock patches and it bootet again ... so there is
a problem ... try to get some more time to look into...

We have a fec, but we had I think two patches more on it. I forget to
answer Marek
about them because I don't have my board now and because he is
partially right (or maybe right).
Anyway when we boot we could have and we have clocks that are enabled
by bootrom or SPL that
we need to declare as enable like PLL2/PLL3 those clocks are parts or
could be part of reparent so,
you need to have a reference counter on them that allow to not disable
during the down chain disable
and up chain enable. I think that what happens to your ethernet it's
that you disable some clock that is
critical to the board to survive. I had a patch merged by Tom that

Yep, thats what I think too! If you access registers in a block for
which the clock is not enabled ... it just hang...
prints the clock name so if you enable
the debug of the clock you will find that your board stops working
during one of this reparinting.

I currently work on 2024.07... will rebase if 2024.10 is out...

Ah, you mean:

commit a70d991212c9684e09ed80ece69ce1ff7bfd9f08
Author: Michael Trimarchi <mich...@amarulasolutions.com>
Date:   Tue Jul 9 08:28:13 2024 +0200

      clk: clk-uclass: Print clk name in clk_enable/clk_disable

      Print clk name in clk_enable and clk_disable. Make sense to know
      what clock get disabled/enabled before a system crash or system
      hang.

      Signed-off-by: Michael Trimarchi <mich...@amarulasolutions.com>

I have the same change when I debug :-P

IIRC I did not see the clock names ... but I will recheck!

I see with your patch the clock names, so fine... and see [1]

Hmm... I am on imx8mp, and I think the changes the patchset do in

"clk: imx8mn: Prevent clock critical path from disabling during reparent and 
set_rate"

are in clk-imx8mp already ...

Ported the patch from patchset

"[PATCH 05/26] clk: imx8mm: Mark IMX8MM_SYS_PLL2 and IMX8MM_SYS_PLL3 as enabled"

to imx8mp [2] and fec ethernet works again for me on imx8mp!

Could you add this if you post a v2 ?

TBH I don't feel like the below change is the correct one, it is too
specific. The clock core is recursive and thus should handle the
reparenting situations gracefully.

I posted a series that is targeting the LVDS output on imx8mp. You
should probably consider checking these patches as well if you work
on imx8mp as well. I also had similar breakages with Ethernet which
happened during the assigned-clocks handling. This patch, which is more
future and platform agnostic, fixed it:

https://lore.kernel.org/u-boot/20240910101344.110633-3-miquel.ray...@bootlin.com/

Yes, I also had such a "fix" first, before seeing Darios patchset!

Damn, I did not see your patches...

regarding your patch ... I am unsure which version is the best one ...
I fear of side effects for other plattforms adding this change in
generic "drivers/clk/clk-uclass.c" file...

My series is complimentary, even though there are some overlaps that we
need to merge.

I try to find time to test them on my board!

bye,
Heiko
--
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

Reply via email to