[PATCH 0/9] ALSA: remove redundant variables

[PATCH 0/9] ALSA: remove redundant variables

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Clean up code by removing redundant variables that were
detected by clang

Colin Ian King (9):
  ALSA: asihpi: remove redundant variable max_streams
  ALSA: cs5535audio: remove redundant pointer 'dma'
  ALSA: emu10k1: remove redundant variable attn
  ALSA: ens137x: remove redundant array pcm_devs
  ALSA: sonicvibes: remove redundant pointer 'dir'
  ASoC: nau8540: remove redundant variable osrate
  ASoC: stm32: remove redundant pointers 'priv' and 'rtd'
  ALSA: usb-audio: remove redundant pointer 'urb'
  ALSA: intel_hdmi: remove redundant variable cfg_val

 sound/pci/asihpi/hpi6205.c          | 5 -----
 sound/pci/cs5535audio/cs5535audio.c | 7 +------
 sound/pci/emu10k1/emupcm.c          | 3 +--
 sound/pci/ens1370.c                 | 3 +--
 sound/pci/sonicvibes.c              | 2 --
 sound/soc/codecs/nau8540.c          | 3 ---
 sound/soc/stm/stm32_adfsdm.c        | 8 +-------
 sound/usb/endpoint.c                | 2 --
 sound/x86/intel_hdmi_audio.c        | 2 --
 9 files changed, 4 insertions(+), 31 deletions(-)

-- 
2.17.1

[PATCH 1/9] ALSA: asihpi: remove redundant variable max_streams

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Variable max_streams is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'max_streams' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/pci/asihpi/hpi6205.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/sound/pci/asihpi/hpi6205.c b/sound/pci/asihpi/hpi6205.c
index 8d5abfa4e24b..2864698436a5 100644
--- a/sound/pci/asihpi/hpi6205.c
+++ b/sound/pci/asihpi/hpi6205.c
@@ -635,7 +635,6 @@ static u16 create_adapter_obj(struct hpi_adapter_obj *pao,
 	{
 		struct hpi_message hm;
 		struct hpi_response hr;
-		u32 max_streams;
 
 		HPI_DEBUG_LOG(VERBOSE, "init ADAPTER_GET_INFO\n");
 		memset(&hm, 0, sizeof(hm));
@@ -660,10 +659,6 @@ static u16 create_adapter_obj(struct hpi_adapter_obj *pao,
 		pao->type = hr.u.ax.info.adapter_type;
 		pao->index = hr.u.ax.info.adapter_index;
 
-		max_streams =
-			hr.u.ax.info.num_outstreams +
-			hr.u.ax.info.num_instreams;
-
 		HPI_DEBUG_LOG(VERBOSE,
 			"got adapter info type %x index %d serial %d\n",
 			hr.u.ax.info.adapter_type, hr.u.ax.info.adapter_index,
-- 
2.17.1

Re: [PATCH 1/9] ALSA: asihpi: remove redundant variable max_streams

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:34 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable max_streams is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'max_streams' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.


Takashi

[PATCH 2/9] ALSA: cs5535audio: remove redundant pointer 'dma'

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Pointer 'dma' is being assigned but is never used hence it is
redundant and can be removed.

Cleans up two clang warnings:
warning: variable 'dma' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/pci/cs5535audio/cs5535audio.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sound/pci/cs5535audio/cs5535audio.c b/sound/pci/cs5535audio/cs5535audio.c
index de409cda50aa..4590086d9cd8 100644
--- a/sound/pci/cs5535audio/cs5535audio.c
+++ b/sound/pci/cs5535audio/cs5535audio.c
@@ -192,8 +192,6 @@ static void process_bm0_irq(struct cs5535audio *cs5535au)
 	bm_stat = cs_readb(cs5535au, ACC_BM0_STATUS);
 	spin_unlock(&cs5535au->reg_lock);
 	if (bm_stat & EOP) {
-		struct cs5535audio_dma *dma;
-		dma = cs5535au->playback_substream->runtime->private_data;
 		snd_pcm_period_elapsed(cs5535au->playback_substream);
 	} else {
 		dev_err(cs5535au->card->dev,
@@ -208,11 +206,8 @@ static void process_bm1_irq(struct cs5535audio *cs5535au)
 	spin_lock(&cs5535au->reg_lock);
 	bm_stat = cs_readb(cs5535au, ACC_BM1_STATUS);
 	spin_unlock(&cs5535au->reg_lock);
-	if (bm_stat & EOP) {
-		struct cs5535audio_dma *dma;
-		dma = cs5535au->capture_substream->runtime->private_data;
+	if (bm_stat & EOP)
 		snd_pcm_period_elapsed(cs5535au->capture_substream);
-	}
 }
 
 static irqreturn_t snd_cs5535audio_interrupt(int irq, void *dev_id)
-- 
2.17.1

Re: [PATCH 2/9] ALSA: cs5535audio: remove redundant pointer 'dma'

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:35 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Pointer 'dma' is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up two clang warnings:
> warning: variable 'dma' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.


Takashi

[PATCH 3/9] ALSA: emu10k1: remove redundant variable attn

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Variable attn is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'attn' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/pci/emu10k1/emupcm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index 69f9b100bd24..26f6eda3e766 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -290,7 +290,7 @@ static void snd_emu10k1_pcm_init_voice(struct snd_emu10k1 *emu,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	unsigned int silent_page, tmp;
 	int voice, stereo, w_16;
-	unsigned char attn, send_amount[8];
+	unsigned char send_amount[8];
 	unsigned char send_routing[8];
 	unsigned long flags;
 	unsigned int pitch_target;
@@ -313,7 +313,6 @@ static void snd_emu10k1_pcm_init_voice(struct snd_emu10k1 *emu,
 
 	/* volume parameters */
 	if (extra) {
-		attn = 0;
 		memset(send_routing, 0, sizeof(send_routing));
 		send_routing[0] = 0;
 		send_routing[1] = 1;
-- 
2.17.1

Re: [PATCH 3/9] ALSA: emu10k1: remove redundant variable attn

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:36 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable attn is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'attn' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.


Takashi

[PATCH 4/9] ALSA: ens137x: remove redundant array pcm_devs

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The array pcm_devs is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'pcm_devs' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/pci/ens1370.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/pci/ens1370.c b/sound/pci/ens1370.c
index 39f79a6b5283..727eb3da1fda 100644
--- a/sound/pci/ens1370.c
+++ b/sound/pci/ens1370.c
@@ -2392,7 +2392,7 @@ static int snd_audiopci_probe(struct pci_dev *pci,
 	static int dev;
 	struct snd_card *card;
 	struct ensoniq *ensoniq;
-	int err, pcm_devs[2];
+	int err;
 
 	if (dev >= SNDRV_CARDS)
 		return -ENODEV;
@@ -2412,7 +2412,6 @@ static int snd_audiopci_probe(struct pci_dev *pci,
 	}
 	card->private_data = ensoniq;
 
-	pcm_devs[0] = 0; pcm_devs[1] = 1;
 #ifdef CHIP1370
 	if ((err = snd_ensoniq_1370_mixer(ensoniq)) < 0) {
 		snd_card_free(card);
-- 
2.17.1

Re: [PATCH 4/9] ALSA: ens137x: remove redundant array pcm_devs

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:37 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> The array pcm_devs is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'pcm_devs' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.


Takashi

[PATCH 5/9] ALSA: sonicvibes: remove redundant pointer 'dir'

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Pointer 'dir' is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'dir' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/pci/sonicvibes.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
index 7fbdb703bfcd..7218f38b59db 100644
--- a/sound/pci/sonicvibes.c
+++ b/sound/pci/sonicvibes.c
@@ -1433,14 +1433,12 @@ static int snd_sonicvibes_midi(struct sonicvibes *sonic,
 {
 	struct snd_mpu401 * mpu = rmidi->private_data;
 	struct snd_card *card = sonic->card;
-	struct snd_rawmidi_str *dir;
 	unsigned int idx;
 	int err;
 
 	mpu->private_data = sonic;
 	mpu->open_input = snd_sonicvibes_midi_input_open;
 	mpu->close_input = snd_sonicvibes_midi_input_close;
-	dir = &rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT];
 	for (idx = 0; idx < ARRAY_SIZE(snd_sonicvibes_midi_controls); idx++)
 		if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_sonicvibes_midi_controls[idx], sonic))) < 0)
 			return err;
-- 
2.17.1

Re: [PATCH 5/9] ALSA: sonicvibes: remove redundant pointer 'dir'

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:38 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Pointer 'dir' is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'dir' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.


Takashi

[PATCH 6/9] ASoC: nau8540: remove redundant variable osrate

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Variable osrate is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'osrate' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/soc/codecs/nau8540.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/codecs/nau8540.c b/sound/soc/codecs/nau8540.c
index 17104f8dc1a9..e3c8cd17daf2 100644
--- a/sound/soc/codecs/nau8540.c
+++ b/sound/soc/codecs/nau8540.c
@@ -362,11 +362,8 @@ static const struct snd_soc_dapm_route nau8540_dapm_routes[] = {
 
 static int nau8540_clock_check(struct nau8540 *nau8540, int rate, int osr)
 {
-	int osrate;
-
 	if (osr >= ARRAY_SIZE(osr_adc_sel))
 		return -EINVAL;
-	osrate = osr_adc_sel[osr].osr;
 
 	if (rate * osr > CLK_ADC_MAX) {
 		dev_err(nau8540->dev, "exceed the maximum frequency of CLK_ADC\n");
-- 
2.17.1

Applied "ASoC: nau8540: remove redundant variable osrate" to the asoc tree

[Mark Brown]

The patch

   ASoC: nau8540: remove redundant variable osrate

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From d101f9b96ee08f0454989bc3adb10e6cf7f3f953 Mon Sep 17 00:00:00 2001
From: Colin Ian King <colin.king@canonical.com>
Date: Wed, 1 Aug 2018 11:47:39 +0100
Subject: [PATCH] ASoC: nau8540: remove redundant variable osrate

Variable osrate is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'osrate' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/nau8540.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/codecs/nau8540.c b/sound/soc/codecs/nau8540.c
index 17104f8dc1a9..e3c8cd17daf2 100644
--- a/sound/soc/codecs/nau8540.c
+++ b/sound/soc/codecs/nau8540.c
@@ -362,11 +362,8 @@ static const struct snd_soc_dapm_route nau8540_dapm_routes[] = {
 
 static int nau8540_clock_check(struct nau8540 *nau8540, int rate, int osr)
 {
-	int osrate;
-
 	if (osr >= ARRAY_SIZE(osr_adc_sel))
 		return -EINVAL;
-	osrate = osr_adc_sel[osr].osr;
 
 	if (rate * osr > CLK_ADC_MAX) {
 		dev_err(nau8540->dev, "exceed the maximum frequency of CLK_ADC\n");
-- 
2.18.0

[PATCH 7/9] ASoC: stm32: remove redundant pointers 'priv' and 'rtd'

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Pointer 'priv' is assigned and not used, removing this allows
the removal of pointer 'rtd'.

Cleans up clang warning:
warning: variable 'priv' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/soc/stm/stm32_adfsdm.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/sound/soc/stm/stm32_adfsdm.c b/sound/soc/stm/stm32_adfsdm.c
index 0e9373064032..706ff005234f 100644
--- a/sound/soc/stm/stm32_adfsdm.c
+++ b/sound/soc/stm/stm32_adfsdm.c
@@ -269,16 +269,10 @@ static int stm32_adfsdm_pcm_new(struct snd_soc_pcm_runtime *rtd)
 static void stm32_adfsdm_pcm_free(struct snd_pcm *pcm)
 {
 	struct snd_pcm_substream *substream;
-	struct snd_soc_pcm_runtime *rtd;
-	struct stm32_adfsdm_priv *priv;
 
 	substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
-	if (substream) {
-		rtd = substream->private_data;
-		priv = snd_soc_dai_get_drvdata(rtd->cpu_dai);
-
+	if (substream)
 		snd_pcm_lib_preallocate_free_for_all(pcm);
-	}
 }
 
 static struct snd_soc_component_driver stm32_adfsdm_soc_platform = {
-- 
2.17.1

Applied "ASoC: stm32: remove redundant pointers 'priv' and 'rtd'" to the asoc tree

[Mark Brown]

The patch

   ASoC: stm32: remove redundant pointers 'priv' and 'rtd'

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 18127744cf446f113ca33f07e5cea893388f781a Mon Sep 17 00:00:00 2001
From: Colin Ian King <colin.king@canonical.com>
Date: Wed, 1 Aug 2018 11:47:40 +0100
Subject: [PATCH] ASoC: stm32: remove redundant pointers 'priv' and 'rtd'

Pointer 'priv' is assigned and not used, removing this allows
the removal of pointer 'rtd'.

Cleans up clang warning:
warning: variable 'priv' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/stm/stm32_adfsdm.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/sound/soc/stm/stm32_adfsdm.c b/sound/soc/stm/stm32_adfsdm.c
index 0e9373064032..706ff005234f 100644
--- a/sound/soc/stm/stm32_adfsdm.c
+++ b/sound/soc/stm/stm32_adfsdm.c
@@ -269,16 +269,10 @@ static int stm32_adfsdm_pcm_new(struct snd_soc_pcm_runtime *rtd)
 static void stm32_adfsdm_pcm_free(struct snd_pcm *pcm)
 {
 	struct snd_pcm_substream *substream;
-	struct snd_soc_pcm_runtime *rtd;
-	struct stm32_adfsdm_priv *priv;
 
 	substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
-	if (substream) {
-		rtd = substream->private_data;
-		priv = snd_soc_dai_get_drvdata(rtd->cpu_dai);
-
+	if (substream)
 		snd_pcm_lib_preallocate_free_for_all(pcm);
-	}
 }
 
 static struct snd_soc_component_driver stm32_adfsdm_soc_platform = {
-- 
2.18.0

[PATCH 8/9] ALSA: usb-audio: remove redundant pointer 'urb'

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Pointer 'urb' is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'urb' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/usb/endpoint.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index c90607ebe155..d86be8bfe412 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -325,7 +325,6 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep)
 		unsigned long flags;
 		struct snd_usb_packet_info *uninitialized_var(packet);
 		struct snd_urb_ctx *ctx = NULL;
-		struct urb *urb;
 		int err, i;
 
 		spin_lock_irqsave(&ep->lock, flags);
@@ -345,7 +344,6 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep)
 			return;
 
 		list_del_init(&ctx->ready_list);
-		urb = ctx->urb;
 
 		/* copy over the length information */
 		for (i = 0; i < packet->packets; i++)
-- 
2.17.1

Re: [PATCH 8/9] ALSA: usb-audio: remove redundant pointer 'urb'

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:41 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Pointer 'urb' is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'urb' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.


Takashi

[PATCH 9/9] ALSA: intel_hdmi: remove redundant variable cfg_val

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

Variable cfg_val is being assigned but is never used hence it is
redundant and can be removed.

Cleans up clang warning:
warning: variable 'cfg_val' set but not used [-Wunused-but-set-variable]

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 sound/x86/intel_hdmi_audio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index edc9f5a34eff..fa7dca5a68c8 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -290,7 +290,6 @@ static void had_reset_audio(struct snd_intelhad *intelhaddata)
 static int had_prog_status_reg(struct snd_pcm_substream *substream,
 			struct snd_intelhad *intelhaddata)
 {
-	union aud_cfg cfg_val = {.regval = 0};
 	union aud_ch_status_0 ch_stat0 = {.regval = 0};
 	union aud_ch_status_1 ch_stat1 = {.regval = 0};
 
@@ -298,7 +297,6 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
 					  IEC958_AES0_NONAUDIO) >> 1;
 	ch_stat0.regx.clk_acc = (intelhaddata->aes_bits &
 					  IEC958_AES3_CON_CLOCK) >> 4;
-	cfg_val.regx.val_bit = ch_stat0.regx.lpcm_id;
 
 	switch (substream->runtime->rate) {
 	case AUD_SAMPLE_RATE_32:
-- 
2.17.1

Re: [PATCH 9/9] ALSA: intel_hdmi: remove redundant variable cfg_val

[Takashi Iwai]

On Wed, 01 Aug 2018 12:47:42 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable cfg_val is being assigned but is never used hence it is
> redundant and can be removed.
> 
> Cleans up clang warning:
> warning: variable 'cfg_val' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  sound/x86/intel_hdmi_audio.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
> index edc9f5a34eff..fa7dca5a68c8 100644
> --- a/sound/x86/intel_hdmi_audio.c
> +++ b/sound/x86/intel_hdmi_audio.c
> @@ -290,7 +290,6 @@ static void had_reset_audio(struct snd_intelhad *intelhaddata)
>  static int had_prog_status_reg(struct snd_pcm_substream *substream,
>  			struct snd_intelhad *intelhaddata)
>  {
> -	union aud_cfg cfg_val = {.regval = 0};
>  	union aud_ch_status_0 ch_stat0 = {.regval = 0};
>  	union aud_ch_status_1 ch_stat1 = {.regval = 0};
>  
> @@ -298,7 +297,6 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
>  					  IEC958_AES0_NONAUDIO) >> 1;
>  	ch_stat0.regx.clk_acc = (intelhaddata->aes_bits &
>  					  IEC958_AES3_CON_CLOCK) >> 4;
> -	cfg_val.regx.val_bit = ch_stat0.regx.lpcm_id;

Actually this made me wonder what is the definition of val_bit.
It seems always 1 in the current code after the commit 964ca8083c02.
Pierre?


thanks,

Takashi

Re: [alsa-devel] [PATCH 9/9] ALSA: intel_hdmi: remove redundant variable cfg_val

[Pierre-Louis Bossart]

On 8/1/18 4:00 AM, Takashi Iwai wrote:
> On Wed, 01 Aug 2018 12:47:42 +0200,
> Colin King wrote:
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Variable cfg_val is being assigned but is never used hence it is
>> redundant and can be removed.
>>
>> Cleans up clang warning:
>> warning: variable 'cfg_val' set but not used [-Wunused-but-set-variable]
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   sound/x86/intel_hdmi_audio.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
>> index edc9f5a34eff..fa7dca5a68c8 100644
>> --- a/sound/x86/intel_hdmi_audio.c
>> +++ b/sound/x86/intel_hdmi_audio.c
>> @@ -290,7 +290,6 @@ static void had_reset_audio(struct snd_intelhad *intelhaddata)
>>   static int had_prog_status_reg(struct snd_pcm_substream *substream,
>>   			struct snd_intelhad *intelhaddata)
>>   {
>> -	union aud_cfg cfg_val = {.regval = 0};
>>   	union aud_ch_status_0 ch_stat0 = {.regval = 0};
>>   	union aud_ch_status_1 ch_stat1 = {.regval = 0};
>>   
>> @@ -298,7 +297,6 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
>>   					  IEC958_AES0_NONAUDIO) >> 1;
>>   	ch_stat0.regx.clk_acc = (intelhaddata->aes_bits &
>>   					  IEC958_AES3_CON_CLOCK) >> 4;
>> -	cfg_val.regx.val_bit = ch_stat0.regx.lpcm_id;
> 
> Actually this made me wonder what is the definition of val_bit.
> It seems always 1 in the current code after the commit 964ca8083c02.
> Pierre?

I'll look into it, give me a couple of days. I vaguely remember this is 
the AES validity bit but I am not sure why it's set with a pcm id, looks 
like a mixup.

Re: [alsa-devel] [PATCH 9/9] ALSA: intel_hdmi: remove redundant variable cfg_val

[Pierre-Louis Bossart]

On 8/1/18 6:00 AM, Takashi Iwai wrote:
> On Wed, 01 Aug 2018 12:47:42 +0200,
> Colin King wrote:
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Variable cfg_val is being assigned but is never used hence it is
>> redundant and can be removed.
>>
>> Cleans up clang warning:
>> warning: variable 'cfg_val' set but not used [-Wunused-but-set-variable]
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   sound/x86/intel_hdmi_audio.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
>> index edc9f5a34eff..fa7dca5a68c8 100644
>> --- a/sound/x86/intel_hdmi_audio.c
>> +++ b/sound/x86/intel_hdmi_audio.c
>> @@ -290,7 +290,6 @@ static void had_reset_audio(struct snd_intelhad *intelhaddata)
>>   static int had_prog_status_reg(struct snd_pcm_substream *substream,
>>   			struct snd_intelhad *intelhaddata)
>>   {
>> -	union aud_cfg cfg_val = {.regval = 0};
>>   	union aud_ch_status_0 ch_stat0 = {.regval = 0};
>>   	union aud_ch_status_1 ch_stat1 = {.regval = 0};
>>   
>> @@ -298,7 +297,6 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream,
>>   					  IEC958_AES0_NONAUDIO) >> 1;
>>   	ch_stat0.regx.clk_acc = (intelhaddata->aes_bits &
>>   					  IEC958_AES3_CON_CLOCK) >> 4;
>> -	cfg_val.regx.val_bit = ch_stat0.regx.lpcm_id;
> 
> Actually this made me wonder what is the definition of val_bit.
> It seems always 1 in the current code after the commit 964ca8083c02.
> Pierre?

This val_bit is only there for debug/test, it should be set to one by 
default and has nothing to do with the lpcm_id. This variable was set 
even in patches before upstream submission and was never needed, I guess 
it must be a 9-yr old issue. Good catch!

Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

BTW looking at the structures in intel_hdmi_lpe_audio.h, the field 
format:3 in audio_ch_status_0 is misleading at best, it should be the 
emphasis if I look at the SPDIF/AES docs. It's not used but I'll have to 
relook at all this when I find some time... We should really set the 
registers to mirror what the aes_bits are, not just the non_audio and 
clock accuracy fields.

Re: [alsa-devel] [PATCH 9/9] ALSA: intel_hdmi: remove redundant variable cfg_val

[Takashi Iwai]

On Mon, 06 Aug 2018 20:37:53 +0200,
Pierre-Louis Bossart wrote:
> 
> On 8/1/18 6:00 AM, Takashi Iwai wrote:
> > On Wed, 01 Aug 2018 12:47:42 +0200,
> > Colin King wrote:
> >>
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> Variable cfg_val is being assigned but is never used hence it is
> >> redundant and can be removed.
> >>
> >> Cleans up clang warning:
> >> warning: variable 'cfg_val' set but not used [-Wunused-but-set-variable]
> >>
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >>   sound/x86/intel_hdmi_audio.c | 2 --
> >>   1 file changed, 2 deletions(-)
> >>
> >> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
> >> index edc9f5a34eff..fa7dca5a68c8 100644
> >> --- a/sound/x86/intel_hdmi_audio.c
> >> +++ b/sound/x86/intel_hdmi_audio.c
> >> @@ -290,7 +290,6 @@ static void had_reset_audio(struct snd_intelhad *intelhaddata)
> >>   static int had_prog_status_reg(struct snd_pcm_substream *substream,
> >>   			struct snd_intelhad *intelhaddata)
> >>   {
> >> -	union aud_cfg cfg_val = {.regval = 0};
> >>   	union aud_ch_status_0 ch_stat0 = {.regval = 0};
> >>   	union aud_ch_status_1 ch_stat1 = {.regval = 0};
> >>   @@ -298,7 +297,6 @@ static int had_prog_status_reg(struct
> >> snd_pcm_substream *substream,
> >>   					  IEC958_AES0_NONAUDIO) >> 1;
> >>   	ch_stat0.regx.clk_acc = (intelhaddata->aes_bits &
> >>   					  IEC958_AES3_CON_CLOCK) >> 4;
> >> -	cfg_val.regx.val_bit = ch_stat0.regx.lpcm_id;
> >
> > Actually this made me wonder what is the definition of val_bit.
> > It seems always 1 in the current code after the commit 964ca8083c02.
> > Pierre?
> 
> This val_bit is only there for debug/test, it should be set to one by
> default and has nothing to do with the lpcm_id. This variable was set
> even in patches before upstream submission and was never needed, I
> guess it must be a 9-yr old issue. Good catch!
> 
> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Good to hear, applied the patch now.

> BTW looking at the structures in intel_hdmi_lpe_audio.h, the field
> format:3 in audio_ch_status_0 is misleading at best, it should be the
> emphasis if I look at the SPDIF/AES docs. It's not used but I'll have
> to relook at all this when I find some time... We should really set
> the registers to mirror what the aes_bits are, not just the non_audio
> and clock accuracy fields.

A fix patch is always welcome ;)


Takashi

Re: [alsa-devel] [PATCH 9/9] ALSA: intel_hdmi: remove redundant variable cfg_val

[Pierre-Louis Bossart]

> Good to hear, applied the patch now.
>
>> BTW looking at the structures in intel_hdmi_lpe_audio.h, the field
>> format:3 in audio_ch_status_0 is misleading at best, it should be the
>> emphasis if I look at the SPDIF/AES docs. It's not used but I'll have
>> to relook at all this when I find some time... We should really set
>> the registers to mirror what the aes_bits are, not just the non_audio
>> and clock accuracy fields.
> A fix patch is always welcome ;)
This sort of fixes is straighforward, the hard part is to locate an HDMI 
analyzer to double-check that the changes result in something meaningful...