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.

ASF: SAMD21 DPLL for internal clock from internal 8MHz.

The SAMD21 comes with a DPLL peripheral which allows scaling up an reference clock up from 48 to 96MHz variably. It can then be used as the source for the CLK_MAIN that runs the CPU.

This is a quick little guide on how to upscale the internal 8MHz clock to run the CPU clock at a higher frequency. This is particularly useful when needing performance and you are running the DFLL for USB clock recovery.

Currently ASF 3.19 and below are broken and do not allow configuring DPLL properly via only conf_clocks.h. The reason being you need to set the DPLL’s reference clock to a GCLK and system_clock_init fails to do so.

The first step to get the DPLL scaling your internal frequency is to configure a GCLK generator to output the internal 8MHZ oscillator. This can be any free available GCLK generator.

/* Configure GCLK generator 1 (DPLL Reference) */
#  define CONF_CLOCK_GCLK_1_ENABLE                true
#  define CONF_CLOCK_GCLK_1_RUN_IN_STANDBY        false
#  define CONF_CLOCK_GCLK_1_CLOCK_SOURCE          SYSTEM_CLOCK_SOURCE_OSC8M
#  define CONF_CLOCK_GCLK_1_PRESCALER             8
#  define CONF_CLOCK_GCLK_1_OUTPUT_ENABLE         false

The prescaler is set to 8 because the input to the DPLL must be 2MHz or lower. It is not rated for a higher input.

Now you must make the GCLK generator feed the DPLL GCLK channel


	/* Set up GCLK */
	struct system_gclk_chan_config gclk_chan_conf;
	system_gclk_chan_get_config_defaults(&gclk_chan_conf);
	gclk_chan_conf.source_generator = GCLK_GENERATOR_1;
	system_gclk_chan_set_config(SYSCTRL_GCLK_ID_FDPLL, &gclk_chan_conf);
	system_gclk_chan_enable(SYSCTRL_GCLK_ID_FDPLL);

Now you can enable the DPLL to start running

	struct system_clock_source_dpll_config config_dpll;
	system_clock_source_dpll_get_config_defaults(&config_dpll);
	config_dpll.reference_clock = SYSTEM_CLOCK_SOURCE_DPLL_REFERENCE_CLOCK_GCLK;
	config_dpll.reference_divider = 1;
	config_dpll.reference_frequency = 1000000;
	config_dpll.output_frequency = 30000000;
	system_clock_source_dpll_set_config(&config_dpll);

reference_frequency is set to the output frequency of GCLK_GENERATOR_1 used above

output_frequency can be any value from 48MHz to 96Mhz. It can go as low as 30Mhz in practice but it’s not specced for that by Atmel.

 

Next you must change the NVM controllers flash wait states according to the table in the SAMD21 datasheet
samd21-nvm-flash-wait-states

system_flash_set_waitstates(2);

Enable the DPLL

	system_clock_source_enable(SYSTEM_CLOCK_SOURCE_DPLL);

And finally switch the GCLK generator 0 which is the CPU clock source over to the DPLL

	struct system_gclk_gen_config config_gclock_gen;
	system_gclk_gen_get_config_defaults(&config_gclock_gen);
	config_gclock_gen.source_clock    = SYSTEM_CLOCK_SOURCE_DPLL;
	config_gclock_gen.division_factor = 1;
	config_gclock_gen.output_enable = true;
	system_gclk_gen_set_config(GCLK_GENERATOR_0, &config_gclock_gen);

Lastly, it is good to check that DPLL is running


 while(!system_clock_source_is_ready(SYSTEM_CLOCK_SOURCE_DPLL));