lkml.org 
[lkml]   [2021]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 05/17] media: atomisp: pci: fix inverted error check for ia_css_mipi_is_source_port_valid()
    From
    Date
    <removed Alan from Cc as the mail address not reachable>

    On Mon, 2021-11-08 at 18:14 +0300, Dan Carpenter wrote:
    > On Tue, Nov 09, 2021 at 12:00:29AM +0900, Tsuchiya Yuto wrote:
    > > On Tue, 2021-11-02 at 14:33 +0300, Dan Carpenter wrote:
    > > > On Mon, Oct 18, 2021 at 01:19:45AM +0900, Tsuchiya Yuto wrote:
    > > > > The function ia_css_mipi_is_source_port_valid() returns true if the port
    > > > > is valid. So, we can't use the existing err variable as is.
    > > > >
    > > > > To fix this issue while reusing that variable, invert the return value
    > > > > when assigning it to the variable.
    > > > >
    > > > > Fixes: 3c0538fbad9f ("media: atomisp: get rid of most checks for ISP2401 version")
    > > > > Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
    > > > > ---
    > > > > .../staging/media/atomisp/pci/sh_css_mipi.c | 24 ++++++++++++-------
    > > > > 1 file changed, 15 insertions(+), 9 deletions(-)
    > > > >
    > > > > diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
    > > > > index 65fc93c5d56b..c1f2f6151c5f 100644
    > > > > --- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
    > > > > +++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
    > > > > @@ -423,10 +423,12 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
    > > > > return 0; /* AM TODO: Check */
    > > > > }
    > > > >
    > > > > - if (!IS_ISP2401)
    > > > > + if (!IS_ISP2401) {
    > > > > port = (unsigned int)pipe->stream->config.source.port.port;
    > > > > - else
    > > > > - err = ia_css_mipi_is_source_port_valid(pipe, &port);
    > > > > + } else {
    > > > > + /* Returns true if port is valid. So, invert it */
    > > > > + err = !ia_css_mipi_is_source_port_valid(pipe, &port);
    > > >
    > > > Don't invert it... This isn't supposed to return 1 on failure it's
    > > > supposed to return negative error codes.
    > >
    > > You mean I should instead modify the return value of
    > > ia_css_mipi_is_source_port_valid() ?
    > >
    >
    > No. ia_css_mipi_is_source_port_valid() is fine. It has a boolean name
    > so returning bool is fine. What I'm saying is that allocate_mipi_frames()
    > should do:
    >
    > if (!ia_css_mipi_is_source_port_valid(pipe, &port))
    > err = -EINVAL;
    >
    > Otherwise it returns negative error codes and 1 on failure.

    Ah, I see! Thank you. I feel I'm a stupid... I'll do so in v2.

    Regards,
    Tsuchiya Yuto

    \
     
     \ /
      Last update: 2021-11-08 16:27    [W:4.236 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site