linux-kernel - Re: [PATCH RESEND 0/7] Initial Support for CS40L26

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: <Z7JVCANOhoUMnnzd@nixie71>
Date: Sun, 16 Feb 2025 15:13:44 -0600
From: Jeff LaBundy <jeff@...undy.com>
To: Fred Treven <ftreven@...nsource.cirrus.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Simon Trimmer <simont@...nsource.cirrus.com>,
	Charles Keepax <ckeepax@...nsource.cirrus.com>,
	Richard Fitzgerald <rf@...nsource.cirrus.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	James Ogletree <jogletre@...nsource.cirrus.com>,
	Ben Bright <ben.bright@...rus.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>,
	David Rhodes <david.rhodes@...rus.com>,
	Heiko Stuebner <heiko@...ech.de>, Karel Balej <balejk@...fyz.cz>,
	Igor Prusov <ivprusov@...utedevices.com>,
	Jack Yu <jack.yu@...ltek.com>,
	Weidong Wang <wangweidong.a@...nic.com>,
	Binbin Zhou <zhoubinbin@...ngson.cn>,
	Prasad Kumpatla <quic_pkumpatl@...cinc.com>,
	Paul Handrigan <paulha@...nsource.cirrus.com>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Nuno Sa <nuno.sa@...log.com>, alsa-devel@...a-project.org,
	patches@...nsource.cirrus.com, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
	linux-sound@...r.kernel.org
Subject: Re: [PATCH RESEND 0/7] Initial Support for CS40L26

Hi Fred,

On Tue, Feb 04, 2025 at 05:18:29PM -0600, Fred Treven wrote:
> Introduce driver for Cirrus Logic Device CS40L26:
> A boosted haptics driver with integrated DSP and
> waveform memory with advanced closed loop algorithms
> and LRA protection.
> 
> The core CS40L26 driver is in MFD and touches the
> Input Force Feedback subsystem for haptics and
> the ASoC subsystem for audio to haptics streaming.
> 
> This patchset includes changes to the CS DSP firmware
> driver which fixes two bugs and introduces support for
> multiple coefficient files.
> 
> Fred Treven (7):
>   firmware: cs_dsp: Fix error checking in wseq_write()
>   firmware: cs_dsp: Check for valid num_regs in
>     cs_dsp_wseq_multi_write()
>   firmware: cs_dsp: Add ability to load multiple coefficient files
>   dt-bindings: mfd: cirrus,cs40l26: Support for CS40L26
>   mfd: cs40l26: Add support for CS40L26 core driver
>   ASoC: cs40l26: Support I2S streaming to CS40L26
>   Input: cs40l26 - Add support for CS40L26 haptic driver
> 
>  .../bindings/mfd/cirrus,cs40l26.yaml          |   81 +
>  MAINTAINERS                                   |    4 +-
>  drivers/firmware/cirrus/cs_dsp.c              |   70 +-
>  drivers/input/misc/Kconfig                    |   10 +
>  drivers/input/misc/Makefile                   |    1 +
>  drivers/input/misc/cs40l26-vibra.c            |  669 ++++++++
>  drivers/mfd/Kconfig                           |   29 +
>  drivers/mfd/Makefile                          |    4 +
>  drivers/mfd/cs40l26-core.c                    | 1412 +++++++++++++++++
>  drivers/mfd/cs40l26-i2c.c                     |   63 +
>  drivers/mfd/cs40l26-spi.c                     |   63 +
>  include/linux/firmware/cirrus/cs_dsp.h        |   14 +
>  include/linux/mfd/cs40l26.h                   |  341 ++++
>  sound/soc/codecs/Kconfig                      |   12 +
>  sound/soc/codecs/Makefile                     |    2 +
>  sound/soc/codecs/cs40l26-codec.c              |  523 ++++++
>  16 files changed, 3281 insertions(+), 17 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/cirrus,cs40l26.yaml
>  create mode 100644 drivers/input/misc/cs40l26-vibra.c
>  create mode 100644 drivers/mfd/cs40l26-core.c
>  create mode 100644 drivers/mfd/cs40l26-i2c.c
>  create mode 100644 drivers/mfd/cs40l26-spi.c
>  create mode 100644 include/linux/mfd/cs40l26.h
>  create mode 100644 sound/soc/codecs/cs40l26-codec.c
> 
> -- 
> 2.34.1
> 

Thank you for this high-quality series!

When the CS40L50 MFD landed last year, I (probably naively) imagined that
CS40L26 and CS40L50 were close cousins. My understanding was that CS40L50
implements some basic DSP functionality in ROM, which can be overridden
with firmware uploaded at runtime, while CS40L26 requires firmware to be
uploaded at runtime in any circumstance.

To that end, I recall during the CS40L50 MFD review that there was some
hope the CS40L50 MFD could be gently extended to support both devices, and
a separate input driver may be required. At the very least, I was surprised
to see an all-new codec driver, since the codec child is essentially just a
stub driver in both cases.

If a completely new MFD and children are utlimately required here, then so
be it; I just wanted to ask whether you had considered if it's possible to
re-use and/or extend any existing driver(s) here. Assuming so, I think it
can be helpful to speak to this exercise in the cover letter and explain
your reasoning to the review audience.

Kind regards,
Jeff LaBundy