ArduinoCore-megaavr icon indicating copy to clipboard operation
ArduinoCore-megaavr copied to clipboard

Arduino Wifi Rev2, bug when reading duty cycle value on pin 6

Open landret opened this issue 4 years ago • 3 comments
trafficstars

Hi, I am trying to read the duty cycle set for pin 6 on the Arduino Wifi Rev2. By default, the timer used is TCB0, used in 8-bit PWM mode. The 16-bit Compare channel CCMP is used as two 8-bit registers, the lowest byte CCMPL is for the period, while the highest byte CCMPH is for the duty cycle.

However, something weird happens when I try to read the Compare channel register, the value of CCMPH is unexpecteadly copied into CCMPL. The same happens with pin 3 (which uses TCB1).

// PWM on pin 6, timer TCB0
//TCB_t* timer_B0 = (TCB_t *)&TCB0; // pointer on timer structure

void setup() {
  pinMode(6, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  setPWMValueOnPin6(20);
  setPWMValueOnPin6(40);
  setPWMValueOnPin6(60);
  setPWMValueOnPin6(80);
}

void printCompareChannelValues()
{
  Serial.print("Compare channel = ");
  unsigned int r = TCB0_CCMP;
  //byte r = timer_B0->CCMP;
  Serial.println(r);
  
  Serial.print("Compare channel L = ");
  byte l = TCB0_CCMPL;
  //byte l = timer_B0->CCMPL;
  Serial.println(l);
  
  Serial.print("Compare channel H = ");
  byte h = TCB0_CCMPH;
  //byte h = timer_B0->CCMPH;
  Serial.println(h);
}

void setPWMValueOnPin6(byte value)
{
  analogWrite(6, value);
  printCompareChannelValues();
  delay(2000);
}

Here is the serial monitor output:

Compare channel = 5375 Compare channel L = 255 Compare channel H = 20 Compare channel = 10260 Compare channel L = 20 Compare channel H = 40 Compare channel = 15400 Compare channel L = 40 Compare channel H = 60 Compare channel = 20540 Compare channel L = 60 Compare channel H = 80 Compare channel = 5200 Compare channel L = 80 Compare channel H = 20 Compare channel = 10260 Compare channel L = 20 Compare channel H = 40 Compare channel = 15400 Compare channel L = 40 Compare channel H = 60 Compare channel = 20540 Compare channel L = 60 Compare channel H = 80 Compare channel = 5200 Compare channel L = 80 Compare channel H = 20

This occurs even when removing the serial functions (I can see that by watching a led connected on pin 6). Is that a normal behavior?

landret avatar Dec 05 '20 19:12 landret

The origin of the problem has been found and lies in the analogWrite() function:

/* set duty cycle */
timer_B->CCMPH = val;

Please see details and solutions proposed in this discussion on Arduino Stack Exchange.

To summarize the discussion and according to "Accessing 16-Bit Registers" of the ATMega4809 datasheet

  • writing/reading to a 16-bit register on an atmega4809 should be done in 2 steps, as it involves the use of a dedicated temporary register when using an 8-bit data bus.

    • ideal writing sequence:

      1. write the low byte => actually write value to the temporary register

      2. write the high byte => also copy the content of the temporary register into the low byte.

    • ideal reading sequence:

      1. read the low byte => also copy the high byte into the temporary register

      2. read the high byte => actually read value from the temporary register

  • as the temporary register used for writing and reading is the same, writing or reading may be corrupted if the previous writing or reading sequence is incomplete (2 steps)

In our case, the write sequence is incomplete and here is what happens:

  • at the end of a reading sequence, the temporary register holds the high byte value.
  • when writing to the high byte of the register only, the content of the temporary register (holding the high byte value) is copied into the low byte of the register.

It results that the high byte value is copied into the low byte value...

To circumvent this, a patch to the analogWrite() function is proposed in the discussion on Stack Exchange. I updated the title of this post as I think it is a bug which should be corrected.

landret avatar Dec 14 '20 20:12 landret

@landret thanks for the extensive clarification. Would you or @timemage for Stack Exchange post a PR for proper attribution? Thanks!

facchinm avatar Dec 22 '20 08:12 facchinm

It is worth noting that this behavior when a TCB is configured for 8-bit PWM - despite the fact that it is hardly news - has recently begun showing up on the Silicon Errata sheets. for 4809: https://ww1.microchip.com/downloads/en/DeviceDoc/ATmega4808-09-SilConErrataClarif-DS80000867B.pdf They say "The errata described in this document will likely be addressed in future revisions of the ATmega4808/4809 devices."

They give no indication of whether this will occur in our lifetime - I can't say the history of the modern AVR or "megaavr" architecture leaves me terribly optimistic.

SpenceKonde avatar Apr 09 '21 17:04 SpenceKonde