Copilot commented on code in PR #18573: URL: https://github.com/apache/nuttx/pull/18573#discussion_r2997707313
########## arch/arm/src/ht32f491x3/ht32f491x3_timerisr.c: ########## @@ -0,0 +1,95 @@ +/**************************************************************************** + * arch/arm/src/ht32f491x3/ht32f491x3_timerisr.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdint.h> +#include <time.h> + +#include <nuttx/arch.h> +#include <nuttx/timers/arch_timer.h> + +#include "nvic.h" +#include "clock/clock.h" +#include "arm_internal.h" +#include "systick.h" + +#include "chip.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define HT32_SYSTICK_RELOAD ((HT32_HCLK_FREQUENCY / CLK_TCK) - 1) + +#if HT32_SYSTICK_RELOAD > 0x00ffffff +# error HT32_SYSTICK_RELOAD exceeds the SysTick reload field +#endif + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +#if !defined(CONFIG_ARMV7M_SYSTICK) && !defined(CONFIG_TIMER_ARCH) +static int ht32f491x3_timerisr(int irq, FAR void *context, FAR void *arg) +{ + UNUSED(irq); + UNUSED(context); + UNUSED(arg); + + nxsched_process_timer(); + return 0; +} +#endif + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +void up_timer_initialize(void) +{ + uint32_t regval; + + regval = getreg32(NVIC_SYSH12_15_PRIORITY); + regval &= ~NVIC_SYSH_PRIORITY_PR15_MASK; + regval |= (NVIC_SYSH_PRIORITY_DEFAULT << NVIC_SYSH_PRIORITY_PR15_SHIFT); + putreg32(regval, NVIC_SYSH12_15_PRIORITY); + +#if defined(CONFIG_ARMV7M_SYSTICK) && defined(CONFIG_TIMER_ARCH) + up_timer_set_lowerhalf(systick_initialize(true, HT32_HCLK_FREQUENCY, -1)); +#else + putreg32(HT32_SYSTICK_RELOAD, NVIC_SYSTICK_RELOAD); + + irq_attach(HT32_IRQ_SYSTICK, ht32f491x3_timerisr, NULL); + + putreg32(NVIC_SYSTICK_CTRL_CLKSOURCE | Review Comment: `ht32f491x3_timerisr()` is only compiled when both `CONFIG_ARMV7M_SYSTICK` and `CONFIG_TIMER_ARCH` are disabled, but `up_timer_initialize()` calls it in the `#else` branch whenever *either* of those options is disabled. This makes builds fail with an undefined symbol when `CONFIG_ARMV7M_SYSTICK=y` and `CONFIG_TIMER_ARCH=n` (or vice versa). Adjust the preprocessor guards so the ISR is available in all configurations where it is referenced (or avoid referencing it when it is not compiled). ########## arch/arm/src/ht32f491x3/ht32f491x3_start.c: ########## @@ -0,0 +1,114 @@ +/**************************************************************************** + * arch/arm/src/ht32f491x3/ht32f491x3_start.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdint.h> + +#include <nuttx/init.h> +#include <arch/irq.h> + +#include "arm_internal.h" + +#include "ht32f491x3_lowputc.h" +#include "ht32f491x3_serial.h" +#include "ht32f491x3_start.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifdef CONFIG_DEBUG_FEATURES +# define showprogress(c) arm_lowputc(c) +#else +# define showprogress(c) +#endif + +/**************************************************************************** + * Public Data + ****************************************************************************/ + +const uintptr_t g_idle_topstack = (uintptr_t)_END_BSS + + CONFIG_IDLETHREAD_STACKSIZE; Review Comment: `g_idle_topstack` is computed as `_END_BSS + CONFIG_IDLETHREAD_STACKSIZE`, but on ARMv7-M the interrupt stack (`CONFIG_ARCH_INTERRUPTSTACK`) is also carved out of the region between end-of-BSS and the heap. With `CONFIG_ARCH_INTERRUPTSTACK` enabled (as in the provided `esk32:nsh` defconfig), this calculation risks placing the heap on top of the interrupt stack, leading to memory corruption once interrupts are enabled. Consider including the interrupt stack reservation (e.g., `INTSTACK_SIZE` from `arm_internal.h`) in the `g_idle_topstack` computation, and ensure the vector table initial SP matches the same reserved stack layout. ```suggestion const uintptr_t g_idle_topstack = (uintptr_t)_END_BSS #if defined(CONFIG_ARMV7M) && defined(CONFIG_ARCH_INTERRUPTSTACK) + INTSTACK_SIZE #endif + CONFIG_IDLETHREAD_STACKSIZE; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
