staging: tidspbridge: clean up drv_interface.c
cover staging: tidspbridge: clean up drv_interface.c
I'm trying to learn how to contribute to the kernel and dsp/bridge is a module that I have used for a while.
These patches are the result of this first effort. It is a clean up of the file drv_interface.c which is the entry point of the kernel module.
I would like to have some review in order to know if I could keep moving in this direction.
Thanks
vmjl
Víctor Manuel Jáquez Leal (5):
staging: tidspbridge: more readable code
staging: tidspbridge: remove unused header
staging: tidspbridge: Lindent to drv_interface.c
staging: tidspbridge: silence the compiler
staging: tidspbridge: remove header inclusions
drivers/staging/tidspbridge/rmgr/drv_interface.c | 352 +++++++++++----------- drivers/staging/tidspbridge/rmgr/drv_interface.h | 28 -- 2 files changed, 171 insertions(+), 209 deletions(-) delete mode 100644 drivers/staging/tidspbridge/rmgr/drv_interface.h
Signature · 1 line
1.7.8.3
PATCH 1/5 staging: tidspbridge: more readable code
Uppercase function names are not pretty. Also the code flow readability is enhanced.
--- drivers/staging/tidspbridge/rmgr/drv_interface.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
drivers/staging/tidspbridge/rmgr/drv_interface.c +6 -7
@@ -428,7 +428,7 @@ func_cont: }#ifdef CONFIG_PM -static int BRIDGE_SUSPEND (struct platform_device *pdev,pm_message_t state) +static int bridge_suspend (struct platform_device *pdev,pm_message_t state) {u32 status;u32 command =PWR_EMERGENCYDEEPSLEEP ;
@@ -441,7 +441,7 @@ static int BRIDGE_SUSPEND(struct platform_device *pdev, pm_message_t state)return 0 ; } -static int BRIDGE_RESUME (struct platform_device *pdev) +static int bridge_resume (struct platform_device *pdev) {u32 status;
@@ -453,9 +453,6 @@ static int BRIDGE_RESUME(struct platform_device *pdev)wake_up (&bridge_suspend_data.suspend_wq);return 0 ; } -#else -#define BRIDGE_SUSPEND NULL -#define BRIDGE_RESUME NULL#endif static struct platform_driver bridge_driver = {
@@ -464,8 +461,10 @@ static struct platform_driver bridge_driver = { }, .probe = omap34_xx_bridge_probe, .remove = __devexit_p(omap34_xx_bridge_remove), - .suspend = BRIDGE_SUSPEND, - .resume = BRIDGE_RESUME, +#ifdef CONFIG_PM + .suspend = bridge_suspend, + .resume = bridge_resume, +#endif };static int __init bridge_init (void )
Signature · 1 line
1.7.8.3
Re: [PATCH 1/5] staging: tidspbridge: more readable code
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 2 lines
Uppercase function names are not pretty. Also the code flow readability is enhanced.
Looks good to me.
Signature · 1 line
Felipe Contreras
Re: [PATCH 1/5] staging: tidspbridge: more readable code
2012/1/24 Felipe Contreras <[email protected]>:
Quoted 5 lines
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 2 lines
Uppercase function names are not pretty. Also the code flow readability is enhanced.
Looks good to me.
FWIW, I agree.
Regards,
Omar
PATCH 2/5 staging: tidspbridge: remove unused header
In reply to Víctor Manuel Jáquez LealNo functional changes.
The header file drv_interface.h was only used locally, hence there's no need to have it.
Also the only prototyped functions were the file_operations callbacks, then this commit moves them up to avoid prototyping too.
--- drivers/staging/tidspbridge/rmgr/drv_interface.c | 313 +++++++++++----------- drivers/staging/tidspbridge/rmgr/drv_interface.h | 28 -- 2 files changed, 155 insertions(+), 186 deletions(-) delete mode 100644 drivers/staging/tidspbridge/rmgr/drv_interface.h
drivers/staging/tidspbridge/rmgr/drv_interface.c +155 -158
@@ -48,9 +48,6 @@/* ----------------------------------- Resource Manager */ #include <dspbridge/pwr.h> -/* ----------------------------------- This */ -#include <drv_interface.h> -#include <dspbridge/resourcecleanup.h> #include <dspbridge/chnl.h> #include <dspbridge/proc.h>
@@ -133,6 +130,161 @@ MODULE_VERSION(DSPBRIDGE_VERSION);static char *driver_name =DRIVER_NAME ; +/* +* This function is called when an application opens handle to the +* bridge driver. +*/ +static int bridge_open (struct inode *ip,struct file *filp) +{ +int status =0 ; +struct process_context *pr_ctxt =NULL ; + +/* +* Allocate a new process context and insert it into global +* process context list. +*/ + +#ifdef CONFIG_TIDSPBRIDGE_RECOVERY +if (recover) { +if (filp->f_flags &O_NONBLOCK || +wait_for_completion_interruptible (&bridge_open_comp)) +return -EBUSY ; + } +#endif + pr_ctxt =kzalloc (sizeof (struct process_context ),GFP_KERNEL ); +if (pr_ctxt) { + pr_ctxt->res_state =PROC_RES_ALLOCATED ; +spin_lock_init (&pr_ctxt->dmm_map_lock); +INIT_LIST_HEAD (&pr_ctxt->dmm_map_list); +spin_lock_init (&pr_ctxt->dmm_rsv_lock); +INIT_LIST_HEAD (&pr_ctxt->dmm_rsv_list); + + pr_ctxt->node_id =kzalloc (sizeof (struct idr ),GFP_KERNEL ); +if (pr_ctxt->node_id) { +idr_init (pr_ctxt->node_id); + }else { + status = -ENOMEM ; + goto err; + } + + pr_ctxt->stream_id =kzalloc (sizeof (struct idr ),GFP_KERNEL ); +if (pr_ctxt->stream_id) +idr_init (pr_ctxt->stream_id); +else + status = -ENOMEM ; + }else { + status = -ENOMEM ; + } +err: + filp->private_data = pr_ctxt; +#ifdef CONFIG_TIDSPBRIDGE_RECOVERY +if (!status) +atomic_inc (&bridge_cref); +#endif +return status; +} + +/* +* This function is called when an application closes handle to the bridge +* driver. +*/ +static int bridge_release (struct inode *ip,struct file *filp) +{ +int status =0 ; +struct process_context *pr_ctxt; + +if (!filp->private_data) { + status = -EIO ; + goto err; + } + + pr_ctxt = filp->private_data; +flush_signals (current); +drv_remove_all_resources (pr_ctxt); +proc_detach (pr_ctxt); +kfree (pr_ctxt); + + filp->private_data =NULL ; + +err: +#ifdef CONFIG_TIDSPBRIDGE_RECOVERY +if (!atomic_dec_return (&bridge_cref)) +complete (&bridge_comp); +#endif +return status; +} + +/* This function provides IO interface to the bridge driver. */ +static long bridge_ioctl (struct file *filp,unsigned int code, +unsigned long args) +{ +int status; +u32 retval =0 ; +union trapped_args buf_in; + +DBC_REQUIRE (filp !=NULL ); +#ifdef CONFIG_TIDSPBRIDGE_RECOVERY +if (recover) { + status = -EIO ; + goto err; + } +#endif +#ifdef CONFIG_PM + status =omap34_xxbridge_suspend_lockout (&bridge_suspend_data, filp); +if (status !=0 ) +return status; +#endif + +if (!filp->private_data) { + status = -EIO ; + goto err; + } + + status =copy_from_user (&buf_in, (union trapped_args *)args, +sizeof (union trapped_args )); + +if (!status) { + status =api_call_dev_ioctl (code, &buf_in, &retval, + filp->private_data); + +if (!status) { + status = retval; + }else { +dev_dbg (bridge,"%s: IOCTL Failed, code: 0x%x " +"status 0x%x\n" , __func__, code, status); + status =-1 ; + } + + } + +err: +return status; +} + +/* This function maps kernel space memory to user space memory. */ +static int bridge_mmap (struct file *filp,struct vm_area_struct *vma) +{ +u32 offset = vma->vm_pgoff <<PAGE_SHIFT ; +u32 status; + +DBC_ASSERT (vma->vm_start < vma->vm_end); + + vma->vm_flags |=VM_RESERVED |VM_IO ; + vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); + +dev_dbg (bridge,"%s: vm filp %p offset %x start %lx end %lx page_prot " +"%lx flags %lx\n" , __func__, filp, offset, + vma->vm_start, vma->vm_end, vma->vm_page_prot, vma->vm_flags); + + status =remap_pfn_range (vma, vma->vm_start, vma->vm_pgoff, + vma->vm_end - vma->vm_start, + vma->vm_page_prot); +if (status !=0 ) + status = -EAGAIN ; + +return status; +} +static const struct file_operations bridge_fops = { .open = bridge_open, .release = bridge_release,
@@ -477,161 +629,6 @@ static void __exit bridge_exit(void)platform_driver_unregister (&bridge_driver); } -/* -* This function is called when an application opens handle to the -* bridge driver. -*/ -static int bridge_open (struct inode *ip,struct file *filp) -{ -int status =0 ; -struct process_context *pr_ctxt =NULL ; - -/* -* Allocate a new process context and insert it into global -* process context list. -*/ - -#ifdef CONFIG_TIDSPBRIDGE_RECOVERY -if (recover) { -if (filp->f_flags &O_NONBLOCK || -wait_for_completion_interruptible (&bridge_open_comp)) -return -EBUSY ; - } -#endif - pr_ctxt =kzalloc (sizeof (struct process_context ),GFP_KERNEL ); -if (pr_ctxt) { - pr_ctxt->res_state =PROC_RES_ALLOCATED ; -spin_lock_init (&pr_ctxt->dmm_map_lock); -INIT_LIST_HEAD (&pr_ctxt->dmm_map_list); -spin_lock_init (&pr_ctxt->dmm_rsv_lock); -INIT_LIST_HEAD (&pr_ctxt->dmm_rsv_list); - - pr_ctxt->node_id =kzalloc (sizeof (struct idr ),GFP_KERNEL ); -if (pr_ctxt->node_id) { -idr_init (pr_ctxt->node_id); - }else { - status = -ENOMEM ; - goto err; - } - - pr_ctxt->stream_id =kzalloc (sizeof (struct idr ),GFP_KERNEL ); -if (pr_ctxt->stream_id) -idr_init (pr_ctxt->stream_id); -else - status = -ENOMEM ; - }else { - status = -ENOMEM ; - } -err: - filp->private_data = pr_ctxt; -#ifdef CONFIG_TIDSPBRIDGE_RECOVERY -if (!status) -atomic_inc (&bridge_cref); -#endif -return status; -} - -/* -* This function is called when an application closes handle to the bridge -* driver. -*/ -static int bridge_release (struct inode *ip,struct file *filp) -{ -int status =0 ; -struct process_context *pr_ctxt; - -if (!filp->private_data) { - status = -EIO ; - goto err; - } - - pr_ctxt = filp->private_data; -flush_signals (current); -drv_remove_all_resources (pr_ctxt); -proc_detach (pr_ctxt); -kfree (pr_ctxt); - - filp->private_data =NULL ; - -err: -#ifdef CONFIG_TIDSPBRIDGE_RECOVERY -if (!atomic_dec_return (&bridge_cref)) -complete (&bridge_comp); -#endif -return status; -} - -/* This function provides IO interface to the bridge driver. */ -static long bridge_ioctl (struct file *filp,unsigned int code, -unsigned long args) -{ -int status; -u32 retval =0 ; -union trapped_args buf_in; - -DBC_REQUIRE (filp !=NULL ); -#ifdef CONFIG_TIDSPBRIDGE_RECOVERY -if (recover) { - status = -EIO ; - goto err; - } -#endif -#ifdef CONFIG_PM - status =omap34_xxbridge_suspend_lockout (&bridge_suspend_data, filp); -if (status !=0 ) -return status; -#endif - -if (!filp->private_data) { - status = -EIO ; - goto err; - } - - status =copy_from_user (&buf_in, (union trapped_args *)args, -sizeof (union trapped_args )); - -if (!status) { - status =api_call_dev_ioctl (code, &buf_in, &retval, - filp->private_data); - -if (!status) { - status = retval; - }else { -dev_dbg (bridge,"%s: IOCTL Failed, code: 0x%x " -"status 0x%x\n" , __func__, code, status); - status =-1 ; - } - - } - -err: -return status; -} - -/* This function maps kernel space memory to user space memory. */ -static int bridge_mmap (struct file *filp,struct vm_area_struct *vma) -{ -u32 offset = vma->vm_pgoff <<PAGE_SHIFT ; -u32 status; - -DBC_ASSERT (vma->vm_start < vma->vm_end); - - vma->vm_flags |=VM_RESERVED |VM_IO ; - vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot); - -dev_dbg (bridge,"%s: vm filp %p offset %x start %lx end %lx page_prot " -"%lx flags %lx\n" , __func__, filp, offset, - vma->vm_start, vma->vm_end, vma->vm_page_prot, vma->vm_flags); - - status =remap_pfn_range (vma, vma->vm_start, vma->vm_pgoff, - vma->vm_end - vma->vm_start, - vma->vm_page_prot); -if (status !=0 ) - status = -EAGAIN ; - -return status; -} -/* To remove all process resources before removing the process from the * process context list */ int drv_remove_all_resources (void *process_ctxt)
drivers/staging/tidspbridge/rmgr/drv_interface.h deleted +0 -28
@@ -1,28 +0,0 @@ -/* -* drv_interface.h -* -* DSP-BIOS Bridge driver support functions for TI OMAP processors. -* -* Copyright (C) 2005-2006 Texas Instruments, Inc. -* -* This package is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -* -* THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR -* IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED -* WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE. -*/ - -#ifndef _DRV_INTERFACE_H_ -#define _DRV_INTERFACE_H_ - -/* Prototypes for all functions in this bridge */ -static int __init bridge_init (void );/* Initialize bridge */ -static void __exit bridge_exit(void); /* Opposite of initialize */ -static int bridge_open (struct inode *ip,struct file *filp);/* Open */ -static int bridge_release (struct inode *ip,struct file *filp);/* Release */ -static long bridge_ioctl (struct file *filp,unsigned int code, -unsigned long args); -static int bridge_mmap (struct file *filp,struct vm_area_struct *vma); -#endif /* ifndef _DRV_INTERFACE_H_ */
Signature · 1 line
1.7.8.3
Re: [PATCH 2/5] staging: tidspbridge: remove unused header
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 7 lines
No functional changes.
The header file
drv_interface.hwas only used locally, hence there's no need to have it.Also the only prototyped functions were the
file_operationscallbacks, then this commit moves them up to avoid prototyping too.
'Also' is a keyword that usually denotes that we are talking about two patches here: one that reshuffles the code, and another one that removes the header.
But looks good to me either way.
Signature · 1 line
Felipe Contreras
Re: [PATCH 2/5] staging: tidspbridge: remove unused header
2012/1/24 Felipe Contreras <[email protected]>:
Quoted 8 lines
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 7 lines
No functional changes.
The header file
drv_interface.hwas only used locally, hence there's no need to have it.Also the only prototyped functions were the
file_operationscallbacks, then this commit moves them up to avoid prototyping too.
...
Quoted 2 lines
But looks good to me either way.
FWIW, I agree.
Regards,
Omar
PATCH 3/5 staging: tidspbridge: Lindent to drv_interface.c
In reply to Víctor Manuel Jáquez LealNo functional changes.
According to Lindent, the file drv_internface.c had some lines with bad indentation.
This commit is the output of Lindent.
--- drivers/staging/tidspbridge/rmgr/drv_interface.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-)
drivers/staging/tidspbridge/rmgr/drv_interface.c +11 -11
@@ -147,7 +147,7 @@ static int bridge_open(struct inode *ip, struct file *filp)#ifdef CONFIG_TIDSPBRIDGE_RECOVERY if (recover) {if (filp->f_flags &O_NONBLOCK || -wait_for_completion_interruptible (&bridge_open_comp)) +wait_for_completion_interruptible (&bridge_open_comp))return -EBUSY ; }#endif
@@ -245,7 +245,7 @@ static long bridge_ioctl(struct file *filp, unsigned int code,if (!status) { status =api_call_dev_ioctl (code, &buf_in, &retval, - filp->private_data); + filp->private_data);if (!status) { status = retval;
@@ -342,9 +342,10 @@ static void bridge_recover(struct work_struct *work)if (atomic_read (&bridge_cref)) {INIT_COMPLETION (bridge_comp);while (!wait_for_completion_timeout (&bridge_comp, -msecs_to_jiffies (REC_TIMEOUT ))) -pr_info ("%s:%d handle(s) still opened\n" , - __func__,atomic_read (&bridge_cref)); +msecs_to_jiffies + (REC_TIMEOUT ))) +pr_info ("%s:%d handle(s) still opened\n" , __func__, +atomic_read (&bridge_cref)); } dev =dev_get_first ();dev_get_dev_node (dev, &dev_node);
@@ -363,10 +364,10 @@ void bridge_recover_schedule(void)#endif #ifdef CONFIG_TIDSPBRIDGE_DVFS static int dspbridge_scale_notification (struct notifier_block *op, -unsigned long val,void *ptr) +unsigned long val,void *ptr) {struct omap_dsp_platform_data *pdata = - omap_dspbridge_dev->dev.platform_data; + omap_dspbridge_dev->dev.platform_data;if (CPUFREQ_POSTCHANGE == val && pdata->dsp_get_opp)pwr_pm_post_scale (PRCM_VDD1 , pdata->dsp_get_opp ());
@@ -413,7 +414,7 @@ static int omap3_bridge_startup(struct platform_device *pdev) CPUFREQ_TRANSITION_NOTIFIER);if (err)pr_err ("%s: clk_notifier_register failed for iva2_ck\n" , - __func__); + __func__);#endif #endif
@@ -471,7 +472,7 @@ err2: err1:#ifdef CONFIG_TIDSPBRIDGE_DVFS cpufreq_unregister_notifier (&iva_clk_notifier, -CPUFREQ_TRANSITION_NOTIFIER ); +CPUFREQ_TRANSITION_NOTIFIER );#endif dsp_clk_exit ();
@@ -547,10 +548,9 @@ static int __devexit omap34_xx_bridge_remove(struct platform_device *pdev)pr_err ("%s: Failed to retrieve the object handle\n" , __func__); goto func_cont; } -#ifdef CONFIG_TIDSPBRIDGE_DVFS if (cpufreq_unregister_notifier (&iva_clk_notifier, -CPUFREQ_TRANSITION_NOTIFIER )) +CPUFREQ_TRANSITION_NOTIFIER ))pr_err ("%s: cpufreq_unregister_notifier failed for iva2_ck\n" , __func__);#endif /* #ifdef CONFIG_TIDSPBRIDGE_DVFS */
Signature · 1 line
1.7.8.3
Re: [PATCH 3/5] staging: tidspbridge: Lindent to drv_interface.c
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 6 lines
No functional changes.
According to Lindent, the file
drv_internface.chad some lines with bad indentation.This commit is the output of Lindent.
Usually lindent tends to do whatever it wants, unless carefully configured...
...
drivers/staging/tidspbridge/rmgr/drv_interface.c static void bridge_recover(struct work_struct *work) · line 346
Quoted 9 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +2
-3@@ -342,9 +342,10 @@ static void bridge_recover(struct work_struct *work) if (atomic_read(&bridge_cref)) { INIT_COMPLETION(bridge_comp); while (!wait_for_completion_timeout(&bridge_comp, - msecs_to_jiffies(REC_TIMEOUT))) - pr_info("%s:%d handle(s) still opened\n", - __func__, atomic_read(&bridge_cref)); + msecs_to_jiffies + (REC_TIMEOUT)))
Like here, it just split msecs_to_jiffies(REC_TIMEOUT) into 2 lines making it a little harder to read.
drivers/staging/tidspbridge/rmgr/drv_interface.c static void bridge_recover(struct work_struct *work) · line 348
Quoted 2 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +2
-0+ pr_info("%s:%d handle(s) still opened\n", __func__, + atomic_read(&bridge_cref));
I remember the rule was to break lines as far to the right as possible, no? Chapter 2 CodingStyle, same for the other similar changes.
...
drivers/staging/tidspbridge/rmgr/drv_interface.c static int __devexit omap34_xx_bridge_remove(struct platform_device *pdev) · line 550
Quoted 5 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +0
-1@@ -547,10 +548,9 @@ static int __devexit omap34_xx_bridge_remove(struct platform_device *pdev) pr_err("%s: Failed to retrieve the object handle\n", __func__); goto func_cont; } -
Blank line removed?
Quoted 10 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +1
-2#ifdef CONFIG_TIDSPBRIDGE_DVFS if (cpufreq_unregister_notifier(&iva_clk_notifier, - CPUFREQ_TRANSITION_NOTIFIER)) + CPUFREQ_TRANSITION_NOTIFIER)) pr_err("%s: cpufreq_unregister_notifier failed for iva2_ck\n", __func__); #endif /* #ifdef CONFIG_TIDSPBRIDGE_DVFS */ --1.7.8.3
Regards,
Omar
Re: [PATCH 3/5] staging: tidspbridge: Lindent to drv_interface.c
Quoted 10 lines · Omar
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 6 lines
No functional changes.
According to Lindent, the file
drv_internface.chad some lines with bad indentation.This commit is the output of Lindent.
Usually lindent tends to do whatever it wants, unless carefully configured...
Thanks Omar!
I'll post a v2 of this patch set, with a couple things more and with more attention on what lindent does in this one, if that's ok.
vmjl
Quoted 50 lines
...
drivers/staging/tidspbridge/rmgr/drv_interface.c static void bridge_recover(struct work_struct *work) · line 346
Quoted 9 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +2
-3@@ -342,9 +342,10 @@ static void bridge_recover(struct work_struct *work) if (atomic_read(&bridge_cref)) { INIT_COMPLETION(bridge_comp); while (!wait_for_completion_timeout(&bridge_comp, - msecs_to_jiffies(REC_TIMEOUT))) - pr_info("%s:%d handle(s) still opened\n", - __func__, atomic_read(&bridge_cref)); + msecs_to_jiffies + (REC_TIMEOUT)))Like here, it just split
msecs_to_jiffies(REC_TIMEOUT)into 2 lines making it a little harder to read.drivers/staging/tidspbridge/rmgr/drv_interface.c static void bridge_recover(struct work_struct *work) · line 348
Quoted 2 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +2
-0+ pr_info("%s:%d handle(s) still opened\n", __func__, + atomic_read(&bridge_cref));I remember the rule was to break lines as far to the right as possible, no? Chapter 2 CodingStyle, same for the other similar changes.
...
drivers/staging/tidspbridge/rmgr/drv_interface.c static int __devexit omap34_xx_bridge_remove(struct platform_device *pdev) · line 550
Quoted 5 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +0
-1@@ -547,10 +548,9 @@ static int __devexit omap34_xx_bridge_remove(struct platform_device *pdev) pr_err("%s: Failed to retrieve the object handle\n", __func__); goto func_cont; } -Blank line removed?
Quoted 10 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +1
-2#ifdef CONFIG_TIDSPBRIDGE_DVFS if (cpufreq_unregister_notifier(&iva_clk_notifier, - CPUFREQ_TRANSITION_NOTIFIER)) + CPUFREQ_TRANSITION_NOTIFIER)) pr_err("%s: cpufreq_unregister_notifier failed for iva2_ck\n", __func__); #endif /* #ifdef CONFIG_TIDSPBRIDGE_DVFS */ --1.7.8.3
Regards,
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] staging: tidspbridge: Lindent to drv_interface.c
In reply to Ramirez Luna, OmarQuoted 7 lines · Omar
Quoted 2 lines
+ pr_info("%s:%d handle(s) still opened\n",
__func__,
+atomic_read(&bridge_cref));I remember the rule was to break lines as far to the right as possible, no? Chapter 2 CodingStyle, same for the other similar changes.
It doesn't mean you have to right justify things, it just means indented. The original code is fine here and the new code is fine here. It's up to whoever writes the code to decide.
regards,
dan carpenter
Re: [PATCH 3/5] staging: tidspbridge: Lindent to drv_interface.c
Quoted 9 lines · Dan Carpenter
Quoted 5 lines · Omar
drivers/staging/tidspbridge/rmgr/drv_interface.c static void bridge_recover(struct work_struct *work) · line 348
Quoted 2 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +2
-0+pr_info ("%s:%d handle(s) still opened\n" , __func__, +atomic_read (&bridge_cref));I remember the rule was to break lines as far to the right as possible, no? Chapter 2 CodingStyle, same for the other similar changes.
It doesn't mean you have to right justify things, it just means indented. The original code is fine here and the new code is fine here. It's up to whoever writes the code to decide.
I concur.
My personal preference is to use a new line after the format string if necessary.
ie:
pr_<level>("fmt\n"[, args to 80 columns if all fit])
or
pr_<level>("fmt\n",
args when single line exceeds 80 columns);So for this case:
pr_info("%s:%d handle(s) still opened\n",
__func__, atomic_read(&bridge_cref));I've done a patch here to tidspbridge that standardizes printk output.
Basically, the patch adds
#define pr_fmt(fmt) KBUILD_MODNAME "%s: ", __func__ to prefix "tidspbridge:%s:", removes the leading "%s:...", __func__ from the uses, coalesces formats and does argument alignment.
It cleans up the DBC_ASSERT, DBC_REQUIRE and DBC_ENSURE macros too.
I'm waiting for the Makefile change and whatever patches Víctor produces to be applied. I'll then redo my patch and submit it.
Re: [PATCH 3/5] staging: tidspbridge: Lindent to drv_interface.c
Quoted 29 lines · Joe Perches
Quoted 9 lines · Dan Carpenter
Quoted 5 lines · Omar
drivers/staging/tidspbridge/rmgr/drv_interface.c static void bridge_recover(struct work_struct *work) · line 348
Quoted 2 lines
drivers/staging/tidspbridge/rmgr/drv_interface.c +2
-0+pr_info ("%s:%d handle(s) still opened\n" , __func__, +atomic_read (&bridge_cref));I remember the rule was to break lines as far to the right as possible, no? Chapter 2 CodingStyle, same for the other similar changes.
It doesn't mean you have to right justify things, it just means indented. The original code is fine here and the new code is fine here. It's up to whoever writes the code to decide.
I concur.
My personal preference is to use a new line after the format string if necessary.
ie:
pr_<level>("fmt\n"[, args to 80 columns if all fit])
orpr_<level>("fmt\n", args when single line exceeds 80 columns);So for this case:
pr_info("%s:%d handle(s) still opened\n", __func__, atomic_read(&bridge_cref));I've done a patch here to tidspbridge that standardizes printk output.
Quoted 8 lines
Basically, the patch adds
#definepr_fmt(fmt)KBUILD_MODNAME"%s: ",__func__to prefix "tidspbridge:%s:", removes the leading "%s:...",__func__from the uses, coalesces formats and does argument alignment.It cleans up the
DBC_ASSERT,DBC_REQUIREandDBC_ENSUREmacros too.
hehehe... I also have one for this... But I prefer yours: I'm a newbie :)
vmjl
Quoted 7 lines
I'm waiting for the
Makefilechange and whatever patches Víctor produces to be applied. I'll then redo my patch and submit it.
Re: [PATCH 3/5] staging: tidspbridge: Lindent to drv_interface.c
Quoted 10 lines · Víctor M. Jáquez L.
Quoted 9 lines · Joe Perches
I've done a patch here to tidspbridge that standardizes printk output.
Basically, the patch adds
#definepr_fmt(fmt)KBUILD_MODNAME"%s: ",__func__to prefix "tidspbridge:%s:", removes the leading "%s:...",__func__from the uses, coalesces formats and does argument alignment.
Quoted 4 lines
Quoted 2 lines
It cleans up the
DBC_ASSERT,DBC_REQUIREandDBC_ENSUREmacros too.hehehe... I also have one for this... But I prefer yours: I'm a newbie :)
Well, given that you haven't done many of these, I prefer yours.
PATCH 4/5 staging: tidspbridge: silence the compiler
In reply to Víctor Manuel Jáquez LealSilence the warning when compiling drv_interface.c
--- drivers/staging/tidspbridge/rmgr/drv_interface.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/staging/tidspbridge/rmgr/drv_interface.c +3 -2
@@ -273,8 +273,9 @@ static int bridge_mmap(struct file *filp, struct vm_area_struct *vma) vma->vm_page_prot =pgprot_noncached (vma->vm_page_prot);dev_dbg (bridge,"%s: vm filp %p offset %x start %lx end %lx page_prot " -"%lx flags %lx\n" , __func__, filp, offset, - vma->vm_start, vma->vm_end, vma->vm_page_prot, vma->vm_flags); +"%ulx flags %lx\n" , __func__, filp, offset, + vma->vm_start, vma->vm_end, vma->vm_page_prot, + vma->vm_flags); status = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, vma->vm_end - vma->vm_start,
Signature · 1 line
1.7.8.3
Re: [PATCH 4/5] staging: tidspbridge: silence the compiler
2012/1/23 Víctor Manuel Jáquez Leal <[email protected]>:
Quoted 21 lines
Silence the warning when compiling
drv_interface.cSigned-off-by Víctor Manuel Jáquez Leal <[email protected]>---
drivers/staging/tidspbridge/rmgr/drv_interface.c | 5 +++--1 files changed, 3 insertions(+), 2 deletions(-)drivers/staging/tidspbridge/rmgr/drv_interface.c +0
-0@@ -273,8 +273,9 @@ static int bridge_mmap(struct file *filp, struct vm_area_struct *vma) vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);dev_dbg(bridge, "%s: vm filp %p offset %x start %lx end %lx
page_prot"
- "%lx flags %lx\n",__func__, filp, offset,
-vma->vm_start,vma->vm_end,vma->vm_page_prot,vma->vm_flags);
+ "%ulx flags %lx\n",__func__, filp, offset,
+vma->vm_start,vma->vm_end,vma->vm_page_prot,
+vma->vm_flags);
Quoted 5 lines
status = remap_pfn_range(vma,
vma->vm_start,vma->vm_pgoff,vma->vm_end-vma->vm_start, --
1.7.8.3
Looks good.
Regards,
Omar
PATCH 5/5 staging: tidspbridge: remove header inclusions
In reply to Víctor Manuel Jáquez Lealdrv_interface.c include several header files that are not really used.
--- drivers/staging/tidspbridge/rmgr/drv_interface.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
drivers/staging/tidspbridge/rmgr/drv_interface.c +0 -7
@@ -16,11 +16,8 @@* WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE. */ -/* ----------------------------------- Host OS */ -#include <plat/dsp.h> -#include <dspbridge/host_os.h> #include <linux/types.h> #include <linux/platform_device.h> #include <linux/pm.h>
@@ -38,10 +35,8 @@/* ----------------------------------- OS Adaptation Layer */ #include <dspbridge/clk.h> -#include <dspbridge/sync.h> /* ----------------------------------- Platform Manager */ -#include <dspbridge/dspapi-ioctl.h> #include <dspbridge/dspapi.h> #include <dspbridge/dspdrv.h>
@@ -49,10 +44,8 @@#include <dspbridge/pwr.h> #include <dspbridge/resourcecleanup.h> -#include <dspbridge/chnl.h> #include <dspbridge/proc.h> #include <dspbridge/dev.h> -#include <dspbridge/drv.h> #ifdef CONFIG_TIDSPBRIDGE_DVFS #include <mach-omap2/omap3-opp.h>
Signature · 1 line
1.7.8.3