[v2,6/6] ASoC: sun4i-i2s: Add support for loopback

Commit Message

Code Kipper March 12, 2018, 3:57 p.m. UTC

From: Marcus Cooper <[e-mail]>

The DAI has a loopback register which can be set and therefore routes
the transmit fifo to receive fifo. This is useful for testing the block
without the need for any external hardware.

Signed-off-by: Marcus Cooper <[e-mail]>
---
 Documentation/devicetree/bindings/sound/sun4i-i2s.txt |  3 +++
 sound/soc/sunxi/sun4i-i2s.c                           | 11 +++++++++++
 2 files changed, 14 insertions(+)

Comments

Mark Brown March 12, 2018, 6:35 p.m. UTC | #1

On Mon, Mar 12, 2018 at 04:57:53PM +0100, [e-mail] wrote:

> +- loopback:	if this property is present then the dai is configured in
> +		loopback mode where the output fifo is redirected to the input
> +		fifo.

This really doesn't seem like something that ought to go into the DT
and hence ABI given that there's obviously no reason to use this in
production.  Just make it be a #define in the code or something.

Christopher Obbard March 13, 2018, 1:39 p.m. UTC | #2

Hi Mark,

>
>> +- loopback:  if this property is present then the dai is configured in
>> +             loopback mode where the output fifo is redirected to the input
>> +             fifo.
>
> This really doesn't seem like something that ought to go into the DT
> and hence ABI given that there's obviously no reason to use this in
> production.  Just make it be a #define in the code or something.

It would be nice to have this as an optional devicetree property so
when testing JACK latency etc I don't have to manually re-compile a
kernel...

Maxime Ripard March 13, 2018, 2:03 p.m. UTC | #3

On Tue, Mar 13, 2018 at 01:39:44PM +0000, Chris Obbard wrote:
> >> +- loopback:  if this property is present then the dai is configured in
> >> +             loopback mode where the output fifo is redirected to the input
> >> +             fifo.
> >
> > This really doesn't seem like something that ought to go into the DT
> > and hence ABI given that there's obviously no reason to use this in
> > production.  Just make it be a #define in the code or something.
> 
> It would be nice to have this as an optional devicetree property so
> when testing JACK latency etc I don't have to manually re-compile a
> kernel...

Another one might also argue that having to recompile the DT and
reflash the whole firmware or EEPROM is overkill for just testing JACK
latency.

Can't we create a module parameter for it?

Maxime

Mark Brown March 13, 2018, 4:26 p.m. UTC | #4

On Tue, Mar 13, 2018 at 01:39:44PM +0000, Chris Obbard wrote:
> >> +- loopback:  if this property is present then the dai is configured in
> >> +             loopback mode where the output fifo is redirected to the input
> >> +             fifo.

> > This really doesn't seem like something that ought to go into the DT
> > and hence ABI given that there's obviously no reason to use this in
> > production.  Just make it be a #define in the code or something.

> It would be nice to have this as an optional devicetree property so
> when testing JACK latency etc I don't have to manually re-compile a
> kernel...

You'll have to rebuild your DT anyway, I'm not sure the kernel is that
much of a difference...  For your use case you'd want a regular runtime
ALSA control, not a DT property anyway.  That would be fine.

Patch

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index 3f966ac61b9e..325e3d4571f1 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -32,6 +32,9 @@  Optional properties:
 - allwinner,slot-width-override:if this property is present then the dai is
 				configured to extend the slot width to the
 				value specified. Min 8, Max 32.
+- loopback:	if this property is present then the dai is configured in
+		loopback mode where the output fifo is redirected to the input
+		fifo.
 
 - allwinner,playback-channels:  if this property is present then the number
 				of available channels is extended and the
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 7cf4dfe440de..a3c33814d33e 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -31,6 +31,7 @@ 
 #define SUN4I_I2S_CTRL_MODE_MASK		BIT(5)
 #define SUN4I_I2S_CTRL_MODE_SLAVE			(1 << 5)
 #define SUN4I_I2S_CTRL_MODE_MASTER			(0 << 5)
+#define SUN4I_I2S_CTRL_LOOP			BIT(3)
 #define SUN4I_I2S_CTRL_TX_EN			BIT(2)
 #define SUN4I_I2S_CTRL_RX_EN			BIT(1)
 #define SUN4I_I2S_CTRL_GL_EN			BIT(0)
@@ -195,6 +196,8 @@  struct sun4i_i2s {
 
 	const struct sun4i_i2s_quirks	*variant;
 
+	bool loopback;
+
 	unsigned int	slot_width;
 };
 
@@ -639,6 +642,11 @@  static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
 	regmap_update_bits(i2s->regmap, SUN4I_I2S_DMA_INT_CTRL_REG,
 			   SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN,
 			   SUN4I_I2S_DMA_INT_CTRL_RX_DRQ_EN);
+
+	/* Debugging without codec */
+	if (i2s->loopback)
+		regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG,
+				   SUN4I_I2S_CTRL_LOOP, SUN4I_I2S_CTRL_LOOP);
 }
 
 static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
@@ -1204,6 +1212,9 @@  static int sun4i_i2s_probe(struct platform_device *pdev)
 			soc_dai->playback.channels_max = val;
 	}
 
+	if (of_property_read_bool(pdev->dev.of_node, "loopback"))
+		i2s->loopback = true;
+
 	pm_runtime_enable(&pdev->dev);
 	if (!pm_runtime_enabled(&pdev->dev)) {
 		ret = sun4i_i2s_runtime_resume(&pdev->dev);