| Message ID | 20240409-enable-sm6115-icc-v1-1-bf894fb5a585@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: defconfig: build INTERCONNECT_QCOM_SM6115 as module | expand |
On 4/9/24 20:27, Dmitry Baryshkov wrote: > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- =y for console? Konrad
On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > =y for console? I think with pinctrl being set to m it won't reach the console anyway. And earlycon should work w/o ICC driver if I'm not mistaken. Several other Qualcomm platforms also have interconnects built as modules in defconfig. I really suppose that we should move all such drivers to =m and force using initrd. But this sounds like a topic for plumbers. > > Konrad
On 4/10/24 00:12, Dmitry Baryshkov wrote: > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> On 4/9/24 20:27, Dmitry Baryshkov wrote: >>> Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the >>> interconnect driver for the SoC used on Qualcomm Robotics RB2 board. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >> >> =y for console? > > I think with pinctrl being set to m it won't reach the console anyway. > And earlycon should work w/o ICC driver if I'm not mistaken. > > Several other Qualcomm platforms also have interconnects built as > modules in defconfig. I really suppose that we should move all such > drivers to =m and force using initrd. But this sounds like a topic for > plumbers. TBF I'm not a fan of anything interconnect-adjacent being =m, as by design it's supposed to load early and must be present Konrad
On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > > =y for console? > > I think with pinctrl being set to m it won't reach the console anyway. > And earlycon should work w/o ICC driver if I'm not mistaken. > > Several other Qualcomm platforms also have interconnects built as > modules in defconfig. I really suppose that we should move all such > drivers to =m and force using initrd. But this sounds like a topic for > plumbers. > That currently does not work very well, because if you probe defer the UART into the future e.g. systemd will open /dev/console before it exist - with no interest in reopening that later. So, if you care about UART, that is suboptimal. Resolve this, and I'd be happy to see them all go =m. Regards, Bjorn
On Thu, 11 Apr 2024 at 06:19, Bjorn Andersson <andersson@kernel.org> wrote: > > On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > --- > > > > > > =y for console? > > > > I think with pinctrl being set to m it won't reach the console anyway. > > And earlycon should work w/o ICC driver if I'm not mistaken. > > > > Several other Qualcomm platforms also have interconnects built as > > modules in defconfig. I really suppose that we should move all such > > drivers to =m and force using initrd. But this sounds like a topic for > > plumbers. > > > > That currently does not work very well, because if you probe defer the > UART into the future e.g. systemd will open /dev/console before it > exist - with no interest in reopening that later. > > So, if you care about UART, that is suboptimal. > > Resolve this, and I'd be happy to see them all go =m. Doesn't /dev/console handle switching between earlycon and actual console? I'll take a look at some point in the future. But I can't help but notice that currently we have 5 ICC drivers built as modules (out of 21 mentioned in the defconfig). Should we fix them too? CONFIG_INTERCONNECT_QCOM_MSM8916=m CONFIG_INTERCONNECT_QCOM_MSM8996=m CONFIG_INTERCONNECT_QCOM_QCS404=m CONFIG_INTERCONNECT_QCOM_SM8150=m CONFIG_INTERCONNECT_QCOM_SM8350=m
On Thu, 11 Apr 2024 at 06:19, Bjorn Andersson <andersson@kernel.org> wrote: > > On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > --- > > > > > > =y for console? > > > > I think with pinctrl being set to m it won't reach the console anyway. > > And earlycon should work w/o ICC driver if I'm not mistaken. > > > > Several other Qualcomm platforms also have interconnects built as > > modules in defconfig. I really suppose that we should move all such > > drivers to =m and force using initrd. But this sounds like a topic for > > plumbers. > > > > That currently does not work very well, because if you probe defer the > UART into the future e.g. systemd will open /dev/console before it > exist - with no interest in reopening that later. > > So, if you care about UART, that is suboptimal. > > Resolve this, and I'd be happy to see them all go =m. BTW, so even if /dev/console doesn't handle switching behind the scenes, Systemd at initramfs opens /dev/console, maybe fails, loads icc and other modules. Then it does pivot_root and executes systemd on the rootfs. At this point the /dev/console exists and the systemd should continue without any issues.
On Thu, Apr 11, 2024 at 06:33:17AM +0300, Dmitry Baryshkov wrote: > On Thu, 11 Apr 2024 at 06:19, Bjorn Andersson <andersson@kernel.org> wrote: > > > > On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > > > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > --- > > > > > > > > =y for console? > > > > > > I think with pinctrl being set to m it won't reach the console anyway. > > > And earlycon should work w/o ICC driver if I'm not mistaken. > > > > > > Several other Qualcomm platforms also have interconnects built as > > > modules in defconfig. I really suppose that we should move all such > > > drivers to =m and force using initrd. But this sounds like a topic for > > > plumbers. > > > > > > > That currently does not work very well, because if you probe defer the > > UART into the future e.g. systemd will open /dev/console before it > > exist - with no interest in reopening that later. > > > > So, if you care about UART, that is suboptimal. > > > > Resolve this, and I'd be happy to see them all go =m. > > BTW, so even if /dev/console doesn't handle switching behind the scenes, > Systemd at initramfs opens /dev/console, maybe fails, loads icc and > other modules. Then it does pivot_root and executes systemd on the > rootfs. At this point the /dev/console exists and the systemd should > continue without any issues. > On which console does it ask for your passphrase for your encrypted rootfs? Closer to home, I do most of my testing in a ramdisk, not having working /dev/console is a problem for m. Regards, Bjorn
On Thu, Apr 11, 2024 at 06:31:08AM +0300, Dmitry Baryshkov wrote: > On Thu, 11 Apr 2024 at 06:19, Bjorn Andersson <andersson@kernel.org> wrote: > > > > On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > > > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > --- > > > > > > > > =y for console? > > > > > > I think with pinctrl being set to m it won't reach the console anyway. > > > And earlycon should work w/o ICC driver if I'm not mistaken. > > > > > > Several other Qualcomm platforms also have interconnects built as > > > modules in defconfig. I really suppose that we should move all such > > > drivers to =m and force using initrd. But this sounds like a topic for > > > plumbers. > > > > > > > That currently does not work very well, because if you probe defer the > > UART into the future e.g. systemd will open /dev/console before it > > exist - with no interest in reopening that later. > > > > So, if you care about UART, that is suboptimal. > > > > Resolve this, and I'd be happy to see them all go =m. > > Doesn't /dev/console handle switching between earlycon and actual > console? I'll take a look at some point in the future. > It does not, selection happens at open(). So user space need to reopen /dev/console once the console has been updated. > But I can't help but notice that currently we have 5 ICC drivers built > as modules (out of 21 mentioned in the defconfig). Should we fix them > too? I reviewed this a while back, at which time none of these had interconnects specified for their UART device. The lack of icc is likely a problem at some point, in which case this would need to be updated. But at this time (at the time I looked at it), there was no problem to motivate such change with. Regards, Bjorn > > CONFIG_INTERCONNECT_QCOM_MSM8916=m > CONFIG_INTERCONNECT_QCOM_MSM8996=m > CONFIG_INTERCONNECT_QCOM_QCS404=m > CONFIG_INTERCONNECT_QCOM_SM8150=m > CONFIG_INTERCONNECT_QCOM_SM8350=m > > > -- > With best wishes > Dmitry
On Thu, Apr 11, 2024 at 08:38:23AM -0700, Bjorn Andersson wrote: > On Thu, Apr 11, 2024 at 06:31:08AM +0300, Dmitry Baryshkov wrote: > > On Thu, 11 Apr 2024 at 06:19, Bjorn Andersson <andersson@kernel.org> wrote: > > > > > > On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > > > > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > --- > > > > > > > > > > =y for console? > > > > > > > > I think with pinctrl being set to m it won't reach the console anyway. > > > > And earlycon should work w/o ICC driver if I'm not mistaken. > > > > > > > > Several other Qualcomm platforms also have interconnects built as > > > > modules in defconfig. I really suppose that we should move all such > > > > drivers to =m and force using initrd. But this sounds like a topic for > > > > plumbers. > > > > > > > > > > That currently does not work very well, because if you probe defer the > > > UART into the future e.g. systemd will open /dev/console before it > > > exist - with no interest in reopening that later. > > > > > > So, if you care about UART, that is suboptimal. > > > > > > Resolve this, and I'd be happy to see them all go =m. > > > > Doesn't /dev/console handle switching between earlycon and actual > > console? I'll take a look at some point in the future. > > > > It does not, selection happens at open(). So user space need to reopen > /dev/console once the console has been updated. > > > But I can't help but notice that currently we have 5 ICC drivers built > > as modules (out of 21 mentioned in the defconfig). Should we fix them > > too? > > I reviewed this a while back, at which time none of these had > interconnects specified for their UART device. > > The lack of icc is likely a problem at some point, in which case this > would need to be updated. But at this time (at the time I looked at it), > there was no problem to motivate such change with. Note that on Android we have no choice but to build the interconnect drivers as modules. Vendor support is added as modules loaded on the vendor-agnostic GKI kernel. We don't have issues with earlycon there. That being said, I'm not really an expert in how the earlycon or serial drivers work. From a HW perspective, all the busses required for the serial UART are enabled by default from bootloaders. So, no explicit voting for them should be necessary for early serial logs.
On Thu, Apr 11, 2024 at 09:55:28AM -0700, Mike Tipton wrote: > On Thu, Apr 11, 2024 at 08:38:23AM -0700, Bjorn Andersson wrote: > > On Thu, Apr 11, 2024 at 06:31:08AM +0300, Dmitry Baryshkov wrote: > > > On Thu, 11 Apr 2024 at 06:19, Bjorn Andersson <andersson@kernel.org> wrote: > > > > > > > > On Wed, Apr 10, 2024 at 01:12:15AM +0300, Dmitry Baryshkov wrote: > > > > > On Tue, 9 Apr 2024 at 23:02, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > > > On 4/9/24 20:27, Dmitry Baryshkov wrote: > > > > > > > Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the > > > > > > > interconnect driver for the SoC used on Qualcomm Robotics RB2 board. > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > --- > > > > > > > > > > > > =y for console? > > > > > > > > > > I think with pinctrl being set to m it won't reach the console anyway. > > > > > And earlycon should work w/o ICC driver if I'm not mistaken. > > > > > > > > > > Several other Qualcomm platforms also have interconnects built as > > > > > modules in defconfig. I really suppose that we should move all such > > > > > drivers to =m and force using initrd. But this sounds like a topic for > > > > > plumbers. > > > > > > > > > > > > > That currently does not work very well, because if you probe defer the > > > > UART into the future e.g. systemd will open /dev/console before it > > > > exist - with no interest in reopening that later. > > > > > > > > So, if you care about UART, that is suboptimal. > > > > > > > > Resolve this, and I'd be happy to see them all go =m. > > > > > > Doesn't /dev/console handle switching between earlycon and actual > > > console? I'll take a look at some point in the future. > > > > > > > It does not, selection happens at open(). So user space need to reopen > > /dev/console once the console has been updated. > > > > > But I can't help but notice that currently we have 5 ICC drivers built > > > as modules (out of 21 mentioned in the defconfig). Should we fix them > > > too? > > > > I reviewed this a while back, at which time none of these had > > interconnects specified for their UART device. > > > > The lack of icc is likely a problem at some point, in which case this > > would need to be updated. But at this time (at the time I looked at it), > > there was no problem to motivate such change with. > > Note that on Android we have no choice but to build the interconnect > drivers as modules. Vendor support is added as modules loaded on the > vendor-agnostic GKI kernel. We don't have issues with earlycon there. > That being said, I'm not really an expert in how the earlycon or serial > drivers work. > > From a HW perspective, all the busses required for the serial UART are > enabled by default from bootloaders. So, no explicit voting for them > should be necessary for early serial logs. You're entirely correct. The problem here is that we get the earlycon driver during kernel boot, but the full fledged serial driver is unable to come up due to lacking the specified interconnect provider. Userspace starts running and opens /dev/console (which isn't our proper UART), it then assists in loading the necessary modules, at which time the interconnect driver and then the serial driver probes - but it doesn't necessarily know to reopen /dev/console when this is done. So this is solely a software problem caused by early (standard Linux) userspace expecting /dev/console to be functional and remain functional. As Dmitry says, in many such configurations the system will indeed open /dev/console again later, hiding this problem - or in a typical end-user product you probably don't have a UART. Regards, Bjorn
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 9957e126e32d..14d727173c8c 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1600,6 +1600,7 @@ CONFIG_INTERCONNECT_QCOM_SC8180X=y CONFIG_INTERCONNECT_QCOM_SC8280XP=y CONFIG_INTERCONNECT_QCOM_SDM845=y CONFIG_INTERCONNECT_QCOM_SDX75=y +CONFIG_INTERCONNECT_QCOM_SM6115=m CONFIG_INTERCONNECT_QCOM_SM8150=m CONFIG_INTERCONNECT_QCOM_SM8250=y CONFIG_INTERCONNECT_QCOM_SM8350=m
Enable CONFIG_INTERCONNECT_QCOM_SM6115 as module to enable the interconnect driver for the SoC used on Qualcomm Robotics RB2 board. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) --- base-commit: a053fd3ca5d1b927a8655f239c84b0d790218fda change-id: 20240409-enable-sm6115-icc-7b0b0c08da2b Best regards,