diff mbox series

[09/13] ASoC: ti: davinci-i2s: Enable unexpected frame pulses detection

Message ID 20240315112745.63230-10-bastien.curutchet@bootlin.com (mailing list archive)
State Superseded
Headers show
Series ASoC: ti: davinci-i2s: Add features to McBSP driver | expand

Commit Message

Bastien Curutchet March 15, 2024, 11:27 a.m. UTC
McBSP can generate an SYNCERR when unexpected frame pulses are
detected. The driver always disables this feature and ignore the
unexpected frame pulses.

Enable the generation of SYNCERR by the McBSP according to the
'ti,enable-sync-err' device-tree property.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Mark Brown March 15, 2024, 2:09 p.m. UTC | #1
On Fri, Mar 15, 2024 at 12:27:41PM +0100, Bastien Curutchet wrote:

> McBSP can generate an SYNCERR when unexpected frame pulses are
> detected. The driver always disables this feature and ignore the
> unexpected frame pulses.

What does "unexpected" mean?

> Enable the generation of SYNCERR by the McBSP according to the
> 'ti,enable-sync-err' device-tree property.

Why would this be optional, and how is this reported - I'm not seeing
any interrupt handling updates?
Bastien Curutchet March 15, 2024, 2:45 p.m. UTC | #2
Hi Mark,

On 3/15/24 15:09, Mark Brown wrote:
> On Fri, Mar 15, 2024 at 12:27:41PM +0100, Bastien Curutchet wrote:
> 
>> McBSP can generate an SYNCERR when unexpected frame pulses are
>> detected. The driver always disables this feature and ignore the
>> unexpected frame pulses.
> 
> What does "unexpected" mean?

Unexpected frame sync pulse is defined in datasheet as a sync pulse that 
occurs <N> bit clocks earlier than the last transmitted bit of the 
previous frame. The <N> can be configured through registers.

> 
>> Enable the generation of SYNCERR by the McBSP according to the
>> 'ti,enable-sync-err' device-tree property.
> 
> Why would this be optional, and how is this reported - I'm not seeing
> any interrupt handling updates?

It is possible to deliberately ignore them and that is what is done 
today in the driver.
This is reported as a status bit in a register. An interrupt can indeed 
be generated from this but I'm not using it (now at least).
I use the fact that McBSP automatically drops previous element and 
starts a new reception when an unexpected frame pulse occurs.


Best regards,
Bastien
Mark Brown March 15, 2024, 2:54 p.m. UTC | #3
On Fri, Mar 15, 2024 at 03:45:24PM +0100, Bastien Curutchet wrote:
> On 3/15/24 15:09, Mark Brown wrote:
> > On Fri, Mar 15, 2024 at 12:27:41PM +0100, Bastien Curutchet wrote:

> > > McBSP can generate an SYNCERR when unexpected frame pulses are
> > > detected. The driver always disables this feature and ignore the
> > > unexpected frame pulses.

> > What does "unexpected" mean?

> Unexpected frame sync pulse is defined in datasheet as a sync pulse that
> occurs <N> bit clocks earlier than the last transmitted bit of the previous
> frame. The <N> can be configured through registers.

> > > Enable the generation of SYNCERR by the McBSP according to the
> > > 'ti,enable-sync-err' device-tree property.

> > Why would this be optional, and how is this reported - I'm not seeing
> > any interrupt handling updates?

> It is possible to deliberately ignore them and that is what is done today in
> the driver.
> This is reported as a status bit in a register. An interrupt can indeed be
> generated from this but I'm not using it (now at least).
> I use the fact that McBSP automatically drops previous element and starts a
> new reception when an unexpected frame pulse occurs.

That sounds like a very standard behaviour for incorrect clocking.  I
don't think this needs configuration at all, just enable this mode.
diff mbox series

Patch

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index 7cdd86f47ead..2c19e45b6b54 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -163,6 +163,8 @@  struct davinci_mcbsp_dev {
 
 	int tdm_slots;
 	int slot_width;
+
+	bool sync_err;
 };
 
 static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
@@ -432,8 +434,10 @@  static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct snd_interval *i = NULL;
 	int mcbsp_word_length, master;
-	unsigned int rcr, xcr, clk_div, freq, framesize;
+	unsigned int clk_div, freq, framesize;
 	unsigned int srgr = 0;
+	unsigned int rcr = 0;
+	unsigned int xcr = 0;
 	u32 spcr;
 	snd_pcm_format_t fmt;
 	unsigned element_cnt = 1;
@@ -530,8 +534,10 @@  static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 
-	rcr = DAVINCI_MCBSP_RCR_RFIG;
-	xcr = DAVINCI_MCBSP_XCR_XFIG;
+	if (!dev->sync_err) {
+		rcr |= DAVINCI_MCBSP_RCR_RFIG;
+		xcr |= DAVINCI_MCBSP_XCR_XFIG;
+	}
 	if (dev->mode == MOD_DSP_B) {
 		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(0);
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(0);
@@ -760,6 +766,8 @@  static int davinci_i2s_probe(struct platform_device *pdev)
 
 	dev->base = io_base;
 
+	dev->sync_err = of_property_read_bool(pdev->dev.of_node, "ti,enable-sync-err");
+
 	/* setup DMA, first TX, then RX */
 	dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
 	dma_data->addr = (dma_addr_t)(mem->start + DAVINCI_MCBSP_DXR_REG);