Re: [PATCH] Staging:tidspbridge: Fixing coding style

Re: [PATCH] Staging:tidspbridge: Fixing coding style

[Masood Mehmood]

On Fri, Feb 28, 2014 at 07:01:56PM -0800, Greg Kroah-Hartman wrote:
> On Fri, Feb 28, 2014 at 06:15:52PM -0800, Masood Mehmood wrote:
> > 
> 
> > 
> > Fixing some basic coding style issues.
> 
> Which issues did you fix?  Please be more specific.  Did you fix them
> for the whole driver, or just a specific file?

- Unnecessary line break and space.
- and some * adjusted to the data name
- Removed braces for single statement if conditions.

I just realized, other files of the same driver also need some style fixes.
I'll send another patch with reset of the files included.

> 
> And what's with the odd multiple attachments?
Sorry, new to mutt.

> 
> thanks,
> 
> greg k-h

Thanks,
Masood

[PATCH] Staging:tidspbridge: Fixing coding style

[Masood Mehmood]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3539 bytes --]


Fixing some basic coding style issues.

Signed-off-by: Masood Mehmood <ody.guru@gmail.com>
---
 drivers/staging/tidspbridge/rmgr/node.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/tidspbridge/rmgr/node.c b/drivers/staging/tidspbridge/rmgr/node.c
index 87dfa92..76f885f 100644
--- a/drivers/staging/tidspbridge/rmgr/node.c
+++ b/drivers/staging/tidspbridge/rmgr/node.c
@@ -246,7 +246,7 @@ static void fill_stream_def(struct node_object *hnode,
 			    struct node_strmdef *pstrm_def,
 			    struct dsp_strmattr *pattrs);
 static void free_stream(struct node_mgr *hnode_mgr, struct stream_chnl stream);
-static int get_fxn_address(struct node_object *hnode, u32 * fxn_addr,
+static int get_fxn_address(struct node_object *hnode, u32 *fxn_addr,
 				  u32 phase);
 static int get_node_props(struct dcd_manager *hdcd_mgr,
 				 struct node_object *hnode,
@@ -406,7 +406,7 @@ int node_allocate(struct proc_object *hprocessor,
 
 	/* check for page aligned Heap size */
 	if (((attr_in->heap_size) & (PG_SIZE4K - 1))) {
-		pr_err("%s: node heap size not aligned to 4K, size = 0x%x \n",
+		pr_err("%s: node heap size not aligned to 4K, size = 0x%x",
 		       __func__, attr_in->heap_size);
 		status = -EINVAL;
 	} else {
@@ -425,7 +425,7 @@ int node_allocate(struct proc_object *hprocessor,
 					task_arg_obj.dsp_heap_res_addr),
 				     pr_ctxt);
 	if (status) {
-		pr_err("%s: Failed to reserve memory for heap: 0x%x\n",
+		pr_err("%s: Failed to reserve memory for heap: 0x%x",
 		       __func__, status);
 		goto func_cont;
 	}
@@ -703,9 +703,9 @@ DBAPI node_alloc_msg_buf(struct node_object *hnode, u32 usize,
 		pattr = &node_dfltbufattrs;	/* set defaults */
 
 	status = proc_get_processor_id(pnode->processor, &proc_id);
-	if (proc_id != DSP_UNIT) {
+	if (proc_id != DSP_UNIT)
 		goto func_end;
-	}
+
 	/*  If segment ID includes MEM_SETVIRTUALSEGID then pbuffer is a
 	 *  virt  address, so set this info in this node's translator
 	 *  object for  future ref. If MEM_GETVIRTUALSEGID then retrieve
@@ -886,11 +886,10 @@ int node_connect(struct node_object *node1, u32 stream1,
 	if (pattrs && pattrs->strm_mode != STRMMODE_PROCCOPY)
 		return -EPERM;	/* illegal stream mode */
 
-	if (node1_type != NODE_GPP) {
+	if (node1_type != NODE_GPP)
 		hnode_mgr = node1->node_mgr;
-	} else {
+	else
 		hnode_mgr = node2->node_mgr;
-	}
 
 	/* Enter critical section */
 	mutex_lock(&hnode_mgr->node_mgr_lock);
@@ -1576,7 +1575,7 @@ func_end:
  *  Purpose:
  *      Frees the message buffer.
  */
-int node_free_msg_buf(struct node_object *hnode, u8 * pbuffer,
+int node_free_msg_buf(struct node_object *hnode, u8 *pbuffer,
 			     struct dsp_bufferattr *pattr)
 {
 	struct node_object *pnode = (struct node_object *)hnode;
@@ -2322,7 +2321,8 @@ int node_terminate(struct node_object *hnode, int *pstatus)
 			if (!hdeh_mgr)
 				goto func_cont;
 
-			bridge_deh_notify(hdeh_mgr, DSP_SYSERROR, DSP_EXCEPTIONABORT);
+			bridge_deh_notify(hdeh_mgr, DSP_SYSERROR,
+							DSP_EXCEPTIONABORT);
 		}
 	}
 func_cont:
@@ -2640,8 +2640,7 @@ static void free_stream(struct node_mgr *hnode_mgr, struct stream_chnl stream)
  *  Purpose:
  *      Retrieves the address for create, execute or delete phase for a node.
  */
-static int get_fxn_address(struct node_object *hnode, u32 * fxn_addr,
-				  u32 phase)
+static int get_fxn_address(struct node_object *hnode, u32 *fxn_addr, u32 phase)
 {
 	char *pstr_fxn_name = NULL;
 	struct node_mgr *hnode_mgr = hnode->node_mgr;
-- 
1.8.1.4

Re: [PATCH] Staging:tidspbridge: Fixing coding style

[Greg Kroah-Hartman]

On Fri, Feb 28, 2014 at 06:15:52PM -0800, Masood Mehmood wrote:
> 

> 
> Fixing some basic coding style issues.

Which issues did you fix?  Please be more specific.  Did you fix them
for the whole driver, or just a specific file?

And what's with the odd multiple attachments?

thanks,

greg k-h

Re: [PATCH] Staging:tidspbridge: Fixing coding style

[Greg Kroah-Hartman]

On Fri, Feb 28, 2014 at 12:30:04AM -0800, Masood Mehmood wrote:
> 
> On Fri, Feb 28, 2014 at 07:01:56PM -0800, Greg Kroah-Hartman wrote:
> > On Fri, Feb 28, 2014 at 06:15:52PM -0800, Masood Mehmood wrote:
> > > 
> > 
> > > 
> > > Fixing some basic coding style issues.
> > 
> > Which issues did you fix?  Please be more specific.  Did you fix them
> > for the whole driver, or just a specific file?
> 
> - Unnecessary line break and space.
> - and some * adjusted to the data name
> - Removed braces for single statement if conditions.

Great, can you put that in the patch itself?

As it's small, they all can be in the same patch, but normally we only
want one patch per "type" of change.  For this case, if it was lots of
changes, you would break it up into different patches.

> I just realized, other files of the same driver also need some style fixes.
> I'll send another patch with reset of the files included.

Watch out, now you might want to send multiple patches, based on the
above "one thing per patch" rule.

thanks,

greg k-h

Re: [PATCH] Staging:tidspbridge: Fixing coding style

[Dan Carpenter]

This patch introduces bugs.

On Fri, Feb 28, 2014 at 06:15:52PM -0800, Masood Mehmood wrote:
> 

> 
> Fixing some basic coding style issues.
> 

Changelog sucks.  Doesn't say what the changes are.

> Signed-off-by: Masood Mehmood <ody.guru@gmail.com>
> ---
>  drivers/staging/tidspbridge/rmgr/node.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/tidspbridge/rmgr/node.c b/drivers/staging/tidspbridge/rmgr/node.c
> index 87dfa92..76f885f 100644
> --- a/drivers/staging/tidspbridge/rmgr/node.c
> +++ b/drivers/staging/tidspbridge/rmgr/node.c
> @@ -246,7 +246,7 @@ static void fill_stream_def(struct node_object *hnode,
>  			    struct node_strmdef *pstrm_def,
>  			    struct dsp_strmattr *pattrs);
>  static void free_stream(struct node_mgr *hnode_mgr, struct stream_chnl stream);
> -static int get_fxn_address(struct node_object *hnode, u32 * fxn_addr,
> +static int get_fxn_address(struct node_object *hnode, u32 *fxn_addr,
>  				  u32 phase);
>  static int get_node_props(struct dcd_manager *hdcd_mgr,
>  				 struct node_object *hnode,
> @@ -406,7 +406,7 @@ int node_allocate(struct proc_object *hprocessor,
>  
>  	/* check for page aligned Heap size */
>  	if (((attr_in->heap_size) & (PG_SIZE4K - 1))) {
> -		pr_err("%s: node heap size not aligned to 4K, size = 0x%x \n",
> +		pr_err("%s: node heap size not aligned to 4K, size = 0x%x",

The correct fix here is:

		pr_err("%s: node heap size not aligned to 4K, size = 0x%x\n",

>  		       __func__, attr_in->heap_size);
>  		status = -EINVAL;
>  	} else {
> @@ -425,7 +425,7 @@ int node_allocate(struct proc_object *hprocessor,
>  					task_arg_obj.dsp_heap_res_addr),
>  				     pr_ctxt);
>  	if (status) {
> -		pr_err("%s: Failed to reserve memory for heap: 0x%x\n",
> +		pr_err("%s: Failed to reserve memory for heap: 0x%x",

What??

>  		       __func__, status);
>  		goto func_cont;
>  	}
> @@ -703,9 +703,9 @@ DBAPI node_alloc_msg_buf(struct node_object *hnode, u32 usize,
>  		pattr = &node_dfltbufattrs;	/* set defaults */
>  
>  	status = proc_get_processor_id(pnode->processor, &proc_id);
> -	if (proc_id != DSP_UNIT) {
> +	if (proc_id != DSP_UNIT)
>  		goto func_end;
> -	}
> +
>  	/*  If segment ID includes MEM_SETVIRTUALSEGID then pbuffer is a
>  	 *  virt  address, so set this info in this node's translator
>  	 *  object for  future ref. If MEM_GETVIRTUALSEGID then retrieve
> @@ -886,11 +886,10 @@ int node_connect(struct node_object *node1, u32 stream1,
>  	if (pattrs && pattrs->strm_mode != STRMMODE_PROCCOPY)
>  		return -EPERM;	/* illegal stream mode */
>  
> -	if (node1_type != NODE_GPP) {
> +	if (node1_type != NODE_GPP)
>  		hnode_mgr = node1->node_mgr;
> -	} else {
> +	else
>  		hnode_mgr = node2->node_mgr;
> -	}
>  
>  	/* Enter critical section */
>  	mutex_lock(&hnode_mgr->node_mgr_lock);
> @@ -1576,7 +1575,7 @@ func_end:
>   *  Purpose:
>   *      Frees the message buffer.
>   */
> -int node_free_msg_buf(struct node_object *hnode, u8 * pbuffer,
> +int node_free_msg_buf(struct node_object *hnode, u8 *pbuffer,
>  			     struct dsp_bufferattr *pattr)
>  {
>  	struct node_object *pnode = (struct node_object *)hnode;
> @@ -2322,7 +2321,8 @@ int node_terminate(struct node_object *hnode, int *pstatus)
>  			if (!hdeh_mgr)
>  				goto func_cont;
>  
> -			bridge_deh_notify(hdeh_mgr, DSP_SYSERROR, DSP_EXCEPTIONABORT);
> +			bridge_deh_notify(hdeh_mgr, DSP_SYSERROR,
> +							DSP_EXCEPTIONABORT);

Do it like this:

			bridge_deh_notify(hdeh_mgr, DSP_SYSERROR,
					  DSP_EXCEPTIONABORT);


regards,
dan carpenter