| Message ID | 20240930030716.179992-1-zhengzengkai@huawei.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | ACPI: GTDT: simplify acpi_gtdt_init() implementation | expand |
Hi Zheng,
kernel test robot noticed the following build warnings:
[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge tip/timers/core linus/master v6.12-rc1 next-20240930]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zheng-Zengkai/ACPI-GTDT-simplify-acpi_gtdt_init-implementation/20240930-105041
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20240930030716.179992-1-zhengzengkai%40huawei.com
patch subject: [PATCH] ACPI: GTDT: simplify acpi_gtdt_init() implementation
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241001/202410010101.2oPkEaoP-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 7773243d9916f98ba0ffce0c3a960e4aa9f03e81)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241001/202410010101.2oPkEaoP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410010101.2oPkEaoP-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/acpi/arm64/gtdt.c:11:
In file included from include/linux/acpi.h:39:
In file included from include/acpi/acpi_io.h:7:
In file included from arch/arm64/include/asm/acpi.h:14:
In file included from include/linux/memblock.h:12:
In file included from include/linux/mm.h:2236:
include/linux/vmstat.h:503:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
503 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
504 | item];
| ~~~~
include/linux/vmstat.h:510:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
510 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
511 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
517 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:523:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
523 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
524 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/acpi/arm64/gtdt.c:383:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
383 | if (!timer_count)
| ^~~~~~~~~~~~
drivers/acpi/arm64/gtdt.c:400:9: note: uninitialized use occurs here
400 | return ret;
| ^~~
drivers/acpi/arm64/gtdt.c:383:2: note: remove the 'if' if its condition is always false
383 | if (!timer_count)
| ^~~~~~~~~~~~~~~~~
384 | goto out_put_gtdt;
| ~~~~~~~~~~~~~~~~~
drivers/acpi/arm64/gtdt.c:365:9: note: initialize the variable 'ret' to silence this warning
365 | int ret, timer_count = 0, gwdt_count = 0;
| ^
| = 0
5 warnings generated.
vim +383 drivers/acpi/arm64/gtdt.c
360
361 static int __init gtdt_sbsa_gwdt_init(void)
362 {
363 void *platform_timer;
364 struct acpi_table_header *table;
365 int ret, timer_count = 0, gwdt_count = 0;
366
367 if (acpi_disabled)
368 return 0;
369
370 if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_GTDT, 0, &table)))
371 return -EINVAL;
372
373 /*
374 * Note: Even though the global variable acpi_gtdt_desc has been
375 * initialized by acpi_gtdt_init() while initializing the arch timers,
376 * when we call this function to get SBSA watchdogs info from GTDT, the
377 * pointers stashed in it are stale (since they are early temporary
378 * mappings carried out before acpi_permanent_mmap is set) and we need
379 * to re-initialize them with permanent mapped pointer values to let the
380 * GTDT parsing possible.
381 */
382 acpi_gtdt_init(table, &timer_count);
> 383 if (!timer_count)
384 goto out_put_gtdt;
385
386 for_each_platform_timer(platform_timer) {
387 if (is_non_secure_watchdog(platform_timer)) {
388 ret = gtdt_import_sbsa_gwdt(platform_timer, gwdt_count);
389 if (ret)
390 break;
391 gwdt_count++;
392 }
393 }
394
395 if (gwdt_count)
396 pr_info("found %d SBSA generic Watchdog(s).\n", gwdt_count);
397
398 out_put_gtdt:
399 acpi_put_table(table);
400 return ret;
401 }
402
diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c index c0e77c1c8e09..b6d248ddb1b3 100644 --- a/drivers/acpi/arm64/gtdt.c +++ b/drivers/acpi/arm64/gtdt.c @@ -147,45 +147,30 @@ bool __init acpi_gtdt_c3stop(int type) * @table: The pointer to GTDT table. * @platform_timer_count: It points to a integer variable which is used * for storing the number of platform timers. - * This pointer could be NULL, if the caller - * doesn't need this info. - * - * Return: 0 if success, -EINVAL if error. */ -int __init acpi_gtdt_init(struct acpi_table_header *table, +void __init acpi_gtdt_init(struct acpi_table_header *table, int *platform_timer_count) { - void *platform_timer; struct acpi_table_gtdt *gtdt; gtdt = container_of(table, struct acpi_table_gtdt, header); acpi_gtdt_desc.gtdt = gtdt; acpi_gtdt_desc.gtdt_end = (void *)table + table->length; acpi_gtdt_desc.platform_timer = NULL; - if (platform_timer_count) - *platform_timer_count = 0; if (table->revision < 2) { pr_warn("Revision:%d doesn't support Platform Timers.\n", table->revision); - return 0; + return; } if (!gtdt->platform_timer_count) { pr_debug("No Platform Timer.\n"); - return 0; + return; } - platform_timer = (void *)gtdt + gtdt->platform_timer_offset; - if (platform_timer < (void *)table + sizeof(struct acpi_table_gtdt)) { - pr_err(FW_BUG "invalid timer data.\n"); - return -EINVAL; - } - acpi_gtdt_desc.platform_timer = platform_timer; - if (platform_timer_count) - *platform_timer_count = gtdt->platform_timer_count; - - return 0; + acpi_gtdt_desc.platform_timer = (void *)gtdt + gtdt->platform_timer_offset; + *platform_timer_count = gtdt->platform_timer_count; } static int __init gtdt_parse_timer_block(struct acpi_gtdt_timer_block *block, @@ -377,7 +362,7 @@ static int __init gtdt_sbsa_gwdt_init(void) { void *platform_timer; struct acpi_table_header *table; - int ret, timer_count, gwdt_count = 0; + int ret, timer_count = 0, gwdt_count = 0; if (acpi_disabled) return 0; @@ -394,8 +379,8 @@ static int __init gtdt_sbsa_gwdt_init(void) * to re-initialize them with permanent mapped pointer values to let the * GTDT parsing possible. */ - ret = acpi_gtdt_init(table, &timer_count); - if (ret || !timer_count) + acpi_gtdt_init(table, &timer_count); + if (!timer_count) goto out_put_gtdt; for_each_platform_timer(platform_timer) { diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 03733101e231..4ca06aba68a4 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -1741,7 +1741,7 @@ static int __init arch_timer_mem_acpi_init(int platform_timer_count) /* Initialize per-processor generic timer and memory-mapped timer(if present) */ static int __init arch_timer_acpi_init(struct acpi_table_header *table) { - int ret, platform_timer_count; + int ret, platform_timer_count = 0; if (arch_timers_present & ARCH_TIMER_TYPE_CP15) { pr_warn("already initialized, skipping\n"); @@ -1750,9 +1750,7 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table) arch_timers_present |= ARCH_TIMER_TYPE_CP15; - ret = acpi_gtdt_init(table, &platform_timer_count); - if (ret) - return ret; + acpi_gtdt_init(table, &platform_timer_count); arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI] = acpi_gtdt_map_ppi(ARCH_TIMER_PHYS_NONSECURE_PPI); diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 3a21f1cf126f..7ebc031ff8c0 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -750,7 +750,7 @@ int acpi_reconfig_notifier_register(struct notifier_block *nb); int acpi_reconfig_notifier_unregister(struct notifier_block *nb); #ifdef CONFIG_ACPI_GTDT -int acpi_gtdt_init(struct acpi_table_header *table, int *platform_timer_count); +void __init acpi_gtdt_init(struct acpi_table_header *table, int *platform_timer_count); int acpi_gtdt_map_ppi(int type); bool acpi_gtdt_c3stop(int type); int acpi_arch_timer_mem_init(struct arch_timer_mem *timer_mem, int *timer_count);
According to GTDT Table Structure of ACPI specification, the result of expression '(void *)gtdt + gtdt->platform_timer_offset' will be same with the expression '(void *)table + sizeof(struct acpi_table_gtdt)' in function acpi_gtdt_init(), so the condition of the "invalid timer data" check will never be true, remove the EINVAL error check branch and change to void return type for acpi_gtdt_init() to simplify the function implementation and error handling by callers. Besides, after commit c2743a36765d ("clocksource: arm_arch_timer: add GTDT support for memory-mapped timer"), acpi_gtdt_init() currently will not be called with parameter platform_timer_count set to NULL and we can explicitly initialize the integer variable which is used for storing the number of platform timers by caller to zero, so there is no need to do null pointer check for platform_timer_count in acpi_gtdt_init(), remove it to make code a bit more concise. Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com> --- drivers/acpi/arm64/gtdt.c | 31 +++++++--------------------- drivers/clocksource/arm_arch_timer.c | 6 ++---- include/linux/acpi.h | 2 +- 3 files changed, 11 insertions(+), 28 deletions(-)