[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08256a88-7165-41ca-b484-4acf1c8e316b@collabora.com>
Date: Wed, 5 Jun 2024 13:25:31 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: Frank Binns <frank.binns@...tec.com>, Matt Coster
<matt.coster@...tec.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
dri-devel@...ts.freedesktop.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] arm64: dts: mediatek: mt8173: Fix MFG_ASYNC power
domain clock
Il 05/06/24 10:25, Chen-Yu Tsai ha scritto:
> On Thu, May 30, 2024 at 6:03 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
>>
>> Il 30/05/24 10:35, Chen-Yu Tsai ha scritto:
>>> The MFG_ASYNC domain, which is likely associated to the whole MFG block,
>>> currently specifies clk26m as its domain clock. This is bogus, since the
>>> clock is an external crystal with no controls. Also, the MFG block has
>>> a independent CLK_TOP_AXI_MFG_IN_SEL clock, which according to the block
>>> diagram, gates access to the hardware registers. Having this one as the
>>> domain clock makes much more sense. This also fixes access to the MFGTOP
>>> registers.
>>>
>>> Change the MFG_ASYNC domain clock to CLK_TOP_AXI_MFG_IN_SEL.
>>>
>>> Fixes: 8b6562644df9 ("arm64: dts: mediatek: Add mt8173 power domain controller")
>>> Signed-off-by: Chen-Yu Tsai <wenst@...omium.org>
>>
>> Just one question... what happens if there's no GPU support at all and this
>> power domain gets powered off?
>>
>> I expect the answer to be "nothing", so I'm preventively giving you my
>
> Well it's powered off by default. Just double checked, and without the final
> patch:
>
> # cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain status children
> performance
> /device runtime status
> ----------------------------------------------------------------------------------------------
> mfg off-0
> 0
> mfg_2d off-0
> 0
> mfg
> mfg_async off-0
> 0
> mfg_2d
>
> And with the last patch but with the powervr removed:
>
> # cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain status children
> performance
> /device runtime status
> ----------------------------------------------------------------------------------------------
> mfg_apm off-0
> 0
> mfg off-0
> 0
> mfg_apm
> /devices/platform/soc/13fff000.clock-controller suspended
> 0
> mfg_2d off-0
> 0
> mfg
> mfg_async off-0
> 0
> mfg_2d
>
> Things seem to work OK. I can SSH in, and the framebuffer console on the screen
> works fine.
>
>
> Note that accessing the regmap through debugfs doesn't do much good. regmap
> doesn't handle runtime PM. And the syscon regmap isn't even tied to a
> struct device. Dumping the regmap through debugfs while the power domain
> is off gives all zeroes, likely due to bus isolation.
>
The last part where you say "gives all zeroes" is actually the best outcome that
I could have ever expected.
So, well, many thanks for this very nice analysis and test.
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
I confirm my green light. It's beautiful when this kind of patches come upstream
especially with your replies actually removing any kind of possible doubt.
>
> Thanks!
Thank *you* for caring about this old platform!
Cheers,
Angelo
>
> ChenYu
>
>> ....but if I'm wrong and the answer isn't exactly "nothing", then I still agree
>> with this commit, but only after removing the Fixes tag.
>>
>> Cheers,
>> Angelo
>>
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> index 3458be7f7f61..136b28f80cc2 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> @@ -497,7 +497,7 @@ power-domain@...173_POWER_DOMAIN_USB {
>>> };
>>> mfg_async: power-domain@...173_POWER_DOMAIN_MFG_ASYNC {
>>> reg = <MT8173_POWER_DOMAIN_MFG_ASYNC>;
>>> - clocks = <&clk26m>;
>>> + clocks = <&topckgen CLK_TOP_AXI_MFG_IN_SEL>;
>>> clock-names = "mfg";
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>
>>
Powered by blists - more mailing lists