Couple of clang warnings for Rockchip boards

[Simon Glass]

Hi Quentin,

On Thu, 23 Apr 2026 at 01:26, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>
> Hi all,
>
> I'm looking into force-enabling CONFIG_WERROR for all Rockchip SoCs.
> First, let me know if this is a bad idea :)

I don't like it, as it makes development slower. I like to see all the
warnings created by a change and tend to fix them iteratively. We
already have a warning check in CI. A better approach is to

Can you use: KCFLAGS=-Werror

either in your environment or on the 'make' cmdline?

>
> gcc version 15.2.1 20250808 (Red Hat Cross 15.2.1-1) (GCC) for both ARM
> and Aarch64 machines report no warnings (built with a config fragment
> enabling CONFIG_WERROR).
>
> For clang, I went for cross-compiling using either
> make HOSTCC=clang CROSS_COMPILE=arm-linux-gnu- CC=clang O=build/ ...
> or
> make HOSTCC=clang CROSS_COMPILE=aarch64-linux-gnu- CC=clang O=build/ ...
>
> First things first, I can't even build any of the Aarch32/ARM with clang
> for some reason, so this only applies for Aarch64 Rockchip platforms.
>
> Some platforms couldn't be built due to hitting the max TPL limit (e.g.
> PX30).

It's interesting that clang code is larger than gcc. There was a time
that I thought clang would replace gcc. But clang is much slower and
if it doesn't provide code-size benefits, I'm not sure that will
happen. We all use it as an lsp though, at least.

>
> clang version 21.1.8 (Fedora 21.1.8-4.fc43) (or clang version 22.1.1
> (Fedora 22.1.1-2.fc44), doesn't matter) issues three warnings:
>
> 1. Chromebooks EC fails to build:
> In file included from
> /home/qschulz/work/upstream/u-boot/drivers/misc/cros_ec.c:21:
> In file included from
> /home/qschulz/work/upstream/u-boot/include/cros_ec.h:12:
> /home/qschulz/work/upstream/u-boot/include/ec_commands.h:3578:2: error:
> field  within 'struct ec_params_charge_state' is less aligned than
> 'union ec_params_charge_state::(anonymous at
> /home/qschulz/work/upstream/u-boot/include/ec_commands.h:3578:2)' and is
> usually due to 'struct ec_params_charge_state' being packed, which can
> lead to unaligned accesses [-Werror,-Wunaligned-access]
>   3578 |         union {
>        |         ^
>
> The following
>
> diff --git a/include/ec_commands.h b/include/ec_commands.h
> index 23597d28b2c..c5f9777c0c7 100644
> --- a/include/ec_commands.h
> +++ b/include/ec_commands.h
> @@ -3575,7 +3575,7 @@ enum charge_state_params {
>
>   struct __ec_todo_packed ec_params_charge_state {
>         uint8_t cmd;                            /* enum charge_state_command */
> -       union {
> +       union __ec_todo_packed {
>                 struct __ec_align1 {
>                         /* no args */
>                 } get_state;
>
> seems to fix it. The upstream EC git repo
> (https://chromium.googlesource.com/chromiumos/platform/ec) has a commit
> with more changes, see 65da9cc08766 ("include/ec_commands.h: Fix
> unaligned access warnings"). It doesn't apply cleanly (and I haven't
> looked into other patches to backport for this to apply cleanly). I'm
> not sure what we're supposed to do here? I also don't have a Chromebook
> to test.

It should be possible to backport this, to avoid divergence. I could
look at that if you like.

For now you could use a #pragma just around the header include, or in cros_ec.c

I don't think you need to worry about the struct going wrong, so long
as the size remains the same. There are two Chromebooks in the SJG lab
you can try it with: bob and kevin

>
> 2. RK3528, RK3576 and RK3588 boards fail with (replace rk3528 in path
> for the other SoCs)
>
> /home/qschulz/work/upstream/u-boot/arch/arm/mach-rockchip/rk3528/rk3528.c:98:45:
> error: value size does not match register size specified by the
> constraint and modifier [-Werror,-Wasm-operand-widths]
>     98 |         asm volatile("msr cntfrq_el0, %0" : : "r"
> (CONFIG_COUNTER_FREQUENCY));
>        |                                                    ^
> include/generated/autoconf.h:372:34: note: expanded from macro
> 'CONFIG_COUNTER_FREQUENCY'
>    372 | #define CONFIG_COUNTER_FREQUENCY 24000000
>        |                                  ^
> /home/qschulz/work/upstream/u-boot/arch/arm/mach-rockchip/rk3528/rk3528.c:98:32:
> note: use constraint modifier "w"
>     98 |         asm volatile("msr cntfrq_el0, %0" : : "r"
> (CONFIG_COUNTER_FREQUENCY));
>        |                                       ^~
>        |                                       %w0
>
> Note that all Rockchip SoCs have CONFIG_COUNTER_FREQUENCY set to 24000000.
>
> The cntfrq_el0 register is 64b
> (https://developer.arm.com/documentation/ddi0601/2026-03/AArch64-Registers/CNTFRQ-EL0--Counter-timer-Frequency-Register?lang=en),
> though the top 32b are reserved. Android decided to ignore this warning
> in the Little Kernel with
> https://android.googlesource.com/kernel/lk/+/923541d4c23565a47d020d84202a6f77d22fc149%5E%21/.
>
> I tried to follow clang's recommendation and switch this to %w0 but it
> fails with:
>
> /tmp/rk3588-a0a4b8.s: Assembler messages:
> /tmp/rk3588-a0a4b8.s:87: Error: operand mismatch -- `msr cntfrq_el0,w9'
> /tmp/rk3588-a0a4b8.s:87: Info:    did you mean this?
> /tmp/rk3588-a0a4b8.s:87: Info:          msr cntfrq_el0, x9
> clang: error: assembler command failed with exit code 1 (use -v to see
> invocation)
>
> I tried casting CONFIG_COUNTER_FREQUENCY into a u64 and it builds and no
> magic smoke when booting a board. Whether this is the proper fix, I
> don't know, but the warning is then gone. We could also decide to ignore
> the warning like for Android's Little Kernel (where and how much this is
> used is unclear as the repo isn't seeing much activity).
>
> The register documentation indicates this is not used by the hardware at
> all, so even if it was broken, we probably wouldn't see a difference?

Similar to what Tom says...does this work?:

asm volatile("msr cntfrq_el0, %0" : : "r"
  ((u64)CONFIG_COUNTER_FREQUENCY));

>
> 3. Boards building USB TCPM fail with
>
> In file included from
> /home/qschulz/work/upstream/u-boot/drivers/usb/tcpm/tcpm-uclass.c:12:
> In file included from
> /home/qschulz/work/upstream/u-boot/include/usb/tcpm.h:12:
> /home/qschulz/work/upstream/u-boot/include/usb/pd.h:212:2: error: field
> within 'struct pd_message' is less aligned than 'union
> pd_message::(anonymous at
> /home/qschulz/work/upstream/u-boot/include/usb/pd.h:212:2)' and is
> usually due to 'struct pd_message' being packed, which can lead to
> unaligned accesses [-Werror,-Wunaligned-access]
>    212 |         union {
>        |         ^
>
> This file is apparently adapted from upstream Linux, where the code is
> identical for that struct. So not sure what to do here.

Maybe add __packed ?

Regards,
Simon