linux-kernel - Re: [PATCH v4 3/4] mfd: cs40l50: Add support for CS40L50 core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231024154714.GK8909@google.com>
Date:   Tue, 24 Oct 2023 16:47:14 +0100
From:   Lee Jones <lee@...nel.org>
To:     Jeff LaBundy <jeff@...undy.com>
Cc:     James Ogletree <James.Ogletree@...rus.com>,
        James Ogletree <james.ogletree@...nsource.cirrus.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Fred Treven <Fred.Treven@...rus.com>,
        Ben Bright <Ben.Bright@...rus.com>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/4] mfd: cs40l50: Add support for CS40L50 core driver

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 [李琼斯]