<div dir="ltr">Hi,<div>Thanks a lot for your feedback.</div><div>I will send another version for this patch.</div><div>Thanks for your time.</div><div><br></div><div>Regards,</div><div>Harsha Sharma</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Sep 24, 2017 at 10:30 PM, Julia Lawall <span dir="ltr"><<a href="mailto:julia.lawall@lip6.fr" target="_blank">julia.lawall@lip6.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
<br>
On Sun, 24 Sep 2017, Harsha Sharma wrote:<br>
<br>
> Replace all occurences of dev_info/err/dbg with DRM_DEV_INFO/<br>
> ERROR/DEBUG as we have DRM_DEV_* variants of drm print macros<br>
> Done using following coccinelle semantic patch<br>
><br>
> @r@<br>
> @@<br>
><br>
> (<br>
> -dev_info<br>
> +DRM_DEV_INFO<br>
> |<br>
> -dev_err<br>
> +DRM_DEV_ERROR<br>
> |<br>
> -dev_dbg<br>
> +DRM_DEV_DEBUG<br>
> )<br>
><br>
> Signed-off-by: Harsha Sharma <<a href="mailto:harshasharmaiitr@gmail.com">harshasharmaiitr@gmail.com</a>><br>
> ---<br>
> Changes in v2:<br>
>  -Break line over 80 characters<br>
>  -Changes in comments not required<br>
><br>
>  drivers/gpu/drm/tegra/dc.c     |  53 +++++++-----<br>
>  drivers/gpu/drm/tegra/dpaux.c  |  24 +++---<br>
>  drivers/gpu/drm/tegra/dsi.c    |  68 ++++++++-------<br>
>  drivers/gpu/drm/tegra/falcon.c |  16 ++--<br>
>  drivers/gpu/drm/tegra/fb.c     |  22 +++--<br>
>  drivers/gpu/drm/tegra/gem.c    |   8 +-<br>
>  drivers/gpu/drm/tegra/gr2d.c   |  10 ++-<br>
>  drivers/gpu/drm/tegra/gr3d.c   |  20 +++--<br>
>  drivers/gpu/drm/tegra/hdmi.c   |  66 +++++++++------<br>
>  drivers/gpu/drm/tegra/output.c |   8 +-<br>
>  drivers/gpu/drm/tegra/rgb.c    |  12 +--<br>
>  drivers/gpu/drm/tegra/sor.c    | 184 +++++++++++++++++++++++++-----<wbr>-----------<br>
>  drivers/gpu/drm/tegra/vic.c    |  15 ++--<br>
>  13 files changed, 304 insertions(+), 202 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c<br>
> index 4df3911..fbc9cc1 100644<br>
> --- a/drivers/gpu/drm/tegra/dc.c<br>
> +++ b/drivers/gpu/drm/tegra/dc.c<br>
> @@ -1137,7 +1137,7 @@ static void tegra_dc_commit_state(struct tegra_dc *dc,<br>
><br>
>       err = clk_set_parent(dc->clk, state->clk);<br>
>       if (err < 0)<br>
> -             dev_err(dc->dev, "failed to set parent clock: %d\n", err);<br>
> +             DRM_DEV_ERROR(dc->dev, "failed to set parent clock: %d\n", err);<br>
><br>
>       /*<br>
>        * Outputs may not want to change the parent clock rate. This is only<br>
> @@ -1150,7 +1150,7 @@ static void tegra_dc_commit_state(struct tegra_dc *dc,<br>
>       if (state->pclk > 0) {<br>
>               err = clk_set_rate(state->clk, state->pclk);<br>
>               if (err < 0)<br>
> -                     dev_err(dc->dev,<br>
> +                     DRM_DEV_ERROR(dc->dev,<br>
>                               "failed to set clock rate to %lu Hz\n",<br>
>                               state->pclk);<br>
>       }<br>
> @@ -1195,7 +1195,7 @@ static int tegra_dc_wait_idle(struct tegra_dc *dc, unsigned long timeout)<br>
>               usleep_range(1000, 2000);<br>
>       }<br>
><br>
> -     dev_dbg(dc->dev, "timeout waiting for DC to become idle\n");<br>
> +     DRM_DEV_DEBUG(dc->dev, "timeout waiting for DC to become idle\n");<br>
>       return -ETIMEDOUT;<br>
>  }<br>
><br>
> @@ -1763,7 +1763,8 @@ static int tegra_dc_init(struct host1x_client *client)<br>
>       if (tegra->domain) {<br>
>               err = iommu_attach_device(tegra-><wbr>domain, dc->dev);<br>
>               if (err < 0) {<br>
> -                     dev_err(dc->dev, "failed to attach to domain: %d\n",<br>
> +                     DRM_DEV_ERROR(dc->dev,<br>
> +                             "failed to attach to domain: %d\n",<br>
>                               err);<br>
>                       return err;<br>
>               }<br>
> @@ -1801,7 +1802,8 @@ static int tegra_dc_init(struct host1x_client *client)<br>
><br>
>       err = tegra_dc_rgb_init(drm, dc);<br>
>       if (err < 0 && err != -ENODEV) {<br>
> -             dev_err(dc->dev, "failed to initialize RGB output: %d\n", err);<br>
> +             DRM_DEV_ERROR(dc->dev,<br>
> +                     "failed to initialize RGB output: %d\n", err);<br>
>               goto cleanup;<br>
>       }<br>
><br>
> @@ -1812,13 +1814,15 @@ static int tegra_dc_init(struct host1x_client *client)<br>
>       if (IS_ENABLED(CONFIG_DEBUG_FS)) {<br>
>               err = tegra_dc_debugfs_init(dc, drm->primary);<br>
>               if (err < 0)<br>
> -                     dev_err(dc->dev, "debugfs setup failed: %d\n", err);<br>
> +                     DRM_DEV_ERROR(dc->dev,<br>
> +                             "debugfs setup failed: %d\n", err);<br>
<br>
</div></div>The string could be on the same line as the function name.  Just the err<br>
could be moved to the next line, and lined up with the right side of the (.<br>
<br>
Overall, looking through the rest, it would probably really be nicer to<br>
line the extra arguments up with the right side of the (.<br>
<span class=""><br>
<br>
>       }<br>
><br>
>       err = devm_request_irq(dc->dev, dc->irq, tegra_dc_irq, 0,<br>
>                              dev_name(dc->dev), dc);<br>
>       if (err < 0) {<br>
> -             dev_err(dc->dev, "failed to request IRQ#%u: %d\n", dc->irq,<br>
> +             DRM_DEV_ERROR(dc->dev,<br>
> +                     "failed to request IRQ#%u: %d\n", dc->irq,<br>
<br>
</span>Same here.  Try to keep the string on the first line.<br>
<span class=""><br>
>                       err);<br>
>               goto cleanup;<br>
>       }<br>
> @@ -1850,12 +1854,14 @@ static int tegra_dc_exit(struct host1x_client *client)<br>
>       if (IS_ENABLED(CONFIG_DEBUG_FS)) {<br>
>               err = tegra_dc_debugfs_exit(dc);<br>
>               if (err < 0)<br>
> -                     dev_err(dc->dev, "debugfs cleanup failed: %d\n", err);<br>
> +                     DRM_DEV_ERROR(dc->dev,<br>
> +                             "debugfs cleanup failed: %d\n", err);<br>
<br>
</span>Same here.  And so on.<br>
<br>
[...]<br>
<span class=""><br>
> diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c<br>
> index 046649e..f9e4ad3 100644<br>
> --- a/drivers/gpu/drm/tegra/dsi.c<br>
> +++ b/drivers/gpu/drm/tegra/dsi.c<br>
> @@ -854,7 +854,8 @@ static void tegra_dsi_unprepare(struct tegra_dsi *dsi)<br>
><br>
>       err = tegra_mipi_disable(dsi->mipi);<br>
>       if (err < 0)<br>
> -             dev_err(dsi->dev, "failed to disable MIPI calibration: %d\n",<br>
> +             DRM_DEV_ERROR(dsi->dev,<br>
> +             "failed to disable MIPI calibration: %d\n",<br>
<br>
</span>Here the string is much too far to the left.<br>
<span class="HOEnZb"><font color="#888888"><br>
julia<br>
</font></span></blockquote></div><br></div>