Re: [PATCH v2 3/3] clk: Drop the rate range on clk_put

From: Maxime Ripard <maxime@cerno.tech>
To: Tony Lindgren <tony@atomide.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Mike Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-clk@vger.kernel.org,
	Dmitry Osipenko <dmitry.osipenko@collabora.com>,
	'Linux Samsung SOC' <linux-samsung-soc@vger.kernel.org>,
	linux-amlogic@lists.infradead.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 3/3] clk: Drop the rate range on clk_put
Date: Thu, 31 Mar 2022 11:54:56 +0200	[thread overview]
Message-ID: <20220331095456.dyyxsiu2b3yw2vvs@houat> (raw)
In-Reply-To: <YkV3ch7R7YxlATW+@atomide.com>

Hi,

On Thu, Mar 31, 2022 at 12:42:10PM +0300, Tony Lindgren wrote:
> * Maxime Ripard <maxime@cerno.tech> [700101 02:00]:
> > Hi Marek,
> > 
> > On Wed, Mar 30, 2022 at 10:06:13AM +0200, Marek Szyprowski wrote:
> > > On 25.03.2022 17:11, Maxime Ripard wrote:
> > > > While the current code will trigger a new clk_set_rate call whenever the
> > > > rate boundaries are changed through clk_set_rate_range, this doesn't
> > > > occur when clk_put() is called.
> > > >
> > > > However, this is essentially equivalent since, after clk_put()
> > > > completes, those boundaries won't be enforced anymore.
> > > >
> > > > Let's add a call to clk_set_rate_range in clk_put to make sure those
> > > > rate boundaries are dropped and the clock drivers can react.
> > > >
> > > > Let's also add a few tests to make sure this case is covered.
> > > >
> > > > Fixes: c80ac50cbb37 ("clk: Always set the rate on clk_set_range_rate")
> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > 
> > > This patch landed recently in linux-next 20220328 as commit 7dabfa2bc480 
> > > ("clk: Drop the rate range on clk_put()"). Sadly it breaks booting of 
> > > the few of my test systems: Samsung ARM 32bit Exynos3250 based Rinato 
> > > board and all Amlogic Meson G12B/SM1 based boards (Odroid C4, N2, Khadas 
> > > VIM3/VIM3l). Rinato hangs always with the following oops:
> > > 
> > > --->8---
> > > 
> > > Kernel panic - not syncing: MCT hangs after writing 4 (offset:0x420)
> > > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.17.0-rc1-00014-g7dabfa2bc480 
> > > #11551
> > > Hardware name: Samsung Exynos (Flattened Device Tree)
> > >   unwind_backtrace from show_stack+0x10/0x14
> > >   show_stack from dump_stack_lvl+0x58/0x70
> > >   dump_stack_lvl from panic+0x10c/0x328
> > >   panic from exynos4_mct_tick_stop+0x0/0x2c
> > > ---[ end Kernel panic - not syncing: MCT hangs after writing 4 
> > > (offset:0x420) ]---
> > > 
> > > --->8---
> > > 
> > > Amlogic boards hang randomly during early userspace init, usually just 
> > > after loading the driver modules.
> > > 
> > > Reverting $subject on top of linux-next fixes all those problems.
> > > 
> > > I will try to analyze it a bit more and if possible provide some more 
> > > useful/meaning full logs later.
> > 
> > I'm not sure what could go wrong there, but if you can figure out the
> > clock, if it tries to set a new rate and what rate it is, it would be
> > awesome :)
> 
> I'm also seeing clockevent break on omaps as a wrong source clock gets
> picked.
> 
> It seems the dts assigned-clock-parents no longer works now?

That would make some kind of sense, __set_clk_parents calls clk_put on
both the assigned clock and its parent.

Could you see what parent (and why?) it tries to enforce then?

It looks like the gpt1_fck driver might favor another parent for that
rate, which, if it's an invalid configuration, shouldn't really happen?

Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel