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
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 last patch of the three:
ec5895c: rtc: mc146818-lib: extract mc146818_avoid_UIP
introduces only a new function; it won't apply cleanly over kernel 5.4,
but this is straightforward to fix.
Greetings,
Mateusz
