Re: [PATCH v5 05/20] drm/rockchip: analogix_dp: Replace DRM_...() functions with drm_...() or dev_...()

On Fri, Jan 24, 2025 at 05:41:58PM +0800, Andy Yan wrote:

Hi,

At 2025-01-23 20:27:50, "Jani Nikula" [off-list ref] wrote:

quoted

On Wed, 22 Jan 2025, Damon Ding [off-list ref] wrote:

quoted

Hi Andy,

On 2025/1/9 14:28, Andy Yan wrote:

quoted

Hi Damon,

At 2025-01-09 11:27:10, "Damon Ding" [off-list ref] wrote:

quoted

According to the comments in include/drm/drm_print.h, the DRM_...()
functions are deprecated in favor of drm_...() or dev_...() functions.

Use drm_err()/drm_dbg_core()/drm_dbg_kms() instead of
DRM_DEV_ERROR()/DRM_ERROR()/DRM_DEV_DEBUG()/DRM_DEBUG_KMS() after
rockchip_dp_bind() is called, and replace DRM_DEV_ERROR() with dev_err()
before calling it.

Signed-off-by: Damon Ding <redacted>
---
.../gpu/drm/rockchip/analogix_dp-rockchip.c   | 29 ++++++++++---------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 546d13f19f9b..8114c3238609 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c

@@ -100,13 +100,13 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)

	ret = clk_prepare_enable(dp->pclk);
	if (ret < 0) {
-		DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret);
+		drm_err(dp->drm_dev, "failed to enable pclk %d\n", ret);

                You just need to pass dp here:
                 drm_err(dp, "failed to enable pclk %d\n", ret);

I see. It is really better to pass dp instead of dp->drm_dev. I will 
update all relevant logs in the next version.

No, this was bad review feedback. You're absolutely expected to pass
struct drm_device to drm_err() and friends.

No, I didn't see how the drm_err macro and friends restrict/expect the passing of only the drm_device pointer.
As for the platform device driver itself,  we hope that when an error occurs, the log clearly indicates
the specific device that the log corresponds to, rather than a generic drm_device.
The original code of this driver used the macro related to DRM_DEV_ERROR to do just that.
And similar patches have been merged before

Please also refer to the discussion here.[0]
[0]https://lore.kernel.org/linux-rockchip/20250109032725.1102465-1-damon.ding@rock-chips.com/T/#m54bd842be660031773834cedea6c73a5033ca973 (local)

Reviewer's mistake (mine). I assumed that you wanted to pass drm device.

-- 
With best wishes
Dmitry