Received-SPF: pass (google.com: domain of linux-kernel+bounces-86218-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240223091122.2430037-1-wenst@chromium.org> <feab2039-5745-41a4-87c3-44fa266efc1d@collabora.com>
In-Reply-To: <feab2039-5745-41a4-87c3-44fa266efc1d@collabora.com>
From: Chen-Yu Tsai <wenst@chromium.org>
Date: Thu, 29 Feb 2024 15:13:34 +0800
Message-ID: <CAGXv+5GpTJzMHxY+J6PshW6-xLfc5G3DTuAG4d9eCc+=WWRd9A@mail.gmail.com>
Subject: Re: [PATCH] arm64: dts: mediatek: mt8183: Add power-domains properity
 to mfgcfg
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>, Ikjoon Jang <ikjn@chromium.org>, 
	devicetree@vger.kernel.org, linux-mediatek@lists.infradead.org, 
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, 
	Weiyi Lu <weiyi.lu@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 26, 2024 at 4:54=E2=80=AFPM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 23/02/24 10:11, Chen-Yu Tsai ha scritto:
> > From: Ikjoon Jang <ikjn@chromium.org>
> >
> > mfgcfg clock is under MFG_ASYNC power domain.
> >
> > Fixes: e526c9bc11f8 ("arm64: dts: Add Mediatek SoC MT8183 and evaluatio=
n board dts and Makefile")
> > Fixes: 37fb78b9aeb7 ("arm64: dts: mediatek: Add mt8183 power domains co=
ntroller")
> > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
> > Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > This patch is long overdue. Could we merge it for fixes for this or the
> > next release?
> >
>
> A clock controller that needs a power domain? Can you please describe the=
 issue
> that you're trying to solve with this?

Our issue tracker doesn't have much on this. But this was part of the initi=
al
submission for MT8183, but somehow got left out. The clk driver patch to
enable runtime PM was even merged, but was effectively reverted following
the clk driver rework.

> It's not very uncommon but I'm not entirely convinced that this is right,=
 because
> the MFG_BG3D is a gate - and it's *not* outputting a clock rate on its ow=
n: the
> mfgcfg is entirely GPU related and if there is no GPU support this clock =
is not
> even ever needed.

It could be just a gate, or could be a buffer. Either way, the registers to
control said device could be in a power domain that is not always on. I don=
't
think this can be ruled out.

And if the registers are in a powered down domain, then either the readout
would be bogus (if access isolation is implemented) or just hang the system=
.

ChenYu

> MediaTek, can you please clarify if (and why) this gate clock needs a MTC=
MOS to
> be ungated?
>
> Thanks,
> Angelo
>
> > Changes since v2:
> > - Rebased onto current tree
> > - Added Fixes tags
> > - Fix up subject prefix
> >
> >   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot=
/dts/mediatek/mt8183.dtsi
> > index 93dfbf130231..774ae5d9143f 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -1637,6 +1637,7 @@ mfgcfg: syscon@13000000 {
> >                       compatible =3D "mediatek,mt8183-mfgcfg", "syscon"=
;
> >                       reg =3D <0 0x13000000 0 0x1000>;
> >                       #clock-cells =3D <1>;
> > +                     power-domains =3D <&spm MT8183_POWER_DOMAIN_MFG_A=
SYNC>;
> >               };
> >
> >               gpu: gpu@13040000 {
>
>