MCUdude / MegaCoreX

An Arduino hardware package for ATmega4809, ATmega4808, ATmega3209, ATmega3208, ATmega1609, ATmega1608, ATmega809 and ATmega808
GNU Lesser General Public License v2.1
239 stars 47 forks source link

correct TCA0.CTRLB (digitalRead) without TCA Split-Mode #179

Open mikrocoder opened 1 year ago

mikrocoder commented 1 year ago

Hello,

Initial situation: Configuration with your MegaCoreX package, original Arduino Nano Every board, setting ATmega4809 with Every Pinout etc.

The timer TCA0 I have reprogrammed for own purposes, therefore for all TCA0 registers a complete reset performed to set then the own configuration. Dual Slope Mode Top with WOn on standard port B. All 3 WOx are on Arduino pin 9, 10 and 5. So far there is no problem. If you use for example pin 9 and 10 for the timer TCA and pin 5 as normal input and read it with digitalRead, then unfortunately the WGM bits in the CTRLB register are cleared and not the Compare Enable bits. The reason for this is that the MegaCoreX package still thinks that the split mode is active. But it is not. It can not know that. This is clear to me. But it is still not nice. The orignal Arduino Framework does not do that.

I have created a workaround for the MegaCoreX package. I have built in a query whether the Every Board with Every Pinout is active and accordingly affects the clearing of the bits. Depending on whether the splitmode is active or not. For this you have to change the function turnOffPWM() in the file wiring_digital.c.

digitalRead() should work correctly, because foreign librays only know digitalRead() and nothing about digitalReadFast().

Do you see a possibility to change this in your case?

static void turnOffPWM(const uint8_t pin)
{
  /* Actually turn off compare channel, not the timer */

  /* Get pin's timer */
  uint8_t timer = digitalPinToTimer(pin);
  if (timer == NOT_ON_TIMER)
    return;

  uint8_t bit_pos;
  TCB_t *timerB;

  switch (timer)
  {
    /* TCA0 */
    case TIMERA0:
      /* Bit position will give output channel */
      bit_pos = digitalPinToBitPosition(pin);

      /* Disable corresponding channel */
      #if defined (NANO_EVERY_PINOUT) && defined (NONA4809_PINOUT) && defined (__AVR_ATmegax09__)
          if ( 0x01 == (TCA0.SPLIT.CTRLD & 0x01) ) {                      // is Splitmode activ?
            if (bit_pos >= 3) ++bit_pos; /* Upper 3 bits are shifted by 1 */
            TCA0.SPLIT.CTRLB &= ~(1 << (TCA_SPLIT_LCMP0EN_bp + bit_pos)); // #define TCA_SPLIT_LCMP0EN_bp  0  /* Low Compare 0 Enable bit position. */
          } else {
            TCA0.SPLIT.CTRLB &= ~(1 << (TCA_SINGLE_CMP0EN_bp + bit_pos)); // #define TCA_SINGLE_CMP0EN_bp  4  /* Compare 0 Enable bit position. */
          }
      #else
          if (bit_pos >= 3) ++bit_pos; /* Upper 3 bits are shifted by 1 */
          TCA0.SPLIT.CTRLB &= ~(1 << (TCA_SPLIT_LCMP0EN_bp + bit_pos));   // #define TCA_SPLIT_LCMP0EN_bp  0  /* Low Compare 0 Enable bit position. */
      #endif
      break;

    /* TCB - only one output */
    case TIMERB0: /* FALLTHRU */  // requires gcc 7, modern syntax is [[fallthrough]];
    case TIMERB1: /* FALLTHRU */
    case TIMERB2: /* FALLTHRU */
    case TIMERB3: /* FALLTHRU */

      timerB = (TCB_t *)&TCB0 + (timer - TIMERB0);

      /* Disable TCB compare channel */
      timerB->CTRLB &= ~(TCB_CCMPEN_bm);

      break;
    default:
      break;
  }
}
SpenceKonde commented 1 year ago

My solution for this problem, which I encourage others to use is to provide a function takeOverTCA0()

I chose the numeric values for TIMERA0 and TIMERA1 (and TIMERD0) so that they would have a bit set specific to the timer (I think i used 0x10 for TCA0, 0x08 for TCA1. I think 0x40 for TCD0

And I have an internal variable that starts out storing a 1 in those three bits, I think I call it __peripheralstate or something like that

When analogWrite or turnOffPWM is called, it looks up the timer, and ends up doing something conceptuallty the same as if (!(timer & 0x51 & __peripheralstate)) { timer = NOT_ON_TIMER;}

calling takeOverTCA0() will disable it and issue the hard reset command, andclear the bit inthat variable. Thereafter the core functions are blind tothe fact that the timer exists,

mikrocoder commented 1 year ago

Hello,

this reads well. Do I need to write the takeOverTCA0() function myself or where is it hidden? If I want to call the function takeOverTCA0(); in my "TimerInit" function it is not declared.

SpenceKonde commented 1 year ago

It's a feature of my cores, not MegaCoreX. It should be a feature of MegaCoreX - That comment was aimed at MCUdude,

But at this point there are a lot of things that should be a feature of MegaCoreX and aren't...Both wire and serial have gotten two major remodelings, and you can move the TCA PWM ports around at will on DxC

mikrocoder commented 1 year ago

Hello,

now I realized that you are not MCUdude but SpenceKonde. :grin:
But you are already best buddies that you can demand that from MCUdude so? :grin: We wait to see what MCUdude has to say about digitalRead and TCA.