Linux kernel staging patches
 help / color / mirror / Atom feed
* [PATCH v3] staging: rtl8723bs: use PTR_ALIGN() for rsp_buf
@ 2026-06-16 12:35 Devansh Soni
  2026-06-16 13:02 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: Devansh Soni @ 2026-06-16 12:35 UTC (permalink / raw)
  To: gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter, Devansh Soni

The original code was using manual bitwise calculation for the
alignment of rsp_buf.

Replace this with the standard PTR_ALIGN() macro. The manual math
((SIZE_PTR)ptr & 3) always returns 0 and is completely unnecessary
because kzalloc() already returns 8-byte aligned memory.

Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Devansh Soni <devanshsoni874@gmail.com>
---
Changes in v3:
- Replaced manual bitwise math with PTR_ALIGN() macro based on feedback
  by Dan.
- Updated commit message to detail kzalloc() 8-byte alignment guarantees.

Changes in v2:
- Wrapped commit log text to resolve line length issue noted by Greg.

 drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index c1185c25e..38ce3156c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -184,7 +184,7 @@ int rtw_init_cmd_priv(struct	cmd_priv *pcmdpriv)
 		return -ENOMEM;
 	}
 
-	pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf + 4 - ((SIZE_PTR)(pcmdpriv->rsp_allocated_buf) & 3);
+	pcmdpriv->rsp_buf = PTR_ALIGN(pcmdpriv->rsp_allocated_buf, 4);
 
 	pcmdpriv->cmd_issued_cnt = 0;
 	pcmdpriv->cmd_done_cnt = 0;
-- 
2.54.0

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

* Re: [PATCH v3] staging: rtl8723bs: use PTR_ALIGN() for rsp_buf
  2026-06-16 12:35 [PATCH v3] staging: rtl8723bs: use PTR_ALIGN() for rsp_buf Devansh Soni
@ 2026-06-16 13:02 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2026-06-16 13:02 UTC (permalink / raw)
  To: Devansh Soni; +Cc: gregkh, linux-staging, linux-kernel, dan.carpenter

On Tue, Jun 16, 2026 at 06:05:28PM +0530, Devansh Soni wrote:
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index c1185c25e..38ce3156c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -184,7 +184,7 @@ int rtw_init_cmd_priv(struct	cmd_priv *pcmdpriv)
>  		return -ENOMEM;
>  	}
>  
> -	pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf + 4 - ((SIZE_PTR)(pcmdpriv->rsp_allocated_buf) & 3);
> +	pcmdpriv->rsp_buf = PTR_ALIGN(pcmdpriv->rsp_allocated_buf, 4);

You're working against an old version of the kernel.  You should be
working against linux-next or devel-next.

The commit message mentions 8-byte aligned memory, but the code is still
doing PTR_ALIGN().  The kmalloc() alignment is determined by
ARCH_KMALLOC_MINALIGN and it's always at least 8 but it can be higher.
Here the code is trying to ensure that it is 4 byte aligned, and 8 is
already a multiple of 4.  You could just delete the PTR_ALIGN() along
with the related code.

1. It allocates 4 extra bytes.  "MAX_RSPSZ + 4" bytes.  It could instead
   just allocate MAX_RSPSZ bytes.
2. This pcmdpriv->rsp_buf = PTR_ALIGN() can be removed.
3. The pcmdpriv->rsp_allocated_buf pointer only exists to store the
   unaligned pointer.  Just get rid of it.

The early part of function which does CMDBUFF_ALIGN_SZ is more complicated
because that is 512 bytes.  It's trying to ensure that the pointers are
aligned enough for DMA.  The proper alignment in that case is probably
ARCH_DMA_MINALIGN but I'm less familiar with how that works.

regards,
dan carpenter


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

end of thread, other threads:[~2026-06-16 13:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-16 12:35 [PATCH v3] staging: rtl8723bs: use PTR_ALIGN() for rsp_buf Devansh Soni
2026-06-16 13:02 ` Dan Carpenter

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