Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[STABLE-v2.7] drivers: hda: insert an empty ";" statement before switch() labels #73

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

marc-hb
Copy link

@marc-hb marc-hb commented Oct 20, 2023

Only statements can be labeled in C, a declaration is not valid. This is an FAQ.

While compilers currently in use don't seem to care, the "sparse" static analyzer complains loudly (and cryptically):

https://github.com/thesofproject/sof/actions/runs/6052920348/job/16427323549

drivers/dma/dma_intel_adsp_hda.c:190:17: error: typename in expression
drivers/dma/dma_intel_adsp_hda.c:190:26: error: Expected ; at end of stmt
drivers/dma/dma_intel_adsp_hda.c:190:26: error: got rp

Add an empty ";" statement after each label makes sparse and probably others happy.

Also add missing const to constants for clarity.

Fixes commit a026370 ("drivers: hda: use interrupt for timing L1 exit on host DMA")

Signed-off-by: Marc Herbert marc.herbert@intel.com
(cherry picked from commit f0fd9f1)

Only statements can be labeled in C, a declaration is not valid. This is
an FAQ.

While compilers currently in use don't seem to care, the "sparse" static
analyzer complains loudly (and cryptically):

https://github.com/thesofproject/sof/actions/runs/6052920348/job/16427323549

```
drivers/dma/dma_intel_adsp_hda.c:190:17: error: typename in expression
drivers/dma/dma_intel_adsp_hda.c:190:26: error: Expected ; at end of stmt
drivers/dma/dma_intel_adsp_hda.c:190:26: error: got rp
```

Add an empty ";" statement after each label makes `sparse` and probably
others happy.

Also add missing `const` to constants for clarity.

Fixes commit a026370 ("drivers: hda: use interrupt for timing L1
exit on host DMA")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
(cherry picked from commit f0fd9f1)
@marc-hb marc-hb changed the title drivers: hda: insert an empty ";" statement before switch() labels [STABLE-v2.7] drivers: hda: insert an empty ";" statement before switch() labels Oct 20, 2023
@marc-hb marc-hb marked this pull request as ready for review October 20, 2023 23:36
@marc-hb marc-hb requested a review from abonislawski as a code owner October 20, 2023 23:36
@marc-hb
Copy link
Author

marc-hb commented Oct 20, 2023

uint32_t next_wp = (wp + INTEL_HDA_MIN_FPI_INCREMENT_FOR_INTERRUPT) %
;
const uint32_t wp = *DGBWP(cfg->base, cfg->regblock_size, channel);
const uint32_t next_wp = (wp + INTEL_HDA_MIN_FPI_INCREMENT_FOR_INTERRUPT) %

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so mixing code with declaration is not all roses ;)

While recognizing that this is pickled from upstream, I would have addressed this issue differently.

diff --git a/drivers/dma/dma_intel_adsp_hda.c b/drivers/dma/dma_intel_adsp_hda.c
index 14768e7531a5..2d62ee1c38b0 100644
--- a/drivers/dma/dma_intel_adsp_hda.c
+++ b/drivers/dma/dma_intel_adsp_hda.c
@@ -187,6 +187,7 @@ int intel_adsp_hda_dma_host_reload(const struct device *dev, uint32_t channel,
 #endif
 	switch (cfg->direction) {
 	case HOST_TO_MEMORY:
+	{
 		uint32_t rp = *DGBRP(cfg->base, cfg->regblock_size, channel);
 		uint32_t next_rp = (rp + INTEL_HDA_MIN_FPI_INCREMENT_FOR_INTERRUPT) %
 			intel_adsp_hda_get_buffer_size(cfg->base, cfg->regblock_size, channel);
@@ -195,7 +196,9 @@ int intel_adsp_hda_dma_host_reload(const struct device *dev, uint32_t channel,
 						      channel, next_rp);
 		intel_adsp_hda_enable_buffer_interrupt(cfg->base, cfg->regblock_size, channel);
 		break;
+	}
 	case MEMORY_TO_HOST:
+	{
 		uint32_t wp = *DGBWP(cfg->base, cfg->regblock_size, channel);
 		uint32_t next_wp = (wp + INTEL_HDA_MIN_FPI_INCREMENT_FOR_INTERRUPT) %
 			intel_adsp_hda_get_buffer_size(cfg->base, cfg->regblock_size, channel);
@@ -204,6 +207,7 @@ int intel_adsp_hda_dma_host_reload(const struct device *dev, uint32_t channel,
 						      channel, next_wp);
 		intel_adsp_hda_enable_buffer_interrupt(cfg->base, cfg->regblock_size, channel);
 		break;
+	}
 	default:
 		break;
 	}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so mixing code with declaration is not all roses ;)

Someone forgot to update the formal language grammar in 1999. sparse seems to be the only one which cares.

BTW sparse is also unmaintained, can't do exit codes and has infinite loops:

I would have addressed this issue differently.

Feel free to send a patch upstream. If anyone cares enough we can then cherry-pick it on top of this one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's handle this in upstream, I can pick this one for the stable series as this is already upstream.

@kv2019i kv2019i merged commit feaa628 into thesofproject:stable-v2.7 Oct 23, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants