linux-kernel - Re: [PATCH RFC 5/5] mfd: cs40l26: Add CODEC driver component

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: <ZHEL30a/YEia7kjr@nixie71>
Date:   Fri, 26 May 2023 14:43:27 -0500
From:   Jeff LaBundy <jeff@...undy.com>
To:     Fred Treven <fred.treven@...rus.com>
Cc:     Ben Bright <ben.bright@...rus.com>,
        James Ogletree <james.ogletree@...rus.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Simon Trimmer <simont@...nsource.cirrus.com>,
        Charles Keepax <ckeepax@...nsource.cirrus.com>,
        Richard Fitzgerald <rf@...nsource.cirrus.com>,
        patches@...nsource.cirrus.com, linux-input@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        lee@...nel.org
Subject: Re: [PATCH RFC 5/5] mfd: cs40l26: Add CODEC driver component

Hi Fred,

On Thu, May 25, 2023 at 07:04:31PM -0500, Fred Treven wrote:
> Use MFD interface to load the CODEC driver along
> with the Input FF driver.
> 
> Signed-off-by: Fred Treven <fred.treven@...rus.com>
> ---

Did you mean to include a thin codec driver as part of this series to
support the audio-to-haptics use-case? I don't see one.

As Lee correctly points out, this isn't an MFD despite the title of the
commit message, and is sort of an abuse of the API.

What you seem to actually want is a true MFD driver responsible for
initializing common resources such as regmap, an IRQ chip, etc. That
driver then adds input and codec drivers as children.

At the moment, you're more or less treating the input driver as the
MFD with one child, but that is not the correct pattern.

>  drivers/input/misc/cs40l26.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/input/misc/cs40l26.c b/drivers/input/misc/cs40l26.c
> index 12c29cbd4ff0..35d15a6c2230 100644
> --- a/drivers/input/misc/cs40l26.c
> +++ b/drivers/input/misc/cs40l26.c
> @@ -13,6 +13,7 @@
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/core.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/string.h>
>  #include <linux/firmware/cirrus/wmfw.h>
> @@ -2136,6 +2137,10 @@ static inline int cs40l26_worker_init(struct cs40l26_private *cs40l26)
>  	return 0;
>  }
>  
> +static const struct mfd_cell cs40l26_devs[] = {
> +	{ .name = "cs40l26-codec" },
> +};
> +
>  static struct regulator_bulk_data cs40l26_supplies[] = {
>  	{ .supply = "VP" },
>  	{ .supply = "VA" },
> @@ -2275,6 +2280,12 @@ int cs40l26_probe(struct cs40l26_private *cs40l26)
>  	if (error)
>  		goto err;
>  
> +	error = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cs40l26_devs, 1, NULL, 0, NULL);
> +	if (error) {
> +		dev_err(dev, "Failed to MFD add device %s: %d\n", cs40l26_devs[0].name, error);
> +		goto err;
> +	}
> +
>  	return 0;
>  err:
>  	cs40l26_remove(cs40l26);
> -- 
> 2.7.4
> 

Kind regards,
Jeff LaBundy