Changes from Review of last PR adding Tiva PWM driver

arch/arm/src/tiva/tiva_pwm.c

@@ -43,7 +43,10 @@
  * Included Files
  ************************************************************************************/
 
+#include <nuttx/config.h>
+
 #include <stdio.h>
+#include <errno.h>
 #include <debug.h>
 
 #include <nuttx/drivers/pwm.h>
@@ -58,28 +61,6 @@
 #include "chip/tiva_pinmap.h"
 #include "chip/tm4c_memorymap.h"
 
-/************************************************************************************
-* Pre-processor Definitions
-************************************************************************************/
-
-#define pwmerr(fmt, args...) printf("%s(%d): " fmt, __FUNCTION__, __LINE__, ##args);
-
-#ifndef CONFIG_DEBUG
-#  undef CONFIG_DEBUG_PWM
-#endif
-
-#ifdef CONFIG_DEBUG_PWM
-#  define pwmdbg              dbg
-#  ifdef CONFIG_DEBUG_VERBOSE
-#    define pwmvdbg           vdbg
-#  else
-#    define pwmvdbg(x...)
-#  endif
-#else
-#  define pwmdbg(x...)
-#  define pwmvdbg(x...)
-#endif
-
 /************************************************************************************
  * Private Types
  ************************************************************************************/
@@ -106,13 +87,14 @@ struct tiva_pwm_chan_s
   uint8_t channel_id;
 };
 
-
 /************************************************************************************
  * Private Function Prototypes
  ************************************************************************************/
 
-static inline void tiva_pwm_putreg(struct tiva_pwm_chan_s *chan, unsigned int offset, uint32_t regval);
+static inline void tiva_pwm_putreg(struct tiva_pwm_chan_s *chan,
-static inline uint32_t tiva_pwm_getreg(struct tiva_pwm_chan_s *chan, unsigned int offset);
+                                   unsigned int offset, uint32_t regval);
+static inline uint32_t tiva_pwm_getreg(struct tiva_pwm_chan_s *chan,
+                                       unsigned int offset);
 
 static int tiva_pwm_setup(FAR struct pwm_lowerhalf_s *dev);
 static int tiva_pwm_shutdown(FAR struct pwm_lowerhalf_s *dev);
@@ -126,8 +108,8 @@ static int tiva_pwm_ioctl(FAR struct pwm_lowerhalf_s *dev,
  * Private Data
  ************************************************************************************/
 
-uint32_t g_pwm_freq = 15000000;
+static uint32_t g_pwm_freq = 15000000;
-uint32_t g_pwm_counter = (1 << 16);
+static uint32_t g_pwm_counter = (1 << 16);
 
 static const struct pwm_ops_s g_pwm_ops =
 {
@@ -246,7 +228,8 @@ static struct tiva_pwm_chan_s g_pwm_chan7 =
  *
  ************************************************************************************/
 
-static inline uint32_t tiva_pwm_getreg(struct tiva_pwm_chan_s *chan, unsigned int offset)
+static inline uint32_t tiva_pwm_getreg(struct tiva_pwm_chan_s *chan,
+                                       unsigned int offset)
 {
     uintptr_t regaddr = chan->generator_base + offset;
     return getreg32(regaddr);
@@ -260,7 +243,8 @@ static inline uint32_t tiva_pwm_getreg(struct tiva_pwm_chan_s *chan, unsigned in
  *
  ************************************************************************************/
 
-static inline void tiva_pwm_putreg(struct tiva_pwm_chan_s *chan, unsigned int offset, uint32_t regval)
+static inline void tiva_pwm_putreg(struct tiva_pwm_chan_s *chan,
+                                   unsigned int offset, uint32_t regval)
 {
     uintptr_t regaddr = chan->generator_base + offset;
     putreg32(regval, regaddr);
@@ -286,14 +270,18 @@ static inline void tiva_pwm_putreg(struct tiva_pwm_chan_s *chan, unsigned int of
 static int tiva_pwm_setup(FAR struct pwm_lowerhalf_s *dev)
 {
   FAR struct tiva_pwm_chan_s *chan = (FAR struct tiva_pwm_chan_s *)dev;
-  pwmdbg("setup PWM for channel %d\n", chan->channel_id);
+  pwminfo("setup PWM for channel %d\n", chan->channel_id);
+
+  /* Enable GPIO port, GPIO pin type and GPIO alternate function (refer to
+   * TM4C1294NC 23.4.2-4)
+   */
 
-  /* Enable GPIO port, GPIO pin type and GPIO alternate function (refer to TM4C1294NC 23.4.2-4) */
   int ret = tiva_configgpio(g_pwm_pinset[chan->channel_id]);
   if (ret < 0)
     {
-      pwmerr("tiva_configgpio failed (%x)\n", g_pwm_pinset[chan->channel_id]);
+      pwmerr("ERROR: tiva_configgpio failed (%x)\n",
-      return -1;
+             g_pwm_pinset[chan->channel_id]);
+      return ret;
     }
 
   return OK;
@@ -318,12 +306,14 @@ static int tiva_pwm_setup(FAR struct pwm_lowerhalf_s *dev)
 static int tiva_pwm_shutdown(FAR struct pwm_lowerhalf_s *dev)
 {
   FAR struct tiva_pwm_chan_s *chan = (FAR struct tiva_pwm_chan_s *)dev;
-  pwmdbg("shutdown PWM for channel %d\n", chan->channel_id);
+  pwminfo("shutdown PWM for channel %d\n", chan->channel_id);
 
   /* Remove unused-variable warning */
+
   (void)chan;
 
   /* Ensure the PWM channel has been stopped */
+
   tiva_pwm_stop(dev);
 
   return OK;
@@ -344,15 +334,17 @@ static int tiva_pwm_shutdown(FAR struct pwm_lowerhalf_s *dev)
  *
  ****************************************************************************/
 
-static int tiva_pwm_start(FAR struct pwm_lowerhalf_s *dev, FAR const struct pwm_info_s *info)
+static int tiva_pwm_start(FAR struct pwm_lowerhalf_s *dev,
+                          FAR const struct pwm_info_s *info)
 {
   FAR struct tiva_pwm_chan_s *chan = (FAR struct tiva_pwm_chan_s *)dev;
-  pwmdbg("start PWM for channel %d\n", chan->channel_id);
+  pwminfo("start PWM for channel %d\n", chan->channel_id);
 
   uint16_t duty = info->duty;
   uint32_t frequency = info->frequency;
 
   /* Configure PWM countdown mode (refer to TM4C1294NC 23.4.6) */
+
   tiva_pwm_putreg(chan, TIVA_PWMn_CTL_OFFSET, 0);
   if (chan->channel_id % 2 == 0)
     {
@@ -366,20 +358,27 @@ static int tiva_pwm_start(FAR struct pwm_lowerhalf_s *dev, FAR const struct pwm_
     }
 
   /* Set the PWM period (refer to TM4C1294NC 23.4.7) */
+
   uint32_t pwm_min_freq = (uint32_t)(g_pwm_freq / g_pwm_counter) + 1;
   uint32_t pwm_max_freq = g_pwm_freq;
   uint32_t load = (uint32_t)(g_pwm_freq / frequency);
-  pwmdbg("channel %d: load = %u (%08x)\n", chan->channel_id, load, load);
+
+  pwminfo("channel %d: load = %u (%08x)\n", chan->channel_id, load, load);
+
   if (load >= g_pwm_counter || load < 1)
     {
-      pwmerr("frequency should be in [%d, %d] Hz\n", pwm_min_freq, pwm_max_freq);
+      pwmerr("ERROR: frequency should be in [%d, %d] Hz\n",
-      return -1;
+              pwm_min_freq, pwm_max_freq);
+      return -ERANGE;
     }
+
   tiva_pwm_putreg(chan, TIVA_PWMn_LOAD_OFFSET, load - 1);
 
   /* Configure PWM duty (refer to TM4C1294NC 23.4.8-9) */
+
   uint32_t comp = (uint32_t)((1 - (float)duty / g_pwm_counter) * load);
-  pwmdbg("channel %d: comp = %u (%08x)\n", chan->channel_id, comp, comp);
+  pwminfo("channel %d: comp = %u (%08x)\n", chan->channel_id, comp, comp);
+
   if (chan->channel_id % 2 == 0)
     {
       tiva_pwm_putreg(chan, TIVA_PWMn_CMPA_OFFSET, comp - 1);
@@ -390,11 +389,12 @@ static int tiva_pwm_start(FAR struct pwm_lowerhalf_s *dev, FAR const struct pwm_
     }
 
   /* Enable the PWM generator (refer to TM4C1294NC 23.4.10) */
+
   tiva_pwm_putreg(chan, TIVA_PWMn_CTL_OFFSET, CTL_ENABLE << TIVA_PWMn_CTL_ENABLE);
 
   /* Enable PWM channel (refer to TM4C1294NC 23.4.11) */
-  putreg32((1 << chan->channel_id), chan->controller_base + TIVA_PWM_ENABLE_OFFSET);
 
+  putreg32((1 << chan->channel_id), chan->controller_base + TIVA_PWM_ENABLE_OFFSET);
   return OK;
 }
 
@@ -420,9 +420,10 @@ static int tiva_pwm_start(FAR struct pwm_lowerhalf_s *dev, FAR const struct pwm_
 static int tiva_pwm_stop(FAR struct pwm_lowerhalf_s *dev)
 {
   FAR struct tiva_pwm_chan_s *chan = (FAR struct tiva_pwm_chan_s *)dev;
-  pwmdbg("stop PWM for channel %d\n", chan->channel_id);
+  pwminfo("stop PWM for channel %d\n", chan->channel_id);
 
   /* Disable PWM channel */
+
   uint32_t value = getreg32(chan->controller_base + TIVA_PWM_ENABLE_OFFSET);
   value &= ~(1 << chan->channel_id);
   putreg32(value, chan->controller_base + TIVA_PWM_ENABLE_OFFSET);
@@ -446,17 +447,19 @@ static int tiva_pwm_stop(FAR struct pwm_lowerhalf_s *dev)
  *
  ****************************************************************************/
 
-static int tiva_pwm_ioctl(FAR struct pwm_lowerhalf_s *dev, int cmd, unsigned long arg)
+static int tiva_pwm_ioctl(FAR struct pwm_lowerhalf_s *dev, int cmd,
+                          unsigned long arg)
 {
   FAR struct tiva_pwm_chan_s *chan = (FAR struct tiva_pwm_chan_s *)dev;
-  pwmdbg("ioctl PWM for channel %d\n", chan->channel_id);
+  pwminfo("ioctl PWM for channel %d\n", chan->channel_id);
 
   /* Remove unused-variable warning */
+
   (void)chan;
 
   /* There are no platform-specific ioctl commands */
 
-  return -1;
+  return -ENOTTY;
 }
 
 /****************************************************************************
@@ -538,39 +541,28 @@ FAR struct pwm_lowerhalf_s *tiva_pwm_initialize(int channel)
       return NULL;
     }
 
-  pwmvdbg("channel %d: channel_id=%d, ", channel, chan->channel_id);
+  pwminfo("channel %d: channel_id=%d, ", channel, chan->channel_id);
-  pwmvdbg("controller_id=%d, controller_base=%08x, ", chan->controller_id, chan->controller_base);
+  pwminfo("controller_id=%d, controller_base=%08x, ",
-  pwmvdbg("generator_id=%d, generator_base=%08x\n", chan->generator_id, chan->generator_base);
+          chan->controller_id, chan->controller_base);
+  pwminfo("generator_id=%d, generator_base=%08x\n",
+          chan->generator_id, chan->generator_base);
 
   /* Enable PWM controller (refer to TM4C1294NC 23.4.1) */
+
   assert(chan->controller_id == 0);
   tiva_pwm_enablepwr(chan->controller_id);
   tiva_pwm_enableclk(chan->controller_id);
 
-  /* Configure PWM Clock Configuration (refer to TM4C1294NC 23.4.5) */
+  /* Configure PWM Clock Configuration (refer to TM4C1294NC 23.4.5)
-  /* on TM4C1294NC, configure the PWM clock source as 15MHz (the system clock 120MHz divided by 8) */
+   *
-  /* TODO: need an algorithm to choose the best divider and load value combo */
+   * On TM4C1294NC, configure the PWM clock source as 15MHz (the system
+   * clock 120MHz divided by 8)
+   *
+   * TODO: need an algorithm to choose the best divider and load value combo.
+   */
+
   putreg32(CC_USEPWM << TIVA_PWM_CC_USEPWM | CC_PWMDIV_8 << TIVA_PWM_CC_PWMDIV,
            chan->controller_base + TIVA_PWM_CC);
 
   return (FAR struct pwm_lowerhalf_s *)chan;
 }
-
-/****************************************************************************
-* Name: board_pwm_setup
-*
-* Description:
-*   No implementation for now, it's called by PWM tool.
-*
-* Input Parameters:
-*   None.
-*
-* Returned Value:
-*   Zero on Success.
-*
-****************************************************************************/
-
-int board_pwm_setup(void)
-{
-    return OK;
-}

arch/arm/src/tiva/tiva_pwm.h

@@ -47,8 +47,6 @@
  ****************************************************************************/
 
 FAR struct pwm_lowerhalf_s *tiva_pwm_initialize(int channel);
-
 void tm4c_pwm_register(int channel);
-int board_pwm_setup(void);
 
 #endif /* __ARCH_ARM_SRC_TIVA_TIVA_PWM_H */

arch/sim/src/up_ioexpander.c

@@ -701,7 +701,7 @@ static ioe_pinset_t sim_int_update(FAR struct sim_dev_s *priv)
         }
       else /* if (SIM_LEVEL_SENSITIVE(priv, pin)) */
         {
-          /* Level triggered. Set intstat if imatch in level type. */
+          /* Level triggered. Set intstat if match in level type. */
 
           if ((pinval  && SIM_LEVEL_HIGH(priv, pin)) ||
               (!pinval && SIM_LEVEL_LOW(priv, pin)))

configs/tm4c1294-launchpad/src/tm4c_bringup.c

@@ -37,13 +37,13 @@
  * Included Files
  ****************************************************************************/
 
-#include <stdio.h>
-#include <stdint.h>
-
 #include <nuttx/config.h>
 
+#include <stdio.h>
+#include <stdint.h>
 #include <debug.h>
 
+#include <nuttx/board.h>
 #include <nuttx/i2c/i2c_master.h>
 #include <arch/board/board.h>
 
@@ -54,7 +54,6 @@
 #define PWM_PATH_FMT        "/dev/pwm%d"
 #define PWM_PATH_FMTLEN     (10)
 
-
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
@@ -171,7 +170,7 @@ void tm4c_pwm_register(int channel)
   dev = tiva_pwm_initialize(channel);
   if (dev == NULL)
     {
-      dbg("ERROR: Failed to get PWM%d interface\n", channel);
+      pwmerr("ERROR: Failed to get PWM%d interface\n", channel);
     }
   else
     {
@@ -179,7 +178,8 @@ void tm4c_pwm_register(int channel)
       ret = pwm_register(pwm_path, dev);
       if (ret < 0)
         {
-          dbg("ERROR: Failed to register PWM%d driver: %d\n", channel, ret);
+          pwmerr("ERROR: Failed to register PWM%d driver: %d\n",
+                 channel, ret);
         }
     }
 }
@@ -193,7 +193,6 @@ void tm4c_pwm_register(int channel)
  ****************************************************************************/
 
 #ifdef HAVE_PWM
-
 static void tm4c_pwm(void)
 {
 #ifdef CONFIG_TIVA_PWM0_CHAN0
@@ -221,8 +220,7 @@ static void tm4c_pwm(void)
   tm4c_pwm_register(7);
 #endif
 }
-
+#endif # HAVE_PWM
-#endif
 
 /****************************************************************************
  * Public Functions
@@ -246,9 +244,11 @@ int tm4c_bringup(void)
 
   tm4c_i2ctool();
 
+#ifdef HAVE_PWM
   /* Register PWM drivers */
 
   tm4c_pwm();
+#endif
 
 #ifdef HAVE_TIMER
   /* Initialize the timer driver */
@@ -262,3 +262,26 @@ int tm4c_bringup(void)
 
   return OK;
 }
+
+/****************************************************************************
+ * Name: board_pwm_setup
+ *
+ * Description:
+ *   No implementation for now, it's called by PWM tool via boardctl.h.
+ *   See include/nuttx/board.h
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on Success.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_BOARDCTL_PWMTEST
+int board_pwm_setup(void)
+{
+  return OK;
+}
+#endif
+