Monthly Archives: August 2014

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));

SAMA5: SMBus block reads throwing kernel panics and fixing it

So a month or two ago I was having trouble with Atmel’s SAMA5 microprocessor running Linux. Most of the time the entire system would work but there were times the system mysteriously kernel panic or freeze without warning. Actually finding the cause was a bit of adventure but started off when I broke a SMBus slave in the system which crashed the system immediately each time.

SMBus Block Reads

Normally, a SMBus block read is just a combination of two i2c operations with minor logic which consists of

  1. Write
    1. Transmit write address
    2. transmit command byte
  2. Read
    1. Transmit read address
    2. Read length byte
    3. Read x number of bytes based on length

This is a straight forward I2C operation especially on Linux where you could just combine write and read i2c messages. The lower-level i2c-core does this automatically with write and read message packets when the i2c_smbus_read_block_data function is used. The length byte is the defining item of SMBus blocks as this indicates the data length which can be variable and denotes where the optional checksum exists in the packet. The SMBus specification also defines the maximum block length is 32. It also notes that 0 is not a valid block length.

Digging through i2c-core

First thing I did was to stare off at the functions inside /drivers/i2c/i2c-core.c We were using the userspace interface to the i2c subsystem via ioctl. ioctl is basically a file descriptor wrapper for the kernel functions. The wrapper is located in i2c-dev.c as the function below.

static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
		unsigned long arg)
{
	struct i2c_smbus_ioctl_data data_arg;
	union i2c_smbus_data temp;
	int datasize, res;

....

	/* Note that command values are always valid! */

	if ((data_arg.size == I2C_SMBUS_QUICK) ||
	    ((data_arg.size == I2C_SMBUS_BYTE) &&
	    (data_arg.read_write == I2C_SMBUS_WRITE)))
		/* These are special: we do not use data */
		return i2c_smbus_xfer(client->adapter, client->addr,
				      client->flags, data_arg.read_write,
				      data_arg.command, data_arg.size, NULL);

The relevant part is i2c_smbus_xfer being called after some verification on the data and function being called. Then looking at i2c_smbus_xfer, it is going to call i2c_smbus_xfer_emulated because the i2c-at91 driver does not have hardware smbus. Now the xfer_emulated function does real work.

	case I2C_SMBUS_BLOCK_DATA:
		if (read_write == I2C_SMBUS_READ) {
			msg[1].flags |= I2C_M_RECV_LEN;
			msg[1].len = 1; /* block length will be added by
					   the underlying bus driver */
		} else {
...
		}

Ok so I2C_M_RECV_LEN is set when we have an smbus block read. But what does it do when we are done?

		case I2C_SMBUS_BLOCK_DATA:
		case I2C_SMBUS_BLOCK_PROC_CALL:
			for (i = 0; i < msgbuf1[0] + 1; i++)
				data->block[i] = msgbuf1[i];
			break;

It simply does a basic for loop to copy from msgbuf[1] to the data->block which is returned back to the higher level i2c_smbus_read_block_data function. This is where it got interesting, none of the i2c-core functions validated that the value returned from msgbuf1[0] was actually a valid value.   Huh. So the i2c bus driver is responsible for validating that the byte length is valid that gets sent up to i2c-core. That’s nice.

Digging through i2c-at91

I had to now examine the i2c-at91 driver. The function that gets called is at91_twi_xfer which validataes the i2c message struct passed is valid for the bus driver to execute. The only interesting bit is what the dev->buf and dev->buf_len variables get set to.

....
		if (msg->flags & I2C_M_RD) {
			dev_err(dev->dev, "first transfer must be write.\n");
			return -EINVAL;
		}
		if (msg->len > 3) {
			dev_err(dev->dev, "first message size must be <= 3.\n");
			return -EINVAL;
		}

		/* 1st msg is put into the internal address, start with 2nd */
		m_start = &msg[1];
		for (i = 0; i < msg->len; ++i) {
			const unsigned addr = msg->buf[msg->len - 1 - i];

			internal_address |= addr << (8 * i);
			int_addr_flag += AT91_TWI_IADRSZ_1;
		}
....
	dev->buf_len = m_start->len;
	dev->buf = m_start->buf;
	dev->msg = m_start;

	ret = at91_do_twi_transfer(dev);

now at91_do_twi_transfer does the bulk of the setup work in the i2c-at91 driver. Inside the driver we will enter this code path since we are doing a read.

} else if (dev->msg->flags & I2C_M_RD) {
		unsigned start_flags = AT91_TWI_START;

		if (at91_twi_read(dev, AT91_TWI_SR) & AT91_TWI_RXRDY) {
			dev_err(dev->dev, "RXRDY still set!");
			at91_twi_read(dev, AT91_TWI_RHR);
		}

		/* if only one byte is to be read, immediately stop transfer */
		if (dev->buf_len <= 1 && !(dev->msg->flags & I2C_M_RECV_LEN))
			start_flags |= AT91_TWI_STOP;
		at91_twi_write(dev, AT91_TWI_CR, start_flags);
		/*
		 * When using dma, the last byte has to be read manually in
		 * order to not send the stop command too late and then
		 * to receive extra data. In practice, there are some issues
		 * if you use the dma to read n-1 bytes because of latency.
		 * Reading n-2 bytes with dma and the two last ones manually
		 * seems to be the best solution.
		 */
		if (dev->use_dma && (dev->buf_len > AT91_I2C_DMA_THRESHOLD)) {
			at91_twi_read_data_dma(dev);
			/*
			 * It is important to enable TXCOMP irq here because
			 * doing it only when transferring the last two bytes
			 * will mask NACK errors since TXCOMP is set when a
			 * NACK occurs.
			 */
			at91_twi_write(dev, AT91_TWI_IER,
			       AT91_TWI_TXCOMP);
		} else
			at91_twi_write(dev, AT91_TWI_IER,
			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);

From that we can tell that we are NOT using the DMA because our buf_len is set to 1. This means it is purely interrupt based fetching. (i.e. TWI_RXRDY being enabled triggers the interrupt). Now to find the interrupt handler which is atmel_twi_interrupt

	else if (irqstatus & AT91_TWI_RXRDY)
		at91_twi_read_next_byte(dev);

Ok, lets look at at91_twi_read_next_byte

static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
{
	if (dev->buf_len <= 0)
		return;

	*dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff;
	--dev->buf_len;

	/* handle I2C_SMBUS_BLOCK_DATA */
	if (unlikely(dev->msg->flags & I2C_M_RECV_LEN)) {
		dev->msg->flags &= ~I2C_M_RECV_LEN;
		dev->buf_len += *dev->buf;
		dev->msg->len = dev->buf_len + 1;
		dev_dbg(dev->dev, "received block length %d\n", dev->buf_len);
	}

	/* send stop if second but last byte has been read */
	if (dev->buf_len == 1)
		at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);

	dev_dbg(dev->dev, "read 0x%x, to go %d\n", *dev->buf, dev->buf_len);

	++dev->buf;
}

O NO, it really isn’t doing what I think it is, is it??? Yes, it’s just accepting the first byte…blindly. This is going to go well, let’s test the theory!

Testing

I grabbed a package of tools called i2c-tools off github https://github.com/groeck/i2c-tools And quickly hacked it to do smbus_block_reads via block reads.

Now for some fun. I had an SMBus battery attached, it conforms for the most part to the SMBUS 1.1 spec.

So the first thing is validate my smbus read words, read address 0x08 and I expected voltage. Get back 0x0baa and the read is fine.

Then I move onto a valid read string command, read 0x20 and get back 8 bytes which make up a string. All is good.

Now to break read block! I noticed address 0x08 returned 0xAA as the LSB of the word in the first test. Perfect, let’s try read block on that.

Read block on 0x20…. KERNEL PANIC Yep, that’s bad.

Let’s reset and test one more thought, what if the byte length was 0?

Read command 0x00 which I knew would return 0 at the time….. System froze. Oooo, that explains alot.

Solution

In a perfect world, an SMBUS compliant device would always return valid data. Unfortunately that ideal is a stretch and thus data should always be validated. There can even be noise interfering with i2c communications in some systems that may make data unreliable (but there are checksums to take care of that issue).

Implementing a fix was relatively easy, there was just one note about how the I2C peripheral works in the SAMA5. The SAMA5 TWI peripheral has a sort of “eager” approach to i2c handling, where conditions like ACKs/NACKs and stops must be set the interrupt before the last data byte. It can’t just be stopped arbitrarily.  The solution was just to modify the function to validate length and just read one more byte(which is discarded).   Patch sent to mailing lists and maintainers.

http://www.spinics.net/lists/arm-kernel/msg355834.html