SPI driver for KINETIS K20.

ChibiOS public support forum for topics related to the Freescale Kinetis family of micro-controllers.

Moderator: utzig

SpaceCoaster
Posts: 49
Joined: Mon Aug 11, 2014 6:40 am

SPI driver for KINETIS K20.

Postby SpaceCoaster » Thu Aug 21, 2014 8:53 pm

Hi,

Attached are three patches.

1. SPI low level driver for the KINETIS K20 processors.
2. Board files for the MCHCK board with a KINETIS MK20DX128 processor.
3. SPI demo program for the MCHCK board driving an AT45DB081 data flash.

The SPI driver for master mode, is restricted to 8 bit and it doesn't use DMA. This SPI device is very different from the SPI device used in the KL series processors and the driver would not be common between the two series.

I am quite pleased with the way it worked out. Chibios and the chip played nicely together.

Cheers,
Derek
Attachments
0003-KINETIS-SPI-demo-for-MCHCK-K20-board.patch.gz
(8.47 KiB) Downloaded 91 times
0002-KINETIS-Support-for-MCHCK-K20-board.patch.gz
(2.16 KiB) Downloaded 74 times
0001-KINETIS-SPI-driver.patch.gz
(8.16 KiB) Downloaded 96 times

colin
Posts: 149
Joined: Thu Dec 22, 2011 7:44 pm

0001-KINETIS-SPI-driver.patch review

Postby colin » Thu Aug 21, 2014 10:16 pm

All right, awesome to see more Kinetis development. Thanks. Here's a quick patch review.

0001-KINETIS-SPI-driver.patch

Good to see the non-ASCII copyright symbol and curly quotes in the file header comment replaced with proper ASCII characters :) . The non-ASCII symbols caused minor annoyances for me before.

Probably should remove the old commented-out #define constant for SPI0

Code: Select all

+//#define SPI0            (*(KINETISK_SPI_t *)0x4002C000)

since the proper SPI0 pointer is defined earlier:

Code: Select all

#define SPI0                    ((SPI_TypeDef *)     SPI0_BASE)


and also delete the commented-out lines for register pointers that are not used since we use SPI0->regname instead of SPI0_regname. I think all the following lines in the patch should be removed: (note these lines are note all consecutive)

Code: Select all

+/***********  Bits definition for SPIx_MCR register  *************/
+//#define SPI0_MCR        (*(volatile uint32_t *)0x4002C000) // DSPI Module Configuration Register
+//#define SPI0_TCR        (*(volatile uint32_t *)0x4002C008) // DSPI Transfer Count Register
+//#define SPI0_CTAR0      (*(volatile uint32_t *)0x4002C00C) // DSPI Clock and Transfer Attributes Register, In Master Mode
+//#define SPI0_CTAR0_SLAVE    (*(volatile uint32_t *)0x4002C00C) // DSPI Clock and Transfer Attributes Register, In Slave Mode
+//#define SPI0_SR         (*(volatile uint32_t *)0x4002C02C) // DSPI Status Register
+//#define SPI0_RSER       (*(volatile uint32_t *)0x4002C030) // DSPI DMA/Interrupt Request Select and Enable Register
+//#define SPI0_PUSHR      (*(volatile uint32_t *)0x4002C034) // DSPI PUSH TX FIFO Register In Master Mode
+//#define SPI0_PUSHR_SLAVE    (*(volatile uint32_t *)0x4002C034) // DSPI PUSH TX FIFO Register In Slave Mode
+
+/***********  Bits definition for SPIx_POPR register  *************/
+//#define SPI0_POPR       (*(volatile uint32_t *)0x4002C038) // DSPI POP RX FIFO Register
+
+/***********  Bits definition for SPIx_TXFRn register  *************/
+//#define SPI0_TXFR0      (*(volatile uint32_t *)0x4002C03C) // DSPI Transmit FIFO Registers
+//#define SPI0_TXFR1      (*(volatile uint32_t *)0x4002C040) // DSPI Transmit FIFO Registers
+//#define SPI0_TXFR2      (*(volatile uint32_t *)0x4002C044) // DSPI Transmit FIFO Registers
+//#define SPI0_TXFR3      (*(volatile uint32_t *)0x4002C048) // DSPI Transmit FIFO Registers
+
+/***********  Bits definition for SPIx_RXFRn register  *************/
+//#define SPI0_RXFR0      (*(volatile uint32_t *)0x4002C07C) // DSPI Receive FIFO Registers
+//#define SPI0_RXFR1      (*(volatile uint32_t *)0x4002C080) // DSPI Receive FIFO Registers
+//#define SPI0_RXFR2      (*(volatile uint32_t *)0x4002C084) // DSPI Receive FIFO Registers
+//#define SPI0_RXFR3      (*(volatile uint32_t *)0x4002C088) // DSPI Receive FIFO Registers
 



os/hal/ports/KINETIS/K20x/spi_lld.h:
I think it's not correct to say that these settings are "24 MHz" or "27 kHz" in the LLD, since it's really just a divisor for the microcontroller clock, which frequency may vary depending on the target configuration. Right?
You could call it "KINETIS_SPI_TAR_24MHZ_FOR_48MHZ_SYSCLK" or "KINETIS_SPI_TAR_SYSCLK_DIV_2" or something, if I understand correctly.

Code: Select all

+/* TAR settings for n bits at 24 Mhz */
+#define KINETIS_SPI_TAR_24MHZ(n)    SPIx_CTARn_FMSZ((n) - 1) | \
+    SPIx_CTARn_CPOL | SPIx_CTARn_CPHA | \
+    SPIx_CTARn_DBR | SPIx_CTARn_PBR(0b00) | SPIx_CTARn_BR(0b0000) | \
+    SPIx_CTARn_CSSCK(0b0000) | SPIx_CTARn_ASC(0b0000) | SPIx_CTARn_DT(0b0000)
+
+/* TAR settings for n bits at 27kHz for debugging */
+#define KINETIS_SPI_TAR_SLOW(n)     SPIx_CTARn_FMSZ(((n) - 1)) | \
+    SPIx_CTARn_CPOL | SPIx_CTARn_CPHA | \
+    SPIx_CTARn_DBR | SPIx_CTARn_PBR(0b11) | SPIx_CTARn_BR(0b1001) | \
+    SPIx_CTARn_CSSCK(0b1011) | SPIx_CTARn_ASC(0b0111) | SPIx_CTARn_DT(0b1011)
+
+#define KINETIS_SPI_TAR_8BIT_FAST   KINETIS_SPI_TAR_24MHZ(8)
+#define KINETIS_SPI_TAR_8BIT_SLOW   KINETIS_SPI_TAR_SLOW(8)
+
+#define KINETIS_SPI_TAR0_DEFAULT    KINETIS_SPI_TAR_24MHZ(8)
+#define KINETIS_SPI_TAR1_DEFAULT    KINETIS_SPI_TAR_24MHZ(8)


Probably should be use hexadecimal literals instead of non-standard gcc binary literals in this code. And note, there's no need for extra parentheses around a literal constant in a #define.

Code: Select all

+#define KINETIS_SPI_PCS_NONE        (0b00000)
+#define KINETIS_SPI_PCS0            (0b00001)
+#define KINETIS_SPI_PCS1            (0b00010)
+#define KINETIS_SPI_PCS2            (0b00100)
+#define KINETIS_SPI_PCS3            (0b01000)
+#define KINETIS_SPI_PCS4            (0b10000)
+#define KINETIS_SPI_PCS_ALL         (0b11111)

colin
Posts: 149
Joined: Thu Dec 22, 2011 7:44 pm

0002-KINETIS-Support-for-MCHCK-K20-board.patch review

Postby colin » Thu Aug 21, 2014 10:24 pm

0002-KINETIS-Support-for-MCHCK-K20-board.patch

os/hal/boards/MCHCK_K20/board.c:
Trivial alignment issue on PTD5 due to a stray tab character.

Code: Select all

+        /* PTD0*/ PAL_MODE_UNCONNECTED,     /* PTD1*/ PAL_MODE_UNCONNECTED,     /* PTD2*/ PAL_MODE_UNCONNECTED,
+        /* PTD3*/ PAL_MODE_UNCONNECTED,     /* PTD4*/ PAL_MODE_UNCONNECTED,    /* PTD5*/ PAL_MODE_UNCONNECTED,
+        /* PTD6*/ PAL_MODE_UNCONNECTED,     /* PTD7*/ PAL_MODE_UNCONNECTED,     /* PTD8*/ PAL_MODE_UNCONNECTED,
+        /* PTD9*/ PAL_MODE_UNCONNECTED,     /*PTD10*/ PAL_MODE_UNCONNECTED,     /*PTD11*/ PAL_MODE_UNCONNECTED,


os/hal/boards/MCHCK_K20/board.h:

1. Fix board name in comment s/Freescale Freedom.*/MCHK whatever/
2. Remove commented-out KINETIS_SYSCLK, _MCG_MODE, _XTAL_FREQUENCY here which are typically defined in the application mcuconf.h. Do we want to allow board.h to provide defaults if mcuconf.h doesn't specify?

Code: Select all

+/*
+ * Setup for Freescale Freedom K20D50M board.
+ */
+
+/*
+ * Board identifier.
+ */
+#define BOARD_MCHCK_MX20DX128
+#define BOARD_NAME                  "MCHCK MX20DX128"
+
+/* External 8 MHz crystal with PLL for 48 MHz core/system clock. */
+//#define KINETIS_SYSCLK_FREQUENCY    48000000UL
+//#define KINETIS_MCG_MODE            KINETIS_MCG_MODE_PEE
+//#define KINETIS_XTAL_FREQUENCY      8000000UL

SpaceCoaster
Posts: 49
Joined: Mon Aug 11, 2014 6:40 am

Re: SPI driver for KINETIS K20.

Postby SpaceCoaster » Thu Aug 21, 2014 10:29 pm

Colin,

Thanks for the code review. All those changes look good.

Derek

colin
Posts: 149
Joined: Thu Dec 22, 2011 7:44 pm

0003-KINETIS-SPI-demo-for-MCHCK-K20-board.patch review

Postby colin » Thu Aug 21, 2014 11:22 pm

0003-KINETIS-SPI-demo-for-MCHCK-K20-board.patch

demos/KINETIS/RT-MCHCK-K20-SPI/Makefile:

Move -std=gnu99 to USE_COPT instead of USE_OPT so it applies only to C and not C++ modules. Granted there may not be any C++ in the demo but it's just good practice, maybe someone will copy this file as a starting point and add C++ files later.

Just a comment, maybe leave it as-is. But any idea why CRT0_INIT_STACKS=0 is defined? I assume you copied this from the other K20 demos (RT-FREEDOM-K20D50M and RT-TEENSY3), but it's not clear why those platforms would specify this. It looks like CRT0_INIT_STACKS fills the main and process stacks with zeros at startup so it must be merely a performance optimization to skip this. It seems it must be very few bytes of code space and runtime cycles. Shall we leave it off and get the default value which is TRUE? I assume that the reason the stack initialization code exists is that it's for predictability, so there is no chance for garbage left over after soft reset or something to affect subsequent behavior in case of referencing unassigned locations on the stack.

Code: Select all

+# Compiler options here.
+ifeq ($(USE_OPT),)
+  USE_OPT = -O2 -ggdb -fomit-frame-pointer -falign-functions=16 -DCRT0_INIT_STACKS=0 -std=gnu99
+endif
+
+# C specific options here (added to USE_OPT).
+ifeq ($(USE_COPT),)
+  USE_COPT =
+endif
+
+# C++ specific options here (added to USE_OPT).
+ifeq ($(USE_CPPOPT),)
+  USE_CPPOPT = -fno-rtti
+endif

SpaceCoaster
Posts: 49
Joined: Mon Aug 11, 2014 6:40 am

Re: SPI driver for KINETIS K20.

Postby SpaceCoaster » Fri Aug 22, 2014 4:35 am

Colin,

I checked out the CRT0_INIT_STACKS and it looks OK to initialize the stack. The code looks good and it runs fine with the stacks being initialized. Fabio commented about it in another thread.

One thing that might cause a problem. In my Makefile I have

Code: Select all

USE_PROCESS_STACKSIZE = 0x200

In GCC/rules.mk I have

Code: Select all

ifeq ($(USE_PROCESS_STACKSIZE),)
  LDOPT := $(LDOPT),--defsym=__process_stack_size__=0x400
else
  LDOPT := $(LDOPT),--defsym=__process_stack_size__=$(USE_PROCESS_STACKSIZE)
endif

in MK20DX128.ld I have

Code: Select all

__process_stack_size__  = 0x0400;

The final result in ch.dmp is that the __process_stack_size__ ends up as 0x200 but 0x400 is actually allocated on the stack.
This would work as long as you decrease the stack size in the Makefile.

Increasing the size to 0x600 in the Makefile and you end up with __process_stack_base__ and __process_stack_end__ being separated by 0x400 while __process_stack_size__ is set to 0x600.

I am not sure if this is actually a problem but it certainly looks odd/wrong.

Cheers,
Derek

SpaceCoaster
Posts: 49
Joined: Mon Aug 11, 2014 6:40 am

Re: SPI driver for KINETIS K20.

Postby SpaceCoaster » Fri Aug 22, 2014 4:43 am

OK, everything looks better if the lines.

Code: Select all

__main_stack_size__     = 0x0400;
__process_stack_size__  = 0x0400;

are taken out of KL25Z128.ld and MK20DX128.ld

utzig
Posts: 353
Joined: Sat Jan 07, 2012 6:22 pm
Location: Brazil
Has thanked: 1 time
Been thanked: 15 times
Contact:

Re: SPI driver for KINETIS K20.

Postby utzig » Fri Aug 22, 2014 8:17 pm

Colin,

CRT0_INIT_STACKS=0 was previously defined because with the stack initialisation the watchdog would fire before getting to __early_init where it was disabled. Giovanni changed the order some weeks ago after I told him of this problem so now __early_init is called before initialising the stack. I should have changed the code back then...

Also, I'm almost done with the I2C code. Just need to make it interrupt driven now, which should be easy and then finish writing a driver for MMA8451 to be added to both freedom demos. Thankfully I2C works the same on both KL2x and K2x...

I will review this in the following days, thanks!

SpaceCoaster
Posts: 49
Joined: Mon Aug 11, 2014 6:40 am

Re: SPI driver for KINETIS K20.

Postby SpaceCoaster » Sat Aug 23, 2014 5:23 am

Fabio,

Great, that explains a lot.

I am looking forward to using the I2C code. I have a few chips I want to try it with. I think I might even have an MMA8451.

Cheers,
Derek

SpaceCoaster
Posts: 49
Joined: Mon Aug 11, 2014 6:40 am

Re: SPI driver for KINETIS K20.

Postby SpaceCoaster » Sat Aug 23, 2014 5:18 pm

Updated patches for SPI to include Colin's comments.
Attachments
0003-KINETIS-SPI-demo-for-MCHCK-K20-board.patch.gz
(8.46 KiB) Downloaded 86 times
0002-KINETIS-Support-for-MCHCK-K20-board.patch.gz
(2.06 KiB) Downloaded 84 times
0001-KINETIS-SPI-driver.patch.gz
(7.97 KiB) Downloaded 87 times


Return to “Kinetis Support”

Who is online

Users browsing this forum: No registered users and 1 guest