ec: i2c_master.c: i2c_lock might call interrupt_disable/enable twice |
|
Issue description
i2c_lock does the following:
interrupt_disable();
i2c_port_active_count++;
/* Ec cannot enter sleep if there's any i2c port active. */
disable_sleep(SLEEP_MASK_I2C_MASTER);
interrupt_enable();
disable_sleep() calls atomic_or()
This should be fine on cortex-m with proper atomic operations.
On cortex-m0 without proper atomic ops, this will call interrupt_disable() a second time, then interrupt_enable() twice in a row:
For example, on hammer:
armv7a-cros-linux-gnueabi-objdump -d build/hammer/RW/ec.RW.elf
080137ac <i2c_lock>:
80137ac: b508 push {r3, lr}
80137ae: 2800 cmp r0, #0
80137b0: db18 blt.n 80137e4 <i2c_lock+0x38>
80137b2: 2900 cmp r1, #0
80137b4: d002 beq.n 80137bc <i2c_lock+0x10>
80137b6: f7ff ffdf bl 8013778 <i2c_lock.part.0.lto_priv.102>
80137ba: e013 b.n 80137e4 <i2c_lock+0x38>
80137bc: b672 cpsid i
80137be: 4a0a ldr r2, [pc, #40] ; (80137e8 <i2c_lock+0x3c>)
80137c0: 6813 ldr r3, [r2, #0]
80137c2: 3b01 subs r3, #1
80137c4: 6013 str r3, [r2, #0]
80137c6: 2b00 cmp r3, #0
80137c8: d106 bne.n 80137d8 <i2c_lock+0x2c>
80137ca: 2204 movs r2, #4
80137cc: 4b07 ldr r3, [pc, #28] ; (80137ec <i2c_lock+0x40>)
80137ce: b672 cpsid i
80137d0: 6819 ldr r1, [r3, #0]
80137d2: 4391 bics r1, r2
80137d4: 6019 str r1, [r3, #0]
80137d6: b662 cpsie i
80137d8: b662 cpsie i
80137da: 4b05 ldr r3, [pc, #20] ; (80137f0 <i2c_lock+0x44>)
80137dc: 00c0 lsls r0, r0, #3
80137de: 18c0 adds r0, r0, r3
80137e0: f7ff fc44 bl 801306c <mutex_unlock>
80137e4: bd08 pop {r3, pc}
80137e6: 46c0 nop ; (mov r8, r8)
80137e8: 20002b34 .word 0x20002b34
80137ec: 20002c60 .word 0x20002c60
80137f0: 20002b24 .word 0x20002b24
Probably harmless anyway, but we might still want to fix this.
,
Mar 23 2018
This why I think we need a flag save w.r.t. interrupt_disable(). That or keep a per task count for interrupt_disable()/interrupt_enable() nesting.
,
Mar 23 2018
Yes. It should be uint32_t prev_state = interrupt_disable(); interrupt_restore(prev_state); |
|
►
Sign in to add a comment |
|
Comment 1 by vpalatin@google.com
, Mar 23 2018