mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read
@ 2026-06-22 14:20 Ben Dooks
  2026-06-22 18:14 ` Dave Jiang
  2026-06-23 23:51 ` Alison Schofield
  0 siblings, 2 replies; 4+ messages in thread
From: Ben Dooks @ 2026-06-22 14:20 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield,
	Ira Weiny, nvdimm, linux-kernel
  Cc: Ben Dooks

The dev_err() call in btt_log_read() is passing a seq value
into dev_err() which is a __le32 without any conversion.

Fix the following (prototype) sparse warnings:
drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 5 (different base types)
drivers/nvdimm/btt.c:342:17:    expected int
drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq
drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 6 (different base types)
drivers/nvdimm/btt.c:342:17:    expected int
drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 drivers/nvdimm/btt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 7e1112960d7f..e9d548442884 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -341,8 +341,9 @@ static int btt_log_read(struct arena_info *arena, u32 lane,
 	if (old_ent < 0 || old_ent > 1) {
 		dev_err(to_dev(arena),
 				"log corruption (%d): lane %d seq [%d, %d]\n",
-				old_ent, lane, log.ent[arena->log_index[0]].seq,
-				log.ent[arena->log_index[1]].seq);
+				old_ent, lane,
+				le32_to_cpu(log.ent[arena->log_index[0]].seq),
+				le32_to_cpu(log.ent[arena->log_index[1]].seq));
 		/* TODO set error state? */
 		return -EIO;
 	}
-- 
2.37.2.352.g3c44437643


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

* Re: [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read
  2026-06-22 14:20 [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read Ben Dooks
@ 2026-06-22 18:14 ` Dave Jiang
  2026-06-23 23:51 ` Alison Schofield
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Jiang @ 2026-06-22 18:14 UTC (permalink / raw)
  To: Ben Dooks, Dan Williams, Vishal Verma, Alison Schofield,
	Ira Weiny, nvdimm, linux-kernel



On 6/22/26 7:20 AM, Ben Dooks wrote:
> The dev_err() call in btt_log_read() is passing a seq value
> into dev_err() which is a __le32 without any conversion.
> 
> Fix the following (prototype) sparse warnings:
> drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 5 (different base types)
> drivers/nvdimm/btt.c:342:17:    expected int
> drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq
> drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 6 (different base types)
> drivers/nvdimm/btt.c:342:17:    expected int
> drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/nvdimm/btt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index 7e1112960d7f..e9d548442884 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -341,8 +341,9 @@ static int btt_log_read(struct arena_info *arena, u32 lane,
>  	if (old_ent < 0 || old_ent > 1) {
>  		dev_err(to_dev(arena),
>  				"log corruption (%d): lane %d seq [%d, %d]\n",
> -				old_ent, lane, log.ent[arena->log_index[0]].seq,
> -				log.ent[arena->log_index[1]].seq);
> +				old_ent, lane,
> +				le32_to_cpu(log.ent[arena->log_index[0]].seq),
> +				le32_to_cpu(log.ent[arena->log_index[1]].seq));
>  		/* TODO set error state? */
>  		return -EIO;
>  	}


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

* Re: [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read
  2026-06-22 14:20 [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read Ben Dooks
  2026-06-22 18:14 ` Dave Jiang
@ 2026-06-23 23:51 ` Alison Schofield
  2026-06-24 15:07   ` Ben Dooks
  1 sibling, 1 reply; 4+ messages in thread
From: Alison Schofield @ 2026-06-23 23:51 UTC (permalink / raw)
  To: Ben Dooks
  Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny, nvdimm, linux-kernel

On Mon, Jun 22, 2026 at 03:20:11PM +0100, Ben Dooks wrote:
> The dev_err() call in btt_log_read() is passing a seq value
> into dev_err() which is a __le32 without any conversion.
> 
> Fix the following (prototype) sparse warnings:
> drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 5 (different base types)
> drivers/nvdimm/btt.c:342:17:    expected int
> drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq
> drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 6 (different base types)
> drivers/nvdimm/btt.c:342:17:    expected int
> drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq

Hi Ben,

Please revise the commit log.

The commit log is a message to all future readers, not a place to
paste static analysis warnings and leave the user visible impact
assumed, or as an exercise for the reader.

Prefer something like this:

	When BTT log corruption is detected, btt_log_read() reports the
	sequence numbers of the two log entries. Those values are stored
	little-endian, so printing them without conversion can report
	byte-swapped sequence numbers on big-endian systems.

	Convert the sequence numbers to CPU endianness before passing
	them to dev_err().

	Issue reported by sparse.


(There is no need for the sparse pastings.)

-- Alison

> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  drivers/nvdimm/btt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index 7e1112960d7f..e9d548442884 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -341,8 +341,9 @@ static int btt_log_read(struct arena_info *arena, u32 lane,
>  	if (old_ent < 0 || old_ent > 1) {
>  		dev_err(to_dev(arena),
>  				"log corruption (%d): lane %d seq [%d, %d]\n",
> -				old_ent, lane, log.ent[arena->log_index[0]].seq,
> -				log.ent[arena->log_index[1]].seq);
> +				old_ent, lane,
> +				le32_to_cpu(log.ent[arena->log_index[0]].seq),
> +				le32_to_cpu(log.ent[arena->log_index[1]].seq));
>  		/* TODO set error state? */
>  		return -EIO;
>  	}
> -- 
> 2.37.2.352.g3c44437643
> 

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

* Re: [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read
  2026-06-23 23:51 ` Alison Schofield
@ 2026-06-24 15:07   ` Ben Dooks
  0 siblings, 0 replies; 4+ messages in thread
From: Ben Dooks @ 2026-06-24 15:07 UTC (permalink / raw)
  To: Alison Schofield
  Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny, nvdimm, linux-kernel

On 24/06/2026 00:51, Alison Schofield wrote:
> On Mon, Jun 22, 2026 at 03:20:11PM +0100, Ben Dooks wrote:
>> The dev_err() call in btt_log_read() is passing a seq value
>> into dev_err() which is a __le32 without any conversion.
>>
>> Fix the following (prototype) sparse warnings:
>> drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 5 (different base types)
>> drivers/nvdimm/btt.c:342:17:    expected int
>> drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq
>> drivers/nvdimm/btt.c:342:17: warning: incorrect type in argument 6 (different base types)
>> drivers/nvdimm/btt.c:342:17:    expected int
>> drivers/nvdimm/btt.c:342:17:    got restricted __le32 [usertype] seq
> 
> Hi Ben,
> 
> Please revise the commit log.
> 
> The commit log is a message to all future readers, not a place to
> paste static analysis warnings and leave the user visible impact
> assumed, or as an exercise for the reader.
> 
> Prefer something like this:
> 
> 	When BTT log corruption is detected, btt_log_read() reports the
> 	sequence numbers of the two log entries. Those values are stored
> 	little-endian, so printing them without conversion can report
> 	byte-swapped sequence numbers on big-endian systems.
> 
> 	Convert the sequence numbers to CPU endianness before passing
> 	them to dev_err().
> 
> 	Issue reported by sparse.
> 
> 
> (There is no need for the sparse pastings.)

Thanks, I've posted a v2, with a reworded patch commit log.

The only comment is that I like the sparse warnings as it makes it
easier to search if there is a patch in flight for this.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

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

end of thread, other threads:[~2026-06-24 15:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-22 14:20 [PATCH] nvdimm/btt: add endian conversion in dev_err in btt_log_read Ben Dooks
2026-06-22 18:14 ` Dave Jiang
2026-06-23 23:51 ` Alison Schofield
2026-06-24 15:07   ` Ben Dooks

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