[PATCH v2] soc: qcom: geni-se: Do not bother about enable/disable of interrupts in secondary sequencer

[PATCH v2] soc: qcom: geni-se: Do not bother about enable/disable of interrupts in secondary sequencer

[Vijaya Krishna Nivarthi]

The select_fifo/dma_mode() functions in geni driver enable/disable
interrupts (secondary included) conditionally for non-uart modes, while
uart is supposed to manage this internally.
However, only uart uses secondary IRQs while spi, i2c do not care about
these at all making their enablement (or disablement) totally unnecessary
for these protos.
Similarly, select_gpi_mode() also does disable s_irq and its useless again.

Drop enabling/disabling secondary IRQs.
This doesn't solve any observed problem but only gets rid of code pieces
that are not required.

Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
---
v1 -> v2:
- apply similar changes to select_gpi_mode()
- modified commit message accordingly
---
 drivers/soc/qcom/qcom-geni-se.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index dd50a25..ba78876 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -281,27 +281,14 @@ static void geni_se_select_fifo_mode(struct geni_se *se)
 
 	geni_se_irq_clear(se);
 
-	/*
-	 * The RX path for the UART is asynchronous and so needs more
-	 * complex logic for enabling / disabling its interrupts.
-	 *
-	 * Specific notes:
-	 * - The done and TX-related interrupts are managed manually.
-	 * - We don't RX from the main sequencer (we use the secondary) so
-	 *   we don't need the RX-related interrupts enabled in the main
-	 *   sequencer for UART.
-	 */
+	/* UART driver manages enabling / disabling interrupts internally */
 	if (proto != GENI_SE_UART) {
+		/* Non-UART use only primary sequencer so dont bother about S_IRQ */
 		val_old = val = readl_relaxed(se->base + SE_GENI_M_IRQ_EN);
 		val |= M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN;
 		val |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
 		if (val != val_old)
 			writel_relaxed(val, se->base + SE_GENI_M_IRQ_EN);
-
-		val_old = val = readl_relaxed(se->base + SE_GENI_S_IRQ_EN);
-		val |= S_CMD_DONE_EN;
-		if (val != val_old)
-			writel_relaxed(val, se->base + SE_GENI_S_IRQ_EN);
 	}
 
 	val_old = val = readl_relaxed(se->base + SE_GENI_DMA_MODE_EN);
@@ -317,17 +304,14 @@ static void geni_se_select_dma_mode(struct geni_se *se)
 
 	geni_se_irq_clear(se);
 
+	/* UART driver manages enabling / disabling interrupts internally */
 	if (proto != GENI_SE_UART) {
+		/* Non-UART use only primary sequencer so dont bother about S_IRQ */
 		val_old = val = readl_relaxed(se->base + SE_GENI_M_IRQ_EN);
 		val &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN);
 		val &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
 		if (val != val_old)
 			writel_relaxed(val, se->base + SE_GENI_M_IRQ_EN);
-
-		val_old = val = readl_relaxed(se->base + SE_GENI_S_IRQ_EN);
-		val &= ~S_CMD_DONE_EN;
-		if (val != val_old)
-			writel_relaxed(val, se->base + SE_GENI_S_IRQ_EN);
 	}
 
 	val_old = val = readl_relaxed(se->base + SE_GENI_DMA_MODE_EN);
@@ -344,10 +328,6 @@ static void geni_se_select_gpi_mode(struct geni_se *se)
 
 	writel(0, se->base + SE_IRQ_EN);
 
-	val = readl(se->base + SE_GENI_S_IRQ_EN);
-	val &= ~S_CMD_DONE_EN;
-	writel(val, se->base + SE_GENI_S_IRQ_EN);
-
 	val = readl(se->base + SE_GENI_M_IRQ_EN);
 	val &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN |
 		 M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.

Re: [PATCH v2] soc: qcom: geni-se: Do not bother about enable/disable of interrupts in secondary sequencer

[Doug Anderson]

Hi,

On Wed, Jun 14, 2023 at 4:28 AM Vijaya Krishna Nivarthi
<quic_vnivarth@quicinc.com> wrote:
>
> The select_fifo/dma_mode() functions in geni driver enable/disable
> interrupts (secondary included) conditionally for non-uart modes, while
> uart is supposed to manage this internally.
> However, only uart uses secondary IRQs while spi, i2c do not care about
> these at all making their enablement (or disablement) totally unnecessary
> for these protos.
> Similarly, select_gpi_mode() also does disable s_irq and its useless again.
>
> Drop enabling/disabling secondary IRQs.
> This doesn't solve any observed problem but only gets rid of code pieces
> that are not required.
>
> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
> ---
> v1 -> v2:
> - apply similar changes to select_gpi_mode()
> - modified commit message accordingly
> ---
>  drivers/soc/qcom/qcom-geni-se.c | 28 ++++------------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v2] soc: qcom: geni-se: Do not bother about enable/disable of interrupts in secondary sequencer

[Bjorn Andersson]

On Wed, 14 Jun 2023 16:58:07 +0530, Vijaya Krishna Nivarthi wrote:
> The select_fifo/dma_mode() functions in geni driver enable/disable
> interrupts (secondary included) conditionally for non-uart modes, while
> uart is supposed to manage this internally.
> However, only uart uses secondary IRQs while spi, i2c do not care about
> these at all making their enablement (or disablement) totally unnecessary
> for these protos.
> Similarly, select_gpi_mode() also does disable s_irq and its useless again.
> 
> [...]

Applied, thanks!

[1/1] soc: qcom: geni-se: Do not bother about enable/disable of interrupts in secondary sequencer
      commit: 5d85ea2c879781d296bd770f562d33a58161d2b1

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>