Good point. U-Boot has instruments to get clk_m rate on time of timer probe. I need some time to prepare this modification for testing.
пт, 27 січ. 2023 р. о 00:12 Dmitry Osipenko <dig...@gmail.com> пише: > > 27.01.2023 01:00, Dmitry Osipenko пишет: > > 26.01.2023 20:54, Thierry Reding пишет: > >> On Thu, Jan 26, 2023 at 07:08:54PM +0200, Svyatoslav Ryhel wrote: > >>> чт, 26 січ. 2023 р. о 12:35 Thierry Reding <thierry.red...@gmail.com> > >>> пише: > >>>> > >>>> On Wed, Jan 25, 2023 at 05:41:08PM +0100, Thierry Reding wrote: > >>>>> On Tue, Jan 24, 2023 at 08:57:48AM +0200, Svyatoslav Ryhel wrote: > >>>>>> - ARM: tegra: remap clock_osc_freq for all Tegra family > >>>>>> Enum clock_osc_freq was designed to use only with T20. > >>>>>> This patch remaps it to use additional frequencies, added in > >>>>>> T30+ SoC while maintaining backwards compatibility with T20. > >>>>>> > >>>>>> - drivers: timer: add timer driver for ARMv7 based Tegra devices > >>>>>> Add timer support for T20/T30/T114 and T124 based devices. > >>>>>> Driver is based on DM, has device tree support and can be > >>>>>> used on SPL and early boot stage. > >>>>>> > >>>>>> - ARM: tegra: include timer as default option > >>>>>> Enable TIMER as default option for all Tegra devices and > >>>>>> enable TEGRA_TIMER for TEGRA_ARMV7_COMMON. Additionally > >>>>>> enable SPL_TIMER if build as SPL part and drop deprecated > >>>>>> configs from common header. > >>>>>> > >>>>>> P. S. I have no arm64 Tegra and according to comment in > >>>>>> tegra-common.h > >>>>>> Use the Tegra US timer on ARMv7, but the architected timer on ARMv8. > >>>>>> > >>>>>> Svyatoslav Ryhel (3): > >>>>>> ARM: tegra: remap clock_osc_freq for all Tegra family > >>>>>> drivers: timer: add timer driver for ARMv7 based Tegra devices > >>>>>> ARM: tegra: include timer as default option > >>>>> > >>>>> This causes a regression on Tegra210 (Jetson TX1). I'm trying to > >>>>> investigate, but it's complicated by the fact that I'm not getting out > >>>>> any debug prints, so I suspect the issue is happening quite early. > >>>> > >>>> Alright, I managed to make this work on Tegra210 using the following > >>>> patch on top of this series: > >>>> > >>> > >>> Hello! Thanks for testing this on T210 SoC. > >>> > >>>> --- >8 --- > >>>> diff --git a/arch/arm/dts/tegra210.dtsi b/arch/arm/dts/tegra210.dtsi > >>>> index a521a43d6cfd..ccb5a927da89 100644 > >>>> --- a/arch/arm/dts/tegra210.dtsi > >>>> +++ b/arch/arm/dts/tegra210.dtsi > >>>> @@ -318,7 +318,7 @@ > >>>> }; > >>>> > >>>> timer@60005000 { > >>>> - compatible = "nvidia,tegra210-timer", > >>>> "nvidia,tegra20-timer"; > >>>> + compatible = "nvidia,tegra210-timer", > >>>> "nvidia,tegra30-timer", "nvidia,tegra20-timer"; > >>> > >>> This compatibe should not be needed since the driver will have t210 > >>> compatible included. > >> > >> Yes, it should be fine to leave this as-is. I had included this before > >> updating the driver, to get the driver to bind to this. Upstream Linux > >> doesn't include "nvidia,tegra20-timer", so it only has the compatible > >> string for Tegra210. I think that's slightly better because the register > >> interface isn't quite compatible. That's a separate issue and we can do > >> that in a follow-up patch. > >> > >>> > >>>> reg = <0x0 0x60005000 0x0 0x400>; > >>>> interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>, > >>>> <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>, > >>>> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig > >>>> index cc3f00e50128..b50eec5b8c9b 100644 > >>>> --- a/arch/arm/mach-tegra/Kconfig > >>>> +++ b/arch/arm/mach-tegra/Kconfig > >>>> @@ -136,6 +136,7 @@ config TEGRA210 > >>>> select TEGRA_PINCTRL > >>>> select TEGRA_PMC > >>>> select TEGRA_PMC_SECURE > >>>> + select TEGRA_TIMER > >>>> > >>>> config TEGRA186 > >>>> bool "Tegra186 family" > >>>> diff --git a/drivers/timer/tegra-timer.c b/drivers/timer/tegra-timer.c > >>>> index d2d163cf3fef..235532ba8926 100644 > >>>> --- a/drivers/timer/tegra-timer.c > >>>> +++ b/drivers/timer/tegra-timer.c > >>>> @@ -58,17 +58,26 @@ static notrace u64 tegra_timer_get_count(struct > >>>> udevice *dev) > >>>> static int tegra_timer_probe(struct udevice *dev) > >>>> { > >>>> struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); > >>>> + enum clock_osc_freq freq; > >>>> u32 usec_config, value; > >>>> > >>>> /* Timer rate has to be set unconditionally */ > >>>> uc_priv->clock_rate = TEGRA_TIMER_RATE; > >>>> > >>>> + /* > >>>> + * The microsecond timer runs off of clk_m on Tegra210, and clk_m > >>>> + * runs at half the OSC, so fake this up. > >>>> + */ > >>>> + freq = clock_get_osc_freq(); > >>>> + if (freq == CLOCK_OSC_FREQ_38_4) > >>>> + freq = CLOCK_OSC_FREQ_19_2; > >>>> + > >>> > >>> May you confirm that ALL known T210 devices use 19.2MHz as calibration > >>> clock for timer? > >> > >> According to the Tegra210 TRM, the TIMERUS depends on the rate of clk_m > >> and clk_m is derived from OSC and either divided by 1, 2, 3 or 4. The > >> TRM goes on to say that: > >> > >> The expectation is that this CLK_M_DIVISOR will only be changed > >> once after powering VDD_SOC on in cold boot/LP0 exit path. So > >> these sequences are verified with an oscillator clock of 38.4 > >> MHz; div2 setting of the CLK_M divisor must be used. The result > >> is 19.2 MHz clk_m. > >> > >> And then it says: > >> > >> Note: Div2 is the recommended clk_m divisor value. Do not use > >> any other value. > >> > >> This is from Section 5.1.2 "Clk_m Divisor" of the Tegra210 TRM. > >> > >>> If yes I can set this change in simpler as a separate commit or > >>> including into existing patches. > >> > >> Anything you have in mind? I tried a couple of variations to the above > >> and they all failed because in other places it's important that OSC is > >> recognized as running at 38.4 MHz. If not, then other PLLs will not > >> work properly and even basic things like the debug UART won't work. > >> > >> Technically the right thing to do would be to base the USEC config off > >> the clk_m rate. We didn't do that back at the time, IIRC, because most > >> of the clock infrastructure didn't exist, but it might be possible to > >> achieve this today. I kept the above because it is still a bit simpler > >> and as the TRM suggests nobody should be using anything other than the > >> div2 setting for clk_m. I'm certainly not aware of any devices that do > >> something different. And U-Boot has always had this assumption as well, > >> so I think it's safe to use. > > > > Am I understanding correctly that for T210 we can/should use > > clk_m_get_rate() instead of clock_get_osc_freq() in tegra_timer_probe()? > > Have you tested this option? > > Although, looking at the kernel code, I see that clk_m is the parent > clock for timer on all SoCs. Hence replacing clock_get_osc_freq() with > clk_m_get_rate() should be the proper solution and then no T210-specific > workarounds are needed. >