| Message ID | 20250311-wip-obbardc-qcom-defconfig-interconnects-builtin-v1-1-675b6bc57176@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: defconfig: Enable Qualcomm interconnects as built-in | expand |
I sent this patch to start the discussion, some things I found: 1) Some interconnects are missing from arm defconfig. Should they be =y too ? $ grep CONFIG_INTERCONNECT_QCOM arch/arm/configs/qcom_defconfig CONFIG_INTERCONNECT_QCOM=y CONFIG_INTERCONNECT_QCOM_MSM8974=m CONFIG_INTERCONNECT_QCOM_SDX55=m 2) Some interconnects are missing from arm64 defconfig (which should probably be in there) (I have included just two examples): $ grep CONFIG_INTERCONNECT drivers/interconnect/qcom/Makefile obj-$(CONFIG_INTERCONNECT_QCOM_QCS615) += qnoc-qcs615.o obj-$(CONFIG_INTERCONNECT_QCOM_SM7150) += qnoc-sm7150.o I can handle these in follow-up or v2 of the patchset as follow-up commits, please let me know what you'd prefer. On Tue, 11 Mar 2025 at 19:03, Christopher Obbard <christopher.obbard@linaro.org> wrote: > > Currently some Qualcomm interconnect drivers are enabled > as modules which isn't overly useful since the interconnects > are required to be loaded during early boot. > > Loading the interconnects late (e.g. in initrd or as module) > can cause boot issues, such as slowdown or even not booting > at all (since the interconnect would be required for storage > devices). > > Be consistent and enable all of the Qualcomm interconnect > drivers as built-in to the kernel image. > > Signed-off-by: Christopher Obbard <christopher.obbard@linaro.org> > --- > arch/arm64/configs/defconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > index 219ef05ee5a757c43a37ec9f8571ce9976354830..6582baee2ab02ecb2ff442c6e73aa6a23fee8d7f 100644 > --- a/arch/arm64/configs/defconfig > +++ b/arch/arm64/configs/defconfig > @@ -1656,11 +1656,11 @@ CONFIG_INTERCONNECT_IMX8MN=m > CONFIG_INTERCONNECT_IMX8MQ=m > CONFIG_INTERCONNECT_IMX8MP=y > CONFIG_INTERCONNECT_QCOM=y > -CONFIG_INTERCONNECT_QCOM_MSM8916=m > +CONFIG_INTERCONNECT_QCOM_MSM8916=y > CONFIG_INTERCONNECT_QCOM_MSM8996=y > -CONFIG_INTERCONNECT_QCOM_OSM_L3=m > +CONFIG_INTERCONNECT_QCOM_OSM_L3=y > CONFIG_INTERCONNECT_QCOM_QCM2290=y > -CONFIG_INTERCONNECT_QCOM_QCS404=m > +CONFIG_INTERCONNECT_QCOM_QCS404=y > CONFIG_INTERCONNECT_QCOM_QCS615=y > CONFIG_INTERCONNECT_QCOM_QCS8300=y > CONFIG_INTERCONNECT_QCOM_QDU1000=y > > --- > base-commit: b098bcd8278b89cb3eb73fdb6e06dc49af75ad37 > change-id: 20250311-wip-obbardc-qcom-defconfig-interconnects-builtin-258fcc961b11 > > Best regards, > -- > Christopher Obbard <christopher.obbard@linaro.org> >
On Tue, Mar 11, 2025 at 07:02:56PM +0100, Christopher Obbard wrote: > Currently some Qualcomm interconnect drivers are enabled > as modules which isn't overly useful since the interconnects > are required to be loaded during early boot. > > Loading the interconnects late (e.g. in initrd or as module) > can cause boot issues, such as slowdown or even not booting > at all (since the interconnect would be required for storage > devices). This is not a good justification. It should be perfectly fine to load block drivers (including their dependencies) from the initramfs. Up to now we have been enabling only those interconnect (and pinctrl, btw) drivers, which are required to be able to open the consoe (thanks, systemd). > > Be consistent and enable all of the Qualcomm interconnect > drivers as built-in to the kernel image. > > Signed-off-by: Christopher Obbard <christopher.obbard@linaro.org> > --- > arch/arm64/configs/defconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > index 219ef05ee5a757c43a37ec9f8571ce9976354830..6582baee2ab02ecb2ff442c6e73aa6a23fee8d7f 100644 > --- a/arch/arm64/configs/defconfig > +++ b/arch/arm64/configs/defconfig > @@ -1656,11 +1656,11 @@ CONFIG_INTERCONNECT_IMX8MN=m > CONFIG_INTERCONNECT_IMX8MQ=m > CONFIG_INTERCONNECT_IMX8MP=y > CONFIG_INTERCONNECT_QCOM=y > -CONFIG_INTERCONNECT_QCOM_MSM8916=m > +CONFIG_INTERCONNECT_QCOM_MSM8916=y > CONFIG_INTERCONNECT_QCOM_MSM8996=y > -CONFIG_INTERCONNECT_QCOM_OSM_L3=m > +CONFIG_INTERCONNECT_QCOM_OSM_L3=y This is the memory / L3 / cpufreq interconnect, it has nothing to do with the block devices. > CONFIG_INTERCONNECT_QCOM_QCM2290=y > -CONFIG_INTERCONNECT_QCOM_QCS404=m > +CONFIG_INTERCONNECT_QCOM_QCS404=y > CONFIG_INTERCONNECT_QCOM_QCS615=y > CONFIG_INTERCONNECT_QCOM_QCS8300=y > CONFIG_INTERCONNECT_QCOM_QDU1000=y > > --- > base-commit: b098bcd8278b89cb3eb73fdb6e06dc49af75ad37 > change-id: 20250311-wip-obbardc-qcom-defconfig-interconnects-builtin-258fcc961b11 > > Best regards, > -- > Christopher Obbard <christopher.obbard@linaro.org> >
On Tue, Mar 11, 2025 at 07:10:06PM +0100, Christopher Obbard wrote: > I sent this patch to start the discussion, some things I found: > > 1) Some interconnects are missing from arm defconfig. Should they be =y too ? No, unless those are required for the UART console. > $ grep CONFIG_INTERCONNECT_QCOM arch/arm/configs/qcom_defconfig > CONFIG_INTERCONNECT_QCOM=y > CONFIG_INTERCONNECT_QCOM_MSM8974=m > CONFIG_INTERCONNECT_QCOM_SDX55=m > > 2) Some interconnects are missing from arm64 defconfig (which should > probably be in there) (I have included just two examples): I think `git log -S CONFIG_INTERCONNECT_QCOM arch/arm64/configs/defconfig` will answer this question. The drivers are enabled on the premises of being required for a particular device, not because they exist in the Linux kernel. > $ grep CONFIG_INTERCONNECT drivers/interconnect/qcom/Makefile > obj-$(CONFIG_INTERCONNECT_QCOM_QCS615) += qnoc-qcs615.o > obj-$(CONFIG_INTERCONNECT_QCOM_SM7150) += qnoc-sm7150.o > > I can handle these in follow-up or v2 of the patchset as follow-up > commits, please let me know what you'd prefer. > > On Tue, 11 Mar 2025 at 19:03, Christopher Obbard > <christopher.obbard@linaro.org> wrote: > > > > Currently some Qualcomm interconnect drivers are enabled > > as modules which isn't overly useful since the interconnects > > are required to be loaded during early boot. > > > > Loading the interconnects late (e.g. in initrd or as module) > > can cause boot issues, such as slowdown or even not booting > > at all (since the interconnect would be required for storage > > devices). > > > > Be consistent and enable all of the Qualcomm interconnect > > drivers as built-in to the kernel image. > > > > Signed-off-by: Christopher Obbard <christopher.obbard@linaro.org> > > --- > > arch/arm64/configs/defconfig | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > index 219ef05ee5a757c43a37ec9f8571ce9976354830..6582baee2ab02ecb2ff442c6e73aa6a23fee8d7f 100644 > > --- a/arch/arm64/configs/defconfig > > +++ b/arch/arm64/configs/defconfig > > @@ -1656,11 +1656,11 @@ CONFIG_INTERCONNECT_IMX8MN=m > > CONFIG_INTERCONNECT_IMX8MQ=m > > CONFIG_INTERCONNECT_IMX8MP=y > > CONFIG_INTERCONNECT_QCOM=y > > -CONFIG_INTERCONNECT_QCOM_MSM8916=m > > +CONFIG_INTERCONNECT_QCOM_MSM8916=y > > CONFIG_INTERCONNECT_QCOM_MSM8996=y > > -CONFIG_INTERCONNECT_QCOM_OSM_L3=m > > +CONFIG_INTERCONNECT_QCOM_OSM_L3=y > > CONFIG_INTERCONNECT_QCOM_QCM2290=y > > -CONFIG_INTERCONNECT_QCOM_QCS404=m > > +CONFIG_INTERCONNECT_QCOM_QCS404=y > > CONFIG_INTERCONNECT_QCOM_QCS615=y > > CONFIG_INTERCONNECT_QCOM_QCS8300=y > > CONFIG_INTERCONNECT_QCOM_QDU1000=y > > > > --- > > base-commit: b098bcd8278b89cb3eb73fdb6e06dc49af75ad37 > > change-id: 20250311-wip-obbardc-qcom-defconfig-interconnects-builtin-258fcc961b11 > > > > Best regards, > > -- > > Christopher Obbard <christopher.obbard@linaro.org> > >
Hi Dmitry, On Tue, 11 Mar 2025 at 19:58, Dmitry Baryshkov <lumag@kernel.org> wrote: > > On Tue, Mar 11, 2025 at 07:10:06PM +0100, Christopher Obbard wrote: > > I sent this patch to start the discussion, some things I found: > > > > 1) Some interconnects are missing from arm defconfig. Should they be =y too ? > > No, unless those are required for the UART console. OK, that makes sense. FWIW the cryptic (to me, at least) commit log on https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6eee808134ecf1c1093ff1ddfc056dc5e469d0c3 made me think that the interconnects should be built-in on all devices. Of course, the real problem here is RB3gen2 not actually finding the UFS/eMMC device due to no interconnect driver. Until now, I have been building that into the kernel. I will investigate instead shoving into the initrd (in both debian and fedora) which should solve my issue and render this patchset useless. Thanks, Chris > > > $ grep CONFIG_INTERCONNECT_QCOM arch/arm/configs/qcom_defconfig > > CONFIG_INTERCONNECT_QCOM=y > > CONFIG_INTERCONNECT_QCOM_MSM8974=m > > CONFIG_INTERCONNECT_QCOM_SDX55=m > > > > 2) Some interconnects are missing from arm64 defconfig (which should > > probably be in there) (I have included just two examples): > > I think `git log -S CONFIG_INTERCONNECT_QCOM > arch/arm64/configs/defconfig` will answer this question. The drivers are > enabled on the premises of being required for a particular device, not > because they exist in the Linux kernel. > > > $ grep CONFIG_INTERCONNECT drivers/interconnect/qcom/Makefile > > obj-$(CONFIG_INTERCONNECT_QCOM_QCS615) += qnoc-qcs615.o > > obj-$(CONFIG_INTERCONNECT_QCOM_SM7150) += qnoc-sm7150.o > > > > I can handle these in follow-up or v2 of the patchset as follow-up > > commits, please let me know what you'd prefer. > > > > On Tue, 11 Mar 2025 at 19:03, Christopher Obbard > > <christopher.obbard@linaro.org> wrote: > > > > > > Currently some Qualcomm interconnect drivers are enabled > > > as modules which isn't overly useful since the interconnects > > > are required to be loaded during early boot. > > > > > > Loading the interconnects late (e.g. in initrd or as module) > > > can cause boot issues, such as slowdown or even not booting > > > at all (since the interconnect would be required for storage > > > devices). > > > > > > Be consistent and enable all of the Qualcomm interconnect > > > drivers as built-in to the kernel image. > > > > > > Signed-off-by: Christopher Obbard <christopher.obbard@linaro.org> > > > --- > > > arch/arm64/configs/defconfig | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > > index 219ef05ee5a757c43a37ec9f8571ce9976354830..6582baee2ab02ecb2ff442c6e73aa6a23fee8d7f 100644 > > > --- a/arch/arm64/configs/defconfig > > > +++ b/arch/arm64/configs/defconfig > > > @@ -1656,11 +1656,11 @@ CONFIG_INTERCONNECT_IMX8MN=m > > > CONFIG_INTERCONNECT_IMX8MQ=m > > > CONFIG_INTERCONNECT_IMX8MP=y > > > CONFIG_INTERCONNECT_QCOM=y > > > -CONFIG_INTERCONNECT_QCOM_MSM8916=m > > > +CONFIG_INTERCONNECT_QCOM_MSM8916=y > > > CONFIG_INTERCONNECT_QCOM_MSM8996=y > > > -CONFIG_INTERCONNECT_QCOM_OSM_L3=m > > > +CONFIG_INTERCONNECT_QCOM_OSM_L3=y > > > CONFIG_INTERCONNECT_QCOM_QCM2290=y > > > -CONFIG_INTERCONNECT_QCOM_QCS404=m > > > +CONFIG_INTERCONNECT_QCOM_QCS404=y > > > CONFIG_INTERCONNECT_QCOM_QCS615=y > > > CONFIG_INTERCONNECT_QCOM_QCS8300=y > > > CONFIG_INTERCONNECT_QCOM_QDU1000=y > > > > > > --- > > > base-commit: b098bcd8278b89cb3eb73fdb6e06dc49af75ad37 > > > change-id: 20250311-wip-obbardc-qcom-defconfig-interconnects-builtin-258fcc961b11 > > > > > > Best regards, > > > -- > > > Christopher Obbard <christopher.obbard@linaro.org> > > > > > -- > With best wishes > Dmitry
On 3/11/25 7:52 PM, Dmitry Baryshkov wrote: > On Tue, Mar 11, 2025 at 07:02:56PM +0100, Christopher Obbard wrote: >> Currently some Qualcomm interconnect drivers are enabled >> as modules which isn't overly useful since the interconnects >> are required to be loaded during early boot. >> >> Loading the interconnects late (e.g. in initrd or as module) >> can cause boot issues, such as slowdown or even not booting >> at all (since the interconnect would be required for storage >> devices). > > This is not a good justification. It should be perfectly fine to load > block drivers (including their dependencies) from the initramfs. > > Up to now we have been enabling only those interconnect (and pinctrl, > btw) drivers, which are required to be able to open the consoe (thanks, > systemd). (non-earlycon) console is a better argument here, though Konrad
On 11/03/2025 19:02, Christopher Obbard wrote: > Currently some Qualcomm interconnect drivers are enabled > as modules which isn't overly useful since the interconnects > are required to be loaded during early boot. > > Loading the interconnects late (e.g. in initrd or as module) > can cause boot issues, such as slowdown or even not booting > at all (since the interconnect would be required for storage > devices). > > Be consistent and enable all of the Qualcomm interconnect > drivers as built-in to the kernel image. I don't agree. If you want consistency, fix booting process and make everything module. We switch them to builtin on exception, so come with reasoning for given exceptions instead of making exceptions the new normal. Best regards, Krzysztof
On 11/03/2025 20:15, Christopher Obbard wrote: > Hi Dmitry, > > On Tue, 11 Mar 2025 at 19:58, Dmitry Baryshkov <lumag@kernel.org> wrote: >> >> On Tue, Mar 11, 2025 at 07:10:06PM +0100, Christopher Obbard wrote: >>> I sent this patch to start the discussion, some things I found: >>> >>> 1) Some interconnects are missing from arm defconfig. Should they be =y too ? >> >> No, unless those are required for the UART console. > > OK, that makes sense. FWIW the cryptic (to me, at least) commit log on > https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6eee808134ecf1c1093ff1ddfc056dc5e469d0c3 > made me think that the interconnects should be built-in on all devices. > > Of course, the real problem here is RB3gen2 not actually finding the > UFS/eMMC device due to no interconnect driver. > Until now, I have been building that into the kernel. I will > investigate instead shoving into the initrd (in both debian and > fedora) which should solve my issue and render this patchset useless. For Qualcomm platforms you are expected to always have initramfs, thus you will have the modules for UFS/eMMC mounts. I don't understand the problem which you were trying to solve. The interconnects were built in *only* because of need for serial console. Only. Best regards, Krzysztof
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 219ef05ee5a757c43a37ec9f8571ce9976354830..6582baee2ab02ecb2ff442c6e73aa6a23fee8d7f 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1656,11 +1656,11 @@ CONFIG_INTERCONNECT_IMX8MN=m CONFIG_INTERCONNECT_IMX8MQ=m CONFIG_INTERCONNECT_IMX8MP=y CONFIG_INTERCONNECT_QCOM=y -CONFIG_INTERCONNECT_QCOM_MSM8916=m +CONFIG_INTERCONNECT_QCOM_MSM8916=y CONFIG_INTERCONNECT_QCOM_MSM8996=y -CONFIG_INTERCONNECT_QCOM_OSM_L3=m +CONFIG_INTERCONNECT_QCOM_OSM_L3=y CONFIG_INTERCONNECT_QCOM_QCM2290=y -CONFIG_INTERCONNECT_QCOM_QCS404=m +CONFIG_INTERCONNECT_QCOM_QCS404=y CONFIG_INTERCONNECT_QCOM_QCS615=y CONFIG_INTERCONNECT_QCOM_QCS8300=y CONFIG_INTERCONNECT_QCOM_QDU1000=y
Currently some Qualcomm interconnect drivers are enabled as modules which isn't overly useful since the interconnects are required to be loaded during early boot. Loading the interconnects late (e.g. in initrd or as module) can cause boot issues, such as slowdown or even not booting at all (since the interconnect would be required for storage devices). Be consistent and enable all of the Qualcomm interconnect drivers as built-in to the kernel image. Signed-off-by: Christopher Obbard <christopher.obbard@linaro.org> --- arch/arm64/configs/defconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: b098bcd8278b89cb3eb73fdb6e06dc49af75ad37 change-id: 20250311-wip-obbardc-qcom-defconfig-interconnects-builtin-258fcc961b11 Best regards,