Hi Ming, On Fri, Aug 26, 2022 at 07:47:47AM +0000, Ming Qian wrote: > [snip] > > >> diff --git a/drivers/media/platform/amphion/vpu_helpers.c > >> b/drivers/media/platform/amphion/vpu_helpers.c > >> index e9aeb3453dfc..019c77e84514 100644 > >> --- a/drivers/media/platform/amphion/vpu_helpers.c > >> +++ b/drivers/media/platform/amphion/vpu_helpers.c > >> @@ -59,6 +59,36 @@ const struct vpu_format > >*vpu_helper_find_format(struct vpu_inst *inst, u32 type, > >> return NULL; > >> } > >> > >> +const struct vpu_format *vpu_helper_find_sibling(struct vpu_inst > >> +*inst, u32 type, u32 pixelfmt) { > >> + const struct vpu_format *fmt; > >> + const struct vpu_format *sibling; > >> + > >> + fmt = vpu_helper_find_format(inst, type, pixelfmt); > >> + if (!fmt || !fmt->sibling) > >> + return NULL; > >> + > >> + sibling = vpu_helper_find_format(inst, type, fmt->sibling); > >> + if (!sibling || sibling->sibling != fmt->pixfmt || > >> + sibling->comp_planes != fmt->comp_planes) > >> + return NULL; > > > >I think to preserve code style you need the following solutions on this if > >statement: > > > >if (!sibling || (sibling->sibling != fmt->pixfmt) || > > (sibling->comp_planes != fmt->comp_planes)) > > return NULL; > > > >I think I have suggested to you this solution on the v4. But never mind we > >need this :) > > > >Thanks > > > > Hi Tommaso, > The parentheses are unnecessary, the checkpatch.pl will report the following style problems if I add the parentheses: > > CHECK: Unnecessary parentheses around 'sibling->sibling != fmt->pixfmt' > #11: FILE: drivers/media/platform/amphion/vpu_helpers.c:72: > + if (!sibling || (sibling->sibling != fmt->pixfmt) || > + (sibling->comp_planes != fmt->comp_planes)) > > CHECK: Unnecessary parentheses around 'sibling->comp_planes != fmt->comp_planes' > #11: FILE: drivers/media/platform/amphion/vpu_helpers.c:72: > + if (!sibling || (sibling->sibling != fmt->pixfmt) || > + (sibling->comp_planes != fmt->comp_planes)) > > total: 0 errors, 0 warnings, 2 checks, 10 lines checked > > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inplace. > > 1.patch has style problems, please review. > > NOTE: If any of the errors are false positives, please report > them to the maintainer, see CHECKPATCH in MAINTAINERS. Sorry, my bad. I don't check this using checkpatch.pl, but checking the others driver code, you are right. Thanks for clarify this. Then, Looks good to me. Regards, Tommaso -- Tommaso Merciai Embedded Linux Engineer tommaso.merciai@xxxxxxxxxxxxxxxxxxxx __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@xxxxxxxxxxxxxxxxxxxx www.amarulasolutions.com