You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "vrmay23 (via GitHub)" <gi...@apache.org> on 2026/02/01 19:11:01 UTC
[PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
vrmay23 opened a new pull request, #18300:
URL: https://github.com/apache/nuttx/pull/18300
fix clock, ILS register and ID (extended).
Fix three issues in the STM32H7 FDCAN SocketCAN driver:
1. Clock configuration: Allow board.h to override STM32_FDCANCLK. Previously the driver hardcoded STM32_HSE_FREQUENCY, ignoring any board-specific clock configuration.
2. ILS register bug: Fix putreg32 call that was writing FDCAN_ILS_TCL constant instead of the computed regval, causing interrupt routing issues.
3. Extended ID filter size: Increase n_extid from 64 to 128. Despite the reference manual (RM0433) suggesting 64 max, testing shows that 128 is required for reliable extended ID frame reception. With 64, some extended ID frames were silently dropped.
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).*
## Summary
*Update this section with information on why change is necessary,
what it exactly does and how, if new feature shows up, provide
references (dependencies, similar problems and solutions), etc.*
## Impact
*Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.*
## Testing
*This section should provide a detailed description of what you did
to verify your changes work and do not break existing code.*
*Please provide information about your host machine, the board(s) you
tested your changes on, and how you tested. Logs should be included.*
*For example, when changing something in the core OS functions, you
may want to run the OSTest application to verify that there are no
regressions. Changes to ADC code may warrant running the `adc`
example. Adding a new uORB driver may require that you run
`uorb_listener` to verify correct operation.*
*Pure documentation changes can just be tested with `make html`
(see docs) and verification of the correct format in your
browser.*
**_PRs without testing information will not be accepted. We will
request test logs._**
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2756268922
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2066,6 +2065,10 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
fdcan_setconfig(priv->base, 1);
+ /* Clear Message RAM */
+
+ memset((void *)STM32_CANRAM_BASE, 0, 2560 * 4);
Review Comment:
Is this a necessary operation? Can the memory be left uninitialized?
I would suggest using `define` macros to replace the magic numbers 2560 and 4 so that readers know what those numbers refer to.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#issuecomment-3837386971
@xiaoxiang781216 I have finished the reviews. Thanks for feedback!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2755423289
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2320,6 +2356,14 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
fdcan_dumpregs(priv);
#endif
+ /* Debug
Review Comment:
done
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2284,7 +2306,21 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
regval = (ram_offset << FDCAN_RXF0C_F0SA_SHIFT) & FDCAN_RXF0C_F0SA_MASK;
regval |= (NUM_RX_FIFO0 << FDCAN_RXF0C_F0S_SHIFT) & FDCAN_RXF0C_F0S_MASK;
+
+ /* DEBUG
+ * printf("[vmay] NUM_RX_FIFO0=%d SHIFT=%d MASK=0x%lx\n",
+ * NUM_RX_FIFO0, FDCAN_RXF0C_F0S_SHIFT, FDCAN_RXF0C_F0S_MASK);
+ */
+
putreg32(regval, priv->base + STM32_FDCAN_RXF0C_OFFSET);
+
+ /* DEBUG
Review Comment:
done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2755421658
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -1276,6 +1279,12 @@ static void fdcan_receive_work(void *arg)
static void fdcan_txdone(struct fdcan_driver_s *priv)
{
+ /* DEBUG
+ * printf("[vmay] TXDONE: IR=0x%lx RXF0S=0x%lx\n",
Review Comment:
yes, for sure! I forgot to remove the comments. Thank you!
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2042,6 +2051,11 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
irqstate_t flags = enter_critical_section();
+ /* DEBUG
Review Comment:
done!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#issuecomment-3837330103
> Please fill out the PR template
I think it is correct now. Could you please check it again? Thank you!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2759920701
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2066,6 +2065,10 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
fdcan_setconfig(priv->base, 1);
+ /* Clear Message RAM */
+
+ memset((void *)STM32_CANRAM_BASE, 0, 2560 * 4);
Review Comment:
Yes, it is necessary. Message RAM is not cleared by hardware reset. Without
initialization, filters and FIFO headers may contain garbage data, causing
silent frame loss or spurious interrupts. This was observed in practice on
Nucleo-H753ZI before this fix.
Regarding the "Magic numbers" I totally agree!!
I Will replace with proper defines. Will update in next push. Basically it canes from ST ddefinition (total words in RAM).
I'll try to update it today!
Thank you for feedback :)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2755419999
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -1420,7 +1429,7 @@ static int fdcan_interrupt(int irq, void *context,
if (irq == priv->config->mb_irq[0])
{
- fdcan_receive(priv);
+ fdcan_receive(priv);
Review Comment:
Hello! nxstylle tool complained about it when I ran the script. Just it.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2752356727
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -1420,7 +1429,7 @@ static int fdcan_interrupt(int irq, void *context,
if (irq == priv->config->mb_irq[0])
{
- fdcan_receive(priv);
+ fdcan_receive(priv);
Review Comment:
why change
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -1276,6 +1279,12 @@ static void fdcan_receive_work(void *arg)
static void fdcan_txdone(struct fdcan_driver_s *priv)
{
+ /* DEBUG
+ * printf("[vmay] TXDONE: IR=0x%lx RXF0S=0x%lx\n",
Review Comment:
remove the comment?
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2042,6 +2051,11 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
irqstate_t flags = enter_critical_section();
+ /* DEBUG
Review Comment:
ditto
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2320,6 +2356,14 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
fdcan_dumpregs(priv);
#endif
+ /* Debug
Review Comment:
ditto
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2284,7 +2306,21 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
regval = (ram_offset << FDCAN_RXF0C_F0SA_SHIFT) & FDCAN_RXF0C_F0SA_MASK;
regval |= (NUM_RX_FIFO0 << FDCAN_RXF0C_F0S_SHIFT) & FDCAN_RXF0C_F0S_MASK;
+
+ /* DEBUG
+ * printf("[vmay] NUM_RX_FIFO0=%d SHIFT=%d MASK=0x%lx\n",
+ * NUM_RX_FIFO0, FDCAN_RXF0C_F0S_SHIFT, FDCAN_RXF0C_F0S_MASK);
+ */
+
putreg32(regval, priv->base + STM32_FDCAN_RXF0C_OFFSET);
+
+ /* DEBUG
Review Comment:
ditto
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2284,7 +2306,21 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
regval = (ram_offset << FDCAN_RXF0C_F0SA_SHIFT) & FDCAN_RXF0C_F0SA_MASK;
regval |= (NUM_RX_FIFO0 << FDCAN_RXF0C_F0S_SHIFT) & FDCAN_RXF0C_F0S_MASK;
+
+ /* DEBUG
+ * printf("[vmay] NUM_RX_FIFO0=%d SHIFT=%d MASK=0x%lx\n",
Review Comment:
ditto
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#issuecomment-3849588597
Starting up:
<img width="1477" height="772" alt="image" src="https://github.com/user-attachments/assets/8e84ce3e-6b01-4ae6-b2bc-b5e7f6edb848" />
candump (500kpbs 1 msg per 10 ms):
<img width="1474" height="754" alt="image" src="https://github.com/user-attachments/assets/a1a8ac6a-2a38-4c13-844b-9daea96ecf14" />
cansend (I have created an 'canflood app' to test NuttX SocketCAN transmission power, later on I'll also submit this PR):
<img width="1474" height="754" alt="image" src="https://github.com/user-attachments/assets/e57b6842-fcf7-4de3-8706-109adce783b3" />
<img width="1919" height="744" alt="image" src="https://github.com/user-attachments/assets/88a2e9ac-78a3-4337-ae51-69240e64039d" />
I'm wonder if it is needed a long stress test. If needed just tell me how to provide it.
Thanks!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/stm32h7: fix sdcan clock, ILS register and ID [nuttx]
Posted by "jerpelea (via GitHub)" <gi...@apache.org>.
jerpelea commented on PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#issuecomment-3852360742
@linguini1 please review
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/stm32h7: fix sdcan clock, ILS register and ID [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #18300:
URL: https://github.com/apache/nuttx/pull/18300
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2756070265
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -1420,7 +1429,7 @@ static int fdcan_interrupt(int irq, void *context,
if (irq == priv->config->mb_irq[0])
{
- fdcan_receive(priv);
+ fdcan_receive(priv);
Review Comment:
@xiaoxiang781216 I retested again and there was no issues. I have back to previous one. Thank you.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/src/stm32h7/stm32_fdcan_sock: fix clock, ILS register and ID [nuttx]
Posted by "vrmay23 (via GitHub)" <gi...@apache.org>.
vrmay23 commented on code in PR #18300:
URL: https://github.com/apache/nuttx/pull/18300#discussion_r2755424577
##########
arch/arm/src/stm32h7/stm32_fdcan_sock.c:
##########
@@ -2284,7 +2306,21 @@ int fdcan_initialize(struct fdcan_driver_s *priv)
regval = (ram_offset << FDCAN_RXF0C_F0SA_SHIFT) & FDCAN_RXF0C_F0SA_MASK;
regval |= (NUM_RX_FIFO0 << FDCAN_RXF0C_F0S_SHIFT) & FDCAN_RXF0C_F0S_MASK;
+
+ /* DEBUG
+ * printf("[vmay] NUM_RX_FIFO0=%d SHIFT=%d MASK=0x%lx\n",
Review Comment:
done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org