> On Mar 5, 2024, at 4:20 AM, Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote: > > 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. I will adopt the first solution here from Charles for v9. Best, James
