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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,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 98111C433DB for ; Tue, 23 Mar 2021 21:43:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6AEBE619CE for ; Tue, 23 Mar 2021 21:43:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233854AbhCWVm4 (ORCPT ); Tue, 23 Mar 2021 17:42:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58388 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233786AbhCWVmE (ORCPT ); Tue, 23 Mar 2021 17:42:04 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69065C061574 for ; Tue, 23 Mar 2021 14:42:01 -0700 (PDT) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DD9D8F9B; Tue, 23 Mar 2021 22:41:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616535719; bh=w+w/Q8v+CNjN7zshlBlBpf1HsmNIwQA6auSIh8VzGc0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qaf01RgHITDnfQwv6+rljPk8akSaBqRl5f4U4erq0MLqjxJNBd36pin08MURIwlcE BhQXppYRkbwR/gEhF1OAX32rP/XwV/UMAOW31xgX493T2frwiUPSCmxs4PncgHT7SK 53gNRB6tY4BKHU6z5isVuUmmsTtOmyF0jkvCvoGs= Date: Tue, 23 Mar 2021 23:41:16 +0200 From: Laurent Pinchart To: Doug Anderson Cc: dri-devel , linux-renesas-soc@vger.kernel.org, Andrzej Hajda , Neil Armstrong , Jonas Karlman , Jernej Skrabec , Stephen Boyd Subject: Re: [RFC PATCH 03/11] drm/bridge: ti-sn65dsi86: Unregister AUX adapter in remove() Message-ID: References: <20210322030128.2283-1-laurent.pinchart+renesas@ideasonboard.com> <20210322030128.2283-4-laurent.pinchart+renesas@ideasonboard.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org Hi Doug, On Tue, Mar 23, 2021 at 02:08:42PM -0700, Doug Anderson wrote: > On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart wrote: > > > > The AUX adapter registered in probe() need to be unregistered in > > remove(). Do so. > > > > Fixes: b814ec6d4535 ("drm/bridge: ti-sn65dsi86: Implement AUX channel") > > Signed-off-by: Laurent Pinchart > > --- > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index da78a12e58b5..c45420a50e73 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -1307,6 +1307,9 @@ static int ti_sn_bridge_remove(struct i2c_client *client) > > return -EINVAL; > > > > kfree(pdata->edid); > > + > > + drm_dp_aux_unregister(&pdata->aux); > > + > > ti_sn_debugfs_remove(pdata); > > > > of_node_put(pdata->host_node); > > Good catch. One question, though. I know DRM sometimes has different > conventions than the rest of the kernel, but I always look for the > "remove" to be backwards of probe. That means that your code (and > probably most of the remove function) should come _after_ the > drm_bridge_remove(), right? ...since drm_bridge_add() was the last > thing in probe then drm_bridge_remove() should be the first thing in > remove? I agree in theory, yes. However, in practice, if you remove a bridge that is currently in use, all hell will break lose. And if the bridge isn't being used, it makes no difference. Still, it's worth changing the order of operations to move drm_bridge_remove() first, as it won't hurt in any case and is logically better. It's not an issue introduced by this series though, so how how about it on top, or in parallel ? You can even submit a patch if you want :-) -- Regards, Laurent Pinchart 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=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,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 B3615C433DB for ; Tue, 23 Mar 2021 21:42:05 +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 4A09161920 for ; Tue, 23 Mar 2021 21:42:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4A09161920 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com 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 584096E17A; Tue, 23 Mar 2021 21:42:04 +0000 (UTC) Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id BF4F96E17A for ; Tue, 23 Mar 2021 21:42:02 +0000 (UTC) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DD9D8F9B; Tue, 23 Mar 2021 22:41:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616535719; bh=w+w/Q8v+CNjN7zshlBlBpf1HsmNIwQA6auSIh8VzGc0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qaf01RgHITDnfQwv6+rljPk8akSaBqRl5f4U4erq0MLqjxJNBd36pin08MURIwlcE BhQXppYRkbwR/gEhF1OAX32rP/XwV/UMAOW31xgX493T2frwiUPSCmxs4PncgHT7SK 53gNRB6tY4BKHU6z5isVuUmmsTtOmyF0jkvCvoGs= Date: Tue, 23 Mar 2021 23:41:16 +0200 From: Laurent Pinchart To: Doug Anderson Subject: Re: [RFC PATCH 03/11] drm/bridge: ti-sn65dsi86: Unregister AUX adapter in remove() Message-ID: References: <20210322030128.2283-1-laurent.pinchart+renesas@ideasonboard.com> <20210322030128.2283-4-laurent.pinchart+renesas@ideasonboard.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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: Jernej Skrabec , Jonas Karlman , Neil Armstrong , dri-devel , Stephen Boyd , linux-renesas-soc@vger.kernel.org, Andrzej Hajda Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Doug, On Tue, Mar 23, 2021 at 02:08:42PM -0700, Doug Anderson wrote: > On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart wrote: > > > > The AUX adapter registered in probe() need to be unregistered in > > remove(). Do so. > > > > Fixes: b814ec6d4535 ("drm/bridge: ti-sn65dsi86: Implement AUX channel") > > Signed-off-by: Laurent Pinchart > > --- > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index da78a12e58b5..c45420a50e73 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -1307,6 +1307,9 @@ static int ti_sn_bridge_remove(struct i2c_client *client) > > return -EINVAL; > > > > kfree(pdata->edid); > > + > > + drm_dp_aux_unregister(&pdata->aux); > > + > > ti_sn_debugfs_remove(pdata); > > > > of_node_put(pdata->host_node); > > Good catch. One question, though. I know DRM sometimes has different > conventions than the rest of the kernel, but I always look for the > "remove" to be backwards of probe. That means that your code (and > probably most of the remove function) should come _after_ the > drm_bridge_remove(), right? ...since drm_bridge_add() was the last > thing in probe then drm_bridge_remove() should be the first thing in > remove? I agree in theory, yes. However, in practice, if you remove a bridge that is currently in use, all hell will break lose. And if the bridge isn't being used, it makes no difference. Still, it's worth changing the order of operations to move drm_bridge_remove() first, as it won't hurt in any case and is logically better. It's not an issue introduced by this series though, so how how about it on top, or in parallel ? You can even submit a patch if you want :-) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel