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.
Original Attachment has been moved to: k22f_ksdk2_sd_card_04.zip