frameworkcomputer / embeddedcontroller Goto Github PK

The instructions in the README only covers building the EC firmware for 11th Gen Core Processors. Could you provide instructions for the upcoming laptops which includes 12th Gen Processors

A firmware image built with GCC 11 or higher will fail to start the AP due to a watchdog timeout processing an ESPI interrupt.

It may be worthwhile to add a note somewhere to this effect.

Why?

The loop here ...

... iterates on the set bits in girq24_result (there is a similar loop below for girq25_result) by using __builtin_ctz to avoid checking each bit.

This compiles into rbit and clz.

clz is specified as returning 32 when no bits are set, e.g. when the value is 0.

In contrast, __builtin_c[lt]z are specified as undefined when the value is 0.

In short, it's relying on undefined behavior!

In GCC 10, we got by with a pass; the code compiled into rbit and clz, the comparison was kept, and everything chugged along happily.

With GCC 11, the optimizer takes advantage of this undefined behavior to remove the comparison (after all: there are not 33 bits in a uint32_t, and the return value after we've cleared all the bits, i.e. x==0, is undefined).

000e7e90 <espi_mswv1_interrupt>:
   e7e90:       e92d 41f0       stmdb   sp!, {r4, r5, r6, r7, r8, lr}
   e7e94:       4b12            ldr     r3, [pc, #72]   ; (e7ee0 <espi_mswv1_interrupt+0x50>)
   e7e96:       4f13            ldr     r7, [pc, #76]   ; (e7ee4 <espi_mswv1_interrupt+0x54>)
   e7e98:       f8d3 2144       ldr.w   r2, [r3, #324]  ; 0x144
   e7e9c:       f8d3 5148       ldr.w   r5, [r3, #328]  ; 0x148
   e7ea0:       4e11            ldr     r6, [pc, #68]   ; (e7ee8 <espi_mswv1_interrupt+0x58>)
   e7ea2:       f8c3 5140       str.w   r5, [r3, #320]  ; 0x140
   e7ea6:       fa95 f4a5       rbit    r4, r5
   e7eaa:       fab4 f484       clz     r4, r4
   e7eae:       f04f 080c       mov.w   r8, #12
// The comparison would fall roughly here; in GCC 10, it does:
///#####:       2c20            cmp     r4, #32
///#####:       d101            bne.n   ##### <espi_mswv1_interrupt+0x##>
///#####:       e8bd 81f0       ldmia.w sp!, {r4, r5, r6, r7, r8, pc}
   e7eb2:       08a2            lsrs    r2, r4, #2
   e7eb4:       f004 0303       and.w   r3, r4, #3
   e7eb8:       fb08 3302       mla     r3, r8, r2, r3
   e7ebc:       4621            mov     r1, r4
   e7ebe:       5dd8            ldrb    r0, [r3, r7]
   e7ec0:       eb06 0384       add.w   r3, r6, r4, lsl #2
   e7ec4:       f000 0001       and.w   r0, r0, #1
   e7ec8:       f8d3 30b8       ldr.w   r3, [r3, #184]  ; 0xb8
   e7ecc:       4798            blx     r3    
   e7ece:       2301            movs    r3, #1
   e7ed0:       40a3            lsls    r3, r4                                                                                                                                               
   e7ed2:       ea25 0503       bic.w   r5, r5, r3
   e7ed6:       fa95 f4a5       rbit    r4, r5
   e7eda:       fab4 f484       clz     r4, r4
   e7ede:       e7e8            b.n     e7eb2 <espi_mswv1_interrupt+0x22>
   e7ee0:       4000e000        .word   0x4000e000
   e7ee4:       400f9c08        .word   0x400f9c08
   e7ee8:       000fcb70        .word   0x000fcb70

This results in an infinite loop with bpos == 32, and the following message printed to the console until the watchdog kicks us out:

Unknown M2S VW: state=0 GIRQ25 bitpos=32
                                      ^^ oops

Note
I know that this code originated in the upstream repository, and I have already notified a couple folks over there! 😄

As in title; unlike previous revisions of the EC, commit 9450caa doesn't seem to exist here.

Sometimes when the system is in sleep mode, pressing the power button doesn't seem to do anything, the Power LED continues pulsing as it does in sleep mode. The keyboard backlight seemed to be on.

• Intel Gen12, BIOS 3.06
• System was put into sleep mode by Linux 6.6.2 (Archlinux)

What additional information would be helpful?

If necessary I could probably also attach UART and JTAG/SWD, but since I'm using this laptop productively I'd prefer to avoid that.

Filing this as a feature request!

It looks like you planned on having a configurable min percentage for the charge limit, presumably to allow the battery to fall to a certain percentage before resuming charging.

Depending on how similar the hx* boards are, it may be possible--and perhaps advantageous!--to unify their implementations as a baseboard (see the baseboard directory for other examples.)

That would make it easier to abstract, say, the input cover/key layout and PS/2 emulation bits and drive bug fixes to both platforms simultaneously. Similar thoughts about the UCSI implementation, if the hx20/30 share one of those.

When playing music/youtube/whatever with the screen off as I go to sleep, the power LED lights up the whole room. Options to dim it were added here, but the ability to disable it entirely would be appreciated. This has also been mentioned in the discussion thread.

Putting electrical tape over a button gets a bit messier than putting it over the charge lights, and I haven't got up the courage to start cutting on the board just yet :)

I cloned the repo and ran make utils . ; but got:

EmbeddedController$ make utils .
make: pkg-config: Aucun fichier ou dossier de ce type                      // No files of this type
make: pkg-config: Aucun fichier ou dossier de ce type
  VERSION ec_version.h
  HOSTCC  util/ectool
  HOSTCC  util/lbplay
  HOSTCC  util/stm32mon
  HOSTCC  util/ec_sb_firmware_update
  HOSTCC  util/lbcc
  HOSTCC  util/ec_parse_panicinfo
  HOSTCC  util/cbi-util
  HOSTCC  util/uartupdatetool
  BUILDCC util/ec_uartd
util/ec_uartd.c:24:10: fatal error: ftdi.h: Aucun fichier ou dossier de ce type          // No file of this type
   24 | #include <ftdi.h>
      |          ^~~~~~~~
compilation terminated.
make: *** [Makefile.rules:614 : build/bds/util/ec_uartd] Erreur 1    // Error 1

on Ubuntu 22.04.2 LTS, with libftdi1-2 installed (libftdi1-2/jammy,now 1.5-5build3 amd64).

Querying the charge limit with host command CHARGE_LIMIT_CONTROL mode CHG_LIMIT_GET_LIMIT reloads the charge limit from bbram, which overwrites the CHG_LIMIT_OVERRIDE flag.

Flag is set here (and not stored to bbram, because it is a one-time override):

and cleared when we read into charging_maximum_level here:

The ADC channels ADC_AUDIO_BOARD_ID and ADC_TP_BOARD_ID are ostensibly used to detect the board revision for the input cover and audio daughterboard.

They are only used in diagnostics.c here (and slightly above, where their raw values are gathered):

get_hardware_id takes one argument, channel:

However, get_hardware_id ignores its argument and always reads ADC_AD_BID:

The easiest way I've found to reproduce this is by:

1. Pressing Vol Down
2. Pressing Vol Up
3. Releasing Vol Down

If done properly, releasing Vol Up will result in a break code for F3 being generated. This can also be accomplished with some combination of Fn and the arrow/page keys.

It appears that the state-tracking variables keep_fn_* aren't transitioning through their state graph correctly.

On Fedora 35, I was able to build like this.

With the compiler arm-none-eabi-gcc.

$ sudo dnf install arm-none-eabi-gcc-cs libftdi-devel

$ which arm-none-eabi-gcc
/bin/arm-none-eabi-gcc

$ make BOARD=hx20 CROSS_COMPILE=arm-none-eabi-

With the compiler arm-linux-gnu-gcc.

$ sudo dnf install gcc-arm-linux-gnu libftdi-devel

$ which arm-linux-gnu-gcc
/bin/arm-linux-gnu-gcc

$ make BOARD=hx20 CROSS_COMPILE=arm-linux-gnu-

If you don't mind, I would like to add the command to install arm-none-eabi-gcc on README.md.

Hi

Would it at all be possible for support for 13th gen intel to be added/documentation updated to provide the entry point for 13th gen?

Thanks!

See the relevant forum thread. It seem like multiple wakeup events are sent, from the lid switch and the keyboard (at least for the lid closing, unsure about the charging thing). This seems to cause operating systems to cancel sleep, i.e. closing the lid of a sleeping laptop wakes it up and so does plugging in the charger. Someone from the Framwork team already has a theory about the code which might case this issue:

I noticed this repository doesn't have any tag info.

$ git tag -n

I can see the tag info on the upstream chromium ec repository.

$ git remote -v
origin	https://chromium.googlesource.com/chromiumos/platform/ec (fetch)
origin	https://chromium.googlesource.com/chromiumos/platform/ec (push)

$ git tag -n | tail
v1.9308_B.0     firmware branch v1.9308_B.0
v1.9311_70_mp   firmware branch v1.9311_70_mp
v1.9311_mp      firmware branch v1.9311_mp
v2.0.0          Adding a new tag to reset revision count
v2.1.0          Adding a new tag to reset revision count in the firmware-grunt-11031.B branch
v2.14294_prepvt.0 firmware branch v2.14294_prepvt.0
v2.2.0          Adding a new tag to reset revision count in the firmware-nocturne-10984.B
v2.3.0          Adding a new tag to reset revision count in the firmware-servo-11011.B
v2.4.0          Adding a new tag to reset revision count
v2.94_pp.0      firmware branch v2.94_pp.0

I expected there are the tags "v3.2", "v3.6", "v3.7" used to ship BIOS version 3.02, 3.06, 3.07 and etc on this repository. Is the missing git tag info intentional?

Calling the FP_LED_LEVEL_CONTROL host command with set requires one of the symbolic LED brightnesses:

• FP_LED_BRIGHTNESS_HIGH = 0
• FP_LED_BRIGHTNESS_MEDIUM = 1
• FP_LED_BRIGHTNESS_LOW = 2

However, calling it with get populated returns the actual stored brightness value:

• high = 55 (FP_LED_HIGH)
• medium = 40 (FP_LED_MEDIUM)
• low = 15 (FP_LED_LOW)

### Write brightness 2 (low)
# ectool raw 0x3e0e b2,b0
3e0e(...2 bytes...)
 02 00                                           |..              |

### Read back (low = 15)
# ectool raw 0x3e0e b0,b1
3e0e(...2 bytes...)
 00 01                                           |..              |
Read 1 bytes
 0f                                              |.               |

I think the code here is wrong as of 42ec4cc.

mode = mode << 4 will produce out-of-range values for the AC modes: EC_AC_BEST_PERFORMANCE will become 256 (16 << 4), EC_AC_BALANCED (32) will become 512, etc.

Reading the condition, it looks like we want to treat low-power AC (AC < 55W) as the same as DC.
I would expect that to downgrade EC_AC_BEST_PERFORMANCE to EC_**DC**_BEST_PERFORMANCE (as an example).

If that's the case, I suspect mode >> 4 is what you want. That reduces EC_AC_BEST_PERFORMANCE to 1 (16 >> 4)... which happens to match the value for EC_DC_BEST_...