[PATCH v1 0/2] Minor fixes for CS35L41 HDA Property driver
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(-)
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;
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) /*
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
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
participants (2)
-
Stefan Binding -
Takashi Iwai