Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
From:   Ying Liu <victor.liu@nxp.com>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        "andrzej.hajda@intel.com" <andrzej.hajda@intel.com>
CC:     "neil.armstrong@linaro.org" <neil.armstrong@linaro.org>,
        "rfoss@kernel.org" <rfoss@kernel.org>,
        "Laurent.pinchart@ideasonboard.com" 
        <Laurent.pinchart@ideasonboard.com>,
        "jonas@kwiboo.se" <jonas@kwiboo.se>,
        "jernej.skrabec@gmail.com" <jernej.skrabec@gmail.com>,
        "airlied@gmail.com" <airlied@gmail.com>,
        "daniel@ffwll.ch" <daniel@ffwll.ch>,
        "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kernel@collabora.com" <kernel@collabora.com>,
        "ehristev@collabora.com" <ehristev@collabora.com>,
        "wenst@chromium.org" <wenst@chromium.org>
Subject: RE: [PATCH] drm/bridge: panel: Fix device link for
 DRM_BRIDGE_ATTACH_NO_CONNECTOR
Thread-Topic: [PATCH] drm/bridge: panel: Fix device link for
 DRM_BRIDGE_ATTACH_NO_CONNECTOR
Thread-Index: AQHZ6kDy3sEuq/n38UORlSaHmuHxHbAhX96Q
Date:   Tue, 19 Sep 2023 01:45:45 +0000
Message-ID: <AM7PR04MB70466D88048831E48EBBBD0898FAA@AM7PR04MB7046.eurprd04.prod.outlook.com>
References: <20230918150043.403250-1-angelogioacchino.delregno@collabora.com>
In-Reply-To: <20230918150043.403250-1-angelogioacchino.delregno@collabora.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?pl/wtKNPmHKfTXoUSXxuoivww5IOLD/FQBk9XHK4wktvCRkHHddUKxq8lMMl?=
 =?us-ascii?Q?1fzZ/gyikXPMOFXuNscd3nfsyJsVCO39yarrmHUF7ZgY/K71ab52qqJ+H5CA?=
 =?us-ascii?Q?FuUXQgzumsQW7MMnMutjAab3S2hV29UVNrZ8eSVu7q1WV19tZq2VfxBBGQTz?=
 =?us-ascii?Q?jBJSFHZmIPUVu3Tz1XepfCgV4OQS1DlXNL2RSGUbBk1Fmx32JiSqFqYFUj7G?=
 =?us-ascii?Q?YJ2g2ECuXnzRPp55Ju8v3hcn3PSjZi0AMAzWH5+dm7yY+cZqkMdGlM929Y9p?=
 =?us-ascii?Q?uI2LX5YtduLvQjD48F6gg4wxBAWUukAzgLBSY2azXs68NF8ieGC+5vWNuOop?=
 =?us-ascii?Q?Ah+FloTAbPhPmj9IFMqNHkHCnAChuOkCIHZni/jU0EGBNgMWEubTtEV1reTo?=
 =?us-ascii?Q?m9eBug3JRbluvErndNkOrVXq+wkPB6NdnNZ6PwSp7/Uzs6QNkqcnc7bIuj/Y?=
 =?us-ascii?Q?k7rhGJbQnOwPJovraNHLEEUvGADfub1D+XxgQ/7uoAzjcZ+qQRyNUleHfW62?=
 =?us-ascii?Q?QtTc8axqWijtYr2/sw3pkENn4Q7I/kV/CghnWHlo8JpTeG5neiyxjTuQJ0lD?=
 =?us-ascii?Q?ezHeqZGMXEeXIBaVCncQhs8Wv9LWVGX4IYmpcaQmZALrdiQTIYQscsnRfxyV?=
 =?us-ascii?Q?fWjix3iLvVTi5/5M5b0o64+naFuBqDhUIrMEp900v2R1/WxO4we17iQP80aW?=
 =?us-ascii?Q?swhdRhg+Wa8OrbEB7WipvWvZkbce24i2pBvpYP5y/s6Oj6sWEUg+rWMaOJvO?=
 =?us-ascii?Q?aqk7zgIRc+Kr5+TXigAIqFZCD1cQ/hA/MkjmPq0ZlE/rKRxZX65ASSggxdsA?=
 =?us-ascii?Q?998tvHxO7jkPXipGoH5lfuJAwwdpuS6a4BQZXo1FVxzHI8ZvDTIaQuTuB916?=
 =?us-ascii?Q?Fy/4xlaq5OTZ+ENnAwE0yuSkymikjuw1lqNUE3uU3IOqARDWC7dvoSyOSBiO?=
 =?us-ascii?Q?5/a/ns+5P0D+pHAyVPitj2AR19NLr3ArWi0NflHdUAvrzNUiitWdccws3Ev2?=
 =?us-ascii?Q?3iXPcSrPMhiQUOBTksHepQ3cJliIOe59UnxsHJgzMDxY8IzGe/EGEurdNE74?=
 =?us-ascii?Q?vQjle8Hrfsfo77l2mIbQU+ZLRqtkIZZpcwSne+iwO/AGLz7JE0yGNm+Ges4H?=
 =?us-ascii?Q?i3Me2V+Z4i2YjuCBKI5ytpJ2+y/Tl734vXK0Ld4sPVLP7OPq0TYi4/Jr+Alp?=
 =?us-ascii?Q?0Fy4/LA8K6hGkqoJb7EL/+WTGfbjH3VrFsnxL8EsAQpsKHiwUruG5X5x28Kf?=
 =?us-ascii?Q?KR7sjRAfgg8LThBnJwUBl2O2Wr2CfyWbf6ePGXOx6n+nAAS5vbBY6+re5vkj?=
 =?us-ascii?Q?WV8AD4CokhAADIEttvmDWG8mzFG2L+rlIvos7Sv+D4UqJb+lDRSmMfkQFQte?=
 =?us-ascii?Q?djTR0BUSbjWASm1wfJeIUCCwFzDx+Td4DXaog+izdvVQybLbcpW/5DZaPhvD?=
 =?us-ascii?Q?fcMwWHlFOYeyTGm2rBJir1GZ0z+OUXIDNEmx0tGeW4Vzt8cTp+i0nnTSIwsQ?=
 =?us-ascii?Q?pGcaPFNlyQHbps5kP+LZVGp6V7MFckfMHllYfxTq3y0CJeSnIue0hphHRe0q?=
 =?us-ascii?Q?GBDYScQT2vnrDxZIld0=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7046.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8bd2893f-9e0a-4997-28e7-08dbb8b22449
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Sep 2023 01:45:45.5401
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: SGD8U5tanqi2WTooVL8xKuhYAPd4gUaEf/eUVpxhfBvDNuDI2dwcq4x83e+YU+IKOL4zHFaUBcqNcAoSO0fQXg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8265
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Mon, 18 Sep 2023 18:45:56 -0700 (PDT)

Hi Angelo,

Thank you for the patch.

On Monday, September 18, 2023 11:01 PM, AngeloGioacchino Del Regno <angelog=
ioacchino.delregno@collabora.com> wrote:
> When external bridges are attached with
> DRM_BRIDGE_ATTACH_NO_CONNECTOR,
> the panel bridge may also get the same flag, but in the .attach()
> callback for the panel bridge a device link is added only when this
> flag is not present; To make things worse, the .detach() callback
> tries to delete the device link unconditionally and without checking
> if it was created in the first place, crashing the kernel with a NULL
> pointer kernel panic upon calling panel_bridge_detach().
>=20
> Fix that by moving the device_link_add() call before checking if the
> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is present.
>=20
> Fixes: 199cf07ebd2b ("drm/bridge: panel: Add a device link between drm
> device and panel device")
> Signed-off-by: AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/bridge/panel.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>=20
> diff --git a/drivers/gpu/drm/bridge/panel.c
> b/drivers/gpu/drm/bridge/panel.c
> index e00d2e94c751..8c507dfd589e 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -67,14 +67,6 @@ static int panel_bridge_attach(struct drm_bridge
> *bridge,
>  	struct drm_device *drm_dev =3D bridge->dev;
>  	int ret;
>=20
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> -		return 0;
> -
> -	if (!bridge->encoder) {
> -		DRM_ERROR("Missing encoder\n");
> -		return -ENODEV;
> -	}
> -
>  	panel_bridge->link =3D device_link_add(drm_dev->dev, panel->dev,
>  					     DL_FLAG_STATELESS);
>  	if (!panel_bridge->link) {
> @@ -83,6 +75,14 @@ static int panel_bridge_attach(struct drm_bridge
> *bridge,
>  		return -EINVAL;
>  	}
>=20
> +	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> +		return 0;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");

Shouldn't the device link be deleted in case of error?

Regards,
Liu Ying

> +		return -ENODEV;
> +	}
> +
>  	drm_connector_helper_add(connector,
>  				 &panel_bridge_connector_helper_funcs);
>=20
> --
> 2.42.0