Hi
First of all, bitfields are well-defined though implementation dependent, this means that both the architecture and the compiler affects how a bitfield behaves. This is especially true when writing portable code shared between little and big endian architectures. The key thing is that they are well defined for each compiler/architecture.
Anyway, I find it convenient and less error prone to use bitfields, like for the DMA configuration. Also, if using named constants, the amount of comments required are also decreased. Less comments result in less risk of divergences between code and comments
A few suggestions, what does the community think?
Change the DMA_CONFIG type to a bitfield, found last in
github.com/pololu/wixel-sdk/blo … 2511_map.h
Suggestion is
// Endianess conversion for 8051 <-> DMA controller
#define DMA_ADDR(a) (((uint16_t) (a) << 8) | ((uint16_t) (a) >> 8))
// Note the reverse order of the bitfields in each byte!
struct DmaConfig {
unsigned SRCADDR : 16;
unsigned DESTADDR : 16;
unsigned LENH : 5;
unsigned VLEN : 3;
unsigned LENL : 8;
unsigned TRIG : 5;
unsigned TMODE : 2;
unsigned WORDSIZE : 1;
unsigned PRIORITY : 2;
unsigned M8 : 1;
unsigned IRQMASK : 1;
unsigned DESTINC : 2;
unsigned SRCINC : 2;
};[/code]
example code would look like this, configuring the DMA channel 3
[code] // DMA Rx
dmaConfig[3].SRCADDR = DMA_ADDR(XDATA_SFR_ADDRESS(U0DBUF));
dmaConfig[3].DESTADDR = DMA_ADDR(dmaRxPacket);
// we simplify things by having a fixed SPI buffer size
dmaConfig[3].LENL = ((sizeof(dmaRxPacket) >> 0) & 0xff) + 0;
dmaConfig[3].TMODE = DMA_TransSingle;
dmaConfig[3].TRIG = DMA_TrigURX0;
dmaConfig[3].SRCINC = DMA_Inc0;
dmaConfig[3].DESTINC = DMA_Inc1;
dmaConfig[3].PRIORITY = DMA_PrioHigh;
Change the dmaConfig variable to vector
Code found last in
github.com/pololu/wixel-sdk/blo … lude/dma.h
Suggestion
This means that initialization (done in library) would look like this
github.com/pololu/wixel-sdk/blo … /dma/dma.c
[code]DmaConfig XDATA dmaConfig[5];
void dmaInit()
{
DMA0CFG = (uint16)(dmaConfig + 0);
DMA1CFG = (uint16)(dmaConfig + 1);
}[/code]
Finally, using named constants for the different bit values for the DMA configuratino fields, Suggestion:
[code]enum {
DMA_TrigNONE = 0,
DMA_TrigPREV = 1,
DMA_TrigT1_CH0 = 2,
DMA_TrigT1_CH1,
DMA_TrigT1_CH2,
DMA_TrigT2_OVFL = 6,
DMA_TrigT3_CH0 = 7,
DMA_TrigT3_CH1,
DMA_TrigT4_CH0,
DMA_TrigT4_CH1,
DMA_TrigIOC_0 = 12,
DMA_TrigIOC_1,
DMA_TrigURX0 = 14,
DMA_TrigUTX0,
DMA_TrigURX1,
DMA_TrigUTX1,
DMA_TrigFLASH = 18,
DMA_TrigRADIO = 19,
DMA_TrigADC_CHALL = 20,
DMA_TrigADC_CH0,
DMA_TrigADC_CH1,
DMA_TrigADC_CH2,
DMA_TrigADC_CH3,
DMA_TrigADC_CH4,
DMA_TrigADC_CH5,
DMA_TrigADC_CH6,
DMA_TrigADC_CH7,
DMA_TrigENC_DW = 29,
DMA_TrigENC_UP = 30,
};
enum {
DMA_Inc0 = 0b00, // Incremented by 0
DMA_Inc1 = 0b01, // Incremented by 1
DMA_Inc2 = 0b10, // Incremented by 2
DMA_Dec1 = 0b11, // Decremented by 1
};
enum {
DMA_LengthVLEN = 0b000, // Length is LENL and LENH
DMA_LengthN0 = 0b010, // First byte + 0
DMA_LengthN1 = 0b001, // First byte + 1
DMA_LengthN2 = 0b011, // First byte + 2
DMA_LengthN3 = 0b100, // First byte + 3
};
enum {
DMA_PrioLOW = 0,
DMA_PrioNormal = 1,
DMA_PrioHigh = 2,
};
enum {
DMA_TransSingle = 0b00,
DMA_TransBlock = 0b01,
DMA_TransRepeatSingle = 0b10,
DMA_TransRepeatBlock = 0b11,
};[/code]
br
Mattias