| Message ID | 20240315112745.63230-14-bastien.curutchet@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: ti: davinci-i2s: Add features to McBSP driver | expand |
On 15/03/2024 13:27, Bastien Curutchet wrote: > The McBSP's DX pin that outputs serial data during playback streams can > be used during capture streams to repeatedly output a chosen pattern. > For instance, this can be useful to drive an active-low signal during > captures (by choosing <0> as output pattern). Are there really any other use of this than to pull down or up the DX pin (0 or 0xffff)? If you just use the pin as GPIO then you don't need to change anything in the driver, The playback would not erach the pin, so no need to block it. > Enable this behaviour when the device-tree property 'ti,drive-dx' is > present. DX pin is driven with the provided pattern every time a > capture stream is launched. It is an interesting use of the hardware... You are controlling an external device (light an LED when capture is on)? > This property is not compatible with classic playback stream so > davinci_i2s_trigger() returns an error if a playback stream is started > while 'ti,drive-dx' flag is present. Propbaly add the .startup() callback and block the playback right there? > > This has been tested on a board designed of a DAVINCI/OMAP-L138 where > the DX pin is linked to the chip select pin of the converters of the > capture side. Isn't the DX will be pulled down as soon as the McBSP is enabled? Can you just re-configure the PUPD_SEL for the pin group to make the pin to be pulled the other way? > Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com> > --- > sound/soc/ti/davinci-i2s.c | 74 ++++++++++++++++++++++++++++++++------ > 1 file changed, 63 insertions(+), 11 deletions(-) > > diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c > index 13e349e7a6ec..e289a84bdd6a 100644 > --- a/sound/soc/ti/davinci-i2s.c > +++ b/sound/soc/ti/davinci-i2s.c > @@ -101,6 +101,9 @@ > #define DAVINCI_MCBSP_PCR_FSRM (1 << 10) > #define DAVINCI_MCBSP_PCR_FSXM (1 << 11) > > +#define PLAYBACK_CLOCK 1 > +#define CAPTURE_CLOCK 0 > + > enum { > DAVINCI_MCBSP_WORD_8 = 0, > DAVINCI_MCBSP_WORD_12, > @@ -164,6 +167,8 @@ struct davinci_mcbsp_dev { > > bool sync_err; > bool free_run; > + bool drive_dx; > + u32 dx_val; > }; > > static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev, > @@ -187,6 +192,19 @@ static void toggle_clock(struct davinci_mcbsp_dev *dev, int playback) > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, dev->pcr); > } > > +static int davinci_drive_dx(struct davinci_mcbsp_dev *dev) > +{ > + unsigned int spcr; > + > + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_DXR_REG, dev->dx_val); > + > + spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); > + spcr |= DAVINCI_MCBSP_SPCR_XRST; > + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); > + > + return 0; > +} > + > static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev, > struct snd_pcm_substream *substream) > { > @@ -194,6 +212,9 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev, > u32 spcr; > u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST; > > + if (!playback && dev->drive_dx) > + davinci_drive_dx(dev); > + > /* Enable transmitter or receiver */ > spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); > spcr |= mask; if (dev->drive_dx) { spcr |= DAVINCI_MCBSP_SPCR_XRST; davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_DXR_REG, dev->dx_val); } and no need for the davinci_drive_dx() function, plus it makes it symmetric of what you do on stop() > @@ -212,9 +233,17 @@ static void davinci_mcbsp_stop(struct davinci_mcbsp_dev *dev, int playback) > /* Reset transmitter/receiver and sample rate/frame sync generators */ > spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); > spcr &= ~(DAVINCI_MCBSP_SPCR_GRST | DAVINCI_MCBSP_SPCR_FRST); > - spcr &= playback ? ~DAVINCI_MCBSP_SPCR_XRST : ~DAVINCI_MCBSP_SPCR_RRST; > - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); > - toggle_clock(dev, playback); > + > + if (!playback) { > + spcr &= ~DAVINCI_MCBSP_SPCR_RRST; > + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); > + toggle_clock(dev, CAPTURE_CLOCK); > + } > + if (playback || dev->drive_dx) { > + spcr &= ~DAVINCI_MCBSP_SPCR_XRST; > + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); > + toggle_clock(dev, PLAYBACK_CLOCK); > + } > } > > static int davinci_i2s_tdm_word_length(int tdm_slot_width) > @@ -408,6 +437,10 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, > } > if (inv_fs == true) > pcr ^= (DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP); > + > + if (dev->drive_dx) > + pcr |= DAVINCI_MCBSP_PCR_CLKRP; > + > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr); > dev->pcr = pcr; > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr); > @@ -562,6 +595,9 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream, > xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1); > } > > + if (dev->drive_dx) > + xcr |= DAVINCI_MCBSP_XCR_XDATDLY(2); > + > if (params_channels(params) == 2) { > element_cnt = 2; > if (double_fmt[fmt] && dev->enable_channel_combine) { > @@ -611,9 +647,9 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream, > xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) | > DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length); > > - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK || dev->drive_dx) > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr); > - else > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr); > > pr_debug("%s - %d srgr=%X\n", __func__, __LINE__, srgr); > @@ -628,16 +664,21 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream, > struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(dai); > int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); > u32 spcr; > - u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST; > > davinci_mcbsp_stop(dev, playback); > > spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); > - if (spcr & mask) { > + if (spcr & DAVINCI_MCBSP_SPCR_XRST) { > /* start off disabled */ > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, > - spcr & ~mask); > - toggle_clock(dev, playback); > + spcr & ~DAVINCI_MCBSP_SPCR_XRST); > + toggle_clock(dev, PLAYBACK_CLOCK); > + } > + if (spcr & DAVINCI_MCBSP_SPCR_RRST) { > + /* start off disabled */ > + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, > + spcr & ~DAVINCI_MCBSP_SPCR_RRST); > + toggle_clock(dev, CAPTURE_CLOCK); > } > if (dev->pcr & (DAVINCI_MCBSP_PCR_FSXM | DAVINCI_MCBSP_PCR_FSRM | > DAVINCI_MCBSP_PCR_CLKXM | DAVINCI_MCBSP_PCR_CLKRM)) { > @@ -646,7 +687,7 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream, > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); > } > > - if (playback) { > + if (playback || dev->drive_dx) { > /* Enable the transmitter */ > spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); > spcr |= DAVINCI_MCBSP_SPCR_XRST; > @@ -659,7 +700,7 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream, > spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); > spcr &= ~DAVINCI_MCBSP_SPCR_XRST; > davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); > - toggle_clock(dev, playback); > + toggle_clock(dev, PLAYBACK_CLOCK); > } > > return 0; > @@ -672,6 +713,11 @@ static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd, > int ret = 0; > int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); > > + if (playback && dev->drive_dx) { > + dev_err(dev->dev, "Playback is not allowed when drive-cs flag is set\n"); > + return -EINVAL; > + } > + > switch (cmd) { > case SNDRV_PCM_TRIGGER_START: > case SNDRV_PCM_TRIGGER_RESUME: > @@ -779,6 +825,12 @@ static int davinci_i2s_probe(struct platform_device *pdev) > > dev->free_run = !of_property_read_bool(pdev->dev.of_node, "ti,disable-free-run"); > dev->sync_err = of_property_read_bool(pdev->dev.of_node, "ti,enable-sync-err"); > + dev->drive_dx = false; no need to initialize it to 0, dev is allocated with devm_kzalloc() > + ret = of_property_read_u32(pdev->dev.of_node, "ti,drive-dx", &dev->dx_val); > + if (ret && ret != -EINVAL) > + return ret; > + if (!ret) > + dev->drive_dx = true; if (!ret) dev->drive_dx = true; else if (ret != -EINVAL) return ret; > > /* setup DMA, first TX, then RX */ > dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
Hi Péter, On 3/19/24 19:29, Péter Ujfalusi wrote: > > > On 15/03/2024 13:27, Bastien Curutchet wrote: >> The McBSP's DX pin that outputs serial data during playback streams can >> be used during capture streams to repeatedly output a chosen pattern. >> For instance, this can be useful to drive an active-low signal during >> captures (by choosing <0> as output pattern). > > Are there really any other use of this than to pull down or up the DX > pin (0 or 0xffff) I don't know, indeed today I can only think about these two patterns. I tried to do something in a 'generic' way so it can evolve if needed. > If you just use the pin as GPIO then you don't need to change anything > in the driver, The playback would not erach the pin, so no need to block it. > >> Enable this behaviour when the device-tree property 'ti,drive-dx' is >> present. DX pin is driven with the provided pattern every time a >> capture stream is launched. > > It is an interesting use of the hardware... You are controlling an > external device (light an LED when capture is on)? Yes I control the chip select pin of the ADC that is sending data to DR pin, that's why I need the DX pin to be synchronized with capture streams. >> This property is not compatible with classic playback stream so >> davinci_i2s_trigger() returns an error if a playback stream is started >> while 'ti,drive-dx' flag is present. > > Propbaly add the .startup() callback and block the playback right there? > Ok, TBH my mastery of the sound subsystem is not high enough to have an opinion of where this should go so I'll trust you on this. >> >> This has been tested on a board designed of a DAVINCI/OMAP-L138 where >> the DX pin is linked to the chip select pin of the converters of the >> capture side. > > Isn't the DX will be pulled down as soon as the McBSP is enabled? > Can you just re-configure the PUPD_SEL for the pin group to make the pin > to be pulled the other way? > Well, the acquisition chain in my use case is a bit convoluted. The DX pin's main purpose is to drive ADC chip select but it is also connected to other components and all this needs synchronization upon captures. I'll integrate your feedback about the code in next iteration, thank you. Best regards, Bastien
Hi Bastien, On 20/03/2024 10:52, Bastien Curutchet wrote: > Hi Péter, > > On 3/19/24 19:29, Péter Ujfalusi wrote: >> >> >> On 15/03/2024 13:27, Bastien Curutchet wrote: >>> The McBSP's DX pin that outputs serial data during playback streams can >>> be used during capture streams to repeatedly output a chosen pattern. >>> For instance, this can be useful to drive an active-low signal during >>> captures (by choosing <0> as output pattern). >> >> Are there really any other use of this than to pull down or up the DX >> pin (0 or 0xffff) > > I don't know, indeed today I can only think about these two patterns. > I tried to do something in a 'generic' way so it can evolve if needed. I think the definition of the 'ti,drive-dx' is somehow odd. It allows you to set it to 0x1234 and the DX pin will show 0x1234 when you capture 32bit. If you capture 16bit then it will transmit 0x12 (or 0x34?), no? If you have 4 channel capture then I won't speculate what will be on the DX pin ;) Would not be better to say that the DX pin will be driven low or high during capture _and_ disable the playback support? > >> If you just use the pin as GPIO then you don't need to change anything >> in the driver, The playback would not erach the pin, so no need to >> block it. >> >>> Enable this behaviour when the device-tree property 'ti,drive-dx' is >>> present. DX pin is driven with the provided pattern every time a >>> capture stream is launched. >> >> It is an interesting use of the hardware... You are controlling an >> external device (light an LED when capture is on)? > > Yes I control the chip select pin of the ADC that is sending data to DR > pin, that's why I need the DX pin to be synchronized with capture > streams. I see. Still a a novel use of a feature ;) > >>> This property is not compatible with classic playback stream so >>> davinci_i2s_trigger() returns an error if a playback stream is started >>> while 'ti,drive-dx' flag is present. >> >> Propbaly add the .startup() callback and block the playback right there? >> > > Ok, TBH my mastery of the sound subsystem is not high enough to have an > opinion of where this should go so I'll trust you on this. It would be more elegant to only create PCM for the capture in this case, but I would not bother with it. Stopping user right at startup time is second better. >>> >>> This has been tested on a board designed of a DAVINCI/OMAP-L138 where >>> the DX pin is linked to the chip select pin of the converters of the >>> capture side. >> >> Isn't the DX will be pulled down as soon as the McBSP is enabled? >> Can you just re-configure the PUPD_SEL for the pin group to make the pin >> to be pulled the other way? >> > > Well, the acquisition chain in my use case is a bit convoluted. The DX > pin's main purpose is to drive ADC chip select but it is also connected > to other components and all this needs synchronization upon captures. OK, thanks for the explanation.
On 20/03/2024 17:42, Péter Ujfalusi wrote: >>> On 15/03/2024 13:27, Bastien Curutchet wrote: >>>> The McBSP's DX pin that outputs serial data during playback streams can >>>> be used during capture streams to repeatedly output a chosen pattern. >>>> For instance, this can be useful to drive an active-low signal during >>>> captures (by choosing <0> as output pattern). >>> >>> Are there really any other use of this than to pull down or up the DX >>> pin (0 or 0xffff) >> >> I don't know, indeed today I can only think about these two patterns. >> I tried to do something in a 'generic' way so it can evolve if needed. > > I think the definition of the 'ti,drive-dx' is somehow odd. It allows > you to set it to 0x1234 and the DX pin will show 0x1234 when you capture > 32bit. If you capture 16bit then it will transmit 0x12 (or 0x34?), no? > If you have 4 channel capture then I won't speculate what will be on the > DX pin ;) > > Would not be better to say that the DX pin will be driven low or high > during capture _and_ disable the playback support? After some thinking, it might be still better to use the DX pin as GPIO and either have a custom machine driver which would handle it (set low when a capture trigger happens) or connect it in DAPM as a supply, bias or something and ASoC would handle it automagically. I think that would be cleaner in many ways. What do you think?
Hi Péter, On 3/20/24 21:30, Péter Ujfalusi wrote: > > > On 20/03/2024 17:42, Péter Ujfalusi wrote: >>>> On 15/03/2024 13:27, Bastien Curutchet wrote: >>>>> The McBSP's DX pin that outputs serial data during playback streams can >>>>> be used during capture streams to repeatedly output a chosen pattern. >>>>> For instance, this can be useful to drive an active-low signal during >>>>> captures (by choosing <0> as output pattern). >>>> >>>> Are there really any other use of this than to pull down or up the DX >>>> pin (0 or 0xffff) >>> >>> I don't know, indeed today I can only think about these two patterns. >>> I tried to do something in a 'generic' way so it can evolve if needed. >> >> I think the definition of the 'ti,drive-dx' is somehow odd. It allows >> you to set it to 0x1234 and the DX pin will show 0x1234 when you capture >> 32bit. If you capture 16bit then it will transmit 0x12 (or 0x34?), no? >> If you have 4 channel capture then I won't speculate what will be on the >> DX pin ;) >> >> Would not be better to say that the DX pin will be driven low or high >> during capture _and_ disable the playback support? > > After some thinking, it might be still better to use the DX pin as GPIO > and either have a custom machine driver which would handle it (set low > when a capture trigger happens) or connect it in DAPM as a supply, bias > or something and ASoC would handle it automagically. > > I think that would be cleaner in many ways. What do you think? > I agree, that would be cleaner. I ran a few tests to see if that would work on my hardware. It doesn't ... So I looked back to the schematics and found two reasons : * the DX pin needs to be in sync with the clock. * the DX pin needs to be in a high-impedance state between two frames so a pull-up can drive it back up. Actually, the DX pin is also linked to the FSR pin so it provides the frame clock to the capture stream. Bast regards, Bastien
Hi Bastien, On 3/21/24 17:14, Bastien Curutchet wrote: >>> I think the definition of the 'ti,drive-dx' is somehow odd. It allows >>> you to set it to 0x1234 and the DX pin will show 0x1234 when you capture >>> 32bit. If you capture 16bit then it will transmit 0x12 (or 0x34?), no? >>> If you have 4 channel capture then I won't speculate what will be on the >>> DX pin ;) >>> >>> Would not be better to say that the DX pin will be driven low or high >>> during capture _and_ disable the playback support? >> >> After some thinking, it might be still better to use the DX pin as GPIO >> and either have a custom machine driver which would handle it (set low >> when a capture trigger happens) or connect it in DAPM as a supply, bias >> or something and ASoC would handle it automagically. >> >> I think that would be cleaner in many ways. What do you think? >> > I agree, that would be cleaner. I ran a few tests to see if that would > work on my hardware. It doesn't ... So I looked back to the schematics > and found two reasons : > * the DX pin needs to be in sync with the clock. I'm not sure what this means, sync with which clock? > * the DX pin needs to be in a high-impedance state between two frames > so a pull-up can drive it back up. Actually, the DX pin is also > linked to the FSR pin so it provides the frame clock to the capture > stream. Hrm, you are using the DX pin as FSR for the capture? Why not McBSP.FSR pin? Looking back to the patch, one thing stood out: you are setting the XDATDLY to 2. You have some sort of T1 framing on the bus? The pullup will make the DX line high in for the framing bit, right? Or you simulate another FSR line with T1 framing DX? The 'ti,drive-dx' sounds like a bad property for sure, you have T1 framing and driving the DX to certain level. It is like DSP_A (1 bit delay) playing constant 0x2 ? Can you use aplay /dev/zero and a DT property to select T1 framing for the playback? Or that would be too coarse for timing the start of playback and capture?
Hi Péter, On 3/21/24 19:31, Péter Ujfalusi wrote: > Hi Bastien, > > On 3/21/24 17:14, Bastien Curutchet wrote: >>>> I think the definition of the 'ti,drive-dx' is somehow odd. It allows >>>> you to set it to 0x1234 and the DX pin will show 0x1234 when you capture >>>> 32bit. If you capture 16bit then it will transmit 0x12 (or 0x34?), no? >>>> If you have 4 channel capture then I won't speculate what will be on the >>>> DX pin ;) >>>> >>>> Would not be better to say that the DX pin will be driven low or high >>>> during capture _and_ disable the playback support? >>> >>> After some thinking, it might be still better to use the DX pin as GPIO >>> and either have a custom machine driver which would handle it (set low >>> when a capture trigger happens) or connect it in DAPM as a supply, bias >>> or something and ASoC would handle it automagically. >>> >>> I think that would be cleaner in many ways. What do you think? >>> >> I agree, that would be cleaner. I ran a few tests to see if that would >> work on my hardware. It doesn't ... So I looked back to the schematics >> and found two reasons : >> * the DX pin needs to be in sync with the clock. > > I'm not sure what this means, sync with which clock? > Sorry, that was not very clear, I meant sync with the bit block that is output on McBSP.CLKR pin. >> * the DX pin needs to be in a high-impedance state between two frames >> so a pull-up can drive it back up. Actually, the DX pin is also >> linked to the FSR pin so it provides the frame clock to the capture >> stream. > > Hrm, you are using the DX pin as FSR for the capture? Why not McBSP.FSR pin > The McBSP.FSR pin is used for the capture but is driven by the McBSP.DX pin. Both pins are linked together. > Looking back to the patch, one thing stood out: you are setting the > XDATDLY to 2. > You have some sort of T1 framing on the bus? The pullup will make the DX > line high in for the framing bit, right? > Or you simulate another FSR line with T1 framing DX? > Yes the goal is to simulate an FSR. > The 'ti,drive-dx' sounds like a bad property for sure, you have T1 > framing and driving the DX to certain level. > It is like DSP_A (1 bit delay) playing constant 0x2 ? > > Can you use aplay /dev/zero and a DT property to select T1 framing for > the playback? Or that would be too coarse for timing the start of > playback and capture? > That's a good idea, thank you. I'll try this and come back to you. Best regards, Bastien
Hi Péter, >> Can you use aplay /dev/zero and a DT property to select T1 framing for >> the playback? Or that would be too coarse for timing the start of >> playback and capture? >> > > That's a good idea, thank you. I'll try this and come back to you. > > I tried it and it works fine, thank you. We still need to run some performance tests before fully adopting it but anyway I'll send a new iteration of the patch series that drops the drive-dx part and just keeps a DT property to select T1 framing. Best regards, Bastien
diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c index 13e349e7a6ec..e289a84bdd6a 100644 --- a/sound/soc/ti/davinci-i2s.c +++ b/sound/soc/ti/davinci-i2s.c @@ -101,6 +101,9 @@ #define DAVINCI_MCBSP_PCR_FSRM (1 << 10) #define DAVINCI_MCBSP_PCR_FSXM (1 << 11) +#define PLAYBACK_CLOCK 1 +#define CAPTURE_CLOCK 0 + enum { DAVINCI_MCBSP_WORD_8 = 0, DAVINCI_MCBSP_WORD_12, @@ -164,6 +167,8 @@ struct davinci_mcbsp_dev { bool sync_err; bool free_run; + bool drive_dx; + u32 dx_val; }; static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev, @@ -187,6 +192,19 @@ static void toggle_clock(struct davinci_mcbsp_dev *dev, int playback) davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, dev->pcr); } +static int davinci_drive_dx(struct davinci_mcbsp_dev *dev) +{ + unsigned int spcr; + + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_DXR_REG, dev->dx_val); + + spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); + spcr |= DAVINCI_MCBSP_SPCR_XRST; + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); + + return 0; +} + static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev, struct snd_pcm_substream *substream) { @@ -194,6 +212,9 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev, u32 spcr; u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST; + if (!playback && dev->drive_dx) + davinci_drive_dx(dev); + /* Enable transmitter or receiver */ spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); spcr |= mask; @@ -212,9 +233,17 @@ static void davinci_mcbsp_stop(struct davinci_mcbsp_dev *dev, int playback) /* Reset transmitter/receiver and sample rate/frame sync generators */ spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); spcr &= ~(DAVINCI_MCBSP_SPCR_GRST | DAVINCI_MCBSP_SPCR_FRST); - spcr &= playback ? ~DAVINCI_MCBSP_SPCR_XRST : ~DAVINCI_MCBSP_SPCR_RRST; - davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); - toggle_clock(dev, playback); + + if (!playback) { + spcr &= ~DAVINCI_MCBSP_SPCR_RRST; + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); + toggle_clock(dev, CAPTURE_CLOCK); + } + if (playback || dev->drive_dx) { + spcr &= ~DAVINCI_MCBSP_SPCR_XRST; + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); + toggle_clock(dev, PLAYBACK_CLOCK); + } } static int davinci_i2s_tdm_word_length(int tdm_slot_width) @@ -408,6 +437,10 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai, } if (inv_fs == true) pcr ^= (DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP); + + if (dev->drive_dx) + pcr |= DAVINCI_MCBSP_PCR_CLKRP; + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr); dev->pcr = pcr; davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr); @@ -562,6 +595,9 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream, xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1); } + if (dev->drive_dx) + xcr |= DAVINCI_MCBSP_XCR_XDATDLY(2); + if (params_channels(params) == 2) { element_cnt = 2; if (double_fmt[fmt] && dev->enable_channel_combine) { @@ -611,9 +647,9 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream, xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) | DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK || dev->drive_dx) davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr); - else + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr); pr_debug("%s - %d srgr=%X\n", __func__, __LINE__, srgr); @@ -628,16 +664,21 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream, struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(dai); int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); u32 spcr; - u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST; davinci_mcbsp_stop(dev, playback); spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); - if (spcr & mask) { + if (spcr & DAVINCI_MCBSP_SPCR_XRST) { /* start off disabled */ davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, - spcr & ~mask); - toggle_clock(dev, playback); + spcr & ~DAVINCI_MCBSP_SPCR_XRST); + toggle_clock(dev, PLAYBACK_CLOCK); + } + if (spcr & DAVINCI_MCBSP_SPCR_RRST) { + /* start off disabled */ + davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, + spcr & ~DAVINCI_MCBSP_SPCR_RRST); + toggle_clock(dev, CAPTURE_CLOCK); } if (dev->pcr & (DAVINCI_MCBSP_PCR_FSXM | DAVINCI_MCBSP_PCR_FSRM | DAVINCI_MCBSP_PCR_CLKXM | DAVINCI_MCBSP_PCR_CLKRM)) { @@ -646,7 +687,7 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream, davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); } - if (playback) { + if (playback || dev->drive_dx) { /* Enable the transmitter */ spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); spcr |= DAVINCI_MCBSP_SPCR_XRST; @@ -659,7 +700,7 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream, spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG); spcr &= ~DAVINCI_MCBSP_SPCR_XRST; davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr); - toggle_clock(dev, playback); + toggle_clock(dev, PLAYBACK_CLOCK); } return 0; @@ -672,6 +713,11 @@ static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd, int ret = 0; int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); + if (playback && dev->drive_dx) { + dev_err(dev->dev, "Playback is not allowed when drive-cs flag is set\n"); + return -EINVAL; + } + switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: @@ -779,6 +825,12 @@ static int davinci_i2s_probe(struct platform_device *pdev) dev->free_run = !of_property_read_bool(pdev->dev.of_node, "ti,disable-free-run"); dev->sync_err = of_property_read_bool(pdev->dev.of_node, "ti,enable-sync-err"); + dev->drive_dx = false; + ret = of_property_read_u32(pdev->dev.of_node, "ti,drive-dx", &dev->dx_val); + if (ret && ret != -EINVAL) + return ret; + if (!ret) + dev->drive_dx = true; /* setup DMA, first TX, then RX */ dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
The McBSP's DX pin that outputs serial data during playback streams can be used during capture streams to repeatedly output a chosen pattern. For instance, this can be useful to drive an active-low signal during captures (by choosing <0> as output pattern). Enable this behaviour when the device-tree property 'ti,drive-dx' is present. DX pin is driven with the provided pattern every time a capture stream is launched. This property is not compatible with classic playback stream so davinci_i2s_trigger() returns an error if a playback stream is started while 'ti,drive-dx' flag is present. This has been tested on a board designed of a DAVINCI/OMAP-L138 where the DX pin is linked to the chip select pin of the converters of the capture side. Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com> --- sound/soc/ti/davinci-i2s.c | 74 ++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 11 deletions(-)