mcux-sdk icon indicating copy to clipboard operation
mcux-sdk copied to clipboard

[BUG] many FSL API parameters not 'const' where required for customer applications

Open DRNadler opened this issue 1 year ago • 9 comments

The Problem Many times applications must call FSL API functions with constant data. For example, the application I'm working on has 6 different SPI peripherals, and each one requires several different control byte strings to be sent. These are all declared const (and placed in flash memory). Thus the FSL transmit API should take a const uint8_t* argument for the bytes to be sent. Similarly, configuration structures used for FSL API are const.

The FSL API as coded produces compile errors where it does not take const arguments. Ignoring this is a violation of many coding standards, hence your customers often must change the FSL functions. Not good!

Suggested Fixes

Review each parameter for all API functions. Where the FSL library will not modify the input, ensure that the parameter is declared const. Especially, any data which is transmitted, or used for configuration.

** Feedback Requested ** @DavidJurajdaNXP , @mcuxsusan - Can you please confirm this explanation is clear for you? Note this is a clarification requested by Susan for issue #59

DRNadler avatar Jun 01 '23 20:06 DRNadler

Hello NXP,

this seems a very valid request. Why is there no response from NXP for months?

Thanks.

herrmanthegerman avatar Nov 27 '23 10:11 herrmanthegerman

It's only been 6 months... @DavidJurajdaNXP, @mcuxsusan - Anybody there?

DRNadler avatar Nov 27 '23 15:11 DRNadler

Hi @DavidJurajdaNXP ,

UART drivers use such way to use const tx data, https://github.com/nxp-mcuxpresso/mcux-sdk/blob/main/drivers/lpuart/fsl_lpuart.h#L276

do you think the the FreeRTOS drivers https://github.com/nxp-mcuxpresso/mcux-sdk/issues/59 can be updated for this purpose? thanks.

zejiang0jason avatar Nov 28 '23 02:11 zejiang0jason

The txData in SPI drivers, such as: https://github.com/nxp-mcuxpresso/mcux-sdk/blob/main/drivers/lpspi/fsl_lpspi.h#L345, will be udpated to const type.

zejiang0jason avatar Nov 28 '23 02:11 zejiang0jason

One down, 1237 to go... You guys need to do a systematic cleanup of missing 'const'. Not just one at a time.

DRNadler avatar Nov 28 '23 13:11 DRNadler

Will update not only the data sending functions, but also the functions which don't modify parameter content, such as SetConfig, the parameter config shall be const. These changes will be applied to all drivers, it will be done step by step. SPI part go first and will be updated soon.

zejiang0jason avatar Dec 07 '23 04:12 zejiang0jason

I add my vote to this request. This kind of fix is necessary for several reasons, especially if you need to get some kind of conformance/certification of ypour source code. I need the fix for UART functions. As an example https://github.com/nxp-mcuxpresso/mcux-sdk/blob/43a5a8df5728436f88dd1044fba66972bae723b6/drivers/uart/fsl_uart_freertos.h#L126

This issue is similar to #59 which is in charge to @mcuxsusan and @DavidJurajdaNXP

escherstair avatar Dec 11 '23 11:12 escherstair

@DavidJurajdaNXP, could you please help?

mcuxsusan avatar Dec 12 '23 01:12 mcuxsusan

I created the PR #154. Feel free to review it and add other commits, if needed

escherstair avatar Dec 12 '23 07:12 escherstair