From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH v2] media: venus: add debugfs support Date: Mon, 7 Jan 2019 16:55:17 +0000 Message-ID: <20190107165517.GS2217@ZenIV.linux.org.uk> References: <1546871340-13009-1-git-send-email-mgottam@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1546871340-13009-1-git-send-email-mgottam@codeaurora.org> Sender: linux-kernel-owner@vger.kernel.org To: Malathi Gottam Cc: stanimir.varbanov@linaro.org, hverkuil@xs4all.nl, mchehab@kernel.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, acourbot@chromium.org, vgarodia@codeaurora.org List-Id: linux-arm-msm@vger.kernel.org On Mon, Jan 07, 2019 at 07:59:00PM +0530, Malathi Gottam wrote: > +static struct dentry *venus_debugfs_init_drv(void) > +{ > + bool ok = false; > + struct dentry *dir = NULL; > + > + dir = debugfs_create_dir("venus", NULL); > + if (IS_ERR_OR_NULL(dir)) { > + dir = NULL; > + pr_err("failed to create debug dir"); > + goto failed_create_dir; > + } Huh? When does debugfs_create_dir() return ERR_PTR()? Any interface that mixes returning NULL for error-reporting with returning ERR_PTR(...) for the same needs to be put out of its misery. As it happens, debugfs_create_dir() returns NULL on all errors. And if you've meant the above as "future-proofing"... don't do that, please. That only breeds confusion down the road, as the code gets cut'n'pasted around. > - if (ret) > + if (ret) { > + dprintk(ERR, > + "Failed to set actual buffer count %d for buffer type %d\n", > + buf_count.count_actual, buf_count.type); > return ret; > > buf_count.type = HFI_BUFFER_OUTPUT; > buf_count.count_actual = output_bufs; > *blink* Does that even compile?