diff mbox series

ARM: mstar: Select HAVE_ARM_ARCH_TIMER

Message ID 20220301104349.3040422-1-daniel@0x0f.com (mailing list archive)
State Accepted
Commit ea49432d184a6a09f84461604b7711a4e9f5ec9c
Headers show
Series ARM: mstar: Select HAVE_ARM_ARCH_TIMER | expand

Commit Message

Daniel Palmer March 1, 2022, 10:43 a.m. UTC
The mstar SoCs have an arch timer but HAVE_ARM_ARCH_TIMER wasn't
selected. If MSC313E_TIMER isn't selected then the kernel gets
stuck at boot because there are no timers available.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---

I moved one of my CI builds from a 5.16 based tree to a 5.17
based one and it stopped booting. At some point I had broken my DTB
so the MStar timer wasn't getting probed anymore and at
the same time the arch timer driver wasn't getting built in.
I'm not entirely sure why the arch timer driver was built
before but isn't now. Anyhow, all of the other ARMv7 machines
seem to do this so I guess it's right.

Arnd/Olof, please apply directly if acceptable. Maybe this should
be in 5.17 as a fix?

 arch/arm/mach-mstar/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

patchwork-bot+linux-soc@kernel.org March 1, 2022, 12:40 p.m. UTC | #1
Hello:

This patch was applied to soc/soc.git (for-next)
by Arnd Bergmann <arnd@arndb.de>:

On Tue,  1 Mar 2022 19:43:49 +0900 you wrote:
> The mstar SoCs have an arch timer but HAVE_ARM_ARCH_TIMER wasn't
> selected. If MSC313E_TIMER isn't selected then the kernel gets
> stuck at boot because there are no timers available.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
> 
> [...]

Here is the summary with links:
  - ARM: mstar: Select HAVE_ARM_ARCH_TIMER
    https://git.kernel.org/soc/soc/c/ea49432d184a

You are awesome, thank you!
Romain Perier March 4, 2022, 10:09 a.m. UTC | #2
Hi all,

Interesting, when I wrote the patch for msc313e-timer, I always had
ARCH_ARM_TIMER implicitly built, so I thought that it was implicitly
selected by MULTI_V7...
I agree that we should at least have ARM_ARCH_TIMER enabled, otherwise
we won't being able to boot correctly (no clockevents)

I have also noticed that we missed this commit
https://github.com/linux-chenxing/linux/commit/4cf42fcb4618cd77d80f767d5524c32fb40a91c6
 (that selects MSC313E_TIMER), which might explain your initial issue
(not being to boot).

MSC313E_TIMER being optional on ARM (because of ARM_ARCH_TIMER), we
should be sure that arm_timer is enabled and we can keep the mstar
timer optional, imho.

What do you think ?

Romain

Le mar. 1 mars 2022 à 11:44, Daniel Palmer <daniel@0x0f.com> a écrit :
>
> The mstar SoCs have an arch timer but HAVE_ARM_ARCH_TIMER wasn't
> selected. If MSC313E_TIMER isn't selected then the kernel gets
> stuck at boot because there are no timers available.
>
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>
> I moved one of my CI builds from a 5.16 based tree to a 5.17
> based one and it stopped booting. At some point I had broken my DTB
> so the MStar timer wasn't getting probed anymore and at
> the same time the arch timer driver wasn't getting built in.
> I'm not entirely sure why the arch timer driver was built
> before but isn't now. Anyhow, all of the other ARMv7 machines
> seem to do this so I guess it's right.
>
> Arnd/Olof, please apply directly if acceptable. Maybe this should
> be in 5.17 as a fix?
>
>  arch/arm/mach-mstar/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-mstar/Kconfig b/arch/arm/mach-mstar/Kconfig
> index cd300eeedc20..0bf4d312bcfd 100644
> --- a/arch/arm/mach-mstar/Kconfig
> +++ b/arch/arm/mach-mstar/Kconfig
> @@ -3,6 +3,7 @@ menuconfig ARCH_MSTARV7
>         depends on ARCH_MULTI_V7
>         select ARM_GIC
>         select ARM_HEAVY_MB
> +       select HAVE_ARM_ARCH_TIMER
>         select MST_IRQ
>         select MSTAR_MSC313_MPLL
>         help
> --
> 2.35.1
>
Daniel Palmer March 4, 2022, 1:08 p.m. UTC | #3
Hi Romain,

On Fri, 4 Mar 2022 at 19:09, Romain Perier <romain.perier@gmail.com> wrote:

> Hi all,
>
> Interesting, when I wrote the patch for msc313e-timer, I always had
> ARCH_ARM_TIMER implicitly built, so I thought that it was implicitly
> selected by MULTI_V7...
>

It was getting built somehow until 5.17 I think and I only noticed it got
broken because the CI build uses a defconfig whereas my development tree
has a full config that has ARCH_ARM_TIMER in it.


> I agree that we should at least have ARM_ARCH_TIMER enabled, otherwise
> we won't being able to boot correctly (no clockevents)
>

Arnd applied this for us already.


> I have also noticed that we missed this commit
>
> https://github.com/linux-chenxing/linux/commit/4cf42fcb4618cd77d80f767d5524c32fb40a91c6
>  (that selects MSC313E_TIMER), which might explain your initial issue
> (not being to boot).
>
> MSC313E_TIMER being optional on ARM (because of ARM_ARCH_TIMER), we
> should be sure that arm_timer is enabled and we can keep the mstar
> timer optional, imho.
>
> What do you think ?
>

I think MSC313E_TIMER should be optional but default for ARCH_MSTARV7.

Cheers,

Daniel
diff mbox series

Patch

diff --git a/arch/arm/mach-mstar/Kconfig b/arch/arm/mach-mstar/Kconfig
index cd300eeedc20..0bf4d312bcfd 100644
--- a/arch/arm/mach-mstar/Kconfig
+++ b/arch/arm/mach-mstar/Kconfig
@@ -3,6 +3,7 @@  menuconfig ARCH_MSTARV7
 	depends on ARCH_MULTI_V7
 	select ARM_GIC
 	select ARM_HEAVY_MB
+	select HAVE_ARM_ARCH_TIMER
 	select MST_IRQ
 	select MSTAR_MSC313_MPLL
 	help