On Fri, Aug 27, 2010 at 1:26 PM, Erik Gilling <konkers@xxxxxxxxxxx> wrote: > 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. Okay, thanks. g. -- 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