Comments on Creating a First KDSK 2.0 Application

[myke_predko]

This is about the fourth comment I've made on starting a new product which will provide an SD Card (as well as USB CDC) interface which I started in January of 2016.  As the tool set is changing (I've done quite a bit with MQX previously) and I want to develop on an active tool path (MQX requires too much low-level work to deal with bugs and problems that look like they will never be resolved). 

With a bit of struggling, I was able to get a basic serial interface application on KSDK 1.3 (with PE) for the SD Card SPI interface working with ChaN's FatFs and I posted the code here on the forums.  The big issue was that despite how many devices do provide SD Card interfaces, the documentation and example code is spotty, conflicting and very often wrong.  Unfortunately, several of the the examples in ChaN's database have errors as does a few other notable examples.  Once I figured out what they are, it's relatively easy to create the application. 

To look at doing something with KDSDK 2.0, I decided to port my SD Card interface code to KSDK 2.0.  It's a fairly substantial application and one that I am very familiar with.  It also allowed me to compare how I modified FatFs with the version included in KSDK 2.0 as it will be an important part of the application.  I was able to carry out the port in about four days with some pretty significant struggles along with way - I've attached the application to this post. 

My comments to NXP are as follows:

1.  There needs to be a lot of work done on the documentation for new users to create new applications.  There is a lot of hand waving indicating that the new project wizard is simple (and it is) but it really isn't helpful in terms of actually getting a working "Hello World" application going.  As noted in this thread (https://community.freescale.com/thread/385016), the basic includes set up in "main.c" do not support even this level of functionality.  The current introductory documentation that I have found shows how to install KDSK 2.0 and import with build and test a sample application but doesn't show the steps for actually creating one.  If you're familiar with C for the Kinetis, KDS/CW, etc., you can probably do it in reasonably short order (as I did) but if you're new to the process it will be very difficult. 

1.1.  On this point, I would really expect a basic project build to a) build cleanly and b) allow adding a 'printf("Hello World!\r\n");' statement to give the developer a warm fuzzy that they can actually create an application (this is generally the first application I create on a new board/hardware). 

2.  There seems to have been a lot of work on the various "drivers" included into the KSDK and I understand the amount of work it is documenting each one completely as well as providing examples, but I did a lot of stumbling that would have been eliminated with appropriate documentation.  Of particular concern (and I see that there has been some comments put on this forum already) is how to implement interrupts.  In the attached application, I was able to figure out how to implement PIT and GPIO Input IRQs, but that was through sheer perseverance - the (API) Reference Manual(s) need to explain how it is implemented as this is not intuitively obvious. 

2.1  As I was writing this, I looked over my code in "main.c" and realized that I was using a macro which used a macro which was already defined in "board.h".  So, it's really pretty difficult to understand how to implement interrupts in the KSDK and see the big picture. 

2.2.  While various aspects of the drivers and their APIs seem to be streamlined, I was surprised to see that my application (which is functionally identical and uses much of the same base source code as well as the same compiler as the KSDK 1.3 & PE version) is 5% larger.  I am building with the "Full Source", so that may be affecting the final application size. 

3.  This and the next four five are in regards to setting conventions.  The documentation and several posts from NXP employees indicate that there is a simplified directory structure but a stated convention for projects needs to be implemented.  In the attached project, you'll see that I created new folders for the SD/SPI interface routines and for the FatFs (hopefully developers will realize that by doing this, the .h files will not be picked up unless the C/C++ Include search path is updated in "Properties") while leaving "drivers" folder undisturbed, updating "board.h" (see next point) and leaving it in the "board" folder and putting in my interrupt handlers ("Events.c" along with "Events.h") in the "source" folder. 

4.  "board.h".  Based on experience, I would expect "board.h" to be a single, big honking file with all the board's (FRDM-K22F in this case) interface information.  "board.h", as it is created by the new project wizard is marginal and seems to be in need of modification (which is done in each example KDSK 2.0 application).  I would think that this is problematic from the developer perspective in terms of code commonality (ie creating new applications faster and using tried and true methodologies & code).  I know this is a lot of work for someone but it is absolutely required for even experienced developers working on a new board/processor. 

5.  As with the last point, I have the same concern with "pin_mux.c" being in the "board" folder and requiring the user to not only know that it is there but also modify the various muxes & pins for different peripherals and drivers.  Ideally, I would think that this should be a generic set of APIs that the user calls for setting up their board/processor. 

5.1.  Thinking about it, I will move "pin_mux.c" and "pin_mux.h" into "sources" because I believe, with how it is used, it belongs in this folder rather than "board". 

6.  "Events.c" and how to handle interrupts is sorely lacking.  I suspect that DMA (which I won't be using on this product, but have in other products and will again in the future) has similar issues.  As noted in this post and in others, documentation is sorely lacking on how to enable and handle interrupt requests as well as document where interrupts handlers should be located.  With PE, the handlers were placed in "Events.c" and referenced in "Events.h" which is a convention I am following with the two files placed in the "source" folder. 

7.  Macros calling methods.  Size is an issue that I am concerned about for the product that I'm working on (I want to use the Kinetis K22 with the smallest amount of Flash) so I was surprised to see that the LED macros in "board.h" are calling methods which are one line long and if you were to replace the call to the method with the one line, for example making "LED_RED_ON();" into:

#define LED_RED_ON() \

    BOARD_LED_RED_GPIO->PCOR = 1U << BOARD_LED_RED_GPIO_PIN

the parameter stack save as well as the calling code (which) takes up less program space, less stack space and runs faster while still retaining the basic code by the user.  I understand the reason why hardware interfaces are ideally placed in separate methods, but in this case, a generic method is called and there is no protection in the processor (which is the primary/traditional reason for handling IO in separate methods).  I know I may have started a religious debate here, but the approach above, is faster, eliminates a call, uses less RAM and uses less program space, all measures of efficiency. 

8.  Reviewing all the APIs and make sure they make sense or are properly documented.  In the examples, I was confused by some calling "EnableIRQ" and "NVIC_EnableIRQ" and in the API document, both APIs are used without explanation of why one is used over another.  Upon looking at the source, I discovered that "EnableIRQ" has some code which is "#if" ignored and calls "NVIC_EnableIRQ". I'm not sure what is going on here and I couldn't find anything to explain it.  In looking things over, I suspect that it has something to do with CMSIS but it isn't clear why or what it provides. 

9.  NXP's FatFs modifications.  I'm guessing that the modifications made by NXP on ChaN's FatFs package were to work with the examples provided with the KSDK.  Well, in doing that, there are extra layers of calls that obfuscate the functionality of the code as well as reduce the flexibility of ChaN's package (which is most bewildering).  In my example code, you'll see I eschewed the use of the FatFs provided in the KSDK with the code I used for the original application (even though they are at apparently at the same level - I did a kDiff on ff.c and they are the same).  The big issue is, I'm very familiar with FatFs and when I look at the changes, it seems to me that it was modified by somebody who really doesn't understand the code and that lowers my confidence in KSDK 2.0 as a whole. 

10.  The EmbSysRegView (EmbSysRegView) is a great Eclipse plug in and one that is invaluable for KSDK 1.x development - unfortunately it doesn't work with KSDK 2.0.  I'm hoping that this is high on the on bug lists and feature adds for KDS (I'm suggesting this because it will make NXP developer's lives easier). 

These are my comments on a farily limited exposure (probably 60 hours or so of coding and debugging).  I can see some positives and potential, but without MUCH better documentation, it's going to frustrate more users than entice. 

Finally, I know I seem obsessed about call levels and it's partially due to memory (Flash and RAM) increases but it's really due to performance hits.  I was recently shown a comparison of the Mac Bluetooth code versus Windows.  The Mac code has minimal calls and seems to be about 10% larger than Windows - BUT it executes many times faster with much fewer bugs introduced along the way.  I'm not saying eliminate calls to common methods, just that I think more effort needs to be made understanding why the calls are there, what are their impact (in terms of opportunities for bugs and performance) and ensuring that they are documented. 

Next on the agenda is to redo this application as a freeRTOS one and then add USB CDC - I believe that will be the most efficient order.

myke

Original Attachment has been moved to: k22f_ksdk2_sd_card_04.zip

[mr_max]

Completely agreed about the complexity of learning KSDK and PEX. I started to use it 2 years ago. I was a true beginner of ARM micro controller programming. CW and KDS tools are very powerful but the lack of clear NXP documentations is to often a very frustrating thing ! Still today, I'm stuck with the way to use fsl_sdcard component. Because PEX added a file spi_dspi_layer.c in source where I have to add some spi transaction code using fsl_spi KSDK drivers ( and plus malloc/colloc issue, check here fsl_sdcard malloc return NULL pointer , this is the reasons why I cam here :smileyhappy:)

10. KSDK 2.0 doesn't support EmbSysRegView. I envisaged to update KSDK but I think I'll keep with v1.3.

Anyway, I like you NXP but pleas keep improving your pedagogy.

[davidjurajda]

Hi Myke,

At first, thank you very much for your comprehensive and valuable feedback.

I would like to react to point 9. - FatFs.

ChaN's FatFs package in SDK is modified in two major areas:

    - Add glue functions for low level drivers (SDHC, SDSPI, RAM disk, MMC). Resulting in modified diskio.c

    - Add RTOS wrappers to make FatFs thread safe. Resulting in RTOS specific <rtos>_syscall.c files.

Hardware abstraction in diskio.c is implemented according to template diskio.c from original package. Each peripheral specific code (SDHC, SDSPI, RAM disk, MMC) is located in specific files to keep functions in diskio.c in reasonable and readable length. RTOS wrappers are based on template fatfs\src\option\syscall.c. FatFs functionality is not affected and only steps necessary for integration with KSDK drivers were performed.

How the flexibility of ChaN's package is affected? I would appreciate more specific information about problematic parts of integration.

Your example contain only one physical interface - SDSPI, but FatFs in SDK must interact with several of them (SDHC, SDSPI, RAM disk, MMC, USB) and with three different RTOSs. I believe separation of this interfaces into several files increasing code clarity in these case.

Regards,

David

[myke_predko]

Hi David,

Thank you for replying.  I realize that you have to support multiple physical interfaces as well as making FatFs reenterent (thread safe) and in the process of implementing this, I found "diskio.c" to be much more difficult to follow and one API in the source file (I believe it's disk_read) the operation was now incorrect. 

Yes, this is for my single case and making it a bit more unique is that I think I'm the first person to get this code working, but as I said I found the changes to diskio.c to be confusing as well as very complex and when I tried following through it, I found an instance which didn't seem to be correct.  In comparison, the basic diskio.c that comes from the basic FatFs download is quite easy to follow, supports multiple physical interfaces, and data is passed in a very straightforward manner.  I would be interested in the comments of other people who are familiar with implementing an interface using FatFs. 

I'm guessing that you are the person who made the modifications to the FatFs package and I apologize if I'm insulting.  I'm not trying to be and I hope you can understand that my comments are as a first user with experience with the KSDK 2.0 package.  

myke

[davidjurajda]

Hi Myke,

I have no issues about way how you express your experience and I am sure our goal is same - useful code. I am owner of FatFs component and I am responsible for its integration.

Let’s start with some information which can be helpful for other developers.

We stripped out sdspi and sdspi_fatfs examples from SDK_2.0_FRDM-K22F package because these board have not populated SD sockets. It can be useful to have this examples available and I am attaching them in SDK_2.0_FRDM-K22F_examles.zip. It  should work directly after *.zip extraction into original package without extra configuration.

I have compared your application with modified version with FatFs from KSDK (attached k22f_sd_card_sdk_fatfs.zip).

First difference is in code size. Modified version (with SDK FatFs) is bigger (debug: 7%, release: 4%).

------------------------ Debug ------------------------

arm-none-eabi-size --format=berkeley "k22f_sd_card_sdk_fatfs.elf"

   text    data     bss     dec     hex filename

  63496     128   12100   75724   127cc k22f_sd_card_sdk_fatfs.elf

arm-none-eabi-size --format=berkeley "k22f_ksdk2_sd_card_04.elf"

   text    data     bss     dec     hex filename

  57912     164   12432   70508   1136c k22f_ksdk2_sd_card_04.elf

------------------------ Release (-Os) ------------------------

arm-none-eabi-size --format=berkeley "k22f_sd_card_sdk_fatfs.elf"

   text    data     bss     dec     hex filename

  40488     128   12096   52712    cde8 k22f_sd_card_sdk_fatfs.elf

arm-none-eabi-size --format=berkeley "k22f_ksdk2_sd_card_04.elf"

   text    data     bss     dec     hex filename

  38040     164   12428   50632    c5c8 k22f_ksdk2_sd_card_04.elf

Read/Write speeds are slightly better in your application. Speed can be still enhanced by SPI baud rate change.

Change from:

DSPI_BUS_BAUDRATE = 500000

to:

DSPI_BUS_BAUDRATE = 10000000U

significantly improve write/read speeds.

According to other issues:

- Incorrect operation of disk_read

    -- not reproduced.

- Confusing and complex changes in diskio.c

Yes, there are several extra calls to satisfy clear abstraction between different layers. We would like to reach reasonable tradeoff between performance and code clarity. Long functions are also confusing in my opinion.

I agree there are still space for improvements (code size, speed performance) and we are working on that. I will provide more information when it will be ready.

Regards,

David

[myke_predko]

Hi David,

Thanx for the reply.  A few comments back:

You say that my read/write is faster - I'm guessing that's because I have eliminated (what I consider) the redundant calls.  I limited the faster read/writes to 4.8MHz because that was the max I could do with ksdk 1.3MHz through PE and I knew that it works.  When I get closer to the final product, I'll crank it up to 10MHz. 

The disk_read error is in the code to read multiple blocks (sectors).  After the end token (0xFD) is sent, nothing more is to be done - you don't get any checksums or 0xE5 value.  The code should be just send 0xFD and return.  If you wait for 0xE5, you'll wait forever (which is the error) - there are a number of code examples on the web (and Erich Styger's is one of them) that has this error and the documentation I could find isn't clear on this issue but, trust me, I spent over a week on this.  Other than the "rw2" test in my example code, I haven't seen any explicit example code testing multiple block read/writes. 

I'm struggling as to what is the right rules for calls vs macros.  If you're working with bare metal or a non-protected memory/resource RTOS and the macro code is obviously less than than the calling code (which is the situation with the LED example above) it makes sense.  "Obviously less" is a judgement call and I generally look at the generated code in both cases.  If it is a protected OS (ie Linux or QNX) or there are two or more lines of code, then I stick with the method call.  

I have now done a number of projects with ksdk 2.0, expanding the peripherals (namely ADC, USB CDC, I2C & PWM) along with the GPIO, Interrupts and SPI in the first example project and it seems to provide a pretty good level of abstraction over the hardware.  Documentation (especially examples in the SDK RM with a focus on interrupts) needs work.  The example apps are pretty good.  I think my comments regarding project folders need to be thought through a bit more is still valid. 

myke

[davidjurajda]

Hi Myke,

I found the package k22f_ksdk2_sd_card_04_SDK_FatFs.zip is wrong. Please see new one k22f_ksdk2_sd_card_04_SDK_FatFs_v2.zip. There is still one issue with card detection and board must be restarted after card insertion, but both tests rw1 and rw2 are functional in this version.

David

[myke_predko]

Hi David,

Could you point to me where you think my code is wrong?  It looks like you replaced my SPI and FatFS code with the original and I would like to understand where the issues are.  I did a KDiff on the board and source and don't see any differences. 

I'm not sure what you mean by there being a problem with card detection.  If you have soldered J8 (and I'm not trying to be condescending, it's a bitch to solder the connection pin even with the right tools) correctly, the green LED on the board will go on when the uSD is inserted and when you send a command, the first thing that's done is to try and mount the drive. 

I should point out that I updated the code to just use PCS0 for Chip Select instead of manually driving the CS pin (as I do in '_04) and things work just fine. 

Thanx!

myke

[ivadorazinova]

Hi Myke,

Many thanks for your feedback and for sharing your code!

I will pass it to our Development team.

Thank you,

Best Regards,

Iva