From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 92EE7C43461 for ; Wed, 28 Apr 2021 16:20:31 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1B75B6143E for ; Wed, 28 Apr 2021 16:20:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1B75B6143E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 80E6E6E284; Wed, 28 Apr 2021 16:20:21 +0000 (UTC) Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7478E6EBA1 for ; Wed, 28 Apr 2021 16:19:58 +0000 (UTC) Received: by mail-pj1-x102b.google.com with SMTP id p17so5178068pjz.3 for ; Wed, 28 Apr 2021 09:19:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4TVKvwfAR1D9CGU5hfJk9qLIspo/mBQMGeGLnlUXBHo=; b=qXjQW1gloNRqT/ioT8O87kH0jp1kiN0yoUy538vQgmOD/Bwmt9RLGyrnJ07peLxOUi ZOKyIb1p5pnGeRFfHbHCa4SdLRmTz/EmEyQYF+Rb788LdVePab1ddARyIT0uSi6N5yUc xLYVyJHjXU7DZN7t/JeYa2zMlszKz45jNjocL9KK7g77hxqHojRJ5IpjxDuyXUfnW9Tv MEisgxRBrrPUr0sf7mg17q4yC46T1p1ltToB/ZarU5Ujpx65otgojLGtKsaYo9w7Acs4 vVOoKfls1eFHJtY4/8iuy1ZnXdyaQIfMjVWFw9P84ViFL+ZKIv7sdN22POGDosLwvHHs pLfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4TVKvwfAR1D9CGU5hfJk9qLIspo/mBQMGeGLnlUXBHo=; b=huuMIGfXRw7ofBrS06u94TCxqxEXZxk9FF/qSFI7UQKOPZViLD7wU8VV07zqqXQeKW erwx6TYqz4PQHzjVToE1jDfXZ/C8bL3zKalaZMjfOtG8ERyC81oMYs+vp1snMtkcrK/v cy4yOVrf0oSX3ZTeAKarW+7MyWHMQDa5zeih9l6G9bFsLra07RB9NZzXBwJlCp0JeYCB AwWsUPGFN711dVyehhYzchGYowfIZ5xDMNjhzadQMaNqOfIreJOfNNtfHY7oJ7vf5KAm l7Bg+3JmWi/yc2pTIDirGdKVPtbEk/tcRt0Agh24fTrCUesrpFlN+bYvKUheLjcbeCx/ Nr3Q== X-Gm-Message-State: AOAM532xXwyqOiy0dnoNXNacopp5kOsEMw+3ZGtONpP/XbyB4Vfp61Oh uRnrOtAxadnkJ+DEj6nGxdjoLoGnE0/5LFBip05n+g== X-Google-Smtp-Source: ABdhPJzFt2+qv728FrylY3DS6G9OQcnTeZbneKKZFjRQHnC35ubSdx2ePAXnujrh0iXaw2HjO6KPWcLw0IW4E/kiQSU= X-Received: by 2002:a17:90b:3504:: with SMTP id ls4mr4637642pjb.222.1619626798043; Wed, 28 Apr 2021 09:19:58 -0700 (PDT) MIME-Version: 1.0 References: <1617847624-53611-1-git-send-email-tiantao6@hisilicon.com> In-Reply-To: <1617847624-53611-1-git-send-email-tiantao6@hisilicon.com> From: Robert Foss Date: Wed, 28 Apr 2021 18:19:47 +0200 Message-ID: Subject: Re: [PATCH] drm/bridge: simplify devm_drm_panel_bridge_add_typed To: Tian Tao X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Neil Armstrong , David Airlie , dri-devel , Yicong Yang , Andrzej Hajda Content-Type: multipart/mixed; boundary="===============0827058210==" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" --===============0827058210== Content-Type: multipart/alternative; boundary="000000000000f76bb405c10abf72" --000000000000f76bb405c10abf72 Content-Type: text/plain; charset="UTF-8" Hey Tian, Thanks for having a look at cleanup patches. On Thu, 8 Apr 2021 at 04:06, Tian Tao wrote: > Use devm_add_action_or_reset() instead of devres_alloc() and > devres_add(), which works the same. This will simplify the > code. There is no functional changes. > > Signed-off-by: Tian Tao > Signed-off-by: Yicong Yang > --- > drivers/gpu/drm/bridge/panel.c | 27 +++++++++++---------------- > 1 file changed, 11 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/panel.c > b/drivers/gpu/drm/bridge/panel.c > index c916f4b..e5ddefb 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -250,11 +250,9 @@ void drm_panel_bridge_remove(struct drm_bridge > *bridge) > } > EXPORT_SYMBOL(drm_panel_bridge_remove); > > -static void devm_drm_panel_bridge_release(struct device *dev, void *res) > +static void devm_drm_panel_bridge_release(void *bridge) > { > - struct drm_bridge **bridge = res; > - > - drm_panel_bridge_remove(*bridge); > + drm_panel_bridge_remove(bridge); > } > > /** > @@ -295,20 +293,17 @@ struct drm_bridge > *devm_drm_panel_bridge_add_typed(struct device *dev, > struct drm_panel *panel, > u32 connector_type) > { > - struct drm_bridge **ptr, *bridge; > - > - ptr = devres_alloc(devm_drm_panel_bridge_release, sizeof(*ptr), > - GFP_KERNEL); > - if (!ptr) > - return ERR_PTR(-ENOMEM); > + struct drm_bridge *bridge; > + int ret; > > bridge = drm_panel_bridge_add_typed(panel, connector_type); > - if (!IS_ERR(bridge)) { > - *ptr = bridge; > - devres_add(dev, ptr); > - } else { > - devres_free(ptr); > - } > + if (IS_ERR(bridge)) > + return bridge; > + > + ret = devm_add_action_or_reset(dev, devm_drm_panel_bridge_release, > + bridge); > + if (ret) > + return ERR_PTR(ret); > > return bridge; > } > > I'm not crazy about this change. In my mind it is harder to read & understand than the current solution. If anyone else feels otherwise, I'm open to have my mind changed. Rob. --000000000000f76bb405c10abf72 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hey Tian,

Th= anks for having a look at cleanup patches.

On Thu, 8 Apr 2021 at 04:06, = Tian Tao <tiantao6@hisilicon.c= om> wrote:
tiantao6@hisilicon.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
=C2=A0drivers/gpu/drm/bridge/panel.c | 27 +++++++++++----------------
=C2=A01 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.= c
index c916f4b..e5ddefb 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -250,11 +250,9 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge= )
=C2=A0}
=C2=A0EXPORT_SYMBOL(drm_panel_bridge_remove);

-static void devm_drm_panel_bridge_release(struct device *dev, void *res) +static void devm_drm_panel_bridge_release(void *bridge)
=C2=A0{
-=C2=A0 =C2=A0 =C2=A0 =C2=A0struct drm_bridge **bridge =3D res;
-
-=C2=A0 =C2=A0 =C2=A0 =C2=A0drm_panel_bridge_remove(*bridge);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0drm_panel_bridge_remove(bridge);
=C2=A0}

=C2=A0/**
@@ -295,20 +293,17 @@ struct drm_bridge *devm_drm_panel_bridge_add_typed(st= ruct device *dev,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct drm_panel *panel,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 connector_type)
=C2=A0{
-=C2=A0 =C2=A0 =C2=A0 =C2=A0struct drm_bridge **ptr, *bridge;
-
-=C2=A0 =C2=A0 =C2=A0 =C2=A0ptr =3D devres_alloc(devm_drm_panel_bridge_rele= ase, sizeof(*ptr),
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 GFP_KERNEL);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!ptr)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return ERR_PTR(-ENO= MEM);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0struct drm_bridge *bridge;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0int ret;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 bridge =3D drm_panel_bridge_add_typed(panel, co= nnector_type);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!IS_ERR(bridge)) {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*ptr =3D bridge; -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0devres_add(dev, ptr= );
-=C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0devres_free(ptr); -=C2=A0 =C2=A0 =C2=A0 =C2=A0}
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (IS_ERR(bridge))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return bridge;
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D devm_add_action_or_reset(dev, devm_drm_= panel_bridge_release,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bridge);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return ERR_PTR(ret)= ;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 return bridge;
=C2=A0}

=C2=A0
I'm not crazy about t= his change. In my mind it is harder to read & understand than the curre= nt solution. If anyone else feels otherwise, I'm open to have my mind c= hanged.


Rob.
--000000000000f76bb405c10abf72-- --===============0827058210== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0827058210==--