diff mbox series

[6/6] ASoC: meson: axg-fifo: use FIELD helpers

Message ID 20240223175116.2005407-7-jbrunet@baylibre.com (mailing list archive)
State New, archived
Headers show
Series ASoC: meson: axg fixes and clean-up | expand

Commit Message

Jerome Brunet Feb. 23, 2024, 5:51 p.m. UTC 
Use FIELD_GET() and FIELD_PREP() helpers instead of doing it manually.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/meson/axg-fifo.c  | 24 ++++++++++++------------
 sound/soc/meson/axg-fifo.h  | 12 +++++-------
 sound/soc/meson/axg-frddr.c |  4 ++--
 sound/soc/meson/axg-toddr.c | 21 +++++++++------------
 4 files changed, 28 insertions(+), 33 deletions(-)

Comments

Mark Brown Feb. 26, 2024, 3:42 p.m. UTC | #1 
On Fri, Feb 23, 2024 at 06:51:12PM +0100, Jerome Brunet wrote:

> Use FIELD_GET() and FIELD_PREP() helpers instead of doing it manually.

This breaks the build for me:

/build/stage/linux/sound/soc/meson/axg-fifo.c: In function ‘axg_fifo_pcm_hw_para
ms’:
/build/stage/linux/sound/soc/meson/axg-fifo.c:149:28: error: implicit declaratio
n of function ‘FIELD_PREP’ [-Werror=implicit-function-declaration]
  149 |                            FIELD_PREP(CTRL0_INT_EN, irq_en));
      |                            ^~~~~~~~~~
/build/stage/linux/sound/soc/meson/axg-fifo.c: In function ‘axg_fifo_pcm_irq_blo
ck’:
/build/stage/linux/sound/soc/meson/axg-fifo.c:207:18: error: implicit declaratio
n of function ‘FIELD_GET’ [-Werror=implicit-function-declaration]
  207 |         status = FIELD_GET(STATUS1_INT_STS, status);
      |                  ^~~~~~~~~
cc1: all warnings being treated as errors
Jerome Brunet Feb. 26, 2024, 6:15 p.m. UTC | #2 
On Mon 26 Feb 2024 at 15:42, Mark Brown <broonie@kernel.org> wrote:

> [[PGP Signed Part:Undecided]]
> On Fri, Feb 23, 2024 at 06:51:12PM +0100, Jerome Brunet wrote:
>
>> Use FIELD_GET() and FIELD_PREP() helpers instead of doing it manually.
>
> This breaks the build for me:
>
> /build/stage/linux/sound/soc/meson/axg-fifo.c: In function ‘axg_fifo_pcm_hw_para
> ms’:
> /build/stage/linux/sound/soc/meson/axg-fifo.c:149:28: error: implicit declaratio
> n of function ‘FIELD_PREP’ [-Werror=implicit-function-declaration]
>   149 |                            FIELD_PREP(CTRL0_INT_EN, irq_en));
>       |                            ^~~~~~~~~~
> /build/stage/linux/sound/soc/meson/axg-fifo.c: In function ‘axg_fifo_pcm_irq_blo
> ck’:
> /build/stage/linux/sound/soc/meson/axg-fifo.c:207:18: error: implicit declaratio
> n of function ‘FIELD_GET’ [-Werror=implicit-function-declaration]
>   207 |         status = FIELD_GET(STATUS1_INT_STS, status);
>       |                  ^~~~~~~~~
> cc1: all warnings being treated as errors

Ah, I forgot to include <linux/bitfield.h>. Thanks for letting me know.
I'll fix it

It was fine when I tested with ARCH=arm64 and the default defconfig so
most likely it got indirectly included.

Could you tell me which ARCH/defconfig triggered this ?

>
> [[End of PGP Signed Part]]
Mark Brown Feb. 26, 2024, 6:22 p.m. UTC | #3 
On Mon, Feb 26, 2024 at 07:15:41PM +0100, Jerome Brunet wrote:
> On Mon 26 Feb 2024 at 15:42, Mark Brown <broonie@kernel.org> wrote:

> > /build/stage/linux/sound/soc/meson/axg-fifo.c:207:18: error: implicit declaratio
> > n of function ‘FIELD_GET’ [-Werror=implicit-function-declaration]
> >   207 |         status = FIELD_GET(STATUS1_INT_STS, status);
> >       |                  ^~~~~~~~~
> > cc1: all warnings being treated as errors

> Ah, I forgot to include <linux/bitfield.h>. Thanks for letting me know.
> I'll fix it

> It was fine when I tested with ARCH=arm64 and the default defconfig so
> most likely it got indirectly included.

> Could you tell me which ARCH/defconfig triggered this ?

Probably an x86 allmodconfig.
diff mbox series

Patch

diff --git a/sound/soc/meson/axg-fifo.c b/sound/soc/meson/axg-fifo.c
index 65541fdb0038..597fd39e6e48 100644
--- a/sound/soc/meson/axg-fifo.c
+++ b/sound/soc/meson/axg-fifo.c
@@ -145,8 +145,8 @@  int axg_fifo_pcm_hw_params(struct snd_soc_component *component,
 	/* Enable irq if necessary  */
 	irq_en = runtime->no_period_wakeup ? 0 : FIFO_INT_COUNT_REPEAT;
 	regmap_update_bits(fifo->map, FIFO_CTRL0,
-			   CTRL0_INT_EN(FIFO_INT_COUNT_REPEAT),
-			   CTRL0_INT_EN(irq_en));
+			   CTRL0_INT_EN,
+			   FIELD_PREP(CTRL0_INT_EN, irq_en));
 
 	return 0;
 }
@@ -176,9 +176,9 @@  int axg_fifo_pcm_hw_free(struct snd_soc_component *component,
 {
 	struct axg_fifo *fifo = axg_fifo_data(ss);
 
-	/* Disable the block count irq */
+	/* Disable irqs */
 	regmap_update_bits(fifo->map, FIFO_CTRL0,
-			   CTRL0_INT_EN(FIFO_INT_COUNT_REPEAT), 0);
+			   CTRL0_INT_EN, 0);
 
 	return 0;
 }
@@ -187,13 +187,13 @@  EXPORT_SYMBOL_GPL(axg_fifo_pcm_hw_free);
 static void axg_fifo_ack_irq(struct axg_fifo *fifo, u8 mask)
 {
 	regmap_update_bits(fifo->map, FIFO_CTRL1,
-			   CTRL1_INT_CLR(FIFO_INT_MASK),
-			   CTRL1_INT_CLR(mask));
+			   CTRL1_INT_CLR,
+			   FIELD_PREP(CTRL1_INT_CLR, mask));
 
 	/* Clear must also be cleared */
 	regmap_update_bits(fifo->map, FIFO_CTRL1,
-			   CTRL1_INT_CLR(FIFO_INT_MASK),
-			   0);
+			   CTRL1_INT_CLR,
+			   FIELD_PREP(CTRL1_INT_CLR, 0));
 }
 
 static irqreturn_t axg_fifo_pcm_irq_block(int irq, void *dev_id)
@@ -204,7 +204,7 @@  static irqreturn_t axg_fifo_pcm_irq_block(int irq, void *dev_id)
 
 	regmap_read(fifo->map, FIFO_STATUS1, &status);
 
-	status = STATUS1_INT_STS(status) & FIFO_INT_MASK;
+	status = FIELD_GET(STATUS1_INT_STS, status);
 	if (status & FIFO_INT_COUNT_REPEAT)
 		snd_pcm_period_elapsed(ss);
 	else
@@ -254,15 +254,15 @@  int axg_fifo_pcm_open(struct snd_soc_component *component,
 
 	/* Setup status2 so it reports the memory pointer */
 	regmap_update_bits(fifo->map, FIFO_CTRL1,
-			   CTRL1_STATUS2_SEL_MASK,
-			   CTRL1_STATUS2_SEL(STATUS2_SEL_DDR_READ));
+			   CTRL1_STATUS2_SEL,
+			   FIELD_PREP(CTRL1_STATUS2_SEL, STATUS2_SEL_DDR_READ));
 
 	/* Make sure the dma is initially disabled */
 	__dma_enable(fifo, false);
 
 	/* Disable irqs until params are ready */
 	regmap_update_bits(fifo->map, FIFO_CTRL0,
-			   CTRL0_INT_EN(FIFO_INT_MASK), 0);
+			   CTRL0_INT_EN, 0);
 
 	/* Clear any pending interrupt */
 	axg_fifo_ack_irq(fifo, FIFO_INT_MASK);
diff --git a/sound/soc/meson/axg-fifo.h b/sound/soc/meson/axg-fifo.h
index a14c31eb06d8..4c48c0a08481 100644
--- a/sound/soc/meson/axg-fifo.h
+++ b/sound/soc/meson/axg-fifo.h
@@ -40,21 +40,19 @@  struct snd_soc_pcm_runtime;
 
 #define FIFO_CTRL0			0x00
 #define  CTRL0_DMA_EN			BIT(31)
-#define  CTRL0_INT_EN(x)		((x) << 16)
+#define  CTRL0_INT_EN			GENMASK(23, 16)
 #define  CTRL0_SEL_MASK			GENMASK(2, 0)
 #define  CTRL0_SEL_SHIFT		0
 #define FIFO_CTRL1			0x04
-#define  CTRL1_INT_CLR(x)		((x) << 0)
-#define  CTRL1_STATUS2_SEL_MASK		GENMASK(11, 8)
-#define  CTRL1_STATUS2_SEL(x)		((x) << 8)
+#define  CTRL1_INT_CLR			GENMASK(7, 0)
+#define  CTRL1_STATUS2_SEL		GENMASK(11, 8)
 #define   STATUS2_SEL_DDR_READ		0
-#define  CTRL1_FRDDR_DEPTH_MASK		GENMASK(31, 24)
-#define  CTRL1_FRDDR_DEPTH(x)		((x) << 24)
+#define  CTRL1_FRDDR_DEPTH		GENMASK(31, 24)
 #define FIFO_START_ADDR			0x08
 #define FIFO_FINISH_ADDR		0x0c
 #define FIFO_INT_ADDR			0x10
 #define FIFO_STATUS1			0x14
-#define  STATUS1_INT_STS(x)		((x) << 0)
+#define  STATUS1_INT_STS		GENMASK(7, 0)
 #define FIFO_STATUS2			0x18
 #define FIFO_INIT_ADDR			0x24
 #define FIFO_CTRL2			0x28
diff --git a/sound/soc/meson/axg-frddr.c b/sound/soc/meson/axg-frddr.c
index 98140f449eb3..97ca0ea5faa5 100644
--- a/sound/soc/meson/axg-frddr.c
+++ b/sound/soc/meson/axg-frddr.c
@@ -59,8 +59,8 @@  static int axg_frddr_dai_hw_params(struct snd_pcm_substream *substream,
 	/* Trim the FIFO depth if the period is small to improve latency */
 	depth = min(period, fifo->depth);
 	val = (depth / AXG_FIFO_BURST) - 1;
-	regmap_update_bits(fifo->map, FIFO_CTRL1, CTRL1_FRDDR_DEPTH_MASK,
-			   CTRL1_FRDDR_DEPTH(val));
+	regmap_update_bits(fifo->map, FIFO_CTRL1, CTRL1_FRDDR_DEPTH,
+			   FIELD_PREP(CTRL1_FRDDR_DEPTH, val));
 
 	return 0;
 }
diff --git a/sound/soc/meson/axg-toddr.c b/sound/soc/meson/axg-toddr.c
index 32ee45cce7f8..5b08b4e841ad 100644
--- a/sound/soc/meson/axg-toddr.c
+++ b/sound/soc/meson/axg-toddr.c
@@ -19,12 +19,9 @@ 
 #define CTRL0_TODDR_EXT_SIGNED		BIT(29)
 #define CTRL0_TODDR_PP_MODE		BIT(28)
 #define CTRL0_TODDR_SYNC_CH		BIT(27)
-#define CTRL0_TODDR_TYPE_MASK		GENMASK(15, 13)
-#define CTRL0_TODDR_TYPE(x)		((x) << 13)
-#define CTRL0_TODDR_MSB_POS_MASK	GENMASK(12, 8)
-#define CTRL0_TODDR_MSB_POS(x)		((x) << 8)
-#define CTRL0_TODDR_LSB_POS_MASK	GENMASK(7, 3)
-#define CTRL0_TODDR_LSB_POS(x)		((x) << 3)
+#define CTRL0_TODDR_TYPE		GENMASK(15, 13)
+#define CTRL0_TODDR_MSB_POS		GENMASK(12, 8)
+#define CTRL0_TODDR_LSB_POS		GENMASK(7, 3)
 #define CTRL1_TODDR_FORCE_FINISH	BIT(25)
 #define CTRL1_SEL_SHIFT			28
 
@@ -76,12 +73,12 @@  static int axg_toddr_dai_hw_params(struct snd_pcm_substream *substream,
 	width = params_width(params);
 
 	regmap_update_bits(fifo->map, FIFO_CTRL0,
-			   CTRL0_TODDR_TYPE_MASK |
-			   CTRL0_TODDR_MSB_POS_MASK |
-			   CTRL0_TODDR_LSB_POS_MASK,
-			   CTRL0_TODDR_TYPE(type) |
-			   CTRL0_TODDR_MSB_POS(TODDR_MSB_POS) |
-			   CTRL0_TODDR_LSB_POS(TODDR_MSB_POS - (width - 1)));
+			   CTRL0_TODDR_TYPE |
+			   CTRL0_TODDR_MSB_POS |
+			   CTRL0_TODDR_LSB_POS,
+			   FIELD_PREP(CTRL0_TODDR_TYPE, type) |
+			   FIELD_PREP(CTRL0_TODDR_MSB_POS, TODDR_MSB_POS) |
+			   FIELD_PREP(CTRL0_TODDR_LSB_POS, TODDR_MSB_POS - (width - 1)));
 
 	return 0;
 }