Re: [PATCH v24 0/7] soc: mediatek: SVS: introduce MTK SVS

From: Kevin Hilman <khilman@kernel.org>
To: Roger Lu <roger.lu@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Enric Balletbo Serra <eballetbo@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Nicolas Boichat <drinkcat@google.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Cc: Fan Chen <fan.chen@mediatek.com>,
	HenryC Chen <HenryC.Chen@mediatek.com>,
	Xiaoqing Liu <Xiaoqing.Liu@mediatek.com>,
	Charles Yang <Charles.Yang@mediatek.com>,
	Angus Lin <Angus.Lin@mediatek.com>,
	Mark Rutland <mark.rutland@arm.com>, Nishanth Menon <nm@ti.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com,
	Guenter Roeck <linux@roeck-us.net>,
	Jia-wei Chang <jia-wei.chang@mediatek.com>
Subject: Re: [PATCH v24 0/7] soc: mediatek: SVS: introduce MTK SVS
Date: Thu, 21 Apr 2022 12:41:52 -0700	[thread overview]
Message-ID: <7hsfq6ql4v.fsf@baylibre.com> (raw)
In-Reply-To: <3d463c8b099fdb1c9a0df9e615a8ca1d8a034120.camel@mediatek.com>

Hi Roger,

Roger Lu <roger.lu@mediatek.com> writes:

> On Wed, 2022-04-20 at 16:22 -0700, Kevin Hilman wrote:

[...]

>> That being said, it would be really nice to see an integration tree
>> where this was all tested on mainline (e.g. v5.17, or v5.18-rc)
>> 
>> For example, I can apply this to v5.18-rc2 and boot on my mt8183-pumpkin
>> board, it fails to probe[1] because there is no CCI node in the upstream
>> mt8183.dtsi.
>> 
>> I'm assuming this series is also not very useful without the CPUfreq
>> series from Rex, so being able to test this, CCI and CPUfreq together on
>> MT8183 on a mainline kernel would be very helpful.
>> 
>> Kevin
>> 
>> [1]
>> [    0.573332] mtk-svs 1100b000.svs: cannot find cci node
>> [    0.574061] mtk-svs 1100b000.svs: error -ENODEV: svs platform probe fail
>
> Just share. I've tested this series on below two platforms and it works as
> expected.
> - mt8183-Krane (kernel-v5.10)
> - mt8192-Hayato (kernel-v5.4)

Unfortunately testing on v5.4 and v5.10 with lots of other additional
out-of-tree patches does not give much confidence that this series works
with upstream, especially when I've given a few reasons why it will not
work uptream.

The examples I gave above for CCI and CPUs/cluster disable are good
examples, but another one I forgot to mention is the dependency on Mali.
The SVS driver will never probe because it also depens on a "mali" node,
which doesn't exist upstream either (but panfrost does, and acutually
loads/probes fine on v5.17/v5.18) so this should be fixed to work with
upstream panfrost.

IMO, in order for this to be merged upstream, it should at least have
some basic validation with upstream, and so far I have not even been
able to make it successfuly probe.  To do that, you will need to either
provide a list of the dependencies for testing this with mainline
(e.g. CCI series, CPUfreq series, any DT changes), or even better, an
integration tree based on recent mainline (e.g. v5.17 stable, or
v5.18-rc) which shows all the patches (in addition to this series) used
to validate this on mainline.

Thanks,

Kevin

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek