[OPW,kernel,2/2] Separating arguments with space
diff mbox

Message ID 20131016075518.GA6302@TimeHacker
State Changes Requested
Headers show

Commit Message

Monam Agarwal Oct. 16, 2013, 7:56 a.m. UTC
This Patch Separates the arguments to DEVICE_ATTR by space

Signed-off-by: Monam Agarwal <monamagarwal123@gmail.com>
---
 drivers/firmware/dcdbas.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Josh Triplett Oct. 16, 2013, 8:03 a.m. UTC | #1
On Wed, Oct 16, 2013 at 01:26:44PM +0530, Monam Agarwal wrote:
> This Patch Separates the arguments to DEVICE_ATTR by space
> 
> Signed-off-by: Monam Agarwal <monamagarwal123@gmail.com>

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> ---
>  drivers/firmware/dcdbas.h |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/dcdbas.h b/drivers/firmware/dcdbas.h
> index ca3cb0a..8aa0682 100644
> --- a/drivers/firmware/dcdbas.h
> +++ b/drivers/firmware/dcdbas.h
> @@ -55,13 +55,13 @@
>  #define SMI_CMD_MAGIC				(0x534D4931)
>  
>  #define DCDBAS_DEV_ATTR_RW(_name) \
> -	DEVICE_ATTR(_name,0600,_name##_show,_name##_store);
> +	DEVICE_ATTR(_name, 0600, _name##_show, _name##_store);
>  
>  #define DCDBAS_DEV_ATTR_RO(_name) \
> -	DEVICE_ATTR(_name,0400,_name##_show,NULL);
> +	DEVICE_ATTR(_name, 0400, _name##_show, NULL);
>  
>  #define DCDBAS_DEV_ATTR_WO(_name) \
> -	DEVICE_ATTR(_name,0200,NULL,_name##_store);
> +	DEVICE_ATTR(_name, 0200, NULL, _name##_store);
>  
>  #define DCDBAS_BIN_ATTR_RW(_name) \
>  struct bin_attribute bin_attr_##_name = { \
> -- 
> 1.7.9.5
> 
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
Greg Kroah-Hartman Oct. 16, 2013, 4:25 p.m. UTC | #2
On Wed, Oct 16, 2013 at 01:26:44PM +0530, Monam Agarwal wrote:
> This Patch Separates the arguments to DEVICE_ATTR by space

Odd capitalization in that sentence :)

Also, you forgot to say in the Subject: what part of the kernel you are
modifying.

As you are now touching things outside of the drivers/staging/ tree, you
will run into the issue that you need to deal with the maintainers of
that part of the kernel, which might not be people here on this mailing
list.

For now, for the application process, I _strongly_ suggest staying in
drivers/staging/ unless you really want to go outside of that area for
some reason.  If you want to, you should ask and find some place with a
maintainer that is part of the OPW process, as you might find it a more
difficult place to get cleanup patches accepted.

> Signed-off-by: Monam Agarwal <monamagarwal123@gmail.com>

As for this patch, I have some comments:

> --- a/drivers/firmware/dcdbas.h
> +++ b/drivers/firmware/dcdbas.h
> @@ -55,13 +55,13 @@
>  #define SMI_CMD_MAGIC				(0x534D4931)
>  
>  #define DCDBAS_DEV_ATTR_RW(_name) \
> -	DEVICE_ATTR(_name,0600,_name##_show,_name##_store);
> +	DEVICE_ATTR(_name, 0600, _name##_show, _name##_store);

This should be using the DEVICE_ATTR_RW() macro instead.  But then, if
you use that, there should not be any need to have a
DCDBAS_DEV_ATTR_RW() macro at all, DEVICE_ATTR_RW() can be used directly
instead, and this should be removed entirely.

>  #define DCDBAS_DEV_ATTR_RO(_name) \
> -	DEVICE_ATTR(_name,0400,_name##_show,NULL);
> +	DEVICE_ATTR(_name, 0400, _name##_show, NULL);

Same here, use the DEVICE_ATTR_RO() macro instead of
DCDBAS_DEV_ATTR_RO().

>  #define DCDBAS_DEV_ATTR_WO(_name) \
> -	DEVICE_ATTR(_name,0200,NULL,_name##_store);
> +	DEVICE_ATTR(_name, 0200, NULL, _name##_store);

And here, use DEVICE_ATTR_WO().

Now to be fair, these are new macros, so when this code was originally
written they were not present, but it should be fixed up to use them
now.

See, fixing things outside of staging is a bit harder than merely
changing the location of spaces :)

Hope this helps,

greg k-h

Patch
diff mbox

diff --git a/drivers/firmware/dcdbas.h b/drivers/firmware/dcdbas.h
index ca3cb0a..8aa0682 100644
--- a/drivers/firmware/dcdbas.h
+++ b/drivers/firmware/dcdbas.h
@@ -55,13 +55,13 @@ 
 #define SMI_CMD_MAGIC				(0x534D4931)
 
 #define DCDBAS_DEV_ATTR_RW(_name) \
-	DEVICE_ATTR(_name,0600,_name##_show,_name##_store);
+	DEVICE_ATTR(_name, 0600, _name##_show, _name##_store);
 
 #define DCDBAS_DEV_ATTR_RO(_name) \
-	DEVICE_ATTR(_name,0400,_name##_show,NULL);
+	DEVICE_ATTR(_name, 0400, _name##_show, NULL);
 
 #define DCDBAS_DEV_ATTR_WO(_name) \
-	DEVICE_ATTR(_name,0200,NULL,_name##_store);
+	DEVICE_ATTR(_name, 0200, NULL, _name##_store);
 
 #define DCDBAS_BIN_ATTR_RW(_name) \
 struct bin_attribute bin_attr_##_name = { \