View Issue Details

IDProjectCategoryView StatusLast Update
0032339FPCRTLpublic2019-04-18 11:47
ReporterChristo CrauseAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version3.1.1Product Build36879 
Target VersionFixed in Version 
Summary0032339: AVR - Incorrect SPI clock rate bit constant names in some microcontroller units
DescriptionIn some microcontroller unit files the constants for the bit positions SPR1 and SPR0 are incorrectly specified as:

SPR = 0; // SPI Clock Rate Selects

Typically two bits are used to select the clock frequency for SPI:
SPR1 = 1; // SPI Clock Rate Selects
SPR0 = 0; // SPI Clock Rate Selects

I have fixed these constants everywhere I could find "SPR = 0", see attached patch.
TagsAVR
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Christo Crause

2017-08-26 19:32

reporter  

avrrtlpatch.dif (52,179 bytes)

Jeppe Johansen

2017-12-29 12:38

developer   ~0105103

I don't think this change is a good idea. The current format of the constants encode bit positions with the constant pointing to the index of the LSB of multi-bit fields.

Ex. WGM0 in TCCR0A

Christo Crause

2017-12-31 07:38

reporter   ~0105177

I noticed this issue because I was translating a C example from the atmega328p datasheet (end of section 18.2):

/* Enable SPI, Master, set clock rate fck/16 */
SPCR = (1<<SPE)|(1<<MSTR)|(1<<SPR0);

It is confusing for me if the naming convention mostly follow avr-gcc / Atmel, but then in a few cases differ. I think that compatibility with datasheet examples is important.

It is still possible to set two consecutive bits with one shift instruction, by referring to bit 0:
  SPCR := 3 shl SPR0; // clock div 128

Christo Crause

2017-12-31 07:51

reporter   ~0105178

After this issue was unanswered for a while, I decided to play around with importing the atdf files. After a very interesting discussion on the Lazarus forum (http://forum.lazarus-ide.org/index.php/topic,38809.0.html) I added bitpacked record and set definitions of the peripheral registers to the processor definition files, in addition to the regular bit constants.

Not sure what the core team's opinion is ?

Issue History

Date Modified Username Field Change
2017-08-26 19:32 Christo Crause New Issue
2017-08-26 19:32 Christo Crause File Added: avrrtlpatch.dif
2017-08-26 19:32 Christo Crause Tag Attached: AVR
2017-12-29 12:38 Jeppe Johansen Note Added: 0105103
2017-12-31 07:38 Christo Crause Note Added: 0105177
2017-12-31 07:51 Christo Crause Note Added: 0105178