From fe68a67a5f11991146f47c2975a4e1156355a92c Mon Sep 17 00:00:00 2001 From: Alexandru Gagniuc Date: Thu, 29 Jul 2021 13:31:21 -0500 Subject: [PATCH 01/19] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0 Older OpenSSL and libressl versions have a slightly different API. This require #ifdefs to support. However, we still can't support it because the ECDSA path does not compile with these older versions. These #ifdefs are truly a vestigial appendage. Alternatively, the ECDSA path could be updated for older libraries, but this requires significant extra code, and #ifdefs. Those libraries are over three years old, and there concerns whether it makes sense to build modern software for real world use against such old libraries. Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support. Signed-off-by: Alexandru Gagniuc --- lib/rsa/rsa-sign.c | 76 +++------------------------------------------- 1 file changed, 4 insertions(+), 72 deletions(-) diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 085dc89bf7..3bc28247e4 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -19,24 +19,6 @@ #include #include -#if OPENSSL_VERSION_NUMBER >= 0x10000000L -#define HAVE_ERR_REMOVE_THREAD_STATE -#endif - -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) -static void RSA_get0_key(const RSA *r, - const BIGNUM **n, const BIGNUM **e, const BIGNUM **d) -{ - if (n != NULL) - *n = r->n; - if (e != NULL) - *e = r->e; - if (d != NULL) - *d = r->d; -} -#endif - static int rsa_err(const char *msg) { unsigned long sslErr = ERR_get_error(); @@ -314,24 +296,11 @@ static int rsa_init(void) { int ret; -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - ret = SSL_library_init(); -#else ret = OPENSSL_init_ssl(0, NULL); -#endif if (!ret) { fprintf(stderr, "Failure to init SSL library\n"); return -1; } -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - SSL_load_error_strings(); - - OpenSSL_add_all_algorithms(); - OpenSSL_add_all_digests(); - OpenSSL_add_all_ciphers(); -#endif return 0; } @@ -347,8 +316,7 @@ static int rsa_engine_init(const char *engine_id, ENGINE **pe) e = ENGINE_by_id(engine_id); if (!e) { fprintf(stderr, "Engine isn't available\n"); - ret = -1; - goto err_engine_by_id; + return -1; } if (!ENGINE_init(e)) { @@ -381,29 +349,9 @@ err_set_rsa: ENGINE_finish(e); err_engine_init: ENGINE_free(e); -err_engine_by_id: -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - ENGINE_cleanup(); -#endif return ret; } -static void rsa_remove(void) -{ -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - CRYPTO_cleanup_all_ex_data(); - ERR_free_strings(); -#ifdef HAVE_ERR_REMOVE_THREAD_STATE - ERR_remove_thread_state(NULL); -#else - ERR_remove_state(0); -#endif - EVP_cleanup(); -#endif -} - static void rsa_engine_remove(ENGINE *e) { if (e) { @@ -476,12 +424,7 @@ static int rsa_sign_with_key(EVP_PKEY *pkey, struct padding_algo *padding_algo, goto err_sign; } - #if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - EVP_MD_CTX_cleanup(context); - #else - EVP_MD_CTX_reset(context); - #endif + EVP_MD_CTX_reset(context); EVP_MD_CTX_destroy(context); debug("Got signature: %zu bytes, expected %d\n", size, EVP_PKEY_size(pkey)); @@ -513,7 +456,7 @@ int rsa_sign(struct image_sign_info *info, if (info->engine_id) { ret = rsa_engine_init(info->engine_id, &e); if (ret) - goto err_engine; + return ret; } ret = rsa_get_priv_key(info->keydir, info->keyname, info->keyfile, @@ -528,7 +471,6 @@ int rsa_sign(struct image_sign_info *info, EVP_PKEY_free(pkey); if (info->engine_id) rsa_engine_remove(e); - rsa_remove(); return ret; @@ -537,8 +479,6 @@ err_sign: err_priv: if (info->engine_id) rsa_engine_remove(e); -err_engine: - rsa_remove(); return ret; } @@ -686,12 +626,8 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) ret = rsa_get_pub_key(info->keydir, info->keyname, e, &pkey); if (ret) goto err_get_pub_key; -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - rsa = EVP_PKEY_get1_RSA(pkey); -#else + rsa = EVP_PKEY_get0_RSA(pkey); -#endif ret = rsa_get_params(rsa, &exponent, &n0_inv, &modulus, &r_squared); if (ret) goto err_get_params; @@ -761,10 +697,6 @@ done: if (ret) ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO; err_get_params: -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL) - RSA_free(rsa); -#endif EVP_PKEY_free(pkey); err_get_pub_key: if (info->engine_id) From 74bda4fe3d6d153a6b66b5f1e412c32b718bcfbc Mon Sep 17 00:00:00 2001 From: Chia-Wei Wang Date: Fri, 30 Jul 2021 09:08:02 +0800 Subject: [PATCH 02/19] lib/md5: Export progressive APIs Export the MD5 hash init/update/finish progressive APIs for better flexibility. Signed-off-by: Chia-Wei Wang --- include/u-boot/md5.h | 4 ++++ lib/md5.c | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/u-boot/md5.h b/include/u-boot/md5.h index e09c16a6e3..e5cb923d77 100644 --- a/include/u-boot/md5.h +++ b/include/u-boot/md5.h @@ -17,6 +17,10 @@ struct MD5Context { }; }; +void MD5Init(struct MD5Context *ctx); +void MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len); +void MD5Final(unsigned char digest[16], struct MD5Context *ctx); + /* * Calculate and store in 'output' the MD5 digest of 'len' bytes at * 'input'. 'output' must have enough space to hold 16 bytes. diff --git a/lib/md5.c b/lib/md5.c index 2ae4a06319..688b7254c6 100644 --- a/lib/md5.c +++ b/lib/md5.c @@ -55,7 +55,7 @@ byteReverse(unsigned char *buf, unsigned longs) * Start MD5 accumulation. Set bit count to 0 and buffer to mysterious * initialization constants. */ -static void +void MD5Init(struct MD5Context *ctx) { ctx->buf[0] = 0x67452301; @@ -71,7 +71,7 @@ MD5Init(struct MD5Context *ctx) * Update context to reflect the concatenation of another buffer full * of bytes. */ -static void +void MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len) { register __u32 t; @@ -120,7 +120,7 @@ MD5Update(struct MD5Context *ctx, unsigned char const *buf, unsigned len) * Final wrapup - pad to 64-byte boundary with the bit pattern * 1 0* (64-bit count of bits processed, MSB-first) */ -static void +void MD5Final(unsigned char digest[16], struct MD5Context *ctx) { unsigned int count; From 4deaff791cd44e95e545ad8cd74dea25b4993499 Mon Sep 17 00:00:00 2001 From: Chia-Wei Wang Date: Fri, 30 Jul 2021 09:08:03 +0800 Subject: [PATCH 03/19] dm: hash: Add new UCLASS_HASH support Add UCLASS_HASH for hash driver development. Thus the hash drivers (SW or HW-accelerated) can be developed in the DM-based fashion. Signed-off-by: Chia-Wei Wang --- drivers/crypto/Kconfig | 2 + drivers/crypto/Makefile | 1 + drivers/crypto/hash/Kconfig | 5 ++ drivers/crypto/hash/Makefile | 5 ++ drivers/crypto/hash/hash-uclass.c | 121 ++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/u-boot/hash.h | 61 +++++++++++++++ 7 files changed, 196 insertions(+) create mode 100644 drivers/crypto/hash/Kconfig create mode 100644 drivers/crypto/hash/Makefile create mode 100644 drivers/crypto/hash/hash-uclass.c create mode 100644 include/u-boot/hash.h diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 1ea116be75..0082177c21 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -1,5 +1,7 @@ menu "Hardware crypto devices" +source drivers/crypto/hash/Kconfig + source drivers/crypto/fsl/Kconfig endmenu diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile index efbd1d3fca..4a12b56be6 100644 --- a/drivers/crypto/Makefile +++ b/drivers/crypto/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_EXYNOS_ACE_SHA) += ace_sha.o obj-y += rsa_mod_exp/ obj-y += fsl/ +obj-y += hash/ diff --git a/drivers/crypto/hash/Kconfig b/drivers/crypto/hash/Kconfig new file mode 100644 index 0000000000..e226144b9b --- /dev/null +++ b/drivers/crypto/hash/Kconfig @@ -0,0 +1,5 @@ +config DM_HASH + bool "Enable Driver Model for Hash" + depends on DM + help + If you want to use driver model for Hash, say Y. diff --git a/drivers/crypto/hash/Makefile b/drivers/crypto/hash/Makefile new file mode 100644 index 0000000000..83acf3d47b --- /dev/null +++ b/drivers/crypto/hash/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (c) 2021 ASPEED Technology Inc. + +obj-$(CONFIG_DM_HASH) += hash-uclass.o diff --git a/drivers/crypto/hash/hash-uclass.c b/drivers/crypto/hash/hash-uclass.c new file mode 100644 index 0000000000..446eb9e56a --- /dev/null +++ b/drivers/crypto/hash/hash-uclass.c @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2021 ASPEED Technology Inc. + * Author: ChiaWei Wang + */ + +#define LOG_CATEGORY UCLASS_HASH + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct hash_info { + char *name; + uint32_t digest_size; +}; + +static const struct hash_info hash_info[HASH_ALGO_NUM] = { + [HASH_ALGO_CRC16_CCITT] = { "crc16-ccitt", 2 }, + [HASH_ALGO_CRC32] = { "crc32", 4 }, + [HASH_ALGO_MD5] = { "md5", 16 }, + [HASH_ALGO_SHA1] = { "sha1", 20 }, + [HASH_ALGO_SHA256] = { "sha256", 32 }, + [HASH_ALGO_SHA384] = { "sha384", 48 }, + [HASH_ALGO_SHA512] = { "sha512", 64}, +}; + +enum HASH_ALGO hash_algo_lookup_by_name(const char *name) +{ + int i; + + if (!name) + return HASH_ALGO_INVALID; + + for (i = 0; i < HASH_ALGO_NUM; ++i) + if (!strcmp(name, hash_info[i].name)) + return i; + + return HASH_ALGO_INVALID; +} + +ssize_t hash_algo_digest_size(enum HASH_ALGO algo) +{ + if (algo >= HASH_ALGO_NUM) + return -EINVAL; + + return hash_info[algo].digest_size; +} + +const char *hash_algo_name(enum HASH_ALGO algo) +{ + if (algo >= HASH_ALGO_NUM) + return NULL; + + return hash_info[algo].name; +} + +int hash_digest(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf) +{ + struct hash_ops *ops = (struct hash_ops *)device_get_ops(dev); + + if (!ops->hash_digest) + return -ENOSYS; + + return ops->hash_digest(dev, algo, ibuf, ilen, obuf); +} + +int hash_digest_wd(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf, uint32_t chunk_sz) +{ + struct hash_ops *ops = (struct hash_ops *)device_get_ops(dev); + + if (!ops->hash_digest_wd) + return -ENOSYS; + + return ops->hash_digest_wd(dev, algo, ibuf, ilen, obuf, chunk_sz); +} + +int hash_init(struct udevice *dev, enum HASH_ALGO algo, void **ctxp) +{ + struct hash_ops *ops = (struct hash_ops *)device_get_ops(dev); + + if (!ops->hash_init) + return -ENOSYS; + + return ops->hash_init(dev, algo, ctxp); +} + +int hash_update(struct udevice *dev, void *ctx, const void *ibuf, const uint32_t ilen) +{ + struct hash_ops *ops = (struct hash_ops *)device_get_ops(dev); + + if (!ops->hash_update) + return -ENOSYS; + + return ops->hash_update(dev, ctx, ibuf, ilen); +} + +int hash_finish(struct udevice *dev, void *ctx, void *obuf) +{ + struct hash_ops *ops = (struct hash_ops *)device_get_ops(dev); + + if (!ops->hash_finish) + return -ENOSYS; + + return ops->hash_finish(dev, ctx, obuf); +} + +UCLASS_DRIVER(hash) = { + .id = UCLASS_HASH, + .name = "hash", +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index e7edd409f3..3768432b68 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -54,6 +54,7 @@ enum uclass_id { UCLASS_FIRMWARE, /* Firmware */ UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */ UCLASS_GPIO, /* Bank of general-purpose I/O pins */ + UCLASS_HASH, /* Hash device */ UCLASS_HWSPINLOCK, /* Hardware semaphores */ UCLASS_I2C, /* I2C bus */ UCLASS_I2C_EEPROM, /* I2C EEPROM device */ diff --git a/include/u-boot/hash.h b/include/u-boot/hash.h new file mode 100644 index 0000000000..f9d47a99a7 --- /dev/null +++ b/include/u-boot/hash.h @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2021 ASPEED Technology Inc. + */ +#ifndef _UBOOT_HASH_H +#define _UBOOT_HASH_H + +enum HASH_ALGO { + HASH_ALGO_CRC16_CCITT, + HASH_ALGO_CRC32, + HASH_ALGO_MD5, + HASH_ALGO_SHA1, + HASH_ALGO_SHA256, + HASH_ALGO_SHA384, + HASH_ALGO_SHA512, + + HASH_ALGO_NUM, + + HASH_ALGO_INVALID = 0xffffffff, +}; + +/* general APIs for hash algo information */ +enum HASH_ALGO hash_algo_lookup_by_name(const char *name); +ssize_t hash_algo_digest_size(enum HASH_ALGO algo); +const char *hash_algo_name(enum HASH_ALGO algo); + +/* device-dependent APIs */ +int hash_digest(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf); +int hash_digest_wd(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf, uint32_t chunk_sz); +int hash_init(struct udevice *dev, enum HASH_ALGO algo, void **ctxp); +int hash_update(struct udevice *dev, void *ctx, const void *ibuf, const uint32_t ilen); +int hash_finish(struct udevice *dev, void *ctx, void *obuf); + +/* + * struct hash_ops - Driver model for Hash operations + * + * The uclass interface is implemented by all hash devices + * which use driver model. + */ +struct hash_ops { + /* progressive operations */ + int (*hash_init)(struct udevice *dev, enum HASH_ALGO algo, void **ctxp); + int (*hash_update)(struct udevice *dev, void *ctx, const void *ibuf, const uint32_t ilen); + int (*hash_finish)(struct udevice *dev, void *ctx, void *obuf); + + /* all-in-one operation */ + int (*hash_digest)(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf); + + /* all-in-one operation with watchdog triggering every chunk_sz */ + int (*hash_digest_wd)(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf, uint32_t chunk_sz); +}; + +#endif From e5d870fa1ecbcd4efcc13fa6d69c6754e39cff62 Mon Sep 17 00:00:00 2001 From: Chia-Wei Wang Date: Fri, 30 Jul 2021 09:08:04 +0800 Subject: [PATCH 04/19] crypto: hash: Add software hash DM driver Add purely software-implmented drivers to support multiple hash operations including CRC, MD5, and SHA family. This driver is based on the new hash uclass. Signed-off-by: Chia-Wei Wang --- drivers/crypto/hash/Kconfig | 11 ++ drivers/crypto/hash/Makefile | 1 + drivers/crypto/hash/hash_sw.c | 301 ++++++++++++++++++++++++++++++++++ 3 files changed, 313 insertions(+) create mode 100644 drivers/crypto/hash/hash_sw.c diff --git a/drivers/crypto/hash/Kconfig b/drivers/crypto/hash/Kconfig index e226144b9b..cd29a5c6a4 100644 --- a/drivers/crypto/hash/Kconfig +++ b/drivers/crypto/hash/Kconfig @@ -3,3 +3,14 @@ config DM_HASH depends on DM help If you want to use driver model for Hash, say Y. + +config HASH_SOFTWARE + bool "Enable driver for Hash in software" + depends on DM_HASH + depends on MD5 + depends on SHA1 + depends on SHA256 + depends on SHA512_ALGO + help + Enable driver for hashing operations in software. Currently + it support multiple hash algorithm including CRC/MD5/SHA. diff --git a/drivers/crypto/hash/Makefile b/drivers/crypto/hash/Makefile index 83acf3d47b..33d88161ed 100644 --- a/drivers/crypto/hash/Makefile +++ b/drivers/crypto/hash/Makefile @@ -3,3 +3,4 @@ # Copyright (c) 2021 ASPEED Technology Inc. obj-$(CONFIG_DM_HASH) += hash-uclass.o +obj-$(CONFIG_HASH_SOFTWARE) += hash_sw.o diff --git a/drivers/crypto/hash/hash_sw.c b/drivers/crypto/hash/hash_sw.c new file mode 100644 index 0000000000..fea9d12609 --- /dev/null +++ b/drivers/crypto/hash/hash_sw.c @@ -0,0 +1,301 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2021 ASPEED Technology Inc. + * Author: ChiaWei Wang + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* CRC16-CCITT */ +static void hash_init_crc16_ccitt(void *ctx) +{ + *((uint16_t *)ctx) = 0; +} + +static void hash_update_crc16_ccitt(void *ctx, const void *ibuf, uint32_t ilen) +{ + *((uint16_t *)ctx) = crc16_ccitt(*((uint16_t *)ctx), ibuf, ilen); +} + +static void hash_finish_crc16_ccitt(void *ctx, void *obuf) +{ + *((uint16_t *)obuf) = *((uint16_t *)ctx); +} + +/* CRC32 */ +static void hash_init_crc32(void *ctx) +{ + *((uint32_t *)ctx) = 0; +} + +static void hash_update_crc32(void *ctx, const void *ibuf, uint32_t ilen) +{ + *((uint32_t *)ctx) = crc32(*((uint32_t *)ctx), ibuf, ilen); +} + +static void hash_finish_crc32(void *ctx, void *obuf) +{ + *((uint32_t *)obuf) = *((uint32_t *)ctx); +} + +/* MD5 */ +static void hash_init_md5(void *ctx) +{ + MD5Init((struct MD5Context *)ctx); +} + +static void hash_update_md5(void *ctx, const void *ibuf, uint32_t ilen) +{ + MD5Update((struct MD5Context *)ctx, ibuf, ilen); +} + +static void hash_finish_md5(void *ctx, void *obuf) +{ + MD5Final(obuf, (struct MD5Context *)ctx); +} + +/* SHA1 */ +static void hash_init_sha1(void *ctx) +{ + sha1_starts((sha1_context *)ctx); +} + +static void hash_update_sha1(void *ctx, const void *ibuf, uint32_t ilen) +{ + sha1_update((sha1_context *)ctx, ibuf, ilen); +} + +static void hash_finish_sha1(void *ctx, void *obuf) +{ + sha1_finish((sha1_context *)ctx, obuf); +} + +/* SHA256 */ +static void hash_init_sha256(void *ctx) +{ + sha256_starts((sha256_context *)ctx); +} + +static void hash_update_sha256(void *ctx, const void *ibuf, uint32_t ilen) +{ + sha256_update((sha256_context *)ctx, ibuf, ilen); +} + +static void hash_finish_sha256(void *ctx, void *obuf) +{ + sha256_finish((sha256_context *)ctx, obuf); +} + +/* SHA384 */ +static void hash_init_sha384(void *ctx) +{ + sha384_starts((sha512_context *)ctx); +} + +static void hash_update_sha384(void *ctx, const void *ibuf, uint32_t ilen) +{ + sha384_update((sha512_context *)ctx, ibuf, ilen); +} + +static void hash_finish_sha384(void *ctx, void *obuf) +{ + sha384_finish((sha512_context *)ctx, obuf); +} + +/* SHA512 */ +static void hash_init_sha512(void *ctx) +{ + sha512_starts((sha512_context *)ctx); +} + +static void hash_update_sha512(void *ctx, const void *ibuf, uint32_t ilen) +{ + sha512_update((sha512_context *)ctx, ibuf, ilen); +} + +static void hash_finish_sha512(void *ctx, void *obuf) +{ + sha512_finish((sha512_context *)ctx, obuf); +} + +struct sw_hash_ctx { + enum HASH_ALGO algo; + uint8_t algo_ctx[]; +}; + +struct sw_hash_impl { + void (*init)(void *ctx); + void (*update)(void *ctx, const void *ibuf, uint32_t ilen); + void (*finish)(void *ctx, void *obuf); + uint32_t ctx_alloc_sz; +}; + +static struct sw_hash_impl sw_hash_impl[HASH_ALGO_NUM] = { + [HASH_ALGO_CRC16_CCITT] = { + .init = hash_init_crc16_ccitt, + .update = hash_update_crc16_ccitt, + .finish = hash_finish_crc16_ccitt, + .ctx_alloc_sz = sizeof(uint16_t), + }, + + [HASH_ALGO_CRC32] = { + .init = hash_init_crc32, + .update = hash_update_crc32, + .finish = hash_finish_crc32, + .ctx_alloc_sz = sizeof(uint32_t), + }, + + [HASH_ALGO_MD5] = { + .init = hash_init_md5, + .update = hash_update_md5, + .finish = hash_finish_md5, + .ctx_alloc_sz = sizeof(struct MD5Context), + }, + + [HASH_ALGO_SHA1] = { + .init = hash_init_sha1, + .update = hash_update_sha1, + .finish = hash_finish_sha1, + .ctx_alloc_sz = sizeof(sha1_context), + }, + + [HASH_ALGO_SHA256] = { + .init = hash_init_sha256, + .update = hash_update_sha256, + .finish = hash_finish_sha256, + .ctx_alloc_sz = sizeof(sha256_context), + }, + + [HASH_ALGO_SHA384] = { + .init = hash_init_sha384, + .update = hash_update_sha384, + .finish = hash_finish_sha384, + .ctx_alloc_sz = sizeof(sha512_context), + }, + + [HASH_ALGO_SHA512] = { + .init = hash_init_sha512, + .update = hash_update_sha512, + .finish = hash_finish_sha512, + .ctx_alloc_sz = sizeof(sha512_context), + }, +}; + +static int sw_hash_init(struct udevice *dev, enum HASH_ALGO algo, void **ctxp) +{ + struct sw_hash_ctx *hash_ctx; + struct sw_hash_impl *hash_impl = &sw_hash_impl[algo]; + + hash_ctx = malloc(sizeof(hash_ctx->algo) + hash_impl->ctx_alloc_sz); + if (!hash_ctx) + return -ENOMEM; + + hash_ctx->algo = algo; + + hash_impl->init(hash_ctx->algo_ctx); + + *ctxp = hash_ctx; + + return 0; +} + +static int sw_hash_update(struct udevice *dev, void *ctx, const void *ibuf, uint32_t ilen) +{ + struct sw_hash_ctx *hash_ctx = ctx; + struct sw_hash_impl *hash_impl = &sw_hash_impl[hash_ctx->algo]; + + hash_impl->update(hash_ctx->algo_ctx, ibuf, ilen); + + return 0; +} + +static int sw_hash_finish(struct udevice *dev, void *ctx, void *obuf) +{ + struct sw_hash_ctx *hash_ctx = ctx; + struct sw_hash_impl *hash_impl = &sw_hash_impl[hash_ctx->algo]; + + hash_impl->finish(hash_ctx->algo_ctx, obuf); + + free(ctx); + + return 0; +} + +static int sw_hash_digest_wd(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf, uint32_t chunk_sz) +{ + int rc; + void *ctx; + const void *cur, *end; + uint32_t chunk; + + rc = sw_hash_init(dev, algo, &ctx); + if (rc) + return rc; + + if (CONFIG_IS_ENABLED(HW_WATCHDOG) || CONFIG_IS_ENABLED(WATCHDOG)) { + cur = ibuf; + end = ibuf + ilen; + + while (cur < end) { + chunk = end - cur; + if (chunk > chunk_sz) + chunk = chunk_sz; + + rc = sw_hash_update(dev, ctx, cur, chunk); + if (rc) + return rc; + + cur += chunk; + WATCHDOG_RESET(); + } + } else { + rc = sw_hash_update(dev, ctx, ibuf, ilen); + if (rc) + return rc; + } + + rc = sw_hash_finish(dev, ctx, obuf); + if (rc) + return rc; + + return 0; +} + +static int sw_hash_digest(struct udevice *dev, enum HASH_ALGO algo, + const void *ibuf, const uint32_t ilen, + void *obuf) +{ + /* re-use the watchdog version with input length as the chunk_sz */ + return sw_hash_digest_wd(dev, algo, ibuf, ilen, obuf, ilen); +} + +static const struct hash_ops hash_ops_sw = { + .hash_init = sw_hash_init, + .hash_update = sw_hash_update, + .hash_finish = sw_hash_finish, + .hash_digest_wd = sw_hash_digest_wd, + .hash_digest = sw_hash_digest, +}; + +U_BOOT_DRIVER(hash_sw) = { + .name = "hash_sw", + .id = UCLASS_HASH, + .ops = &hash_ops_sw, + .flags = DM_FLAG_PRE_RELOC, +}; + +U_BOOT_DRVINFO(hash_sw) = { + .name = "hash_sw", +}; From ca47955a66161adf5d6e8f8e1de852ccda6626c0 Mon Sep 17 00:00:00 2001 From: Chia-Wei Wang Date: Fri, 30 Jul 2021 09:08:05 +0800 Subject: [PATCH 05/19] fit: Use DM hash driver if supported Calculate hash using DM driver if supported. For backward compatibility, the call to legacy hash functions is reserved. Signed-off-by: Chia-Wei Wang --- common/image-fit.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/common/image-fit.c b/common/image-fit.c index aff4670be3..aa9c781462 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -25,6 +25,10 @@ #include #include #include +#ifdef CONFIG_DM_HASH +#include +#include +#endif DECLARE_GLOBAL_DATA_PTR; #endif /* !USE_HOSTCC*/ @@ -1214,6 +1218,31 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) int calculate_hash(const void *data, int data_len, const char *algo, uint8_t *value, int *value_len) { +#if !defined(USE_HOSTCC) && defined(CONFIG_DM_HASH) + int rc; + enum HASH_ALGO hash_algo; + struct udevice *dev; + + rc = uclass_get_device(UCLASS_HASH, 0, &dev); + if (rc) { + debug("failed to get hash device, rc=%d\n", rc); + return -1; + } + + hash_algo = hash_algo_lookup_by_name(algo); + if (hash_algo == HASH_ALGO_INVALID) { + debug("Unsupported hash algorithm\n"); + return -1; + }; + + rc = hash_digest_wd(dev, hash_algo, data, data_len, value, CHUNKSZ); + if (rc) { + debug("failed to get hash value, rc=%d\n", rc); + return -1; + } + + *value_len = hash_algo_digest_size(hash_algo); +#else if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) { *((uint32_t *)value) = crc32_wd(0, data, data_len, CHUNKSZ_CRC32); @@ -1242,6 +1271,7 @@ int calculate_hash(const void *data, int data_len, const char *algo, debug("Unsupported hash alogrithm\n"); return -1; } +#endif return 0; } From 295ab733df05f673117d1576dd257afbba41366b Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 30 Jul 2021 17:05:07 +0200 Subject: [PATCH 06/19] lib: -Wformat-truncation in rsa_engine_get_priv_key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With glibc 2.33 (Ubuntu package glibc6 2.33-0ubuntu9) building sifive_unmatched_defconfig results in: In file included from /usr/include/stdio.h:866, from ././include/compiler.h:26, from : In function ‘snprintf’, inlined from ‘rsa_engine_get_priv_key’ at ./tools/../^:273:4: /usr/include/riscv64-linux-gnu/bits/stdio2.h:71:10: warning: ‘%s’ directive argument is null [-Wformat-truncation=] 71 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 72 | __glibc_objsize (__s), __fmt, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 73 | __va_arg_pack ()); | ~~~~~~~~~~~~~~~~~ Avoid passing a NULL string. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- lib/rsa/rsa-sign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 3bc28247e4..64db1429c1 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -254,7 +254,7 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name, else if (keydir) snprintf(key_id, sizeof(key_id), "%s", - name); + name ? name : ""); else if (keyfile) snprintf(key_id, sizeof(key_id), "%s", keyfile); else From eaa6442e4f4883418f595127b5bb12355ca9817d Mon Sep 17 00:00:00 2001 From: Thomas Hebb Date: Sun, 1 Aug 2021 15:23:13 -0700 Subject: [PATCH 07/19] mkimage: clarify error message for empty input files Currently, an empty imput file causes `mmap()` to fail, and you get an error like "mkimage: Can't read file.img: Invalid argument", which is extremely unintuitive and hard to diagnose if you don't know what to look for. Add an explicit check for an empty file and provide a clear error message instead. We already bounds check the image size when listing and re-signing existing images, so we only need this check here, when opening data files going into a image. Signed-off-by: Thomas Hebb Reviewed-by: Simon Glass --- tools/mkimage.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/mkimage.c b/tools/mkimage.c index 302bfcf971..fbe883ce36 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -732,6 +732,12 @@ copy_file (int ifd, const char *datafile, int pad) exit (EXIT_FAILURE); } + if (sbuf.st_size == 0) { + fprintf (stderr, "%s: Input file %s is empty, bailing out\n", + params.cmdname, datafile); + exit (EXIT_FAILURE); + } + ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, dfd, 0); if (ptr == MAP_FAILED) { fprintf (stderr, "%s: Can't read %s: %s\n", From e199fb35b4a04aa80c5623176c5925ab99bcd6c3 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Tue, 3 Aug 2021 08:31:56 -0400 Subject: [PATCH 08/19] scripts/checkpatch.pl: Resync with v5.13 This resyncs us with the version found in v5.13 of the Linux kernel with the following exceptions: - Keep our u-boot specific tests / code area. - Change the location of checkpatch.rst (which we now import) - Drop the "use strscpy" test as we don't have that, but do have strlcpy and want that used now. - Keep debug/printf in the list for $logFunctions And note that we now also include the spdxcheck.py tool that checkpatch.pl supports calling out to, and include upstream's checkpatch.rst in our develop section of the documentation. Signed-off-by: Tom Rini --- doc/develop/checkpatch.rst | 755 ++++++++++++++++++++++++ doc/develop/index.rst | 1 + scripts/checkpatch.pl | 1116 ++++++++++++++++++++++++++++-------- scripts/spdxcheck.py | 296 ++++++++++ 4 files changed, 1920 insertions(+), 248 deletions(-) create mode 100644 doc/develop/checkpatch.rst create mode 100755 scripts/spdxcheck.py diff --git a/doc/develop/checkpatch.rst b/doc/develop/checkpatch.rst new file mode 100644 index 0000000000..51fed1bd72 --- /dev/null +++ b/doc/develop/checkpatch.rst @@ -0,0 +1,755 @@ +.. SPDX-License-Identifier: GPL-2.0-only + +========== +Checkpatch +========== + +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial +style violations in patches and optionally corrects them. Checkpatch can +also be run on file contexts and without the kernel tree. + +Checkpatch is not always right. Your judgement takes precedence over checkpatch +messages. If your code looks better with the violations, then its probably +best left alone. + + +Options +======= + +This section will describe the options checkpatch can be run with. + +Usage:: + + ./scripts/checkpatch.pl [OPTION]... [FILE]... + +Available options: + + - -q, --quiet + + Enable quiet mode. + + - -v, --verbose + Enable verbose mode. Additional verbose test descriptions are output + so as to provide information on why that particular message is shown. + + - --no-tree + + Run checkpatch without the kernel tree. + + - --no-signoff + + Disable the 'Signed-off-by' line check. The sign-off is a simple line at + the end of the explanation for the patch, which certifies that you wrote it + or otherwise have the right to pass it on as an open-source patch. + + Example:: + + Signed-off-by: Random J Developer + + Setting this flag effectively stops a message for a missing signed-off-by + line in a patch context. + + - --patch + + Treat FILE as a patch. This is the default option and need not be + explicitly specified. + + - --emacs + + Set output to emacs compile window format. This allows emacs users to jump + from the error in the compile window directly to the offending line in the + patch. + + - --terse + + Output only one line per report. + + - --showfile + + Show the diffed file position instead of the input file position. + + - -g, --git + + Treat FILE as a single commit or a git revision range. + + Single commit with: + + - + - ^ + - ~n + + Multiple commits with: + + - .. + - ... + - - + + - -f, --file + + Treat FILE as a regular source file. This option must be used when running + checkpatch on source files in the kernel. + + - --subjective, --strict + + Enable stricter tests in checkpatch. By default the tests emitted as CHECK + do not activate by default. Use this flag to activate the CHECK tests. + + - --list-types + + Every message emitted by checkpatch has an associated TYPE. Add this flag + to display all the types in checkpatch. + + Note that when this flag is active, checkpatch does not read the input FILE, + and no message is emitted. Only a list of types in checkpatch is output. + + - --types TYPE(,TYPE2...) + + Only display messages with the given types. + + Example:: + + ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,BRACES + + - --ignore TYPE(,TYPE2...) + + Checkpatch will not emit messages for the specified types. + + Example:: + + ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,BRACES + + - --show-types + + By default checkpatch doesn't display the type associated with the messages. + Set this flag to show the message type in the output. + + - --max-line-length=n + + Set the max line length (default 100). If a line exceeds the specified + length, a LONG_LINE message is emitted. + + + The message level is different for patch and file contexts. For patches, + a WARNING is emitted. While a milder CHECK is emitted for files. So for + file contexts, the --strict flag must also be enabled. + + - --min-conf-desc-length=n + + Set the Kconfig entry minimum description length, if shorter, warn. + + - --tab-size=n + + Set the number of spaces for tab (default 8). + + - --root=PATH + + PATH to the kernel tree root. + + This option must be specified when invoking checkpatch from outside + the kernel root. + + - --no-summary + + Suppress the per file summary. + + - --mailback + + Only produce a report in case of Warnings or Errors. Milder Checks are + excluded from this. + + - --summary-file + + Include the filename in summary. + + - --debug KEY=[0|1] + + Turn on/off debugging of KEY, where KEY is one of 'values', 'possible', + 'type', and 'attr' (default is all off). + + - --fix + + This is an EXPERIMENTAL feature. If correctable errors exists, a file + .EXPERIMENTAL-checkpatch-fixes is created which has the + automatically fixable errors corrected. + + - --fix-inplace + + EXPERIMENTAL - Similar to --fix but input file is overwritten with fixes. + + DO NOT USE this flag unless you are absolutely sure and you have a backup + in place. + + - --ignore-perl-version + + Override checking of perl version. Runtime errors maybe encountered after + enabling this flag if the perl version does not meet the minimum specified. + + - --codespell + + Use the codespell dictionary for checking spelling errors. + + - --codespellfile + + Use the specified codespell file. + Default is '/usr/share/codespell/dictionary.txt'. + + - --typedefsfile + + Read additional types from this file. + + - --color[=WHEN] + + Use colors 'always', 'never', or only when output is a terminal ('auto'). + Default is 'auto'. + + - --kconfig-prefix=WORD + + Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`). + + - -h, --help, --version + + Display the help text. + +Message Levels +============== + +Messages in checkpatch are divided into three levels. The levels of messages +in checkpatch denote the severity of the error. They are: + + - ERROR + + This is the most strict level. Messages of type ERROR must be taken + seriously as they denote things that are very likely to be wrong. + + - WARNING + + This is the next stricter level. Messages of type WARNING requires a + more careful review. But it is milder than an ERROR. + + - CHECK + + This is the mildest level. These are things which may require some thought. + +Type Descriptions +================= + +This section contains a description of all the message types in checkpatch. + +.. Types in this section are also parsed by checkpatch. +.. The types are grouped into subsections based on use. + + +Allocation style +---------------- + + **ALLOC_ARRAY_ARGS** + The first argument for kcalloc or kmalloc_array should be the + number of elements. sizeof() as the first argument is generally + wrong. + See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html + + **ALLOC_SIZEOF_STRUCT** + The allocation style is bad. In general for family of + allocation functions using sizeof() to get memory size, + constructs like:: + + p = alloc(sizeof(struct foo), ...) + + should be:: + + p = alloc(sizeof(*p), ...) + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#allocating-memory + + **ALLOC_WITH_MULTIPLY** + Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a + sizeof multiply. + See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html + + +API usage +--------- + + **ARCH_DEFINES** + Architecture specific defines should be avoided wherever + possible. + + **ARCH_INCLUDE_LINUX** + Whenever asm/file.h is included and linux/file.h exists, a + conversion can be made when linux/file.h includes asm/file.h. + However this is not always the case (See signal.h). + This message type is emitted only for includes from arch/. + + **AVOID_BUG** + BUG() or BUG_ON() should be avoided totally. + Use WARN() and WARN_ON() instead, and handle the "impossible" + error condition as gracefully as possible. + See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on + + **CONSIDER_KSTRTO** + The simple_strtol(), simple_strtoll(), simple_strtoul(), and + simple_strtoull() functions explicitly ignore overflows, which + may lead to unexpected results in callers. The respective kstrtol(), + kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the + correct replacements. + See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull + + **LOCKDEP** + The lockdep_no_validate class was added as a temporary measure to + prevent warnings on conversion of device->sem to device->mutex. + It should not be used for any other purpose. + See: https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/ + + **MALFORMED_INCLUDE** + The #include statement has a malformed path. This has happened + because the author has included a double slash "//" in the pathname + accidentally. + + **USE_LOCKDEP** + lockdep_assert_held() annotations should be preferred over + assertions based on spin_is_locked() + See: https://www.kernel.org/doc/html/latest/locking/lockdep-design.html#annotations + + **UAPI_INCLUDE** + No #include statements in include/uapi should use a uapi/ path. + + +Comment style +------------- + + **BLOCK_COMMENT_STYLE** + The comment style is incorrect. The preferred style for multi- + line comments is:: + + /* + * This is the preferred style + * for multi line comments. + */ + + The networking comment style is a bit different, with the first line + not empty like the former:: + + /* This is the preferred comment style + * for files in net/ and drivers/net/ + */ + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting + + **C99_COMMENTS** + C99 style single line comments (//) should not be used. + Prefer the block comment style instead. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting + + +Commit message +-------------- + + **BAD_SIGN_OFF** + The signed-off-by line does not fall in line with the standards + specified by the community. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1 + + **BAD_STABLE_ADDRESS_STYLE** + The email format for stable is incorrect. + Some valid options for stable address are:: + + 1. stable@vger.kernel.org + 2. stable@kernel.org + + For adding version info, the following comment style should be used:: + + stable@vger.kernel.org # version info + + **COMMIT_COMMENT_SYMBOL** + Commit log lines starting with a '#' are ignored by git as + comments. To solve this problem addition of a single space + infront of the log line is enough. + + **COMMIT_MESSAGE** + The patch is missing a commit description. A brief + description of the changes made by the patch should be added. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes + + **MISSING_SIGN_OFF** + The patch is missing a Signed-off-by line. A signed-off-by + line should be added according to Developer's certificate of + Origin. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin + + **NO_AUTHOR_SIGN_OFF** + The author of the patch has not signed off the patch. It is + required that a simple sign off line should be present at the + end of explanation of the patch to denote that the author has + written it or otherwise has the rights to pass it on as an open + source patch. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin + + **DIFF_IN_COMMIT_MSG** + Avoid having diff content in commit message. + This causes problems when one tries to apply a file containing both + the changelog and the diff because patch(1) tries to apply the diff + which it found in the changelog. + See: https://lore.kernel.org/lkml/20150611134006.9df79a893e3636019ad2759e@linux-foundation.org/ + + **GERRIT_CHANGE_ID** + To be picked up by gerrit, the footer of the commit message might + have a Change-Id like:: + + Change-Id: Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b + Signed-off-by: A. U. Thor + + The Change-Id line must be removed before submitting. + + **GIT_COMMIT_ID** + The proper way to reference a commit id is: + commit <12+ chars of sha1> ("") + + An example may be:: + + Commit e21d2170f36602ae2708 ("video: remove unnecessary + platform_set_drvdata()") removed the unnecessary + platform_set_drvdata(), but left the variable "dev" unused, + delete it. + + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes + + +Comparison style +---------------- + + **ASSIGN_IN_IF** + Do not use assignments in if condition. + Example:: + + if ((foo = bar(...)) < BAZ) { + + should be written as:: + + foo = bar(...); + if (foo < BAZ) { + + **BOOL_COMPARISON** + Comparisons of A to true and false are better written + as A and !A. + See: https://lore.kernel.org/lkml/1365563834.27174.12.camel@joe-AO722/ + + **COMPARISON_TO_NULL** + Comparisons to NULL in the form (foo == NULL) or (foo != NULL) + are better written as (!foo) and (foo). + + **CONSTANT_COMPARISON** + Comparisons with a constant or upper case identifier on the left + side of the test should be avoided. + + +Macros, Attributes and Symbols +------------------------------ + + **ARRAY_SIZE** + The ARRAY_SIZE(foo) macro should be preferred over + sizeof(foo)/sizeof(foo[0]) for finding number of elements in an + array. + + The macro is defined in include/linux/kernel.h:: + + #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + + **AVOID_EXTERNS** + Function prototypes don't need to be declared extern in .h + files. It's assumed by the compiler and is unnecessary. + + **AVOID_L_PREFIX** + Local symbol names that are prefixed with `.L` should be avoided, + as this has special meaning for the assembler; a symbol entry will + not be emitted into the symbol table. This can prevent `objtool` + from generating correct unwind info. + + Symbols with STB_LOCAL binding may still be used, and `.L` prefixed + local symbol names are still generally usable within a function, + but `.L` prefixed local symbol names should not be used to denote + the beginning or end of code regions via + `SYM_CODE_START_LOCAL`/`SYM_CODE_END` + + **BIT_MACRO** + Defines like: 1 << <digit> could be BIT(digit). + The BIT() macro is defined in include/linux/bitops.h:: + + #define BIT(nr) (1UL << (nr)) + + **CONST_READ_MOSTLY** + When a variable is tagged with the __read_mostly annotation, it is a + signal to the compiler that accesses to the variable will be mostly + reads and rarely(but NOT never) a write. + + const __read_mostly does not make any sense as const data is already + read-only. The __read_mostly annotation thus should be removed. + + **DATE_TIME** + It is generally desirable that building the same source code with + the same set of tools is reproducible, i.e. the output is always + exactly the same. + + The kernel does *not* use the ``__DATE__`` and ``__TIME__`` macros, + and enables warnings if they are used as they can lead to + non-deterministic builds. + See: https://www.kernel.org/doc/html/latest/kbuild/reproducible-builds.html#timestamps + + **DEFINE_ARCH_HAS** + The ARCH_HAS_xyz and ARCH_HAVE_xyz patterns are wrong. + + For big conceptual features use Kconfig symbols instead. And for + smaller things where we have compatibility fallback functions but + want architectures able to override them with optimized ones, we + should either use weak functions (appropriate for some cases), or + the symbol that protects them should be the same symbol we use. + See: https://lore.kernel.org/lkml/CA+55aFycQ9XJvEOsiM3txHL5bjUc8CeKWJNR_H+MiicaddB42Q@mail.gmail.com/ + + **INIT_ATTRIBUTE** + Const init definitions should use __initconst instead of + __initdata. + + Similarly init definitions without const require a separate + use of const. + + **INLINE_LOCATION** + The inline keyword should sit between storage class and type. + + For example, the following segment:: + + inline static int example_function(void) + { + ... + } + + should be:: + + static inline int example_function(void) + { + ... + } + + **MULTISTATEMENT_MACRO_USE_DO_WHILE** + Macros with multiple statements should be enclosed in a + do - while block. Same should also be the case for macros + starting with `if` to avoid logic defects:: + + #define macrofun(a, b, c) \ + do { \ + if (a == 5) \ + do_this(b, c); \ + } while (0) + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl + + **WEAK_DECLARATION** + Using weak declarations like __attribute__((weak)) or __weak + can have unintended link defects. Avoid using them. + + +Functions and Variables +----------------------- + + **CAMELCASE** + Avoid CamelCase Identifiers. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#naming + + **FUNCTION_WITHOUT_ARGS** + Function declarations without arguments like:: + + int foo() + + should be:: + + int foo(void) + + **GLOBAL_INITIALISERS** + Global variables should not be initialized explicitly to + 0 (or NULL, false, etc.). Your compiler (or rather your + loader, which is responsible for zeroing out the relevant + sections) automatically does it for you. + + **INITIALISED_STATIC** + Static variables should not be initialized explicitly to zero. + Your compiler (or rather your loader) automatically does + it for you. + + **RETURN_PARENTHESES** + return is not a function and as such doesn't need parentheses:: + + return (bar); + + can simply be:: + + return bar; + + +Spacing and Brackets +-------------------- + + **ASSIGNMENT_CONTINUATIONS** + Assignment operators should not be written at the start of a + line but should follow the operand at the previous line. + + **BRACES** + The placement of braces is stylistically incorrect. + The preferred way is to put the opening brace last on the line, + and put the closing brace first:: + + if (x is true) { + we do y + } + + This applies for all non-functional blocks. + However, there is one special case, namely functions: they have the + opening brace at the beginning of the next line, thus:: + + int function(int x) + { + body of function + } + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces + + **BRACKET_SPACE** + Whitespace before opening bracket '[' is prohibited. + There are some exceptions: + + 1. With a type on the left:: + + ;int [] a; + + 2. At the beginning of a line for slice initialisers:: + + [0...10] = 5, + + 3. Inside a curly brace:: + + = { [0...10] = 5 } + + **CODE_INDENT** + Code indent should use tabs instead of spaces. + Outside of comments, documentation and Kconfig, + spaces are never used for indentation. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation + + **CONCATENATED_STRING** + Concatenated elements should have a space in between. + Example:: + + printk(KERN_INFO"bar"); + + should be:: + + printk(KERN_INFO "bar"); + + **ELSE_AFTER_BRACE** + `else {` should follow the closing block `}` on the same line. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces + + **LINE_SPACING** + Vertical space is wasted given the limited number of lines an + editor window can display when multiple blank lines are used. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **OPEN_BRACE** + The opening brace should be following the function definitions on the + next line. For any non-functional block it should be on the same line + as the last construct. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces + + **POINTER_LOCATION** + When using pointer data or a function that returns a pointer type, + the preferred use of * is adjacent to the data name or function name + and not adjacent to the type name. + Examples:: + + char *linux_banner; + unsigned long long memparse(char *ptr, char **retptr); + char *match_strdup(substring_t *s); + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **SPACING** + Whitespace style used in the kernel sources is described in kernel docs. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **SWITCH_CASE_INDENT_LEVEL** + switch should be at the same indent as case. + Example:: + + switch (suffix) { + case 'G': + case 'g': + mem <<= 30; + break; + case 'M': + case 'm': + mem <<= 20; + break; + case 'K': + case 'k': + mem <<= 10; + /* fall through */ + default: + break; + } + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation + + **TRAILING_WHITESPACE** + Trailing whitespace should always be removed. + Some editors highlight the trailing whitespace and cause visual + distractions when editing files. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **WHILE_AFTER_BRACE** + while should follow the closing bracket on the same line:: + + do { + ... + } while(something); + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces + + +Others +------ + + **CONFIG_DESCRIPTION** + Kconfig symbols should have a help text which fully describes + it. + + **CORRUPTED_PATCH** + The patch seems to be corrupted or lines are wrapped. + Please regenerate the patch file before sending it to the maintainer. + + **DOS_LINE_ENDINGS** + For DOS-formatted patches, there are extra ^M symbols at the end of + the line. These should be removed. + + **EXECUTE_PERMISSIONS** + There is no reason for source files to be executable. The executable + bit can be removed safely. + + **NON_OCTAL_PERMISSIONS** + Permission bits should use 4 digit octal permissions (like 0700 or 0444). + Avoid using any other base like decimal. + + **NOT_UNIFIED_DIFF** + The patch file does not appear to be in unified-diff format. Please + regenerate the patch file before sending it to the maintainer. + + **PRINTF_0XDECIMAL** + Prefixing 0x with decimal output is defective and should be corrected. + + **TRAILING_STATEMENTS** + Trailing statements (for example after any conditional) should be + on the next line. + Like:: + + if (x == y) break; + + should be:: + + if (x == y) + break; diff --git a/doc/develop/index.rst b/doc/develop/index.rst index 83c929babd..a7e625562c 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -54,5 +54,6 @@ Refactoring .. toctree:: :maxdepth: 1 + checkpatch coccinelle moveconfig diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 08a827535a..5696d3a5f3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -23,6 +23,9 @@ my $V = '0.32'; use Getopt::Long qw(:config no_auto_abbrev); my $quiet = 0; +my $verbose = 0; +my %verbose_messages = (); +my %verbose_emitted = (); my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; @@ -43,6 +46,8 @@ my $list_types = 0; my $fix = 0; my $fix_inplace = 0; my $root; +my $gitroot = $ENV{'GIT_DIR'}; +$gitroot = ".git" if !defined($gitroot); my %debug; my %camelcase = (); my %use_type = (); @@ -59,13 +64,15 @@ my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; -my $typedefsfile = ""; my $u_boot = 0; +my $docsfile = "$D/../doc/develop/checkpatch.rst"; +my $typedefsfile; my $color = "auto"; my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE # git output parsing needs US English output, so first set backtick child process LANGUAGE my $git_command ='export LANGUAGE=en_US.UTF-8; git'; my $tabsize = 8; +my ${CONFIG_} = "CONFIG_"; sub help { my ($exitcode) = @_; @@ -76,6 +83,7 @@ Version: $V Options: -q, --quiet quiet + -v, --verbose verbose mode --no-tree run without a kernel tree --no-signoff do not check for 'Signed-off-by' line --patch treat FILE as patchfile (default) @@ -129,6 +137,8 @@ Options: --color[=WHEN] Use colors 'always', 'never', or only when output is a terminal ('auto'). Default is 'auto'. --u-boot Run additional checks for U-Boot + --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default + ${CONFIG_}) -h, --help, --version display this help and exit When FILE is - read standard input. @@ -155,15 +165,51 @@ sub list_types { my $text = <$script>; close($script); - my @types = (); + my %types = (); # Also catch when type or level is passed through a variable - for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { - push (@types, $_); + while ($text =~ /(?:(\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { + if (defined($1)) { + if (exists($types{$2})) { + $types{$2} .= ",$1" if ($types{$2} ne $1); + } else { + $types{$2} = $1; + } + } else { + $types{$2} = "UNDETERMINED"; + } } - @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); - foreach my $type (@types) { + if ($color) { + print(" ( Color coding: "); + print(RED . "ERROR" . RESET); + print(" | "); + print(YELLOW . "WARNING" . RESET); + print(" | "); + print(GREEN . "CHECK" . RESET); + print(" | "); + print("Multiple levels / Undetermined"); + print(" )\n\n"); + } + + foreach my $type (sort keys %types) { + my $orig_type = $type; + if ($color) { + my $level = $types{$type}; + if ($level eq "ERROR") { + $type = RED . $type . RESET; + } elsif ($level eq "WARN") { + $type = YELLOW . $type . RESET; + } elsif ($level eq "CHK") { + $type = GREEN . $type . RESET; + } + } print(++$count . "\t" . $type . "\n"); + if ($verbose && exists($verbose_messages{$orig_type})) { + my $message = $verbose_messages{$orig_type}; + $message =~ s/\n/\n\t/g; + print("\t" . $message . "\n\n"); + } } exit($exitcode); @@ -195,6 +241,46 @@ if (-f $conf) { unshift(@ARGV, @conf_args) if @conf_args; } +sub load_docs { + open(my $docs, '<', "$docsfile") + or warn "$P: Can't read the documentation file $docsfile $!\n"; + + my $type = ''; + my $desc = ''; + my $in_desc = 0; + + while (<$docs>) { + chomp; + my $line = $_; + $line =~ s/\s+$//; + + if ($line =~ /^\s*\*\*(.+)\*\*$/) { + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + $type = $1; + $desc = ''; + $in_desc = 1; + } elsif ($in_desc) { + if ($line =~ /^(?:\s{4,}|$)/) { + $line =~ s/^\s{4}//; + $desc .= $line; + $desc .= "\n"; + } else { + $verbose_messages{$type} = trim($desc); + $type = ''; + $desc = ''; + $in_desc = 0; + } + } + } + + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + close($docs); +} + # Perl's Getopt::Long allows options to take optional arguments after a space. # Prevent --color by itself from consuming other arguments foreach (@ARGV) { @@ -205,6 +291,7 @@ foreach (@ARGV) { GetOptions( 'q|quiet+' => \$quiet, + 'v|verbose!' => \$verbose, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!' => \$chk_patch, @@ -238,12 +325,29 @@ GetOptions( 'color=s' => \$color, 'no-color' => \$color, #keep old behaviors of -nocolor 'nocolor' => \$color, #keep old behaviors of -nocolor + 'kconfig-prefix=s' => \${CONFIG_}, 'h|help' => \$help, 'version' => \$help ) or help(1); help(0) if ($help); +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); +die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse); + +if ($color =~ /^[01]$/) { + $color = !$color; +} elsif ($color =~ /^always$/i) { + $color = 1; +} elsif ($color =~ /^never$/i) { + $color = 0; +} elsif ($color =~ /^auto$/i) { + $color = (-t STDOUT); +} else { + die "$P: Invalid color mode: $color\n"; +} + +load_docs() if ($verbose); list_types(0) if ($list_types); $fix = 1 if ($fix_inplace); @@ -263,20 +367,8 @@ if ($#ARGV < 0) { push(@ARGV, '-'); } -if ($color =~ /^[01]$/) { - $color = !$color; -} elsif ($color =~ /^always$/i) { - $color = 1; -} elsif ($color =~ /^never$/i) { - $color = 0; -} elsif ($color =~ /^auto$/i) { - $color = (-t STDOUT); -} else { - die "Invalid color mode: $color\n"; -} - # skip TAB size 1 to avoid additional checks on $tabsize - 1 -die "Invalid TAB size: $tabsize\n" if ($tabsize < 2); +die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2); sub hash_save_array_words { my ($hashRef, $arrayRef) = @_; @@ -377,6 +469,7 @@ our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeIni # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + volatile| __percpu| __nocast| __safe| @@ -483,7 +576,7 @@ our $logFunctions = qr{(?x: our $allocFunctions = qr{(?x: (?:(?:devm_)? - (?:kv|k|v)[czm]alloc(?:_node|_array)? | + (?:kv|k|v)[czm]alloc(?:_array)?(?:_node)? | kstrdup(?:_const)? | kmemdup(?:_nul)?) | (?:\w+)?alloc_skb(?:_ip_align)? | @@ -503,6 +596,88 @@ our $signature_tags = qr{(?xi: Cc: )}; +our $tracing_logging_tags = qr{(?xi: + [=-]*> | + <[=-]* | + \[ | + \] | + start | + called | + entered | + entry | + enter | + in | + inside | + here | + begin | + exit | + end | + done | + leave | + completed | + out | + return | + [\.\!:\s]* +)}; + +sub edit_distance_min { + my (@arr) = @_; + my $len = scalar @arr; + if ((scalar @arr) < 1) { + # if underflow, return + return; + } + my $min = $arr[0]; + for my $i (0 .. ($len-1)) { + if ($arr[$i] < $min) { + $min = $arr[$i]; + } + } + return $min; +} + +sub get_edit_distance { + my ($str1, $str2) = @_; + $str1 = lc($str1); + $str2 = lc($str2); + $str1 =~ s/-//g; + $str2 =~ s/-//g; + my $len1 = length($str1); + my $len2 = length($str2); + # two dimensional array storing minimum edit distance + my @distance; + for my $i (0 .. $len1) { + for my $j (0 .. $len2) { + if ($i == 0) { + $distance[$i][$j] = $j; + } elsif ($j == 0) { + $distance[$i][$j] = $i; + } elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) { + $distance[$i][$j] = $distance[$i - 1][$j - 1]; + } else { + my $dist1 = $distance[$i][$j - 1]; #insert distance + my $dist2 = $distance[$i - 1][$j]; # remove + my $dist3 = $distance[$i - 1][$j - 1]; #replace + $distance[$i][$j] = 1 + edit_distance_min($dist1, $dist2, $dist3); + } + } + } + return $distance[$len1][$len2]; +} + +sub find_standard_signature { + my ($sign_off) = @_; + my @standard_signature_tags = ( + 'Signed-off-by:', 'Co-developed-by:', 'Acked-by:', 'Tested-by:', + 'Reviewed-by:', 'Reported-by:', 'Suggested-by:' + ); + foreach my $signature (@standard_signature_tags) { + return $signature if (get_edit_distance($sign_off, $signature) <= 2); + } + + return ""; +} + our @typeListMisordered = ( qr{char\s+(?:un)?signed}, qr{int\s+(?:(?:un)?signed\s+)?short\s}, @@ -591,6 +766,8 @@ our @mode_permission_funcs = ( ["__ATTR", 2], ); +my $word_pattern = '\b[A-Z]?[a-z]{2,}\b'; + #Create a search pattern for all these functions to speed up a loop below our $mode_perms_search = ""; foreach my $entry (@mode_permission_funcs) { @@ -759,7 +936,7 @@ sub read_words { next; } - $$wordsRef .= '|' if ($$wordsRef ne ""); + $$wordsRef .= '|' if (defined $$wordsRef); $$wordsRef .= $line; } close($file); @@ -769,16 +946,18 @@ sub read_words { return 0; } -my $const_structs = ""; -read_words(\$const_structs, $conststructsfile) - or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +my $const_structs; +if (show_type("CONST_STRUCT")) { + read_words(\$const_structs, $conststructsfile) + or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +} -my $typeOtherTypedefs = ""; -if (length($typedefsfile)) { +if (defined($typedefsfile)) { + my $typeOtherTypedefs; read_words(\$typeOtherTypedefs, $typedefsfile) or warn "No additional types will be considered - file '$typedefsfile': $!\n"; + $typeTypedefs .= '|' . $typeOtherTypedefs if (defined $typeOtherTypedefs); } -$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne ""); sub build_types { my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; @@ -843,10 +1022,16 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)}; our $declaration_macros = qr{(?x: (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(| (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(| - (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\(| (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\( )}; +our %allow_repeated_words = ( + add => '', + added => '', + bad => '', + be => '', +); + sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); @@ -904,7 +1089,7 @@ sub is_maintained_obsolete { sub is_SPDX_License_valid { my ($license) = @_; - return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$root/.git")); + return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$gitroot")); my $root_path = abs_path($root); my $status = `cd "$root_path"; echo "$license" | python scripts/spdxcheck.py -`; @@ -922,7 +1107,7 @@ sub seed_camelcase_includes { $camelcase_seeded = 1; - if (-e ".git") { + if (-e "$gitroot") { my $git_last_include_commit = `${git_command} log --no-merges --pretty=format:"%h%n" -1 -- include`; chomp $git_last_include_commit; $camelcase_cache = ".checkpatch-camelcase.git.$git_last_include_commit"; @@ -950,7 +1135,7 @@ sub seed_camelcase_includes { return; } - if (-e ".git") { + if (-e "$gitroot") { $files = `${git_command} ls-files "include/*.h"`; @include_files = split('\n', $files); } @@ -970,10 +1155,20 @@ sub seed_camelcase_includes { } } +sub git_is_single_file { + my ($filename) = @_; + + return 0 if ((which("git") eq "") || !(-e "$gitroot")); + + my $output = `${git_command} ls-files -- $filename 2>/dev/null`; + my $count = $output =~ tr/\n//; + return $count eq 1 && $output =~ m{^${filename}$}; +} + sub git_commit_info { my ($commit, $id, $desc) = @_; - return ($id, $desc) if ((which("git") eq "") || !(-e ".git")); + return ($id, $desc) if ((which("git") eq "") || !(-e "$gitroot")); my $output = `${git_command} log --no-color --format='%H %s' -1 $commit 2>&1`; $output =~ s/^\s*//gm; @@ -1012,7 +1207,7 @@ my $fixlinenr = -1; # If input is git commits, extract all commits from the commit expressions. # For example, HEAD-3 means we need check 'HEAD, HEAD~1, HEAD~2'. -die "$P: No git repository found\n" if ($git && !-e ".git"); +die "$P: No git repository found\n" if ($git && !-e "$gitroot"); if ($git) { my @commits = (); @@ -1043,6 +1238,9 @@ my $vname; $allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; + my $is_git_file = git_is_single_file($filename); + my $oldfile = $file; + $file = 1 if ($is_git_file); if ($git) { open($FILE, '-|', "git format-patch -M --stdout -1 $filename") || die "$P: $filename: git format-patch failed - $!\n"; @@ -1065,6 +1263,7 @@ for my $filename (@ARGV) { while (<$FILE>) { chomp; push(@rawlines, $_); + $vname = qq("$1") if ($filename eq '-' && $_ =~ m/^Subject:\s+(.+)/i); } close($FILE); @@ -1086,6 +1285,7 @@ for my $filename (@ARGV) { @modifierListFile = (); @typeListFile = (); build_types(); + $file = $oldfile if ($is_git_file); } if (!$quiet) { @@ -1131,6 +1331,7 @@ sub parse_email { my ($formatted_email) = @_; my $name = ""; + my $quoted = ""; my $name_comment = ""; my $address = ""; my $comment = ""; @@ -1162,14 +1363,20 @@ sub parse_email { } } - $name = trim($name); - $name =~ s/^\"|\"$//g; - $name =~ s/(\s*\([^\)]+\))\s*//; - if (defined($1)) { - $name_comment = trim($1); + # Extract comments from names excluding quoted parts + # "John D. (Doe)" - Do not extract + if ($name =~ s/\"(.+)\"//) { + $quoted = $1; } + while ($name =~ s/\s*($balanced_parens)\s*/ /) { + $name_comment .= trim($1); + } + $name =~ s/^[ \"]+|[ \"]+$//g; + $name = trim("$quoted $name"); + $address = trim($address); $address =~ s/^\<|\>$//g; + $comment = trim($comment); if ($name =~ /[^\w \-]/i) { ##has "must quote" chars $name =~ s/(?<!\\)"/\\"/g; ##escape quotes @@ -1180,25 +1387,30 @@ sub parse_email { } sub format_email { - my ($name, $address) = @_; + my ($name, $name_comment, $address, $comment) = @_; my $formatted_email; - $name = trim($name); - $name =~ s/^\"|\"$//g; + $name =~ s/^[ \"]+|[ \"]+$//g; $address = trim($address); + $address =~ s/(?:\.|\,|\")+$//; ##trailing commas, dots or quotes if ($name =~ /[^\w \-]/i) { ##has "must quote" chars $name =~ s/(?<!\\)"/\\"/g; ##escape quotes $name = "\"$name\""; } + $name_comment = trim($name_comment); + $name_comment = " $name_comment" if ($name_comment ne ""); + $comment = trim($comment); + $comment = " $comment" if ($comment ne ""); + if ("$name" eq "") { $formatted_email = "$address"; } else { - $formatted_email = "$name <$address>"; + $formatted_email = "$name$name_comment <$address>"; } - + $formatted_email .= "$comment"; return $formatted_email; } @@ -1206,7 +1418,7 @@ sub reformat_email { my ($email) = @_; my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); - return format_email($email_name, $email_address); + return format_email($email_name, $name_comment, $email_address, $comment); } sub same_email_addresses { @@ -1216,7 +1428,9 @@ sub same_email_addresses { my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2); return $email1_name eq $email2_name && - $email1_address eq $email2_address; + $email1_address eq $email2_address && + $name1_comment eq $name2_comment && + $comment1 eq $comment2; } sub which { @@ -1681,8 +1895,16 @@ sub ctx_statement_level { sub ctx_locate_comment { my ($first_line, $end_line) = @_; + # If c99 comment on the current line, or the line before or after + my ($current_comment) = ($rawlines[$end_line - 1] =~ m@^\+.*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line - 2] =~ m@^[\+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line] =~ m@^[\+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + # Catch a comment on the end of the line itself. - my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); + ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); return $current_comment if (defined $current_comment); # Look through the context and try and figure out if there is a @@ -2076,7 +2298,16 @@ sub report { splice(@lines, 1, 1); $output = join("\n", @lines); } - $output = (split('\n', $output))[0] . "\n" if ($terse); + + if ($terse) { + $output = (split('\n', $output))[0] . "\n"; + } + + if ($verbose && exists($verbose_messages{$type}) && + !exists($verbose_emitted{$type})) { + $output .= $verbose_messages{$type} . "\n\n"; + $verbose_emitted{$type} = 1; + } push(our @report, $output); @@ -2425,6 +2656,15 @@ sub u_boot_line { "DEVICE_PLAT_AUTO", $herecurr); } +sub exclude_global_initialisers { + my ($realfile) = @_; + + # Do not check for BPF programs (tools/testing/selftests/bpf/progs/*.c, samples/bpf/*_kern.c, *.bpf.c). + return $realfile =~ m@^tools/testing/selftests/bpf/progs/.*\.c$@ || + $realfile =~ m@^samples/bpf/.*_kern\.c$@ || + $realfile =~ m@/bpf/.*\.bpf\.c$@; +} + sub process { my $filename = shift; @@ -2443,6 +2683,7 @@ sub process { my $signoff = 0; my $author = ''; my $authorsignoff = 0; + my $author_sob = ''; my $is_patch = 0; my $is_binding_patch = -1; my $in_header_lines = $file ? 0 : 1; @@ -2506,7 +2747,7 @@ sub process { if ($rawline=~/^\+\+\+\s+(\S+)/) { $setup_docs = 0; - if ($1 =~ m@Documentation/admin-guide/kernel-parameters.rst$@) { + if ($1 =~ m@Documentation/admin-guide/kernel-parameters.txt$@) { $setup_docs = 1; } #next; @@ -2706,7 +2947,7 @@ sub process { if (($last_binding_patch != -1) && ($last_binding_patch ^ $is_binding_patch)) { WARN("DT_SPLIT_BINDING_PATCH", - "DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.txt\n"); + "DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst\n"); } } @@ -2735,8 +2976,8 @@ sub process { # Check if the commit log has what seems like a diff which can confuse patch if ($in_commit_log && !$commit_log_has_diff && - (($line =~ m@^\s+diff\b.*a/[\w/]+@ && - $line =~ m@^\s+diff\b.*a/([\w/]+)\s+b/$1\b@) || + (($line =~ m@^\s+diff\b.*a/([\w/]+)@ && + $line =~ m@^\s+diff\b.*a/[\w/]+\s+b/$1\b@) || $line =~ m@^\s*(?:\-\-\-\s+a/|\+\+\+\s+b/)@ || $line =~ m/^\s*\@\@ \-\d+,\d+ \+\d+,\d+ \@\@/)) { ERROR("DIFF_IN_COMMIT_MSG", @@ -2757,6 +2998,10 @@ sub process { # Check the patch for a From: if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) { $author = $1; + my $curline = $linenr; + while(defined($rawlines[$curline]) && ($rawlines[$curline++] =~ /^[ \t]\s*(.*)/)) { + $author .= $1; + } $author = encode("utf8", $author) if ($line =~ /=\?utf-8\?/i); $author =~ s/"//g; $author = reformat_email($author); @@ -2766,9 +3011,37 @@ sub process { if ($line =~ /^\s*signed-off-by:\s*(.*)/i) { $signoff++; $in_commit_log = 0; - if ($author ne '') { + if ($author ne '' && $authorsignoff != 1) { if (same_email_addresses($1, $author)) { $authorsignoff = 1; + } else { + my $ctx = $1; + my ($email_name, $email_comment, $email_address, $comment1) = parse_email($ctx); + my ($author_name, $author_comment, $author_address, $comment2) = parse_email($author); + + if ($email_address eq $author_address && $email_name eq $author_name) { + $author_sob = $ctx; + $authorsignoff = 2; + } elsif ($email_address eq $author_address) { + $author_sob = $ctx; + $authorsignoff = 3; + } elsif ($email_name eq $author_name) { + $author_sob = $ctx; + $authorsignoff = 4; + + my $address1 = $email_address; + my $address2 = $author_address; + + if ($address1 =~ /(\S+)\+\S+(\@.*)/) { + $address1 = "$1$2"; + } + if ($address2 =~ /(\S+)\+\S+(\@.*)/) { + $address2 = "$1$2"; + } + if ($address1 eq $address2) { + $authorsignoff = 5; + } + } } } } @@ -2795,8 +3068,17 @@ sub process { my $ucfirst_sign_off = ucfirst(lc($sign_off)); if ($sign_off !~ /$signature_tags/) { - WARN("BAD_SIGN_OFF", - "Non-standard signature: $sign_off\n" . $herecurr); + my $suggested_signature = find_standard_signature($sign_off); + if ($suggested_signature eq "") { + WARN("BAD_SIGN_OFF", + "Non-standard signature: $sign_off\n" . $herecurr); + } else { + if (WARN("BAD_SIGN_OFF", + "Non-standard signature: '$sign_off' - perhaps '$suggested_signature'?\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/; + } + } } if (defined $space_before && $space_before ne "") { if (WARN("BAD_SIGN_OFF", @@ -2825,7 +3107,7 @@ sub process { } my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); - my $suggested_email = format_email(($email_name, $email_address)); + my $suggested_email = format_email(($email_name, $name_comment, $email_address, $comment)); if ($suggested_email eq "") { ERROR("BAD_SIGN_OFF", "Unrecognized email address: '$email'\n" . $herecurr); @@ -2836,8 +3118,76 @@ sub process { # Don't force email to have quotes # Allow just an angle bracketed address if (!same_email_addresses($email, $suggested_email)) { + if (WARN("BAD_SIGN_OFF", + "email address '$email' might be better as '$suggested_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/; + } + } + + # Address part shouldn't have comments + my $stripped_address = $email_address; + $stripped_address =~ s/\([^\(\)]*\)//g; + if ($email_address ne $stripped_address) { + if (WARN("BAD_SIGN_OFF", + "address part of email should not have comments: '$email_address'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/; + } + } + + # Only one name comment should be allowed + my $comment_count = () = $name_comment =~ /\([^\)]+\)/g; + if ($comment_count > 1) { WARN("BAD_SIGN_OFF", - "email address '$email' might be better as '$suggested_email$comment'\n" . $herecurr); + "Use a single name comment in email: '$email'\n" . $herecurr); + } + + + # stable@vger.kernel.org or stable@kernel.org shouldn't + # have an email name. In addition comments should strictly + # begin with a # + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/i) { + if (($comment ne "" && $comment !~ /^#.+/) || + ($email_name ne "")) { + my $cur_name = $email_name; + my $new_comment = $comment; + $cur_name =~ s/[a-zA-Z\s\-\"]+//g; + + # Remove brackets enclosing comment text + # and # from start of comments to get comment text + $new_comment =~ s/^\((.*)\)$/$1/; + $new_comment =~ s/^\[(.*)\]$/$1/; + $new_comment =~ s/^[\s\#]+|\s+$//g; + + $new_comment = trim("$new_comment $cur_name") if ($cur_name ne $new_comment); + $new_comment = " # $new_comment" if ($new_comment ne ""); + my $new_email = "$email_address$new_comment"; + + if (WARN("BAD_STABLE_ADDRESS_STYLE", + "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/; + } + } + } elsif ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) { + my $new_comment = $comment; + + # Extract comment text from within brackets or + # c89 style /*...*/ comments + $new_comment =~ s/^\[(.*)\]$/$1/; + $new_comment =~ s/^\/\*(.*)\*\/$/$1/; + + $new_comment = trim($new_comment); + $new_comment =~ s/^[^\w]$//; # Single lettered comment with non word character is usually a typo + $new_comment = "($new_comment)" if ($new_comment ne ""); + my $new_email = format_email($email_name, $name_comment, $email_address, $new_comment); + + if (WARN("BAD_SIGN_OFF", + "Unexpected content after email: '$email', should be: '$new_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/; + } } } @@ -2860,7 +3210,7 @@ sub process { } if (!defined $lines[$linenr]) { WARN("BAD_SIGN_OFF", - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) { WARN("BAD_SIGN_OFF", "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); @@ -2880,8 +3230,11 @@ sub process { # Check for Gerrit Change-Ids not in any patch context if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) { - ERROR("GERRIT_CHANGE_ID", - "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr); + if (ERROR("GERRIT_CHANGE_ID", + "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } # Check if the commit log is in a possible stack dump @@ -2903,8 +3256,8 @@ sub process { # file delta changes $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ || # filename then : - $line =~ /^\s*(?:Fixes:|Link:)/i || - # A Fixes: or Link: line + $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i || + # A Fixes: or Link: line or signature tag line $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr); @@ -2917,6 +3270,15 @@ sub process { $commit_log_possible_stack_dump = 0; } +# Check for lines starting with a # + if ($in_commit_log && $line =~ /^#/) { + if (WARN("COMMIT_COMMENT_SYMBOL", + "Commit log lines starting with '#' are dropped by git as comments\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^/ /; + } + } + # Check for git id commit length and improperly formed commit descriptions if ($in_commit_log && !$commit_log_possible_stack_dump && $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink|base-commit):/i && @@ -2993,7 +3355,7 @@ sub process { ($line =~ /^new file mode\s*\d+\s*$/) && ($realfile =~ m@^Documentation/devicetree/bindings/.*\.txt$@)) { WARN("DT_SCHEMA_BINDING_PATCH", - "DT bindings should be in DT schema format. See: Documentation/devicetree/writing-schema.rst\n"); + "DT bindings should be in DT schema format. See: Documentation/devicetree/bindings/writing-schema.rst\n"); } # Check for wrappage within a valid hunk of the file @@ -3057,15 +3419,18 @@ sub process { # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { - while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) { + while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { my $typo = $1; + my $blank = copy_spacing($rawline); + my $ptr = substr($blank, 0, $-[1]) . "^" x length($typo); + my $hereptr = "$hereline$ptr\n"; my $typo_fix = $spelling_fix{lc($typo)}; $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/); $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/); my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("TYPO_SPELLING", - "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && + "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $hereptr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/; } @@ -3083,6 +3448,60 @@ sub process { } } +# check for repeated words separated by a single space +# avoid false positive from list command eg, '-rw-r--r-- 1 root root' + if (($rawline =~ /^\+/ || $in_commit_log) && + $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) { + pos($rawline) = 1 if (!$in_commit_log); + while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) { + + my $first = $1; + my $second = $2; + my $start_pos = $-[1]; + my $end_pos = $+[2]; + if ($first =~ /(?:struct|union|enum)/) { + pos($rawline) += length($first) + length($second) + 1; + next; + } + + next if (lc($first) ne lc($second)); + next if ($first eq 'long'); + + # check for character before and after the word matches + my $start_char = ''; + my $end_char = ''; + $start_char = substr($rawline, $start_pos - 1, 1) if ($start_pos > ($in_commit_log ? 0 : 1)); + $end_char = substr($rawline, $end_pos, 1) if ($end_pos < length($rawline)); + + next if ($start_char =~ /^\S$/); + next if (index(" \t.,;?!", $end_char) == -1); + + # avoid repeating hex occurrences like 'ff ff fe 09 ...' + if ($first =~ /\b[0-9a-f]{2,}\b/i) { + next if (!exists($allow_repeated_words{lc($first)})); + } + + if (WARN("REPEATED_WORD", + "Possible repeated word: '$first'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$first $second\b/$first/; + } + } + + # if it's a repeated word on consecutive lines in a comment block + if ($prevline =~ /$;+\s*$/ && + $prevrawline =~ /($word_pattern)\s*$/) { + my $last_word = $1; + if ($rawline =~ /^\+\s*\*\s*$last_word /) { + if (WARN("REPEATED_WORD", + "Possible repeated word: '$last_word'\n" . $hereprev) && + $fix) { + $fixed[$fixlinenr] =~ s/(\+\s*\*\s*)$last_word /$1/; + } + } + } + } + # ignore non-hunk lines and lines being removed next if (!$hunk_line || $line =~ /^-/); @@ -3141,11 +3560,7 @@ sub process { if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { $is_start = 1; - } elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) { - if ($lines[$ln - 1] =~ "---help---") { - WARN("CONFIG_DESCRIPTION", - "prefer 'help' over '---help---' for new help texts\n" . $herecurr); - } + } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) { $length = -1; } @@ -3172,22 +3587,44 @@ sub process { #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } -# check for MAINTAINERS entries that don't have the right form - if ($realfile =~ /^MAINTAINERS$/ && - $rawline =~ /^\+[A-Z]:/ && - $rawline !~ /^\+[A-Z]:\t\S/) { - if (WARN("MAINTAINERS_STYLE", - "MAINTAINERS entries use one tab after TYPE:\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/^(\+[A-Z]):\s*/$1:\t/; +# check MAINTAINERS entries + if ($realfile =~ /^MAINTAINERS$/) { +# check MAINTAINERS entries for the right form + if ($rawline =~ /^\+[A-Z]:/ && + $rawline !~ /^\+[A-Z]:\t\S/) { + if (WARN("MAINTAINERS_STYLE", + "MAINTAINERS entries use one tab after TYPE:\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^(\+[A-Z]):\s*/$1:\t/; + } + } +# check MAINTAINERS entries for the right ordering too + my $preferred_order = 'MRLSWQBCPTFXNK'; + if ($rawline =~ /^\+[A-Z]:/ && + $prevrawline =~ /^[\+ ][A-Z]:/) { + $rawline =~ /^\+([A-Z]):\s*(.*)/; + my $cur = $1; + my $curval = $2; + $prevrawline =~ /^[\+ ]([A-Z]):\s*(.*)/; + my $prev = $1; + my $prevval = $2; + my $curindex = index($preferred_order, $cur); + my $previndex = index($preferred_order, $prev); + if ($curindex < 0) { + WARN("MAINTAINERS_STYLE", + "Unknown MAINTAINERS entry type: '$cur'\n" . $herecurr); + } else { + if ($previndex >= 0 && $curindex < $previndex) { + WARN("MAINTAINERS_STYLE", + "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev); + } elsif ((($prev eq 'F' && $cur eq 'F') || + ($prev eq 'X' && $cur eq 'X')) && + ($prevval cmp $curval) > 0) { + WARN("MAINTAINERS_STYLE", + "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev); + } + } } - } - -# discourage the use of boolean for type definition attributes of Kconfig options - if ($realfile =~ /Kconfig/ && - $line =~ /^\+\s*\bboolean\b/) { - WARN("CONFIG_TYPE_BOOLEAN", - "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); } if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && @@ -3284,6 +3721,12 @@ sub process { } } +# check for embedded filenames + if ($rawline =~ /^\+.*\Q$realfile\E/) { + WARN("EMBEDDED_FILENAME", + "It's generally not useful to have the filename in the file\n" . $herecurr); + } + # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); @@ -3361,8 +3804,18 @@ sub process { # check for adding lines without a newline. if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) { - WARN("MISSING_EOF_NEWLINE", - "adding a line without newline at end of file\n" . $herecurr); + if (WARN("MISSING_EOF_NEWLINE", + "adding a line without newline at end of file\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr+1, "No newline at end of file"); + } + } + +# check for .L prefix local symbols in .S files + if ($realfile =~ /\.S$/ && + $line =~ /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) { + WARN("AVOID_L_PREFIX", + "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr); } if ($u_boot) { @@ -3400,14 +3853,28 @@ sub process { # check for assignments on the start of a line if ($sline =~ /^\+\s+($Assignment)[^=]/) { - CHK("ASSIGNMENT_CONTINUATIONS", - "Assignment operator '$1' should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("ASSIGNMENT_CONTINUATIONS", + "Assignment operator '$1' should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + # add assignment operator to the previous line, remove from current line + $fixed[$fixlinenr - 1] .= " $operator"; + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } } # check for && or || at the start of a line if ($rawline =~ /^\+\s*(&&|\|\|)/) { - CHK("LOGICAL_CONTINUATIONS", - "Logical continuations should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("LOGICAL_CONTINUATIONS", + "Logical continuations should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + # insert logical operator at last non-comment, non-whitepsace char on previous line + $prevline =~ /[\s$;]*$/; + my $line_end = substr($prevrawline, $-[0]); + $fixed[$fixlinenr - 1] =~ s/\Q$line_end\E$/ $operator$line_end/; + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } } # check indentation starts on a tab stop @@ -3475,7 +3942,7 @@ sub process { if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ && $rawline =~ /^\+[ \t]*\*/ && - $realline > 2) { + $realline > 3) { # Do not warn about the initial copyright comment block after SPDX-License-Identifier WARN("NETWORKING_BLOCK_COMMENT_STYLE", "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev); } @@ -3557,43 +4024,48 @@ sub process { } # check for missing blank lines after declarations - if ($sline =~ /^\+\s+\S/ && #Not at char 1 - # actual declarations - ($prevline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || +# (declarations must have the same indentation and not be at the start of line) + if (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/) { + # use temporaries + my $sl = $sline; + my $pl = $prevline; + # remove $Attribute/$Sparse uses to simplify comparisons + $sl =~ s/\b(?:$Attribute|$Sparse)\b//g; + $pl =~ s/\b(?:$Attribute|$Sparse)\b//g; + if (($pl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || # function pointer declarations - $prevline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || + $pl =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || # foo bar; where foo is some local typedef or #define - $prevline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + $pl =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || # known declaration macros - $prevline =~ /^\+\s+$declaration_macros/) && + $pl =~ /^\+\s+$declaration_macros/) && # for "else if" which can look like "$Ident $Ident" - !($prevline =~ /^\+\s+$c90_Keywords\b/ || + !($pl =~ /^\+\s+$c90_Keywords\b/ || # other possible extensions of declaration lines - $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + $pl =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || # not starting a section or a macro "\" extended line - $prevline =~ /(?:\{\s*|\\)$/) && + $pl =~ /(?:\{\s*|\\)$/) && # looks like a declaration - !($sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + !($sl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || # function pointer declarations - $sline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || + $sl =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || # foo bar; where foo is some local typedef or #define - $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + $sl =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || # known declaration macros - $sline =~ /^\+\s+$declaration_macros/ || + $sl =~ /^\+\s+$declaration_macros/ || # start of struct or union or enum - $sline =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || + $sl =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || # start or end of block or continuation of declaration - $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || + $sl =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || # bitfield continuation - $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || + $sl =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || # other possible extensions of declaration lines - $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/) && - # indentation of previous and current line are the same - (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/)) { - if (WARN("LINE_SPACING", - "Missing a blank line after declarations\n" . $hereprev) && - $fix) { - fix_insert_line($fixlinenr, "\+"); + $sl =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) { + if (WARN("LINE_SPACING", + "Missing a blank line after declarations\n" . $hereprev) && + $fix) { + fix_insert_line($fixlinenr, "\+"); + } } } @@ -3646,12 +4118,16 @@ sub process { } # check indentation of a line with a break; -# if the previous line is a goto or return and is indented the same # of tabs +# if the previous line is a goto, return or break +# and is indented the same # of tabs if ($sline =~ /^\+([\t]+)break\s*;\s*$/) { my $tabs = $1; - if ($prevline =~ /^\+$tabs(?:goto|return)\b/) { - WARN("UNNECESSARY_BREAK", - "break is not useful after a goto or return\n" . $hereprev); + if ($prevline =~ /^\+$tabs(goto|return|break)\b/) { + if (WARN("UNNECESSARY_BREAK", + "break is not useful after a $1\n" . $hereprev) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } } @@ -3934,6 +4410,17 @@ sub process { #ignore lines not being added next if ($line =~ /^[^\+]/); +# check for self assignments used to avoid compiler warnings +# e.g.: int foo = foo, *bar = NULL; +# struct foo bar = *(&(bar)); + if ($line =~ /^\+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) { + my $var = $1; + if ($line =~ /^\+\s*(?:$Declare)?$var\s*=\s*(?:$var|\*\s*\(?\s*&\s*\(?\s*$var\s*\)?\s*\)?)\s*[;,]/) { + WARN("SELF_ASSIGNMENT", + "Do not use self-assignments to avoid compiler warnings\n" . $herecurr); + } + } + # check for dereferences that span multiple lines if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ && $line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) { @@ -4049,8 +4536,7 @@ sub process { if (defined $realline_next && exists $lines[$realline_next - 1] && !defined $suppress_export{$realline_next} && - ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ || - $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/)) { # Handle definitions which produce identifiers with # a prefix: # XXX(foo); @@ -4077,8 +4563,7 @@ sub process { } if (!defined $suppress_export{$linenr} && $prevline =~ /^.\s*$/ && - ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ || - $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/)) { #print "FOO B <$lines[$linenr - 1]>\n"; $suppress_export{$linenr} = 2; } @@ -4089,7 +4574,8 @@ sub process { } # check for global initialisers. - if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) { + if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/ && + !exclude_global_initialisers($realfile)) { if (ERROR("GLOBAL_INITIALISERS", "do not initialise globals to $1\n" . $herecurr) && $fix) { @@ -4168,12 +4654,24 @@ sub process { } } +# check for const static or static <non ptr type> const declarations +# prefer 'static const <foo>' over 'const static <foo>' and 'static <foo> const' + if ($sline =~ /^\+\s*const\s+static\s+($Type)\b/ || + $sline =~ /^\+\s*static\s+($BasicType)\s+const\b/) { + if (WARN("STATIC_CONST", + "Move const after static - use 'static const $1'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bconst\s+static\b/static const/; + $fixed[$fixlinenr] =~ s/\bstatic\s+($BasicType)\s+const\b/static const $1/; + } + } + # check for non-global char *foo[] = {"bar", ...} declarations. if ($line =~ /^.\s+(?:static\s+|const\s+)?char\s+\*\s*\w+\s*\[\s*\]\s*=\s*\{/) { WARN("STATIC_CONST_CHAR_ARRAY", "char * array declaration might be better as static const\n" . $herecurr); - } + } # check for sizeof(foo)/sizeof(foo[0]) that could be ARRAY_SIZE(foo) if ($line =~ m@\bsizeof\s*\(\s*($Lval)\s*\)@) { @@ -4290,16 +4788,23 @@ sub process { "printk() should include KERN_<LEVEL> facility level\n" . $herecurr); } - if ($line =~ /\bprintk\s*\(\s*KERN_([A-Z]+)/) { - my $orig = $1; +# prefer variants of (subsystem|netdev|dev|pr)_<level> to printk(KERN_<LEVEL> + if ($line =~ /\b(printk(_once|_ratelimited)?)\s*\(\s*KERN_([A-Z]+)/) { + my $printk = $1; + my $modifier = $2; + my $orig = $3; + $modifier = "" if (!defined($modifier)); my $level = lc($orig); $level = "warn" if ($level eq "warning"); my $level2 = $level; $level2 = "dbg" if ($level eq "debug"); + $level .= $modifier; + $level2 .= $modifier; WARN("PREFER_PR_LEVEL", - "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then pr_$level(... to printk(KERN_$orig ...\n" . $herecurr); + "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then pr_$level(... to $printk(KERN_$orig ...\n" . $herecurr); } +# prefer dev_<level> to dev_printk(KERN_<LEVEL> if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) { my $orig = $1; my $level = lc($orig); @@ -4309,6 +4814,12 @@ sub process { "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr); } +# trace_printk should not be used in production code. + if ($line =~ /\b(trace_printk|trace_puts|ftrace_vprintk)\s*\(/) { + WARN("TRACE_PRINTK", + "Do not use $1() in production code (this can be ignored if built only with a debug config option)\n" . $herecurr); + } + # ENOSYS means "bad syscall nr" and nothing else. This will have a small # number of false positives, but assembly files are not checked, so at # least the arch entry code will not trigger this warning. @@ -4317,6 +4828,17 @@ sub process { "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr); } +# ENOTSUPP is not a standard error code and should be avoided in new patches. +# Folks usually mean EOPNOTSUPP (also called ENOTSUP), when they type ENOTSUPP. +# Similarly to ENOSYS warning a small number of false positives is expected. + if (!$file && $line =~ /\bENOTSUPP\b/) { + if (WARN("ENOTSUPP", + "ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bENOTSUPP\b/EOPNOTSUPP/; + } + } + # function brace can't be on same line, except for #defines of do while, # or if closed on same line if ($perl_version_ok && @@ -4328,7 +4850,7 @@ sub process { $fix) { fix_delete_line($fixlinenr, $rawline); my $fixed_line = $rawline; - $fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*){(.*)$/; + $fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*)\{(.*)$/; my $line1 = $1; my $line2 = $2; fix_insert_line($fixlinenr, ltrim($line1)); @@ -4739,7 +5261,7 @@ sub process { # A colon needs no spaces before when it is # terminating a case value or a label. } elsif ($opv eq ':C' || $opv eq ':L') { - if ($ctx =~ /Wx./) { + if ($ctx =~ /Wx./ and $realfile !~ m@.*\.lds\.h$@) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); @@ -4823,7 +5345,7 @@ sub process { ## $line !~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Type\s*$Ident.*/) { ## ## # Remove any bracketed sections to ensure we do not -## # falsly report the parameters of functions. +## # falsely report the parameters of functions. ## my $ln = $line; ## while ($ln =~ s/\([^\(\)]*\)//g) { ## } @@ -4964,6 +5486,17 @@ sub process { } } +# check if a statement with a comma should be two statements like: +# foo = bar(), /* comma should be semicolon */ +# bar = baz(); + if (defined($stat) && + $stat =~ /^\+\s*(?:$Lval\s*$Assignment\s*)?$FuncArg\s*,\s*(?:$Lval\s*$Assignment\s*)?$FuncArg\s*;\s*$/) { + my $cnt = statement_rawlines($stat); + my $herectx = get_stat_here($linenr, $cnt, $here); + WARN("SUSPECT_COMMA_SEMICOLON", + "Possible comma where semicolon could be used\n" . $herectx); + } + # return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)\(/s) { my $spacing = $1; @@ -4991,7 +5524,7 @@ sub process { $lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { WARN("RETURN_VOID", "void function return statements are not generally useful\n" . $hereprev); - } + } # if statements using unnecessary parentheses - ie: if ((foo == bar)) if ($perl_version_ok && @@ -5084,8 +5617,30 @@ sub process { my ($s, $c) = ($stat, $cond); if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) { - ERROR("ASSIGN_IN_IF", - "do not use assignment in if condition\n" . $herecurr); + if (ERROR("ASSIGN_IN_IF", + "do not use assignment in if condition\n" . $herecurr) && + $fix && $perl_version_ok) { + if ($rawline =~ /^\+(\s+)if\s*\(\s*(\!)?\s*\(\s*(($Lval)\s*=\s*$LvalOrFunc)\s*\)\s*(?:($Compare)\s*($FuncArg))?\s*\)\s*(\{)?\s*$/) { + my $space = $1; + my $not = $2; + my $statement = $3; + my $assigned = $4; + my $test = $8; + my $against = $9; + my $brace = $15; + fix_delete_line($fixlinenr, $rawline); + fix_insert_line($fixlinenr, "$space$statement;"); + my $newline = "${space}if ("; + $newline .= '!' if defined($not); + $newline .= '(' if (defined $not && defined($test) && defined($against)); + $newline .= "$assigned"; + $newline .= " $test $against" if (defined($test) && defined($against)); + $newline .= ')' if (defined $not && defined($test) && defined($against)); + $newline .= ')'; + $newline .= " {" if (defined($brace)); + fix_insert_line($fixlinenr + 1, $newline); + } + } } # Find out what is on the end of the line after the @@ -5206,6 +5761,8 @@ sub process { #CamelCase if ($var !~ /^$Constant$/ && $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && +#Ignore some autogenerated defines and enum values + $var !~ /^(?:[A-Z]+_){1,5}[A-Z]{1,3}[a-z]/ && #Ignore Page<foo> variants $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && #Ignore SI style variants like nS, mV and dB @@ -5301,9 +5858,9 @@ sub process { $dstat =~ s/\s*$//s; # Flatten any parentheses and braces - while ($dstat =~ s/\([^\(\)]*\)/1/ || - $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/.\[[^\[\]]*\]/1/) + while ($dstat =~ s/\([^\(\)]*\)/1u/ || + $dstat =~ s/\{[^\{\}]*\}/1u/ || + $dstat =~ s/.\[[^\[\]]*\]/1u/) { } @@ -5344,6 +5901,7 @@ sub process { $dstat !~ /^\.$Ident\s*=/ && # .foo = $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ && # stringification #foo $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...) + $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ && # while (...) {...} $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... @@ -5385,7 +5943,7 @@ sub process { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(sizeof|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; $tmp_stmt =~ s/\#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*\#\#//g; my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g; @@ -5662,6 +6220,17 @@ sub process { "Prefer using '\"%s...\", __func__' to using '$context_function', this function's name, in a string\n" . $herecurr); } +# check for unnecessary function tracing like uses +# This does not use $logFunctions because there are many instances like +# 'dprintk(FOO, "%s()\n", __func__);' which do not match $logFunctions + if ($rawline =~ /^\+.*\([^"]*"$tracing_logging_tags{0,3}%s(?:\s*\(\s*\)\s*)?$tracing_logging_tags{0,3}(?:\\n)?"\s*,\s*__func__\s*\)\s*;/) { + if (WARN("TRACING_LOGGING", + "Unnecessary ftrace-like logging - prefer using ftrace\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + } + # check for spaces before a quoted newline if ($rawline =~ /^.*\".*\s\\n/) { if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", @@ -5808,6 +6377,28 @@ sub process { "Avoid logging continuation uses where feasible\n" . $herecurr); } +# check for unnecessary use of %h[xudi] and %hh[xudi] in logging functions + if (defined $stat && + $line =~ /\b$logFunctions\s*\(/ && + index($stat, '"') >= 0) { + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = get_stat_real($linenr, $lc); + pos($stat_real) = index($stat_real, '"'); + while ($stat_real =~ /[^\"%]*(%[\#\d\.\*\-]*(h+)[idux])/g) { + my $pspec = $1; + my $h = $2; + my $lineoff = substr($stat_real, 0, $-[1]) =~ tr@\n@@; + if (WARN("UNNECESSARY_MODIFIER", + "Integer promotion: Using '$h' in '$pspec' is unnecessary\n" . "$here\n$stat_real\n") && + $fix && $fixed[$fixlinenr + $lineoff] =~ /^\+/) { + my $nspec = $pspec; + $nspec =~ s/h//g; + $fixed[$fixlinenr + $lineoff] =~ s/\Q$pspec\E/$nspec/; + } + } + } + # check for mask then right shift without a parentheses if ($perl_version_ok && $line =~ /$LvalOrFunc\s*\&\s*($LvalOrFunc)\s*>>/ && @@ -5966,8 +6557,7 @@ sub process { my $barriers = qr{ mb| rmb| - wmb| - read_barrier_depends + wmb }x; my $barrier_stems = qr{ mb__before_atomic| @@ -6008,10 +6598,12 @@ sub process { } } -# check for smp_read_barrier_depends and read_barrier_depends - if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) { - WARN("READ_BARRIER_DEPENDS", - "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr); +# check for data_race without a comment. + if ($line =~ /\bdata_race\s*\(/) { + if (!ctx_has_comment($first_line, $linenr)) { + WARN("DATA_RACE", + "data_race without comment\n" . $herecurr); + } } # check of hardware specific defines @@ -6053,50 +6645,68 @@ sub process { } } -# Check for __attribute__ packed, prefer __packed +# Check for compiler attributes if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) { - WARN("PREFER_PACKED", - "__packed is preferred over __attribute__((packed))\n" . $herecurr); - } + $rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) { + my $attr = $1; + $attr =~ s/\s*\(\s*(.*)\)\s*/$1/; -# Check for __attribute__ aligned, prefer __aligned - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(.*aligned/) { - WARN("PREFER_ALIGNED", - "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); - } + my %attr_list = ( + "alias" => "__alias", + "aligned" => "__aligned", + "always_inline" => "__always_inline", + "assume_aligned" => "__assume_aligned", + "cold" => "__cold", + "const" => "__attribute_const__", + "copy" => "__copy", + "designated_init" => "__designated_init", + "externally_visible" => "__visible", + "format" => "printf|scanf", + "gnu_inline" => "__gnu_inline", + "malloc" => "__malloc", + "mode" => "__mode", + "no_caller_saved_registers" => "__no_caller_saved_registers", + "noclone" => "__noclone", + "noinline" => "noinline", + "nonstring" => "__nonstring", + "noreturn" => "__noreturn", + "packed" => "__packed", + "pure" => "__pure", + "section" => "__section", + "used" => "__used", + "weak" => "__weak" + ); -# Check for __attribute__ section, prefer __section - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) { - my $old = substr($rawline, $-[1], $+[1] - $-[1]); - my $new = substr($old, 1, -1); - if (WARN("PREFER_SECTION", - "__section(\"$new\") is preferred over __attribute__((section($old)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/; + while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { + my $orig_attr = $1; + my $params = ''; + $params = $2 if defined($2); + my $curr_attr = $orig_attr; + $curr_attr =~ s/^[\s_]+|[\s_]+$//g; + if (exists($attr_list{$curr_attr})) { + my $new = $attr_list{$curr_attr}; + if ($curr_attr eq "format" && $params) { + $params =~ /^\s*\(\s*(\w+)\s*,\s*(.*)/; + $new = "__$1\($2"; + } else { + $new = "$new$params"; + } + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "Prefer $new over __attribute__(($orig_attr$params))\n" . $herecurr) && + $fix) { + my $remove = "\Q$orig_attr\E" . '\s*' . "\Q$params\E" . '(?:\s*,\s*)?'; + $fixed[$fixlinenr] =~ s/$remove//; + $fixed[$fixlinenr] =~ s/\b__attribute__/$new __attribute__/; + $fixed[$fixlinenr] =~ s/\}\Q$new\E/} $new/; + $fixed[$fixlinenr] =~ s/ __attribute__\s*\(\s*\(\s*\)\s*\)//; + } + } } - } -# Check for __attribute__ format(printf, prefer __printf - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) { - if (WARN("PREFER_PRINTF", - "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex; - - } - } - -# Check for __attribute__ format(scanf, prefer __scanf - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\b/) { - if (WARN("PREFER_SCANF", - "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex; + # Check for __attribute__ unused, prefer __always_unused or __maybe_unused + if ($attr =~ /^_*unused/) { + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr); } } @@ -6132,18 +6742,18 @@ sub process { if ($line =~ /(\(\s*$C90_int_types\s*\)\s*)($Constant)\b/) { my $cast = $1; my $const = $2; + my $suffix = ""; + my $newconst = $const; + $newconst =~ s/${Int_type}$//; + $suffix .= 'U' if ($cast =~ /\bunsigned\b/); + if ($cast =~ /\blong\s+long\b/) { + $suffix .= 'LL'; + } elsif ($cast =~ /\blong\b/) { + $suffix .= 'L'; + } if (WARN("TYPECAST_INT_CONSTANT", - "Unnecessary typecast of c90 int constant\n" . $herecurr) && + "Unnecessary typecast of c90 int constant - '$cast$const' could be '$const$suffix'\n" . $herecurr) && $fix) { - my $suffix = ""; - my $newconst = $const; - $newconst =~ s/${Int_type}$//; - $suffix .= 'U' if ($cast =~ /\bunsigned\b/); - if ($cast =~ /\blong\s+long\b/) { - $suffix .= 'LL'; - } elsif ($cast =~ /\blong\b/) { - $suffix .= 'L'; - } $fixed[$fixlinenr] =~ s/\Q$cast\E$const\b/$newconst$suffix/; } } @@ -6203,9 +6813,11 @@ sub process { $specifier = $1; $extension = $2; $qualifier = $3; - if ($extension !~ /[SsBKRraEehMmIiUDdgVCbGNOxtf]/ || + if ($extension !~ /[4SsBKRraEehMmIiUDdgVCbGNOxtf]/ || ($extension eq "f" && - defined $qualifier && $qualifier !~ /^w/)) { + defined $qualifier && $qualifier !~ /^w/) || + ($extension eq "4" && + defined $qualifier && $qualifier !~ /^cc/)) { $bad_specifier = $specifier; last; } @@ -6385,8 +6997,7 @@ sub process { if (defined $cond) { substr($s, 0, length($cond), ''); } - if ($s =~ /^\s*;/ && - $function_name ne 'uninitialized_var') + if ($s =~ /^\s*;/) { WARN("AVOID_EXTERNS", "externs should be avoided in .c files\n" . $herecurr); @@ -6405,17 +7016,13 @@ sub process { } # check for function declarations that have arguments without identifier names -# while avoiding uninitialized_var(x) if (defined $stat && - $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:($Ident)|\(\s*\*\s*$Ident\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s && - (!defined($1) || - (defined($1) && $1 ne "uninitialized_var")) && - $2 ne "void") { - my $args = trim($2); + $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s && + $1 ne "void") { + my $args = trim($1); while ($args =~ m/\s*($Type\s*(?:$Ident|\(\s*\*\s*$Ident?\s*\)\s*$balanced_parens)?)/g) { my $arg = trim($1); - if ($arg =~ /^$Type$/ && - $arg !~ /enum\s+$Ident$/) { + if ($arg =~ /^$Type$/ && $arg !~ /enum\s+$Ident$/) { WARN("FUNCTION_ARGUMENTS", "function definition argument '$arg' should also have an identifier name\n" . $herecurr); } @@ -6451,7 +7058,7 @@ sub process { if (!grep(/$name/, @setup_docs)) { CHK("UNDOCUMENTED_SETUP", - "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.rst\n" . $herecurr); + "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.txt\n" . $herecurr); } } @@ -6507,7 +7114,7 @@ sub process { } # check for alloc argument mismatch - if ($line =~ /\b(kcalloc|kmalloc_array)\s*\(\s*sizeof\b/) { + if ($line =~ /\b((?:devm_)?(?:kcalloc|kmalloc_array))\s*\(\s*sizeof\b/) { WARN("ALLOC_ARRAY_ARGS", "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } @@ -6533,38 +7140,19 @@ sub process { } } -# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE - if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) { - my $config = $1; - if (WARN("PREFER_IS_ENABLED", - "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; - } +# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too) + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^${CONFIG_}/) { + WARN("IS_ENABLED_CONFIG", + "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr); } -# check for case / default statements not preceded by break/fallthrough/switch - if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) { - my $has_break = 0; - my $has_statement = 0; - my $count = 0; - my $prevline = $linenr; - while ($prevline > 1 && ($file || $count < 3) && !$has_break) { - $prevline--; - my $rline = $rawlines[$prevline - 1]; - my $fline = $lines[$prevline - 1]; - last if ($fline =~ /^\@\@/); - next if ($fline =~ /^\-/); - next if ($fline =~ /^.(?:\s*(?:case\s+(?:$Ident|$Constant)[\s$;]*|default):[\s$;]*)*$/); - $has_break = 1 if ($rline =~ /fall[\s_-]*(through|thru)/i); - next if ($fline =~ /^.[\s$;]*$/); - $has_statement = 1; - $count++; - $has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|exit\s*\(\b|return\b|goto\b|continue\b)/); - } - if (!$has_break && $has_statement) { - WARN("MISSING_BREAK", - "Possible switch case/default not preceded by break or fallthrough comment\n" . $herecurr); +# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) { + my $config = $1; + if (WARN("PREFER_IS_ENABLED", + "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; } } @@ -6683,7 +7271,8 @@ sub process { # check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' - if ($line !~ /\bconst\b/ && + if (defined($const_structs) && + $line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); @@ -6691,12 +7280,14 @@ sub process { # use of NR_CPUS is usually wrong # ignore definitions of NR_CPUS and usage to define arrays as likely right +# ignore designated initializers using NR_CPUS if ($line =~ /\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*if\b.*\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*define\b.*\bNR_CPUS\b/ && $line !~ /^.\s*$Declare\s.*\[[^\]]*NR_CPUS[^\]]*\]/ && $line !~ /\[[^\]]*\.\.\.[^\]]*NR_CPUS[^\]]*\]/ && - $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/) + $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/ && + $line !~ /^.\s*\.\w+\s*=\s*.*\bNR_CPUS\b/) { WARN("NR_CPUS", "usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); @@ -6715,6 +7306,17 @@ sub process { "Using $1 should generally have parentheses around the comparison\n" . $herecurr); } +# return sysfs_emit(foo, fmt, ...) fmt without newline + if ($line =~ /\breturn\s+sysfs_emit\s*\(\s*$FuncArg\s*,\s*($String)/ && + substr($rawline, $-[6], $+[6] - $-[6]) !~ /\\n"$/) { + my $offset = $+[6] - 1; + if (WARN("SYSFS_EMIT", + "return sysfs_emit(...) formats should include a terminating newline\n" . $herecurr) && + $fix) { + substr($fixed[$fixlinenr], $offset, 0) = '\\n'; + } + } + # nested likely/unlikely calls if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { WARN("LIKELY_MISUSE", @@ -6732,12 +7334,6 @@ sub process { } } -# check for mutex_trylock_recursive usage - if ($line =~ /mutex_trylock_recursive/) { - ERROR("LOCKING", - "recursive locking is bad, do not use this ever.\n" . $herecurr); - } - # check for lockdep_set_novalidate_class if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ || $line =~ /__lockdep_no_validate__\s*\)/ ) { @@ -6900,7 +7496,7 @@ sub process { exit(0); } - # This is not a patch, and we are are in 'no-patch' mode so + # This is not a patch, and we are in 'no-patch' mode so # just keep quiet. if (!$chk_patch && !$is_patch) { exit(0); @@ -6914,9 +7510,33 @@ sub process { if ($signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); - } elsif (!$authorsignoff) { - WARN("NO_AUTHOR_SIGN_OFF", - "Missing Signed-off-by: line by nominal patch author '$author'\n"); + } elsif ($authorsignoff != 1) { + # authorsignoff values: + # 0 -> missing sign off + # 1 -> sign off identical + # 2 -> names and addresses match, comments mismatch + # 3 -> addresses match, names different + # 4 -> names match, addresses different + # 5 -> names match, addresses excluding subaddress details (refer RFC 5233) match + + my $sob_msg = "'From: $author' != 'Signed-off-by: $author_sob'"; + + if ($authorsignoff == 0) { + ERROR("NO_AUTHOR_SIGN_OFF", + "Missing Signed-off-by: line by nominal patch author '$author'\n"); + } elsif ($authorsignoff == 2) { + CHK("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email comments mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 3) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email name mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 4) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email address mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 5) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email subaddress mismatch: $sob_msg\n"); + } } } diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py new file mode 100755 index 0000000000..3e784cf9f4 --- /dev/null +++ b/scripts/spdxcheck.py @@ -0,0 +1,296 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 +# Copyright Thomas Gleixner <tglx@linutronix.de> + +from argparse import ArgumentParser +from ply import lex, yacc +import locale +import traceback +import sys +import git +import re +import os + +class ParserException(Exception): + def __init__(self, tok, txt): + self.tok = tok + self.txt = txt + +class SPDXException(Exception): + def __init__(self, el, txt): + self.el = el + self.txt = txt + +class SPDXdata(object): + def __init__(self): + self.license_files = 0 + self.exception_files = 0 + self.licenses = [ ] + self.exceptions = { } + +# Read the spdx data from the LICENSES directory +def read_spdxdata(repo): + + # The subdirectories of LICENSES in the kernel source + # Note: exceptions needs to be parsed as last directory. + license_dirs = [ "preferred", "dual", "deprecated", "exceptions" ] + lictree = repo.head.commit.tree['LICENSES'] + + spdx = SPDXdata() + + for d in license_dirs: + for el in lictree[d].traverse(): + if not os.path.isfile(el.path): + continue + + exception = None + for l in open(el.path).readlines(): + if l.startswith('Valid-License-Identifier:'): + lid = l.split(':')[1].strip().upper() + if lid in spdx.licenses: + raise SPDXException(el, 'Duplicate License Identifier: %s' %lid) + else: + spdx.licenses.append(lid) + + elif l.startswith('SPDX-Exception-Identifier:'): + exception = l.split(':')[1].strip().upper() + spdx.exceptions[exception] = [] + + elif l.startswith('SPDX-Licenses:'): + for lic in l.split(':')[1].upper().strip().replace(' ', '').replace('\t', '').split(','): + if not lic in spdx.licenses: + raise SPDXException(None, 'Exception %s missing license %s' %(exception, lic)) + spdx.exceptions[exception].append(lic) + + elif l.startswith("License-Text:"): + if exception: + if not len(spdx.exceptions[exception]): + raise SPDXException(el, 'Exception %s is missing SPDX-Licenses' %exception) + spdx.exception_files += 1 + else: + spdx.license_files += 1 + break + return spdx + +class id_parser(object): + + reserved = [ 'AND', 'OR', 'WITH' ] + tokens = [ 'LPAR', 'RPAR', 'ID', 'EXC' ] + reserved + + precedence = ( ('nonassoc', 'AND', 'OR'), ) + + t_ignore = ' \t' + + def __init__(self, spdx): + self.spdx = spdx + self.lasttok = None + self.lastid = None + self.lexer = lex.lex(module = self, reflags = re.UNICODE) + # Initialize the parser. No debug file and no parser rules stored on disk + # The rules are small enough to be generated on the fly + self.parser = yacc.yacc(module = self, write_tables = False, debug = False) + self.lines_checked = 0 + self.checked = 0 + self.spdx_valid = 0 + self.spdx_errors = 0 + self.curline = 0 + self.deepest = 0 + + # Validate License and Exception IDs + def validate(self, tok): + id = tok.value.upper() + if tok.type == 'ID': + if not id in self.spdx.licenses: + raise ParserException(tok, 'Invalid License ID') + self.lastid = id + elif tok.type == 'EXC': + if id not in self.spdx.exceptions: + raise ParserException(tok, 'Invalid Exception ID') + if self.lastid not in self.spdx.exceptions[id]: + raise ParserException(tok, 'Exception not valid for license %s' %self.lastid) + self.lastid = None + elif tok.type != 'WITH': + self.lastid = None + + # Lexer functions + def t_RPAR(self, tok): + r'\)' + self.lasttok = tok.type + return tok + + def t_LPAR(self, tok): + r'\(' + self.lasttok = tok.type + return tok + + def t_ID(self, tok): + r'[A-Za-z.0-9\-+]+' + + if self.lasttok == 'EXC': + print(tok) + raise ParserException(tok, 'Missing parentheses') + + tok.value = tok.value.strip() + val = tok.value.upper() + + if val in self.reserved: + tok.type = val + elif self.lasttok == 'WITH': + tok.type = 'EXC' + + self.lasttok = tok.type + self.validate(tok) + return tok + + def t_error(self, tok): + raise ParserException(tok, 'Invalid token') + + def p_expr(self, p): + '''expr : ID + | ID WITH EXC + | expr AND expr + | expr OR expr + | LPAR expr RPAR''' + pass + + def p_error(self, p): + if not p: + raise ParserException(None, 'Unfinished license expression') + else: + raise ParserException(p, 'Syntax error') + + def parse(self, expr): + self.lasttok = None + self.lastid = None + self.parser.parse(expr, lexer = self.lexer) + + def parse_lines(self, fd, maxlines, fname): + self.checked += 1 + self.curline = 0 + try: + for line in fd: + line = line.decode(locale.getpreferredencoding(False), errors='ignore') + self.curline += 1 + if self.curline > maxlines: + break + self.lines_checked += 1 + if line.find("SPDX-License-Identifier:") < 0: + continue + expr = line.split(':')[1].strip() + # Remove trailing comment closure + if line.strip().endswith('*/'): + expr = expr.rstrip('*/').strip() + # Remove trailing xml comment closure + if line.strip().endswith('-->'): + expr = expr.rstrip('-->').strip() + # Special case for SH magic boot code files + if line.startswith('LIST \"'): + expr = expr.rstrip('\"').strip() + self.parse(expr) + self.spdx_valid += 1 + # + # Should we check for more SPDX ids in the same file and + # complain if there are any? + # + break + + except ParserException as pe: + if pe.tok: + col = line.find(expr) + pe.tok.lexpos + tok = pe.tok.value + sys.stdout.write('%s: %d:%d %s: %s\n' %(fname, self.curline, col, pe.txt, tok)) + else: + sys.stdout.write('%s: %d:0 %s\n' %(fname, self.curline, col, pe.txt)) + self.spdx_errors += 1 + +def scan_git_tree(tree): + for el in tree.traverse(): + # Exclude stuff which would make pointless noise + # FIXME: Put this somewhere more sensible + if el.path.startswith("LICENSES"): + continue + if el.path.find("license-rules.rst") >= 0: + continue + if not os.path.isfile(el.path): + continue + with open(el.path, 'rb') as fd: + parser.parse_lines(fd, args.maxlines, el.path) + +def scan_git_subtree(tree, path): + for p in path.strip('/').split('/'): + tree = tree[p] + scan_git_tree(tree) + +if __name__ == '__main__': + + ap = ArgumentParser(description='SPDX expression checker') + ap.add_argument('path', nargs='*', help='Check path or file. If not given full git tree scan. For stdin use "-"') + ap.add_argument('-m', '--maxlines', type=int, default=15, + help='Maximum number of lines to scan in a file. Default 15') + ap.add_argument('-v', '--verbose', action='store_true', help='Verbose statistics output') + args = ap.parse_args() + + # Sanity check path arguments + if '-' in args.path and len(args.path) > 1: + sys.stderr.write('stdin input "-" must be the only path argument\n') + sys.exit(1) + + try: + # Use git to get the valid license expressions + repo = git.Repo(os.getcwd()) + assert not repo.bare + + # Initialize SPDX data + spdx = read_spdxdata(repo) + + # Initialize the parser + parser = id_parser(spdx) + + except SPDXException as se: + if se.el: + sys.stderr.write('%s: %s\n' %(se.el.path, se.txt)) + else: + sys.stderr.write('%s\n' %se.txt) + sys.exit(1) + + except Exception as ex: + sys.stderr.write('FAIL: %s\n' %ex) + sys.stderr.write('%s\n' %traceback.format_exc()) + sys.exit(1) + + try: + if len(args.path) and args.path[0] == '-': + stdin = os.fdopen(sys.stdin.fileno(), 'rb') + parser.parse_lines(stdin, args.maxlines, '-') + else: + if args.path: + for p in args.path: + if os.path.isfile(p): + parser.parse_lines(open(p, 'rb'), args.maxlines, p) + elif os.path.isdir(p): + scan_git_subtree(repo.head.reference.commit.tree, p) + else: + sys.stderr.write('path %s does not exist\n' %p) + sys.exit(1) + else: + # Full git tree scan + scan_git_tree(repo.head.commit.tree) + + if args.verbose: + sys.stderr.write('\n') + sys.stderr.write('License files: %12d\n' %spdx.license_files) + sys.stderr.write('Exception files: %12d\n' %spdx.exception_files) + sys.stderr.write('License IDs %12d\n' %len(spdx.licenses)) + sys.stderr.write('Exception IDs %12d\n' %len(spdx.exceptions)) + sys.stderr.write('\n') + sys.stderr.write('Files checked: %12d\n' %parser.checked) + sys.stderr.write('Lines checked: %12d\n' %parser.lines_checked) + sys.stderr.write('Files with SPDX: %12d\n' %parser.spdx_valid) + sys.stderr.write('Files with errors: %12d\n' %parser.spdx_errors) + + sys.exit(0) + + except Exception as ex: + sys.stderr.write('FAIL: %s\n' %ex) + sys.stderr.write('%s\n' %traceback.format_exc()) + sys.exit(1) From 515381414d5ce2032fbee6bb55206061660762bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> Date: Fri, 6 Aug 2021 18:07:39 +0200 Subject: [PATCH 09/19] loadb: Properly indicate aborted kermit transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When k_recv() returns zero it indicates that kermit transfer was aborted. Function do_load_serial_bin() (caller of load_serial_bin()) interprets value ~0 as aborted transfer, so properly propagates information about aborted transfer from k_recv() to do_load_serial_bin(). Signed-off-by: Pali Rohár <pali@kernel.org> --- cmd/load.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/load.c b/cmd/load.c index 381ed1b3e2..3904e133c4 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -535,6 +535,9 @@ static ulong load_serial_bin(ulong offset) udelay(1000); } + if (size == 0) + return ~0; /* Download aborted */ + flush_cache(offset, size); printf("## Total Size = 0x%08x = %d Bytes\n", size, size); From 7093df3c75576f90c69503e8cf832d221ab6878e Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:18 +0200 Subject: [PATCH 10/19] MAINTAINERS: Add new drivers for ARM U8500 Update MAINTAINERS with various drivers for ARM U8500 that were added during the U-Boot 2021.10 merge window. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- MAINTAINERS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..01276dbee7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -532,7 +532,12 @@ R: Linus Walleij <linus.walleij@linaro.org> S: Maintained F: arch/arm/dts/ste-* F: arch/arm/mach-u8500/ +F: drivers/gpio/nmk_gpio.c +F: drivers/phy/phy-ab8500-usb.c +F: drivers/power/pmic/ab8500.c F: drivers/timer/nomadik-mtu-timer.c +F: drivers/usb/musb-new/ux500.c +F: drivers/video/mcde_simple.c ARM UNIPHIER S: Orphan (Since 2020-09) From 9f78ccf18679a17ebfd5c9e3de40d824faf24f63 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:19 +0200 Subject: [PATCH 11/19] arm: u8500: Imply options for new drivers Imply the options for new drivers added for ARM U8500 during the U-Boot 2021.10 merge window. Adding these as "imply" in the Kconfig avoids having to add them to all the board defconfigs but still allows disabling them if wanted. Also select DM_USB_GADGET if DM_USB is selected because otherwise the Ux500 MUSB glue driver does not show up in the configuration. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- arch/arm/Kconfig | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b730d3ebd8..18fcc7a124 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1047,13 +1047,20 @@ config ARCH_U8500 select DM_GPIO select DM_MMC if MMC select DM_SERIAL + select DM_USB_GADGET if DM_USB select OF_CONTROL select SYSRESET select TIMER + imply AB8500_USB_PHY imply ARM_PL180_MMCI + imply CLK + imply DM_PMIC imply DM_RTC + imply NOMADIK_GPIO imply NOMADIK_MTU_TIMER + imply PHY imply PL01X_SERIAL + imply PMIC_AB8500 imply RTC_PL031 imply SYSRESET_SYSCON From 1ae43a0e23596e9107228d3c9e6800318f25b84d Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:20 +0200 Subject: [PATCH 12/19] arm: dts: u8500: Update from Linux ux500-dts-for-v5.15 Update ste-dbx5x0.dtsi, ste-ab8500.dtsi and ste-ab8505.dtsi with the changes made in upstream Linux. They are taken from https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/ branch "ux500-dts-for-v5.15" (pending merge for Linux 5.15). The only relevant change for U-Boot here is "ARM: dts: ux500: ab8500: Link USB PHY to USB controller node" [1] which links the USB PHY to the USB controller. This is necessary on U-Boot because the PHY driver is implemented as part of the generic PHY subsystem that makes use of these bindings. [1]: https://lore.kernel.org/linux-arm-kernel/20210709182234.47232-1-stephan@gerhold.net/ Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- arch/arm/dts/ste-ab8500.dtsi | 116 +++++++++++++++++++++++++++-------- arch/arm/dts/ste-ab8505.dtsi | 95 ++++++++++++++++++++++------ arch/arm/dts/ste-dbx5x0.dtsi | 14 ++--- 3 files changed, 175 insertions(+), 50 deletions(-) diff --git a/arch/arm/dts/ste-ab8500.dtsi b/arch/arm/dts/ste-ab8500.dtsi index 14d4d8617d..dcc4a60c0c 100644 --- a/arch/arm/dts/ste-ab8500.dtsi +++ b/arch/arm/dts/ste-ab8500.dtsi @@ -42,15 +42,15 @@ ab8500-rtc { compatible = "stericsson,ab8500-rtc"; - interrupts = <17 IRQ_TYPE_LEVEL_HIGH - 18 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>, + <18 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "60S", "ALARM"; }; gpadc: ab8500-gpadc { compatible = "stericsson,ab8500-gpadc"; - interrupts = <32 IRQ_TYPE_LEVEL_HIGH - 39 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <32 IRQ_TYPE_LEVEL_HIGH>, + <39 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "HW_CONV_END", "SW_CONV_END"; vddadc-supply = <&ab8500_ldo_tvout_reg>; #address-cells = <1>; @@ -122,9 +122,11 @@ ab8500_temp { compatible = "stericsson,abx500-temp"; + interrupts = <3 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "ABX500_TEMP_WARM"; io-channels = <&gpadc 0x06>, <&gpadc 0x07>; - io-channel-name = "aux1", "aux2"; + io-channel-names = "aux1", "aux2"; }; ab8500_battery: ab8500_battery { @@ -134,29 +136,77 @@ ab8500_fg { compatible = "stericsson,ab8500-fg"; - battery = <&ab8500_battery>; + interrupts = <24 IRQ_TYPE_LEVEL_HIGH>, + <8 IRQ_TYPE_LEVEL_HIGH>, + <28 IRQ_TYPE_LEVEL_HIGH>, + <27 IRQ_TYPE_LEVEL_HIGH>, + <26 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "NCONV_ACCU", + "BATT_OVV", + "LOW_BAT_F", + "CC_INT_CALIB", + "CCEOC"; + battery = <&ab8500_battery>; io-channels = <&gpadc 0x08>; - io-channel-name = "main_bat_v"; + io-channel-names = "main_bat_v"; }; ab8500_btemp { compatible = "stericsson,ab8500-btemp"; - battery = <&ab8500_battery>; + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>, + <80 IRQ_TYPE_LEVEL_HIGH>, + <83 IRQ_TYPE_LEVEL_HIGH>, + <81 IRQ_TYPE_LEVEL_HIGH>, + <82 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "BAT_CTRL_INDB", + "BTEMP_LOW", + "BTEMP_HIGH", + "BTEMP_LOW_MEDIUM", + "BTEMP_MEDIUM_HIGH"; + battery = <&ab8500_battery>; io-channels = <&gpadc 0x02>, <&gpadc 0x01>; - io-channel-name = "btemp_ball", + io-channel-names = "btemp_ball", "bat_ctrl"; }; ab8500_charger { - compatible = "stericsson,ab8500-charger"; + compatible = "stericsson,ab8500-charger"; + interrupts = <10 IRQ_TYPE_LEVEL_HIGH>, + <11 IRQ_TYPE_LEVEL_HIGH>, + <0 IRQ_TYPE_LEVEL_HIGH>, + <107 IRQ_TYPE_LEVEL_HIGH>, + <106 IRQ_TYPE_LEVEL_HIGH>, + <14 IRQ_TYPE_LEVEL_HIGH>, + <15 IRQ_TYPE_LEVEL_HIGH>, + <79 IRQ_TYPE_LEVEL_HIGH>, + <105 IRQ_TYPE_LEVEL_HIGH>, + <104 IRQ_TYPE_LEVEL_HIGH>, + <89 IRQ_TYPE_LEVEL_HIGH>, + <22 IRQ_TYPE_LEVEL_HIGH>, + <21 IRQ_TYPE_LEVEL_HIGH>, + <16 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "MAIN_CH_UNPLUG_DET", + "MAIN_CHARGE_PLUG_DET", + "MAIN_EXT_CH_NOT_OK", + "MAIN_CH_TH_PROT_R", + "MAIN_CH_TH_PROT_F", + "VBUS_DET_F", + "VBUS_DET_R", + "USB_LINK_STATUS", + "USB_CH_TH_PROT_R", + "USB_CH_TH_PROT_F", + "USB_CHARGER_NOT_OKR", + "VBUS_OVV", + "CH_WD_EXP", + "VBUS_CH_DROP_END"; battery = <&ab8500_battery>; vddadc-supply = <&ab8500_ldo_tvout_reg>; io-channels = <&gpadc 0x03>, <&gpadc 0x0a>, <&gpadc 0x09>, <&gpadc 0x0b>; - io-channel-name = "main_charger_v", + io-channel-names = "main_charger_v", "main_charger_c", "vbus_v", "usb_charger_c"; @@ -167,15 +217,15 @@ battery = <&ab8500_battery>; }; - ab8500_usb { + ab8500_usb: ab8500_usb { compatible = "stericsson,ab8500-usb"; - interrupts = < 90 IRQ_TYPE_LEVEL_HIGH - 96 IRQ_TYPE_LEVEL_HIGH - 14 IRQ_TYPE_LEVEL_HIGH - 15 IRQ_TYPE_LEVEL_HIGH - 79 IRQ_TYPE_LEVEL_HIGH - 74 IRQ_TYPE_LEVEL_HIGH - 75 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <90 IRQ_TYPE_LEVEL_HIGH>, + <96 IRQ_TYPE_LEVEL_HIGH>, + <14 IRQ_TYPE_LEVEL_HIGH>, + <15 IRQ_TYPE_LEVEL_HIGH>, + <79 IRQ_TYPE_LEVEL_HIGH>, + <74 IRQ_TYPE_LEVEL_HIGH>, + <75 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "ID_WAKEUP_R", "ID_WAKEUP_F", "VBUS_DET_F", @@ -188,12 +238,13 @@ musb_1v8-supply = <&db8500_vsmps2_reg>; clocks = <&prcmu_clk PRCMU_SYSCLK>; clock-names = "sysclk"; + #phy-cells = <0>; }; ab8500-ponkey { compatible = "stericsson,ab8500-poweron-key"; - interrupts = <6 IRQ_TYPE_LEVEL_HIGH - 7 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <6 IRQ_TYPE_LEVEL_HIGH>, + <7 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "ONKEY_DBF", "ONKEY_DBR"; }; @@ -201,7 +252,19 @@ compatible = "stericsson,ab8500-sysctrl"; }; - ab8500-pwm { + ab8500-pwm-1 { + compatible = "stericsson,ab8500-pwm"; + clocks = <&ab8500_clock AB8500_SYSCLK_INT>; + clock-names = "intclk"; + }; + + ab8500-pwm-2 { + compatible = "stericsson,ab8500-pwm"; + clocks = <&ab8500_clock AB8500_SYSCLK_INT>; + clock-names = "intclk"; + }; + + ab8500-pwm-3 { compatible = "stericsson,ab8500-pwm"; clocks = <&ab8500_clock AB8500_SYSCLK_INT>; clock-names = "intclk"; @@ -255,8 +318,8 @@ // supplies to the display/camera ab8500_ldo_aux1_reg: ab8500_ldo_aux1 { - regulator-min-microvolt = <2500000>; - regulator-max-microvolt = <2900000>; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <3300000>; regulator-boot-on; /* BUG: If turned off MMC will be affected. */ regulator-always-on; @@ -324,5 +387,10 @@ vana-supply = <&ab8500_ldo_ana_reg>; }; }; + + usb_per5@a03e0000 { + phys = <&ab8500_usb>; + phy-names = "usb"; + }; }; }; diff --git a/arch/arm/dts/ste-ab8505.dtsi b/arch/arm/dts/ste-ab8505.dtsi index c72aa250bf..a1197fd37e 100644 --- a/arch/arm/dts/ste-ab8505.dtsi +++ b/arch/arm/dts/ste-ab8505.dtsi @@ -13,7 +13,8 @@ <&gpadc 0x08>, /* Main battery voltage */ <&gpadc 0x09>, /* VBUS */ <&gpadc 0x0b>, /* Charger current */ - <&gpadc 0x0c>; /* Backup battery voltage */ + <&gpadc 0x0c>, /* Backup battery voltage */ + <&gpadc 0x0d>; /* Die temperature */ }; soc { @@ -38,16 +39,15 @@ ab8500-rtc { compatible = "stericsson,ab8500-rtc"; - interrupts = <17 IRQ_TYPE_LEVEL_HIGH - 18 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>, + <18 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "60S", "ALARM"; }; gpadc: ab8500-gpadc { compatible = "stericsson,ab8500-gpadc"; - interrupts = <32 IRQ_TYPE_LEVEL_HIGH - 39 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "HW_CONV_END", "SW_CONV_END"; + interrupts = <39 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "SW_CONV_END"; vddadc-supply = <&ab8500_ldo_adc_reg>; #address-cells = <1>; #size-cells = <0>; @@ -84,42 +84,93 @@ bk_bat_v: channel@0c { reg = <0x0c>; }; + die_temp: channel@0d { + reg = <0x0d>; + }; usb_id: channel@0e { reg = <0x0e>; }; }; ab8500_battery: ab8500_battery { - status = "disabled"; + stericsson,battery-type = "LIPO"; thermistor-on-batctrl; }; ab8500_fg { status = "disabled"; compatible = "stericsson,ab8500-fg"; + interrupts = <24 IRQ_TYPE_LEVEL_HIGH>, + <8 IRQ_TYPE_LEVEL_HIGH>, + <28 IRQ_TYPE_LEVEL_HIGH>, + <27 IRQ_TYPE_LEVEL_HIGH>, + <26 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "NCONV_ACCU", + "BATT_OVV", + "LOW_BAT_F", + "CC_INT_CALIB", + "CCEOC"; battery = <&ab8500_battery>; io-channels = <&gpadc 0x08>; - io-channel-name = "main_bat_v"; + io-channel-names = "main_bat_v"; }; ab8500_btemp { status = "disabled"; compatible = "stericsson,ab8500-btemp"; + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>, + <80 IRQ_TYPE_LEVEL_HIGH>, + <83 IRQ_TYPE_LEVEL_HIGH>, + <81 IRQ_TYPE_LEVEL_HIGH>, + <82 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "BAT_CTRL_INDB", + "BTEMP_LOW", + "BTEMP_HIGH", + "BTEMP_LOW_MEDIUM", + "BTEMP_MEDIUM_HIGH"; battery = <&ab8500_battery>; io-channels = <&gpadc 0x02>, <&gpadc 0x01>; - io-channel-name = "btemp_ball", + io-channel-names = "btemp_ball", "bat_ctrl"; }; ab8500_charger { status = "disabled"; compatible = "stericsson,ab8500-charger"; + interrupts = <10 IRQ_TYPE_LEVEL_HIGH>, + <11 IRQ_TYPE_LEVEL_HIGH>, + <0 IRQ_TYPE_LEVEL_HIGH>, + <107 IRQ_TYPE_LEVEL_HIGH>, + <106 IRQ_TYPE_LEVEL_HIGH>, + <14 IRQ_TYPE_LEVEL_HIGH>, + <15 IRQ_TYPE_LEVEL_HIGH>, + <79 IRQ_TYPE_LEVEL_HIGH>, + <105 IRQ_TYPE_LEVEL_HIGH>, + <104 IRQ_TYPE_LEVEL_HIGH>, + <89 IRQ_TYPE_LEVEL_HIGH>, + <22 IRQ_TYPE_LEVEL_HIGH>, + <21 IRQ_TYPE_LEVEL_HIGH>, + <16 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "MAIN_CH_UNPLUG_DET", + "MAIN_CHARGE_PLUG_DET", + "MAIN_EXT_CH_NOT_OK", + "MAIN_CH_TH_PROT_R", + "MAIN_CH_TH_PROT_F", + "VBUS_DET_F", + "VBUS_DET_R", + "USB_LINK_STATUS", + "USB_CH_TH_PROT_R", + "USB_CH_TH_PROT_F", + "USB_CHARGER_NOT_OKR", + "VBUS_OVV", + "CH_WD_EXP", + "VBUS_CH_DROP_END"; battery = <&ab8500_battery>; vddadc-supply = <&ab8500_ldo_adc_reg>; io-channels = <&gpadc 0x09>, <&gpadc 0x0b>; - io-channel-name = "vbus_v", + io-channel-names = "vbus_v", "usb_charger_c"; }; @@ -131,13 +182,13 @@ ab8500_usb: ab8500_usb { compatible = "stericsson,ab8500-usb"; - interrupts = < 90 IRQ_TYPE_LEVEL_HIGH - 96 IRQ_TYPE_LEVEL_HIGH - 14 IRQ_TYPE_LEVEL_HIGH - 15 IRQ_TYPE_LEVEL_HIGH - 79 IRQ_TYPE_LEVEL_HIGH - 74 IRQ_TYPE_LEVEL_HIGH - 75 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <90 IRQ_TYPE_LEVEL_HIGH>, + <96 IRQ_TYPE_LEVEL_HIGH>, + <14 IRQ_TYPE_LEVEL_HIGH>, + <15 IRQ_TYPE_LEVEL_HIGH>, + <79 IRQ_TYPE_LEVEL_HIGH>, + <74 IRQ_TYPE_LEVEL_HIGH>, + <75 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "ID_WAKEUP_R", "ID_WAKEUP_F", "VBUS_DET_F", @@ -150,12 +201,13 @@ musb_1v8-supply = <&db8500_vsmps2_reg>; clocks = <&prcmu_clk PRCMU_SYSCLK>; clock-names = "sysclk"; + #phy-cells = <0>; }; ab8500-ponkey { compatible = "stericsson,ab8500-poweron-key"; - interrupts = <6 IRQ_TYPE_LEVEL_HIGH - 7 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <6 IRQ_TYPE_LEVEL_HIGH>, + <7 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "ONKEY_DBF", "ONKEY_DBR"; }; @@ -271,5 +323,10 @@ vana-supply = <&ab8500_ldo_ana_reg>; }; }; + + usb_per5@a03e0000 { + phys = <&ab8500_usb>; + phy-names = "usb"; + }; }; }; diff --git a/arch/arm/dts/ste-dbx5x0.dtsi b/arch/arm/dts/ste-dbx5x0.dtsi index 6671f74c9f..68607e4ad8 100644 --- a/arch/arm/dts/ste-dbx5x0.dtsi +++ b/arch/arm/dts/ste-dbx5x0.dtsi @@ -260,7 +260,7 @@ reg = <0x80150000 0x2000>; }; - L2: l2-cache { + L2: cache-controller { compatible = "arm,pl310-cache"; reg = <0xa0412000 0x1000>; interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>; @@ -883,7 +883,7 @@ status = "disabled"; }; - sdi0_per1@80126000 { + mmc@80126000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x80126000 0x1000>; interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>; @@ -899,7 +899,7 @@ status = "disabled"; }; - sdi1_per2@80118000 { + mmc@80118000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x80118000 0x1000>; interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>; @@ -915,7 +915,7 @@ status = "disabled"; }; - sdi2_per3@80005000 { + mmc@80005000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x80005000 0x1000>; interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>; @@ -931,7 +931,7 @@ status = "disabled"; }; - sdi3_per2@80119000 { + mmc@80119000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x80119000 0x1000>; interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>; @@ -947,7 +947,7 @@ status = "disabled"; }; - sdi4_per2@80114000 { + mmc@80114000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x80114000 0x1000>; interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; @@ -963,7 +963,7 @@ status = "disabled"; }; - sdi5_per3@80008000 { + mmc@80008000 { compatible = "arm,pl18x", "arm,primecell"; reg = <0x80008000 0x1000>; interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>; From fc9d4b123d50f95185baf37bea58b8ec499f3e66 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:21 +0200 Subject: [PATCH 13/19] arm: dts: u8500: u-boot: Add fixed clock for eMMC So far there is no need for a clock driver in U-Boot because the previous boot stage leaves all the necessary clocks on. However, some drivers in U-Boot (e.g. arm_pl180_mmci) depend on having a clock driver to obtain the clock frequency. Setting up the clock drivers properly is a bit tricky on U8500, so for now add a simple fixed-clock for the eMMC that allows obtaining the clock frequency. This should be replaced eventually if some board actually requires enabling some of the clocks. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- arch/arm/dts/ste-dbx5x0-u-boot.dtsi | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/arm/dts/ste-dbx5x0-u-boot.dtsi b/arch/arm/dts/ste-dbx5x0-u-boot.dtsi index 4a99ee5a92..e350175305 100644 --- a/arch/arm/dts/ste-dbx5x0-u-boot.dtsi +++ b/arch/arm/dts/ste-dbx5x0-u-boot.dtsi @@ -4,8 +4,14 @@ #include "ste-dbx5x0.dtsi" / { + /* FIXME: Remove this when clk driver is implemented */ + sdmmcclk: sdmmcclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <100000000>; + }; + soc { - /* FIXME: Remove this when clk driver is implemented */ mtu@a03c6000 { clock-frequency = <133000000>; }; @@ -18,6 +24,9 @@ uart@80007000 { clock = <38400000>; }; + mmc@80005000 { + clocks = <&sdmmcclk>; + }; }; reboot { From f64011e11e283b34b6792165a36c42b8241f2a24 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:22 +0200 Subject: [PATCH 14/19] board: stemmy: Add basic Fastboot support Make use of the new drivers for ARM U8500 introduced in the U-Boot 2021.10 merge window by adding basic support for USB Fastboot with the "stemmy" board. As a first step this will always boot directly into USB Fastboot for now with the console displayed on the screen to make that obvious. Samsung uses quite strange GPT partition labels on these boards, so also add a bunch of fastboot_partition_alias_* to make this more easy to use. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- arch/arm/dts/ste-ux500-samsung-stemmy.dts | 16 ++++++++++++++ configs/stemmy_defconfig | 14 ++++++++++++ include/configs/stemmy.h | 27 +++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/arch/arm/dts/ste-ux500-samsung-stemmy.dts b/arch/arm/dts/ste-ux500-samsung-stemmy.dts index 7e7f4c823a..14be86086b 100644 --- a/arch/arm/dts/ste-ux500-samsung-stemmy.dts +++ b/arch/arm/dts/ste-ux500-samsung-stemmy.dts @@ -12,9 +12,25 @@ }; soc { + /* eMMC */ + mmc@80005000 { + status = "okay"; + + arm,primecell-periphid = <0x10480180>; + max-frequency = <100000000>; + bus-width = <8>; + + non-removable; + cap-mmc-highspeed; + }; + /* Debugging console UART */ uart@80007000 { status = "okay"; }; + + mcde@a0350000 { + status = "okay"; + }; }; }; diff --git a/configs/stemmy_defconfig b/configs/stemmy_defconfig index c026081906..77bc5c71dd 100644 --- a/configs/stemmy_defconfig +++ b/configs/stemmy_defconfig @@ -6,6 +6,8 @@ CONFIG_NR_DRAM_BANKS=2 CONFIG_SYS_MALLOC_LEN=0x0200000 CONFIG_DEFAULT_DEVICE_TREE="ste-ux500-samsung-stemmy" CONFIG_SYS_LOAD_ADDR=0x100000 +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="run fastbootcmd" CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_HUSH_PARSER=y CONFIG_CMD_CONFIG=y @@ -17,5 +19,17 @@ CONFIG_CMD_PART=y CONFIG_CMD_GETTIME=y CONFIG_EFI_PARTITION=y # CONFIG_NET is not set +CONFIG_USB_FUNCTION_FASTBOOT=y +CONFIG_FASTBOOT_BUF_ADDR=0x18100000 +CONFIG_FASTBOOT_FLASH=y +CONFIG_FASTBOOT_FLASH_MMC_DEV=0 # CONFIG_MMC_HW_PARTITIONING is not set +CONFIG_USB=y +CONFIG_USB_MUSB_GADGET=y +CONFIG_USB_GADGET=y +CONFIG_USB_GADGET_VENDOR_NUM=0x04e8 +CONFIG_USB_GADGET_PRODUCT_NUM=0x685d +CONFIG_DM_VIDEO=y +CONFIG_SYS_WHITE_ON_BLACK=y +CONFIG_VIDEO_MCDE_SIMPLE=y # CONFIG_EFI_LOADER is not set diff --git a/include/configs/stemmy.h b/include/configs/stemmy.h index b3a17c5b4f..b250a53e60 100644 --- a/include/configs/stemmy.h +++ b/include/configs/stemmy.h @@ -22,4 +22,31 @@ /* Generate initrd atag for downstream kernel (others are copied in stemmy.c) */ #define CONFIG_INITRD_TAG +/* Linux does not boot if FDT / initrd is loaded to end of RAM */ +#define BOOT_ENV \ + "fdt_high=0x6000000\0" \ + "initrd_high=0x6000000\0" + +#define CONSOLE_ENV \ + "stdin=serial\0" \ + "stdout=serial,vidconsole\0" \ + "stderr=serial,vidconsole\0" + +#define FASTBOOT_ENV \ + "fastboot_partition_alias_boot=Kernel\0" \ + "fastboot_partition_alias_recovery=Kernel2\0" \ + "fastboot_partition_alias_system=SYSTEM\0" \ + "fastboot_partition_alias_cache=CACHEFS\0" \ + "fastboot_partition_alias_hidden=HIDDEN\0" \ + "fastboot_partition_alias_userdata=DATAFS\0" + +#define BOOTCMD_ENV \ + "fastbootcmd=echo '*** FASTBOOT MODE ***'; fastboot usb 0\0" + +#define CONFIG_EXTRA_ENV_SETTINGS \ + BOOT_ENV \ + CONSOLE_ENV \ + FASTBOOT_ENV \ + BOOTCMD_ENV + #endif From f629895fa34b7ba12572c9185209e2413a758208 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:23 +0200 Subject: [PATCH 15/19] board: stemmy: Update documentation Over the time, the "stemmy" U-Boot board was tested on several other Samsung smartphones based on ST-Ericsson NovaThor Ux500. Convert the documentation to reStructuredText at doc/board/ste/stemmy.rst and make the device list complete. Also note that the board now boots into USB Fastboot instead of just ending up at the U-Boot prompt. The device table is mostly taken from the postmarketOS wiki article (https://wiki.postmarketos.org/wiki/ST-Ericsson_NovaThor_U8500). All the newly added devices were tested by Linus Walleij. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- arch/arm/mach-u8500/Kconfig | 13 +++--- board/ste/stemmy/MAINTAINERS | 3 +- board/ste/stemmy/README | 50 ---------------------- doc/board/index.rst | 1 + doc/board/ste/index.rst | 9 ++++ doc/board/ste/stemmy.rst | 81 ++++++++++++++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 57 deletions(-) delete mode 100644 board/ste/stemmy/README create mode 100644 doc/board/ste/index.rst create mode 100644 doc/board/ste/stemmy.rst diff --git a/arch/arm/mach-u8500/Kconfig b/arch/arm/mach-u8500/Kconfig index db7a29a54c..b067a719e7 100644 --- a/arch/arm/mach-u8500/Kconfig +++ b/arch/arm/mach-u8500/Kconfig @@ -13,14 +13,15 @@ config TARGET_STEMMY The Samsung "stemmy" board supports Samsung smartphones released with the ST-Ericsson NovaThor U8500 SoC, e.g. - - Samsung Galaxy S III mini (GT-I8190) "golden" - - Samsung Galaxy S Advance (GT-I9070) "janice" - - Samsung Galaxy Xcover 2 (GT-S7710) "skomer" - Samsung Galaxy Ace 2 (GT-I8160) "codina" + - Samsung Galaxy Amp (SGH-I407) "kyle" + - Samsung Galaxy Beam (GT-I8530) "gavini" + - Samsung Galaxy Exhibit (SGH-T599) "codina" (TMO) + - Samsung Galaxy S Advance (GT-I9070) "janice" + - Samsung Galaxy S III mini (GT-I8190) "golden" + - Samsung Galaxy Xcover 2 (GT-S7710) "skomer" - and likely others as well (untested). - - See board/ste/stemmy/README for details. + See doc/board/ste/stemmy.rst for details. endchoice diff --git a/board/ste/stemmy/MAINTAINERS b/board/ste/stemmy/MAINTAINERS index 37daabea9c..fa06488284 100644 --- a/board/ste/stemmy/MAINTAINERS +++ b/board/ste/stemmy/MAINTAINERS @@ -2,5 +2,6 @@ STEMMY BOARD M: Stephan Gerhold <stephan@gerhold.net> S: Maintained F: board/ste/stemmy/ -F: include/configs/stemmy.h F: configs/stemmy_defconfig +F: doc/board/ste/stemmy.rst +F: include/configs/stemmy.h diff --git a/board/ste/stemmy/README b/board/ste/stemmy/README deleted file mode 100644 index 1b83b833c0..0000000000 --- a/board/ste/stemmy/README +++ /dev/null @@ -1,50 +0,0 @@ -ST-Ericsson U8500 Samsung "stemmy" board -======================================== - -The "stemmy" board supports Samsung smartphones released with -the ST-Ericsson NovaThor U8500 SoC, e.g. - - - Samsung Galaxy S III mini (GT-I8190) "golden" - - Samsung Galaxy S Advance (GT-I9070) "janice" - - Samsung Galaxy Xcover 2 (GT-S7710) "skomer" - - Samsung Galaxy Ace 2 (GT-I8160) "codina" - -and likely others as well (untested). - -At the moment, U-Boot is intended to be chain-loaded from -the original Samsung bootloader, not replacing it entirely. - -Installation ------------- - -1. Setup cross compiler, e.g. export CROSS_COMPILE=arm-none-eabi- -2. make stemmy_defconfig -3. make - -For newer devices (golden and skomer), the U-Boot binary has to be packed into -an Android boot image. janice boots the raw U-Boot binary from the boot partition. - -4. Obtain mkbootimg, e.g. https://android.googlesource.com/platform/system/core/+/refs/tags/android-7.1.2_r37/mkbootimg/mkbootimg -5. mkbootimg \ - --kernel=u-boot.bin \ - --base=0x00000000 \ - --kernel_offset=0x00100000 \ - --ramdisk_offset=0x02000000 \ - --tags_offset=0x00000100 \ - --output=u-boot.img - -6. Enter Samsung download mode (press Power + Home + Volume Down) -7. Flash U-Boot image to Android boot partition using Heimdall: - https://gitlab.com/BenjaminDobell/Heimdall - - heimdall flash --Kernel u-boot.(bin|img) - -8. After reboot U-Boot prompt should appear via UART. - -UART ----- - -UART is available through the micro USB port, similar to the Carkit standard. -With a ~619kOhm resistor between ID and GND, 1.8V RX/TX is available at D+/D-. - -Make sure to connect the UART cable *before* turning on the phone. diff --git a/doc/board/index.rst b/doc/board/index.rst index 33087074fa..8588e453d5 100644 --- a/doc/board/index.rst +++ b/doc/board/index.rst @@ -26,6 +26,7 @@ Board-specific doc sipeed/index socionext/index st/index + ste/index tbs/index ti/index toradex/index diff --git a/doc/board/ste/index.rst b/doc/board/ste/index.rst new file mode 100644 index 0000000000..bef520ce63 --- /dev/null +++ b/doc/board/ste/index.rst @@ -0,0 +1,9 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +ST-Ericsson +=========== + +.. toctree:: + :maxdepth: 2 + + stemmy diff --git a/doc/board/ste/stemmy.rst b/doc/board/ste/stemmy.rst new file mode 100644 index 0000000000..6d77fe9c83 --- /dev/null +++ b/doc/board/ste/stemmy.rst @@ -0,0 +1,81 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. sectionauthor:: Stephan Gerhold <stephan@gerhold.net> + +ST-Ericsson U8500 Samsung "stemmy" board +======================================== + +The "stemmy" board supports Samsung smartphones released with +the ST-Ericsson NovaThor U8500 SoC, e.g. + + +---------------------------+----------+--------------+----------------+ + | Device | Model | Codename | U-Boot | + +===========================+==========+==============+================+ + | Samsung Galaxy Ace 2 | GT-I8160 | codina | ``u-boot.bin`` | + +---------------------------+----------+--------------+----------------+ + | Samsung Galaxy Amp | SGH-I407 | kyle | ``u-boot.img`` | + +---------------------------+----------+--------------+----------------+ + | Samsung Galaxy Beam | GT-I8530 | gavini | ``u-boot.bin`` | + +---------------------------+----------+--------------+----------------+ + | Samsung Galaxy Exhibit | SGH-T599 | codina (TMO) | ``u-boot.bin`` | + +---------------------------+----------+--------------+----------------+ + | Samsung Galaxy S Advance | GT-I9070 | janice | ``u-boot.bin`` | + +---------------------------+----------+--------------+----------------+ + | Samsung Galaxy S III mini | GT-I8190 | golden | ``u-boot.img`` | + +---------------------------+----------+--------------+----------------+ + | Samsung Galaxy Xcover 2 | GT-S7710 | skomer | ``u-boot.img`` | + +---------------------------+----------+--------------+----------------+ + +At the moment, U-Boot is intended to be chain-loaded from +the original Samsung bootloader, not replacing it entirely. + +Installation +------------ +First, setup ``CROSS_COMPILE`` for ARMv7. Then, build U-Boot for ``stemmy``:: + + $ export CROSS_COMPILE=arm-none-eabi- + $ make stemmy_defconfig + $ make + +This will build ``u-boot.bin`` in the configured output directory. + +For newer devices (check ``u-boot.img`` in the table above), the U-Boot binary +has to be packed into an Android boot image. Devices with ``u-boot.bin`` boot +the raw U-Boot binary from the boot partition. You can build the Android boot +image with ``mkbootimg``, e.g. from from android-7.1.2_r37_:: + + $ mkbootimg \ + --kernel=u-boot.bin \ + --base=0x00000000 \ + --kernel_offset=0x00100000 \ + --ramdisk_offset=0x02000000 \ + --tags_offset=0x00000100 \ + --output=u-boot.img + +.. _android-7.1.2_r37: https://android.googlesource.com/platform/system/core/+/refs/tags/android-7.1.2_r37/mkbootimg/mkbootimg + +To flash the U-Boot binary, enter the Samsung download mode +(press Power + Home + Volume Down). Use Heimdall_ to flash the U-Boot image to +the Android boot partition:: + + $ heimdall flash --Kernel u-boot.(bin|img) + +If this is not working but there are messages like ``Android recovery image`` in +the UART console, you can try flashing to the recovery partition instead:: + + $ heimdall flash --Kernel2 u-boot.(bin|img) + +.. _Heimdall: https://gitlab.com/BenjaminDobell/Heimdall + +After a reboot the U-Boot prompt should appear via UART. Unless interrupted it +automatically boots to USB Fastboot mode where Android boot images can be booted +via ``fastboot boot boot.img``. It is mainly intended to boot mainline Linux, +but booting original Samsung Android boot images is also supported (e.g. for +charging). + +UART +---- +UART is available through the micro USB port, similar to the Carkit standard. +With a ~619kOhm resistor between ID and GND, 1.8V RX/TX is available at D+/D-. + +.. note:: + Make sure to connect the UART cable **before** turning on the phone. From 8956854d4839341b61b70aae1f6df53d05084fb2 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold <stephan@gerhold.net> Date: Sat, 7 Aug 2021 15:07:24 +0200 Subject: [PATCH 16/19] arm: u8500: Prefer building in thumb mode by default Enabling CONFIG_SYS_THUMB_BUILD produces a significantly smaller U-Boot binary (250 KiB vs 320 KiB) that still seems to be fully functional. Make use of that by default but keep it as "imply" so it can be disabled for testing in case this causes trouble for someone. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 18fcc7a124..c815ad461a 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1062,6 +1062,7 @@ config ARCH_U8500 imply PL01X_SERIAL imply PMIC_AB8500 imply RTC_PL031 + imply SYS_THUMB_BUILD imply SYSRESET_SYSCON config ARCH_VERSAL From 223c44904d86a14d6c17e73696186c402e9ba958 Mon Sep 17 00:00:00 2001 From: Callum Sinclair <callum.sinclair@alliedtelesis.co.nz> Date: Tue, 10 Aug 2021 14:51:15 +1200 Subject: [PATCH 17/19] rtc: ds1307: Fix incorrect clock reset for DS13xx The ds1307 driver also supports the DS1339 and DS1340. However, in ds1307_rtc_reset the register writes assume that the chip is a DS1307. This is evident in the writing of bits SQWE, RS1, RS0 to the control register. While this applies correctly to the DS1307, on a DS1340 the control register doesn't contain those bits (instead, the register is used for clock calibration). By writing these bits the clock calibration will be changed and the chip can become non-functional after a reset call. Signed-off-by: Callum Sinclair <callum.sinclair@alliedtelesis.co.nz> --- drivers/rtc/ds1307.c | 69 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/drivers/rtc/ds1307.c b/drivers/rtc/ds1307.c index 2015ce9bbc..3be97c9d93 100644 --- a/drivers/rtc/ds1307.c +++ b/drivers/rtc/ds1307.c @@ -43,11 +43,21 @@ enum ds_type { #define RTC_SEC_BIT_CH 0x80 /* Clock Halt (in Register 0) */ +/* DS1307-specific bits */ #define RTC_CTL_BIT_RS0 0x01 /* Rate select 0 */ #define RTC_CTL_BIT_RS1 0x02 /* Rate select 1 */ #define RTC_CTL_BIT_SQWE 0x10 /* Square Wave Enable */ #define RTC_CTL_BIT_OUT 0x80 /* Output Control */ +/* DS1337-specific bits */ +#define DS1337_CTL_BIT_RS1 0x08 /* Rate select 1 */ +#define DS1337_CTL_BIT_RS2 0x10 /* Rate select 2 */ +#define DS1337_CTL_BIT_EOSC 0x80 /* Enable Oscillator */ + +/* DS1340-specific bits */ +#define DS1340_SEC_BIT_EOSC 0x80 /* Enable Oscillator */ +#define DS1340_CTL_BIT_OUT 0x80 /* Output Control */ + /* MCP7941X-specific bits */ #define MCP7941X_BIT_ST 0x80 #define MCP7941X_BIT_VBATEN 0x08 @@ -261,9 +271,25 @@ read_rtc: buf[RTC_SEC_REG_ADDR]); return -1; } - } - - if (type == m41t11) { + } else if (type == ds_1337) { + if (buf[RTC_CTL_REG_ADDR] & DS1337_CTL_BIT_EOSC) { + printf("### Warning: RTC oscillator has stopped\n"); + /* clear the not oscillator enable (~EOSC) flag */ + buf[RTC_CTL_REG_ADDR] &= ~DS1337_CTL_BIT_EOSC; + dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR, + buf[RTC_CTL_REG_ADDR]); + return -1; + } + } else if (type == ds_1340) { + if (buf[RTC_SEC_REG_ADDR] & DS1340_SEC_BIT_EOSC) { + printf("### Warning: RTC oscillator has stopped\n"); + /* clear the not oscillator enable (~EOSC) flag */ + buf[RTC_SEC_REG_ADDR] &= ~DS1340_SEC_BIT_EOSC; + dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR, + buf[RTC_SEC_REG_ADDR]); + return -1; + } + } else if (type == m41t11) { /* clock halted? turn it on, so clock can tick. */ if (buf[RTC_SEC_REG_ADDR] & RTC_SEC_BIT_CH) { buf[RTC_SEC_REG_ADDR] &= ~RTC_SEC_BIT_CH; @@ -273,9 +299,7 @@ read_rtc: buf[RTC_SEC_REG_ADDR]); goto read_rtc; } - } - - if (type == mcp794xx) { + } else if (type == mcp794xx) { /* make sure that the backup battery is enabled */ if (!(buf[RTC_DAY_REG_ADDR] & MCP7941X_BIT_VBATEN)) { dm_i2c_reg_write(dev, RTC_DAY_REG_ADDR, @@ -314,18 +338,37 @@ read_rtc: static int ds1307_rtc_reset(struct udevice *dev) { int ret; + enum ds_type type = dev_get_driver_data(dev); - /* clear Clock Halt */ + /* + * reset clock/oscillator in the seconds register: + * on DS1307 bit 7 enables Clock Halt (CH), + * on DS1340 bit 7 disables the oscillator (not EOSC) + * on MCP794xx bit 7 enables Start Oscillator (ST) + */ ret = dm_i2c_reg_write(dev, RTC_SEC_REG_ADDR, 0x00); if (ret < 0) return ret; - ret = dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR, - RTC_CTL_BIT_SQWE | RTC_CTL_BIT_RS1 | - RTC_CTL_BIT_RS0); - if (ret < 0) - return ret; - return 0; + if (type == ds_1307) { + /* Write control register in order to enable square-wave + * output (SQWE) and set a default rate of 32.768kHz (RS1|RS0). + */ + ret = dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR, + RTC_CTL_BIT_SQWE | RTC_CTL_BIT_RS1 | + RTC_CTL_BIT_RS0); + } else if (type == ds_1337) { + /* Write control register in order to enable oscillator output + * (not EOSC) and set a default rate of 32.768kHz (RS2|RS1). + */ + ret = dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR, + DS1337_CTL_BIT_RS2 | DS1337_CTL_BIT_RS1); + } else if (type == ds_1340 || type == mcp794xx || type == m41t11) { + /* Reset clock calibration, frequency test and output level. */ + ret = dm_i2c_reg_write(dev, RTC_CTL_REG_ADDR, 0x00); + } + + return ret; } static int ds1307_probe(struct udevice *dev) From 20aa320a4f546b90432cdaa88e7c3cc3d970b3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> Date: Sun, 15 Aug 2021 16:27:36 +0200 Subject: [PATCH 18/19] ata: ahci-pci: Fix dependency on DM_PCI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File drivers/ata/ahci-pci.c calls function ahci_probe_scsi_pci() which is compiled only when DM_PCI is enabled. So add missing dependency into Kconfig. Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Stefan Roese <sr@denx.de> --- drivers/ata/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index 6f0b772383..0c1490a9f9 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -36,6 +36,7 @@ menu "SATA/SCSI device support" config AHCI_PCI bool "Support for PCI-based AHCI controller" + depends on DM_PCI depends on DM_SCSI help Enables support for the PCI-based AHCI controller. From 73059529b2046638971aeaa3c75c857458a5ec82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> Date: Sun, 15 Aug 2021 16:27:37 +0200 Subject: [PATCH 19/19] ata: ahci-pci: Add new option CONFIG_SPL_AHCI_PCI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This new option allows to disable ahci-pci driver in SPL. Disabling it is needed when SPL_PCI is not enabled as ahci-pci depends on PCI. This change fixes following compile error when CONFIG_SPL_SATA_SUPPORT is enabled and SPL_PCI is disabled. LD spl/u-boot-spl arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.o: in function `ahci_probe_scsi_pci': drivers/ata/ahci.c:1205: undefined reference to `dm_pci_map_bar' arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.c:1215: undefined reference to `dm_pci_read_config16' arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.c:1216: undefined reference to `dm_pci_read_config16' arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.c:1220: undefined reference to `dm_pci_map_bar' make[1]: *** [scripts/Makefile.spl:512: spl/u-boot-spl] Error 1 make: *** [Makefile:1977: spl/u-boot-spl] Error 2 LD spl/u-boot-spl arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci-pci.o: in function `ahci_pci_probe': drivers/ata/ahci-pci.c:21: undefined reference to `ahci_probe_scsi_pci' make[1]: *** [scripts/Makefile.spl:512: spl/u-boot-spl] Error 1 make: *** [Makefile:1977: spl/u-boot-spl] Error 2 Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Stefan Roese <sr@denx.de> --- drivers/ata/Kconfig | 6 ++++++ drivers/ata/Makefile | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index 0c1490a9f9..96c7c30375 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -41,6 +41,12 @@ config AHCI_PCI help Enables support for the PCI-based AHCI controller. +config SPL_AHCI_PCI + bool "Support for PCI-based AHCI controller for SPL" + depends on SPL + depends on SPL_PCI + depends on SPL_SATA_SUPPORT && DM_SCSI + config SATA_CEVA bool "Ceva Sata controller" depends on AHCI diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index 4811b2f82c..cd88131dcd 100644 --- a/drivers/ata/Makefile +++ b/drivers/ata/Makefile @@ -5,7 +5,7 @@ obj-$(CONFIG_DWC_AHCI) += dwc_ahci.o obj-$(CONFIG_AHCI) += ahci-uclass.o -obj-$(CONFIG_AHCI_PCI) += ahci-pci.o +obj-$(CONFIG_$(SPL_)AHCI_PCI) += ahci-pci.o obj-$(CONFIG_SCSI_AHCI) += ahci.o obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o obj-$(CONFIG_FSL_SATA) += fsl_sata.o