Re: [PATCH v2 7/7] clk: mediatek: Add MediaTek Helio X10 MT6795 clock drivers

From: Boris Lysov <arz65xx@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Chen-Yu Tsai <wenst@chromium.org>,
	Miles Chen <miles.chen@mediatek.com>,
	bgolaszewski@baylibre.com, chun-jie.chen@mediatek.com,
	ck.hu@mediatek.com, devicetree@vger.kernel.org,
	fparent@baylibre.com, ikjn@chromium.org,
	jason-jh.lin@mediatek.com, kernel@collabora.com,
	konrad.dybcio@somainline.org, krzysztof.kozlowski+dt@linaro.org,
	linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	marijn.suijten@somainline.org, martin.botka@somainline.org,
	matthias.bgg@gmail.com, mturquette@baylibre.com,
	p.zabel@pengutronix.de, paul.bouchara@somainline.org,
	phone-devel@vger.kernel.org, rex-bc.chen@mediatek.com,
	robh+dt@kernel.org, sam.shih@mediatek.com, sboyd@kernel.org,
	tinghan.shen@mediatek.com, weiyi.lu@mediatek.com,
	y.oudjana@protonmail.com, ~postmarketos/upstreaming@lists.sr.ht
Subject: Re: [PATCH v2 7/7] clk: mediatek: Add MediaTek Helio X10 MT6795 clock drivers
Date: Fri, 20 May 2022 22:58:03 +0300	[thread overview]
Message-ID: <20220520225803.193bcda9@pc> (raw)
In-Reply-To: <a253f338-c162-17b8-f412-95cd63195151@collabora.com>

Hello, I'd like to chime in with some feedback.

> >> Hence, the usecases for this kind of splitting are:
> >> 1. Somewhat rare (corner) cases: someone may not want to compile in any of
> >> the mm/venc/vdec/mfg clock drivers because they don't need the
> >> functionality at all (probably, including the other related drivers), or;
> >> 2. It would be possible to compile as built-in only the "main" drivers
> >> (apmixed, infra, peri, topck) to achieve a boot (ex.: you need eMMC to
> >> boot, at least) and then compile the mm/venc/vdec/mfg as modules to be
> >> loaded after mounting a rootfs (where you probably also have mediatek-drm,
> >> vcodec, etc as modules).
> > 
> > I assume you mean split them into two groups:
> > 
> >    - essential for booting to a state capable of loading modules from
> > storage So apmixedsys + topckgen + infra_ao + peri_ao + imp_iic_wrap
> > (maybe?)
> >    - everything else
> { snip }
> > IMO having two Kconfig symbols for one chip is still much better than
> > having ten though.
This sounds good.

I think it would've been even better if selecting a Kconfig option like
MACH_MT6795 would automatically select the base clock driver for booting to a
state capable of loading modules from storage. But a quick check showed me that
arm64 doesn't use such an approach unlike arm.

> For MT8195... and 92, 83, 73... and others from the same era, being them for
> chromebooks, iot, smartphones and whatever else... yeah you're totally right.
> 
> The issue starts raising when looking at older SoCs featuring an older
> bootloader that does have a kernel size limitation; for example, to make the
> loader happy on MT6795, I had to strip the defconfig a lot and keep the
> Android-style boot.img smaller than 10MB (that's Image.gz-dtb + ramdisk).
This issue gets even more relevant if/when we consider older ARM32 SoCs such as
mt65xx series. As far as I know, most of them (with a notable exception of
mt6580) have bootloaders that restrict max boot.img size to 6144 kB.

However, I think too much granularity in Kconfig might cause unnecessary
confusion. The "essential clock infra" + "everything else" split sounds better
to me.

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