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

digitalPinHasPWM and EXTERNAL_NUM_INTERRUPTS macros wrong #66

Closed
SpenceKonde opened this issue Apr 21, 2020 · 12 comments
Closed

digitalPinHasPWM and EXTERNAL_NUM_INTERRUPTS macros wrong #66

SpenceKonde opened this issue Apr 21, 2020 · 12 comments

Comments

@SpenceKonde
Copy link
Contributor

for 28 and 32 pin parts, digitalPinHasPWM is wrong - it doesn't account for TCA0 being in SPLIT_MODE.

For all parts, EXTERNAL_NUM_INTERUPTS is set to 47. This should be set to the number of digital pins (which is never greater than 41!). Even assuming no user code relies on this, it still makes WInterrupts allocate an excessively sized array to store user functions, even when said functions could never exist.

@MCUdude
Copy link
Owner

MCUdude commented Apr 21, 2020

for 28 and 32 pin parts, digitalPinHasPWM is wrong - it doesn't account for TCA0 being in SPLIT_MODE.

Thanks, I will fix this.

For all parts, EXTERNAL_NUM_INTERUPTS is set to 47. This should be set to the number of digital pins (which is never greater than 41!). Even assuming no user code relies on this, it still makes WInterrupts allocate an excessively sized array to store user functions, even when said functions could never exist.

Actually, I'm pretty sure the interrupt number should be 48 instead. The default Arduino implementation assumes that all ports have 8 pins, and that all pins are available. 8*6 = 48.
I opened an issue about the interrupt implementation here, but I never got much response.

@SpenceKonde
Copy link
Contributor Author

Oh FFS did they really do that.... /facepalm

Ugh.

47 is correct in that case, as there's no PF7.

@MCUdude
Copy link
Owner

MCUdude commented Apr 21, 2020

for 28 and 32 pin parts, digitalPinHasPWM is wrong - it doesn't account for TCA0 being in SPLIT_MODE.

Just looked at this, and can't find anything wrong really.

The 28-pin parts only have PC0-PC3 broken out, and gives us only four PWM pins.
The 32-pin parts have PC0-PC3 + PF4 and PF5. This gives us six PWM pins.

@MCUdude
Copy link
Owner

MCUdude commented May 3, 2020

@SpenceKonde can you confirm this?

@SpenceKonde
Copy link
Contributor Author

You have to select a port for the PWM output, see
15.3.5 PORTMUX Control for TCA

I can't fathom why you'd pick a port that doesn't have all six pins available though?

@MCUdude
Copy link
Owner

MCUdude commented May 3, 2020

Yes, of course, I should probably use a different port on these chips. Which ports would you prefer to use on these parts, and for your ongoing project?

@MCUdude
Copy link
Owner

MCUdude commented May 3, 2020

Perhaps PORTD, since useful peripherals such as UART0, i2c, and SPI are by default located on PORTA?

@MCUdude
Copy link
Owner

MCUdude commented May 3, 2020

OK, this will be the new timer routing for the 28 and 32 pin parts. @SpenceKonde do you agree that this is the most compatible one?

// Timer pin mapping from pins_arduino.h
#define TCA0_PINS PORTMUX_TCA0_PORTD_gc // TCA0 output on PD[0:5]
#define TCB0_PINS 0x00                  // TCB0 output on PA2 instead of PF4
#define TCB1_PINS 0x00                  // TCB1 output on PA3 instead of PF5
#define TCB2_PINS 0x00                  // TCB2 output on PC0 instead of PB4
#define TCB3_PINS PORTMUX_TCB3_bm       // TCB3 output on PC1 instead of PB5

@SpenceKonde
Copy link
Contributor Author

I think PORTD makes the most sense for TCA0 yeah... looking at the TCBs....

@MCUdude
Copy link
Owner

MCUdude commented May 3, 2020

Thanks. the 32-pin parts do have PF4 and PF4, but I'd rather prefer to use PA2 and PA3 instead, to be consistent with the 28-pin parts.

@SpenceKonde
Copy link
Contributor Author

I would rather like to get the TCB PWM pins onto PORTF instead of PORTA - though it saddens me to think of a type B timer reduced to generating 8-bit PWM no matter what pin it's outputting on....

@MCUdude
Copy link
Owner

MCUdude commented May 3, 2020

I would rather like to get the TCB PWM pins onto PORTF instead of PORTA

The problem is that the 28-pin parts only have PF0, PF1, and PF6. This means you can't route any timers to PORTF on this part.

I will assume people will migrate from a 28-pin DIP part to the 32-pin TQFP part. It would be great if these two packages were compatible, just like the ATmega328 in DIP and TQFP is.

MCUdude added a commit that referenced this issue May 3, 2020
@MCUdude MCUdude closed this as completed May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants