RE: [PATCH 2/4] ASoC: sh: rz-ssi: Update interrupt handling for half duplex channels

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Prabhakar <prabhakar.csengg@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: RE: [PATCH 2/4] ASoC: sh: rz-ssi: Update interrupt handling for half duplex channels
Date: Sat, 18 Feb 2023 08:15:14 +0000	[thread overview]
Message-ID: <OS0PR01MB592203E429BEB581034811BB86A69@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20230217185225.43310-3-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

Thanks for the patch.

> Subject: [PATCH 2/4] ASoC: sh: rz-ssi: Update interrupt handling for half
> duplex channels
> 
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> For half duplex channels we dont have separate interrupts for Tx and Rx
> instead we have single interrupt Rt (where the signal for Rx and Tx is
> muxed). To handle such a case install a handler in case we have a dma_rt
> interrupt specified in the DT for the PIO mode.
> 
> Note, for backward compatibility we check if the Rx and Tx interrupts are
> present first instead of checking Rt interrupt.

Just a thought, 

As dt-binding doc mentions, a way to distinguish half duplex and full duplex by
Counting the number of interrupts. Maybe we could use that property for
detecting channel with full/half duplex mode.

See below

> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  sound/soc/sh/rz-ssi.c | 63 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c index
> 5d6bae33ae34..d502aa55c5a8 100644
> --- a/sound/soc/sh/rz-ssi.c
> +++ b/sound/soc/sh/rz-ssi.c
> @@ -109,6 +109,7 @@ struct rz_ssi_priv {
>  	int irq_int;
>  	int irq_tx;
>  	int irq_rx;
> +	int irq_rt;
> 
>  	spinlock_t lock;
> 
> @@ -565,6 +566,17 @@ static irqreturn_t rz_ssi_interrupt(int irq, void
> *data)
>  		rz_ssi_reg_mask_setl(ssi, SSIFSR, SSIFSR_RDF, 0);
>  	}
> 
> +	if (irq == ssi->irq_rt) {
> +		struct snd_pcm_substream *substream = strm->substream;
> +
> +		if (rz_ssi_stream_is_play(ssi, substream)) {
> +			strm->transfer(ssi, &ssi->playback);
> +		} else {
> +			strm->transfer(ssi, &ssi->capture);
> +			rz_ssi_reg_mask_setl(ssi, SSIFSR, SSIFSR_RDF, 0);
> +		}
> +	}
> +
>  	return IRQ_HANDLED;
>  }
> 
> @@ -993,26 +1005,39 @@ static int rz_ssi_probe(struct platform_device *pdev)
>  	if (!rz_ssi_is_dma_enabled(ssi)) {

Here, Detect Half duplex or full duplex by counting number of interrupts.

If half duplex get IRQ associated with dma_rt

If full duplex get IRQ associated with dma_rx and dma_tx.

>  		/* Tx and Rx interrupts (pio only) */
>  		ssi->irq_tx = platform_get_irq_byname(pdev, "dma_tx");
> -		if (ssi->irq_tx < 0)
> -			return ssi->irq_tx;
> -
> -		ret = devm_request_irq(&pdev->dev, ssi->irq_tx,
> -				       &rz_ssi_interrupt, 0,
> -				       dev_name(&pdev->dev), ssi);
> -		if (ret < 0)
> -			return dev_err_probe(&pdev->dev, ret,
> -					     "irq request error (dma_tx)\n");
> -
>  		ssi->irq_rx = platform_get_irq_byname(pdev, "dma_rx");
> -		if (ssi->irq_rx < 0)
> -			return ssi->irq_rx;
> -
> -		ret = devm_request_irq(&pdev->dev, ssi->irq_rx,
> -				       &rz_ssi_interrupt, 0,
> -				       dev_name(&pdev->dev), ssi);
> -		if (ret < 0)
> -			return dev_err_probe(&pdev->dev, ret,
> -					     "irq request error (dma_rx)\n");
> +		if (ssi->irq_tx == -ENXIO && ssi->irq_rx == -ENXIO) {
> +			ssi->irq_rt = platform_get_irq_byname(pdev, "dma_rt");
> +			if (ssi->irq_rt < 0)
> +				return ssi->irq_rt;
> +
> +			ret = devm_request_irq(&pdev->dev, ssi->irq_rt,
> +					       &rz_ssi_interrupt, 0,
> +					       dev_name(&pdev->dev), ssi);
> +			if (ret < 0)
> +				return dev_err_probe(&pdev->dev, ret,
> +						"irq request error (dma_tx)\n");

Typo dma_rt??

Cheers,
Biju

> +		} else {
> +			if (ssi->irq_tx < 0)
> +				return ssi->irq_tx;
> +
> +			if (ssi->irq_rx < 0)
> +				return ssi->irq_rx;
> +
> +			ret = devm_request_irq(&pdev->dev, ssi->irq_tx,
> +					       &rz_ssi_interrupt, 0,
> +					       dev_name(&pdev->dev), ssi);
> +			if (ret < 0)
> +				return dev_err_probe(&pdev->dev, ret,
> +						"irq request error (dma_tx)\n");
> +
> +			ret = devm_request_irq(&pdev->dev, ssi->irq_rx,
> +					       &rz_ssi_interrupt, 0,
> +					       dev_name(&pdev->dev), ssi);
> +			if (ret < 0)
> +				return dev_err_probe(&pdev->dev, ret,
> +						"irq request error (dma_rx)\n");
> +		}
>  	}
> 
>  	ssi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> --
> 2.25.1