[PATCH v1 0/2] Minor fixes for CS35L41 HDA Property driver

[PATCH v1 0/2] Minor fixes for CS35L41 HDA Property driver

[Stefan Binding]

Minor issues were found in static analysis.
First fix ensures unitialised variables will never be freed.
Second fix only compiles in the SPI workaround if SPI is enabled in the
kernel.

Stefan Binding (2):
  ALSA: hda: cs35l41: Do not allow uninitialised variables to be freed
  ALSA: hda: cs35l41: Only add SPI CS GPIO if SPI is enabled in kernel

 sound/pci/hda/cs35l41_hda_property.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH v1 1/2] ALSA: hda: cs35l41: Do not allow uninitialised variables to be freed

[Stefan Binding]

Initialise the variables to NULL so that they cannot be uninitialised
when devm_kfree is called.

Found by static analysis.

Fixes: 8c4c216db8fb ("ALSA: hda: cs35l41: Add config table to support many laptops without _DSD")

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/pci/hda/cs35l41_hda_property.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/cs35l41_hda_property.c b/sound/pci/hda/cs35l41_hda_property.c
index c9eb70290973..73b304e6c83c 100644
--- a/sound/pci/hda/cs35l41_hda_property.c
+++ b/sound/pci/hda/cs35l41_hda_property.c
@@ -77,10 +77,10 @@ static const struct cs35l41_config cs35l41_config_table[] = {
 static int cs35l41_add_gpios(struct cs35l41_hda *cs35l41, struct device *physdev, int reset_gpio,
 			     int spkid_gpio, int cs_gpio_index, int num_amps)
 {
-	struct acpi_gpio_mapping *gpio_mapping;
-	struct acpi_gpio_params *reset_gpio_params;
-	struct acpi_gpio_params *spkid_gpio_params;
-	struct acpi_gpio_params *cs_gpio_params;
+	struct acpi_gpio_mapping *gpio_mapping = NULL;
+	struct acpi_gpio_params *reset_gpio_params = NULL;
+	struct acpi_gpio_params *spkid_gpio_params = NULL;
+	struct acpi_gpio_params *cs_gpio_params = NULL;
 	unsigned int num_entries = 0;
 	unsigned int reset_index, spkid_index, csgpio_index;
 	int i;
-- 
2.34.1

[PATCH v1 2/2] ALSA: hda: cs35l41: Only add SPI CS GPIO if SPI is enabled in kernel

[Stefan Binding]

If CONFIG_SPI is not set in the kernel, there is no point in trying
to set the chip selects. We can selectively compile it.

Fixes: 8c4c216db8fb ("ALSA: hda: cs35l41: Add config table to support many laptops without _DSD")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202312192256.lJelQEoZ-lkp@intel.com/

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/pci/hda/cs35l41_hda_property.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/pci/hda/cs35l41_hda_property.c b/sound/pci/hda/cs35l41_hda_property.c
index 73b304e6c83c..194e1179a253 100644
--- a/sound/pci/hda/cs35l41_hda_property.c
+++ b/sound/pci/hda/cs35l41_hda_property.c
@@ -210,6 +210,8 @@ static int generic_dsd_config(struct cs35l41_hda *cs35l41, struct device *physde
 
 	if (cfg->bus == SPI) {
 		cs35l41->index = id;
+
+#if IS_ENABLED(CONFIG_SPI)
 		/*
 		 * Manually set the Chip Select for the second amp <cs_gpio_index> in the node.
 		 * This is only supported for systems with 2 amps, since we cannot expand the
@@ -249,6 +251,7 @@ static int generic_dsd_config(struct cs35l41_hda *cs35l41, struct device *physde
 				spi_setup(spi);
 			}
 		}
+#endif
 	} else {
 		if (cfg->num_amps > 2)
 			/*
-- 
2.34.1

Re: [PATCH v1 2/2] ALSA: hda: cs35l41: Only add SPI CS GPIO if SPI is enabled in kernel

[Takashi Iwai]

On Tue, 19 Dec 2023 17:22:32 +0100,
Stefan Binding wrote:
> 
> If CONFIG_SPI is not set in the kernel, there is no point in trying
> to set the chip selects. We can selectively compile it.

I guess it should with IS_REACHABLE() instead of IS_ENABLED()?
It can be still CONFIG_SPI=m while CONFIG_SND_HDA_*=y.


thanks,

Takashi


> 
> Fixes: 8c4c216db8fb ("ALSA: hda: cs35l41: Add config table to support many laptops without _DSD")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202312192256.lJelQEoZ-lkp@intel.com/
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---
>  sound/pci/hda/cs35l41_hda_property.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sound/pci/hda/cs35l41_hda_property.c b/sound/pci/hda/cs35l41_hda_property.c
> index 73b304e6c83c..194e1179a253 100644
> --- a/sound/pci/hda/cs35l41_hda_property.c
> +++ b/sound/pci/hda/cs35l41_hda_property.c
> @@ -210,6 +210,8 @@ static int generic_dsd_config(struct cs35l41_hda *cs35l41, struct device *physde
>  
>  	if (cfg->bus == SPI) {
>  		cs35l41->index = id;
> +
> +#if IS_ENABLED(CONFIG_SPI)
>  		/*
>  		 * Manually set the Chip Select for the second amp <cs_gpio_index> in the node.
>  		 * This is only supported for systems with 2 amps, since we cannot expand the
> @@ -249,6 +251,7 @@ static int generic_dsd_config(struct cs35l41_hda *cs35l41, struct device *physde
>  				spi_setup(spi);
>  			}
>  		}
> +#endif
>  	} else {
>  		if (cfg->num_amps > 2)
>  			/*
> -- 
> 2.34.1
> 

Re: [PATCH v1 2/2] ALSA: hda: cs35l41: Only add SPI CS GPIO if SPI is enabled in kernel

[Takashi Iwai]

On Wed, 20 Dec 2023 09:10:37 +0100,
Takashi Iwai wrote:
> 
> On Tue, 19 Dec 2023 17:22:32 +0100,
> Stefan Binding wrote:
> > 
> > If CONFIG_SPI is not set in the kernel, there is no point in trying
> > to set the chip selects. We can selectively compile it.
> 
> I guess it should with IS_REACHABLE() instead of IS_ENABLED()?
> It can be still CONFIG_SPI=m while CONFIG_SND_HDA_*=y.

In anyway, I applied the patches as is for now, as it should work in most cases.
Let's see whether the use of IS_REACHABLE() is required.


thanks,

Takashi