You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/04 08:34:00 UTC
[GitHub] [incubator-nuttx] robertobucher opened a new pull request, #6568: Files for stm32h7zi2
robertobucher opened a new pull request, #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568
## Summary
Files for using pysimCoder with the NUCLEO-H743ZI2 board
## Impact
## Testing
--
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
[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#discussion_r912872900
##########
boards/arm/stm32h7/nucleo-h743zi2/src/nucleo-h743zi2.h:
##########
@@ -148,4 +201,20 @@ void weak_function stm32_usbinitialize(void);
int stm32_usbhost_initialize(void);
#endif
+/****************************************************************************
+ * Name: stm32_pwm_setup
+ *
+ * Description:
+ * Initialize PWM and register the PWM device.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_PWM
+int stm32_pwm_setup(void);
+#endif
+
+#ifdef CONFIG_SENSORS_QENCODER
+int stm32h7_qencoder_initialize(const char *devpath, int timer);
Review Comment:
@pkarashchenko although I agree with your suggestion, I think we are doing it incorrectly: see, all arch:stm32/stm32f7/stm32h7, etc are using "stm32_" for function names. It makes it hard to IDE find the right definition. I think we should always adopt the arch name instead using a same stm32_ prefix for all of them.
--
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
[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#discussion_r912869018
##########
boards/arm/stm32h7/nucleo-h743zi2/src/stm32_pwm.c:
##########
@@ -0,0 +1,113 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi2/src/stm32_pwm.c
+ *
+ * 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 <errno.h>
+#include <debug.h>
+
+#include <nuttx/timers/pwm.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "arm_internal.h"
+#include "stm32_pwm.h"
+#include "nucleo-h743zi2.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define OK 0
Review Comment:
@robertobucher please use sys/types.h
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#discussion_r912890679
##########
boards/arm/stm32h7/nucleo-h743zi2/src/nucleo-h743zi2.h:
##########
@@ -148,4 +201,20 @@ void weak_function stm32_usbinitialize(void);
int stm32_usbhost_initialize(void);
#endif
+/****************************************************************************
+ * Name: stm32_pwm_setup
+ *
+ * Description:
+ * Initialize PWM and register the PWM device.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_PWM
+int stm32_pwm_setup(void);
+#endif
+
+#ifdef CONFIG_SENSORS_QENCODER
+int stm32h7_qencoder_initialize(const char *devpath, int timer);
Review Comment:
yes. I just want to make things consistent. Either all use `stm32h7_` or `stm32_`
--
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
[GitHub] [incubator-nuttx] acassis commented on pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#issuecomment-1174181551
Hi @robertobucher please squash these 5 commits in a single one. Then you will see all the files. Also please change the PR name to something like: "boards/stm32h7zi2: Add support to PWM, ADC and QE to use with PySimCoder"
--
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
[GitHub] [incubator-nuttx] robertobucher commented on pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
robertobucher commented on PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#issuecomment-1174001025
The "pull request" seems ok, but I can't see my new files under boards/arm/stm32h7/nucleo-h743zi2/src:
stm32_pwm.c
stm32_qencoder.c
stm32_gpio.c
and the modified files for pysimCoder
stm32_bringup.c, stm32_appinitialize etc...
--
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
[GitHub] [incubator-nuttx] robertobucher closed pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
robertobucher closed pull request #6568: Files for stm32h7zi2
URL: https://github.com/apache/incubator-nuttx/pull/6568
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#discussion_r913039747
##########
arch/arm/src/stm32h7/stm32_adc.c:
##########
@@ -767,7 +767,7 @@ static int adc_timinit(struct stm32_dev_s *priv)
* position.
*/
- ainfo("Initializing timers extsel = 0x%08x\n", priv->extsel);
+ ainfo("Initializing timers extsel = 0x%08lx\n", priv->extsel);
Review Comment:
I think it is better to use `PRIx` family specifiers here and in other similar places
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#discussion_r912838308
##########
boards/arm/stm32h7/nucleo-h743zi2/src/stm32_adc.c:
##########
@@ -0,0 +1,219 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi2/src/stm32_adc.c
+ *
+ * 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 <errno.h>
+#include <debug.h>
+
+#include <nuttx/board.h>
+#include <nuttx/analog/adc.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "stm32_gpio.h"
+#include "stm32_adc.h"
+#include "nucleo-h743zi2.h"
+
+#ifdef CONFIG_ADC
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* Up to 3 ADC interfaces are supported */
+
+#if defined(CONFIG_STM32H7_ADC1) || defined(CONFIG_STM32H7_ADC2) || \
+ defined(CONFIG_STM32H7_ADC3)
+#ifndef CONFIG_STM32H7_ADC1
+# warning "Channel information only available for ADC1"
+#endif
+
+/* The number of ADC channels in the conversion list */
+
+#define ADC1_NCHANNELS 5
+#define ADC3_NCHANNELS 1
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_STM32H7_ADC1
+/* Identifying number of each ADC channel: Variable Resistor.
+ *
+ * ADC1: {5, 10, 12, 13, 15};
+ */
+
+static const uint8_t g_adc1_chanlist[ADC1_NCHANNELS] =
Review Comment:
```suggestion
static const uint8_t g_adc1_chanlist[ADC1_NCHANNELS] =
```
##########
boards/arm/stm32h7/nucleo-h743zi2/src/stm32_pwm.c:
##########
@@ -0,0 +1,113 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi2/src/stm32_pwm.c
+ *
+ * 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 <errno.h>
+#include <debug.h>
+
+#include <nuttx/timers/pwm.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "arm_internal.h"
+#include "stm32_pwm.h"
+#include "nucleo-h743zi2.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define OK 0
Review Comment:
maybe better to add proper include?
##########
boards/arm/stm32h7/nucleo-h743zi2/src/stm32_adc.c:
##########
@@ -0,0 +1,219 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi2/src/stm32_adc.c
+ *
+ * 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 <errno.h>
+#include <debug.h>
+
+#include <nuttx/board.h>
+#include <nuttx/analog/adc.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "stm32_gpio.h"
+#include "stm32_adc.h"
+#include "nucleo-h743zi2.h"
+
+#ifdef CONFIG_ADC
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* Up to 3 ADC interfaces are supported */
+
+#if defined(CONFIG_STM32H7_ADC1) || defined(CONFIG_STM32H7_ADC2) || \
+ defined(CONFIG_STM32H7_ADC3)
+#ifndef CONFIG_STM32H7_ADC1
+# warning "Channel information only available for ADC1"
+#endif
+
+/* The number of ADC channels in the conversion list */
+
+#define ADC1_NCHANNELS 5
+#define ADC3_NCHANNELS 1
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_STM32H7_ADC1
+/* Identifying number of each ADC channel: Variable Resistor.
+ *
+ * ADC1: {5, 10, 12, 13, 15};
+ */
+
+static const uint8_t g_adc1_chanlist[ADC1_NCHANNELS] =
+{
+ 5, 10, 12, 13, 15
+};
+
+/* Configurations of pins used by each ADC channels
+ *
+ * ADC1:
+ * {GPIO_ADC12_INP5, GPIO_ADC123_INP10, GPIO_ADC123_INP12, GPIO_ADC12_INP13,
+ * GPIO_ADC12_INP15};
+ */
+
+static const uint32_t g_adc1_pinlist[ADC1_NCHANNELS] =
+{
+ GPIO_ADC12_INP5,
+ GPIO_ADC123_INP10,
+ GPIO_ADC123_INP12,
+ GPIO_ADC12_INP13,
+ GPIO_ADC12_INP15
+};
+#endif
+
+#ifdef CONFIG_STM32H7_ADC3
+/* Identifying number of each ADC channel: Variable Resistor.
+ *
+ * ADC3: {6,};
+ */
+
+static const uint8_t g_adc3_chanlist[ADC1_NCHANNELS] =
Review Comment:
```suggestion
static const uint8_t g_adc3_chanlist[ADC1_NCHANNELS] =
```
##########
boards/arm/stm32h7/nucleo-h743zi2/src/nucleo-h743zi2.h:
##########
@@ -148,4 +201,20 @@ void weak_function stm32_usbinitialize(void);
int stm32_usbhost_initialize(void);
#endif
+/****************************************************************************
+ * Name: stm32_pwm_setup
+ *
+ * Description:
+ * Initialize PWM and register the PWM device.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_PWM
+int stm32_pwm_setup(void);
+#endif
+
+#ifdef CONFIG_SENSORS_QENCODER
+int stm32h7_qencoder_initialize(const char *devpath, int timer);
Review Comment:
Please add description in comments.
Maybe also is relevant to rename from `stm32h7_qencoder_initialize` to `stm32_qencoder_initialize` to follow similar style as in other places?
--
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
[GitHub] [incubator-nuttx] robertobucher commented on a diff in pull request #6568: Files for stm32h7zi2
Posted by GitBox <gi...@apache.org>.
robertobucher commented on code in PR #6568:
URL: https://github.com/apache/incubator-nuttx/pull/6568#discussion_r913062122
##########
arch/arm/src/stm32h7/stm32_adc.c:
##########
@@ -767,7 +767,7 @@ static int adc_timinit(struct stm32_dev_s *priv)
* position.
*/
- ainfo("Initializing timers extsel = 0x%08x\n", priv->extsel);
+ ainfo("Initializing timers extsel = 0x%08lx\n", priv->extsel);
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