[Trac #908] EC code, irq stubs do not reset pending flags

[Zarro Boogs per Child]

#908: EC code, irq stubs do not reset pending flags
--------------------+-------------------------------------------------------
 Reporter:  frief   |       Owner:  blizzard 
     Type:  defect  |      Status:  new      
 Priority:  normal  |   Milestone:  Untriaged
Component:  distro  |    Keywords:  EC       
--------------------+-------------------------------------------------------
 some interrupt stubs do not reset the corresponding pending flags.

 This (long) message falls into three parts: a) says the code as it is now
 is not ok.
 b) says the information recorded by the stubs would not survive a
 (watchdog or whatever) reset of the EC and proposes a concept how to keep
 it. c) says the code for the stubs has room for improvement.
 So now for it:


 a)
 It seems like the EC code contains interrupt stubs for unused
 interrupts. While these stubs should never be called they
 should reset their pending flags - otherwise if one of these
 flags should ever be set (by ESD or whatever) the 8051 embedded
 controler sits in a tight loop.

 I would have expected a reset of pending flags at (or around)
 the following locations:

 b28a
 b2b0
 b2d6
 b2fc
 b322
 b346
 b36e
 b394
 b3ba
 b3e0
 b406
 b42c
 b452

 An example which does not reset its pending flag:

 {{{
 irq_lpc:
         push    acc             ; b2a3   c0 e0
         push    dph             ; b2a5   c0 83      @.
         push    dpl             ; b2a7   c0 82      @.
         push    psw             ; b2a9   c0 d0      @P
         mov     psw,#0          ; b2ab   75 d0 00   uP.
         push    rb0r7           ; b2ae   c0 07      @.
         mov     dptr,#Xf505     ; b2b0   90 f5 05   .u.
         movx    a, at dptr         ; b2b3   e0
         mov     r7,a            ; b2b4   ff         .
         mov     a,r7            ; b2b5   ef         o
         orl     a,#20h          ; b2b6   44 20      D
         mov     r7,a            ; b2b8   ff         .
         mov     dptr,#Xf505     ; b2b9   90 f5 05   .u.
         mov     a,r7            ; b2bc   ef         o
         movx    @dptr,a         ; b2bd   f0         p
         pop     rb0r7           ; b2be   d0 07      P.
         pop     psw             ; b2c0   d0 d0      PP
         pop     dpl             ; b2c2   d0 82      P.
         pop     dph             ; b2c4   d0 83      P.
         pop     acc             ; b2c6   d0 e0      P
         reti                    ; b2c8   32         2
 }}}

 The other irq stubs are very similar - they
 set a bit in a bitfield at 0xf504..0xf506 and return.


 Now an example which resets its pending flag:

 {{{
 irq_gpt0:
         push    acc             ; b067   c0 e0      @
         push    dph             ; b069   c0 83      @.
         push    dpl             ; b06b   c0 82      @.
         push    psw             ; b06d   c0 d0      @P
         mov     psw,#0          ; b06f   75 d0 00   uP.
         push    rb0r7           ; b072   c0 07      @.
         mov     dptr,#GPTPF     ; b074   90 fe 51   .~Q  <---- pending irq
         mov     a,#1            ; b077   74 01      t.   <---- flag is
 reset
         movx    @dptr,a         ; b079   f0         p    <---- here -
 fine!
         mov     dptr,#Xf506     ; b07a   90 f5 06   .u.
         movx    a, at dptr         ; b07d   e0
         mov     r7,a            ; b07e   ff         .
         mov     a,r7            ; b07f   ef         o
         orl     a,#10h          ; b080   44 10      D.
         mov     r7,a            ; b082   ff         .
         mov     dptr,#Xf506     ; b083   90 f5 06   .u.
         mov     a,r7            ; b086   ef         o
         movx    @dptr,a         ; b087   f0         p
         pop     rb0r7           ; b088   d0 07      P.
         pop     psw             ; b08a   d0 d0      PP
         pop     dpl             ; b08c   d0 82      P.
         pop     dph             ; b08e   d0 83      P.
         pop     acc             ; b090   d0 e0      P
         reti                    ; b092   32         2
 }}}

 b)
 Note: any spurious irqs these stubs might have recorded are overwritten
 on reset of the embedded controler (see loop at 0xac37).

 I propose to not clear the complete 2kByte xdata region
 upon reset.
 Instead an area of say 16 bytes could be kept over reset. The upper
 8 byte of this area could hold the spurious interrupt information
 (now at 0xf504..0xf506) and eventually the last command received
 from the Geode, voltage or other status info.
 On startup the upper 8 byte of the 16 byte area would be
 copied into the lower 8 byte of the 16 byte area.
 The 16 byte area would be cleared upon reading from the Geode.


 c)
 The code generated for the stubs is longer than needed.
 As no flags are changed and r0..r7 need not be used
 this should do:

 {{{
 irq_gpt0:
         push    acc
         push    dph
         push    dpl
         mov     dptr,#GPTPF
         mov     a,#1
         movx    @dptr,a
         mov     dptr,#Xf506
         movx    a, at dptr
         orl     a,#10h
         movx    @dptr,a
         pop     dpl
         pop     dph
         pop     acc
         reti
 }}}

 I am not sure whether blob based feedback
 makes much sense/is appreciated?

 Greetings,
 Frieder

-- 
Ticket URL: <http://dev.laptop.org/ticket/908>
One Laptop Per Child <http://laptop.org/>