On Tue, Sep 3, 2024 at 12:27 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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!
--
With Best Regards,
Andy Shevchenko
