Hi Mauro, On Mon, 2016-10-24 at 07:05 -0200, Mauro Carvalho Chehab wrote: > Em Mon, 24 Oct 2016 11:22:08 +0800 > Tiffany Lin <tiffany.lin@xxxxxxxxxxxx> escreveu: > > > Hi Mauro, > > > > On Fri, 2016-10-21 at 11:01 -0200, Mauro Carvalho Chehab wrote: > > > Em Fri, 2 Sep 2016 20:19:54 +0800 > > > Tiffany Lin <tiffany.lin@xxxxxxxxxxxx> escreveu: > > > > > > > Add v4l2 layer decoder driver for MT8173 > > > > > > > > Signed-off-by: Tiffany Lin <tiffany.lin@xxxxxxxxxxxx> > > > > > > > +int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc) > > > > +{ > > > > + int ret = 0; > > > > + > > > > + switch (fourcc) { > > > > + case V4L2_PIX_FMT_H264: > > > > + case V4L2_PIX_FMT_VP8: > > > > + default: > > > > + return -EINVAL; > > > > + } > > > > > > Did you ever test this driver? The above code will *always* return > > > -EINVAL, with will cause vidioc_vdec_s_fmt() to always fail! > > > > > > I suspect that what you wanted to do, instead, is: > > > > > > switch (fourcc) { > > > case V4L2_PIX_FMT_H264: > > > case V4L2_PIX_FMT_VP8: > > > break; > > > default: > > > return -EINVAL; > > > > > > > The original idea here is that vp8 and h264 are added in later patches. > > If get this patch without later patches, it should return -EINVAL. > > I noticed your idea, but next time, don't add dead code like that. > Reviewers check patch by patch at the order they're present at the > patch series. > > So, don't add something broken by purpose, assuming that it would > be fixed later. > Got it. > > > > > > > Btw, this patch series has also several issues that were pointed by > > > checkpatch. Please *always* run checkpatch when submitting your work. > > > > > > You should take a look at the Kernel documentation about how to > > > submit patches, at: > > > https://mchehab.fedorapeople.org/kernel_docs/process/index.html > > > > > > PS.: this time, I fixed the checkpatch issues for you. So, let me know > > > if the patch below is OK, and I'll merge it at media upstream, > > > assuming that the other patches in this series are ok. > > > > > > > I did run checkpatch, but I don't know why these issues missed. > > probably I run checkpatch for all files not for patches. > > I will take a look at the documentation and keep this in mind for future > > upstream. > > Appreciated for your help. > > Checkpatch should be run patch by patch, as we expect that all patches > will follow the coding style and will compile fine, without introducing > warnings. > > I do compile the Kernel for every single patch I merge. > Got it. I will follow this. best regards, Tiffany > Regards, > Mauro -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html