Merge pull request #9004 from Bob-the-Kuhn/PWM-for-any-pin-(LPC1768)

[2.0.x] LPC1768 - PWM for any pin (replaces PR #8991)

Marlin/src/HAL/HAL_LPC1768/HAL_timers.h

@@ -66,6 +66,11 @@ typedef uint32_t hal_timer_t;
 #define HAL_STEP_TIMER_ISR  extern "C" void TIMER0_IRQHandler(void)
 #define HAL_TEMP_TIMER_ISR  extern "C" void TIMER1_IRQHandler(void)
 
+// PWM timer 
+#define HAL_PWM_TIMER      LPC_TIM3
+#define HAL_PWM_TIMER_ISR  extern "C" void TIMER3_IRQHandler(void)
+#define HAL_PWM_TIMER_IRQn TIMER3_IRQn
+
 // --------------------------------------------------------------------------
 // Types
 // --------------------------------------------------------------------------

Marlin/src/HAL/HAL_LPC1768/LPC1768_PWM.cpp

@@ -32,7 +32,7 @@
  * The PWM1 module is used to directly control the Servo 0, 1 & 3 pins and D9 & D10 pins.  This keeps
  * the pulse width jitter to under a microsecond.
  *
- * For all other pins the PWM1 module is used to generate interrupts.  The ISR
+ * For all other pins a timer is used to generate interrupts.  The ISR
  * routine does the actual setting/clearing of pins.  The upside is that any pin can
  * have a PWM channel assigned to it.  The downside is that there is more pulse width
  * jitter. The jitter depends on what else is happening in the system and what ISRs
@@ -41,25 +41,11 @@
 
 /**
  * The data structures are set up to minimize the computation done by the ISR which
- * minimizes ISR execution time.  Execution times are 2-4µs except when updating to
- * a new value when they are 19µs.
+ * minimizes ISR execution time.  Execution times are 5-14µs depending on how full the
+ * ISR table is.  14uS is for a 20 element ISR table.
  *
  * Two tables are used.  One table contains the data used by the ISR to update/control
- * the PWM pins.  The other is used as an aid when rebuilding the ISR table.
- *
- * The LPC1768_PWM_attach_pin routine disables the ISR and then adds the new info to
- * ISR table.  It can update the table directly because none of its changes affect
- * what the ISR does.
- *
- * LPC1768_PWM_detach_pin routine disables the ISR, disables the pin immediately if
- * it's a directly controlled pin and updates the helper table.  It then flags the
- * ISR that the ISR table needs to be rebuilt.
- *
- * LPC1768_PWM_write routine disables the ISR and updates the helper table.  It then
- * flags the ISR that the ISR table needs to be rebuilt.
- *
- * The ISR's priority is set to less than the stepper ISR otherwise it could cause jitter
- * in the step pulses.
+ * the PWM pins.  The other is used as an aid when updating the ISR table.
  *
  * See the end of this file for details on the hardware/firmware interaction
  */
@@ -86,46 +72,36 @@
 #ifdef TARGET_LPC1768
 
 #include "../../inc/MarlinConfig.h"
-
 #include <lpc17xx_pinsel.h>
 #include "LPC1768_PWM.h"
 #include "arduino.h"
 
-#define NUM_PWMS 6
+#define NUM_ISR_PWMS 20
+
+
+#define LPC_PORT_OFFSET         (0x0020)
+#define LPC_PIN(pin)            (1UL << pin)
+#define LPC_GPIO(port)          ((volatile LPC_GPIO_TypeDef *)(LPC_GPIO0_BASE + LPC_PORT_OFFSET * port))
+
 
 typedef struct {            // holds all data needed to control/init one of the PWM channels
-  uint8_t             sequence;       // 0: available slot, 1 - 6: PWM channel assigned to that slot
+  bool                active_flag;    // THIS TABLE ENTRY IS ACTIVELY TOGGLING A PIN
   pin_t               pin;
-  uint16_t            PWM_mask;       // MASK TO CHECK/WRITE THE IR REGISTER
   volatile uint32_t*  set_register;
   volatile uint32_t*  clr_register;
   uint32_t            write_mask;     // USED BY SET/CLEAR COMMANDS
   uint32_t            microseconds;   // value written to MR register
   uint32_t            min;            // lower value limit checked by WRITE routine before writing to the MR register
   uint32_t            max;            // upper value limit checked by WRITE routine before writing to the MR register
-  bool                PWM_flag;       // 0 - USED BY hardware PWM, 1 - USED BY ANALOGWRITE
   uint8_t             servo_index;    // 0 - MAX_SERVO -1 : servo index,  0xFF : PWM channel
-  bool                active_flag;    // THIS TABLE ENTRY IS ACTIVELY TOGGLING A PIN
-  uint32_t            PCR_bit;        // PCR register bit to enable PWM1 control of this pin
-  volatile uint32_t*  PINSEL_reg;     // PINSEL register
-  uint32_t            PINSEL_bits;    // PINSEL register bits to set pin mode to PWM1 control
-
 } PWM_map;
 
+PWM_map PWM1_map_A[NUM_ISR_PWMS];  // compiler will initialize to all zeros
+PWM_map PWM1_map_B[NUM_ISR_PWMS];  // compiler will initialize to all zeros
 
-#define MICRO_MAX 0xFFFFFFFF
-
-#define PWM_MAP_INIT_ROW { 0, 0x7FFF, 0, 0, 0, 0, MICRO_MAX, 0, 0, 0, 0, 0, 0, 0, 0 }
-#define PWM_MAP_INIT { PWM_MAP_INIT_ROW, PWM_MAP_INIT_ROW, PWM_MAP_INIT_ROW, \
-                       PWM_MAP_INIT_ROW, PWM_MAP_INIT_ROW, PWM_MAP_INIT_ROW, \
-                     };
-
-PWM_map ISR_table[NUM_PWMS] = PWM_MAP_INIT;
-
-#define IR_BIT(p) ((p) >= 0 && (p) <= 3 ? (p) : p + 4 )
-#define PIN_IS_INVERTED(p) 0  // placeholder in case inverting PWM output is offered
-
-
+PWM_map *active_table = PWM1_map_A;
+PWM_map *work_table = PWM1_map_B;
+PWM_map *temp_table;
 
 #define P1_18_PWM_channel  1  // servo 3
 #define P1_20_PWM_channel  2  // servo 0
@@ -134,35 +110,13 @@ PWM_map ISR_table[NUM_PWMS] = PWM_MAP_INIT;
 #define P2_04_PWM_channel  5  // D9
 #define P2_05_PWM_channel  6  // D10
 
-// used to keep track of which Match Registers have been used and if they will be used by the
-// PWM1 module to directly control the pin or will be used to generate an interrupt
-typedef struct {                    // status of PWM1 channel
-  uint8_t             map_used;     // 0 - this MR register not used/assigned
-  uint8_t             map_PWM_INT;  // 0 - available for interrupts, 1 - in use by PWM
-  pin_t               map_PWM_PIN;  // pin for this PwM1 controlled pin / port
-  volatile uint32_t*  MR_register;  // address of the MR register for this PWM1 channel
-  uint32_t            PCR_bit;      // PCR register bit to enable PWM1 control of this pin
-                                    // 0 - don't switch to PWM1 direct control
-  volatile uint32_t*  PINSEL_reg;   // PINSEL register
-  uint32_t            PINSEL_bits;  // PINSEL register bits to set pin mode to PWM1 control
-} MR_map;
-
-MR_map map_MR[NUM_PWMS];
-
-void LPC1768_PWM_update_map_MR(void) {
-  map_MR[0] = { 0, (uint8_t) (LPC_PWM1->PCR & _BV(8 + P1_18_PWM_channel) ? 1 : 0), P1_18, &LPC_PWM1->MR1, 0, 0, 0 };
-  map_MR[1] = { 0, (uint8_t) (LPC_PWM1->PCR & _BV(8 + P1_20_PWM_channel) ? 1 : 0), P1_20, &LPC_PWM1->MR2, 0, 0, 0 };
-  map_MR[2] = { 0, (uint8_t) (LPC_PWM1->PCR & _BV(8 + P1_21_PWM_channel) ? 1 : 0), P1_21, &LPC_PWM1->MR3, 0, 0, 0 };
-  map_MR[3] = {
-    #if MB(MKS_SBASE)
-      0, (uint8_t) (LPC_PWM1->PCR & _BV(8 + P1_23_PWM_channel) ? 1 : 0), P1_23, &LPC_PWM1->MR4, 0, 0, 0
-    #else
-      0, 0, P_NC, &LPC_PWM1->MR4, 0, 0, 0
-    #endif
-  };
-  map_MR[4] = { 0, (uint8_t) (LPC_PWM1->PCR & _BV(8 + P2_04_PWM_channel) ? 1 : 0), P2_04, &LPC_PWM1->MR5, 0, 0, 0 };
-  map_MR[5] = { 0, (uint8_t) (LPC_PWM1->PCR & _BV(8 + P2_05_PWM_channel) ? 1 : 0), P2_05, &LPC_PWM1->MR6, 0, 0, 0 };
-}
+typedef struct {
+  uint32_t min;
+  uint32_t max;
+  bool     assigned;
+} table_direct;
+
+table_direct direct_table[6];  // compiler will initialize to all zeros
 
 /**
  *  Prescale register and MR0 register values
@@ -181,8 +135,8 @@ void LPC1768_PWM_update_map_MR(void) {
  *   0.25       399        4,999    199        4,999      99       4,999   49          4,999    0.720
  *  0.125       799        2,499    399        2,499     199       2,499   99          2,499    1.440
  *
- *  The desired prescale frequency comes from an input in the range of 544 - 2400 microseconds and the
- *  desire to just shift the input left or right as needed.
+ *  The desired prescale frequency column comes from an input in the range of 544 - 2400 microseconds
+ *  and the desire to just shift the input left or right as needed.
  *
  *  A resolution of 0.2 degrees seems reasonable so a prescale frequency output of 1MHz is being used.
  *  It also means we don't need to scale the input.
@@ -197,9 +151,10 @@ void LPC1768_PWM_update_map_MR(void) {
  *
  */
 
-bool ISR_table_update = false;  // flag to tell the ISR that the tables need to be updated & swapped
-
 void LPC1768_PWM_init(void) {
+
+  /////  directly controlled PWM pins (interrupts not used for these)
+
   #define SBIT_CNTEN      0  // PWM1 counter & pre-scaler enable/disable
   #define SBIT_CNTRST     1  // reset counters to known state
   #define SBIT_PWMEN      3  // 1 - PWM, 0 - timer
@@ -221,43 +176,116 @@ void LPC1768_PWM_init(void) {
   LPC_PWM1->LER  = 0x07F;                                                 // Set the latch Enable Bits to load the new Match Values for MR0 - MR6
   LPC_PWM1->PCR  = 0;                                                     // Single edge mode for all channels, PWM1 control of outputs off
 
-  NVIC_EnableIRQ(PWM1_IRQn);                                              // Enable interrupt handler
-  NVIC_SetPriority(PWM1_IRQn, NVIC_EncodePriority(0, 10, 0));             // Normal priority for PWM module
-  //NVIC_SetPriority(PWM1_IRQn, NVIC_EncodePriority(0, 0, 0));            // Minimizes jitter due to higher priority ISRs
+  ////  interrupt controlled PWM setup
+
+  LPC_SC->PCONP |= 1 << 23;  // power on timer3
+  HAL_PWM_TIMER->PR = LPC_PWM1_PR;
+  HAL_PWM_TIMER->MCR = 0x0B;              // Interrupt on MR0 & MR1, reset on MR0
+  HAL_PWM_TIMER->MR0 = LPC_PWM1_MR0;
+  HAL_PWM_TIMER->MR1 = 0;
+  HAL_PWM_TIMER->TCR = _BV(0);       // enable
+  NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);
+  NVIC_SetPriority(HAL_PWM_TIMER_IRQn, NVIC_EncodePriority(0, 4, 0));
 }
 
 
-bool LPC1768_PWM_attach_pin(pin_t pin, uint32_t min /* = 1 */, uint32_t max /* = (LPC_PWM1_MR0 - MR0_MARGIN) */, uint8_t servo_index /* = 0xff */) {
+bool ISR_table_update = false;  // flag to tell the ISR that the tables need to be updated & swapped
+uint8_t ISR_index = 0;          // index used by ISR to skip already actioned entries
+#define COPY_ACTIVE_TABLE    for (uint8_t i = 0; i < NUM_ISR_PWMS ; i++) work_table[i] = active_table[i]
+uint32_t first_MR1_value = LPC_PWM1_MR0 + 1;
+
+void LPC1768_PWM_sort(void) {
+
+  for (uint8_t i = NUM_ISR_PWMS; --i;) {  // (bubble) sort table by microseconds
+    bool didSwap = false;
+    PWM_map temp;
+    for (uint16_t j = 0; j < i; ++j) {
+      if (work_table[j].microseconds > work_table[j + 1].microseconds) {
+        temp = work_table[j + 1];
+        work_table[j + 1] = work_table[j];
+        work_table[j] = temp;
+        didSwap = true;
+      }
+    }
+    if (!didSwap) break;
+  }
+}
+
+bool LPC1768_PWM_attach_pin(pin_t pin, uint32_t min /* = 1 */, uint32_t max /* = (LPC_PWM1_MR0 - 1) */, uint8_t servo_index /* = 0xff */) {
 
   pin = GET_PIN_MAP_PIN(GET_PIN_MAP_INDEX(pin & 0xFF));  // Sometimes the upper byte is garbled
 
-  NVIC_DisableIRQ(PWM1_IRQn);    // make it safe to update the active table
+////  direct control PWM code
+  switch(pin) {
+    case P1_23:                                       // MKS Sbase Servo 0, PWM1 channel 4  (J3-8 PWM1.4)
+      direct_table[P1_23_PWM_channel - 1].min = min;
+      direct_table[P1_23_PWM_channel - 1].max = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
+      direct_table[P1_23_PWM_channel - 1].assigned = true;
+      return true;
+    case P1_20:                                       // Servo 0, PWM1 channel 2  (Pin 11  P1.20 PWM1.2)
+      direct_table[P1_20_PWM_channel - 1].min = min;
+      direct_table[P1_20_PWM_channel - 1].max = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
+      direct_table[P1_20_PWM_channel - 1].assigned = true;
+      return true;
+    case P1_21:                                       // Servo 1, PWM1 channel 3  (Pin 6  P1.21 PWM1.3)
+      direct_table[P1_21_PWM_channel - 1].min = min;
+      direct_table[P1_21_PWM_channel - 1].max = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
+      direct_table[P1_21_PWM_channel - 1].assigned = true;
+      return true;
+    case P1_18:                                       // Servo 3, PWM1 channel 1 (Pin 4  P1.18 PWM1.1)
+      direct_table[P1_18_PWM_channel - 1].min = min;
+      direct_table[P1_18_PWM_channel - 1].max = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
+      direct_table[P1_18_PWM_channel - 1].assigned = true;
+      return true;
+    case P2_04:                                       // D9 FET, PWM1 channel 5  (Pin 9  P2_04 PWM1.5)
+      direct_table[P2_04_PWM_channel - 1].min = min;
+      direct_table[P2_04_PWM_channel - 1].max = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
+      direct_table[P2_04_PWM_channel - 1].assigned = true;
+      return true;
+    case P2_05:                                       // D10 FET, PWM1 channel 6 (Pin 10  P2_05 PWM1.6)
+      direct_table[P2_05_PWM_channel - 1].min = min;
+      direct_table[P2_05_PWM_channel - 1].max = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
+      direct_table[P2_05_PWM_channel - 1].assigned = true;
+      return true;
+  }
+
+////  interrupt controlled PWM code
+  NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);    // make it safe to update the active table
                                  // OK to update the active table because the
                                  // ISR doesn't use any of the changed items
+
+  if (ISR_table_update) //use work table if that's the newest
+    temp_table = work_table;
+  else
+    temp_table = active_table;
+
   uint8_t slot = 0;
-  for (uint8_t i = 0; i < NUM_PWMS ; i++)         // see if already in table
-    if (ISR_table[i].pin == pin) {
-      NVIC_EnableIRQ(PWM1_IRQn);  // re-enable PWM interrupts
+  for (uint8_t i = 0; i < NUM_ISR_PWMS; i++)         // see if already in table
+    if (temp_table[i].pin == pin) {
+      NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
       return 1;
     }
 
-  for (uint8_t i = 1; (i < NUM_PWMS + 1) && !slot; i++)         // find empty slot
-    if ( !(ISR_table[i - 1].set_register)) { slot = i; break; }  // any item that can't be zero when active or just attached is OK
-  if (!slot) return 0;
+  for (uint8_t i = 1; (i < NUM_ISR_PWMS + 1) && !slot; i++)         // find empty slot
+    if ( !(temp_table[i - 1].set_register)) { slot = i; break; }  // any item that can't be zero when active or just attached is OK
+
+  if (!slot) {
+    NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
+    return 0;
+  }
+
   slot--;  // turn it into array index
 
-  ISR_table[slot].pin          = pin;     // init slot
-  ISR_table[slot].PWM_mask     = 0;  // real value set by PWM_write
-  ISR_table[slot].set_register = PIN_IS_INVERTED(pin) ? &LPC_GPIO(LPC1768_PIN_PORT(pin))->FIOCLR : &LPC_GPIO(LPC1768_PIN_PORT(pin))->FIOSET;
-  ISR_table[slot].clr_register = PIN_IS_INVERTED(pin) ? &LPC_GPIO(LPC1768_PIN_PORT(pin))->FIOSET : &LPC_GPIO(LPC1768_PIN_PORT(pin))->FIOCLR;
-  ISR_table[slot].write_mask   = LPC_PIN(LPC1768_PIN_PIN(pin));
-  ISR_table[slot].microseconds = MICRO_MAX;
-  ISR_table[slot].min          = min;
-  ISR_table[slot].max          = MIN(max, LPC_PWM1_MR0 - MR0_MARGIN);
-  ISR_table[slot].servo_index  = servo_index;
-  ISR_table[slot].active_flag  = false;
+  temp_table[slot].pin          = pin;     // init slot
+  temp_table[slot].set_register = &LPC_GPIO(LPC1768_PIN_PORT(pin))->FIOSET;
+  temp_table[slot].clr_register = &LPC_GPIO(LPC1768_PIN_PORT(pin))->FIOCLR;
+  temp_table[slot].write_mask   = LPC_PIN(LPC1768_PIN_PIN(pin));
+  temp_table[slot].min          = min;
+  temp_table[slot].max          = max;                // different max for ISR PWMs than for direct PWMs
+  temp_table[slot].servo_index  = servo_index;
+  temp_table[slot].active_flag  = false;
 
-  NVIC_EnableIRQ(PWM1_IRQn);  // re-enable PWM interrupts
+  NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
 
   return 1;
 }
@@ -267,201 +295,154 @@ bool LPC1768_PWM_detach_pin(pin_t pin) {
 
   pin = GET_PIN_MAP_PIN(GET_PIN_MAP_INDEX(pin & 0xFF));
 
-  NVIC_DisableIRQ(PWM1_IRQn);
-
-  uint8_t slot = 0xFF;
-  for (uint8_t i = 0; i < NUM_PWMS; i++)         // find slot
-    if (ISR_table[i].pin == pin) { slot = i; break; }
-  if (slot == 0xFF) {   // return error if pin not found
-    NVIC_EnableIRQ(PWM1_IRQn);
-    return false;
-  }
-
-  LPC1768_PWM_update_map_MR();
-
-  // OK to make these changes before the MR0 interrupt
+////  direct control PWM code
   switch(pin) {
     case P1_23:                                       // MKS Sbase Servo 0, PWM1 channel 4  (J3-8 PWM1.4)
+      if (!direct_table[P1_23_PWM_channel - 1].assigned) return false;
       CBI(LPC_PWM1->PCR, 8 + P1_23_PWM_channel);      // disable PWM1 module control of this pin
-      map_MR[P1_23_PWM_channel - 1].PCR_bit = 0;
       LPC_PINCON->PINSEL3 &= ~(0x3 <<  14);           // return pin to general purpose I/O
-      map_MR[P1_23_PWM_channel - 1].PINSEL_bits = 0;
-      map_MR[P1_23_PWM_channel - 1].map_PWM_INT = 0;  // 0 - available for interrupts, 1 - in use by PWM
-      break;
+      direct_table[P1_23_PWM_channel - 1].assigned = false;
+      return true;
     case P1_20:                                       // Servo 0, PWM1 channel 2  (Pin 11  P1.20 PWM1.2)
+      if (!direct_table[P1_20_PWM_channel - 1].assigned) return false;
       CBI(LPC_PWM1->PCR, 8 + P1_20_PWM_channel);      // disable PWM1 module control of this pin
-      map_MR[P1_20_PWM_channel - 1].PCR_bit = 0;
       LPC_PINCON->PINSEL3 &= ~(0x3 <<  8);            // return pin to general purpose I/O
-      map_MR[P1_20_PWM_channel - 1].PINSEL_bits = 0;
-      map_MR[P1_20_PWM_channel - 1].map_PWM_INT = 0;  // 0 - available for interrupts, 1 - in use by PWM
-      break;
+      direct_table[P1_20_PWM_channel - 1].assigned = false;
+      return true;
     case P1_21:                                       // Servo 1, PWM1 channel 3  (Pin 6  P1.21 PWM1.3)
+      if (!direct_table[P1_21_PWM_channel - 1].assigned) return false;
       CBI(LPC_PWM1->PCR, 8 + P1_21_PWM_channel);      // disable PWM1 module control of this pin
-      map_MR[P1_21_PWM_channel - 1].PCR_bit = 0;
       LPC_PINCON->PINSEL3 &= ~(0x3 << 10);            // return pin to general purpose I/O
-      map_MR[P1_21_PWM_channel - 1].PINSEL_bits = 0;
-      map_MR[P1_21_PWM_channel - 1].map_PWM_INT = 0;  // 0 - available for interrupts, 1 - in use by PWM
-      break;
+      direct_table[P1_21_PWM_channel - 1].assigned = false;
+      return true;
     case P1_18:                                       // Servo 3, PWM1 channel 1 (Pin 4  P1.18 PWM1.1)
+      if (!direct_table[P1_18_PWM_channel - 1].assigned) return false;
       CBI(LPC_PWM1->PCR, 8 + P1_18_PWM_channel);      // disable PWM1 module control of this pin
-      map_MR[P1_18_PWM_channel - 1].PCR_bit =  0;
       LPC_PINCON->PINSEL3 &= ~(0x3 <<  4);            // return pin to general purpose I/O
-      map_MR[P1_18_PWM_channel - 1].PINSEL_bits =  0;
-      map_MR[P1_18_PWM_channel - 1].map_PWM_INT = 0;  // 0 - available for interrupts, 1 - in use by PWM
-      break;
+      direct_table[P1_18_PWM_channel - 1].assigned = false;
+      return true;
     case P2_04:                                       // D9 FET, PWM1 channel 5  (Pin 9  P2_04 PWM1.5)
+      if (!direct_table[P2_04_PWM_channel - 1].assigned) return false;
       CBI(LPC_PWM1->PCR, 8 + P2_04_PWM_channel);      // disable PWM1 module control of this pin
-      map_MR[P2_04_PWM_channel - 1].PCR_bit = 0;
       LPC_PINCON->PINSEL4 &= ~(0x3 << 10);            // return pin to general purpose I/O
-      map_MR[P2_04_PWM_channel - 1].PINSEL_bits = 0;
-      map_MR[P2_04_PWM_channel - 1].map_PWM_INT = 0;  // 0 - available for interrupts, 1 - in use by PWM
-      break;
+      direct_table[P2_04_PWM_channel - 1].assigned = false;
+      return true;
     case P2_05:                                       // D10 FET, PWM1 channel 6 (Pin 10  P2_05 PWM1.6)
+      if (!direct_table[P2_05_PWM_channel - 1].assigned) return false;
       CBI(LPC_PWM1->PCR, 8 + P2_05_PWM_channel);      // disable PWM1 module control of this pin
-      map_MR[P2_05_PWM_channel - 1].PCR_bit =  0;
       LPC_PINCON->PINSEL4 &= ~(0x3 <<  4);            // return pin to general purpose I/O
-      map_MR[P2_05_PWM_channel - 1].PINSEL_bits =  0;
-      map_MR[P2_05_PWM_channel - 1].map_PWM_INT = 0;  // 0 - available for interrupts, 1 - in use by PWM
-      break;
-    default:
+      direct_table[P2_05_PWM_channel - 1].assigned = false;
+      return true;
+  }
+
+////  interrupt controlled PWM code
+  NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);
+
+  if (ISR_table_update) {
+    ISR_table_update = false;    // don't update yet - have another update to do
+    NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
+  }
+  else {
+    NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
+    COPY_ACTIVE_TABLE;  // copy active table into work table
+  }
+
+  uint8_t slot = 0xFF;
+  for (uint8_t i = 0; i < NUM_ISR_PWMS; i++) {        // find slot
+    if (work_table[i].pin == pin) {
+      slot = i;
       break;
+    }
   }
+  if (slot == 0xFF)    // return error if pin not found
+    return false;
 
-  ISR_table[slot] = PWM_MAP_INIT_ROW;
+  work_table[slot] = {0, 0, 0, 0, 0, 0, 0, 0, 0};
 
+  LPC1768_PWM_sort();    // sort table by microseconds
   ISR_table_update = true;
-  NVIC_EnableIRQ(PWM1_IRQn);  // re-enable PWM interrupts
-
-  return 1;
+  return true;
 }
 
-
+// value is 0-20,000 microseconds (0% to 100% duty cycle)
+// servo routine provides values in the 544 - 2400 range
 bool LPC1768_PWM_write(pin_t pin, uint32_t value) {
 
   pin = GET_PIN_MAP_PIN(GET_PIN_MAP_INDEX(pin & 0xFF));
 
-  NVIC_DisableIRQ(PWM1_IRQn);
-
-  uint8_t slot = 0xFF;
-  for (uint8_t i = 0; i < NUM_PWMS; i++)         // find slot
-    if (ISR_table[i].pin == pin) { slot = i; break; }
-  if (slot == 0xFF) {   // return error if pin not found
-    NVIC_EnableIRQ(PWM1_IRQn);
-    return false;
-  }
-
-  LPC1768_PWM_update_map_MR();
-
+////  direct control PWM code
   switch(pin) {
     case P1_23:                                                           // MKS Sbase Servo 0, PWM1 channel 4  (J3-8 PWM1.4)
-      map_MR[P1_23_PWM_channel - 1].PCR_bit = _BV(8 + P1_23_PWM_channel); // enable PWM1 module control of this pin
-      map_MR[P1_23_PWM_channel - 1].PINSEL_reg = &LPC_PINCON->PINSEL3;
-      map_MR[P1_23_PWM_channel - 1].PINSEL_bits = 0x2 <<  14;             // ISR must do this AFTER setting PCR
-      break;
+      if (!direct_table[P1_23_PWM_channel - 1].assigned) return false;
+      LPC_PWM1->PCR |=  _BV(8 + P1_23_PWM_channel); // enable PWM1 module control of this pin
+      LPC_PINCON->PINSEL3 = 0x2 <<  14;             // must set pin function AFTER setting PCR
+      // load the new time value
+      LPC_PWM1->MR4 = MAX(MIN(value, direct_table[P1_23_PWM_channel - 1].max), direct_table[P1_23_PWM_channel - 1].min);
+      LPC_PWM1->LER = 0x1 << P1_23_PWM_channel; // Set the latch Enable Bit to load the new Match Value on the next MR0
+      return true;
     case P1_20:                                                           // Servo 0, PWM1 channel 2 (Pin 11  P1.20 PWM1.2)
-      map_MR[P1_20_PWM_channel - 1].PCR_bit = _BV(8 + P1_20_PWM_channel); // enable PWM1 module control of this pin
-      map_MR[P1_20_PWM_channel - 1].PINSEL_reg = &LPC_PINCON->PINSEL3;
-      map_MR[P1_20_PWM_channel - 1].PINSEL_bits = 0x2 <<  8;              // ISR must do this AFTER setting PCR
-      break;
+      if (!direct_table[P1_20_PWM_channel - 1].assigned) return false;
+      LPC_PWM1->PCR |=  _BV(8 + P1_20_PWM_channel); // enable PWM1 module control of this pin
+      LPC_PINCON->PINSEL3 |= 0x2 <<  8;             // must set pin function AFTER setting PCR
+      // load the new time value
+      LPC_PWM1->MR2 = MAX(MIN(value, direct_table[P1_20_PWM_channel - 1].max), direct_table[P1_20_PWM_channel - 1].min);
+      LPC_PWM1->LER = 0x1 << P1_20_PWM_channel; // Set the latch Enable Bit to load the new Match Value on the next MR0
+      return true;
     case P1_21:                                                           // Servo 1, PWM1 channel 3 (Pin 6  P1.21 PWM1.3)
-      map_MR[P1_21_PWM_channel - 1].PCR_bit = _BV(8 + P1_21_PWM_channel); // enable PWM1 module control of this pin
-      map_MR[P1_21_PWM_channel - 1].PINSEL_reg = &LPC_PINCON->PINSEL3;
-      map_MR[P1_21_PWM_channel - 1].PINSEL_bits = 0x2 << 10;              // ISR must do this AFTER setting PCR
-      break;
+      if (!direct_table[P1_21_PWM_channel - 1].assigned) return false;
+      LPC_PWM1->PCR |=  _BV(8 + P1_21_PWM_channel); // enable PWM1 module control of this pin
+      LPC_PINCON->PINSEL3 |= 0x2 << 10;              // must set pin function AFTER setting PCR
+      // load the new time value
+      LPC_PWM1->MR3 = MAX(MIN(value, direct_table[P1_21_PWM_channel - 1].max), direct_table[P1_21_PWM_channel - 1].min);
+      LPC_PWM1->LER = 0x1 << P1_21_PWM_channel; // Set the latch Enable Bit to load the new Match Value on the next MR0
+      return true;
     case P1_18:                                                           // Servo 3, PWM1 channel 1 (Pin 4  P1.18 PWM1.1)
-      map_MR[P1_18_PWM_channel - 1].PCR_bit = _BV(8 + P1_18_PWM_channel); // enable PWM1 module control of this pin
-      map_MR[P1_18_PWM_channel - 1].PINSEL_reg = &LPC_PINCON->PINSEL3;
-      map_MR[P1_18_PWM_channel - 1].PINSEL_bits = 0x2 <<  4;              // ISR must do this AFTER setting PCR
-      break;
+      if (!direct_table[P1_18_PWM_channel - 1].assigned) return false;
+      LPC_PWM1->PCR |=  _BV(8 + P1_18_PWM_channel); // enable PWM1 module control of this pin
+      LPC_PINCON->PINSEL3 |= 0x2 <<  4;             // must set pin function AFTER setting PCR
+      // load the new time value
+      LPC_PWM1->MR1 = MAX(MIN(value, direct_table[P1_18_PWM_channel - 1].max), direct_table[P1_18_PWM_channel - 1].min);
+      LPC_PWM1->LER = 0x1 << P1_18_PWM_channel; // Set the latch Enable Bit to load the new Match Value on the next MR0
+      return true;
     case P2_04:                                                           // D9 FET, PWM1 channel 5 (Pin 9  P2_04 PWM1.5)
-      map_MR[P2_04_PWM_channel - 1].PCR_bit = _BV(8 + P2_04_PWM_channel); // enable PWM1 module control of this pin
-      map_MR[P2_04_PWM_channel - 1].PINSEL_reg = &LPC_PINCON->PINSEL4;
-      map_MR[P2_04_PWM_channel - 1].PINSEL_bits = 0x1 <<  8;              // ISR must do this AFTER setting PCR
-      break;
+      if (!direct_table[P2_04_PWM_channel - 1].assigned) return false;
+      LPC_PWM1->PCR |=  _BV(8 + P2_04_PWM_channel); // enable PWM1 module control of this pin
+      LPC_PINCON->PINSEL4 |= 0x1 <<  8;             // must set pin function AFTER setting PCR
+      // load the new time value
+      LPC_PWM1->MR5 = MAX(MIN(value, direct_table[P2_04_PWM_channel - 1].max), direct_table[P2_04_PWM_channel - 1].min);
+      LPC_PWM1->LER = 0x1 << P2_04_PWM_channel; // Set the latch Enable Bit to load the new Match Value on the next MR0
+      return true;
     case P2_05:                                                           // D10 FET, PWM1 channel 6 (Pin 10  P2_05 PWM1.6)
-      map_MR[P2_05_PWM_channel - 1].PCR_bit = _BV(8 + P2_05_PWM_channel); // enable PWM1 module control of this pin
-      map_MR[P2_05_PWM_channel - 1].PINSEL_reg = &LPC_PINCON->PINSEL4;
-      map_MR[P2_05_PWM_channel - 1].PINSEL_bits = 0x1 << 10;              // ISR must do this AFTER setting PCR
-      break;
-    default:  // ISR pins
-      pinMode(pin, OUTPUT);  // set pin to output
-      break;
+      if (!direct_table[P2_05_PWM_channel - 1].assigned) return false;
+      LPC_PWM1->PCR |=  _BV(8 + P2_05_PWM_channel); // enable PWM1 module control of this pin
+      LPC_PINCON->PINSEL4 |= 0x1 << 10;             // must set pin function AFTER setting PCR
+      // load the new time value
+      LPC_PWM1->MR6 = MAX(MIN(value, direct_table[P2_05_PWM_channel - 1].max), direct_table[P2_05_PWM_channel - 1].min);
+      LPC_PWM1->LER = 0x1 << P2_05_PWM_channel; // Set the latch Enable Bit to load the new Match Value on the next MR0
+      return true;
   }
 
-  ISR_table[slot].microseconds = MAX(MIN(value, ISR_table[slot].max), ISR_table[slot].min);
-  ISR_table[slot].active_flag = 1;
-
-  ISR_table_update = true;
-
-  NVIC_EnableIRQ(PWM1_IRQn);  // re-enable PWM interrupts
-
-  return 1;
-}
+////  interrupt controlled PWM code
+  NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);
+  if (!ISR_table_update)   // use the most up to date table
+    COPY_ACTIVE_TABLE;  // copy active table into work table
 
-
-uint32_t LPC1768_PWM_interrupt_mask = 1;
-
-
-void LPC1768_PWM_update(void) {     // only called by the ISR
-  LPC1768_PWM_interrupt_mask = 0;                          // set match registers to new values, build IRQ mask
-  // first setup directly controlled PWM pin slots
-
-  bool found;
-  for (uint8_t i = 0; i < NUM_PWMS; i++) {
-    ISR_table[i].PCR_bit = 0;     // clear entries
-    ISR_table[i].PINSEL_reg = 0;
-    ISR_table[i].PINSEL_bits = 0;
-    ISR_table[i].PWM_flag = 1;    // mark slot as interrupt mode until find differently
-
-    if (ISR_table[i].active_flag) {
-      ISR_table[i].sequence = i + 1;
-
-      // first see if there is a PWM1 controlled pin for this entry
-      found = false;
-      for (uint8_t j = 0; (j < NUM_PWMS) && !found; j++) {
-        if ( (map_MR[j].map_PWM_PIN == ISR_table[i].pin)) {
-          map_MR[j].map_PWM_INT = 1;                          // flag that it's already setup for direct control
-          ISR_table[i].PWM_mask = 0;
-          ISR_table[i].PCR_bit = map_MR[j].PCR_bit;           // PCR register bit to enable PWM1 control of this pin
-          ISR_table[i].PINSEL_reg = map_MR[j].PINSEL_reg;     // PINSEL register address to set pin mode to PWM1 control} MR_map;
-          ISR_table[i].PINSEL_bits = map_MR[j].PINSEL_bits;   // PINSEL register bits to set pin mode to PWM1 control} MR_map;
-          map_MR[j].map_used = 2;
-          ISR_table[i].PWM_flag = 0;
-          *map_MR[j].MR_register = ISR_table[i].microseconds;
-          found = true;
-        }
-      }
-    }
-    else
-    ISR_table[i].sequence = 0;
+  uint8_t slot = 0xFF;
+  for (uint8_t i = 0; i < NUM_ISR_PWMS; i++)         // find slot
+    if (work_table[i].pin == pin) { slot = i; break; }
+  if (slot == 0xFF) {   // return error if pin not found
+    NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);
+    return false;
   }
 
-  // next fill in interrupt slots
-  for (uint8_t i = 0; i < NUM_PWMS; i++) {
-
-    if (ISR_table[i].active_flag && ISR_table[i].PWM_flag) {
-
-      // setup interrupt slot
-      found = false;
-      for (uint8_t k = 0; (k < NUM_PWMS) && !found; k++) {
-        if ( !(map_MR[k].map_PWM_INT || map_MR[k].map_used)) {
-          *map_MR[k].MR_register = ISR_table[i].microseconds;  // found one for an interrupt pin
-          map_MR[k].map_used = 1;
-          LPC1768_PWM_interrupt_mask |= _BV(3 * (k + 1));  // set bit in the MCR to enable this MR to generate an interrupt
-          ISR_table[i].set_register = PIN_IS_INVERTED(ISR_table[i].pin) ? &LPC_GPIO(LPC1768_PIN_PORT(ISR_table[i].pin))->FIOCLR : &LPC_GPIO(LPC1768_PIN_PORT(ISR_table[i].pin))->FIOSET;
-          ISR_table[i].clr_register = PIN_IS_INVERTED(ISR_table[i].pin) ? &LPC_GPIO(LPC1768_PIN_PORT(ISR_table[i].pin))->FIOSET : &LPC_GPIO(LPC1768_PIN_PORT(ISR_table[i].pin))->FIOCLR;
-          ISR_table[i].write_mask = LPC_PIN(LPC1768_PIN_PIN(ISR_table[i].pin));
-          ISR_table[i].PWM_mask = _BV(IR_BIT(k + 1));  // bit in the IR that will go active when this MR generates an interrupt
-          ISR_table[i].PWM_flag = 1;
-          found = true;
-        }
-      }
-    }
-  }
+  work_table[slot].microseconds = MAX(MIN(value, work_table[slot].max), work_table[slot].min);;
+  work_table[slot].active_flag  = true;
 
-  LPC1768_PWM_interrupt_mask |= (uint32_t) _BV(0);  // add in MR0 interrupt
+  LPC1768_PWM_sort();    // sort table by microseconds
+  ISR_table_update = true;
 
-  LPC_PWM1->LER = 0x07E;  // Set the latch Enable Bits to load the new Match Values for MR1 - MR6
+  NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
+  return 1;
 }
 
 
@@ -469,97 +450,102 @@ bool useable_hardware_PWM(pin_t pin) {
 
   pin = GET_PIN_MAP_PIN(GET_PIN_MAP_INDEX(pin & 0xFF));
 
-  NVIC_DisableIRQ(PWM1_IRQn);
+  NVIC_DisableIRQ(HAL_PWM_TIMER_IRQn);
 
   bool return_flag = false;
-  for (uint8_t i = 0; i < NUM_PWMS; i++)         // see if it's already setup
-    if (ISR_table[i].pin == pin && ISR_table[i].sequence) return_flag = true;
-  for (uint8_t i = 0; i < NUM_PWMS; i++)         // see if there is an empty slot
-    if (!ISR_table[i].sequence) return_flag = true;
-  NVIC_EnableIRQ(PWM1_IRQn);  // re-enable PWM interrupts
+  for (uint8_t i = 0; i < NUM_ISR_PWMS; i++)         // see if it's already setup
+    if (active_table[i].pin == pin) return_flag = true;
+  for (uint8_t i = 0; i < NUM_ISR_PWMS; i++)         // see if there is an empty slot
+    if (!active_table[i].set_register) return_flag = true;
+  NVIC_EnableIRQ(HAL_PWM_TIMER_IRQn);  // re-enable PWM interrupts
   return return_flag;
 }
 
+
 ////////////////////////////////////////////////////////////////////////////////
 
-#define HAL_PWM_LPC1768_ISR  extern "C" void PWM1_IRQHandler(void)
 
-// Both loops could be terminated when the last active channel is found but that would
-// result in variations ISR run time which results in variations in pulse width
+#define PWM_LPC1768_ISR_SAFETY_FACTOR 5  // amount of time needed to guarantee MR1 count will be above TC
+volatile bool in_PWM_isr = false;
 
-/**
- * Changes to PINSEL, PCR and MCR are only done during the MR0 interrupt otherwise
- * the wrong pin may be toggled or even have the system hang.
- */
+HAL_PWM_TIMER_ISR {
+  bool first_active_entry = true;
+  uint32_t next_MR1_val;
 
+  if (in_PWM_isr) goto exit_PWM_ISR;  // prevent re-entering this ISR
+  in_PWM_isr = true;
 
-HAL_PWM_LPC1768_ISR {
-
-  if (LPC_PWM1->IR & 0x1) {                       // MR0 interrupt
-    if (ISR_table_update) {                       // new values have been loaded so set everything
-      LPC1768_PWM_update();                       // update & swap table
-      LPC_PWM1->MCR = LPC1768_PWM_interrupt_mask; // enable new PWM individual channel interrupts
+  if (HAL_PWM_TIMER->IR & 0x01) {  // MR0 interrupt
+    next_MR1_val = first_MR1_value;               // only used if have a blank ISR table
+    if (ISR_table_update) {                       // new values have been loaded so swap tables
+      temp_table = active_table;
+      active_table = work_table;
+      work_table = temp_table;
+      ISR_table_update = false;
     }
-    for (uint8_t i = 0; i < NUM_PWMS; i++) {
-      if (ISR_table[i].active_flag && !((ISR_table[i].pin == P1_20) ||
-                                        (ISR_table[i].pin == P1_21) ||
-                                        (ISR_table[i].pin == P1_18) ||
-                                        (ISR_table[i].pin == P2_04) ||
-                                        (ISR_table[i].pin == P2_05))
-      ) {
-        *ISR_table[i].set_register = ISR_table[i].write_mask;       // set pins for all enabled interrupt channels active
+  }
+  HAL_PWM_TIMER->IR = 0x3F;  // clear all interrupts
+
+  for (uint8_t i = 0; i < NUM_ISR_PWMS; i++) {
+    if (active_table[i].active_flag) {
+      if (first_active_entry) {
+        first_active_entry = false;
+        next_MR1_val = active_table[i].microseconds;
       }
-      if (ISR_table_update && ISR_table[i].PCR_bit) {
-        LPC_PWM1->PCR |= ISR_table[i].PCR_bit;                  // enable PWM1 module control of this pin
-        *ISR_table[i].PINSEL_reg |= ISR_table[i].PINSEL_bits;   // set pin mode to PWM1 control - must be done after PCR
+      if (HAL_PWM_TIMER->TC < active_table[i].microseconds) {
+        *active_table[i].set_register = active_table[i].write_mask;   // set pin high
       }
-    }
-    ISR_table_update = false;
-    LPC_PWM1->IR = 0x01;                                             // clear the MR0 interrupt flag bit
-  }
-  else {
-    for (uint8_t i = 0; i < NUM_PWMS; i++)
-      if (ISR_table[i].active_flag && (LPC_PWM1->IR & ISR_table[i].PWM_mask)) {
-        LPC_PWM1->IR = ISR_table[i].PWM_mask;       // clear the interrupt flag bits for expected interrupts
-        *ISR_table[i].clr_register = ISR_table[i].write_mask;   // set channel to inactive
+      else {
+        *active_table[i].clr_register = active_table[i].write_mask;   // set pin low
+        next_MR1_val = (i == NUM_ISR_PWMS -1)
+          ? LPC_PWM1_MR0 + 1                  // done with table, wait for MR0
+          : active_table[i + 1].microseconds; // set next MR1 interrupt?
       }
+    }
   }
-
-  LPC_PWM1->IR = 0x70F;  // guarantees all interrupt flags are cleared which, if there is an unexpected
-                         // PWM interrupt, will keep the ISR from hanging which will crash the controller
+  if (first_active_entry) next_MR1_val = LPC_PWM1_MR0 + 1;  // empty table so disable MR1 interrupt
+  HAL_PWM_TIMER->MR1 = MAX(next_MR1_val, HAL_PWM_TIMER->TC + PWM_LPC1768_ISR_SAFETY_FACTOR); // set next
+  in_PWM_isr = false;
+  
+  exit_PWM_ISR:
+  return;
 }
-
 #endif
 
+
 /////////////////////////////////////////////////////////////////
 /////////////////  HARDWARE FIRMWARE INTERACTION ////////////////
 /////////////////////////////////////////////////////////////////
 
 /**
- *  Almost all changes to the hardware registers must be coordinated with the Match Register 0 (MR0)
- *  interrupt.  The only exception is detaching pins.  It doesn't matter when they go
- *  tristate.
+ *  There are two distinct systems used for PWMs:
+ *    directly controlled pins
+ *    ISR controlled pins.
  *
- *  The LPC1768_PWM_init routine kicks off the MR0 interrupt.  This interrupt is never disabled.  It
- *  can be delayed by higher priority interrupts.  Actions on directly controlled pins are not delayed
- *  by other interrupts
+ *  The two systems are independent of each other.  The use the same counter frequency so there's no
+ *  translation needed when setting the time values.  The init, attach, detach and write routines all
+ *  start with the direct pin code which is followed by the ISR pin code.
  *
- *  The ISR_table_update flag is set when the ISR table needs to be rebuilt.  It is
- *  cleared by the ISR during the MR0 interrupt after it rebuilds the ISR table.
+ *  The PMW1 module handles the directly controlled pins.  Each directly controlled pin is associated
+ *  with a match register (MR1 - MR6).  When the associated MR equals the module's TIMER/COUNTER (TC)
+ *  then the pins is set to low.  The MR0 register controls the repetition rate.  When the TC equals
+ *  MR0 then the TC is reset and ALL directly controlled pins are set high.  The resulting pulse widths
+ *  are almost immune to system loading and ISRs.  No PWM1 interrupts are used.
  *
- *  The sequence of events during a write to a PWM channel is:
- *    1) Attach routine puts the pin number in the ISR table but doesn't mark it active.
- *    2) Write routine marks the pin as active, updates the helper table and flags the ISR that the
- *       ISR table needs to be rebuilt.
- *    3) On the MR0 interrupt the ISR:
- *         a. Rebuilds the ISR table if needed.
- *                 MR1-MR6 are updated at this time.  The updates aren't put into use until the first
- *                 MR0 after the LER register has been written.  The LER register is written during the
- *                 table rebuild process.  The result is new timing takes 20-40 mS to be implemented.
- *         b. Sets the interrupt controlled pin(s) to their active state
- *         c. Writes to the PCR register to enable the directly controlled pins
- *         d. Sets the PINSEL register to the function/mode for the directly controlled pins
+ *  The ISR controlled pins use the TIMER/COUNTER, MR0 and MR1 registers from one timer.  MR0 controls
+ *  period of the controls the repetition rate.  When the TC equals MR0 then the TC is reset and an
+ *  interrupt is generated. When the TC equals MR1 then an interrupt is generated.
  *
- *    4) For each interrupt controlled pin there is another ISR call.  During this call the pin is set
- *       to its inactive state.  The call is initiated when a MR1-MR6 reg times out.
+ *  Each interrupt does the following:
+ *    1) Swaps the tables if it's a MR0 interrupt and the swap flag is set.  It then clears the swap flag.
+ *    2) Scans the entire ISR table (it's been sorted low to high time)
+ *         a. If its the first active entry then it grabs the time as a tentative time for MR1
+ *         b. If active and TC is less than the time then it sets the pin high
+ *         c. If active and TC is more than the time it sets the pin high
+ *         d. On every entry that sets a pin low it grabs the NEXT entry's time for use as the next MR1.
+ *            This results in MR1 being set to the time in the first active entry that does NOT set a
+ *            pin low.
+ *         e. If it's setting the last entry's pin low then it sets MR1 to a value bigger than MR0
+ *         f. If no value has been grabbed for the next MR1 then it's an empty table and MR1 is set to a
+ *            value greater than MR0
  */

Marlin/src/HAL/HAL_LPC1768/arduino.cpp

@@ -136,8 +136,8 @@ void analogWrite(pin_t pin, int pwm_value) {  // 1 - 254: pwm_value, 0: LOW, 255
     digitalWrite(pin, value);
   }
   else {
-    if (LPC1768_PWM_attach_pin(pin, 1, (LPC_PWM1->MR0 - MR0_MARGIN),  0xff))   // locks up if get too close to MR0 value
-      LPC1768_PWM_write(pin, map(value, 1, 254, 1, (LPC_PWM1->MR0 - MR0_MARGIN)));  // map 1-254 onto PWM range
+    if (LPC1768_PWM_attach_pin(pin, 1, LPC_PWM1->MR0,  0xff))   
+      LPC1768_PWM_write(pin, map(value, 0, 255, 1, LPC_PWM1->MR0));  // map 1-254 onto PWM range
     else {                                                                 // out of PWM channels
       if (!out_of_PWM_slots) MYSERIAL.printf(".\nWARNING - OUT OF PWM CHANNELS\n.\n");  //only warn once
       out_of_PWM_slots = true;