| 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
| ||
|
Message-ID: <e9cf0aa8-ed32-4ffb-a755-150742455808@rock-chips.com>
Date: Tue, 23 Sep 2025 09:34:39 +0800
From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Chaoyi Chen <kernel@...kyi.com>
Cc: 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>,
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>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
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 v4 1/7] usb: typec: Add default HPD device when register
DisplayPort altmode
On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:
> On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
>>
>> Add default DRM AUX HPD bridge device when register DisplayPort
>> altmode. That makes it redundant for each Type-C driver to implement
>> a similar registration process in embedded scenarios.
>>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@...k-chips.com>
>> ---
>> drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
>> drivers/usb/typec/altmodes/displayport.h | 2 ++
>> drivers/usb/typec/class.c | 8 +++++++
>> include/linux/usb/typec_altmode.h | 2 ++
>> 4 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
>> index 1dcb77faf85d..e026dc6e5430 100644
>> --- a/drivers/usb/typec/altmodes/displayport.c
>> +++ b/drivers/usb/typec/altmodes/displayport.c
>> @@ -14,6 +14,7 @@
>> #include <linux/property.h>
>> #include <linux/usb/pd_vdo.h>
>> #include <linux/usb/typec_dp.h>
>> +#include <drm/bridge/aux-bridge.h>
>> #include <drm/drm_connector.h>
>> #include "displayport.h"
>>
>> @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
>> dp->pending_irq_hpd = true;
>> }
>> } else {
>> + if (dp->port->hpd_dev)
>> + drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>> + hpd ? connector_status_connected :
>> + connector_status_disconnected);
> There should be no need for these calls. Once the HPD bridge is added to
> a correct fwnode, the drm_connector_oob_hotplug_event() calls should
> deliver the signal as expected.
It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.
>
>> drm_connector_oob_hotplug_event(dp->connector_fwnode,
>> hpd ? connector_status_connected :
>> connector_status_disconnected);
>> @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
>> * configuration is complete to signal HPD.
>> */
>> if (dp->pending_hpd) {
>> + if (dp->port->hpd_dev)
>> + drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>> + connector_status_connected);
>> drm_connector_oob_hotplug_event(dp->connector_fwnode,
>> connector_status_connected);
>> sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
>> @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>> dp->data.status = 0;
>> dp->data.conf = 0;
>> if (dp->hpd) {
>> + if (dp->port->hpd_dev)
>> + drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>> + connector_status_disconnected);
>> drm_connector_oob_hotplug_event(dp->connector_fwnode,
>> connector_status_disconnected);
>> dp->hpd = false;
>> @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
>> NULL,
>> };
>>
>> +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
>> +{
>> + if (alt->svid != USB_TYPEC_DP_SID)
>> + return;
>> +
>> + alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
>> + dev_of_node(alt->dev.parent->parent));
> This needs at least a comment, what is dev.parent->parent. Also, the
> of_node is not correct here. It should be a node of the connector,
> rather than the device itself. Consider USB-C controllers which handle
> several USB-C connectors (e.g. UCSI). The DRM core won't be able to
> identify the correct bridge.
I think alt.dev->parent->parent is the connector device. Am I missing something?
>
>> + if (IS_ERR(alt->hpd_dev))
>> + alt->hpd_dev = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
> Having the function here will bring a typec -> displayport dependency
> between drivers (which you didn't document). It means it won't be
> possible to build typec core into the kernel, having the DP AltMode
> driver in the module (which also doesn't sound like a good idea).
It make sense. Perhaps moving it into class.c would be a good idea.
>
>> +
>> int dp_altmode_probe(struct typec_altmode *alt)
>> {
>> const struct typec_altmode *port = typec_altmode_get_partner(alt);
--
Best,
Chaoyi
Powered by blists - more mailing lists