[PATCH] drm/bridge: nxp-ptn3460: replace deprecated DRM_ERROR macros

[PATCH] drm/bridge: nxp-ptn3460: replace deprecated DRM_ERROR macros

[Piyush Patle]

Replace DRM_ERROR() with device-aware equivalents throughout:

- In bridge callbacks (pre_enable, edid_read, attach), use
  drm_err(bridge->dev, ...).
- In i2c helpers (read_bytes, write_byte, select_edid), use
  dev_err(&ptn_bridge->client->dev, ...).
- In probe context, use dev_err(dev, ...), consistent with
  existing error paths in the same function.

The kmalloc() failure message in ptn3460_edid_read() is dropped
as the allocator already reports OOM conditions.

No functional changes.

Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
 drivers/gpu/drm/bridge/nxp-ptn3460.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index 7acb11f16dc1..6f09c1247d7d 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -54,13 +54,15 @@ static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr,
 
 	ret = i2c_master_send(ptn_bridge->client, &addr, 1);
 	if (ret < 0) {
-		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
+		dev_err(&ptn_bridge->client->dev,
+			"Failed to send i2c command, ret=%d\n", ret);
 		return ret;
 	}
 
 	ret = i2c_master_recv(ptn_bridge->client, buf, len);
 	if (ret < 0) {
-		DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret);
+		dev_err(&ptn_bridge->client->dev,
+			"Failed to recv i2c data, ret=%d\n", ret);
 		return ret;
 	}
 
@@ -78,7 +80,8 @@ static int ptn3460_write_byte(struct ptn3460_bridge *ptn_bridge, char addr,
 
 	ret = i2c_master_send(ptn_bridge->client, buf, ARRAY_SIZE(buf));
 	if (ret < 0) {
-		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
+		dev_err(&ptn_bridge->client->dev,
+			"Failed to send i2c command, ret=%d\n", ret);
 		return ret;
 	}
 
@@ -94,7 +97,8 @@ static int ptn3460_select_edid(struct ptn3460_bridge *ptn_bridge)
 	ret = ptn3460_write_byte(ptn_bridge, PTN3460_EDID_SRAM_LOAD_ADDR,
 			ptn_bridge->edid_emulation);
 	if (ret) {
-		DRM_ERROR("Failed to transfer EDID to sram, ret=%d\n", ret);
+		dev_err(&ptn_bridge->client->dev,
+			"Failed to transfer EDID to sram, ret=%d\n", ret);
 		return ret;
 	}
 
@@ -104,7 +108,8 @@ static int ptn3460_select_edid(struct ptn3460_bridge *ptn_bridge)
 
 	ret = ptn3460_write_byte(ptn_bridge, PTN3460_EDID_EMULATION_ADDR, val);
 	if (ret) {
-		DRM_ERROR("Failed to write EDID value, ret=%d\n", ret);
+		dev_err(&ptn_bridge->client->dev,
+			"Failed to write EDID value, ret=%d\n", ret);
 		return ret;
 	}
 
@@ -134,7 +139,7 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
 
 	ret = ptn3460_select_edid(ptn_bridge);
 	if (ret)
-		DRM_ERROR("Select EDID failed ret=%d\n", ret);
+		drm_err(bridge->dev, "Select EDID failed ret=%d\n", ret);
 
 	ptn_bridge->enabled = true;
 }
@@ -166,10 +171,8 @@ static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge,
 	ptn3460_pre_enable(&ptn_bridge->bridge);
 
 	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
-	if (!edid) {
-		DRM_ERROR("Failed to allocate EDID\n");
+	if (!edid)
 		goto out;
-	}
 
 	ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid,
 				 EDID_LENGTH);
@@ -233,7 +236,7 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
 	ret = drm_connector_init(bridge->dev, &ptn_bridge->connector,
 			&ptn3460_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
 	if (ret) {
-		DRM_ERROR("Failed to initialize connector with drm\n");
+		drm_err(bridge->dev, "Failed to initialize connector with drm\n");
 		return ret;
 	}
 	drm_connector_helper_add(&ptn_bridge->connector,
@@ -289,7 +292,7 @@ static int ptn3460_probe(struct i2c_client *client)
 						GPIOD_OUT_LOW);
 	if (IS_ERR(ptn_bridge->gpio_rst_n)) {
 		ret = PTR_ERR(ptn_bridge->gpio_rst_n);
-		DRM_ERROR("cannot get gpio_rst_n %d\n", ret);
+		dev_err(dev, "cannot get gpio_rst_n %d\n", ret);
 		return ret;
 	}
 
-- 
2.43.0

Re: [PATCH] drm/bridge: nxp-ptn3460: replace deprecated DRM_ERROR macros

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak of the EDID buffer in `ptn3460_edid_read()` on successful reads.
- [High] Race condition in bridge power state tracking (`ptn_bridge->enabled`).
- [High] Improper error handling of short I2C transfers leads to kernel memory disclosure.
--

commit 4e9f35dc1b32e95bb7e2572ee3b691a3841b8e02
Author: Piyush Patle <piyushpatle228@gmail.com>

drm/bridge: nxp-ptn3460: replace deprecated DRM_ERROR macros
    
This commit replaces DRM_ERROR() with device-aware equivalents like
drm_err() and dev_err() throughout the nxp-ptn3460 bridge driver.

> diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
> index 7acb11f16dc19..6f09c1247d7d8 100644
> --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
> +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
> @@ -54,13 +54,15 @@ static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr,
>  
>  	ret = i2c_master_send(ptn_bridge->client, &addr, 1);
>  	if (ret < 0) {
> -		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
> +		dev_err(&ptn_bridge->client->dev,
> +			"Failed to send i2c command, ret=%d\n", ret);
>  		return ret;
>  	}
>  
>  	ret = i2c_master_recv(ptn_bridge->client, buf, len);
>  	if (ret < 0) {
> -		DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret);
> +		dev_err(&ptn_bridge->client->dev,
> +			"Failed to recv i2c data, ret=%d\n", ret);
>  		return ret;
>  	}

[Severity: High]
This is a pre-existing issue, but does this properly handle short I2C
transfers?

The i2c_master_recv() function returns the positive number of bytes
transferred on success. If a short read occurs (e.g. the hardware NACKs
mid-transfer), the return value is positive but less than len.

Because the check is only for ret < 0, a short read is incorrectly treated
as a full success, which leaves the target buffer partially uninitialized.

In ptn3460_edid_read(), this uninitialized memory is then copied by
drm_edid_alloc() and exposed to userspace, which could leak kernel memory.

>  
> @@ -78,7 +80,8 @@ static int ptn3460_write_byte(struct ptn3460_bridge *ptn_bridge, char addr,
>  
>  	ret = i2c_master_send(ptn_bridge->client, buf, ARRAY_SIZE(buf));
>  	if (ret < 0) {
> -		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
> +		dev_err(&ptn_bridge->client->dev,
> +			"Failed to send i2c command, ret=%d\n", ret);
>  		return ret;
>  	}

[Severity: High]
This is a pre-existing issue, but does this similarly miss short writes?

Like i2c_master_recv(), i2c_master_send() returns the number of bytes
transferred. If this is a short write, ret will be positive but less than
the requested size.

Ignoring a short write could cause silent hardware configuration failures.

[ ... ]

> @@ -166,10 +171,8 @@ static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge,
>  	ptn3460_pre_enable(&ptn_bridge->bridge);

[Severity: High]
This is a pre-existing issue, but is the bridge power state tracking
subject to a race condition here?

In ptn3460_edid_read(), ptn_bridge->enabled is read and then updated
by ptn3460_pre_enable() without locking.

Because ptn3460_edid_read() runs under connector->mutex while the atomic
commit tail runs bridge callbacks asynchronously, a concurrent atomic modeset
could disable the bridge while ptn3460_edid_read() is actively performing I2C
transactions.

Could this lead to hardware lockups and I2C bus hangs?

>  
>  	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
> -	if (!edid) {
> -		DRM_ERROR("Failed to allocate EDID\n");
> +	if (!edid)
>  		goto out;
> -	}
>  
>  	ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid,
>  				 EDID_LENGTH);

[Severity: High]
This is a pre-existing issue, but does this function leak the edid buffer
on the success path?

Looking at ptn3460_edid_read() after the I2C read succeeds:

	drm_edid = drm_edid_alloc(edid, EDID_LENGTH);

out:
	if (power_off)
		ptn3460_disable(&ptn_bridge->bridge);

	return drm_edid;

The drm_edid_alloc() function uses kmemdup() to create its own copy of
the memory. Since there is no kfree(edid) before returning on success,
the original edid buffer appears to be leaked.

Since userspace can repeatedly trigger EDID reads, could this lead to
an unbounded memory leak and kernel memory exhaustion?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606195316.28048-1-piyushpatle228@gmail.com?part=1