On Mon, 23 Oct 2023, Jeff LaBundy wrote: > I understand that no customer would ever build the to-be-added codec > driver _without_ the input driver, but the MFD must be generic enough > to support this case. Would a codec-only implementation use f0 and ReDC > estimation? If so, then these functions _do_ belong in the MFD, albeit > with some comments to explain their nature. I'm not going to be able to accept a single-function device into the multi-function devices subsystem. Please submit both once the codec is ready. > > > >> + struct device *dev = cs40l50->dev; > > > >> + int error; > > > >> + > > > >> + mutex_init(&cs40l50->lock); > > > > > > > > Don't you need to destroy this in the error path? > > > > > > My understanding based on past feedback is that mutex_destroy() > > > is an empty function unless mutex debugging is enabled, and there > > > is no need cleanup the mutex explicitly. I will change this if you > > > disagree with that feedback. > > > > It just seems odd to create something and not tear it down. > > mutex_init() is not creating anything; the mutex struct is allocated as > part of the driver's private data, which is de-allocated as part of device > managed resources being freed when the module is unloaded. > > mutex_destroy() is a NOP unless CONFIG_DEBUG_MUTEXES is set, and there are > roughly 4x fewer instances of it than mutex_init() in mainline. I recommend > not to add mutex_destroy() because it adds unnecessary tear-down paths and > remove() callbacks that wouldn't otherwise have to exist. Fair enough. -- Lee Jones [李琼斯]
