On Wed, Aug 25, 2010 at 2:22 PM, Erik Gilling <konkers@xxxxxxxxxxx> wrote: >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html