From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-179.mta1.migadu.com (out-179.mta1.migadu.com [95.215.58.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6E7531FCCEE for ; Fri, 17 Jan 2025 13:13:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737119587; cv=none; b=AKBzVZyQh4aHPiBYdEwuzn2BkBlky2d1BnlJeTm0q7rSPQtTvMc66acUTmy7beAhvVZWrDCbVKpcFi6wi2SuHBXBksoAoMiFJsUv1OFpX5GEC777kOucZvoFTtDdCDxbJ0XS43XBB5nRfkqCXiTp+IJ8WWIQ7GHvAzN+1iWD41s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737119587; c=relaxed/simple; bh=eOlium9uun+BX2pm9NYkRGPMV78TnJJfw/yIBcvUDlc=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=NMCGNXl5aTs7r/VeuUBjoi36sufiTdhj1wOjDChoD0KSvwWD+wvXeNcg2ZPM+uoZ7gx6uv8Xq2d3PDxKOwMpyIBWVL7tFDFEnVU3M8fUmsji5eDtLmIvBnlfRkvOEQH2HVSu1CLKoLPHCrPvUkAAede1jwxQtdp0VA/oAUUY+6k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=xxoaM2gg; arc=none smtp.client-ip=95.215.58.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="xxoaM2gg" Message-ID: <8757b595-e1f3-4a04-b201-621237709e3c@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1737119582; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6kEmfvKGroG6EGG4ETw82Z+RqEAdG6vteEmzk4A6OrM=; b=xxoaM2ggG6CVzwTzh24CKOUNQghhK40xRzcO4Mu0AQ9K+yC4Ydqlh3EnJDbEXZJWHNvXhv zJvakx4rQog5BpZgP+E7WHi5ggWhKjlqy1aKGP2Ou2AspA0E7+RmT6SWnHRLRtucK9U60S LUCh/Yu6vsqvbbiYEiyzwPu5F+fgZT4= Date: Fri, 17 Jan 2025 18:42:11 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Aradhya Bhatia Subject: Re: [PATCH v7 03/12] drm/bridge: cdns-dsi: Fix phy de-init and flag it so To: Tomi Valkeinen Cc: Nishanth Menon , Vignesh Raghavendra , Devarsh Thakkar , Praneeth Bajjuri , Udit Kumar , Jayesh Choudhary , DRI Development List , Linux Kernel List , Dmitry Baryshkov , Laurent Pinchart , Andrzej Hajda , Neil Armstrong , Robert Foss , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter References: <20250114055626.18816-1-aradhya.bhatia@linux.dev> <20250114055626.18816-4-aradhya.bhatia@linux.dev> <84ca02de-9788-4e16-bf24-1651bd365ebd@ideasonboard.com> <7cfc1561-a229-43e7-b4bf-23ad258733c6@linux.dev> <0e0ee18e-28f6-4c57-a47d-cd7ace84fa70@ideasonboard.com> <6f7bafba-9b40-491f-bf6b-00094840089c@linux.dev> <0157aa47-9901-4f3d-b238-5b0ebeba78be@ideasonboard.com> Content-Language: en-US In-Reply-To: <0157aa47-9901-4f3d-b238-5b0ebeba78be@ideasonboard.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT Hi Tomi, On 15/01/25 13:47, Tomi Valkeinen wrote: > Hi, > > On 14/01/2025 18:32, Aradhya Bhatia wrote: > >>> But generally speaking, yes, it's good to keep fixes simple, and do >>> cleanups later on top. Keeping that in mind, maybe this current patch is >>> fine as it is. Although... if the init is done in pre_enable, shouldn't >>> the deinit be done in post_disable? >> >> Yes, I will move the deinit to _bridge_post_disable(). >> >> >> So, if we keep the fix limited to deinit in _bridge_post_disable(), then >> the cleanup would involve dropping the init calls from _bridge_enable(). >> And then the patch-12 would do 3 things - >> >>     1. Drop older _bridge_pre_enable() >>     2. Rename old _bridge_enable() to _bridge_pre_enable() >>     3. Since the _old_ _bridge_enable() has the calls dropped in the >>        cleanup patch, add those calls again in the _new_ >>        _bridge_pre_enable() (which are really the same function >>        bodies). > > I would think patch-12 differently: it doesn't do what you list above, > but rather combines the current pre_enable() and enable() into a new > pre_enable(). Right, yes! > >> Do you think we can instead skip the cleanup patch, as well as #3 from >> patch-12? > > Yes, I think the cleanup patch can just be dropped. It's not really > relevant. > >> Fun fact: We already have patch-4 which fixes the order of init calls in >> _bridge_enable()! =) > > Right. And I guess that fix doesn't fix anything in practice, as those > init calls are no-ops in the bridge_enable()... Yeah, it doesn't do anything... until patch-12 comes back in picture. So, I shall drop patch-4 too as there's no point in getting that patch backported. And I will let patch-12 take care of the correct ordering. > > It's a bit difficult to make meaningful fixes when things are so badly > messed up =). > > So, maybe try to arrange the series so that the obvious "makes-sense" > fixes for stable are in the beginning. So... patches 1, 3, 5? And then > work towards the patch 12. > Yes, this sounds good. Regards Aradhya