Re: [PATCH v8 3/5] mfd: cs40l50: Add support for CS40L50 core driver

[Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>]

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