linux-kernel - Re: [PATCH v3 4/4] MIPS: CI20: Add second percpu timer for SMP.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <67L9VQ.H1SRDC272GKW@crapouillou.net>
Date:   Fri, 25 Jun 2021 16:47:30 +0100
From:   Paul Cercueil <paul@...pouillou.net>
To:     周琰杰 <zhouyanjie@...yeetech.com>
Cc:     tsbogend@...ha.franken.de, mturquette@...libre.com,
        sboyd@...nel.org, robh+dt@...nel.org, linux-mips@...r.kernel.org,
        devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, dongsheng.qiu@...enic.com,
        aric.pzqi@...enic.com, rick.tyliu@...enic.com,
        sihui.liu@...enic.com, jun.jiang@...enic.com,
        sernia.zhou@...mail.com
Subject: Re: [PATCH v3 4/4] MIPS: CI20: Add second percpu timer for SMP.

Hi Zhou,

Le ven., juin 25 2021 at 23:19:42 +0800, 周琰杰 
<zhouyanjie@...yeetech.com> a écrit :
> Hi Paul,
> 
> 于 Fri, 25 Jun 2021 12:31:17 +0100
> Paul Cercueil <paul@...pouillou.net> 写道:
> 
>>  Hi Zhou,
>> 
>>  Le jeu., juin 24 2021 at 23:06:29 +0800, 周琰杰 (Zhou Yanjie)
>>  <zhouyanjie@...yeetech.com> a écrit :
>>  > 1.Add a new TCU channel as the percpu timer of core1, this is to
>>  >   prepare for the subsequent SMP support. The newly added channel
>>  >   will not adversely affect the current single-core state.
>>  > 2.Adjust the position of TCU node to make it consistent with the
>>  >   order in jz4780.dtsi file.
>> 
>>  That's a bit superfluous, the order matters when adding new nodes,
>>  but once they are added, moving them around only cause annoyance.
>> 
>>  >
>>  > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>>  > ---
>>  >
>>  > Notes:
>>  >     v2:
>>  >     New patch.
>>  >
>>  >     v2->v3:
>>  >     No change.
>>  >
>>  >  arch/mips/boot/dts/ingenic/ci20.dts | 21 +++++++++++----------
>>  >  1 file changed, 11 insertions(+), 10 deletions(-)
>>  >
>>  > diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
>>  > b/arch/mips/boot/dts/ingenic/ci20.dts
>>  > index 8877c62..70005cc 100644
>>  > --- a/arch/mips/boot/dts/ingenic/ci20.dts
>>  > +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>>  > @@ -118,6 +118,17 @@
>>  >  	assigned-clock-rates = <48000000>;
>>  >  };
>>  >
>>  > +&tcu {
>>  > +	/*
>>  > +	 * 750 kHz for the system timers and 3 MHz for the
>>  > clocksources,
>>  > +	 * use channel #0 and #1 for the per cpu system timers,
>>  > and use
>>  > +	 * channel #2 for the clocksource.
>>  > +	 */
>>  > +	assigned-clocks = <&tcu TCU_CLK_TIMER0>, <&tcu
>>  > TCU_CLK_TIMER1>,
>>  > +					  <&tcu TCU_CLK_TIMER2>,
>>  > <&tcu TCU_CLK_OST>;
>>  > +	assigned-clock-rates = <750000>, <750000>, <3000000>,
>>  > <3000000>;
>> 
>>  Ideally you'd set TIMER1 to 3 MHz and TIMER2 to 750 kHz, otherwise 
>> it
>>  kind of breaks support for older kernels (they would still boot, but
>>  with a very slow clocksource). So in the new DTS you could use the
>>  timer0 clock for CPU #0, timer1 for the clocksource, and timer2+ for
>>  cpus > 0.
> 
> I checked the ingenic-timer driver, and it seems that the last TCU
> channel is always used as the clocksource in the driver, so it seems
> that we can only use timer2 as the clocksource in smp mode. Maybe we
> should add a note for smp is closed in the comment. And I found that I
> missed a problem, Nikolaus Schaller once reported that because the
> frequency of the tcu timer (only 16bit) used to provide the 
> clocksource
> is too high, there will be a chance that the system will get stuck
> before the clocksource is switched to ost. And reducing the 
> clocksource
> to 750kz can prevent it from happening. I will add this part to v4.
> When this part is added, both clockevent and clocksource will be
> 750kHz, but the 750kHz clocksource is only temporary, because it will
> then switch to the clocksource provided by ost, and ost works at 3MHz.

Ok, then first change the clocksource to 750 kHz, then update it with 
timer2.

Cheers,
-Paul

> 
> Thanks and best regards!
> 
>> 
>>  Cheers,
>>  -Paul
>> 
>>  > +};
>>  > +
>>  >  &mmc0 {
>>  >  	status = "okay";
>>  >
>>  > @@ -522,13 +533,3 @@
>>  >  		bias-disable;
>>  >  	};
>>  >  };
>>  > -
>>  > -&tcu {
>>  > -	/*
>>  > -	 * 750 kHz for the system timer and 3 MHz for the
>>  > clocksource,
>>  > -	 * use channel #0 for the system timer, #1 for the
>>  > clocksource.
>>  > -	 */
>>  > -	assigned-clocks = <&tcu TCU_CLK_TIMER0>, <&tcu
>>  > TCU_CLK_TIMER1>,
>>  > -					  <&tcu TCU_CLK_OST>;
>>  > -	assigned-clock-rates = <750000>, <3000000>, <3000000>;
>>  > -};
>>  > --
>>  > 2.7.4
>>  >
>> 
>