diff mbox series

[3/3] media: atomisp: Improve binary finding debug logging

Message ID 20240902095229.59059-3-hdegoede@redhat.com (mailing list archive)
State New
Headers show
Series [1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 | expand

Commit Message

Hans de Goede Sept. 2, 2024, 9:52 a.m. UTC 
The atomisp firmware contains a number of different pipeline binaries
inside its firmware file and the driver selects the right one depending
on the selected pipeline configuration.

Sometimes (e.g. when the selected output resolution is too big) it fails
to find a binary. This happens especially when adding support for new
sensors.

Improve the logging when this happens to make debugging easier:

1. Replace ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, ...) with standard
   dev_dbg() calls so that the logs can be enabled with dyndbg

2. Do not dump_stack() when this fails, doing so adds no useful extra
   info

3. With the dump_stack() call gone, remove the wrapper and rename
   __ia_css_binary_find() to ia_css_binary_find()

4. On error use dev_err() instead of dev_dbg() so that when things
   fail it is clear why they fail without needing to enable dyndbg

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note the log messages could also do with a bit of rewording and
reflowing them to put more arguments on a single line to use less
lines. I consider that out of scope for this patch which already
enough (see the 1-4 enough) something like this should be done
in a follow-up patch IMHO.
---
 .../atomisp/pci/runtime/binary/src/binary.c   | 295 +++++++++---------
 1 file changed, 139 insertions(+), 156 deletions(-)

Comments

Andy Shevchenko Sept. 2, 2024, 12:03 p.m. UTC | #1 
On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
> The atomisp firmware contains a number of different pipeline binaries
> inside its firmware file and the driver selects the right one depending
> on the selected pipeline configuration.
> 
> Sometimes (e.g. when the selected output resolution is too big) it fails
> to find a binary. This happens especially when adding support for new
> sensors.
> 
> Improve the logging when this happens to make debugging easier:
> 
> 1. Replace ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, ...) with standard
>    dev_dbg() calls so that the logs can be enabled with dyndbg
> 
> 2. Do not dump_stack() when this fails, doing so adds no useful extra
>    info
> 
> 3. With the dump_stack() call gone, remove the wrapper and rename
>    __ia_css_binary_find() to ia_css_binary_find()
> 
> 4. On error use dev_err() instead of dev_dbg() so that when things
>    fail it is clear why they fail without needing to enable dyndbg
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note the log messages could also do with a bit of rewording and
> reflowing them to put more arguments on a single line to use less
> lines. I consider that out of scope for this patch which already
> enough (see the 1-4 enough) something like this should be done
> in a follow-up patch IMHO.

Yes, but...

...

> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
> -				struct ia_css_binary *binary) {
> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
> +		       struct ia_css_binary *binary)

...for example, in this case you touched both lines, so making them a single
line just reduces the future churn.

...

> -	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> -			    "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
> -			    descr, descr->mode,
> -			    binary);
> +	dev_dbg(atomisp_dev,
> +		"ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",

> +		descr, descr->mode,
> +		binary);

So does this.

...

>  	/* print a map of the binary file */
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"BINARY INFO:\n");
> +	dev_dbg(atomisp_dev,	"BINARY INFO:\n");

TAB instead of space.

...

> -			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> -					    "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
> -					    __LINE__, candidate->enable.continuous,
> -					    continuous, mode,
> -					    IA_CSS_BINARY_MODE_COPY);
> +			dev_dbg(atomisp_dev,
> +				"ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
> +				__LINE__, candidate->enable.continuous,

> +				continuous, mode,
> +				IA_CSS_BINARY_MODE_COPY);

Now one line?

...

> -			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> -					    "ia_css_binary_find() [%d] continue: binary is not striped\n",
> -					    __LINE__);
> +			dev_dbg(atomisp_dev,
> +				"ia_css_binary_find() [%d] continue: binary is not striped\n",
> +				__LINE__);

Now the __LINE__ argument is redundant as one may run-time turn it on and off.
So, trimming it while converting to dev_dbg() makes sense to me as a one
logical change. Ditto for other __LINE__ cases.

...

Otherwise it's a good clean up!
Hans de Goede Sept. 3, 2024, 9:27 a.m. UTC | #2 
Hi Andy,

Thank you for the reviews.

On 9/2/24 2:03 PM, Andy Shevchenko wrote:
> On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
>> The atomisp firmware contains a number of different pipeline binaries
>> inside its firmware file and the driver selects the right one depending
>> on the selected pipeline configuration.
>>
>> Sometimes (e.g. when the selected output resolution is too big) it fails
>> to find a binary. This happens especially when adding support for new
>> sensors.
>>
>> Improve the logging when this happens to make debugging easier:
>>
>> 1. Replace ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, ...) with standard
>>    dev_dbg() calls so that the logs can be enabled with dyndbg
>>
>> 2. Do not dump_stack() when this fails, doing so adds no useful extra
>>    info
>>
>> 3. With the dump_stack() call gone, remove the wrapper and rename
>>    __ia_css_binary_find() to ia_css_binary_find()
>>
>> 4. On error use dev_err() instead of dev_dbg() so that when things
>>    fail it is clear why they fail without needing to enable dyndbg
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Note the log messages could also do with a bit of rewording and
>> reflowing them to put more arguments on a single line to use less
>> lines. I consider that out of scope for this patch which already
>> enough (see the 1-4 enough) something like this should be done
>> in a follow-up patch IMHO.
> 
> Yes, but...
> 
> ...
> 
>> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
>> -				struct ia_css_binary *binary) {
>> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
>> +		       struct ia_css_binary *binary)
> 
> ...for example, in this case you touched both lines, so making them a single
> line just reduces the future churn.

Ack, fixed in my media-atomisp branch.

> 
> ...
> 
>> -	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>> -			    "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
>> -			    descr, descr->mode,
>> -			    binary);
>> +	dev_dbg(atomisp_dev,
>> +		"ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
> 
>> +		descr, descr->mode,
>> +		binary);
> 
> So does this.

Ack, fixed in my media-atomisp branch.

> 
> ...
> 
>>  	/* print a map of the binary file */
>> -	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"BINARY INFO:\n");
>> +	dev_dbg(atomisp_dev,	"BINARY INFO:\n");
> 
> TAB instead of space.

Ack, fixed in my media-atomisp branch.

> ...
> 
>> -			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>> -					    "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
>> -					    __LINE__, candidate->enable.continuous,
>> -					    continuous, mode,
>> -					    IA_CSS_BINARY_MODE_COPY);
>> +			dev_dbg(atomisp_dev,
>> +				"ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
>> +				__LINE__, candidate->enable.continuous,
> 
>> +				continuous, mode,
>> +				IA_CSS_BINARY_MODE_COPY);
> 
> Now one line?

Ack, fixed in my media-atomisp branch.

> ...
> 
>> -			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>> -					    "ia_css_binary_find() [%d] continue: binary is not striped\n",
>> -					    __LINE__);
>> +			dev_dbg(atomisp_dev,
>> +				"ia_css_binary_find() [%d] continue: binary is not striped\n",
>> +				__LINE__);
> 
> Now the __LINE__ argument is redundant as one may run-time turn it on and off.
> So, trimming it while converting to dev_dbg() makes sense to me as a one
> logical change. Ditto for other __LINE__ cases.

Dropping __LINE__ is the bit which I want to postpone to a follow-up patch,
the messages rejecting certain binaries as not suitable are not very
unique and the __LINE__ print is necessary (atm) to help find which check
exactly is failing. Not ideal I know, something for the TODO list.

> Otherwise it's a good clean up!

Thanks.

Regards,

Hans
Andy Shevchenko Sept. 3, 2024, 10:30 a.m. UTC | #3 
On Tue, Sep 3, 2024 at 12:27 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 9/2/24 2:03 PM, Andy Shevchenko wrote:
> > On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:

...

> >> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
> >> -                            struct ia_css_binary *binary) {
> >> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
> >> +                   struct ia_css_binary *binary)
> >
> > ...for example, in this case you touched both lines, so making them a single
> > line just reduces the future churn.
>
> Ack, fixed in my media-atomisp branch.

(Actually there are more opportunities like this that I haven't
commented on in the previous reply. I hope you have already found them
and fixed accordingly)

...

> >> +                    dev_dbg(atomisp_dev,
> >> +                            "ia_css_binary_find() [%d] continue: binary is not striped\n",
> >> +                            __LINE__);
> >
> > Now the __LINE__ argument is redundant as one may run-time turn it on and off.
> > So, trimming it while converting to dev_dbg() makes sense to me as a one
> > logical change. Ditto for other __LINE__ cases.
>
> Dropping __LINE__ is the bit which I want to postpone to a follow-up patch,
> the messages rejecting certain binaries as not suitable are not very
> unique and the __LINE__ print is necessary (atm) to help find which check
> exactly is failing. Not ideal I know, something for the TODO list.

Yes, it's fine, after I sent the previous reply I saw that the
previous implementation was a simple wrapper over dev_dbg() and
__LINE__ "issue" was already there. I.o.w. I agree on the followup!
Hans de Goede Sept. 3, 2024, 11:12 a.m. UTC | #4 
Hi,

On 9/3/24 12:30 PM, Andy Shevchenko wrote:
> On Tue, Sep 3, 2024 at 12:27 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 9/2/24 2:03 PM, Andy Shevchenko wrote:
>>> On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
> 
> ...
> 
>>>> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
>>>> -                            struct ia_css_binary *binary) {
>>>> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
>>>> +                   struct ia_css_binary *binary)
>>>
>>> ...for example, in this case you touched both lines, so making them a single
>>> line just reduces the future churn.
>>
>> Ack, fixed in my media-atomisp branch.
> 
> (Actually there are more opportunities like this that I haven't
> commented on in the previous reply. I hope you have already found them
> and fixed accordingly)

Yes I've gone over the entire patch and made lines longer to avoid
needing multiple lines where possible.

Regards,

Hans
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index b0f904a5e442..1c72c2b5817f 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -923,8 +923,9 @@  ia_css_binary_fill_info(const struct ia_css_binary_xinfo *xinfo,
 	return 0;
 }
 
-static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
-				struct ia_css_binary *binary) {
+int ia_css_binary_find(struct ia_css_binary_descr *descr,
+		       struct ia_css_binary *binary)
+{
 	int mode;
 	bool online;
 	bool two_ppc;
@@ -953,10 +954,10 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 	/* MW: used after an error check, may accept NULL, but doubtfull */
 	assert(binary);
 
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
-			    descr, descr->mode,
-			    binary);
+	dev_dbg(atomisp_dev,
+		"ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
+		descr, descr->mode,
+		binary);
 
 	mode = descr->mode;
 	online = descr->online;
@@ -1001,15 +1002,15 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 	}
 
 	/* print a map of the binary file */
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"BINARY INFO:\n");
+	dev_dbg(atomisp_dev,	"BINARY INFO:\n");
 	for (i = 0; i < IA_CSS_BINARY_NUM_MODES; i++) {
 		xcandidate = binary_infos[i];
 		if (xcandidate) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"%d:\n", i);
+			dev_dbg(atomisp_dev, "%d:\n", i);
 			while (xcandidate) {
-				ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, " Name:%s Type:%d Cont:%d\n",
-						    xcandidate->blob->name, xcandidate->type,
-						    xcandidate->sp.enable.continuous);
+				dev_dbg(atomisp_dev, " Name:%s Type:%d Cont:%d\n",
+					xcandidate->blob->name, xcandidate->type,
+					xcandidate->sp.enable.continuous);
 				xcandidate = xcandidate->next;
 			}
 		}
@@ -1021,9 +1022,9 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		struct ia_css_binary_info *candidate = &xcandidate->sp;
 		/* printf("sh_css_binary_find: evaluating candidate:
 		 * %d\n",candidate->id); */
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-				    "ia_css_binary_find() candidate = %p, mode = %d ID = %d\n",
-				    candidate, candidate->pipeline.mode, candidate->id);
+		dev_dbg(atomisp_dev,
+			"ia_css_binary_find() candidate = %p, mode = %d ID = %d\n",
+			candidate, candidate->pipeline.mode, candidate->id);
 
 		/*
 		 * MW: Only a limited set of jointly configured binaries can
@@ -1032,17 +1033,17 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		*/
 		if (!candidate->enable.continuous &&
 		    continuous && (mode != IA_CSS_BINARY_MODE_COPY)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
-					    __LINE__, candidate->enable.continuous,
-					    continuous, mode,
-					    IA_CSS_BINARY_MODE_COPY);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
+				__LINE__, candidate->enable.continuous,
+				continuous, mode,
+				IA_CSS_BINARY_MODE_COPY);
 			continue;
 		}
 		if (striped && candidate->iterator.num_stripes == 1) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: binary is not striped\n",
-					    __LINE__);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: binary is not striped\n",
+				__LINE__);
 			continue;
 		}
 
@@ -1050,58 +1051,58 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		    (mode != IA_CSS_BINARY_MODE_COPY) &&
 		    (mode != IA_CSS_BINARY_MODE_CAPTURE_PP) &&
 		    (mode != IA_CSS_BINARY_MODE_VF_PP)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d != %d)\n",
-					    __LINE__,
-					    candidate->pipeline.isp_pipe_version, isp_pipe_version);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d != %d)\n",
+				__LINE__,
+				candidate->pipeline.isp_pipe_version, isp_pipe_version);
 			continue;
 		}
 		if (!candidate->enable.reduced_pipe && enable_reduced_pipe) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && %d\n",
-					    __LINE__,
-					    candidate->enable.reduced_pipe,
-					    enable_reduced_pipe);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && %d\n",
+				__LINE__,
+				candidate->enable.reduced_pipe,
+				enable_reduced_pipe);
 			continue;
 		}
 		if (!candidate->enable.dvs_6axis && enable_dvs_6axis) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && %d\n",
-					    __LINE__,
-					    candidate->enable.dvs_6axis,
-					    enable_dvs_6axis);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && %d\n",
+				__LINE__,
+				candidate->enable.dvs_6axis,
+				enable_dvs_6axis);
 			continue;
 		}
 		if (candidate->enable.high_speed && !enable_high_speed) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: %d && !%d\n",
-					    __LINE__,
-					    candidate->enable.high_speed,
-					    enable_high_speed);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: %d && !%d\n",
+				__LINE__,
+				candidate->enable.high_speed,
+				enable_high_speed);
 			continue;
 		}
 		if (!candidate->enable.xnr && need_xnr) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: %d && !%d\n",
-					    __LINE__,
-					    candidate->enable.xnr,
-					    need_xnr);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: %d && !%d\n",
+				__LINE__,
+				candidate->enable.xnr,
+				need_xnr);
 			continue;
 		}
 		if (!(candidate->enable.ds & 2) && enable_yuv_ds) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && %d\n",
-					    __LINE__,
-					    ((candidate->enable.ds & 2) != 0),
-					    enable_yuv_ds);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && %d\n",
+				__LINE__,
+				((candidate->enable.ds & 2) != 0),
+				enable_yuv_ds);
 			continue;
 		}
 		if ((candidate->enable.ds & 2) && !enable_yuv_ds) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: %d && !%d\n",
-					    __LINE__,
-					    ((candidate->enable.ds & 2) != 0),
-					    enable_yuv_ds);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: %d && !%d\n",
+				__LINE__,
+				((candidate->enable.ds & 2) != 0),
+				enable_yuv_ds);
 			continue;
 		}
 
@@ -1115,100 +1116,100 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 				       candidate->vf_dec.is_variable ||
 				       /* or more than one output pin. */
 				       xcandidate->num_output_pins > 1)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%p != NULL) && !(%d || %d || (%d >%d))\n",
-					    __LINE__, req_vf_info,
-					    candidate->enable.vf_veceven,
-					    candidate->vf_dec.is_variable,
-					    xcandidate->num_output_pins, 1);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%p != NULL) && !(%d || %d || (%d >%d))\n",
+				__LINE__, req_vf_info,
+				candidate->enable.vf_veceven,
+				candidate->vf_dec.is_variable,
+				xcandidate->num_output_pins, 1);
 			continue;
 		}
 		if (!candidate->enable.dvs_envelope && need_dvs) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && %d\n",
-					    __LINE__,
-					    candidate->enable.dvs_envelope, (int)need_dvs);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && %d\n",
+				__LINE__,
+				candidate->enable.dvs_envelope, (int)need_dvs);
 			continue;
 		}
 		/* internal_res check considers input, output, and dvs envelope sizes */
 		ia_css_binary_internal_res(req_in_info, req_bds_out_info,
 					   req_bin_out_info, &dvs_env, candidate, &internal_res);
 		if (internal_res.width > candidate->internal.max_width) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d > %d)\n",
-					    __LINE__, internal_res.width,
-					    candidate->internal.max_width);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d > %d)\n",
+				__LINE__, internal_res.width,
+				candidate->internal.max_width);
 			continue;
 		}
 		if (internal_res.height > candidate->internal.max_height) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d > %d)\n",
-					    __LINE__, internal_res.height,
-					    candidate->internal.max_height);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d > %d)\n",
+				__LINE__, internal_res.height,
+				candidate->internal.max_height);
 			continue;
 		}
 		if (!candidate->enable.ds && need_ds && !(xcandidate->num_output_pins > 1)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && %d\n",
-					    __LINE__, candidate->enable.ds, (int)need_ds);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && %d\n",
+				__LINE__, candidate->enable.ds, (int)need_ds);
 			continue;
 		}
 		if (!candidate->enable.uds && !candidate->enable.dvs_6axis && need_dz) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && !%d && %d\n",
-					    __LINE__, candidate->enable.uds,
-					    candidate->enable.dvs_6axis, (int)need_dz);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && !%d && %d\n",
+				__LINE__, candidate->enable.uds,
+				candidate->enable.dvs_6axis, (int)need_dz);
 			continue;
 		}
 		if (online && candidate->input.source == IA_CSS_BINARY_INPUT_MEMORY) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: %d && (%d == %d)\n",
-					    __LINE__, online, candidate->input.source,
-					    IA_CSS_BINARY_INPUT_MEMORY);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: %d && (%d == %d)\n",
+				__LINE__, online, candidate->input.source,
+				IA_CSS_BINARY_INPUT_MEMORY);
 			continue;
 		}
 		if (!online && candidate->input.source == IA_CSS_BINARY_INPUT_SENSOR) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d && (%d == %d)\n",
-					    __LINE__, online, candidate->input.source,
-					    IA_CSS_BINARY_INPUT_SENSOR);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d && (%d == %d)\n",
+				__LINE__, online, candidate->input.source,
+				IA_CSS_BINARY_INPUT_SENSOR);
 			continue;
 		}
 		if (req_bin_out_info->res.width < candidate->output.min_width ||
 		    req_bin_out_info->res.width > candidate->output.max_width) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d > %d) || (%d < %d)\n",
-					    __LINE__,
-					    req_bin_out_info->padded_width,
-					    candidate->output.min_width,
-					    req_bin_out_info->padded_width,
-					    candidate->output.max_width);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d > %d) || (%d < %d)\n",
+				__LINE__,
+				req_bin_out_info->padded_width,
+				candidate->output.min_width,
+				req_bin_out_info->padded_width,
+				candidate->output.max_width);
 			continue;
 		}
 		if (xcandidate->num_output_pins > 1 &&
 		    /* in case we have a second output pin, */
 		    req_vf_info) { /* and we need vf output. */
 			if (req_vf_info->res.width > candidate->output.max_width) {
-				ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-						    "ia_css_binary_find() [%d] continue: (%d < %d)\n",
-						    __LINE__,
-						    req_vf_info->res.width,
-						    candidate->output.max_width);
+				dev_dbg(atomisp_dev,
+					"ia_css_binary_find() [%d] continue: (%d < %d)\n",
+					__LINE__,
+					req_vf_info->res.width,
+					candidate->output.max_width);
 				continue;
 			}
 		}
 		if (req_in_info->padded_width > candidate->input.max_width) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d > %d)\n",
-					    __LINE__, req_in_info->padded_width,
-					    candidate->input.max_width);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d > %d)\n",
+				__LINE__, req_in_info->padded_width,
+				candidate->input.max_width);
 			continue;
 		}
 		if (!binary_supports_output_format(xcandidate, req_bin_out_info->format)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: !%d\n",
-					    __LINE__,
-					    binary_supports_output_format(xcandidate, req_bin_out_info->format));
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: !%d\n",
+				__LINE__,
+				binary_supports_output_format(xcandidate, req_bin_out_info->format));
 			continue;
 		}
 		if (xcandidate->num_output_pins > 1 &&
@@ -1217,11 +1218,11 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		    /* check if the required vf format
 		    is supported. */
 		    !binary_supports_output_format(xcandidate, req_vf_info->format)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d > %d) && (%p != NULL) && !%d\n",
-					    __LINE__, xcandidate->num_output_pins, 1,
-					    req_vf_info,
-					    binary_supports_output_format(xcandidate, req_vf_info->format));
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d > %d) && (%p != NULL) && !%d\n",
+				__LINE__, xcandidate->num_output_pins, 1,
+				req_vf_info,
+				binary_supports_output_format(xcandidate, req_vf_info->format));
 			continue;
 		}
 
@@ -1229,11 +1230,11 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		if (xcandidate->num_output_pins == 1 &&
 		    req_vf_info && candidate->enable.vf_veceven &&
 		    !binary_supports_vf_format(xcandidate, req_vf_info->format)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: (%d == %d) && (%p != NULL) && %d && !%d\n",
-					    __LINE__, xcandidate->num_output_pins, 1,
-					    req_vf_info, candidate->enable.vf_veceven,
-					    binary_supports_vf_format(xcandidate, req_vf_info->format));
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: (%d == %d) && (%p != NULL) && %d && !%d\n",
+				__LINE__, xcandidate->num_output_pins, 1,
+				req_vf_info, candidate->enable.vf_veceven,
+				binary_supports_vf_format(xcandidate, req_vf_info->format));
 			continue;
 		}
 
@@ -1241,37 +1242,37 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		if (xcandidate->num_output_pins == 1 &&
 		    req_vf_info && candidate->enable.vf_veceven) { /* and we need vf output. */
 			if (req_vf_info->res.width > candidate->output.max_width) {
-				ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-						    "ia_css_binary_find() [%d] continue: (%d < %d)\n",
-						    __LINE__,
-						    req_vf_info->res.width,
-						    candidate->output.max_width);
+				dev_dbg(atomisp_dev,
+					"ia_css_binary_find() [%d] continue: (%d < %d)\n",
+					__LINE__,
+					req_vf_info->res.width,
+					candidate->output.max_width);
 				continue;
 			}
 		}
 
 		if (!supports_bds_factor(candidate->bds.supported_bds_factors,
 					 descr->required_bds_factor)) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
-					    __LINE__, candidate->bds.supported_bds_factors,
-					    descr->required_bds_factor);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
+				__LINE__, candidate->bds.supported_bds_factors,
+				descr->required_bds_factor);
 			continue;
 		}
 
 		if (!candidate->enable.dpc && need_dpc) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
-					    __LINE__, candidate->enable.dpc,
-					    descr->enable_dpc);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
+				__LINE__, candidate->enable.dpc,
+				descr->enable_dpc);
 			continue;
 		}
 
 		if (candidate->uds.use_bci && enable_capture_pp_bli) {
-			ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-					    "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
-					    __LINE__, candidate->uds.use_bci,
-					    descr->enable_capture_pp_bli);
+			dev_dbg(atomisp_dev,
+				"ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
+				__LINE__, candidate->uds.use_bci,
+				descr->enable_capture_pp_bli);
 			continue;
 		}
 
@@ -1290,13 +1291,6 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 		break;
 	}
 
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "ia_css_binary_find() selected = %p, mode = %d ID = %d\n",
-			    xcandidate, xcandidate ? xcandidate->sp.pipeline.mode : 0, xcandidate ? xcandidate->sp.id : 0);
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "ia_css_binary_find() leave: return_err=%d\n", err);
-
 	if (!err && xcandidate)
 		dev_dbg(atomisp_dev,
 			"Using binary %s (id %d), type %d, mode %d, continuous %s\n",
@@ -1306,23 +1300,12 @@  static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
 			xcandidate->sp.pipeline.mode,
 			xcandidate->sp.enable.continuous ? "true" : "false");
 
+	if (err)
+		dev_err(atomisp_dev, "Failed to find a firmware binary matching the pipeline parameters\n");
 
 	return err;
 }
 
-int ia_css_binary_find(struct ia_css_binary_descr *descr,
-		       struct ia_css_binary *binary)
-{
-	int ret = __ia_css_binary_find(descr, binary);
-
-	if (unlikely(ret)) {
-		dev_dbg(atomisp_dev, "Seeking for binary failed at:");
-		dump_stack();
-	}
-
-	return ret;
-}
-
 unsigned
 ia_css_binary_max_vf_width(void)
 {