Category Archives: SAMD21

Using SAMD Emulated EEPROM ASF driver coming from the 8-bit micro world

Introduction

In a typical 8-bit microcontroller application with real EEPROM, I used to have to read EEPROM byte by byte to obtain values I want and store them into variables in the world of PIC16/18s (how awful they are). Or in the case of AVRs, I had the nice helper functions provided by <avr/eeprom.h>

Now the SAMD with it’s emulated EEPROM ASF driver, things suddenly get more interesting. Now when you read the “EEPROM”, you get 60 bytes back. Wow, all that in one read and now I have to step through bytes to create my variables?

Hell no, now I can use the power of structs, memcpy and magic!

My new usage of emulated EEPROM is to store structs directly to/from the emulated EEPROM pages. The only risk is if we create a struct greater than the page size but there’s an fix for that from the Linux kernel 😀

Implementation

Structs themselves define a physically grouped set of data, so all the bytes we want to store are roughly next to each other. (see Struct Padding below)
Now to start, say this is my EEPROM variable storage struct

struct settings
{
	uint8_t device_name[10];
	uint16_t serial_number;
	uint16_t voltage_output;
	uint16_t date_of_manufacture;
}

How do we store this struct into the emulated EEPROM?
Simple!

uint8_t page_data[EEPROM_PAGE_SIZE];
memcpy(page_data,&settings,sizeof(settings));

eeprom_emulator_write_page(0, page_data);
eeprom_emulator_commit_page_buffer();

This copies the settings struct as raw bytes into the page_data byte buffer and then saves it into page 0.

How do we read it back from emulated EEPROM?
Also simple!

	uint8_t page_data[EEPROM_PAGE_SIZE];
	eeprom_emulator_read_page(0, page_data);
	memcpy(&settings,page_data,sizeof(settings));

 Struct Padding

Because of compiler optimization, the variables will be padded to match the default size of the chip’s memory, i.e. 32-bits. Meaning there will be 2 “padding” bytes between device_name and serial_number to make 4 bytes. When your struct isn’t big and its being used internally for your firmware only then it isn’t a big deal to just leave.

Here’s an example of how it would turn out for the previous struct:

struct settings
{
	uint8_t device_name[10];  //10 bytes means 2 + 1/2 dword
	uint8_t padding1[2];        //1/2 dword
	uint16_t serial_number;    //16bit means 1/2 dword
	uint8_t padding2[2];         //1/2 dword
	uint16_t voltage_output;    //16bit means 1/2 dword
	uint8_t padding3[2];        //1/2 dword
	uint16_t date_of_manufacture;    //16bit means 1/2 dword
	uint8_t padding4[2];        //1/2 dword
}

However, when you start to exceed the 60 bytes limit you may want to force packing. Packing simply tells the compiler to smash all the bytes next to each other. The downside of this is optimization. The compiler will now have to recreate shorts, words, etc when you access them from the struct and they are split memory words. i.e. Memory word 1 and memory 2 might have the LSB and MSB of the serial_number struct define and the compiler must insert assembly to convert it to a single memory word 3 before usage. Without packing, it would already be stored as a single word.

So how do you pack?

 

struct settings
{
	uint8_t device_name[10];
	uint16_t serial_number;
	uint16_t voltage_output;
	uint16_t date_of_manufacture;
} __attribute__((__packed__));

Simply apply the gcc attribute informing it to do so like above.

I personally have not noticed any real performance deficiencies by using packed structs but its all application dependent on how often you access struct members, how the compiler packs struct members by chance (i.e. device_name gets split between words but voltage_output doesn’t), etc.

Struct Size Validation

This reads the page into the byte buffer and then we dump the byte buffer into the settings struct.
Why do I copy to a buffer instead of the struct directly? Because the driver will always try and return 60 bytes and there struct is not necessarily that big.

Now you may wonder, what happens if our struct is bigger than 60 bytes? Well, bad things will happen because memcpy is blind to that problem and other variables could be affected.

The worst case is maybe your struct is initially fine but then you later add onto it and cause it to grow past the page size unknowingly. It may then keep working or fail horribly in different conditions, the worse of which isn’t noticeable until some edge case. This is definitely something you do not want.

But there’s a solution!
The sizeof function is a operator that’ll normally compute the size of data types but it only works at compile-time. Sadly the pre-processor won’t be able to evaluate it and stop with a #error or #warning.
But,the Linux kernel has this useful macro in “include/linux/kernel.h”

#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))

This beautiful macro is a bit of magic that will let us do a sizeof check that will stop compiling. Incidentally, the macro uses a sizeof check that causes the build error.

So the beautiful solution is to shove
BUILD_BUG_ON( sizeof(settings) > EEPROM_PAGE_SIZE); somewhere the compiler will come across, when the sizeof the settings struct is greater than EEPROM_PAGE_SIZE, the compiler will emit an unrelated error but will point to the line that BUILD_BUG_ON is on.

This is the protection to make this implementation safe to use in your firmware. You can then fix the error by trimming your struct by adjusting the data type sizes of variables or splitting it into a new struct. You can ideally store a different struct per page.

I used this trick because I had around 90 end user configurable settings, I easily split them off into four different structs that could be saved into four different pages. This reduced the work for me from having to explicitly assign data from EEPROM to RAM variables on start-up and doing the reverse when I needed to save them. Now I just blindly store and read the struct.

ASF: SAMD ADC Callback Driver Risky Bug with Jobs

I encountered a rather dangerous bug when using a SAMD21 with the ASF ADC Callback driver as of ASF 3.20.1

In the callback driver, to start interrupts you would do

adc_register_callback(&adc_instance, adc_complete_callback, ADC_CALLBACK_READ_BUFFER);
adc_enable_callback(&adc_instance, ADC_CALLBACK_READ_BUFFER);

uint16_t adc_buffer[8] = {0};
adc_read_buffer_job(&adc_instance,adc_buffer,8);

after configuration of the ADC of course.

Then the ADC callback will be


/**
 * \brief ADC complete callback, stores ADC measurement
 *
 *	\param module	ADC Module
 */
void adc_complete_callback(const struct adc_module *const module)
{
	measurement = adc_buffer[0];
}

This will work just fine.

The bug comes into play with adc_abort_job

	adc_abort_job(&adc_instance,ADC_JOB_READ_BUFFER);

This will disable the callback and ADC interrupts like we want. Except if you look into it


void adc_abort_job(
		struct adc_module *module_inst,
		enum adc_job_type type)
{
	/* Sanity check arguments */
	Assert(module_inst);

	if (type == ADC_JOB_READ_BUFFER) {
		/* Disable interrupt */
		adc_disable_interrupt(module_inst, ADC_INTERRUPT_RESULT_READY);
		/* Mark job as aborted */
		module_inst->job_status = STATUS_ABORTED;
		module_inst->remaining_conversions = 0;
	}
}

It does not appear to cancel running conversions (which isn’t possible anyway).

Now why are running conversions important? Because they set the INTFLAG.RESRDY bit when finished. This bit does not require interrupts turned on to get set, it allows for polling based conversions instead of interrupts.

Now to continue.

static inline void adc_disable_interrupt(struct adc_module *const module_inst,
		enum adc_interrupt_flag interrupt)
{
	/* Sanity check arguments */
	Assert(module_inst);
	Assert(module_inst->hw);

	Adc *const adc_module = module_inst->hw;
	/* Enable interrupt */
	adc_module->INTENCLR.reg = interrupt;
}

Only the INTEN register gets modified in disable_interrupt which means interrupts will no longer generate which is okay.

Now the problem occurs when adc_read_buffer_job is called to start our ADC measurements again.

enum status_code adc_read_buffer_job(
....
	adc_enable_interrupt(module_inst, ADC_INTERRUPT_RESULT_READY);

	if(module_inst->software_trigger == true) {
		adc_start_conversion(module_inst);
	}

	return STATUS_OK;
}

adc_enable_interrupt only changes INTEN to turn on RESRDY:

adc_module->INTENSET.reg = interrupt;

adc_start_conversion only does:

adc_module->SWTRIG.reg |= ADC_SWTRIG_START;

And that’s how we have a bug.

The issue is, if
1. Conversion is running
2. adc_abort_job is called while a conversion is running, interrupts get disabled (that’s ok)
3. INTFLAG.RESRDY gets set because the conversion completed sometime later after #2
4. adc_read_buffer_job is called later in the code and enables interrupts
5. NVIC does not generate an interrupt as desired

Why does #5 happen? Because INTFLAG.RESRDY must be cleared before the NVIC can see the interrupt occur.

Fixing this right now is easy using ASF routines:

adc_clear_status(&adc_instance, ADC_STATUS_RESULT_READY);
which will gurantee RESRDY is cleared before starting a conversion.

before calling

adc_read_buffer_job(&adc_instance,adc_buffer,8);

I tried arguing with Atmel support with it but the current support rep doesn’t understand INTFLAG.RESRDY will be always be set regardless of INTEN which is just for the NVIC to generate interrupts.