renesas / fsp

Flexible Software Package (FSP) for Renesas RA MCU Family
https://renesas.github.io/fsp/
Other
182 stars 81 forks source link

rm_block_media_ram api functions are not passing context info to the filex layer above it (via callback) #292

Closed cv007 closed 8 months ago

cv007 commented 10 months ago

A simple project with the stack for FileX on block media (standalone), with block media ram implementation, FSP 4.5.0 (FSP 4.6.0 looks the same).

file- fsp/ra/fsp/src/rm_block_media_ram/rm_block_media_ram.c

function (showing one of the api functions as an example, relevant lines shown)-

fsp_err_t RM_BLOCK_MEDIA_RAM_MediaInit (rm_block_media_ctrl_t * const p_ctrl)
    rm_block_media_callback_args_t       block_media_ram_args;
    block_media_ram_args.event = RM_BLOCK_MEDIA_EVENT_OPERATION_COMPLETE;
    p_instance_ctrl->p_callback(&block_media_ram_args);

note the block_media_ram_args.p_context member is never set, but will be used in the callback function

The p_callback function pointer will point to a function located in the file- fsp/ra/fsp/src/rm_filex_block_media/rm_filex_block_media.c

this function-

void rm_filex_block_media_memory_callback (rm_block_media_callback_args_t * p_args)
{
    /* Pass the event up to the application layer. */
    rm_filex_block_media_instance_ctrl_t * p_instance_ctrl = (rm_filex_block_media_instance_ctrl_t *) p_args->p_context;

which uses the p_context member of of p_args, which was not set in the calling function. The p_context member is cast and used with the invalid data, so ends up in Default_Handler as it will be trying to call another callback (filex user callback).

A temporary solution is to populate the lower level g_rm_block_media0_ctrl.p_context member with the needed value of &g_rm_filex_block_media_0_ctrl, then use that p_context member to supply block_media_ram_args.p_context before using-

After running g_rm_filex_block_media_0_instance.p_api->open(...), add the line- g_rm_block_media0_ctrl.p_context = &g_rm_filex_block_media_0_ctrl;

which will allow RM_BLOCK_MEDIARAM* functions that use the callback to provide the filex layer above with the needed args.p_context info by adding a line before using the callback- block_media_ram_args.p_context = p_instance_ctrl->p_context;

It appears rm_block_media_ram may need more work-

I think the code generator needs to fill in this missing info in the struct data it generates (&g_rm_filex_block_media_0_ctrl into g_rm_block_media0_cfg,p_context, which will be in common_data.c), which can then be used in the rm_block_media_ram.c functions as shown above. Or maybe there is some other way to handle this.

It also appears the struct rm_block_media_ram_instance_ctrl_t may need to be looked at. The p_cfg member is never filled in (the block media ram api open function does not set it), and there are duplicate members p_callback and p_context which are populated from the p_cfg data which otherwise could be removed and p_cfg used instead (p_cfg->p_cfg). I'm not sure if the intention for this is to allow changing these values at runtime, but if not then these members can be seen in the p_cfg member instead (if is set).

TakahiroMuranaka227 commented 9 months ago

Could you please create RAP ticket for this issue ? (If possible, please provide a project file that reproduces this issue.)

renesas-brandon-hussey commented 9 months ago

@cv007 please ignore the "RAP ticket" request. Our RAP ticketing system is for internal use only. We'll discuss this internally and reply back with any questions.

renesas-brandon-hussey commented 8 months ago

Thanks @cv007 , this will be fixed in our next release.