[PATCH 08/26] power: Add iMX8M block ctrl driver for dispmix

[Michael Nazzareno Trimarchi]

Hi Miguel

On Tue, Oct 1, 2024 at 10:29 AM Miquel Raynal <miquel.raynal at bootlin.com> 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 <michael at 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 <michael at 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.raynal@bootlin.com/
>

The clock patches are not specific at all. You need to have it working
for the parent for each component.
This is a standard way to do it and nothing magic compared to other
implementations. I don't see
in your series any addressing or reparent clock or take in account
that a clock should be enable before
reparenting.

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

The only collision I can see from your series is that you re-write the
imx approach of power domain.
Can you please expand here a bit your point?

Michael

>
> Thanks,
> Miquèl



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com