You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "comejv (via GitHub)" <gi...@apache.org> on 2025/08/04 18:18:19 UTC

[PR] arch/arm/stm32h7: Port timer capture driver from stm32 [nuttx]

comejv opened a new pull request, #16809:
URL: https://github.com/apache/nuttx/pull/16809

   ## Summary
   
   This commit introduces a timer capture driver for the STM32H7 series ported from the STM32 F series.
   
   The current implementation is based on a driver originally for PWM input, and as such, it calculates duty cycle and frequency. It is also limited to a single capture channel per timer.
   
   This serves as a foundation for future development of more comprehensive input capture capabilities on STM32H7 platforms.
   
   The main changes include:
   - A new generic timer capture driver for STM32H7.
   - A lower-half driver to integrate with the NuttX capture subsystem.
   - Kconfig options to enable and configure capture for various timers.
   - Pin definitions for TIM1-4 capture inputs on the nucleo-h743zi.
   - An update to `cap_register_multiple` to handle multiple device registration.
   - An update to `stm32_bringup` to register the capture drivers.
   
   ## Impact
   
   STM32H7 users can now use the timer capture devices after enabling them in menuconfig.
   
   ## Testing
   
   Tested by polling and reading `/dev/cap0-4` with
   `ioctl(fds[i], CAPIOC_FREQUENCE, freq)` while sending a square wave signal to appropriate pins and checking frequency.
   


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3154718788

   @comejv nice work! Kudos!!!
   
   Please include a "capture" board configure and include documentation, similar to:
   
   https://nuttx.apache.org/docs/latest/platforms/xtensa/esp32/boards/esp32-devkitc/index.html#capture


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3151896710

   I seem to have formatting errors, is there a clang-format file that I could use? What are the formatting rules?


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3160776963

   Converting to draft until I solve the issues I have with the current driver (#16762)


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3154688896

   @comejv please use checkpatch.sh as explained here: https://nuttx.apache.org/docs/latest/contributing/making-changes.html


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3151944921

   I tried running `uncrustify -c tools/uncrustify.cfg` but I just got more errors.


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3151894198

   Maybe the changes to drivers/timers/capture.c should be in a different pull request? The register_multiple was previously identical to a single device registration, now properly iterates over the given `FAR struct cap_lowerhalf_s **lower` array.


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "hartmannathan (via GitHub)" <gi...@apache.org>.
hartmannathan commented on code in PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#discussion_r2257395268


##########
arch/arm/src/stm32h7/stm32_capture.h:
##########
@@ -0,0 +1,240 @@
+/****************************************************************************
+ * arch/arm/src/stm32h7/stm32_capture.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+#define __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include "chip.h"
+#include <arch/board/board.h>
+#include "hardware/stm32_tim.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Helpers ******************************************************************/
+
+#define STM32_CAP_SETSMC(d,cfg)                 ((d)->ops->setsmc(d,cfg))
+#define STM32_CAP_SETCLOCK(d,clk,max)           ((d)->ops->setclock(d,clk,max))
+#define STM32_CAP_SETCHANNEL(d,ch,cfg)          ((d)->ops->setchannel(d,ch,cfg))
+#define STM32_CAP_GETCAPTURE(d,ch)              ((d)->ops->getcapture(d,ch))
+#define STM32_CAP_SETISR(d,hnd,arg)             ((d)->ops->setisr(d,hnd,arg))
+#define STM32_CAP_ENABLEINT(d,s,on)             ((d)->ops->enableint(d,s,on))
+#define STM32_CAP_ACKFLAGS(d,f)                 ((d)->ops->ackflags(d,f))
+#define STM32_CAP_GETFLAGS(d)                   ((d)->ops->getflags(d))
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/* Capture Device Structure */
+
+struct stm32_cap_dev_s
+{
+  struct stm32_cap_ops_s *ops;
+};
+
+/* Capture input EDGE sources */
+
+typedef enum
+{
+  /* Mapped */
+
+  STM32_CAP_MAPPED_MASK         = (GTIM_CCMR1_CC1S_MASK),
+  STM32_CAP_MAPPED_TI1          = (GTIM_CCMR_CCS_CCIN1),
+  STM32_CAP_MAPPED_TI2          = (GTIM_CCMR_CCS_CCIN2),
+
+/* TODO STM32_CAP_MAPPED_TRC     = (GTIM_CCMR_CCS_CCINTRC), */
+
+  /* Event prescaler */
+
+  STM32_CAP_INPSC_MASK          = (GTIM_CCMR1_IC1PSC_MASK),
+  STM32_CAP_INPSC_NO            = (0 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_2EVENTS       = (1 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_4EVENTS       = (2 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_8EVENTS       = (3 << GTIM_CCMR1_IC1PSC_SHIFT),
+
+  /* Event prescaler */
+
+  STM32_CAP_FILTER_MASK         = (GTIM_CCMR1_IC1F_MASK),
+  STM32_CAP_FILTER_NO           = (0 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* Internal clock with N time to confirm event */
+
+  STM32_CAP_FILTER_INT_N2       = (1 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N4       = (2 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N8       = (3 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* DTS clock div by D with N time to confirm event */
+
+  STM32_CAP_FILTER_DTS_D2_N6    = (4 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D2_N8    = (5 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N6    = (6 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N8    = (7 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N6    = (8 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N8    = (9 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N5   = (10 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N6   = (11 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N8   = (12 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N5   = (13 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N6   = (14 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N8   = (15 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* EDGE */
+
+  STM32_CAP_EDGE_MASK           = (3 << 8),
+  STM32_CAP_EDGE_DISABLED       = (0 << 8),
+  STM32_CAP_EDGE_RISING         = (1 << 8),
+  STM32_CAP_EDGE_FALLING        = (2 << 8),
+  STM32_CAP_EDGE_BOTH           = (3 << 8),

Review Comment:
   I wonder if the 8 could be replaced with a define to make the code more self-documenting?
   
   (I looked through the microcontroller reference manual and the `arch/arm/src/stm32h7/hardware/stm32_tim.h` header and these do not appear to be register bits? Perhaps they only pertain to the NuttX driver implementation, or I missed something...)



-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3161748328

   Had missed a formatting error in the cmakelist. Sorry for the mess of commits and force-pushs!


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #16809:
URL: https://github.com/apache/nuttx/pull/16809


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3161652795

   I forgot to fix the spaces after comments, will push again...


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3161749858

   Will make documentation later this week.


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3161649234

   I am still failing the checkpatch checks but they don't really make sense imo, like for example asking me to add an empty line after this comment:
   
   ```c
     /* Nothing to do if no timers enabled */
     if (count == 0)
       {
         return OK;
       }
   ```


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on code in PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#discussion_r2257699767


##########
arch/arm/src/stm32h7/stm32_capture.h:
##########
@@ -0,0 +1,240 @@
+/****************************************************************************
+ * arch/arm/src/stm32h7/stm32_capture.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+#define __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include "chip.h"
+#include <arch/board/board.h>
+#include "hardware/stm32_tim.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Helpers ******************************************************************/
+
+#define STM32_CAP_SETSMC(d,cfg)                 ((d)->ops->setsmc(d,cfg))
+#define STM32_CAP_SETCLOCK(d,clk,max)           ((d)->ops->setclock(d,clk,max))
+#define STM32_CAP_SETCHANNEL(d,ch,cfg)          ((d)->ops->setchannel(d,ch,cfg))
+#define STM32_CAP_GETCAPTURE(d,ch)              ((d)->ops->getcapture(d,ch))
+#define STM32_CAP_SETISR(d,hnd,arg)             ((d)->ops->setisr(d,hnd,arg))
+#define STM32_CAP_ENABLEINT(d,s,on)             ((d)->ops->enableint(d,s,on))
+#define STM32_CAP_ACKFLAGS(d,f)                 ((d)->ops->ackflags(d,f))
+#define STM32_CAP_GETFLAGS(d)                   ((d)->ops->getflags(d))
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/* Capture Device Structure */
+
+struct stm32_cap_dev_s
+{
+  struct stm32_cap_ops_s *ops;
+};
+
+/* Capture input EDGE sources */
+
+typedef enum
+{
+  /* Mapped */
+
+  STM32_CAP_MAPPED_MASK         = (GTIM_CCMR1_CC1S_MASK),
+  STM32_CAP_MAPPED_TI1          = (GTIM_CCMR_CCS_CCIN1),
+  STM32_CAP_MAPPED_TI2          = (GTIM_CCMR_CCS_CCIN2),
+
+/* TODO STM32_CAP_MAPPED_TRC     = (GTIM_CCMR_CCS_CCINTRC), */
+
+  /* Event prescaler */
+
+  STM32_CAP_INPSC_MASK          = (GTIM_CCMR1_IC1PSC_MASK),
+  STM32_CAP_INPSC_NO            = (0 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_2EVENTS       = (1 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_4EVENTS       = (2 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_8EVENTS       = (3 << GTIM_CCMR1_IC1PSC_SHIFT),
+
+  /* Event prescaler */
+
+  STM32_CAP_FILTER_MASK         = (GTIM_CCMR1_IC1F_MASK),
+  STM32_CAP_FILTER_NO           = (0 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* Internal clock with N time to confirm event */
+
+  STM32_CAP_FILTER_INT_N2       = (1 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N4       = (2 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N8       = (3 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* DTS clock div by D with N time to confirm event */
+
+  STM32_CAP_FILTER_DTS_D2_N6    = (4 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D2_N8    = (5 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N6    = (6 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N8    = (7 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N6    = (8 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N8    = (9 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N5   = (10 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N6   = (11 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N8   = (12 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N5   = (13 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N6   = (14 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N8   = (15 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* EDGE */
+
+  STM32_CAP_EDGE_MASK           = (3 << 8),
+  STM32_CAP_EDGE_DISABLED       = (0 << 8),
+  STM32_CAP_EDGE_RISING         = (1 << 8),
+  STM32_CAP_EDGE_FALLING        = (2 << 8),
+  STM32_CAP_EDGE_BOTH           = (3 << 8),

Review Comment:
   My understanding is that these vales are just a way to pass parameters to the stm32_cap_setchannel function through the `stm32_cap_ch_cfg_t` parameter, so NuttX implementation specific I think. We could add a define in `stm32_tim.h` after:
   ```c
   /*                                             Bits 1-0
    *                                            (Same as Output Compare Mode)
    */
   
   #define GTIM_CCMR1_IC1PSC_SHIFT   (2)       /* Bits 3-2: Input Capture 1 Prescaler */
   #define GTIM_CCMR1_IC1PSC_MASK    (3 << GTIM_CCMR1_IC1PSC_SHIFT)
                                               /* (See common CCMR Input Capture Prescaler definitions below) */
   #define GTIM_CCMR1_IC1F_SHIFT     (4)       /* Bits 7-4: Input Capture 1 Filter */
   #define GTIM_CCMR1_IC1F_MASK      (0x0f << GTIM_CCMR1_IC1F_SHIFT)
   ```
   
   maybe something like `#define CAP_CH_EDGE_SHIFT`? But also this value isn't used anywhere else so it could just be a local macro.



-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#discussion_r2260375965


##########
arch/arm/src/stm32h7/stm32_capture.h:
##########
@@ -0,0 +1,240 @@
+/****************************************************************************
+ * arch/arm/src/stm32h7/stm32_capture.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+#define __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include "chip.h"
+#include <arch/board/board.h>
+#include "hardware/stm32_tim.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Helpers ******************************************************************/
+
+#define STM32_CAP_SETSMC(d,cfg)                 ((d)->ops->setsmc(d,cfg))
+#define STM32_CAP_SETCLOCK(d,clk,max)           ((d)->ops->setclock(d,clk,max))
+#define STM32_CAP_SETCHANNEL(d,ch,cfg)          ((d)->ops->setchannel(d,ch,cfg))
+#define STM32_CAP_GETCAPTURE(d,ch)              ((d)->ops->getcapture(d,ch))
+#define STM32_CAP_SETISR(d,hnd,arg)             ((d)->ops->setisr(d,hnd,arg))
+#define STM32_CAP_ENABLEINT(d,s,on)             ((d)->ops->enableint(d,s,on))
+#define STM32_CAP_ACKFLAGS(d,f)                 ((d)->ops->ackflags(d,f))
+#define STM32_CAP_GETFLAGS(d)                   ((d)->ops->getflags(d))
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/* Capture Device Structure */
+
+struct stm32_cap_dev_s
+{
+  struct stm32_cap_ops_s *ops;
+};
+
+/* Capture input EDGE sources */
+
+typedef enum
+{
+  /* Mapped */
+
+  STM32_CAP_MAPPED_MASK         = (GTIM_CCMR1_CC1S_MASK),
+  STM32_CAP_MAPPED_TI1          = (GTIM_CCMR_CCS_CCIN1),
+  STM32_CAP_MAPPED_TI2          = (GTIM_CCMR_CCS_CCIN2),
+
+/* TODO STM32_CAP_MAPPED_TRC     = (GTIM_CCMR_CCS_CCINTRC), */
+
+  /* Event prescaler */
+
+  STM32_CAP_INPSC_MASK          = (GTIM_CCMR1_IC1PSC_MASK),
+  STM32_CAP_INPSC_NO            = (0 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_2EVENTS       = (1 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_4EVENTS       = (2 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_8EVENTS       = (3 << GTIM_CCMR1_IC1PSC_SHIFT),
+
+  /* Event prescaler */
+
+  STM32_CAP_FILTER_MASK         = (GTIM_CCMR1_IC1F_MASK),
+  STM32_CAP_FILTER_NO           = (0 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* Internal clock with N time to confirm event */
+
+  STM32_CAP_FILTER_INT_N2       = (1 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N4       = (2 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N8       = (3 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* DTS clock div by D with N time to confirm event */
+
+  STM32_CAP_FILTER_DTS_D2_N6    = (4 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D2_N8    = (5 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N6    = (6 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N8    = (7 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N6    = (8 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N8    = (9 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N5   = (10 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N6   = (11 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N8   = (12 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N5   = (13 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N6   = (14 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N8   = (15 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* EDGE */
+
+  STM32_CAP_EDGE_MASK           = (3 << 8),
+  STM32_CAP_EDGE_DISABLED       = (0 << 8),
+  STM32_CAP_EDGE_RISING         = (1 << 8),
+  STM32_CAP_EDGE_FALLING        = (2 << 8),
+  STM32_CAP_EDGE_BOTH           = (3 << 8),

Review Comment:
   Yes, I think you could "define CAP_CH_EDGE_SHIFT  8" and use it afterwards, I think the original author forgot to define the _SHIFT and this end up replicated on F7 and now on H7. I will merge this way, but if you can, please send later a PR fixing these three families



-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#discussion_r2260375965


##########
arch/arm/src/stm32h7/stm32_capture.h:
##########
@@ -0,0 +1,240 @@
+/****************************************************************************
+ * arch/arm/src/stm32h7/stm32_capture.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+#define __ARCH_ARM_SRC_STM32H7_STM32_CAPTURE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include "chip.h"
+#include <arch/board/board.h>
+#include "hardware/stm32_tim.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Helpers ******************************************************************/
+
+#define STM32_CAP_SETSMC(d,cfg)                 ((d)->ops->setsmc(d,cfg))
+#define STM32_CAP_SETCLOCK(d,clk,max)           ((d)->ops->setclock(d,clk,max))
+#define STM32_CAP_SETCHANNEL(d,ch,cfg)          ((d)->ops->setchannel(d,ch,cfg))
+#define STM32_CAP_GETCAPTURE(d,ch)              ((d)->ops->getcapture(d,ch))
+#define STM32_CAP_SETISR(d,hnd,arg)             ((d)->ops->setisr(d,hnd,arg))
+#define STM32_CAP_ENABLEINT(d,s,on)             ((d)->ops->enableint(d,s,on))
+#define STM32_CAP_ACKFLAGS(d,f)                 ((d)->ops->ackflags(d,f))
+#define STM32_CAP_GETFLAGS(d)                   ((d)->ops->getflags(d))
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/* Capture Device Structure */
+
+struct stm32_cap_dev_s
+{
+  struct stm32_cap_ops_s *ops;
+};
+
+/* Capture input EDGE sources */
+
+typedef enum
+{
+  /* Mapped */
+
+  STM32_CAP_MAPPED_MASK         = (GTIM_CCMR1_CC1S_MASK),
+  STM32_CAP_MAPPED_TI1          = (GTIM_CCMR_CCS_CCIN1),
+  STM32_CAP_MAPPED_TI2          = (GTIM_CCMR_CCS_CCIN2),
+
+/* TODO STM32_CAP_MAPPED_TRC     = (GTIM_CCMR_CCS_CCINTRC), */
+
+  /* Event prescaler */
+
+  STM32_CAP_INPSC_MASK          = (GTIM_CCMR1_IC1PSC_MASK),
+  STM32_CAP_INPSC_NO            = (0 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_2EVENTS       = (1 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_4EVENTS       = (2 << GTIM_CCMR1_IC1PSC_SHIFT),
+  STM32_CAP_INPSC_8EVENTS       = (3 << GTIM_CCMR1_IC1PSC_SHIFT),
+
+  /* Event prescaler */
+
+  STM32_CAP_FILTER_MASK         = (GTIM_CCMR1_IC1F_MASK),
+  STM32_CAP_FILTER_NO           = (0 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* Internal clock with N time to confirm event */
+
+  STM32_CAP_FILTER_INT_N2       = (1 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N4       = (2 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_INT_N8       = (3 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* DTS clock div by D with N time to confirm event */
+
+  STM32_CAP_FILTER_DTS_D2_N6    = (4 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D2_N8    = (5 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N6    = (6 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D4_N8    = (7 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N6    = (8 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D8_N8    = (9 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N5   = (10 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N6   = (11 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D16_N8   = (12 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N5   = (13 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N6   = (14 << GTIM_CCMR1_IC1F_SHIFT),
+  STM32_CAP_FILTER_DTS_D32_N8   = (15 << GTIM_CCMR1_IC1F_SHIFT),
+
+  /* EDGE */
+
+  STM32_CAP_EDGE_MASK           = (3 << 8),
+  STM32_CAP_EDGE_DISABLED       = (0 << 8),
+  STM32_CAP_EDGE_RISING         = (1 << 8),
+  STM32_CAP_EDGE_FALLING        = (2 << 8),
+  STM32_CAP_EDGE_BOTH           = (3 << 8),

Review Comment:
   Yes, I think you could "define CAP_CH_EDGE_SHIFT  8" and use it afterwards



-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3151989215

   Force pushed because I also forgot to sign off the commit.


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "hartmannathan (via GitHub)" <gi...@apache.org>.
hartmannathan commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3161437131

   > Converting to draft until I solve the issues I have with the current driver (#16762)
   
   Hi, my comment about changing the 8 to a define shouldn't block this PR. You can change it to a define if you think it makes sense, or just leave it as-is.
   
   Of course, if there are other issues you need to solve, keeping this as draft is a good idea until then.
   
   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: Port timer capture driver from stm32 [nuttx]

Posted by "comejv (via GitHub)" <gi...@apache.org>.
comejv commented on PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#issuecomment-3161676387

   And now I used the wrong commit message... Shouldn't do all that last thing in the day lol.


-- 
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: Port timer capture driver from stm32 [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #16809:
URL: https://github.com/apache/nuttx/pull/16809#discussion_r2253969689


##########
arch/arm/src/stm32h7/Kconfig:
##########
@@ -4584,125 +4584,451 @@ config STM32H7_ADC3_TIMTRIG
 	---help---
 		Values 0:CC1 1:CC2 2:CC3 3:CC4 4:TRGO
 
+config STM32H7_CAP
+	bool
+	default n
+  select CAPTURE

Review Comment:
   Please use TAB here to format the line



##########
drivers/timers/capture.c:
##########
@@ -469,30 +469,27 @@ int cap_register(FAR const char *devpath, FAR struct cap_lowerhalf_s *lower)
 }
 
 int cap_register_multiple(FAR const char *devpath,
-                          FAR struct cap_lowerhalf_s **lower, int n)
+                          FAR struct cap_lowerhalf_s **lower,
+                          int n)
 {
-  FAR struct cap_upperhalf_s *upper;
+  char fullpath[16];
+  int ret;
 
-  /* Allocate the upper-half data structure */
+  if (n < 1) return -EINVAL;

Review Comment:
   Please change to:
   ```
     if (n < 1)
       {
          return -EINVAL;
       }
   ```



-- 
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