[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1df2b47e-74d1-4232-957a-21d7a6f5da1d@gmail.com>
Date: Tue, 12 Aug 2025 00:08:57 +0300
From: Eslam Khafagy <eslam.medhat1993@...il.com>
To: Manikandan.M@...rochip.com
Cc: skhan@...uxfoundation.com, Dharma.B@...rochip.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, simona@...ll.ch, Nicolas.Ferre@...rochip.com,
alexandre.belloni@...tlin.com, claudiu.beznea@...on.dev,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] drm: atmel-hlcdc: replace dev_* print funtions with
drm_* variants
On 8/11/25 11:54, Manikandan.M@...rochip.com wrote:
> Hi Eslam Khafagy,
Hi Manikandan,
>
> The patch appears to have been sent with spaces in place of
> tabs in the diff, which breaks kernel coding style [1] and makes it fail
> to apply.
> Please ensure your editor is configured to use tabs for indentation and
> use `git send-email` or b4 directly, not via an email client that may
> alter whitespace.
Thank you for the feedback.
But i have rechecked the patch. I don't seem to have altered any tabs
with spaces.
>
> Could you please run checkpatch.sh and fix any reported errors or
> warnings before submitting the patch?
checkpatch.sh only reports two warnings:
WARNING: 'funtions' may be misspelled - perhaps 'functions'?
#78:
Subject: [PATCH v3] drm: atmel-hlcdc: replace dev_* print funtions with drm_* variants
^^^^^^^^
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#95:
https://docs.kernel.org/gpu/todo.html#convert-logging-to-drm-functions-with-drm-device-parameter
but no errors regarding tabs or spaces.
I'll fix the first one in v4. can you confirm indeed i have not altered tabs with spaces first?
>
> [1] Documentation/process/coding-style.rst
>
> On 08/08/25 12:33 am, Eslam Khafagy wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Update the Atmel HLCDC code to use DRM print macros drm_*() instead of
>> dev_warn() and dev_err(). This change ensures consistency with DRM
>> subsystem logging conventions [1].
>>
>> [1]
>> https://docs.kernel.org/gpu/todo.html#convert-logging-to-drm-functions-with-drm-device-parameter
>>
>> Signed-off-by: Eslam Khafagy <eslam.medhat1993@...il.com>
>> ---
>> v3:
>> * include header file to fix compilation. Thanks Manikandan.
>>
>> v2:
>> * replace dev_dbg with drm_dbg in atmel_hlcdc_plane.c
>> * https://lore.kernel.org/all/20250806183049.52112-2-eslam.medhat1993@gmail.com/
>>
>> v1:
>> * replace dev_* print funtions with drm_* variants
>> *https://lore.kernel.org/all/20250806000141.239753-2-eslam.medhat1993@gmail.com/
>>
>> .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 21 ++++++++++---------
>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 14 ++++++-------
>> .../gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 3 ++-
>> .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 6 +++---
>> 4 files changed, 23 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> index 0f7ffb3ced20..e0efc7309b1b 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> @@ -20,6 +20,7 @@
>> #include <drm/drm_atomic_helper.h>
>> #include <drm/drm_crtc.h>
>> #include <drm/drm_modeset_helper_vtables.h>
>> +#include <drm/drm_print.h>
>> #include <drm/drm_probe_helper.h>
>> #include <drm/drm_vblank.h>
>>
>> @@ -215,32 +216,32 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> !(status & ATMEL_XLCDC_CM),
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register CMSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register CMSTS timeout\n");
>>
>> regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> status & ATMEL_XLCDC_SD,
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register SDSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register SDSTS timeout\n");
>> }
>>
>> regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> !(status & ATMEL_HLCDC_DISP),
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register DISPSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register DISPSTS timeout\n");
>>
>> regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_SYNC);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> !(status & ATMEL_HLCDC_SYNC),
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register LCDSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register LCDSTS timeout\n");
>>
>> regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_PIXEL_CLK);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> !(status & ATMEL_HLCDC_PIXEL_CLK),
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register CLKSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register CLKSTS timeout\n");
>>
>> clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
>> pinctrl_pm_select_sleep_state(dev->dev);
>> @@ -269,32 +270,32 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> status & ATMEL_HLCDC_PIXEL_CLK,
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register CLKSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register CLKSTS timeout\n");
>>
>> regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_SYNC);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> status & ATMEL_HLCDC_SYNC,
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register LCDSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register LCDSTS timeout\n");
>>
>> regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_DISP);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> status & ATMEL_HLCDC_DISP,
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register DISPSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register DISPSTS timeout\n");
>>
>> if (crtc->dc->desc->is_xlcdc) {
>> regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> status & ATMEL_XLCDC_CM,
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register CMSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register CMSTS timeout\n");
>>
>> regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
>> if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> !(status & ATMEL_XLCDC_SD),
>> 10, 1000))
>> - dev_warn(dev->dev, "Atmel LCDC status register SDSTS timeout\n");
>> + drm_warn(dev, "Atmel LCDC status register SDSTS timeout\n");
>> }
>>
>> pm_runtime_put_sync(dev->dev);
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index fa8ad94e431a..acb017a2486b 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -724,19 +724,19 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev)
>>
>> ret = atmel_hlcdc_create_outputs(dev);
>> if (ret) {
>> - dev_err(dev->dev, "failed to create HLCDC outputs: %d\n", ret);
>> + drm_err(dev, "failed to create HLCDC outputs: %d\n", ret);
>> return ret;
>> }
>>
>> ret = atmel_hlcdc_create_planes(dev);
>> if (ret) {
>> - dev_err(dev->dev, "failed to create planes: %d\n", ret);
>> + drm_err(dev, "failed to create planes: %d\n", ret);
>> return ret;
>> }
>>
>> ret = atmel_hlcdc_crtc_create(dev);
>> if (ret) {
>> - dev_err(dev->dev, "failed to create crtc\n");
>> + drm_err(dev, "failed to create crtc\n");
>> return ret;
>> }
>>
>> @@ -778,7 +778,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>>
>> ret = clk_prepare_enable(dc->hlcdc->periph_clk);
>> if (ret) {
>> - dev_err(dev->dev, "failed to enable periph_clk\n");
>> + drm_err(dev, "failed to enable periph_clk\n");
>> return ret;
>> }
>>
>> @@ -786,13 +786,13 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>>
>> ret = drm_vblank_init(dev, 1);
>> if (ret < 0) {
>> - dev_err(dev->dev, "failed to initialize vblank\n");
>> + drm_err(dev, "failed to initialize vblank\n");
>> goto err_periph_clk_disable;
>> }
>>
>> ret = atmel_hlcdc_dc_modeset_init(dev);
>> if (ret < 0) {
>> - dev_err(dev->dev, "failed to initialize mode setting\n");
>> + drm_err(dev, "failed to initialize mode setting\n");
>> goto err_periph_clk_disable;
>> }
>>
>> @@ -802,7 +802,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>> ret = atmel_hlcdc_dc_irq_install(dev, dc->hlcdc->irq);
>> pm_runtime_put_sync(dev->dev);
>> if (ret < 0) {
>> - dev_err(dev->dev, "failed to install IRQ handler\n");
>> + drm_err(dev, "failed to install IRQ handler\n");
>> goto err_periph_clk_disable;
>> }
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
>> index 50fee6a93964..0b8a86afb096 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
>> @@ -15,6 +15,7 @@
>> #include <drm/drm_bridge.h>
>> #include <drm/drm_encoder.h>
>> #include <drm/drm_of.h>
>> +#include <drm/drm_print.h>
>> #include <drm/drm_simple_kms_helper.h>
>>
>> #include "atmel_hlcdc_dc.h"
>> @@ -92,7 +93,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
>> output->bus_fmt = atmel_hlcdc_of_bus_fmt(ep);
>> of_node_put(ep);
>> if (output->bus_fmt < 0) {
>> - dev_err(dev->dev, "endpoint %d: invalid bus width\n", endpoint);
>> + drm_err(dev, "endpoint %d: invalid bus width\n", endpoint);
>> return -EINVAL;
>> }
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> index 4a7ba0918eca..817284509b57 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> @@ -1034,7 +1034,7 @@ static void atmel_hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>> if (isr &
>> (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
>> - dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>> + drm_dbg(plane->base.dev, "overrun on plane %s\n",
>> desc->name);
>> }
>>
>> @@ -1051,7 +1051,7 @@ static void atmel_xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>> if (isr &
>> (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>> ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
>> - dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>> + drm_dbg(plane->base.dev, "overrun on plane %s\n",
>> desc->name);
>> }
>>
>> @@ -1140,7 +1140,7 @@ static void atmel_hlcdc_plane_reset(struct drm_plane *p)
>> if (state) {
>> if (atmel_hlcdc_plane_alloc_dscrs(p, state)) {
>> kfree(state);
>> - dev_err(p->dev->dev,
>> + drm_err(p->dev,
>> "Failed to allocate initial plane state\n");
>> return;
>> }
>> --
>> 2.43.0
>>
Powered by blists - more mailing lists