On Tue, Mar 05, 2024 at 09:58:18AM +0000, Lee Jones wrote: > On Fri, 01 Mar 2024, James Ogletree wrote: > > >> +static const struct cs_dsp_client_ops cs40l50_client_ops; > > > > > > What's this for? Where is it used? > > > > > > In general, I'm not a fan of empty struct definitions like this. > > > > From the same cs_dsp module as mentioned above, it is a structure containing > > callbacks that gives the client driver an opportunity to respond to certain events > > during the DSP's lifecycle. It just so happens that this driver does not need to do > > anything. However, no struct definition will result in a null pointer dereference by > > cs_dsp when it checks for the callbacks. > > Then check for NULL before dereferencing it? It would probably make sense to move the cs40l50_stop_core writes into the pre_run callback, and the CLOCK_ENABLE / cs40l50_configure_dsp stuff into the post_run callback. Which is probably a slightly more idiomatic way to use the API of cs_dsp and would mean some of the callbacks are initialised, thus dodging this problem. We could check for there being no client_ops in the cs_dsp core, but it feels kinda redundant since there really should generally be client ops. Thanks, Charles
