From a4eb5a5399068bc3e2a7548de26d1e54b203f828 Mon Sep 17 00:00:00 2001
From: 100ask-Alen <106950620+100ask-Alen@users.noreply.github.com>
Date: Sat, 29 Oct 2022 06:38:17 +0800
Subject: [PATCH] =?UTF-8?q?=E6=8F=90=E4=BA=A4stm32=20bsp=20drv=5Fpwm.c?=
 =?UTF-8?q?=E9=87=8C=E9=9D=A2=E7=9A=84=E4=B8=80=E4=B8=AA=E5=B0=8F=E5=BB=BA?=
 =?UTF-8?q?=E8=AE=AE=20(#6517)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

 在使用stm32的pwm驱动时，发现其初始化channel的时候是根据device->channel这个值去初始化的，device->channel这个值是在pwm_configh.h里面定义设置的，又根据rtt的pwm例程，和常规习惯，应该大多数人会更喜欢设置channel=1/2/3/4，而不是在pwm_configh.h中将channel设置为TIM_CHANNEL_1/2/3/4，因此建议drv_pwm.c做出PR中的修改。

在实际开发中，因pwm_configh.h的示例均是.channel=0，又没有做出解释说明要让.channel=TIM_CHANNEL_1/2/3/4的值，容易误操作导致没有成功输出PWM，这是我在实际开发中遇到的现实问题，因而提出此建议。
---
 .../HAL_Drivers/config/f0/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/f1/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/f2/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/f3/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/f4/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/f7/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/g0/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/g4/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/h7/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/l1/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/l4/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/l5/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/mp1/pwm_config.h       |  6 ++
 .../HAL_Drivers/config/u5/pwm_config.h        |  6 ++
 .../HAL_Drivers/config/wb/pwm_config.h        |  6 ++
 bsp/stm32/libraries/HAL_Drivers/drv_pwm.c     | 82 +++++++++----------
 16 files changed, 130 insertions(+), 42 deletions(-)

diff --git a/bsp/stm32/libraries/HAL_Drivers/config/f0/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/f0/pwm_config.h
index 052e4ccec5..84a5b9b81b 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/f0/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/f0/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/f1/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/f1/pwm_config.h
index f5b32eef73..7518f26354 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/f1/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/f1/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/f2/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/f2/pwm_config.h
index b8ef2d37f7..5849b6274b 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/f2/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/f2/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/f3/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/f3/pwm_config.h
index eafd4644c9..c20baf1d2c 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/f3/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/f3/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/f4/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/f4/pwm_config.h
index 05563ce434..d1b870574a 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/f4/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/f4/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/f7/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/f7/pwm_config.h
index b8ef2d37f7..5849b6274b 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/f7/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/f7/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/g0/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/g0/pwm_config.h
index bc9c375b7a..9fc12acf91 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/g0/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/g0/pwm_config.h
@@ -18,6 +18,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/g4/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/g4/pwm_config.h
index ca382e4742..436435ac85 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/g4/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/g4/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/h7/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/h7/pwm_config.h
index 3306a20b25..370127883d 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/h7/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/h7/pwm_config.h
@@ -18,6 +18,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/l1/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/l1/pwm_config.h
index b8ef2d37f7..5849b6274b 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/l1/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/l1/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/l4/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/l4/pwm_config.h
index c4d4bcf202..aaa7b3fc02 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/l4/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/l4/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/l5/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/l5/pwm_config.h
index c4d4bcf202..aaa7b3fc02 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/l5/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/l5/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/mp1/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/mp1/pwm_config.h
index b8ef2d37f7..5849b6274b 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/mp1/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/mp1/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM2
 #ifndef PWM2_CONFIG
 #define PWM2_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/u5/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/u5/pwm_config.h
index c4d4bcf202..aaa7b3fc02 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/u5/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/u5/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/config/wb/pwm_config.h b/bsp/stm32/libraries/HAL_Drivers/config/wb/pwm_config.h
index 1d129e6792..c61d283860 100644
--- a/bsp/stm32/libraries/HAL_Drivers/config/wb/pwm_config.h
+++ b/bsp/stm32/libraries/HAL_Drivers/config/wb/pwm_config.h
@@ -17,6 +17,12 @@
 extern "C" {
 #endif
 
+/*
+ * .tim_handle.Instance     = TIM1/2/3...,
+ * .name					= "your pwm device name",
+ * .channel					= 1/2/3/4
+ */
+
 #ifdef BSP_USING_PWM1
 #ifndef PWM1_CONFIG
 #define PWM1_CONFIG                             \
diff --git a/bsp/stm32/libraries/HAL_Drivers/drv_pwm.c b/bsp/stm32/libraries/HAL_Drivers/drv_pwm.c
index abcffa7288..20ea626832 100644
--- a/bsp/stm32/libraries/HAL_Drivers/drv_pwm.c
+++ b/bsp/stm32/libraries/HAL_Drivers/drv_pwm.c
@@ -234,7 +234,13 @@ static struct rt_pwm_ops drv_ops =
 static rt_err_t drv_pwm_enable(TIM_HandleTypeDef *htim, struct rt_pwm_configuration *configuration, rt_bool_t enable)
 {
     /* Converts the channel number to the channel number of Hal library */
-    rt_uint32_t channel = 0x04 * (configuration->channel - 1);
+    rt_uint32_t channel = configuration->channel;
+    if(channel < 1 || channel > 4)
+    {
+        LOG_E("pwm channel %d is invalid, failed to enable", configuration->channel);
+        return -RT_ERROR;
+    }
+    channel = 0x04 * (configuration->channel - 1);
 
     if (!configuration->complementary)
     {
@@ -265,7 +271,13 @@ static rt_err_t drv_pwm_enable(TIM_HandleTypeDef *htim, struct rt_pwm_configurat
 static rt_err_t drv_pwm_get(TIM_HandleTypeDef *htim, struct rt_pwm_configuration *configuration)
 {
     /* Converts the channel number to the channel number of Hal library */
-    rt_uint32_t channel = 0x04 * (configuration->channel - 1);
+    rt_uint32_t channel = configuration->channel;
+    if(channel < 1 || channel > 4)
+    {
+        LOG_E("pwm channel %d is invalid, failed to get", configuration->channel);
+        return -RT_ERROR;
+    }
+    channel = 0x04 * (configuration->channel - 1);
     rt_uint64_t tim_clock;
 
     tim_clock = tim_clock_get(htim);
@@ -291,7 +303,13 @@ static rt_err_t drv_pwm_set(TIM_HandleTypeDef *htim, struct rt_pwm_configuration
     rt_uint32_t period, pulse;
     rt_uint64_t tim_clock, psc;
     /* Converts the channel number to the channel number of Hal library */
-    rt_uint32_t channel = 0x04 * (configuration->channel - 1);
+    rt_uint32_t channel = configuration->channel;
+    if(channel < 1 || channel > 4)
+    {
+        LOG_E("pwm channel %d is invalid, failed to set", configuration->channel);
+        return -RT_ERROR;
+    }
+    channel = 0x04 * (configuration->channel - 1);
 
     tim_clock = tim_clock_get(htim);
     /* Convert nanosecond to frequency and duty cycle. 1s = 1 * 1000 * 1000 * 1000 ns */
@@ -354,7 +372,13 @@ static rt_err_t drv_pwm_set_pulse(TIM_HandleTypeDef *htim, struct rt_pwm_configu
     rt_uint32_t period, pulse;
     rt_uint64_t tim_clock;
     /* Converts the channel number to the channel number of Hal library */
-    rt_uint32_t channel = 0x04 * (configuration->channel - 1);
+    rt_uint32_t channel = configuration->channel;
+    if(channel < 1 || channel > 4)
+    {
+        LOG_E("pwm channel %d is invalid, failed to set pulse", configuration->channel);
+        return -RT_ERROR;
+    }
+    channel = 0x04 * (configuration->channel - 1);
 
     tim_clock = tim_clock_get(htim);
     /* Convert nanosecond to frequency and duty cycle. 1s = 1 * 1000 * 1000 * 1000 ns */
@@ -408,6 +432,14 @@ static rt_err_t stm32_hw_pwm_init(struct stm32_pwm *device)
     TIM_MasterConfigTypeDef master_config = {0};
     TIM_ClockConfigTypeDef clock_config = {0};
 
+    rt_uint32_t channel = device->channel;
+    if(channel < 1 || channel > 4)
+    {
+        LOG_E("%s pwm channel %d is invalid, failed to init", device->name, device->channel);
+        return -RT_ERROR;
+    }
+    channel = 0x04 * (device->channel - 1);
+
     RT_ASSERT(device != RT_NULL);
 
     tim = (TIM_HandleTypeDef *)&device->tim_handle;
@@ -458,45 +490,11 @@ static rt_err_t stm32_hw_pwm_init(struct stm32_pwm *device)
     oc_config.OCNIdleState = TIM_OCNIDLESTATE_RESET;
     oc_config.OCIdleState  = TIM_OCIDLESTATE_RESET;
 
-    /* config pwm channel */
-    if (device->channel & 0x01)
-    {
-        if (HAL_TIM_PWM_ConfigChannel(tim, &oc_config, TIM_CHANNEL_1) != HAL_OK)
-        {
-            LOG_E("%s channel1 config failed", device->name);
-            result = -RT_ERROR;
-            goto __exit;
-        }
-    }
-
-    if (device->channel & 0x02)
-    {
-        if (HAL_TIM_PWM_ConfigChannel(tim, &oc_config, TIM_CHANNEL_2) != HAL_OK)
-        {
-            LOG_E("%s channel2 config failed", device->name);
-            result = -RT_ERROR;
-            goto __exit;
-        }
-    }
-
-    if (device->channel & 0x04)
+    if (HAL_TIM_PWM_ConfigChannel(tim, &oc_config, channel) != HAL_OK)
     {
-        if (HAL_TIM_PWM_ConfigChannel(tim, &oc_config, TIM_CHANNEL_3) != HAL_OK)
-        {
-            LOG_E("%s channel3 config failed", device->name);
-            result = -RT_ERROR;
-            goto __exit;
-        }
-    }
-
-    if (device->channel & 0x08)
-    {
-        if (HAL_TIM_PWM_ConfigChannel(tim, &oc_config, TIM_CHANNEL_4) != HAL_OK)
-        {
-            LOG_E("%s channel4 config failed", device->name);
-            result = -RT_ERROR;
-            goto __exit;
-        }
+        LOG_E("%s %d config failed", device->name, device->channel);
+        result = -RT_ERROR;
+        goto __exit;
     }
 
     /* pwm pin configuration */
-- 
GitLab