[libcamera-devel] [PATCH 2/3] libcamera: raspberrypi: dma_heaps: Be verbose on errors

[Kieran Bingham]

Hi Jacopo,

On 27/08/2020 09:20, Jacopo Mondi wrote:
> Be a tad more verbose on failures in opening the dma_heaps allocator
> devices.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> index 739f05d3d4d8..500f1eac2eb8 100644
> --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> @@ -45,9 +45,14 @@ int DmaHeap::open()
>  {
>  	dmaHeapHandle_ = ::open(DMA_HEAP_CMA_NAME, O_RDWR, 0);
>  	if (dmaHeapHandle_ == -1) {
> +		LOG(RPI, Error) << "Could not open dmaHeap device "
> +				<< DMA_HEAP_CMA_NAME << ": "
> +				<< strerror(errno);

This will report an error, even if the ALT_NAME is successful...


>  		dmaHeapHandle_ = ::open(DMA_HEAP_CMA_ALT_NAME, O_RDWR, 0);
>  		if (dmaHeapHandle_ == -1) {
> -			LOG(RPI, Error) << "Could not open dmaHeap device";
> +			LOG(RPI, Error) << "Could not open dmaHeap device "
> +					<< DMA_HEAP_CMA_ALT_NAME << ": "
> +					<< strerror(errno);
>  			return dmaHeapHandle_;
>  		}
>  	}
> 

I wonder if this is a good opportunity to refactor this code:

(untested/uncompiled psuedo code to follow)


std::array<const char *, 2> heap_names {
	"/dev/dma_heap/linux,cma",
	"/dev/dma_heap/reserved",
}

int DmaHeap::open()
{
	for (const char *heap : heap_names) {
		if (!libcamera::File::exists(heap))
			continue;

		dmaHeapHandle_ = ::open(heap, O_RDWR, 0);

		if (dmaHeapHandle_ != -1) /* Success! */
			return 0;

		LOG(RPI, Warning)
			<< "Could not open dmaHeap "
			<< heap << ": "	<< strerror(errno);
	}

	/* No DMA Heap was available */
	LOG(RPI, Error) << "Could not open any dmaHeap device";

	return -ENODEV;
}

-- 
Regards
--
Kieran