public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] accel/amdxdna: fix missing newline in pr_err message
@ 2026-03-20 11:29 Haoyu Lu
  2026-03-20 15:54 ` Lizhi Hou
  0 siblings, 1 reply; 6+ messages in thread
From: Haoyu Lu @ 2026-03-20 11:29 UTC (permalink / raw)
  To: Min Ma, Lizhi Hou, Oded Gabbay; +Cc: dri-devel, linux-kernel, haoyu.lu

From: "haoyu.lu" <hechushiguitu666@gmail.com>

Add missing newline to pr_err message in amdxdna_mailbox.c.

Signed-off-by: haoyu.lu <hechushiguitu666@gmail.com>
---
 drivers/accel/amdxdna/amdxdna_mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c
index 46d844a73a94..e681a090752d 100644
--- a/drivers/accel/amdxdna/amdxdna_mailbox.c
+++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
@@ -499,7 +499,7 @@ xdna_mailbox_start_channel(struct mailbox_channel *mb_chann,
 	int ret;
 
 	if (!is_power_of_2(x2i->rb_size) || !is_power_of_2(i2x->rb_size)) {
-		pr_err("Ring buf size must be power of 2");
+		pr_err("Ring buf size must be power of 2\n");
 		return -EINVAL;
 	}
 
-- 
2.53.0.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] accel/amdxdna: fix missing newline in pr_err message
  2026-03-20 11:29 [PATCH] accel/amdxdna: fix missing newline in pr_err message Haoyu Lu
@ 2026-03-20 15:54 ` Lizhi Hou
  2026-03-21 17:46   ` Claude review: " Claude Code Review Bot
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lizhi Hou @ 2026-03-20 15:54 UTC (permalink / raw)
  To: Haoyu Lu, Min Ma, Oded Gabbay; +Cc: dri-devel, linux-kernel


On 3/20/26 04:29, Haoyu Lu wrote:
> [You don't often get email from hechushiguitu666@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> From: "haoyu.lu" <hechushiguitu666@gmail.com>
>
> Add missing newline to pr_err message in amdxdna_mailbox.c.
>
> Signed-off-by: haoyu.lu <hechushiguitu666@gmail.com>

Please add 'Fixes' tag.

Lizhi

> ---
>   drivers/accel/amdxdna/amdxdna_mailbox.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c
> index 46d844a73a94..e681a090752d 100644
> --- a/drivers/accel/amdxdna/amdxdna_mailbox.c
> +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
> @@ -499,7 +499,7 @@ xdna_mailbox_start_channel(struct mailbox_channel *mb_chann,
>          int ret;
>
>          if (!is_power_of_2(x2i->rb_size) || !is_power_of_2(i2x->rb_size)) {
> -               pr_err("Ring buf size must be power of 2");
> +               pr_err("Ring buf size must be power of 2\n");
>                  return -EINVAL;
>          }
>
> --
> 2.53.0.windows.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: Re: [PATCH] accel/amdxdna: fix missing newline in pr_err message
  2026-03-20 15:54 ` Lizhi Hou
@ 2026-03-21 17:46   ` Claude Code Review Bot
  2026-03-21 17:46   ` Claude Code Review Bot
  2026-03-23  3:49   ` [PATCH v2] " Haoyu Lu
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 17:46 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Re: [PATCH] accel/amdxdna: fix missing newline in pr_err message
Author: Lizhi Hou <lizhi.hou@amd.com>
Patches: 2
Reviewed: 2026-03-22T03:46:13.307891

---

This is a single-patch series that adds a missing `\n` newline character to a `pr_err()` call in the amdxdna mailbox driver. The fix is correct — `pr_err()` requires an explicit newline, unlike `dev_err()` which appends one automatically. Without the `\n`, the message will run into subsequent kernel log output.

The patch is trivial and correct. The only minor nit is the commit author name — "haoyu.lu" with a lowercase first name and dot separator is unconventional for a Signed-off-by; a real name like "Haoyu Lu" (as used in the From header) would be more appropriate.

The patch failed to apply likely because the drm-next tree has already changed the surrounding context in `amdxdna_mailbox.c` (the line is at 502 in drm-next vs 501 in the patch context).

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: Re: [PATCH] accel/amdxdna: fix missing newline in pr_err message
  2026-03-20 15:54 ` Lizhi Hou
  2026-03-21 17:46   ` Claude review: " Claude Code Review Bot
@ 2026-03-21 17:46   ` Claude Code Review Bot
  2026-03-23  3:49   ` [PATCH v2] " Haoyu Lu
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 17:46 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Verdict: Correct, minor nits only.**

The change itself is straightforward and correct:

```c
-		pr_err("Ring buf size must be power of 2");
+		pr_err("Ring buf size must be power of 2\n");
```

I confirmed the current drm-next tree still has the missing newline at `drivers/accel/amdxdna/amdxdna_mailbox.c:502`, so this fix is still needed.

**Nits:**

1. **Signed-off-by name**: The `Signed-off-by: haoyu.lu <hechushiguitu666@gmail.com>` should use the author's real name, e.g., `Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>`, consistent with the `From:` header and kernel conventions (see `Documentation/process/submitting-patches.rst`).

2. **Consider dev_err()**: This `pr_err()` could potentially be converted to `dev_err()` to include device context in the log message, making it easier to identify which device triggered the error in multi-device systems. However, that would be a separate cleanup patch and is outside the scope of this fix.

3. **Commit message**: The commit message is adequate but could mention the specific consequence (log line concatenation with subsequent messages) to explain *why* the newline matters.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] accel/amdxdna: fix missing newline in pr_err message
  2026-03-20 15:54 ` Lizhi Hou
  2026-03-21 17:46   ` Claude review: " Claude Code Review Bot
  2026-03-21 17:46   ` Claude Code Review Bot
@ 2026-03-23  3:49   ` Haoyu Lu
  2026-03-23 16:40     ` Lizhi Hou
  2 siblings, 1 reply; 6+ messages in thread
From: Haoyu Lu @ 2026-03-23  3:49 UTC (permalink / raw)
  To: lizhi.hou; +Cc: mamin506, ogabbay, dri-devel, linux-kernel, haoyu.lu

From: "haoyu.lu" <hechushiguitu666@gmail.com>

Add missing newline to pr_err message in amdxdna_mailbox.c.

Fixes: b87f920b9344 ("accel/amdxdna: Support hardware mailbox")
Signed-off-by: haoyu.lu <hechushiguitu666@gmail.com>
---
Changes in v2:
- Added Fixes tag as requested

 drivers/accel/amdxdna/amdxdna_mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c
index 46d844a73a94..e681a090752d 100644
--- a/drivers/accel/amdxdna/amdxdna_mailbox.c
+++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
@@ -499,7 +499,7 @@ xdna_mailbox_start_channel(struct mailbox_channel *mb_chann,
 	int ret;
 
 	if (!is_power_of_2(x2i->rb_size) || !is_power_of_2(i2x->rb_size)) {
-		pr_err("Ring buf size must be power of 2");
+		pr_err("Ring buf size must be power of 2\n");
 		return -EINVAL;
 	}
 
-- 
2.53.0.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] accel/amdxdna: fix missing newline in pr_err message
  2026-03-23  3:49   ` [PATCH v2] " Haoyu Lu
@ 2026-03-23 16:40     ` Lizhi Hou
  0 siblings, 0 replies; 6+ messages in thread
From: Lizhi Hou @ 2026-03-23 16:40 UTC (permalink / raw)
  To: Haoyu Lu; +Cc: mamin506, ogabbay, dri-devel, linux-kernel

Thanks, applied to drm-misc-next.

On 3/22/26 20:49, Haoyu Lu wrote:
> From: "haoyu.lu" <hechushiguitu666@gmail.com>
>
> Add missing newline to pr_err message in amdxdna_mailbox.c.
>
> Fixes: b87f920b9344 ("accel/amdxdna: Support hardware mailbox")
> Signed-off-by: haoyu.lu <hechushiguitu666@gmail.com>
> ---
> Changes in v2:
> - Added Fixes tag as requested
>
>   drivers/accel/amdxdna/amdxdna_mailbox.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c
> index 46d844a73a94..e681a090752d 100644
> --- a/drivers/accel/amdxdna/amdxdna_mailbox.c
> +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
> @@ -499,7 +499,7 @@ xdna_mailbox_start_channel(struct mailbox_channel *mb_chann,
>          int ret;
>
>          if (!is_power_of_2(x2i->rb_size) || !is_power_of_2(i2x->rb_size)) {
> -               pr_err("Ring buf size must be power of 2");
> +               pr_err("Ring buf size must be power of 2\n");
>                  return -EINVAL;
>          }
>
> --
> 2.53.0.windows.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-23 16:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 11:29 [PATCH] accel/amdxdna: fix missing newline in pr_err message Haoyu Lu
2026-03-20 15:54 ` Lizhi Hou
2026-03-21 17:46   ` Claude review: " Claude Code Review Bot
2026-03-21 17:46   ` Claude Code Review Bot
2026-03-23  3:49   ` [PATCH v2] " Haoyu Lu
2026-03-23 16:40     ` Lizhi Hou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox