On 9/3/20 10:45, Lukasz Stelmach wrote:

It was <2020-09-02 śro 10:14>, when Sylwester Nawrocki wrote:

quoted

On 9/1/20 17:21, Lukasz Stelmach wrote:

quoted

It was <2020-08-25 wto 21:06>, when Sylwester Nawrocki wrote:

quoted

On 8/21/20 18:13, Łukasz Stelmach wrote:

quoted

Check return values in prepare_dma() and s3c64xx_spi_config() and
propagate errors upwards.

Signed-off-by: Łukasz Stelmach<l.stelmach@samsung.com>
---
    drivers/spi/spi-s3c64xx.c | 47 ++++++++++++++++++++++++++++++++-------
    1 file changed, 39 insertions(+), 8 deletions(-)

quoted

@@ -298,12 +299,24 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
      	desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
    				       dma->direction, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		dev_err(&sdd->pdev->dev, "unable to prepare %s scatterlist",
+			dma->direction == DMA_DEV_TO_MEM ? "rx" : "tx");
+		return -ENOMEM;
+	}
      	desc->callback = s3c64xx_spi_dmacb;
    	desc->callback_param = dma;
      	dma->cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(dma->cookie);
+	if (ret) {
+		dev_err(&sdd->pdev->dev, "DMA submission failed");
+		return -EIO;

Just return the error value from dma_submit_error() here?

--8<---------------cut here---------------start------------->8---
static inline int dma_submit_error(dma_cookie_t cookie)
{
          return cookie < 0 ? cookie : 0;

}
--8<---------------cut here---------------end--------------->8---

Not quite meaningful IMHO, is it?

dma_submit_error() returns 0 or an error code, I think it makes sense
to propagate that error code rather than replacing it with -EIO.

It is not an error code that d_s_e() returns it is a value returned by
dma_cookie_assign() called from within the tx_submit() operation of a
DMA driver.

--8<---------------cut here---------------start------------->8---
static inline dma_cookie_t dma_cookie_assign(struct
dma_async_tx_descriptor *tx)
{
         struct dma_chan *chan = tx->chan;
         dma_cookie_t cookie;

         cookie = chan->cookie + 1;
         if (cookie < DMA_MIN_COOKIE)
                 cookie = DMA_MIN_COOKIE;
         tx->cookie = chan->cookie = cookie;

         return cookie;
}
--8<---------------cut here---------------end--------------->8---

Yes, a non-zero value returned by d_s_e() indicates an error but it
definitely isn't one of error codes from errno*.h.

I guess we can end that discussion at this point and keep your patch
as is, I just followed comment at the dma_submit_error() function:

"if dma_cookie_t is >0 it's a DMA request cookie, <0 it's an error code"
 
AFAICS dma_cookie_assign() always returns value > 0 and dma_submit_error()
only returns the cookie if its value is < 0 so in consequence d_s_e() will 
be always returning 0 in your case (PL330 DMAC)?

The below commit, likely a result of static code analysis, might be 
a confirmation. It could also explain why some drivers overwrite the return
value of d_s_e() and some just pass it up to the callers.

--------------------------------8<------------------------------------
commit 71ea148370f8b6c745a8a42f6fd983cf5ebade18
Author: Dan Carpenter [off-list ref]
Date:   Sat Aug 10 10:46:50 2013 +0300

    dmaengine: make dma_submit_error() return an error code
    
    The problem here is that the dma_xfer() functions in
    drivers/ata/pata_arasan_cf.c and drivers/mtd/nand/fsmc_nand.c expect
    dma_submit_error() to return an error code so they return 1 when they
    intended to return a negative.
    
    So far as I can tell, none of the ->tx_submit() functions ever do
    return error codes so this patch should have no effect in the current
    code.
    
    I also changed it from a define to an inline.
    
    Signed-off-by: Dan Carpenter [off-list ref]
    Signed-off-by: Dan Williams [off-list ref]

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cb286b1a..b3ba7e4 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h

@@ -38,7 +38,10 @@ typedef s32 dma_cookie_t;
 #define DMA_MIN_COOKIE 1
 #define DMA_MAX_COOKIE INT_MAX
 
-#define dma_submit_error(cookie) ((cookie) < 0 ? 1 : 0)
+static inline int dma_submit_error(dma_cookie_t cookie)
+{
+       return cookie < 0 ? cookie : 0;
+}
--------------------------------8<------------------------------------

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel