Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PWM on digital pin 2 acts strange #29

Open
MCUdude opened this issue Feb 11, 2018 · 11 comments
Open

PWM on digital pin 2 acts strange #29

MCUdude opened this issue Feb 11, 2018 · 11 comments

Comments

@MCUdude
Copy link
Contributor

MCUdude commented Feb 11, 2018

Hi!
I'm working on an implementation for the ATmega48PB/88PB/168PB/328PB family and stumbled upon an issue I want to discuss. First you'll have to merge the pending PR in order to reproduce the issue.

The issue is PWM on digital pin 2 (PD2). It doesn't matter if TIMER3B or TIMER4B is used (in pins_arduino.h), the problem is the same. When running the Fade.ino example, the pin only "fades down" from 255-0, not up again like it should. If I change the boundaries from 0->10 and 255->200, it won't output any signal. I've attached my oscilloscope to verify this.

My implementation uses its own Arduino core files, but I haven't been able to figure out what's wrong. Any idea?

Here's a GIF to show you what's going on:
https://ephmedia.giphy.com/69aeb183-b0bc-440e-b60c-faa4d751e3c3.gif

@MCUdude
Copy link
Contributor Author

MCUdude commented Feb 11, 2018

A little more testing shows that this code works fine:

uint16_t ledPin = 2;

void setup() 
{
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, HIGH);
}

void loop() 
{
  for(uint16_t i = 1; i < 256; i++)
  {
    analogWrite(ledPin, i);
    delay(10);
  }
    
  for(uint16_t j = 255; j > 0; j--)
  {
    analogWrite(ledPin, j);
    delay(10);
  }
}

However if I uncomment digitalWrite(ledPin, HIGH); it won't count up the first time.
If I change the initial value in the first for loop to uint16_t i = 0;, it won't count up at all.

It seems like this is related to when analogWrite(ledPin, 0) is executed, which actually is just digitalWrite(ledPin, LOW)

@awatterott
Copy link
Member

I had a short look on the functions, but I also see no problem.
The timers are initialized in init() from wiring.c and the OC outputs are activated with analogWrite().
A digitalWrite(pin,val) will deactivate the OC output, because turnOffPWM(pin) is executed.

@MCUdude
Copy link
Contributor Author

MCUdude commented Feb 11, 2018

I suspect there might be a bug in the new and experimental toolchain (1.6.200).
I'm not able to print numbers using Serial.println. You can read my post about it here: arduino/Arduino#6899.

Too bad the 328PB isn't supported by the older toolchain (1.6.20).

@awatterott
Copy link
Member

There seems to be a hardware bug on timer 3+4 and the port register has to be written to 1 in order to get the output compare working: http://www.avrfreaks.net/comment/1717946#comment-1717946
So if you run analogWrite(ledPin, 0) then the output compare (pwm) is deactivated on.

I have also tested the serial number printing and it is working with 1.6.200 for me.

@MCUdude
Copy link
Contributor Author

MCUdude commented Feb 11, 2018

OK. Seems like a workaround is necessary in order to get PWM pin pin 2 working properly. I think the fix below should be OK. I only own one ATmega328PB chip, with date code 1550. Do you know if this silicon error have been fixed in newer chips?

      #if defined(TCCR3A) && defined(COM3B1)
      case TIMER3B:
        // connect pwm to pin on timer 3, channel B
        PORTD |= _BV(PD2);
        sbi(TCCR3A, COM3B1);
        OCR3B = val; // set pwm duty
        break;
      #endif

 #if defined(TCCR4A) && defined(COM4B1)
      case TIMER4B:
        // connect pwm to pin on timer 4, channel B
        PORTD |= _BV(PD2);
        sbi(TCCR4A, COM4B1);
        OCR4B = val; // set pwm duty
        break;
      #endif

@awatterott
Copy link
Member

I have tested it with one of our boards and I have to check the date code, but I think it is newer. In the datasheet (errata) is also nothing about it.
I would suggest to apply the workaround only for the 328PB and to use the sbi() macro:

#if defined(__AVR_ATmega328PB__)
        sbi(PORTD, PD2);
#endif

@MCUdude
Copy link
Contributor Author

MCUdude commented Feb 11, 2018

Why use the sbi macro? It generates the same assembly code as "regular" bitwise operations. As far as I know the earlier version of the gcc tool chain wasn't properly optimized for the AVR architecture, and didn't "understood" that bitwise operations could be converted into sbi and cbi instructions.

Here's an avrfreaks thread about the topic: http://www.avrfreaks.net/forum/sbi-cbi

In fact the deprecated.h contains:
#define sbi(port, bit) (port) |= (1 << (bit))
and
#define cbi(port, bit) (port) &= ~(1 << (bit))
It no longer needs to use inline asm() as it knows that |= and &=~ will invoke usage of SBI and CLI

@awatterott
Copy link
Member

Because it is used in the Arduino source code and the chance that a pull-request is accepted is better, when the coding style/formatting is the same.

@MCUdude
Copy link
Contributor Author

MCUdude commented Feb 11, 2018

Well, good point..

@awatterott
Copy link
Member

The date code of my mega328PB where the issue also exists is 1619.

@jclxx
Copy link

jclxx commented Jun 3, 2023

I have had the same issue but solved it and have the explanation. The fix is trivial, finding it took three days.

Summary

The "output compare modulator" is used when either output compare OC3B or OC4B or both is enabled. The way the datasheet describes it suggests the modulator is enabled only when both are enabled. The fix is trivial: set PD2 to 1 to select the "OR" function of the modulator's multiplexer.

There is a line in the datasheet which says this, but it's actually pretty well hidden and I didn't find it until writing this note. Even if you noticed the section on Output Modulation, you might not have read it in detail if you weren't intending to use it. There is no bug except in the documentation.

Explanation

Timer 3 and Timer 4 each has two output compare registers, A and B. OC3A and OC4A have their own pins 30 and 31 respectively, shared with PD0 and PD1. OC3B and OC4B outputs share the same pin, 32, also with PD2.

Enabling the output comparison pins is done with the COM3B and COM4B bits. If you enable both of them, this makes two (internal) signals OC3B and OC4B, controlled by the Waveform Generation Mode bits WGM3 and WGM4. The output on pin 32 is the modulation of the two, chosen as 0C3B & OC4B when PD2 is 0, OC3B | OC4B when PD2 is 1.

The obvious way to get, say, simple PWM from OC3B on pin 32 is to set WGM3=14 (fast PWM noninverting), COM3B = 2 (clear output on match, set on 0), DDRD2=1 (output), and OC4B=0 because it's not in use.

This is the surpise: It appears that when COM4B=0, signal OC4B is effectively zero, but the modulator is still in effect. Thus to get the desired OC3B on pin 32, the "OR" function must be selected.by setting PD2 to 1. The same is true with COM4B=0, to get OC3B on pin 32, set PD2 to 1.

References: ATMega328PB datasheet DS40001906 rev C Feb 2018.

See figure 22.2, p247. What I've called OC3B and OCR4B are the outputs of their respective AND gates inside the grey boxes TC3 and TC4. Notice the only path for these two signals towards the output buffer is through the modulator.

The following is the description given at Section 22.2 for the modulator with my edits of what I believe it should say.

The Output Compare unit 3B and Output Compare unit 4B shares the PD2 port pin for output. The
outputs of the Output Compare units (OC3B and OC4B) overrides the normal PORTD2 Register when
one of them is enabled (that is, when COMnx[1:0] is not equal to zero). When either or both OC3B and OC4B are
enabled at the same time, it will also enable this modulator.

The functional equivalent schematic of the modulator is shown in the figure below. The schematic
includes part of the Timer/Counter units and the port D pin 2 output driver circuit. When the modulator is
enabled the type of modulation (logical AND or OR) can be selected by setting the PORTD2 Register as
'0' for AND, ‘1’ for OR.

The line at 19.11.8 TC3 Control Register A and 19.11.15 TC4 Control Register A (p198) are very easily missed, and should be a note in the tables 19.8, 19.13 etc.

For OC3B or OC4B when not using the Output Compare Modulator, PORTD2 must also be set in order to
enable the output.

Honestly it's a great feature, but it really needs flagging!

Testing

Tested on bare ATMega328PB datecode 2239 with the following:

No output on Pin 32

DDRD=04 PORTD=00
TCCR3A=82 TCCR3B=19 TCCR3C=00 ICR3=ffff OCR3A=0000 OCR3B=0000
T3: (A=10,B=00,W=1110,N=0,E=0,S=001,F=00)
TCCR4A=a2 TCCR4B=19 TCCR4C=00 ICR4=ffff OCR4A=0000 OCR4B=8000
T4: (A=10,B=10,W=1110,N=0,E=0,S=001,F=00)

Output on Pin 32

DDRD=04 PORTD=04
TCCR3A=82 TCCR3B=19 TCCR3C=00 ICR3=ffff OCR3A=0000 OCR3B=0000
T3: (A=10,B=00,W=1110,N=0,E=0,S=001,F=00)
TCCR4A=a2 TCCR4B=19 TCCR4C=00 ICR4=ffff OCR4A=0000 OCR4B=8000
T4: (A=10,B=10,W=1110,N=0,E=0,S=001,F=00)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants