Re: [PATCH] drm/vmwgfx: use *_32_bits() macros

[Joe Perches <joe@perches.com>]

On Thu, 2016-04-14 at 13:32 +0200, Paul Bolle wrote:
> On do, 2016-03-03 at 11:26 +0100, Paul Bolle wrote:
> > 
> > Use the upper_32_bits() macro instead of the four line equivalent that
> > triggers a GCC warning on 32 bits x86:
> >     drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c: In function
> > 'vmw_cmdbuf_header_submit':
> >     drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c:297:25: warning: right
> > shift count >= width of type [-Wshift-count-overflow]
> >        val = (header->handle >> 32);
> >                              ^
> > 
> > And use the lower_32_bits() macro instead of and-ing with a 32 bits
> > mask.
> > 
> > Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> > ---
> > Note: compile tested only (I don't use any of vmware's products).
> The warning can still be seen on v4.6-rc3 for 32 bits x86. This patch
> applies cleanly to that rc.
> 
> Has anyone had a chance to look at this patch, and perhaps even test it?

Test?  Nope.  Seems obviously correct.

It might be clearer removing the temporary though:

Maybe:
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
index 67cebb2..330794f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
@@ -291,17 +291,12 @@ void vmw_cmdbuf_header_free(struct vmw_cmdbuf_header *header)
 static int vmw_cmdbuf_header_submit(struct vmw_cmdbuf_header *header)
 {
 	struct vmw_cmdbuf_man *man = header->man;
-	u32 val;
 
-	if (sizeof(header->handle) > 4)
-		val = (header->handle >> 32);
-	else
-		val = 0;
-	vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH, val);
-
-	val = (header->handle & 0xFFFFFFFFULL);
-	val |= header->cb_context & SVGA_CB_CONTEXT_MASK;
-	vmw_write(man->dev_priv, SVGA_REG_COMMAND_LOW, val);
+	vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH,
+		  upper_32_bits(header->handle));
+	vmw_write(man->dev_priv, SVGA_REG_COMMAND_LOW,
+		  lower_32_bits(header->handle) |
+		  (header->cb_context & SVGA_CB_CONTEXT_MASK));
 
 	return header->cb_header->status;
 }