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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> 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






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux