nxp-mcuxpresso / mcux-sdk

MCUXpresso SDK
BSD 3-Clause "New" or "Revised" License
323 stars 142 forks source link

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

Open DRNadler opened 1 year ago

DRNadler commented 1 year ago

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

herrmanthegerman commented 10 months ago

Hello NXP,

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

Thanks.

DRNadler commented 10 months ago

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

zejiang0jason commented 10 months ago

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 commented 10 months ago

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.

DRNadler commented 10 months ago

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

zejiang0jason commented 10 months ago

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.

escherstair commented 9 months ago

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

mcuxsusan commented 9 months ago

@DavidJurajdaNXP, could you please help?

escherstair commented 9 months ago

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