Message-ID: <efab844a-4ffe-bc68-d99e-8688ad222e3a@quicinc.com>
Date:   Mon, 6 Feb 2023 19:12:43 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v2 01/22] rtc: pm8xxx: fix set-alarm race
Content-Language: en-US
To:     Johan Hovold <johan+linaro@kernel.org>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Bjorn Andersson <andersson@kernel.org>
CC:     Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Alessandro Zummo <a.zummo@towertech.it>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Maximilian Luz <luzmaximilian@gmail.com>,
        <linux-arm-msm@vger.kernel.org>, <linux-rtc@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <stable@vger.kernel.org>
References: <20230202155448.6715-1-johan+linaro@kernel.org>
 <20230202155448.6715-2-johan+linaro@kernel.org>
From:   David Collins <quic_collinsd@quicinc.com>
In-Reply-To: <20230202155448.6715-2-johan+linaro@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/2/23 07:54, Johan Hovold wrote:
> Make sure to disable the alarm before updating the four alarm time
> registers to avoid spurious alarms during the update.

What scenario can encounter a spurious alarm triggering upon writing the
new alarm time inside of pm8xxx_rtc_set_alarm()?

> Note that the disable needs to be done outside of the ctrl_reg_lock
> section to prevent a racing alarm interrupt from disabling the newly set
> alarm when the lock is released.

What scenario shows the IRQ race issue that you mentioned?  How does not
protecting this register write with a lock avoid the race condition?

> Fixes: 9a9a54ad7aa2 ("drivers/rtc: add support for Qualcomm PMIC8xxx RTC")
> Cc: stable@vger.kernel.org      # 3.1
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/rtc/rtc-pm8xxx.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)

Note that since locking is removed later in the patch series, my
questions above are mainly for the sake of curiosity.


Reviewed-by: David Collins <quic_collinsd@quicinc.com>

Thanks,
David