Re: [PATCH] drivers/base: wrong return value of dmam_declare_coherent_memory

[Vyacheslav Yurkov <uvv.mail@gmail.com>]

On 28.04.2016 16:31, Andrey Gursky wrote:
> Vyacheslav,
> 
> thanks for your patch.
> 
> For now it introduces a new bug.
> 
> Vyacheslav Yurkov <uvv.mail <at> gmail.com> writes:
> 
>>
>> Hi guys,
>> I found an issue in managed version of dma_declare_coherent_memory,
>> i.e. in dmam_declare_coherent_memory. It looks like the return value
>> of dma_declare_coherent_memory is zero in case of error and a
>> requested flag on success,
> 
> You should take this into account and not ignore the value of rc at all
> by using flags instead, which is not being altered by
> dma_declare_coherent_memory() making the latter appear to succeed
> always.
> 
> 

Hi Andrey,
Thanks for spotting this. My intention was to compare rc of course,
not the flags. While thinking it over I came up with improved version,
which is inlined below.
--- 
Yours sincerely,
Vyacheslav V. Yurkov

P.S.: I'm not subscribed to the maillist, so please include me in CC
when responding to this Email.

---
--- linux-4.5.2/drivers/base/dma-mapping.c.orig	2016-04-20 08:46:35.000000000 +0200
+++ linux-4.5.2/drivers/base/dma-mapping.c	2016-04-28 16:55:13.211945276 +0200
@@ -198,10 +198,14 @@ int dmam_declare_coherent_memory(struct
 
 	rc = dma_declare_coherent_memory(dev, phys_addr, device_addr, size,
 					 flags);
-	if (rc == 0)
+
+	if ((rc & (DMA_MEMORY_MAP | DMA_MEMORY_IO)) != 0) {
 		devres_add(dev, res);
-	else
+		rc = 0;
+	} else {
 		devres_free(res);
+		rc = -ENOMEM;
+	}
 
 	return rc;
 }
 
Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>