Re: [PATCH v1 2/3] spi: geni-qcom: Fix abort sequence execution for serial engine errors

[Praveen Talari]

HI Konrad,

Thank you for review.

On 1/27/2026 6:47 PM, Konrad Dybcio wrote:
> On 1/22/26 4:10 PM, Praveen Talari wrote:
> > The driver currently skips the abort sequence for target mode when serial
> > engine errors occur. This leads to improper error recovery as the serial
> > engine may remain in an undefined state without proper cleanup, potentially
> > causing subsequent operations to fail or behave unpredictably.
> >
> > Fix this by ensuring the abort sequence and DMA reset always execute during
> > error recovery, as both are required for proper serial engine error
> > handling.
> >
> > Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/spi/spi-geni-qcom.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> > index f5d05025b196..e5320e2fb834 100644
> > --- a/drivers/spi/spi-geni-qcom.c
> > +++ b/drivers/spi/spi-geni-qcom.c
> > @@ -167,7 +167,7 @@ static void handle_se_timeout(struct spi_controller *spi,
> >   		 * doesn`t support CMD Cancel sequnece
> >   		 */
> >   		spin_unlock_irq(&mas->lock);
> > -		goto reset_if_dma;
> > +		goto abort;
> >   	}
> >   
> >   	reinit_completion(&mas->cancel_done);
> > @@ -178,6 +178,7 @@ static void handle_se_timeout(struct spi_controller *spi,
> >   	if (time_left)
> >   		goto reset_if_dma;
> >   
> > +abort:
> >   	spin_lock_irq(&mas->lock);
>
> Now that the jump is just 5 LoC, you can dispose of the goto and change it
> to an if-statement

Is the modification below good?

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index f5d05025b196..4feaf24d47ea 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -167,16 +167,15 @@ static void handle_se_timeout(struct 
spi_controller *spi,
                 * doesn`t support CMD Cancel sequnece
                 */
                spin_unlock_irq(&mas->lock);
-               goto reset_if_dma;
-       }
-
-       reinit_completion(&mas->cancel_done);
-       geni_se_cancel_m_cmd(se);
-       spin_unlock_irq(&mas->lock);
+       } else {
+               reinit_completion(&mas->cancel_done);
+               geni_se_cancel_m_cmd(se);
+               spin_unlock_irq(&mas->lock);

-       time_left = wait_for_completion_timeout(&mas->cancel_done, HZ);
-       if (time_left)
-               goto reset_if_dma;
+               time_left = 
wait_for_completion_timeout(&mas->cancel_done, HZ);
+               if (time_left)
+                       goto reset_if_dma;
+       }

        spin_lock_irq(&mas->lock);
        reinit_completion(&mas->abort_done);

Thanks,
Praveen Talari
> Konrad