From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 E0A3761FFE for ; Tue, 23 Jun 2026 19:49:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782244163; cv=none; b=HYIYfv/PzyLIsIGTThv5z9f55rZyv5gagmZAOv2dIhHCBN7TkOtON3FqeaOm3JCc5k3qupSYPTA1qTXhVFEU1f5ty/oIbB9La6ZvdUrz5/MI5utwoZQKKuJKXAFlnuAFTVtygecx3eRq7iaMRNeO2mMC5oBcy19TUO0T0yaCOtQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782244163; c=relaxed/simple; bh=fqv+L8lo30inwOd4Y63isNwYuTnbTan3sOLhR6psEuU=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=OiKOizfu4yCDEZmyL08jmzygMKji5navLDGjkHExDpzwUIxoKlr2QQ2WeWIu0FAP9fRnFk8ki62Rj2swFCF7bl+dVuPiW6eCO98x6g7HcIV1vIVrmmdfR+HLYSrCmOyijdP7z6S0Y8oO9NrCTkvu/XB39SkzSHdYtvMgYjiWaXI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Nf8r1Zyz; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Nf8r1Zyz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4F4BD1F000E9; Tue, 23 Jun 2026 19:49:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782244162; bh=Ja5q2fZKG3v7RFMZdWsvuqRkGzU83kSeBhA8tTduTvU=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=Nf8r1ZyzR8a96W1ECLiC8OFvtA9C7n/wi23dyb5/qB2gz3uyuXhxGIMZLzVdJdtr1 nrNYR4+bKSLunMKzAEFu9Lqr7BjVpDCAu7ggkvZrJizqkriF6fHY/ZR9r/D2cQ2qtj GlhcxG5CFsJV8Met+N8YaUlNkizhq0nZqCzvJOxcwOTqbF/PRsh+0w3PPKI6RSkMza /TJ2N2oxytZbYye5EhBuRixKCEMhVRuc8xDJje0YZMAi/eRM7UBqfjxoF3v5g7t9Ut 9H76mBekept6+w18fE12+t1i/SeXCKvMwbvIfnFom+Z6Xr3dqZnU1TIoek3RuakAEv 38zHPFtwtFOFw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v3 03/10] drm/bridge: synopsys: dw-dp: Add follow-up bridge support Reply-To: sashiko-reviews@lists.linux.dev To: "Sebastian Reichel" Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org, dri-devel@lists.freedesktop.org In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-3-dc61e6352508@collabora.com> References: <20260612-synopsys-dw-dp-improvements-v3-3-dc61e6352508@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 23 Jun 2026 19:49:21 +0000 Message-Id: <20260623194922.4F4BD1F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 4 potential issue(= s) to consider: New issues: - [High] Memory leak of the follow-up bridge reference count. - [High] Unconditional probe failure causing DT backward compatibility brea= kage. Pre-existing issues: - [High] Missing completion reinitialization causes race conditions with st= ale completion states during AUX transfers. - [High] Premature registration and exposure of the DP AUX channel to users= pace before hardware and IRQ initialization. -- commit ceef3e5c9dcfb09773dad5a727e1e2147c87f8e5 Author: Sebastian Reichel drm/bridge: synopsys: dw-dp: Add follow-up bridge support Add support to use USB-C connectors with the DP altmode helper code on devicetree based platforms. To get this working there must be a DRM bridge chain from the DisplayPort controller to the USB-C connector. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/br= idge/synopsys/dw-dp.c > index ac7779e42bb24..98cff435dfb87 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c > @@ -1970,7 +1970,7 @@ struct dw_dp *dw_dp_bind(struct device *dev, struct= drm_encoder *encoder, > { > struct platform_device *pdev =3D to_platform_device(dev); > struct dw_dp *dp; > - struct drm_bridge *bridge; > + struct drm_bridge *bridge, *next_bridge; > void __iomem *res; > int ret; > =20 > @@ -2064,6 +2064,20 @@ struct dw_dp *dw_dp_bind(struct device *dev, struc= t drm_encoder *encoder, > goto unregister_aux; > } > =20 > + next_bridge =3D of_drm_get_bridge_by_endpoint(dev->of_node, 1, 0); > + if (IS_ERR(next_bridge)) { [Severity: High] Can this unconditional check break device tree backward compatibility? By making the absence of a follow-up bridge a fatal error instead of an optional configuration, this appears to cause the display driver to completely fail to probe on existing platforms that do not define a downstr= eam bridge endpoint in their device tree. > + ret =3D PTR_ERR(next_bridge); > + dev_err_probe(dev, ret, "failed to get follow-up bridge.\n"); > + goto unregister_aux; > + } > + > + ret =3D drm_bridge_attach(encoder, next_bridge, bridge, > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); [Severity: High] Does this code leak the next_bridge reference? Calling of_drm_get_bridge_by_endpoint() increments the reference count of t= he returned bridge, but there doesn't seem to be a corresponding drm_bridge_pu= t() on either the success or error paths to release it. > + if (ret) { > + dev_err_probe(dev, ret, "Failed to attach next bridge\n"); > + goto unregister_aux; > + } > + > dw_dp_init_hw(dp); > =20 [Severity: High] This is a pre-existing issue, but does registering the AUX channel early expose uninitialized hardware to userspace? Looking earlier in dw_dp_bind(): ret =3D drm_dp_aux_register(&dp->aux); if (ret) { dev_err_probe(dev, ret, "Aux register failed\n"); return ERR_PTR(ret); } And then later: dw_dp_init_hw(dp); ret =3D phy_init(dp->phy); If a userspace process opens the AUX character device and initiates a transaction before dw_dp_init_hw() and the IRQ setup are called, the hardwa= re may enter an invalid state and miss completion interrupts. > ret =3D phy_init(dp->phy); [Severity: High] This is a pre-existing issue, but can the lack of completion reinitializati= on cause race conditions? Looking at dw_dp_aux_transfer() in the same file: regmap_write(dp->regmap, DW_DP_AUX_CMD, value); status =3D wait_for_completion_timeout(&dp->complete, timeout); If a previous AUX transfer timed out but the hardware eventually triggered a late interrupt, the completion object will remain in a done state. The next AUX transfer will see this stale state, causing wait_for_completion_timeout= () to return immediately before the new transfer has actually completed. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260612-synopsys-d= w-dp-improvements-v3-0-dc61e6352508@collabora.com?part=3D3