Re: [Xen-devel] [PATCH RFC 3/3] x86/hvm/rtc: Always deassert the IRQ line when clearing REG_C.IRQF.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.02.14 at 18:32, Tim Deegan <tim@xxxxxxx> wrote:
> Even in no-ack mode, there's no reason to leave the line asserted
> after an explicit ack of the interrupt.
> 
> Signed-off-by: Tim Deegan <tim@xxxxxxx>
> ---
>  xen/arch/x86/hvm/rtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c
> index 18a4fe8..b592547 100644
> --- a/xen/arch/x86/hvm/rtc.c
> +++ b/xen/arch/x86/hvm/rtc.c
> @@ -674,7 +674,7 @@ static uint32_t rtc_ioport_read(RTCState *s, uint32_t 
> addr)
>          check_for_pf_ticks(s);
>          ret = s->hw.cmos_data[s->hw.cmos_index];
>          s->hw.cmos_data[RTC_REG_C] = 0x00;
> -        if ( (ret & RTC_IRQF) && !rtc_mode_is(s, no_ack) )
> +        if ( ret & RTC_IRQF )
>              hvm_isa_irq_deassert(d, RTC_IRQ);
>          rtc_update_irq(s);
>          check_update_timer(s);

Wait - does one of the earlier patches remove the other de-assert?
Looking... No, they don't. Doing it in exactly one of the two places
should be sufficient, shouldn't it? The more that the other de-assert
is in rtc_update_irq(), which is being called right afterwards. 

But then again - that call seems pointless (I think I mentioned this in
response to Andrew's first attempt): Since REG_C is now clear, the
first conditional return path in that function will never be taken, and
the second one always will be.

So I think together with removing that call, and considering the
intended level-ness of the IRQ, I think I agree with you after all,
provided two de-asserts with no assert in between are not a
problem.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel