linux-kernel - Re: [PATCH v7 03/12] drm/bridge: cdns-dsi: Fix phy de-init and flag it so

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0157aa47-9901-4f3d-b238-5b0ebeba78be@ideasonboard.com>
Date: Wed, 15 Jan 2025 10:17:16 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Aradhya Bhatia <aradhya.bhatia@...ux.dev>
Cc: Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
 Devarsh Thakkar <devarsht@...com>, Praneeth Bajjuri <praneeth@...com>,
 Udit Kumar <u-kumar1@...com>, Jayesh Choudhary <j-choudhary@...com>,
 DRI Development List <dri-devel@...ts.freedesktop.org>,
 Linux Kernel List <linux-kernel@...r.kernel.org>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Andrzej Hajda <andrzej.hajda@...el.com>,
 Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
 Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Subject: Re: [PATCH v7 03/12] drm/bridge: cdns-dsi: Fix phy de-init and flag
 it so

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().

> 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()...

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.

And I'll try to not nit-pick too much, so that we can actually get this 
series merged, and then later do the cleanups on top =).

  Tomi