Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20221018070653.gsvnacqe7chvzux2@houat>
References: <20221011135548.318323-1-angelogioacchino.delregno@collabora.com> <20221012085555.3nls7ja56vlnaz2w@houat> <c4a1eb9f-016d-c184-e494-c869038b87ff@collabora.com> <20221012094004.jgiyvmbgomiyedik@houat> <6e76f90f-3b6a-330d-6902-b31bf3d33ad4@collabora.com> <20221012114813.6d6adro746w5bd7c@houat> <decfd5e5-a48b-b2d1-f21b-1a4d52e05538@collabora.com> <20221012135619.wxyzuqheolhypoec@houat> <20221014193652.0C745C433D6@smtp.kernel.org> <20221018070653.gsvnacqe7chvzux2@houat>
Subject: Re: [PATCH v2] clk: mediatek: clk-mux: Add .determine_rate() callback
From:   Stephen Boyd <sboyd@kernel.org>
Cc:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>, mturquette@baylibre.com,
        matthias.bgg@gmail.com, chun-jie.chen@mediatek.com,
        miles.chen@mediatek.com, wenst@chromium.org,
        linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
To:     Maxime Ripard <maxime@cerno.tech>
Date:   Thu, 27 Oct 2022 17:30:54 -0700
User-Agent: alot/0.10
Message-Id: <20221028003056.6E83AC433D6@smtp.kernel.org>
Precedence: bulk

Quoting Maxime Ripard (2022-10-18 00:06:53)
>=20
> We spent some time with Angelo yesterday debugging this, and it's still
> not clear to me what happens.
>=20
> He provided a significant amount of traces, and we first checked that
> the round_rate part of set_rate was returning something meaningful, and
> it does. The rate is fine, the parent is too, everything's great.
>=20
> Next we checked the decisions by clk_calc_new_rates, and it does return
> the proper top clock, and its proper rate.
>=20
> Finally, we hooked into clk_change_rate() to see what kind of decision
> it was enforcing, and it seems to be ok as well. It doesn't change
> parent, and it sets the proper rate, in both cases.
>=20
> There's still one thing we haven't checked: one of the clock in the tree
> (the parent of the one we want to change the rate on, and it has
> SET_RATE_PARENT) has a notifier. As we've had a bug recently over this
> I've not ruled out that this could be a similar bug.
>=20
> I don't really think it is though, since the notifier callback doesn't
> use the data provided by the framework:
> https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/clk/mediatek/clk=
-mux.c#L279
>=20
> I've pushed a branch for Angelo to test, just to confirm.
>=20
> So... yeah. I can't explain the regression at this point. Do you have an
> idea?

I don't really know. If the removal of WARN_ON() helps then it sounds
like a console related problem where we hang the system trying to print
the warning to the console. Did you try replacing that case with a
trace_printk()?

>=20
> The good news is, since you merged this patch the regression is
> invisible now to that platform. We still could encounter it on another
> platform, but maybe it will also have a more obvious setup to replicate?
>=20

I guess that's good news :-/