On Thu, 2024-08-01 at 16:21 +0200, Thomas Gleixner wrote:
> On Tue, Feb 07 2023 at 09:14, lirongqing@xxxxxxxxx wrote:
> > @@ -117,11 +110,6 @@ static int pit_shutdown(struct clock_event_device *evt)
> >
> > outb_p(0x30, PIT_MODE);
> >
> > - if (i8253_clear_counter_on_shutdown) {
> > - outb_p(0, PIT_CH0);
> > - outb_p(0, PIT_CH0);
> > - }
> > -
>
> The stop sequence is wrong:
>
> When there is a count in progress, writing a new LSB before the
> counter has counted down to 0 and rolled over to FFFFh, WILL stop
> the counter. However, if the LSB is loaded AFTER the counter has
> rolled over to FFFFh, so that an MSB now exists in the counter, then
> the counter WILL NOT stop.
>
> The original i8253 datasheet says:
>
> 1) Write 1st byte stops the current counting
> 2) Write 2nd byte starts the new count
It says that for mode zero ("Interrupt on Terminal Count"), yes. But in
that mode, shouldn't the IRQ only fire *one* more time anyway, rather
than repeatedly? That should be OK, shouldn't it?
"When terminal count is reached, the output will go high and remain
high until the selected count register is reloaded wityh the mode or a
new count is loaded".
It's OK for it to keep *counting* as long as it stops firing
interrupts, isn't it?
Either way, this is somewhat orthogonal to the patch I posted in
https://lore.kernel.org/kvm/6cd62b5058e11a6262cb2e798cc85cc5daead3b1.camel@xxxxxxxxxxxxx/T/#u
for the fact that we don't shut down the PIT at *all* if we aren't ever
going to use it.
I'm glad I decided to export a function from the clocksource driver and
just *call* it from pit_timer_init() though. Means we can bikeshed the
shutdown sequence in *one* place and it isn't duplicated.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
