Thread (18 messages) 18 messages, 5 authors, 2010-10-11
STALE5716d

[PATCH v2] spi: add spi_tegra driver

From: Erik Gilling <hidden>
Date: 2010-08-27 19:26:03
Also in: linux-spi

On Wed, Aug 25, 2010 at 2:22 PM, Erik Gilling [off-list ref] wrote:
quoted
As an aside, and commenting on the tegra DMA api... Is the IS_ERR()
pattern in the tegra_dma_allocate_channel() return really a good idea
here? ?Personally I find that pattern isn't very useful, especially
when the results is either "yes, allocated" or "no, not-alocated", and
it is very easy to write code that tests the wrong thing on the return
value because the compiler cannot catch it. ?In other words, if it
isn't vital to return a specific error code, the using the PTR_ERR()
pattern adds unnecessary risk to the code because developers are used
to seeing and writing this pattern:

tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
if (!tspi->rx_dma)
? ? ? ?return -ENOMEM;

The compiler won't catch it if the return value is an PTR_ERR.

(I know that some subsystems make heavy use of it, particularly
filesystems and the block layer which do need to return specific error
codes to userspace. ?My argument is that the specific error code is
probably irrelevant when allocating DMA channels)
I don't have a strong opinion either way but that's the way Colin's
code is written. ?I'll let him speak to that.
I spoke with colin about this.  He's fine getting rid of the PTR_ERR.
I'll post a DMA and SPI patch when that's done.

-Erik
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help