From patchwork Tue Jul 11 13:18:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 107372 Delivered-To: patch@linaro.org Received: by 10.140.101.44 with SMTP id t41csp4868023qge; Tue, 11 Jul 2017 06:20:42 -0700 (PDT) X-Received: by 10.98.157.207 with SMTP id a76mr50291476pfk.25.1499779242151; Tue, 11 Jul 2017 06:20:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1499779242; cv=none; d=google.com; s=arc-20160816; b=gUMZlS5sLfrtwP6DMCy26+lJae8N2+F/e5Pl9Xa74mb7QZ811wEt93GEf2pjLuvGKc QQ70TOxyhtRzP6oHj8mRXarP/fNV18CKjAPOj3KB4gJcgZDpNjk8HJaJT5ootO2egpGW FbxCB2Fx8yGzHjLXBflVN0n+iasTMOeX+aRYHct3dRtaU0IMs4SgaEAIOCZJ9vwGe4+Y YmjKQKLIBPaUu+Gd/M0vHc0cnoYN4Y7JB7b8LHYXb6FOEVzTP0y6KWN5y4EHkEwN9KKs kNp2yqLzrpncs2jkHDNWB7TaPbo1nDxEgLHijggEa+5WH7dhuATvKZ7kqsP6gNAvEgb7 roRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :arc-authentication-results; bh=tf3EskgNhM0+iFk2uNhCLwsIwzN14ez7OTnAlBsmJmI=; b=IHNeau7W7sXFha5Yny3DKRn2YMOLbuEZgmtfcXiKNmtelM9Aki2Bj+ulcp0VfIYCK8 GZw0TdIeVfMDW6u7Ez5PI7V5pnpwPUZy96FjmhpEZ/lJ4nhJdk56dQ08AoKwLK8CgmC6 CB8qundH7YeP7DvgCU/gbjeLvPT1xzQXVoTqu4zP/Pf+IddnhTen1M1sr7ZKHcvAxL8B q0O1HjKL1pWy6clAQqIn8cAHo/xdaT6rb9eaaBbD9WF409HYypqWbJfj2d6EwpFqIRP4 0E+eigJpVrJs3vSwFwSDf0Cksp+fPK3RO5dM0M03K2W6FBS+tVL86UJD3/rfYywamKoy F6Bw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id d188si10166204pfc.312.2017.07.11.06.20.41; Tue, 11 Jul 2017 06:20:42 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932396AbdGKNUc (ORCPT + 25 others); Tue, 11 Jul 2017 09:20:32 -0400 Received: from mout.kundenserver.de ([212.227.126.130]:63458 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932158AbdGKNUa (ORCPT ); Tue, 11 Jul 2017 09:20:30 -0400 Received: from wuerfel.lan ([5.56.224.194]) by mrelayeu.kundenserver.de (mreue003 [212.227.15.129]) with ESMTPA (Nemesis) id 0MC5H2-1dM9en3mtc-008sEM; Tue, 11 Jul 2017 15:20:06 +0200 From: Arnd Bergmann To: Steve Longerbeam , Philipp Zabel , Mauro Carvalho Chehab , Greg Kroah-Hartman Cc: Arnd Bergmann , Hans Verkuil , Marek Vasut , Russell King , linux-media@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] [media] staging/imx: remove confusing IS_ERR_OR_NULL usage Date: Tue, 11 Jul 2017 15:18:35 +0200 Message-Id: <20170711132001.2266388-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:BK6K4baMPUBV7qplhXJMekOFV8XkkEU9uc4IlfuCK6BEz9ouxW0 zy6pMgL//vVGK9eNog/tPpGgivHXEv4DgftD8JTF9SHH1gZjauD6BQ/SUugmDPnDFeXPDUC YF3J0de0aant2+xaFjZItQ0wUELT0PF3A1i/Zdq93xRAFwfH4sg9y1RqKnOgDyVChBMmoK9 8AStRgcfeT2vUeG8pbW1g== X-UI-Out-Filterresults: notjunk:1; V01:K0:KHIVr1kjtH4=:Q4tDNQTLMS9ftvBRnAfPlA t6k2pwJCqOPDJl9GTZaPt9x76fpgfxpA2jacRmnP7DSz/Rv7EXvpqWDdqhG073j9YQ1nnCRw+ LLtv75cyKIkEn0wMld+E7dC8CQyp0fAgH+uHowfqQqHYCo0JDDs9okxVnjTdF/iRTQ1lzjMnb qKwF352TSsWt6H57NYntKdpFki1hBfRka5rN7mutdWR7J6yvHLCGjKS8sxLkUJSYOGWqK1eof yGXDu5Tn1Ij1o3tqZ0jCJD66v/BQ5D2QT+aBtFSF0Xam2/FrETlc5eRW94mkoLUGthc5fC4ZH AXOLxjPMq1pMJTYeqs4c2P++qFedVttoczcxRgc8c5wFBooskU7ElFuFW1+K9FNiI8F2cOfSP 1kAjV+6ME/42bJoX/B3ZDksC1EnZtKYgHQvd+3CcYOCOfIMZb9e0DRoE9m4Dizxc6KUAr5bsx iQeIqbM38sQSyKkpD/OpcQSjeZrWyUALaMIRnLKdp11TEr1hwKWWstOsm91hyIO016RNOuoap 8XYr8hNdv3e6fmcgvC7W8vHOhdUN4O6cjgnKCbEdJ5oxxzdcAksjMCaAST69jZUJx1xWUHAsS mZC9kOOIC9x67H++RzSXWm45svE2g7RmaurIc0DOGlKxMA22iaNYiUYDnPKwpOaFGPjXZCy6T bi/9zUejAZZR20kUuUv1yJiMU8DS0w4GSpw6BkAzV2cFx1XHzIdsATCNSI7zsHxnjdCM= Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org While looking at a compiler warning, I noticed the use of IS_ERR_OR_NULL, which is generally a sign of a bad API design and should be avoided. In this driver, this is fairly easy, we can simply stop storing error pointers in persistent structures, and change the two functions that might return either a NULL pointer or an error code to consistently return error pointers when failing. of_parse_subdev() now separates the error code and the pointer it looks up, to clarify the interface. There are two cases where this function originally returns 'NULL', and I have changed that to '0' for success to keep the current behavior, though returning an error would also make sense there. Fixes: e130291212df ("[media] media: Add i.MX media core driver") Signed-off-by: Arnd Bergmann --- v2: fix type mismatch v3: rework of_parse_subdev() as well. --- drivers/staging/media/imx/imx-ic-prpencvf.c | 41 ++++++++++++----------- drivers/staging/media/imx/imx-media-csi.c | 30 ++++++++++------- drivers/staging/media/imx/imx-media-dev.c | 4 +-- drivers/staging/media/imx/imx-media-of.c | 50 ++++++++++++++++------------- drivers/staging/media/imx/imx-media-vdic.c | 37 +++++++++++---------- 5 files changed, 90 insertions(+), 72 deletions(-) -- 2.9.0 Reviewed-by: Steve Longerbeam Tested-by: Steve Longerbeam diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c index ed363fe3b3d0..7a9d9f32f989 100644 --- a/drivers/staging/media/imx/imx-ic-prpencvf.c +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c @@ -134,19 +134,19 @@ static inline struct prp_priv *sd_to_priv(struct v4l2_subdev *sd) static void prp_put_ipu_resources(struct prp_priv *priv) { - if (!IS_ERR_OR_NULL(priv->ic)) + if (priv->ic) ipu_ic_put(priv->ic); priv->ic = NULL; - if (!IS_ERR_OR_NULL(priv->out_ch)) + if (priv->out_ch) ipu_idmac_put(priv->out_ch); priv->out_ch = NULL; - if (!IS_ERR_OR_NULL(priv->rot_in_ch)) + if (priv->rot_in_ch) ipu_idmac_put(priv->rot_in_ch); priv->rot_in_ch = NULL; - if (!IS_ERR_OR_NULL(priv->rot_out_ch)) + if (priv->rot_out_ch) ipu_idmac_put(priv->rot_out_ch); priv->rot_out_ch = NULL; } @@ -154,43 +154,46 @@ static void prp_put_ipu_resources(struct prp_priv *priv) static int prp_get_ipu_resources(struct prp_priv *priv) { struct imx_ic_priv *ic_priv = priv->ic_priv; + struct ipu_ic *ic; + struct ipuv3_channel *out_ch, *rot_in_ch, *rot_out_ch; int ret, task = ic_priv->task_id; priv->ipu = priv->md->ipu[ic_priv->ipu_id]; - priv->ic = ipu_ic_get(priv->ipu, task); - if (IS_ERR(priv->ic)) { + ic = ipu_ic_get(priv->ipu, task); + if (IS_ERR(ic)) { v4l2_err(&ic_priv->sd, "failed to get IC\n"); - ret = PTR_ERR(priv->ic); + ret = PTR_ERR(ic); goto out; } + priv->ic = ic; - priv->out_ch = ipu_idmac_get(priv->ipu, - prp_channel[task].out_ch); - if (IS_ERR(priv->out_ch)) { + out_ch = ipu_idmac_get(priv->ipu, prp_channel[task].out_ch); + if (IS_ERR(out_ch)) { v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n", prp_channel[task].out_ch); - ret = PTR_ERR(priv->out_ch); + ret = PTR_ERR(out_ch); goto out; } + priv->out_ch = out_ch; - priv->rot_in_ch = ipu_idmac_get(priv->ipu, - prp_channel[task].rot_in_ch); - if (IS_ERR(priv->rot_in_ch)) { + rot_in_ch = ipu_idmac_get(priv->ipu, prp_channel[task].rot_in_ch); + if (IS_ERR(rot_in_ch)) { v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n", prp_channel[task].rot_in_ch); - ret = PTR_ERR(priv->rot_in_ch); + ret = PTR_ERR(rot_in_ch); goto out; } + priv->rot_in_ch = rot_in_ch; - priv->rot_out_ch = ipu_idmac_get(priv->ipu, - prp_channel[task].rot_out_ch); - if (IS_ERR(priv->rot_out_ch)) { + rot_out_ch = ipu_idmac_get(priv->ipu, prp_channel[task].rot_out_ch); + if (IS_ERR(rot_out_ch)) { v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n", prp_channel[task].rot_out_ch); - ret = PTR_ERR(priv->rot_out_ch); + ret = PTR_ERR(rot_out_ch); goto out; } + priv->rot_out_ch = rot_out_ch; return 0; out: diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index a2d26693912e..17fd1e61dd5d 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -122,11 +122,11 @@ static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev) static void csi_idmac_put_ipu_resources(struct csi_priv *priv) { - if (!IS_ERR_OR_NULL(priv->idmac_ch)) + if (priv->idmac_ch) ipu_idmac_put(priv->idmac_ch); priv->idmac_ch = NULL; - if (!IS_ERR_OR_NULL(priv->smfc)) + if (priv->smfc) ipu_smfc_put(priv->smfc); priv->smfc = NULL; } @@ -134,23 +134,27 @@ static void csi_idmac_put_ipu_resources(struct csi_priv *priv) static int csi_idmac_get_ipu_resources(struct csi_priv *priv) { int ch_num, ret; + struct ipu_smfc *smfc; + struct ipuv3_channel *idmac_ch; ch_num = IPUV3_CHANNEL_CSI0 + priv->smfc_id; - priv->smfc = ipu_smfc_get(priv->ipu, ch_num); - if (IS_ERR(priv->smfc)) { + smfc = ipu_smfc_get(priv->ipu, ch_num); + if (IS_ERR(smfc)) { v4l2_err(&priv->sd, "failed to get SMFC\n"); - ret = PTR_ERR(priv->smfc); + ret = PTR_ERR(smfc); goto out; } + priv->smfc = smfc; - priv->idmac_ch = ipu_idmac_get(priv->ipu, ch_num); - if (IS_ERR(priv->idmac_ch)) { + idmac_ch = ipu_idmac_get(priv->ipu, ch_num); + if (IS_ERR(idmac_ch)) { v4l2_err(&priv->sd, "could not get IDMAC channel %u\n", ch_num); - ret = PTR_ERR(priv->idmac_ch); + ret = PTR_ERR(idmac_ch); goto out; } + priv->idmac_ch = idmac_ch; return 0; out: @@ -1583,6 +1587,7 @@ static int csi_unsubscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh, static int csi_registered(struct v4l2_subdev *sd) { struct csi_priv *priv = v4l2_get_subdevdata(sd); + struct ipu_csi *csi; int i, ret; u32 code; @@ -1590,11 +1595,12 @@ static int csi_registered(struct v4l2_subdev *sd) priv->md = dev_get_drvdata(sd->v4l2_dev->dev); /* get handle to IPU CSI */ - priv->csi = ipu_csi_get(priv->ipu, priv->csi_id); - if (IS_ERR(priv->csi)) { + csi = ipu_csi_get(priv->ipu, priv->csi_id); + if (IS_ERR(csi)) { v4l2_err(&priv->sd, "failed to get CSI%d\n", priv->csi_id); - return PTR_ERR(priv->csi); + return PTR_ERR(csi); } + priv->csi = csi; for (i = 0; i < CSI_NUM_PADS; i++) { priv->pad[i].flags = (i == CSI_SINK_PAD) ? @@ -1663,7 +1669,7 @@ static void csi_unregistered(struct v4l2_subdev *sd) if (priv->fim) imx_media_fim_free(priv->fim); - if (!IS_ERR_OR_NULL(priv->csi)) + if (priv->csi) ipu_csi_put(priv->csi); } diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c index 48cbc7716758..d96f4512224f 100644 --- a/drivers/staging/media/imx/imx-media-dev.c +++ b/drivers/staging/media/imx/imx-media-dev.c @@ -87,11 +87,11 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd, if (pdev) devname = dev_name(&pdev->dev); - /* return NULL if this subdev already added */ + /* return -EEXIST if this subdev already added */ if (imx_media_find_async_subdev(imxmd, np, devname)) { dev_dbg(imxmd->md.dev, "%s: already added %s\n", __func__, np ? np->name : devname); - imxsd = NULL; + imxsd = ERR_PTR(-EEXIST); goto out; } diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c index b026fe66467c..12df09f52490 100644 --- a/drivers/staging/media/imx/imx-media-of.c +++ b/drivers/staging/media/imx/imx-media-of.c @@ -100,9 +100,9 @@ static void of_get_remote_pad(struct device_node *epnode, } } -static struct imx_media_subdev * +static int of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np, - bool is_csi_port) + bool is_csi_port, struct imx_media_subdev **subdev) { struct imx_media_subdev *imxsd; int i, num_pads, ret; @@ -110,13 +110,25 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np, if (!of_device_is_available(sd_np)) { dev_dbg(imxmd->md.dev, "%s: %s not enabled\n", __func__, sd_np->name); - return NULL; + *subdev = NULL; + /* unavailable is not an error */ + return 0; } /* register this subdev with async notifier */ imxsd = imx_media_add_async_subdev(imxmd, sd_np, NULL); - if (IS_ERR_OR_NULL(imxsd)) - return imxsd; + ret = PTR_ERR_OR_ZERO(imxsd); + if (ret) { + if (ret == -EEXIST) { + /* already added, everything is fine */ + *subdev = NULL; + return 0; + } + + /* other error, can't continue */ + return ret; + } + *subdev = imxsd; if (is_csi_port) { /* @@ -137,10 +149,11 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np, } else { num_pads = of_get_port_count(sd_np); if (num_pads != 1) { + /* confused, but no reason to give up here */ dev_warn(imxmd->md.dev, "%s: unknown device %s with %d ports\n", __func__, sd_np->name, num_pads); - return NULL; + return 0; } /* @@ -151,7 +164,7 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np, } if (imxsd->num_sink_pads >= num_pads) - return ERR_PTR(-EINVAL); + return -EINVAL; imxsd->num_src_pads = num_pads - imxsd->num_sink_pads; @@ -191,20 +204,15 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np, ret = of_add_pad_link(imxmd, pad, sd_np, remote_np, i, remote_pad); - if (ret) { - imxsd = ERR_PTR(ret); + if (ret) break; - } if (i < imxsd->num_sink_pads) { /* follow sink endpoints upstream */ - remote_imxsd = of_parse_subdev(imxmd, - remote_np, - false); - if (IS_ERR(remote_imxsd)) { - imxsd = remote_imxsd; + ret = of_parse_subdev(imxmd, remote_np, + false, &remote_imxsd); + if (ret) break; - } } of_node_put(remote_np); @@ -212,14 +220,14 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np, if (port != sd_np) of_node_put(port); - if (IS_ERR(imxsd)) { + if (ret) { of_node_put(remote_np); of_node_put(epnode); break; } } - return imxsd; + return ret; } int imx_media_of_parse(struct imx_media_dev *imxmd, @@ -236,11 +244,9 @@ int imx_media_of_parse(struct imx_media_dev *imxmd, if (!csi_np) break; - lcsi = of_parse_subdev(imxmd, csi_np, true); - if (IS_ERR(lcsi)) { - ret = PTR_ERR(lcsi); + ret = of_parse_subdev(imxmd, csi_np, true, &lcsi); + if (ret) goto err_put; - } ret = of_property_read_u32(csi_np, "reg", &csi_id); if (ret) { diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c index 7eabdc4aa79f..433474d58e3e 100644 --- a/drivers/staging/media/imx/imx-media-vdic.c +++ b/drivers/staging/media/imx/imx-media-vdic.c @@ -126,15 +126,15 @@ struct vdic_priv { static void vdic_put_ipu_resources(struct vdic_priv *priv) { - if (!IS_ERR_OR_NULL(priv->vdi_in_ch_p)) + if (priv->vdi_in_ch_p) ipu_idmac_put(priv->vdi_in_ch_p); priv->vdi_in_ch_p = NULL; - if (!IS_ERR_OR_NULL(priv->vdi_in_ch)) + if (priv->vdi_in_ch) ipu_idmac_put(priv->vdi_in_ch); priv->vdi_in_ch = NULL; - if (!IS_ERR_OR_NULL(priv->vdi_in_ch_n)) + if (priv->vdi_in_ch_n) ipu_idmac_put(priv->vdi_in_ch_n); priv->vdi_in_ch_n = NULL; @@ -146,40 +146,43 @@ static void vdic_put_ipu_resources(struct vdic_priv *priv) static int vdic_get_ipu_resources(struct vdic_priv *priv) { int ret, err_chan; + struct ipuv3_channel *ch; + struct ipu_vdi *vdi; priv->ipu = priv->md->ipu[priv->ipu_id]; - priv->vdi = ipu_vdi_get(priv->ipu); - if (IS_ERR(priv->vdi)) { + vdi = ipu_vdi_get(priv->ipu); + if (IS_ERR(vdi)) { v4l2_err(&priv->sd, "failed to get VDIC\n"); - ret = PTR_ERR(priv->vdi); + ret = PTR_ERR(vdi); goto out; } + priv->vdi = vdi; if (!priv->csi_direct) { - priv->vdi_in_ch_p = ipu_idmac_get(priv->ipu, - IPUV3_CHANNEL_MEM_VDI_PREV); - if (IS_ERR(priv->vdi_in_ch_p)) { + ch = ipu_idmac_get(priv->ipu, IPUV3_CHANNEL_MEM_VDI_PREV); + if (IS_ERR(ch)) { err_chan = IPUV3_CHANNEL_MEM_VDI_PREV; - ret = PTR_ERR(priv->vdi_in_ch_p); + ret = PTR_ERR(ch); goto out_err_chan; } + priv->vdi_in_ch_p = ch; - priv->vdi_in_ch = ipu_idmac_get(priv->ipu, - IPUV3_CHANNEL_MEM_VDI_CUR); - if (IS_ERR(priv->vdi_in_ch)) { + ch = ipu_idmac_get(priv->ipu, IPUV3_CHANNEL_MEM_VDI_CUR); + if (IS_ERR(ch)) { err_chan = IPUV3_CHANNEL_MEM_VDI_CUR; - ret = PTR_ERR(priv->vdi_in_ch); + ret = PTR_ERR(ch); goto out_err_chan; } + priv->vdi_in_ch = ch; - priv->vdi_in_ch_n = ipu_idmac_get(priv->ipu, - IPUV3_CHANNEL_MEM_VDI_NEXT); + ch = ipu_idmac_get(priv->ipu, IPUV3_CHANNEL_MEM_VDI_NEXT); if (IS_ERR(priv->vdi_in_ch_n)) { err_chan = IPUV3_CHANNEL_MEM_VDI_NEXT; - ret = PTR_ERR(priv->vdi_in_ch_n); + ret = PTR_ERR(ch); goto out_err_chan; } + priv->vdi_in_ch_n = ch; } return 0;