diff mbox series

Staging: vchi: Add license id and change int type

Message ID 20181107000738.26963-1-andrealmeid@riseup.net (mailing list archive)
State Superseded
Headers show
Series Staging: vchi: Add license id and change int type | expand

Commit Message

André Almeida Nov. 7, 2018, 12:07 a.m. UTC
From: André Almeida <andrealmeid@riseup.net>

Fix the following checkpatch issues:

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
CHECK: Prefer kernel type 's32' over 'int32_t'

Signed-off-by: André Almeida <andrealmeid@riseup.net>

---

Hello! This is my first patch to Linux Kernel. Let me know any feedback
---
 drivers/staging/vc04_services/interface/vchi/vchi_mh.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Stefan Wahren Nov. 7, 2018, 8:30 a.m. UTC | #1
Hi Andre,

Am 07.11.18 um 01:07 schrieb andrealmeid@riseup.net:
> From: André Almeida <andrealmeid@riseup.net>
>
> Fix the following checkpatch issues:
>
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> CHECK: Prefer kernel type 's32' over 'int32_t'

thanks for your patch, but there are some changes required.

First don't change two different things in one patch. So please make a
V2 with one patch for SPDX and the other one for the type change.

>
> Signed-off-by: André Almeida <andrealmeid@riseup.net>
>
> ---
>
> Hello! This is my first patch to Linux Kernel. Let me know any feedback
> ---
>  drivers/staging/vc04_services/interface/vchi/vchi_mh.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> index 198bd076b666..42fc0c693d43 100644
> --- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> +++ b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> @@ -1,5 +1,5 @@
> -/**
> - * Copyright (c) 2010-2012 Broadcom. All rights reserved.
> +/* SPDX-License-Identifier: GPL-2.0 */

Please use the C++ style for the SPDX id.

Unfortunately your id doesn't match the license text. It's a dual
license|: BSD-3-Clause OR GPL-2.0|

> +/* Copyright (c) 2010-2012 Broadcom. All rights reserved.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions
After adding the SPDX id we can drop the whole license text.
> @@ -36,7 +36,7 @@
>  
>  #include <linux/types.h>
>  
> -typedef int32_t VCHI_MEM_HANDLE_T;
> +typedef s32 VCHI_MEM_HANDLE_T;

In general we should drop the whole typedef, but since the firmware
counterpart would use this. I'm okay with this change.

Stefan

>  #define VCHI_MEM_HANDLE_INVALID 0
>  
>  #endif
Greg KH Nov. 7, 2018, 8:57 a.m. UTC | #2
On Tue, Nov 06, 2018 at 10:07:38PM -0200, andrealmeid@riseup.net wrote:
> From: André Almeida <andrealmeid@riseup.net>
> 
> Fix the following checkpatch issues:
> 
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> CHECK: Prefer kernel type 's32' over 'int32_t'
> 
> Signed-off-by: André Almeida <andrealmeid@riseup.net>
> 
> ---
> 
> Hello! This is my first patch to Linux Kernel. Let me know any feedback
> ---
>  drivers/staging/vc04_services/interface/vchi/vchi_mh.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> index 198bd076b666..42fc0c693d43 100644
> --- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> +++ b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> @@ -1,5 +1,5 @@
> -/**

Why change this line?

> - * Copyright (c) 2010-2012 Broadcom. All rights reserved.
> +/* SPDX-License-Identifier: GPL-2.0 */

That is not the correct license for this file.  Please never make a
change like this unless you really know your license information.


> +/* Copyright (c) 2010-2012 Broadcom. All rights reserved.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions
> @@ -36,7 +36,7 @@
>  
>  #include <linux/types.h>
>  
> -typedef int32_t VCHI_MEM_HANDLE_T;
> +typedef s32 VCHI_MEM_HANDLE_T;

You are doing multiple things in the same patch, which isn't allowed.
Please break all patches up into one "logical" thing per patch and send
a patch series.

Also, are you sure this type change is correct?

thanks,

greg k-h
Stefan Wahren Nov. 10, 2018, 5:12 p.m. UTC | #3
Hi Andre,

> Stefan Wahren <stefan.wahren@i2se.com> hat am 7. November 2018 um 09:30 geschrieben:
> 
> 
> >
> > Signed-off-by: André Almeida <andrealmeid@riseup.net>
> >
> > ---
> >
> > Hello! This is my first patch to Linux Kernel. Let me know any feedback
> > ---
> >  drivers/staging/vc04_services/interface/vchi/vchi_mh.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> > index 198bd076b666..42fc0c693d43 100644
> > --- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> > +++ b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
> > @@ -1,5 +1,5 @@
> > -/**
> > - * Copyright (c) 2010-2012 Broadcom. All rights reserved.
> > +/* SPDX-License-Identifier: GPL-2.0 */
> 
> Please use the C++ style for the SPDX id.

Apologies. Please ignore this suggestion. C style is correct for header files.

Stefan
diff mbox series

Patch

diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
index 198bd076b666..42fc0c693d43 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
@@ -1,5 +1,5 @@ 
-/**
- * Copyright (c) 2010-2012 Broadcom. All rights reserved.
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2010-2012 Broadcom. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -36,7 +36,7 @@ 
 
 #include <linux/types.h>
 
-typedef int32_t VCHI_MEM_HANDLE_T;
+typedef s32 VCHI_MEM_HANDLE_T;
 #define VCHI_MEM_HANDLE_INVALID 0
 
 #endif