Message-ID: <2025112448-brush-porcupine-c851@gregkh>
Date: Mon, 24 Nov 2025 17:33:27 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Chaoyi Chen <chaoyi.chen@...k-chips.com>
Cc: Chaoyi Chen <kernel@...kyi.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
	Peter Chen <hzpeterchen@...il.com>,
	Luca Ceresoli <luca.ceresoli@...tlin.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Heiko Stuebner <heiko@...ech.de>, Sandy Huang <hjc@...k-chips.com>,
	Andy Yan <andy.yan@...k-chips.com>,
	Yubing Zhang <yubing.zhang@...k-chips.com>,
	Frank Wang <frank.wang@...k-chips.com>,
	Andrzej Hajda <andrzej.hajda@...el.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Robert Foss <rfoss@...nel.org>,
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
	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>,
	Amit Sunil Dhamne <amitsd@...gle.com>,
	Dragan Simic <dsimic@...jaro.org>, Johan Jonker <jbx6244@...il.com>,
	Diederik de Haas <didi.debian@...ow.org>,
	Peter Robinson <pbrobinson@...il.com>, linux-usb@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-phy@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v10 01/11] usb: typec: Add notifier functions

On Mon, Nov 24, 2025 at 04:05:53PM +0800, Chaoyi Chen wrote:
> Hi Greg,
> 
> On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote:
> 
> > On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
> > > Hi Greg,
> > > 
> > > On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
> > > > On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
> > > > > From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
> > > > > 
> > > > > Some other part of kernel may want to know the event of typec bus.
> > > > Be specific, WHAT part of the kernel will need to know this?
> > > For now, it is DRM.
> > Then say this.
> 
> Okay, please refer to the discussion below.
> 
> > 
> > > > And why a new notifier, why not just use the existing notifiers that you
> > > > already have?  And what is this going to be used for?
> > > We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
> > > 
> > > [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/
> > Then you need to document the heck out of this in the changelog text.
> > But I'm still not quite understanding why the bus notifier does not work
> > here, as you only want this information if the usb device is bound to
> > the bus there, you do not want to know this if it did not complete.
> > 
> > That thread says you want this not "too late", but why?  What is the
> > problem there, and how will you handle your code getting loaded after
> > the typec code is loaded?  Notifier callbacks don't work for that
> > situation, right?
> 
> In fact, the typec_register_altmode() function generates two
> registered events. The first one is the registered event of the port
> device, and the second one is the registered event of the partner
> device. The second one event only occurs after a Type-C device is
> inserted.
> The bus notifier event does not actually take effect for the port
> device, because it only sets the bus for the partner device:
> 
>     /* The partners are bind to drivers */
>     if (is_typec_partner(parent))
>         alt->adev.dev.bus = &typec_bus;

Setting the bus is correct, then it needs to be registered with the
driver core so the bus link shows up (and a driver is bound to it.)
That is when the bus notifier can happen, right?

> I hope it's not too late. In fact, the notifier here will notify DRM to establish a bridge chain.

What is a "bridge chain"?

> The downstream DP controller driver hopes to obtain the fwnode of the last-level Type-C device
> through this bridge chain to create a DRM connector. And when a device is inserted,
> drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug Detect) event.

But aren't you just the driver for the "partner device"?

If not, why isn't a real device being created that you then bind to,
what "fake" type of thing are you attempting to do here that would
require you to do this out-of-band?

> If relying on the second event, the bridge chain may never be established, and the operations of the DP driver will be
> always deferred. Furthermore, other parts of the display controller driver will also be deferred accordingly.

What operations?  What exactly is delayed?  You should not be touching a
device before you have it on your bus, right?

> > > > Notifiers are a pain, and should almost never be added.  Use real
> > > > function calls instead.
> > > In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.
> > Only allow this DRM code to be built if typec code is enabled, do NOT
> > use a select, use a depends in the drm code.
> 
> Sorry, I didn't get your point. Does this mean that the current notifiers approach still needs to be changed to direct function calls?

If you somehow convince me that the existing bus notifiers will not
work, yes :)

> If so, then based on the previous discussion, typec should not depend
> on any DRM components. Does this mean that we should add the if
> (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call?

No, do it properly like any other function call to another subsystem.

> Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected
> by the DP driver in patch9.

Don't do "select" if at all possible, always try to do "depends on".

thanks,

greg k-h

Powered by blists - more mailing lists