All of lore.kernel.org
 help / color / mirror / Atom feed
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.

WARNING: multiple messages have this Message-ID (diff)
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-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
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

  parent reply	other threads:[~2022-05-20 19:58 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-18 11:16 [PATCH v2 0/7] MediaTek Helio X10 MT6795 - Clock drivers AngeloGioacchino Del Regno
2022-05-18 11:16 ` AngeloGioacchino Del Regno
2022-05-18 11:16 ` AngeloGioacchino Del Regno
2022-05-18 11:16 ` [PATCH v2 1/7] dt-bindings: mediatek: Document MT6795 system controllers bindings AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16 ` [PATCH v2 2/7] dt-bindings: clock: Add MediaTek Helio X10 MT6795 clock bindings AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-06-01 20:02   ` Rob Herring
2022-06-01 20:02     ` Rob Herring
2022-06-01 20:02     ` Rob Herring
2022-05-18 11:16 ` [PATCH v2 3/7] dt-bindings: reset: Add bindings for MT6795 Helio X10 reset controllers AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-06-01 20:02   ` Rob Herring
2022-06-01 20:02     ` Rob Herring
2022-06-01 20:02     ` Rob Herring
2022-05-18 11:16 ` [PATCH v2 4/7] dt-bindings: clock: mediatek: Add clock driver bindings for MT6795 AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 13:46   ` Rob Herring
2022-05-18 13:46     ` Rob Herring
2022-05-18 13:46     ` Rob Herring
2022-05-18 11:16 ` [PATCH v2 5/7] clk: mediatek: clk-apmixed: Remove unneeded __init annotation AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16 ` [PATCH v2 6/7] clk: mediatek: Export required symbols to compile clk drivers as module AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-19  4:41   ` Miles Chen
2022-05-19  4:41     ` Miles Chen
2022-05-19  4:41     ` Miles Chen
2022-05-19  8:05     ` AngeloGioacchino Del Regno
2022-05-19  8:05       ` AngeloGioacchino Del Regno
2022-05-19  8:05       ` AngeloGioacchino Del Regno
2022-05-19  8:15       ` Chen-Yu Tsai
2022-05-19  8:15         ` Chen-Yu Tsai
2022-05-19  8:15         ` Chen-Yu Tsai
2022-05-19  8:26         ` AngeloGioacchino Del Regno
2022-05-19  8:26           ` AngeloGioacchino Del Regno
2022-05-19  8:26           ` AngeloGioacchino Del Regno
2022-05-19  8:45           ` Chen-Yu Tsai
2022-05-19  8:45             ` Chen-Yu Tsai
2022-05-19  8:45             ` Chen-Yu Tsai
2022-05-19  8:53             ` AngeloGioacchino Del Regno
2022-05-19  8:53               ` AngeloGioacchino Del Regno
2022-05-19  8:53               ` AngeloGioacchino Del Regno
2022-05-18 11:16 ` [PATCH v2 7/7] clk: mediatek: Add MediaTek Helio X10 MT6795 clock drivers AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:16   ` AngeloGioacchino Del Regno
2022-05-18 11:23   ` Matthias Brugger
2022-05-18 11:23     ` Matthias Brugger
2022-05-18 11:23     ` Matthias Brugger
2022-05-19  4:53   ` Miles Chen
2022-05-19  4:53     ` Miles Chen
2022-05-19  4:53     ` Miles Chen
2022-05-19  8:11     ` Chen-Yu Tsai
2022-05-19  8:11       ` Chen-Yu Tsai
2022-05-19  8:11       ` Chen-Yu Tsai
2022-05-19  8:17     ` AngeloGioacchino Del Regno
2022-05-19  8:17       ` AngeloGioacchino Del Regno
2022-05-19  8:17       ` AngeloGioacchino Del Regno
2022-05-19  8:37       ` Chen-Yu Tsai
2022-05-19  8:37         ` Chen-Yu Tsai
2022-05-19  8:37         ` Chen-Yu Tsai
2022-05-19  8:49         ` AngeloGioacchino Del Regno
2022-05-19  8:49           ` AngeloGioacchino Del Regno
2022-05-19  8:49           ` AngeloGioacchino Del Regno
2022-05-20  4:54           ` Miles Chen
2022-05-20  4:54             ` Miles Chen
2022-05-20  4:54             ` Miles Chen
2022-05-20 19:58           ` Boris Lysov [this message]
2022-05-20 19:58             ` Boris Lysov
2022-05-20 19:58             ` Boris Lysov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220520225803.193bcda9@pc \
    --to=arz65xx@gmail.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=chun-jie.chen@mediatek.com \
    --cc=ck.hu@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=fparent@baylibre.com \
    --cc=ikjn@chromium.org \
    --cc=jason-jh.lin@mediatek.com \
    --cc=kernel@collabora.com \
    --cc=konrad.dybcio@somainline.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=marijn.suijten@somainline.org \
    --cc=martin.botka@somainline.org \
    --cc=matthias.bgg@gmail.com \
    --cc=miles.chen@mediatek.com \
    --cc=mturquette@baylibre.com \
    --cc=p.zabel@pengutronix.de \
    --cc=paul.bouchara@somainline.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=rex-bc.chen@mediatek.com \
    --cc=robh+dt@kernel.org \
    --cc=sam.shih@mediatek.com \
    --cc=sboyd@kernel.org \
    --cc=tinghan.shen@mediatek.com \
    --cc=weiyi.lu@mediatek.com \
    --cc=wenst@chromium.org \
    --cc=y.oudjana@protonmail.com \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.