[PATCH] ALSA: hda: cs35l56: Stop creating ALSA controls for firmware coefficients

[PATCH] ALSA: hda: cs35l56: Stop creating ALSA controls for firmware coefficients

[Simon Trimmer]

A number of laptops have gone to market with old firmware versions that
export controls that have since been hidden, but we can't just install a
newer firmware because the firmware for each product is customized and
qualified by the OEM. The issue is that alsactl save and restore has no
idea what controls are good to persist which can lead to
misconfiguration.

As the ALSA controls for the firmware coefficients are not used in
normal operation they can all be hidden, but add a kernel parameter so
that they can be re-enabled for debugging.

Fixes: 73cfbfa9caea ("ALSA: hda/cs35l56: Add driver for Cirrus Logic CS35L56 amplifier")
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
---
 sound/pci/hda/cs35l56_hda.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index 96d3f13c5abf..1494383b22c9 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -23,6 +23,10 @@
 #include "hda_cs_dsp_ctl.h"
 #include "hda_generic.h"
 
+static bool expose_dsp_controls;
+module_param(expose_dsp_controls, bool, 0444);
+MODULE_PARM_DESC(expose_dsp_controls, "Expose firmware controls as ALSA controls 0=no (default), 1=yes");
+
  /*
   * The cs35l56_hda_dai_config[] reg sequence configures the device as
   *  ASP1_BCLK_FREQ = 3.072 MHz
@@ -758,6 +762,9 @@ static int cs35l56_hda_bind(struct device *dev, struct device *master, void *mas
 
 	cs35l56_hda_create_controls(cs35l56);
 
+	if (expose_dsp_controls)
+		cs35l56_hda_add_dsp_controls(cs35l56);
+
 #if IS_ENABLED(CONFIG_SND_DEBUG)
 	cs35l56->debugfs_root = debugfs_create_dir(dev_name(cs35l56->base.dev), sound_debugfs_root);
 	cs_dsp_init_debugfs(&cs35l56->cs_dsp, cs35l56->debugfs_root);
-- 
2.43.0

RE: [PATCH] ALSA: hda: cs35l56: Stop creating ALSA controls for firmware coefficients

[Simon Trimmer]

Apologies Takashi - can you ignore this one please? It will need a V2

> -----Original Message-----
> From: Simon Trimmer <simont@opensource.cirrus.com>
> Sent: Monday, July 29, 2024 5:16 PM
> To: tiwai@suse.com
> Cc: linux-sound@vger.kernel.org; alsa-devel@alsa-project.org; linux-
> kernel@vger.kernel.org; patches@opensource.cirrus.com; Simon Trimmer
> <simont@opensource.cirrus.com>
> Subject: [PATCH] ALSA: hda: cs35l56: Stop creating ALSA controls for
firmware
> coefficients
> 
> A number of laptops have gone to market with old firmware versions that
> export controls that have since been hidden, but we can't just install a
> newer firmware because the firmware for each product is customized and
> qualified by the OEM. The issue is that alsactl save and restore has no
> idea what controls are good to persist which can lead to
> misconfiguration.
> 
> As the ALSA controls for the firmware coefficients are not used in
> normal operation they can all be hidden, but add a kernel parameter so
> that they can be re-enabled for debugging.
> 
> Fixes: 73cfbfa9caea ("ALSA: hda/cs35l56: Add driver for Cirrus Logic
CS35L56
> amplifier")
> Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
> ---
>  sound/pci/hda/cs35l56_hda.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
> index 96d3f13c5abf..1494383b22c9 100644
> --- a/sound/pci/hda/cs35l56_hda.c
> +++ b/sound/pci/hda/cs35l56_hda.c
> @@ -23,6 +23,10 @@
>  #include "hda_cs_dsp_ctl.h"
>  #include "hda_generic.h"
> 
> +static bool expose_dsp_controls;
> +module_param(expose_dsp_controls, bool, 0444);
> +MODULE_PARM_DESC(expose_dsp_controls, "Expose firmware controls as
> ALSA controls 0=no (default), 1=yes");
> +
>   /*
>    * The cs35l56_hda_dai_config[] reg sequence configures the device as
>    *  ASP1_BCLK_FREQ = 3.072 MHz
> @@ -758,6 +762,9 @@ static int cs35l56_hda_bind(struct device *dev, struct
> device *master, void *mas
> 
>  	cs35l56_hda_create_controls(cs35l56);
> 
> +	if (expose_dsp_controls)
> +		cs35l56_hda_add_dsp_controls(cs35l56);
> +
>  #if IS_ENABLED(CONFIG_SND_DEBUG)
>  	cs35l56->debugfs_root = debugfs_create_dir(dev_name(cs35l56-
> >base.dev), sound_debugfs_root);
>  	cs_dsp_init_debugfs(&cs35l56->cs_dsp, cs35l56->debugfs_root);
> --
> 2.43.0

Re: [PATCH] ALSA: hda: cs35l56: Stop creating ALSA controls for firmware coefficients

[Takashi Iwai]

On Mon, 29 Jul 2024 19:14:13 +0200,
Simon Trimmer wrote:
> 
> Apologies Takashi - can you ignore this one please? It will need a V2

Sure.


thanks,

Takashi

> 
> > -----Original Message-----
> > From: Simon Trimmer <simont@opensource.cirrus.com>
> > Sent: Monday, July 29, 2024 5:16 PM
> > To: tiwai@suse.com
> > Cc: linux-sound@vger.kernel.org; alsa-devel@alsa-project.org; linux-
> > kernel@vger.kernel.org; patches@opensource.cirrus.com; Simon Trimmer
> > <simont@opensource.cirrus.com>
> > Subject: [PATCH] ALSA: hda: cs35l56: Stop creating ALSA controls for
> firmware
> > coefficients
> > 
> > A number of laptops have gone to market with old firmware versions that
> > export controls that have since been hidden, but we can't just install a
> > newer firmware because the firmware for each product is customized and
> > qualified by the OEM. The issue is that alsactl save and restore has no
> > idea what controls are good to persist which can lead to
> > misconfiguration.
> > 
> > As the ALSA controls for the firmware coefficients are not used in
> > normal operation they can all be hidden, but add a kernel parameter so
> > that they can be re-enabled for debugging.
> > 
> > Fixes: 73cfbfa9caea ("ALSA: hda/cs35l56: Add driver for Cirrus Logic
> CS35L56
> > amplifier")
> > Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
> > ---
> >  sound/pci/hda/cs35l56_hda.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
> > index 96d3f13c5abf..1494383b22c9 100644
> > --- a/sound/pci/hda/cs35l56_hda.c
> > +++ b/sound/pci/hda/cs35l56_hda.c
> > @@ -23,6 +23,10 @@
> >  #include "hda_cs_dsp_ctl.h"
> >  #include "hda_generic.h"
> > 
> > +static bool expose_dsp_controls;
> > +module_param(expose_dsp_controls, bool, 0444);
> > +MODULE_PARM_DESC(expose_dsp_controls, "Expose firmware controls as
> > ALSA controls 0=no (default), 1=yes");
> > +
> >   /*
> >    * The cs35l56_hda_dai_config[] reg sequence configures the device as
> >    *  ASP1_BCLK_FREQ = 3.072 MHz
> > @@ -758,6 +762,9 @@ static int cs35l56_hda_bind(struct device *dev, struct
> > device *master, void *mas
> > 
> >  	cs35l56_hda_create_controls(cs35l56);
> > 
> > +	if (expose_dsp_controls)
> > +		cs35l56_hda_add_dsp_controls(cs35l56);
> > +
> >  #if IS_ENABLED(CONFIG_SND_DEBUG)
> >  	cs35l56->debugfs_root = debugfs_create_dir(dev_name(cs35l56-
> > >base.dev), sound_debugfs_root);
> >  	cs_dsp_init_debugfs(&cs35l56->cs_dsp, cs35l56->debugfs_root);
> > --
> > 2.43.0
> 
>