Date:   Sun, 12 Feb 2023 18:19:52 +0100
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc:     agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org,
        djakov@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, linux-arm-msm@vger.kernel.org,
        linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, benl@squareup.com,
        shawn.guo@linaro.org, fabien.parent@linaro.org, leo.yan@linaro.org,
        dmitry.baryshkov@linaro.org, Jun Nie <jun.nie@linaro.org>,
        James Willcox <jwillcox@squareup.com>,
        Joseph Gates <jgates@squareup.com>,
        Max Chen <mchen@squareup.com>, Zac Crosby <zac@squareup.com>,
        Vincent Knecht <vincent.knecht@mailoo.org>
Subject: Re: [PATCH v5 2/5] arm64: dts: qcom: Add msm8939 SoC
Message-ID: <Y+kfuHDMZCu1vKsA@gerhold.net>
References: <20230206012336.2130341-1-bryan.odonoghue@linaro.org>
 <20230206012336.2130341-3-bryan.odonoghue@linaro.org>
 <Y+T5cF4d667RhrJp@gerhold.net>
 <2b6bc14b-20df-b61e-6464-61ebb11f24f0@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2b6bc14b-20df-b61e-6464-61ebb11f24f0@linaro.org>
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sun, Feb 12, 2023 at 04:49:25PM +0000, Bryan O'Donoghue wrote:
> On 09/02/2023 13:47, Stephan Gerhold wrote:
> > > +			frame@b021000 {
> > > +				reg = <0x0b021000 0x1000>,
> > > +				      <0x0b022000 0x1000>;
> > > +				interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> > > +					     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > > +				frame-number = <0>;
> > > +			};
> > These timer interrupts are still wrong like mentioned in v3:
> > https://lore.kernel.org/linux-arm-msm/Y8fC%2FGCHfENQmBNC@gerhold.net/
> > 
> 
> Hmm.
> 
> This is a copy/paste error from 8916 which I guess we never see in our
> production system, since we use LPM to get to do idle
> 

Huh?

We had this discussion several times before and once you mentioned that
Shawn fixed this in your production kernel at some point. You just keep
forgetting to apply the same change to your upstream tree. ;)

You can either change the timer interrupt numbers or (more easily) just
fix the address to point to the timer of the other cluster (which has
the same interrupt numbers as on 8916):

On Mon, Sep 19, 2022 at 03:11:01PM +0100, Bryan O'Donoghue wrote:
> Yep Shawn found that on our internal tree.
>
> commit 91a842b81a713ede9ba76f3957e6fdd9067b5493
> Author: Shawn Guo <shawn.guo@linaro.org>
> Date:   Thu May 28 11:03:40 2020 +0800
>
>     arm64: dts: msm8939: fix base address of memory mapped timer
> 
>     The base address of memory mapped timer is changed from msm8916's
>     0xb020000 to 0xb120000 on msm8939.  Fix it, so that the timer can start
>     working as the broadcast device to wake up cpu from deep idle state.
>
>     Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi
> b/arch/arm64/boot/dts/qcom/msm8939.dtsi
> index dde56c2197eb..907f076b9808 100644
> --- a/arch/arm64/boot/dts/qcom/msm8939.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi
> @@ -1326,61 +1326,61 @@
>                         reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>;
>                 };
> 
> -               timer@b020000 {
> +               timer@b120000 {
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
>                         compatible = "arm,armv7-timer-mem";
> -                       reg = <0xb020000 0x1000>;
> +                       reg = <0xb120000 0x1000>;
>                         clock-frequency = <19200000>;
>
> -                       frame@b021000 {
> +                       frame@b121000 {
> 
> but I see that didn't make it into my working tree