On Fri, Jan 24, 2025 at 05:41:58PM +0800, Andy Yan wrote:
> Hi,
>
> At 2025-01-23 20:27:50, "Jani Nikula" <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> >On Wed, 22 Jan 2025, Damon Ding <damon.ding@xxxxxxxxxxxxxx> wrote:
> >> Hi Andy,
> >>
> >> On 2025/1/9 14:28, Andy Yan wrote:
> >>>
> >>> Hi Damon,
> >>>
> >>> At 2025-01-09 11:27:10, "Damon Ding" <damon.ding@xxxxxxxxxxxxxx> wrote:
> >>>> 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 <damon.ding@xxxxxxxxxxxxxx>
> >>>> ---
> >>>> .../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@xxxxxxxxxxxxxx/T/#m54bd842be660031773834cedea6c73a5033ca973
Reviewer's mistake (mine). I assumed that you wanted to pass drm device.
>
>
--
With best wishes
Dmitry
