From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E7517C52D11 for ; Thu, 26 Jan 2023 17:54:13 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0D9428573A; Thu, 26 Jan 2023 18:54:12 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="iF/Fa+QI"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5443F8572D; Thu, 26 Jan 2023 18:54:10 +0100 (CET) Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3A7A685733 for ; Thu, 26 Jan 2023 18:54:06 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=thierry.reding@gmail.com Received: by mail-wr1-x42d.google.com with SMTP id q5so2624354wrv.0 for ; Thu, 26 Jan 2023 09:54:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=FYbBatIvBIbOwkPNQzCSM/hjQinHhz3XuFaKYlgPwas=; b=iF/Fa+QIPuv9l9KBNWpD1Uhz9cSavTzSq0jDN7sRlaOO6u6cpkD4LMtSbb31HNZxYc pRZsZOqx87RPiz+FXILJUZ8gLmn+CNd0B1dWG4nlAdxQ9BNIcCswy22RJS2UuCtOIhoM Cn+XECrZs6FaRuHBNE+1LPxwrVic0Ka1436iQJg7fFygRtiCCnneRXzpDeH4409maN9A G96tJ+qQvZpW7WPmljis7EvgYktxGxQkv8wiNM8ACNVp71IfyoeiuqZ2EgvJCNBlZwGc xbq3PJbj1DRYJItWsl1WXTUX3ZtuJOHqplrHewnAgPNKoGo1yuG3ueHNS25tCyAZpIId 9QHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=FYbBatIvBIbOwkPNQzCSM/hjQinHhz3XuFaKYlgPwas=; b=0bnSx4DU+Y3Avpmj/AbuEIiK5FcerQtQJFW7DjIWMQa+CxA/Ptsd9YsgCKMZmT0ClX i4wPtVv/TLyvx12XuCTXipNtSdE3L10GQ2nAkACVn/lX+1HOnmQQDayHYUpXVhQZQi31 w6SKVDbWWrVa6G/ETwP6j1yxiyp10sQtV7Mz/xEQXbHESdncdnfNsHAD1+3FQ6R8VlVF 7kPw+sVy+izXwCjiM5NovG6PnVcQDmhc5RzgdE+tkVgO52kqCUQMaxa/nPNH8q43K6B3 F3wiKzqV05MvpBiHwiVplfL1qkDDZVN8SRuF96mYp227ZYwDYpOY9Q6vfCKnss6QR2cI ivKw== X-Gm-Message-State: AFqh2kr0tSkTxMJQVYyG6XXHgSPVyQfpFbx/1+cbjD8M/j4IOTrpzDy5 Ns9itKDtk8dGzrZg4L0cnn0= X-Google-Smtp-Source: AMrXdXspWw8tV0juDVykPsRbGF3hGPUUngsWUEm4EThiYv8blRguA6ZX6F+zNZEdwK7XRkS7UTJNEg== X-Received: by 2002:adf:a14f:0:b0:2bf:950e:7fc4 with SMTP id r15-20020adfa14f000000b002bf950e7fc4mr19229458wrr.39.1674755645492; Thu, 26 Jan 2023 09:54:05 -0800 (PST) Received: from orome (p200300e41f201d00f22f74fffe1f3a53.dip0.t-ipconnect.de. [2003:e4:1f20:1d00:f22f:74ff:fe1f:3a53]) by smtp.gmail.com with ESMTPSA id a4-20020a056000100400b00286ad197346sm1828535wrx.70.2023.01.26.09.54.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Jan 2023 09:54:05 -0800 (PST) Date: Thu, 26 Jan 2023 18:54:03 +0100 From: Thierry Reding To: Svyatoslav Ryhel Cc: Rayagonda Kokatanur , Tom Warren , Marek Vasut , Maxim Schwalm , Dmitry Osipenko , Heinrich Schuchardt , Michal Simek , Stefan Roese , Eugen Hristev , Michael Walle , Simon Glass , Jim Liu , William Zhang , Rick Chen , Stefan Herbrechtsmeier , Andre Przywara , Jaehoon Chung , u-boot@lists.denx.de Subject: Re: [PATCH v6 0/3] Timer support for ARM Tegra Message-ID: References: <20230124065751.5973-1-clamor95@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="PO+9smPesjgdlSL2" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.2.9 (2022-11-12) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean --PO+9smPesjgdlSL2 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 26, 2023 at 07:08:54PM +0200, Svyatoslav Ryhel wrote: > =D1=87=D1=82, 26 =D1=81=D1=96=D1=87. 2023 =D1=80. =D0=BE 12:35 Thierry Re= ding =D0=BF=D0=B8=D1=88=D0=B5: > > > > 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: > > >=20 > Hello! Thanks for testing this on T210 SoC. >=20 > > --- >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 =3D "nvidia,tegra210-timer", "nvidia,tegra20= -timer"; > > + compatible =3D "nvidia,tegra210-timer", "nvidia,tegra30= -timer", "nvidia,tegra20-timer"; >=20 > 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. >=20 > > reg =3D <0x0 0x60005000 0x0 0x400>; > > interrupts =3D , > > , > > 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 ude= vice *dev) > > static int tegra_timer_probe(struct udevice *dev) > > { > > struct timer_dev_priv *uc_priv =3D 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 =3D 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 =3D clock_get_osc_freq(); > > + if (freq =3D=3D CLOCK_OSC_FREQ_38_4) > > + freq =3D CLOCK_OSC_FREQ_19_2; > > + >=20 > 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. Thierry > > /* > > * Configure microsecond timers to have 1MHz clock > > * Config register is 0xqqww, where qq is "dividend", ww is "di= visor" > > * Uses n+1 scheme > > */ > > - switch (clock_get_osc_freq()) { > > + switch (freq) { > > case CLOCK_OSC_FREQ_13_0: > > usec_config =3D 0x000c; /* (12+1)/(0+1) */ > > break; > > @@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = =3D { > > { .compatible =3D "nvidia,tegra30-timer" }, > > { .compatible =3D "nvidia,tegra114-timer" }, > > { .compatible =3D "nvidia,tegra124-timer" }, > > + { .compatible =3D "nvidia,tegra210-timer" }, > > { } > > }; > > --- >8 --- > > > > I've also tested this on Tegra186, though no additional changes were > > needed since Tegra186 doesn't use the Tegra timer. > > > > With the above folded in, the series is: > > > > Tested-by: Thierry Reding --PO+9smPesjgdlSL2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmPSvjgACgkQ3SOs138+ s6Hg7A/9FFNWeRePDRiDsy99V3WrB/+0ARBHyYzFM11SKlTohnbkkO8pz4NHgut0 UhcN7JnYNFrlexGM1j4XVn8cwKQwNBuPaoeZRvVECNihBMotXztzMJkk6X/v+Ff9 5vSeQSOCeTPxWO6xzudFgZb0ON0YLpdlUpV+qjHSiEIssiFZGUhuaxHd0QKOL6v/ BBX6Im1XtKf9a7YEAcbnozVmMo7CnhVyMpJm4CaDVlMEy/8h9oSiOnDcDmHHU+g+ kvxYTRfXbvIWCar0JxKTRQqqwIdWJtVBI1fTgB7z65DexgEi6h1b5J2T3x318+dC wKHLL4cP3/og0BGYaIQRAL9LZ+ApaASOGC1BalLB3cDVZCsNPm8ZcWJaSzWm5ERS rq5gPcVY4z7Hg0sWoKu1xeY3h0QB+3zORLmu3v3Xc+mzGZ76ewE1A9K1i7GS81bs CBBqcHXnprMZEzlDsiMfQzW2JU5ZCUI6N9UZ9FSH+o7YrZyIQUiIIOyo4OeLyAnN s7j9q2m43WjOhdgJZFef72V/qQm4vgw5vTc2gPKdCaBNgIe5gXQaagG1kvJo0XRv B82/gxtWXOEmVjFISBb/zauQbzL1sFwv6XuxRPzBDZ3QynZOTIj6xtvll8RqDJfT MJ8n9gBsJC0vvY9UBnAgxskRWKs9bNjvgWEwxyImAcyAVVoIYNw= =Fdmy -----END PGP SIGNATURE----- --PO+9smPesjgdlSL2--