Re: [PATCH v6 0/3] Timer support for ARM Tegra
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 пише:
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 = ,
,
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
Re: [PATCH v6 0/3] Timer support for ARM Tegra
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 пише:
>>>
>>> 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 = ,
>>> ,
>>> 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
Re: [PATCH v6 0/3] Timer support for ARM Tegra
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 пише:
>
> 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
> >>> пише:
>
> 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 = ,
> ,
> 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
RE: [PATCH v6 0/3] Timer support for ARM Tegra
Using your existing patches and generating a v7 to fix the T210 boards sounds like the right approach to me. Tom -Original Message- From: Svyatoslav Ryhel Sent: Thursday, January 26, 2023 11:11 AM To: Thierry Reding Cc: Tom Warren ; Rayagonda Kokatanur ; 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 External email: Use caution opening links or attachments чт, 26 січ. 2023 р. о 19:58 Thierry Reding пише: > > On Thu, Jan 26, 2023 at 07:12:34PM +0200, Svyatoslav Ryhel wrote: > > I may implement changes of Thierry Reding in a proper form as a > > separate patch or include in existing (depends on his choice). > > I think it's ultimately better if this is properly integrated into the > series because the series would remain bisectible. > > If the existing series is applied as-is, we have a few patches in the > middle during which Tegra210 is unusable. So if we ever have to track > down a regression that might be problematic. > > It's not a big deal since we've rarely had regressions in U-Boot. It's > ultimately up to Tom to decide how he wants to handle this. > > If you send out another series, can you please add me on Cc so I don't > miss it? > > Thanks, > Thierry Thierry Reding thanks for your clarification. If you and Tom Warren are ok, I will modify existing patches and send them as v7 in final form. Then you can check if T210 works as intended and if everything is correct. We can apply it. P. S. You may be sure that I will include you in all my patches for u-boot since it is explicitly hard to find a person with boards you own. Best Regards. Svyatoslav R. > > The only thing I need to know is if ALL known T210 devices use > > 19.2MHz as calibration clock for timer? > > > > Best regards. > > Svyatoslav R. > > > > чт, 26 січ. 2023 р. о 18:50 Tom Warren пише: > > > > > > Thanks for testing T210/T186, Thierry. > > > > > > I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need > > > either a patch from you for the T210 changes that I can apply on top of > > > his, or I'll need Svyatoslav to adopt your changes as a 4th patch in his > > > series. Once I have that and can pass buildman OK, I'll be ready to send > > > a PR to TomR. > > > > > > Tom > > > > > > -Original Message- > > > From: Thierry Reding > > > Sent: Thursday, January 26, 2023 4:41 AM > > > 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 > > > > > > On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote: > > > > 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
Re: [PATCH v6 0/3] Timer support for ARM Tegra
I propose this solution, based on Thierry idea.
diff --git a/drivers/timer/tegra-timer.c b/drivers/timer/tegra-timer.c
index d2d163cf3fef..86abc0e8fa54 100644
--- a/drivers/timer/tegra-timer.c
+++ b/drivers/timer/tegra-timer.c
@@ -94,6 +94,15 @@ static int tegra_timer_probe(struct udevice *dev)
return -EINVAL;
}
+ /*
+ * T210 TIMERUS is calibrated using clk_m which is
+ * div2 of osc clock. All known T210 boards use
+ * 38.4 MHz osc clock which means clk_m should be
+ * 19.2 MHz, hence correct usec_config is applied.
+ */
+ if (IS_ENABLED(CONFIG_TEGRA210))
+ usec_config = 0x045f; /* (95+1)/(4+1) */
+
/* Enable clock to timer hardware */
value = readl_relaxed(TEGRA_OSC_CLK_ENB_L_SET);
writel_relaxed(value | TEGRA_OSC_SET_CLK_ENB_TMR,
@@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = {
{ .compatible = "nvidia,tegra30-timer" },
{ .compatible = "nvidia,tegra114-timer" },
{ .compatible = "nvidia,tegra124-timer" },
+ { .compatible = "nvidia,tegra210-timer" },
{ }
};
In case if T210 board with non 38.4MHz exists we can get back to this
for proper and exact fix.
Best Regards.
Svyatoslav R.
чт, 26 січ. 2023 р. о 19:58 Thierry Reding пише:
>
> On Thu, Jan 26, 2023 at 07:12:34PM +0200, Svyatoslav Ryhel wrote:
> > I may implement changes of Thierry Reding in a proper form as a
> > separate patch or include in existing (depends on his choice).
>
> I think it's ultimately better if this is properly integrated into the
> series because the series would remain bisectible.
>
> If the existing series is applied as-is, we have a few patches in the
> middle during which Tegra210 is unusable. So if we ever have to track
> down a regression that might be problematic.
>
> It's not a big deal since we've rarely had regressions in U-Boot. It's
> ultimately up to Tom to decide how he wants to handle this.
>
> If you send out another series, can you please add me on Cc so I don't
> miss it?
>
> Thanks,
> Thierry
>
> > The only thing I need to know is if ALL known T210 devices use
> > 19.2MHz as calibration clock for timer?
> >
> > Best regards.
> > Svyatoslav R.
> >
> > чт, 26 січ. 2023 р. о 18:50 Tom Warren пише:
> > >
> > > Thanks for testing T210/T186, Thierry.
> > >
> > > I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need
> > > either a patch from you for the T210 changes that I can apply on top of
> > > his, or I'll need Svyatoslav to adopt your changes as a 4th patch in his
> > > series. Once I have that and can pass buildman OK, I'll be ready to send
> > > a PR to TomR.
> > >
> > > Tom
> > >
> > > -Original Message-
> > > From: Thierry Reding
> > > Sent: Thursday, January 26, 2023 4:41 AM
> > > 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
> > >
> > > On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> > > > 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.
> > > > > >
> > > > >
Re: [PATCH v6 0/3] Timer support for ARM Tegra
чт, 26 січ. 2023 р. о 19:58 Thierry Reding пише: > > On Thu, Jan 26, 2023 at 07:12:34PM +0200, Svyatoslav Ryhel wrote: > > I may implement changes of Thierry Reding in a proper form as a > > separate patch or include in existing (depends on his choice). > > I think it's ultimately better if this is properly integrated into the > series because the series would remain bisectible. > > If the existing series is applied as-is, we have a few patches in the > middle during which Tegra210 is unusable. So if we ever have to track > down a regression that might be problematic. > > It's not a big deal since we've rarely had regressions in U-Boot. It's > ultimately up to Tom to decide how he wants to handle this. > > If you send out another series, can you please add me on Cc so I don't > miss it? > > Thanks, > Thierry Thierry Reding thanks for your clarification. If you and Tom Warren are ok, I will modify existing patches and send them as v7 in final form. Then you can check if T210 works as intended and if everything is correct. We can apply it. P. S. You may be sure that I will include you in all my patches for u-boot since it is explicitly hard to find a person with boards you own. Best Regards. Svyatoslav R. > > The only thing I need to know is if ALL known T210 devices use > > 19.2MHz as calibration clock for timer? > > > > Best regards. > > Svyatoslav R. > > > > чт, 26 січ. 2023 р. о 18:50 Tom Warren пише: > > > > > > Thanks for testing T210/T186, Thierry. > > > > > > I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need > > > either a patch from you for the T210 changes that I can apply on top of > > > his, or I'll need Svyatoslav to adopt your changes as a 4th patch in his > > > series. Once I have that and can pass buildman OK, I'll be ready to send > > > a PR to TomR. > > > > > > Tom > > > > > > -Original Message- > > > From: Thierry Reding > > > Sent: Thursday, January 26, 2023 4:41 AM > > > 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 > > > > > > On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote: > > > > 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 > > > > >
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Thu, Jan 26, 2023 at 07:12:34PM +0200, Svyatoslav Ryhel wrote:
> I may implement changes of Thierry Reding in a proper form as a
> separate patch or include in existing (depends on his choice).
I think it's ultimately better if this is properly integrated into the
series because the series would remain bisectible.
If the existing series is applied as-is, we have a few patches in the
middle during which Tegra210 is unusable. So if we ever have to track
down a regression that might be problematic.
It's not a big deal since we've rarely had regressions in U-Boot. It's
ultimately up to Tom to decide how he wants to handle this.
If you send out another series, can you please add me on Cc so I don't
miss it?
Thanks,
Thierry
> The only thing I need to know is if ALL known T210 devices use
> 19.2MHz as calibration clock for timer?
>
> Best regards.
> Svyatoslav R.
>
> чт, 26 січ. 2023 р. о 18:50 Tom Warren пише:
> >
> > Thanks for testing T210/T186, Thierry.
> >
> > I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need
> > either a patch from you for the T210 changes that I can apply on top of
> > his, or I'll need Svyatoslav to adopt your changes as a 4th patch in his
> > series. Once I have that and can pass buildman OK, I'll be ready to send a
> > PR to TomR.
> >
> > Tom
> >
> > -Original Message-
> > From: Thierry Reding
> > Sent: Thursday, January 26, 2023 4:41 AM
> > 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
> >
> > On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> > > 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:
> > >
> > > --- >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",
> > > +&
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Thu, Jan 26, 2023 at 07:08:54PM +0200, Svyatoslav Ryhel wrote:
> чт, 26 січ. 2023 р. о 12:35 Thierry Reding пише:
> >
> > 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 = ,
> > ,
> > 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
Re: [PATCH v6 0/3] Timer support for ARM Tegra
чт, 26 січ. 2023 р. о 12:35 Thierry Reding пише:
>
> 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.
> reg = <0x0 0x60005000 0x0 0x400>;
> interrupts = ,
> ,
> 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?
If yes I can set this change in simpler as a separate commit or
including into existing patches.
> /*
> * Configure microsecond timers to have 1MHz clock
> * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
> * Uses n+1 scheme
> */
> - switch (clock_get_osc_freq()) {
> + switch (freq) {
> case CLOCK_OSC_FREQ_13_0:
> usec_config = 0x000c; /* (12+1)/(0+1) */
> break;
> @@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = {
> { .compatible = "nvidia,tegra30-timer" },
> { .compatible = "nvidia,tegra114-timer" },
> { .compatible = "nvidia,tegra124-timer" },
> + { .compatible = "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
Re: [PATCH v6 0/3] Timer support for ARM Tegra
I may implement changes of Thierry Reding in a proper form as a
separate patch or include in existing (depends on his choice).
The only thing I need to know is if ALL known T210 devices use
19.2MHz as calibration clock for timer?
Best regards.
Svyatoslav R.
чт, 26 січ. 2023 р. о 18:50 Tom Warren пише:
>
> Thanks for testing T210/T186, Thierry.
>
> I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need
> either a patch from you for the T210 changes that I can apply on top of his,
> or I'll need Svyatoslav to adopt your changes as a 4th patch in his series.
> Once I have that and can pass buildman OK, I'll be ready to send a PR to TomR.
>
> Tom
>
> -Original Message-
> From: Thierry Reding
> Sent: Thursday, January 26, 2023 4:41 AM
> 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
>
> On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> > 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:
> >
> > --- >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";
> > reg = <0x0 0x60005000 0x0 0x400>;
> > interrupts = ,
> >, 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) {
> >
RE: [PATCH v6 0/3] Timer support for ARM Tegra
Thanks for testing T210/T186, Thierry.
I had Svyatoslav's patches ready to go for a PR yesterday, so I'll need either
a patch from you for the T210 changes that I can apply on top of his, or I'll
need Svyatoslav to adopt your changes as a 4th patch in his series. Once I have
that and can pass buildman OK, I'll be ready to send a PR to TomR.
Tom
-Original Message-
From: Thierry Reding
Sent: Thursday, January 26, 2023 4:41 AM
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
On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> 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:
>
> --- >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";
> reg = <0x0 0x60005000 0x0 0x400>;
> interrupts = ,
>, 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;
> +
> /*
>* Configure microsecond timers to have 1MHz clock
>* Config register is 0xqqww, where qq is "dividend", ww is "divisor
Re: [PATCH v6 0/3] Timer support for ARM Tegra
On Thu, Jan 26, 2023 at 11:34:59AM +0100, Thierry Reding wrote:
> 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:
>
> --- >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";
> reg = <0x0 0x60005000 0x0 0x400>;
> interrupts = ,
>,
> 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;
> +
> /*
>* Configure microsecond timers to have 1MHz clock
>* Config register is 0xqqww, where qq is "dividend", ww is "divisor"
>* Uses n+1 scheme
>*/
> - switch (clock_get_osc_freq()) {
> + switch (freq) {
> case CLOCK_OSC_FREQ_13_0:
> usec_config = 0x000c; /* (12+1)/(0+1) */
> break;
> @@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = {
> { .compatible = "nvidia,tegra30-timer" },
> { .compatible = "nvidia,tegra114-timer" },
> { .compatible = "nvidia,tegra124-timer" },
> + { .compatible = "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
I've also tested your series with the above on Tegra30 (Beaver) and
Tegra124 (Jetson TK1), both seem to work fine.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v6 0/3] Timer support for ARM Tegra
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:
--- >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";
reg = <0x0 0x60005000 0x0 0x400>;
interrupts = ,
,
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;
+
/*
* Configure microsecond timers to have 1MHz clock
* Config register is 0xqqww, where qq is "dividend", ww is "divisor"
* Uses n+1 scheme
*/
- switch (clock_get_osc_freq()) {
+ switch (freq) {
case CLOCK_OSC_FREQ_13_0:
usec_config = 0x000c; /* (12+1)/(0+1) */
break;
@@ -113,6 +122,7 @@ static const struct udevice_id tegra_timer_ids[] = {
{ .compatible = "nvidia,tegra30-timer" },
{ .compatible = "nvidia,tegra114-timer" },
{ .compatible = "nvidia,tegra124-timer" },
+ { .compatible = "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
signature.asc
Description: PGP signature
Re: [PATCH v6 0/3] Timer support for ARM Tegra
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. Thierry > > arch/arm/Kconfig| 1 + > arch/arm/include/asm/arch-tegra/clock.h | 9 +- > arch/arm/mach-tegra/Kconfig | 2 + > arch/arm/mach-tegra/clock.c | 17 +++- > arch/arm/mach-tegra/cpu.c | 70 ++--- > arch/arm/mach-tegra/tegra114/clock.c| 13 +-- > arch/arm/mach-tegra/tegra124/clock.c| 13 +-- > arch/arm/mach-tegra/tegra20/clock.c | 4 +- > arch/arm/mach-tegra/tegra210/clock.c| 22 + > arch/arm/mach-tegra/tegra30/clock.c | 10 +- > drivers/timer/Kconfig | 8 ++ > drivers/timer/Makefile | 1 + > drivers/timer/tegra-timer.c | 126 > drivers/usb/host/ehci-tegra.c | 46 +++-- > include/configs/tegra-common.h | 6 -- > 15 files changed, 269 insertions(+), 79 deletions(-) > create mode 100644 drivers/timer/tegra-timer.c > > -- > 2.37.2 > signature.asc Description: PGP signature
[PATCH v6 0/3] Timer support for ARM Tegra
- 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 arch/arm/Kconfig| 1 + arch/arm/include/asm/arch-tegra/clock.h | 9 +- arch/arm/mach-tegra/Kconfig | 2 + arch/arm/mach-tegra/clock.c | 17 +++- arch/arm/mach-tegra/cpu.c | 70 ++--- arch/arm/mach-tegra/tegra114/clock.c| 13 +-- arch/arm/mach-tegra/tegra124/clock.c| 13 +-- arch/arm/mach-tegra/tegra20/clock.c | 4 +- arch/arm/mach-tegra/tegra210/clock.c| 22 + arch/arm/mach-tegra/tegra30/clock.c | 10 +- drivers/timer/Kconfig | 8 ++ drivers/timer/Makefile | 1 + drivers/timer/tegra-timer.c | 126 drivers/usb/host/ehci-tegra.c | 46 +++-- include/configs/tegra-common.h | 6 -- 15 files changed, 269 insertions(+), 79 deletions(-) create mode 100644 drivers/timer/tegra-timer.c -- 2.37.2
