[PATCH 2/2] cros_ec: Sync ec_commands.h from upstream Chrome OS EC

[Quentin Schulz]

Hi Simon,

On 4/30/26 1:38 PM, Simon Glass wrote:
> Sync include/ec_commands.h from upstream commit 4f3d17aa34
> ("skywalker: set SLEEP_TIMEOUT_MS to 50 seconds"). The new file makes
> two build assumptions that do not hold for U-Boot.
> 
> It gates '<linux/limits.h>' on '#ifdef __KERNEL__' for the Linux
> kernel. U-Boot also defines __KERNEL__ but has no <linux/limits.h>,
> and the file references no limits.h symbol; widen the gate with
> '!defined(__UBOOT__)'
> 
> It also hides '<stdint.h>' from __KERNEL__ builds, leaving UINT16_MAX
> (used by EC_RES_MAX) undefined for U-Boot; tighten the gate to
> '!defined(__KERNEL__) || defined(__UBOOT__)'
> 
> Adapt callers to two interface changes. The 'ec_current_image' enum
> tag is now 'ec_image' (EC_IMAGE_* constants unchanged); rename it in
> affected files to match. The VBNV-context interface was dropped

Idly wondering if we can have a patch doing an enum alias? A "pre-patch" 
that adds this alias, this patch which syncs the file from upstream, 
then a "post-patch" which removes the alias and rename it. Not sure it's 
worth it but was wondering if this was something C could do (typedef 
likely won't work because it removes the enum part when specifying it?)

[...]

> diff --git a/include/ec_commands.h b/include/ec_commands.h
> index 23597d28b2c..b131aa0a414 100644
> --- a/include/ec_commands.h
> +++ b/include/ec_commands.h
> @@ -1,31 +1,91 @@
> -/* Copyright (c) 2018 The Chromium OS Authors. All rights reserved.
> +/* Copyright 2014 The ChromiumOS Authors
>    * Use of this source code is governed by a BSD-style license that can be
>    * found in the LICENSE file.
>    */
>   
>   /* Host communication command constants for Chrome EC */
>   
> -#ifndef __CROS_EC_COMMANDS_H
> -#define __CROS_EC_COMMANDS_H
> +#ifndef __CROS_EC_EC_COMMANDS_H
> +#define __CROS_EC_EC_COMMANDS_H
>   
> +#if !defined(__ACPI__) && (!defined(__KERNEL__) || defined(__UBOOT__))
> +#include <stdint.h>
> +#endif
> +
> +#ifdef CHROMIUM_EC
>   /*
> - * Protocol overview
> - *
> - * request:  CMD [ P0 P1 P2 ... Pn S ]
> - * response: ERR [ P0 P1 P2 ... Pn S ]
> - *
> - * where the bytes are defined as follow :
> - *      - CMD is the command code. (defined by EC_CMD_ constants)
> - *      - ERR is the error code. (defined by EC_RES_ constants)
> - *      - Px is the optional payload.
> - *        it is not sent if the error code is not success.
> - *        (defined by ec_params_ and ec_response_ structures)
> - *      - S is the checksum which is the sum of all payload bytes.
> - *
> - * On LPC, CMD and ERR are sent/received at EC_LPC_ADDR_KERNEL|USER_CMD
> - * and the payloads are sent/received at EC_LPC_ADDR_KERNEL|USER_PARAM.
> - * On I2C, all bytes are sent serially in the same message.
> + * CHROMIUM_EC is defined by the Makefile system of Chromium EC repository.
> + * It is used to not include macros that may cause conflicts in foreign
> + * projects (refer to crbug.com/984623).
> + */
> +
> +/*
> + * Include common.h for CONFIG_HOSTCMD_ALIGNED, if it's defined. This
> + * generates more efficient code for accessing request/response structures on
> + * ARM Cortex-M if the structures are guaranteed 32-bit aligned.
> + */
> +#include "common.h"
> +#include "compile_time_macros.h"
> +
> +#else
> +/* If BUILD_ASSERT isn't already defined, make it a no-op */
> +#ifndef BUILD_ASSERT
> +#define BUILD_ASSERT(_cond)
> +#endif /* !BUILD_ASSERT */
> +#endif /* CHROMIUM_EC */
> +
> +#if defined(__KERNEL__) && !defined(__UBOOT__)
> +#include <linux/limits.h>

I think we want

#if defined(__KERNEL__)
#if !defined(__U_BOOT__)
#include <linux/limits.h>
#endif
#else

here. Such that the BIT/BIT_ULL/GENMASK and other macros aren't defined 
and then you wouldn't need the first patch in this series.

> +#else> +/*
> + * Defines macros that may be needed but are for sure defined by the linux
> + * kernel. This section is removed when cros_ec_commands.h is generated (by
> + * util/make_linux_ec_commands_h.sh).
> + * cros_ec_commands.h looks more integrated to the kernel.
> + */
> +
> +#ifndef BIT
> +#define BIT(nr) (1UL << (nr))
> +#endif
> +
> +#ifndef BIT_ULL
> +#define BIT_ULL(nr) (1ULL << (nr))
> +#endif
> +
> +/*
> + * When building Zephyr, this file ends up being included before Zephyr's
> + * include/sys/util.h so causes a warning there. We don't want to add an #ifdef
> + * in that file since it won't be accepted upstream. So work around it here.
> + */
> +#ifndef CONFIG_ZEPHYR
> +#ifndef GENMASK
> +#define GENMASK(h, l) (((BIT(h) << 1) - 1) ^ (BIT(l) - 1))
> +#endif
> +
> +#ifndef GENMASK_ULL
> +#define GENMASK_ULL(h, l) (((BIT_ULL(h) << 1) - 1) ^ (BIT_ULL(l) - 1))
> +#endif
> +#endif
> +
> +#endif /* __KERNEL__ */

Cheers,
Quentin