diff mbox series

[2/6] ASoC: sun4i-codec: Add playback only flag to quirks

Message ID 20240929100750.860329-3-ryan@testtoast.com (mailing list archive)
State Not Applicable, archived
Headers show
Series ASoC: add Allwinner H616 audio codec support | expand

Commit Message

Ryan Walklin Sept. 29, 2024, 10:06 a.m. UTC
From: Marcus Cooper <codekipper@gmail.com>

Some devices only have the playback side of the codec implemented
so add a quirk to check for this.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Ryan Walklin <ryan@testtoast.com>
---
 sound/soc/sunxi/sun4i-codec.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Andre Przywara Oct. 8, 2024, 12:32 p.m. UTC | #1
On Sun, 29 Sep 2024 23:06:03 +1300
Ryan Walklin <ryan@testtoast.com> wrote:

> From: Marcus Cooper <codekipper@gmail.com>
> 
> Some devices only have the playback side of the codec implemented
> so add a quirk to check for this.

That's odd, is this really the only place where we need to 
consider the lack of sampling functionality? I mean it just prevents the
fields to be populated in our internal struct, how does the rest of the
kernel know that there is no capture? Is that magically achieved by those
fields being zero now?

Cheers,
Andre

> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> ---
>  sound/soc/sunxi/sun4i-codec.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 37f5678b55291..312d2655c3f4e 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -1571,6 +1571,7 @@ struct sun4i_codec_quirks {
>  	unsigned int reg_dac_txdata;	/* TX FIFO offset for DMA config */
>  	unsigned int reg_adc_rxdata;	/* RX FIFO offset for DMA config */
>  	bool has_reset;
> +	bool playback_only;
>  };
>  
>  static const struct sun4i_codec_quirks sun4i_codec_quirks = {
> @@ -1779,10 +1780,13 @@ static int sun4i_codec_probe(struct platform_device *pdev)
>  	scodec->playback_dma_data.maxburst = 8;
>  	scodec->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>  
> -	/* DMA configuration for RX FIFO */
> -	scodec->capture_dma_data.addr = res->start + quirks->reg_adc_rxdata;
> -	scodec->capture_dma_data.maxburst = 8;
> -	scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	if (!quirks->playback_only) {
> +		/* DMA configuration for RX FIFO */
> +		scodec->capture_dma_data.addr = res->start +
> +						quirks->reg_adc_rxdata;
> +		scodec->capture_dma_data.maxburst = 8;
> +		scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	}
>  
>  	ret = devm_snd_soc_register_component(&pdev->dev, quirks->codec,
>  				     &sun4i_codec_dai, 1);
Ryan Walklin Oct. 20, 2024, 6:04 a.m. UTC | #2
On Wed, 9 Oct 2024, at 1:32 AM, Andre Przywara wrote:

Hi Andre, thanks for reviewing! 

> On Sun, 29 Sep 2024 23:06:03 +1300
> Ryan Walklin <ryan@testtoast.com> wrote:
>
>> From: Marcus Cooper <codekipper@gmail.com>
>> 
>> Some devices only have the playback side of the codec implemented
>> so add a quirk to check for this.
>
> That's odd, is this really the only place where we need to 
> consider the lack of sampling functionality? I mean it just prevents the
> fields to be populated in our internal struct, how does the rest of the
> kernel know that there is no capture? Is that magically achieved by those
> fields being zero now?

Yes this is only used internally by the codec driver. The playback only nature of an individual codec is communicated to the DAI when the machine driver is created, for example in sun50i_h616_codec_create_card():

	card->dai_link->playback_only = true;
	card->dai_link->capture_only = false;

Regards,

Ryan
Andre Przywara Oct. 20, 2024, 10:19 a.m. UTC | #3
On Sun, 20 Oct 2024 19:04:24 +1300
"Ryan Walklin" <ryan@testtoast.com> wrote:

Hi Ryan,

> On Wed, 9 Oct 2024, at 1:32 AM, Andre Przywara wrote:
> 
> Hi Andre, thanks for reviewing! 
> 
> > On Sun, 29 Sep 2024 23:06:03 +1300
> > Ryan Walklin <ryan@testtoast.com> wrote:
> >  
> >> From: Marcus Cooper <codekipper@gmail.com>
> >> 
> >> Some devices only have the playback side of the codec implemented
> >> so add a quirk to check for this.  
> >
> > That's odd, is this really the only place where we need to 
> > consider the lack of sampling functionality? I mean it just prevents the
> > fields to be populated in our internal struct, how does the rest of the
> > kernel know that there is no capture? Is that magically achieved by those
> > fields being zero now?  
> 
> Yes this is only used internally by the codec driver. The playback only nature of an individual codec is communicated to the DAI when the machine driver is created, for example in sun50i_h616_codec_create_card():
> 
> 	card->dai_link->playback_only = true;
> 	card->dai_link->capture_only = false;

Ah, that makes sense indeed, I now see those lines.

Many thanks for the explanation!

Cheers,
Andre
diff mbox series

Patch

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 37f5678b55291..312d2655c3f4e 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -1571,6 +1571,7 @@  struct sun4i_codec_quirks {
 	unsigned int reg_dac_txdata;	/* TX FIFO offset for DMA config */
 	unsigned int reg_adc_rxdata;	/* RX FIFO offset for DMA config */
 	bool has_reset;
+	bool playback_only;
 };
 
 static const struct sun4i_codec_quirks sun4i_codec_quirks = {
@@ -1779,10 +1780,13 @@  static int sun4i_codec_probe(struct platform_device *pdev)
 	scodec->playback_dma_data.maxburst = 8;
 	scodec->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 
-	/* DMA configuration for RX FIFO */
-	scodec->capture_dma_data.addr = res->start + quirks->reg_adc_rxdata;
-	scodec->capture_dma_data.maxburst = 8;
-	scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	if (!quirks->playback_only) {
+		/* DMA configuration for RX FIFO */
+		scodec->capture_dma_data.addr = res->start +
+						quirks->reg_adc_rxdata;
+		scodec->capture_dma_data.maxburst = 8;
+		scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	}
 
 	ret = devm_snd_soc_register_component(&pdev->dev, quirks->codec,
 				     &sun4i_codec_dai, 1);