From: Robertus Diawan Chris <robertusdchris@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: hansg@kernel.org, mchehab@kernel.org,
sakari.ailus@linux.intel.com, andy@kernel.org,
gregkh@linuxfoundation.org, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev,
linux-kernel-mentees@lists.linuxfoundation.org,
skhan@linuxfoundation.org, me@brighamcampbell.com
Subject: Re: [PATCH RFT v2] media: atomisp: Add error check in create_host_regular_capture_pipeline()
Date: Sat, 25 Apr 2026 11:11:25 +0700 [thread overview]
Message-ID: <aew-7ecz_4MCfu4t@soyboi> (raw)
In-Reply-To: <aesudPcQ8Fec0_VU@ashevche-desk.local>
Hello Andy,
On Fri, Apr 24, 2026 at 11:48:52AM +0300, Andy Shevchenko wrote:
> On Fri, Apr 24, 2026 at 03:15:38PM +0700, Robertus Diawan Chris wrote:
> > When calling ia_css_pipeline_create_and_add_stage() function, there's no
> > condition to check whether ia_css_pipeline_create_and_add_stage() throw
> > an error or not. So add conditional check for the return value of
> > ia_css_pipeline_create_and_add_stage() function.
>
> > This is reported by Coverity Scan with CID 1408952 as UNUSED_VALUE.
>
> Unneeded detail in the commit message. Use the comment block for that (as there
> changelog right now and other stuff you explained).
Alright, I will keep that in mind for future patches.
> > Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> > Signed-off-by: Robertus Diawan Chris <robertusdchris@gmail.com>
> > ---
> > v1 -> v2: Add Fixes tag (suggested by Dan Carpenter).
> >
> > v1:
> > https://lore.kernel.org/all/20260423060246.296986-1-robertusdchris@gmail.com/
> >
> > I am not sure if omitting the error check is intentional or not, but from
> > what I see from another ia_css_pipeline_create_and_add_stage() usage
> > in the same function, we check the return value after the call and return
> > the error code if it's non-zero.
>
> Okay, while this observation sounds like a +1 to the suggested change,
> you still need to dive into the code and try to get if
> ia_css_pipeline_create_and_add_stage() failure is fatal or not for
> create_host_regular_capture_pipeline().
After diving further into the code, I am not confident enough to
continue this patch. I lack the context and not sure if this _seemingly_
small change can have a big side effect for the overall driver.
With that in mind, I will not continue with this patch. I will try to take a
look at another driver.
> > I also don't have the device to test this change.
>
> It's a pity as this driver is quite complicated thing...
Yeah... Let's just say it is _a sight to behold_ for newcomer like
myself :)
Thank you for taking the time to check this patch.
Best regards,
Robertus Diawan Chris
prev parent reply other threads:[~2026-04-25 4:11 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-24 8:15 [PATCH RFT v2] media: atomisp: Add error check in create_host_regular_capture_pipeline() Robertus Diawan Chris
2026-04-24 8:48 ` Andy Shevchenko
2026-04-25 4:11 ` Robertus Diawan Chris [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aew-7ecz_4MCfu4t@soyboi \
--to=robertusdchris@gmail.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hansg@kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=mchehab@kernel.org \
--cc=me@brighamcampbell.com \
--cc=sakari.ailus@linux.intel.com \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.