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,URIBL_BLOCKED autolearn=unavailable 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 9466FC433C1 for ; Fri, 26 Mar 2021 01:03:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 52AB161A1E for ; Fri, 26 Mar 2021 01:03:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229733AbhCZBCv (ORCPT ); Thu, 25 Mar 2021 21:02:51 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:60772 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229988AbhCZBC2 (ORCPT ); Thu, 25 Mar 2021 21:02:28 -0400 Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EBEC5443; Fri, 26 Mar 2021 02:02:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1616720547; bh=mSy6v/pmSuKkHHCJgekfe0uCrYrOMXd+/Xdd5V+A4GY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RgevzU3x9LecEhwbZgL1gkb869v7LzAfTHSl8mrV+bAyqYjSisHMzcxzuxIaxqb1/ 0aMwsNdvhCGYK+Ld0kgamhykI5f0V/In+C6SN+86EkmHpqyaY740mGPZy2R+8M7L/a B7lyOPnPZGfLWmVpg/n2bgFdcy//Iup11zwhf4Yc= Date: Fri, 26 Mar 2021 03:01:44 +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 Thu, Mar 25, 2021 at 05:43:22PM -0700, Doug Anderson wrote: > On Tue, Mar 23, 2021 at 4:03 PM Laurent Pinchart wrote: > > On Tue, Mar 23, 2021 at 03:55:05PM -0700, Doug Anderson wrote: > > > On Tue, Mar 23, 2021 at 2:42 PM Laurent Pinchart wrote: > > > > 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 ? > > > > > > Sure, it can be a separate patch. I'd kinda prefer it be a patch > > > _before_ ${SUBJECT} patch, though. Specifically it's harder for me to > > > reason about whether your new function call is in the right place and > > > won't cause any problems with the order being all jumbled. If we fix > > > the order first then it's easy to reason about your patch. > > > > > > > You can > > > > even submit a patch if you want :-) > > > > > > Happy to post it up if it won't cause more confusion w/ you posting > > > your next version and trying to figure out what to base it on (since > > > it will definitely conflict with your series). > > > > I'll need quite a bit of time before v2, as I'd like to test it, and > > that requires finishing support for the DSI bridge and the display > > controller :-) Please feel free to post a patch if you have time, I > > think it could get merged in drm-misc quite quickly. > > I haven't forgotten about this and I've got it written, but I'm trying > to put it together with the work I'm doing to fix EDID reading and > that's still going to take me a while longer. I'm out tomorrow but > _hoping_ that I'll be able to at least get a new patch series (at > least RFC quality) next week... No worries at all, it will take a few weeks at least before I get the display controller and DSI working on my board, so you're not blocking me :-) -- Regards, Laurent Pinchart