Messages in this thread |  | | Subject | Re: [PATCH 05/17] media: atomisp: pci: fix inverted error check for ia_css_mipi_is_source_port_valid() | From | Tsuchiya Yuto <> | Date | Tue, 09 Nov 2021 00:00:29 +0900 |
| |
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() ?
Yeah, I also thought that the current true/false return value was a little bit confusing.
In another words, should the function return:
- negative values (maybe -EINVAL for this case) for invalid case - 0 for valid case
instead? and if we go this way, we should also rename the function name like
- check_ia_css_mipi_source_port_validity
or something. How does this sound?
Regards, Tsuchiya Yuto
|  |