On Fri, Dec 09, 2022 at 11:40:56PM +0100, Mateusz Jończyk wrote:
W dniu 9.12.2022 o 01:37, Sasha Levin pisze:
On Thu, Dec 08, 2022 at 10:47:17PM +0100, Mateusz Jończyk wrote:
W dniu 7.12.2022 o 07:51, Chen, Kane pisze:
-----Original Message-----
From: Sasha Levin <sashal@xxxxxxxxxx>
Sent: Wednesday, December 7, 2022 1:13 PM
To: Chen, Kane <kane.chen@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Subject: Re: [PATCH v1] rtc: cmos: avoid UIP when writing/reading alarm time
On Wed, Dec 07, 2022 at 11:57:22AM +0800, Kane Chen wrote:
While runnings s0ix cycling test based on rtc alarm wakeup on ADL-P
devices, We found the data from CMOS_READ is not reasonable and causes
RTC wake up fail.
With the below changes, we don't see unreasonable data from cmos and
issue is gone.
Thanks for the analysis, I can queue most of these up. There are two which
won't go in:
cd17420: rtc: cmos: avoid UIP when writing alarm time
cdedc45: rtc: cmos: avoid UIP when reading alarm time
ec5895c: rtc: mc146818-lib: extract mc146818_avoid_UIP
ea6fa49: rtc: mc146818-lib: fix RTC presence check
13be2ef: rtc: cmos: Disable irq around direct invocation of
cmos_interrupt()
0dd8d6c: rtc: Check return value from mc146818_get_time()
e1aba37: rtc: cmos: remove stale REVISIT comments
6950d04: rtc: cmos: Replace spin_lock_irqsave with spin_lock in hard
IRQ
This one fixes a commit which isn't in the 5.10 tree.
d35786b: rtc: mc146818-lib: change return values of mc146818_get_time()
ebb22a0: rtc: mc146818: Dont test for bit 0-5 in Register D
211e5db: rtc: mc146818: Detect and handle broken RTCs
dcf257e: rtc: mc146818: Reduce spinlock section in mc146818_set_time()
This one looks like an optimization.
--
I'm sorry,
I thought dcf257e and 6950d04, 13be2ef are also required to avoid cherry-pick conflict
After checking again, dcf257e, 6950d04, 13be2ef are not needed.
Here is the list I would like to pick
cd17420: rtc: cmos: avoid UIP when writing alarm time
cdedc45: rtc: cmos: avoid UIP when reading alarm time
ec5895c: rtc: mc146818-lib: extract mc146818_avoid_UIP
ea6fa49: rtc: mc146818-lib: fix RTC presence check
0dd8d6c: rtc: Check return value from mc146818_get_time()
e1aba37: rtc: cmos: remove stale REVISIT comments
d35786b: rtc: mc146818-lib: change return values of mc146818_get_time()
ebb22a0: rtc: mc146818: Dont test for bit 0-5 in Register D
211e5db: rtc: mc146818: Detect and handle broken RTCs
05a0302: rtc: mc146818: Prevent reading garbage
Thanks a lot
Thanks,
Sasha
Hello,
I think that you should also pick these patches which fix issues in the series
that is prepared for 5.4:
1) commit 7372971c1be5 ("rtc: mc146818-lib: fix signedness bug in mc146818_get_time()")
which fixes d35786b: rtc: mc146818-lib: change return values of mc146818_get_time()
2) commit 13be2efc390a ("rtc: cmos: Disable irq around direct invocation of cmos_interrupt()")
which fixes 6950d04: rtc: cmos: Replace spin_lock_irqsave with spin_lock in hard IRQ
and is not prepared for 5.4 stable even though it was mentioned above.
3) commit 811f5559270f ("rtc: mc146818-lib: fix locking in mc146818_set_time")
which fixes dcf257e: rtc: mc146818: Reduce spinlock section in mc146818_set_time()
Ack, will do, thanks.
Hello,
However, I would like to ask: is it really necessary to include all of
these patches in stable? I think that it is very likely that only these patches
are sufficient to fix the original problem reported by Kane Chen:
cd17420: rtc: cmos: avoid UIP when writing alarm time
cdedc45: rtc: cmos: avoid UIP when reading alarm time
ec5895c: rtc: mc146818-lib: extract mc146818_avoid_UIP
I would agree that this could be enough to fix the issue that was
described, but...
These patches should be most relevant when using the RTC alarm and are
self-contained. So I would like to ask Kane Chen to recheck with these patches
only. Other patches change the CMOS RTC reading routines significantly
and have a higher risk of introducing regressions (but I am not aware of any
outstanding problems).
The rest of the patches do look like fixes that should have been added
to stable (or are dependencies of such fixes) on their own. Given
those patches are pretty old, any reason to not just include them?
The process works better if we address issues before making users come
and complain on the mailing list :)
--
Thanks,
Sasha
