drivers: pwm: nrf: Removed Kconfig prescaler option. Rewrite HW driver

1. Kconfig options clock_prescaler removed.
2. Rewritten pwm_nrfx.c nRF HW PWM driver.

Signed-off-by: Gaute Gamnes <gaute.gamnes@nordicsemi.no>

drivers/pwm/Kconfig.nrfx

@@ -20,12 +20,6 @@ config PWM_0_NAME
 	  Specify the device name for the Nordic Semiconductor nRF52 series HW
 	  PWM module 0.
 
-config PWM_0_NRF_CLOCK_PRESCALER
-	int "Clock prescaler"
-	range 0 7
-	default 0
-	help
-	  Clock = 16MHz / (2^prescaler).
 endif # PWM_0
 
 if PWM_1
@@ -35,13 +29,6 @@ config PWM_1_NAME
 	help
 	  Specify the device name for the Nordic Semiconductor nRF52 series HW
 	  PWM module 1.
-
-config PWM_1_NRF_CLOCK_PRESCALER
-	int "Clock prescaler"
-	range 0 7
-	default 0
-	help
-	  Clock = 16MHz / (2^prescaler).
 endif # PWM_1
 
 
@@ -52,13 +39,6 @@ config PWM_2_NAME
 	help
 	  Specify the device name for the Nordic Semiconductor nRF52 series HW
 	  PWM module 2.
-
-config PWM_2_NRF_CLOCK_PRESCALER
-	int "Clock prescaler"
-	range 0 7
-	default 0
-	help
-	  Clock = 16MHz / (2^prescaler).
 endif # PWM_2
 
 if PWM_3
@@ -68,13 +48,6 @@ config PWM_3_NAME
 	help
 	  Specify the device name for the Nordic Semiconductor nRF52 series HW
 	  PWM module 3.
-
-config PWM_3_NRF_CLOCK_PRESCALER
-	int "Clock prescaler"
-	range 0 7
-	default 0
-	help
-	  Clock = 16MHz / (2^prescaler).
 endif # PWM_3
 
 endif # PWM_NRFX

drivers/pwm/pwm_nrfx.c

@@ -5,58 +5,223 @@
  */
 #include <nrfx_pwm.h>
 #include <pwm.h>
+#include <hal/nrf_gpio.h>
+#include <stdbool.h>
 
 #define LOG_LEVEL CONFIG_PWM_LOG_LEVEL
 #include <logging/log.h>
 LOG_MODULE_REGISTER(pwm_nrfx);
 
-#define PWM_NRFX_CH_VALUE_NORMAL        (1UL << 15)
+#define PWM_NRFX_CH_POLARITY_MASK       BIT(15)
+#define PWM_NRFX_CH_PULSE_CYCLES_MASK   BIT_MASK(15)
+#define PWM_NRFX_CH_VALUE_NORMAL        PWM_NRFX_CH_POLARITY_MASK
 #define PWM_NRFX_CH_VALUE_INVERTED      (0)
+#define PWM_NRFX_CH_PIN_MASK            ~NRFX_PWM_PIN_INVERTED
 
 struct pwm_nrfx_config {
 	nrfx_pwm_t pwm;
-	nrfx_pwm_config_t config;
+	nrfx_pwm_config_t initial_config;
 	nrf_pwm_sequence_t seq;
 };
 
 struct pwm_nrfx_data {
+	u32_t period_cycles;
 	u16_t current[NRF_PWM_CHANNEL_COUNT];
-	u16_t top_value;
+	u16_t countertop;
+	u8_t  prescaler;
 };
 
+static u32_t pwm_period_check_and_set(const struct pwm_nrfx_config *config,
+				      struct pwm_nrfx_data *data,
+				      u32_t pwm,
+				      u32_t period_cycles)
+{
+	NRF_PWM_Type *pwm_instance = config->pwm.p_registers;
+
+	if (!nrfx_pwm_is_stopped(&config->pwm)) {
+		/* Succeed if requested period matches already used period */
+		if (period_cycles == data->period_cycles) {
+			return 0;
+		}
+
+		/* Fail if requested period != already running period */
+		LOG_ERR("Fail:requested period cycles:%d, != used %d\n",
+			period_cycles, data->period_cycles);
+		return -EINVAL;
+	}
+
+	/* Check if period_cycles is above COUNTERTOP MAX value, if so, we
+	 * have to see if we can change frequency to something that will fit
+	 */
+	if (period_cycles > PWM_COUNTERTOP_COUNTERTOP_Msk) {
+		/* See if there is a prescaler that will make it work: */
+		bool matching_prescaler_found = false;
+
+		/* Go through all available prescaler values on device.
+		 * nRF52832 has 0-7 (Div1 - Div128)
+		 */
+		for (u8_t prescaler = 0;
+		     prescaler <= PWM_PRESCALER_PRESCALER_Msk;
+		     prescaler++) {
+			u32_t new_countertop = period_cycles >> prescaler;
+
+			/* If we find value that fits, set it, continue */
+			if (new_countertop <= PWM_COUNTERTOP_COUNTERTOP_Msk) {
+				data->prescaler = prescaler;
+				data->countertop = new_countertop;
+				data->period_cycles = period_cycles;
+				matching_prescaler_found = true;
+				break;
+			}
+		}
+
+		/* Check if able to find matching prescaler and countertop */
+		if (matching_prescaler_found == false) {
+			LOG_ERR("Prescaler for period_cycles %d not found.\n",
+				period_cycles);
+			return -EINVAL;
+		}
+	} else {
+		/* If period_cycles fit with standard prescaler,
+		 * set it directly
+		 */
+		data->prescaler = 1;
+		data->countertop = period_cycles;
+		data->period_cycles = period_cycles;
+	}
+
+	/* Write new PRESCALER and COUNTERTOP to PWM instance */
+	nrf_pwm_configure(pwm_instance,
+			  data->prescaler,
+			  config->initial_config.count_mode,
+			  data->countertop);
+
+	return 0;
+}
+
+static u8_t pwm_channel_map(const uint8_t *output_pins, u32_t pwm)
+{
+	u8_t i;
+
+	/* Find pin, return channel number */
+	for (i = 0; i < NRF_PWM_CHANNEL_COUNT; i++) {
+		if (output_pins[i] != NRFX_PWM_PIN_NOT_USED
+		    && (pwm == (output_pins[i] & PWM_NRFX_CH_PIN_MASK))) {
+			return i;
+		}
+	}
+
+	/* Return NRF_PWM_CHANNEL_COUNT to show that PWM pin was not found. */
+	return NRF_PWM_CHANNEL_COUNT;
+}
+
+static bool any_channel_active(const struct pwm_nrfx_data *data)
+{
+	u8_t channel;
+
+	for (channel = 0; channel < NRF_PWM_CHANNEL_COUNT; channel++) {
+		u16_t channel_pulse_cycle =
+			data->current[channel]
+			& PWM_NRFX_CH_PULSE_CYCLES_MASK;
+
+		if (channel_pulse_cycle > 0
+		    && channel_pulse_cycle < data->countertop) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 static int pwm_nrfx_pin_set(struct device *dev, u32_t pwm,
 			    u32_t period_cycles, u32_t pulse_cycles)
 {
-	const struct pwm_nrfx_config *pconfig = dev->config->config_info;
+	/* We assume here that period_cycles will always be 16MHz
-	struct pwm_nrfx_data *pdata = dev->driver_data;
+	 * peripheral clock. Since pwm_nrfx_get_cycles_per_sec() function might
-
+	 * be removed, see ISSUE #6958.
-	if (pwm >= NRF_PWM_CHANNEL_COUNT) {
+	 * TODO: Remove this comment when issue has been resolved.
-		return -EINVAL;
-	}
-
-	if (period_cycles != pdata->top_value) {
-		if (period_cycles > PWM_COUNTERTOP_COUNTERTOP_Msk) {
-			return -EINVAL;
-		}
-
-		pdata->top_value = period_cycles;
-		nrf_pwm_configure(pconfig->pwm.p_registers,
-				  pconfig->config.base_clock,
-				  pconfig->config.count_mode,
-				  pdata->top_value);
-	}
-
-	if (pulse_cycles > pdata->top_value) {
-		return -EINVAL;
-	}
-
-	/* Modify only the COMPARE bits while preserving the POLARITY
-	 * bit that controls the inversion of the channel.
-	 * For more details see product specification.
 	 */
-	pdata->current[pwm] = ((pdata->current[pwm]
+	const struct pwm_nrfx_config *config = dev->config->config_info;
-				& ~PWM_COUNTERTOP_COUNTERTOP_Msk)
+	struct pwm_nrfx_data *data = dev->driver_data;
-				| pulse_cycles);
+	u8_t channel;
+	u32_t ret;
+
+	/* Check if PWM pin is one of the predefiend DTS config pins.
+	 * Return its array index (channel number),
+	 * or NRF_PWM_CHANNEL_COUNT if not initialized through DTS.
+	 */
+	channel = pwm_channel_map(config->initial_config.output_pins, pwm);
+	if (channel == NRF_PWM_CHANNEL_COUNT) {
+		LOG_ERR("PWM pin %d not enabled through DTS configuration.",
+			pwm);
+		return -EINVAL;
+	}
+
+	/* Check if period_cycle is either matching currently used, or
+	 * possible to use with our prescaler options.
+	 */
+	ret = pwm_period_check_and_set(config, data, pwm, period_cycles);
+	if (ret) {
+		LOG_ERR("Incompatible period %d", period_cycles);
+		return ret;
+	}
+
+	/* Check if pulse is bigger than period, fail if so */
+	if (pulse_cycles > period_cycles) {
+		LOG_ERR("Invalid pulse_cycles %d, > period_cycles %d.",
+			pulse_cycles, period_cycles);
+		return -EINVAL;
+	}
+
+	/* Store new pulse value bit[14:0], and polarity bit[15] for channel. */
+	data->current[channel] = (
+		(data->current[channel] & PWM_NRFX_CH_POLARITY_MASK)
+		| (pulse_cycles >> data->prescaler));
+
+	/* If Channel is off/fully on (duty 0% or 100%), also set GPIO register
+	 * since this will the setting if we in the future disable the
+	 * peripheral when no channels are active.
+	 */
+	if (pulse_cycles == 0 || pulse_cycles == period_cycles) {
+		/* If pulse 0% and pin not inverted, set LOW.
+		 * If pulse 100% and pin inverted, set LOW.
+		 * If pulse 0% and pin inverted, set HIGH.
+		 * If pulse 100% and pin not inverted, set HIGH.
+		 */
+		bool channel_inverted_state =
+			config->initial_config.output_pins[channel]
+			& NRFX_PWM_PIN_INVERTED;
+
+		bool pulse_0_and_not_inverted =
+			(pulse_cycles == 0)
+			&& !channel_inverted_state;
+		bool pulse_100_and_inverted =
+			(pulse_cycles == period_cycles)
+			&& channel_inverted_state;
+
+		if (pulse_0_and_not_inverted || pulse_100_and_inverted) {
+			nrf_gpio_pin_clear(pwm);
+		} else {
+			nrf_gpio_pin_set(pwm);
+		}
+	}
+
+	/* Check if all channels are off (duty 0% or 100%) */
+	if (!any_channel_active(data)) {
+		nrfx_pwm_stop(&config->pwm, false);
+	} else {
+		/* A PWM Channel is active: Start sequence. */
+
+		/* Since we are playing the sequence in a loop, the
+		 * sequence only has to be started when its not already
+		 * playing. The new channel values will be used
+		 * immediately when they are written into the seq array.
+		 */
+		nrfx_pwm_simple_playback(&config->pwm,
+			 &config->seq,
+			 1,
+			 NRFX_PWM_FLAG_LOOP);
+	}
 
 	return 0;
 }
@@ -64,34 +229,12 @@ static int pwm_nrfx_pin_set(struct device *dev, u32_t pwm,
 static int pwm_nrfx_get_cycles_per_sec(struct device *dev, u32_t pwm,
 				       u64_t *cycles)
 {
-	const struct pwm_nrfx_config *pconfig = dev->config->config_info;
+	/* TODO: Since this function might be removed, we will always return
+	 * 16MHz from this function and handle the conversion with prescaler,
+	 * etc, in the pin set function. See issue #6958.
+	 */
+	*cycles = 16ul * 1000ul * 1000ul;
 
-	switch (pconfig->config.base_clock) {
-	case NRF_PWM_CLK_16MHz:
-		*cycles = 16ul * 1000ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_8MHz:
-		*cycles = 8ul * 1000ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_4MHz:
-		*cycles = 4ul * 1000ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_2MHz:
-		*cycles = 2ul * 1000ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_1MHz:
-		*cycles = 1ul * 1000ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_500kHz:
-		*cycles = 500ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_250kHz:
-		*cycles = 250ul * 1000ul;
-		break;
-	case NRF_PWM_CLK_125kHz:
-		*cycles = 125ul * 1000ul;
-		break;
-	}
 	return 0;
 }
 
@@ -102,22 +245,16 @@ static const struct pwm_driver_api pwm_nrfx_drv_api_funcs = {
 
 static int pwm_nrfx_init(struct device *dev)
 {
-	const struct pwm_nrfx_config *pconfig = dev->config->config_info;
+	const struct pwm_nrfx_config *config = dev->config->config_info;
 
-	nrfx_err_t result = nrfx_pwm_init(&pconfig->pwm, &pconfig->config,
+	nrfx_err_t result = nrfx_pwm_init(&config->pwm,
+					  &config->initial_config,
 					  NULL);
-
 	if (result != NRFX_SUCCESS) {
-		LOG_ERR("Failed to initialize device: %s",
+		LOG_ERR("Failed to initialize device: %s", dev->config->name);
-			    dev->config->name);
 		return -EBUSY;
 	}
 
-	nrfx_pwm_simple_playback(&pconfig->pwm,
-				 &pconfig->seq,
-				 1,
-				 NRFX_PWM_FLAG_LOOP);
-
 	return 0;
 }
 
@@ -125,9 +262,9 @@ static int pwm_nrfx_init(struct device *dev)
 
 static void pwm_nrfx_uninit(struct device *dev)
 {
-	const struct pwm_nrfx_config *pconfig = dev->config->config_info;
+	const struct pwm_nrfx_config *config = dev->config->config_info;
 
-	nrfx_pwm_uninit(&pconfig->pwm);
+	nrfx_pwm_uninit(&config->pwm);
 }
 
 static int pwm_nrfx_set_power_state(u32_t new_state,
@@ -196,13 +333,13 @@ static int pwm_nrfx_pm_control(struct device *dev,
 
 #endif /* CONFIG_DEVICE_POWER_MANAGEMENT */
 
-#define PWM_NRFX_OUTPUT_PIN(dev_idx, ch_idx)		      \
+#define PWM_NRFX_OUTPUT_PIN(dev_idx, ch_idx)				      \
-	(DT_NORDIC_NRF_PWM_PWM_##dev_idx##_CH##ch_idx##_PIN | \
+	(DT_NORDIC_NRF_PWM_PWM_##dev_idx##_CH##ch_idx##_PIN |		      \
-	 (DT_NORDIC_NRF_PWM_PWM_##dev_idx##_CH##ch_idx##_INVERTED ?   \
+	 (DT_NORDIC_NRF_PWM_PWM_##dev_idx##_CH##ch_idx##_INVERTED ?	      \
 	  NRFX_PWM_PIN_INVERTED : 0))
 
-#define PWM_NRFX_DEFAULT_VALUE(dev_idx, ch_idx)		    \
+#define PWM_NRFX_DEFAULT_VALUE(dev_idx, ch_idx)                             \
-	(DT_NORDIC_NRF_PWM_PWM_##dev_idx##_CH##ch_idx##_INVERTED ? \
+	(DT_NORDIC_NRF_PWM_PWM_##dev_idx##_CH##ch_idx##_INVERTED ?            \
 	 PWM_NRFX_CH_VALUE_INVERTED : PWM_NRFX_CH_VALUE_NORMAL)
 
 #define PWM_NRFX_DEVICE(idx)						      \
@@ -213,19 +350,19 @@ static int pwm_nrfx_pm_control(struct device *dev,
 			PWM_NRFX_DEFAULT_VALUE(idx, 2),			      \
 			PWM_NRFX_DEFAULT_VALUE(idx, 3),			      \
 		},							      \
-		.top_value = NRFX_PWM_DEFAULT_CONFIG_TOP_VALUE		      \
+		.countertop = NRFX_PWM_DEFAULT_CONFIG_TOP_VALUE,	      \
+		.prescaler = NRFX_PWM_DEFAULT_CONFIG_BASE_CLOCK 	      \
 	};								      \
 	static const struct pwm_nrfx_config pwm_nrfx_##idx##_config = {	      \
 		.pwm = NRFX_PWM_INSTANCE(idx),				      \
-		.config = {						      \
+		.initial_config = {					      \
 			.output_pins = {				      \
 				PWM_NRFX_OUTPUT_PIN(idx, 0),		      \
 				PWM_NRFX_OUTPUT_PIN(idx, 1),		      \
 				PWM_NRFX_OUTPUT_PIN(idx, 2),		      \
 				PWM_NRFX_OUTPUT_PIN(idx, 3),		      \
 			},						      \
-			.base_clock = (nrf_pwm_clk_t)			      \
+			.base_clock = NRFX_PWM_DEFAULT_CONFIG_BASE_CLOCK,     \
-				      CONFIG_PWM_##idx##_NRF_CLOCK_PRESCALER, \
 			.count_mode = NRF_PWM_MODE_UP,			      \
 			.top_value = NRFX_PWM_DEFAULT_CONFIG_TOP_VALUE,	      \
 			.load_mode = NRF_PWM_LOAD_INDIVIDUAL,		      \