public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: sep: Fix issues reported by checkpatch
@ 2014-07-19 17:34 LABBE Corentin
  2014-07-19 17:34 ` [PATCH 1/4] staging: sep: Solves some problems " LABBE Corentin
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: LABBE Corentin @ 2014-07-19 17:34 UTC (permalink / raw)
  To: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack
  Cc: devel, linux-kernel

Hello

The drivers/staging/sep/sep_main.c have lots of checkpatch issue.
This patch series solves them and since there are many, I have splitted corrections in 4 patch.

Note that this work is done for the Eudyptulla Challenge.

Best regards


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] staging: sep: Solves some problems reported by checkpatch
  2014-07-19 17:34 [PATCH] staging: sep: Fix issues reported by checkpatch LABBE Corentin
@ 2014-07-19 17:34 ` LABBE Corentin
  2014-07-21 19:19   ` Greg KH
  2014-07-19 17:34 ` [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch) LABBE Corentin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: LABBE Corentin @ 2014-07-19 17:34 UTC (permalink / raw)
  To: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack
  Cc: devel, linux-kernel, LABBE Corentin

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/staging/sep/sep_main.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 75ca15e..177e4b9 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -155,7 +155,7 @@ struct sep_queue_info *sep_queue_status_add(
 	unsigned long lck_flags;
 	struct sep_queue_info *my_elem = NULL;
 
-	my_elem = kzalloc(sizeof(struct sep_queue_info), GFP_KERNEL);
+	my_elem = kzalloc(sizeof(*my_elem), GFP_KERNEL);
 
 	if (!my_elem)
 		return NULL;
@@ -1006,8 +1006,8 @@ static int sep_crypto_dma(
 		return -ENOMEM;
 	}
 
-	sep_dma = kmalloc(sizeof(struct sep_dma_map) *
-		count_mapped, GFP_ATOMIC);
+	sep_dma = kmalloc_array(count_mapped, sizeof(struct sep_dma_map),
+			GFP_ATOMIC);
 
 	if (sep_dma == NULL) {
 		dev_dbg(&sep->pdev->dev, "Cannot allocate dma_maps\n");
@@ -1070,7 +1070,8 @@ static int sep_crypto_lli(
 
 	sep_map = *maps;
 
-	sep_lli = kmalloc(sizeof(struct sep_lli_entry) * nbr_ents, GFP_ATOMIC);
+	sep_lli = kmalloc_array(nbr_ents, sizeof(struct sep_lli_entry),
+			GFP_ATOMIC);
 
 	if (sep_lli == NULL) {
 		dev_dbg(&sep->pdev->dev, "Cannot allocate lli_maps\n");
@@ -3398,7 +3399,7 @@ static ssize_t sep_create_dcb_dmatables_context(struct sep_device *sep,
 	}
 
 	/* Allocate thread-specific memory for DCB */
-	*dcb_region = kzalloc(num_dcbs * sizeof(struct sep_dcblock),
+	*dcb_region = kcalloc(num_dcbs, sizeof(struct sep_dcblock),
 			      GFP_KERNEL);
 	if (!(*dcb_region)) {
 		error = -ENOMEM;
@@ -3480,7 +3481,7 @@ int sep_create_dcb_dmatables_context_kernel(struct sep_device *sep,
 		current->pid, num_dcbs);
 
 	/* Allocate thread-specific memory for DCB */
-	*dcb_region = kzalloc(num_dcbs * sizeof(struct sep_dcblock),
+	*dcb_region = kcalloc(num_dcbs, sizeof(struct sep_dcblock),
 			      GFP_KERNEL);
 	if (!(*dcb_region)) {
 		error = -ENOMEM;
@@ -4090,7 +4091,7 @@ static int sep_probe(struct pci_dev *pdev,
 	}
 
 	/* Allocate the sep_device structure for this device */
-	sep_dev = kzalloc(sizeof(struct sep_device), GFP_ATOMIC);
+	sep_dev = kzalloc(sizeof(*sep_dev), GFP_ATOMIC);
 	if (sep_dev == NULL) {
 		error = -ENOMEM;
 		goto end_function_disable_device;
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)
  2014-07-19 17:34 [PATCH] staging: sep: Fix issues reported by checkpatch LABBE Corentin
  2014-07-19 17:34 ` [PATCH 1/4] staging: sep: Solves some problems " LABBE Corentin
@ 2014-07-19 17:34 ` LABBE Corentin
  2014-07-19 18:00   ` Joe Perches
  2014-07-19 17:34 ` [PATCH 3/4] staging: sep: Fix misceanellous problems reported by checkpatch LABBE Corentin
  2014-07-19 17:34 ` [PATCH 4/4] staging: sep: Fix blank lines issue " LABBE Corentin
  3 siblings, 1 reply; 8+ messages in thread
From: LABBE Corentin @ 2014-07-19 17:34 UTC (permalink / raw)
  To: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack
  Cc: devel, linux-kernel, LABBE Corentin

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/staging/sep/sep_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 177e4b9..1580d95f 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
 				if (is_kva) {
 					error = -ENODEV;
 					break;
-				} else {
-					error_temp = copy_to_user(
+				}
+				error_temp = copy_to_user(
 						(void __user *)tail_pt,
 						dcb_table_ptr->tail_data,
 						dcb_table_ptr->tail_data_size);
-				}
 				if (error_temp) {
 					/* Release the DMA resource */
 					error = -EFAULT;
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] staging: sep: Fix misceanellous problems reported by checkpatch
  2014-07-19 17:34 [PATCH] staging: sep: Fix issues reported by checkpatch LABBE Corentin
  2014-07-19 17:34 ` [PATCH 1/4] staging: sep: Solves some problems " LABBE Corentin
  2014-07-19 17:34 ` [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch) LABBE Corentin
@ 2014-07-19 17:34 ` LABBE Corentin
  2014-07-19 17:34 ` [PATCH 4/4] staging: sep: Fix blank lines issue " LABBE Corentin
  3 siblings, 0 replies; 8+ messages in thread
From: LABBE Corentin @ 2014-07-19 17:34 UTC (permalink / raw)
  To: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack
  Cc: devel, linux-kernel, LABBE Corentin

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/staging/sep/sep_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 1580d95f..6f98881 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -130,7 +130,6 @@ void sep_queue_status_remove(struct sep_device *sep,
 
 	dev_dbg(&sep->pdev->dev, "[PID%d] sep_queue_status_remove return\n",
 		current->pid);
-	return;
 }
 
 /**
@@ -1737,7 +1736,7 @@ static void sep_debug_print_lli_tables(struct sep_device *sep,
 		return;
 	}
 
-	while ((unsigned long) lli_table_ptr->bus_address != 0xffffffff) {
+	while ((unsigned long)lli_table_ptr->bus_address != 0xffffffff) {
 		dev_dbg(&sep->pdev->dev,
 			"[PID%d] lli table %08lx, table_data_size is (hex) %lx\n",
 			current->pid, table_count, table_data_size);
@@ -1752,7 +1751,7 @@ static void sep_debug_print_lli_tables(struct sep_device *sep,
 			dev_dbg(&sep->pdev->dev,
 				"[PID%d] lli_table_ptr address is %08lx\n",
 				current->pid,
-				(unsigned long) lli_table_ptr);
+				(unsigned long)lli_table_ptr);
 
 			dev_dbg(&sep->pdev->dev,
 				"[PID%d] phys address is %08lx block size is (hex) %x\n",
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] staging: sep: Fix blank lines issue reported by checkpatch
  2014-07-19 17:34 [PATCH] staging: sep: Fix issues reported by checkpatch LABBE Corentin
                   ` (2 preceding siblings ...)
  2014-07-19 17:34 ` [PATCH 3/4] staging: sep: Fix misceanellous problems reported by checkpatch LABBE Corentin
@ 2014-07-19 17:34 ` LABBE Corentin
  3 siblings, 0 replies; 8+ messages in thread
From: LABBE Corentin @ 2014-07-19 17:34 UTC (permalink / raw)
  To: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack
  Cc: devel, linux-kernel, LABBE Corentin

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/staging/sep/sep_main.c | 47 ++++--------------------------------------
 1 file changed, 4 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 6f98881..89a1d53 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -492,7 +492,6 @@ int sep_free_dma_table_data_handler(struct sep_device *sep,
 		 * memory is not available to the o/s at all.
 		 */
 		if (!(*dma_ctx)->secure_dma && dma->out_map_array) {
-
 			for (count = 0; count < dma->out_num_pages; count++) {
 				dma_unmap_page(&sep->pdev->dev,
 					dma->out_map_array[count].dma_addr,
@@ -513,7 +512,6 @@ int sep_free_dma_table_data_handler(struct sep_device *sep,
 
 		/* Again, we do this only for non secure dma */
 		if (!(*dma_ctx)->secure_dma && dma->out_page_array) {
-
 			for (count = 0; count < dma->out_num_pages; count++) {
 				if (!PageReserved(dma->out_page_array[count]))
 
@@ -633,7 +631,6 @@ static int sep_end_transaction_handler(struct sep_device *sep,
 	return 0;
 }
 
-
 /**
  * sep_release - close a SEP device
  * @inode: inode of SEP device
@@ -766,7 +763,6 @@ static unsigned int sep_poll(struct file *filp, poll_table *wait)
 		goto end_function;
 	}
 
-
 	/* Add the event to the polling wait table */
 	dev_dbg(&sep->pdev->dev, "[PID%d] poll: calling wait sep_event\n",
 					current->pid);
@@ -856,7 +852,6 @@ static unsigned long sep_set_time(struct sep_device *sep)
 	struct timeval time;
 	u32 *time_addr;	/* Address of time as seen by the kernel */
 
-
 	do_gettimeofday(&time);
 
 	/* Set value in the SYSTEM MEMORY offset */
@@ -905,7 +900,6 @@ int sep_send_command_handler(struct sep_device *sep)
 	msg_pool += 1;
 	if ((*msg_pool < 2) ||
 		(*msg_pool > SEP_DRIVER_MAX_MESSAGE_SIZE_IN_BYTES)) {
-
 		dev_warn(&sep->pdev->dev, "invalid message size\n");
 		error = -EPROTO;
 		goto end_function;
@@ -1023,7 +1017,6 @@ static int sep_crypto_dma(
 
 	*dma_maps = sep_dma;
 	return count_mapped;
-
 }
 
 /**
@@ -1053,7 +1046,6 @@ static int sep_crypto_lli(
 	u32 data_size,
 	enum dma_data_direction direction)
 {
-
 	int ct1;
 	struct sep_lli_entry *sep_lli;
 	struct sep_dma_map *sep_map;
@@ -1318,7 +1310,6 @@ static int sep_lock_user_pages(struct sep_device *sep,
 			(unsigned long)lli_array[0].bus_address,
 			lli_array[0].block_size);
 
-
 	/* Check the size of the last page */
 	if (num_pages > 1) {
 		lli_array[num_pages - 1].block_size =
@@ -1663,7 +1654,6 @@ static void sep_build_lli_table(struct sep_device *sep,
 
 	/* Set the output parameter */
 	*num_processed_entries_ptr += array_counter;
-
 }
 
 /**
@@ -1747,7 +1737,6 @@ static void sep_debug_print_lli_tables(struct sep_device *sep,
 		/* Print entries of the table (without info entry) */
 		for (entries_count = 0; entries_count < num_table_entries;
 			entries_count++, lli_table_ptr++) {
-
 			dev_dbg(&sep->pdev->dev,
 				"[PID%d] lli_table_ptr address is %08lx\n",
 				current->pid,
@@ -1773,7 +1762,6 @@ static void sep_debug_print_lli_tables(struct sep_device *sep,
 			current->pid,
 			(unsigned long)lli_table_ptr->bus_address);
 
-
 		table_data_size = lli_table_ptr->block_size & 0xffffff;
 		num_table_entries = (lli_table_ptr->block_size >> 24) & 0xff;
 
@@ -1796,7 +1784,6 @@ static void sep_debug_print_lli_tables(struct sep_device *sep,
 #endif
 }
 
-
 /**
  * sep_prepare_empty_lli_table - create a blank LLI table
  * @sep: pointer to struct sep_device
@@ -1972,7 +1959,6 @@ static int sep_prepare_input_dma_table(struct sep_device *sep,
 
 	/* Loop till all the entries in in array are processed */
 	while (current_entry < sep_lli_entries) {
-
 		/* Set the new input and output tables */
 		in_lli_table_ptr =
 			(struct sep_lli_entry *)lli_table_alloc_addr;
@@ -1988,10 +1974,8 @@ static int sep_prepare_input_dma_table(struct sep_device *sep,
 			((void *)sep->shared_addr +
 			SYNCHRONIC_DMA_TABLES_AREA_OFFSET_BYTES +
 			SYNCHRONIC_DMA_TABLES_AREA_SIZE_BYTES)) {
-
 			error = -ENOMEM;
 			goto end_function_error;
-
 		}
 
 		/* Update the number of created tables */
@@ -2022,7 +2006,6 @@ static int sep_prepare_input_dma_table(struct sep_device *sep,
 			&current_entry, &num_entries_in_table, table_data_size);
 
 		if (info_entry_ptr == NULL) {
-
 			/* Set the output parameters to physical addresses */
 			*lli_table_ptr = sep_shared_area_virt_to_bus(sep,
 				dma_in_lli_table_ptr);
@@ -2071,7 +2054,6 @@ end_function_error:
 
 end_function:
 	return error;
-
 }
 
 /**
@@ -2193,7 +2175,6 @@ static int sep_construct_dma_tables_from_lli(
 			((void *)sep->shared_addr +
 			SYNCHRONIC_DMA_TABLES_AREA_OFFSET_BYTES +
 			SYNCHRONIC_DMA_TABLES_AREA_SIZE_BYTES)) {
-
 			dev_warn(&sep->pdev->dev, "dma table limit overrun\n");
 			return -ENOMEM;
 		}
@@ -2450,10 +2431,7 @@ static int sep_prepare_input_output_dma_table(struct sep_device *sep,
 
 			goto end_function_free_lli_in;
 		}
-
-	}
-
-	else {
+	} else {
 		dev_dbg(&sep->pdev->dev, "[PID%d] Locking user input pages\n",
 						current->pid);
 		error = sep_lock_user_pages(sep, app_virt_in_addr,
@@ -2554,7 +2532,6 @@ end_function_with_error:
 	dma_ctx->dma_res_arr[dma_ctx->nr_dcb_creat].out_page_array = NULL;
 	kfree(lli_out_array);
 
-
 end_function_free_lli_in:
 	kfree(dma_ctx->dma_res_arr[dma_ctx->nr_dcb_creat].in_map_array);
 	dma_ctx->dma_res_arr[dma_ctx->nr_dcb_creat].in_map_array = NULL;
@@ -2563,9 +2540,7 @@ end_function_free_lli_in:
 	kfree(lli_in_array);
 
 end_function:
-
 	return error;
-
 }
 
 /**
@@ -2703,7 +2678,6 @@ int sep_prepare_input_output_dma_table_in_dcb(struct sep_device *sep,
 	dcb_table_ptr->out_vr_tail_pt = 0;
 
 	if (isapplet) {
-
 		/* Check if there is enough data for DMA operation */
 		if (data_in_size < SEP_DRIVER_MIN_DATA_SIZE_PER_TABLE) {
 			if (is_kva) {
@@ -2829,10 +2803,8 @@ end_function_error:
 
 end_function:
 	return error;
-
 }
 
-
 /**
  * sep_free_dma_tables_and_dcb - free DMA tables and DCBs
  * @sep: pointer to struct sep_device
@@ -2963,7 +2935,6 @@ static int sep_prepare_dcb_handler(struct sep_device *sep, unsigned long arg,
 
 end_function:
 	return error;
-
 }
 
 /**
@@ -3167,7 +3138,6 @@ static irqreturn_t sep_inthandler(int irq, void *dev_id)
 	dev_dbg(&sep->pdev->dev, "sep int: IRR REG val: %x\n", reg_val);
 
 	if (reg_val & (0x1 << 13)) {
-
 		/* Lock and update the counter of reply messages */
 		spin_lock_irqsave(&sep->snd_rply_lck, lock_irq_flag);
 		sep->reply_ct++;
@@ -3244,8 +3214,9 @@ static int sep_reconfig_shared_area(struct sep_device *sep)
 		dev_warn(&sep->pdev->dev, "could not reconfig shared area\n");
 		dev_warn(&sep->pdev->dev, "result was %x\n", ret_val);
 		ret_val = -ENOMEM;
-	} else
+	} else {
 		ret_val = 0;
+	}
 
 	dev_dbg(&sep->pdev->dev, "reconfig shared area end\n");
 
@@ -3432,7 +3403,6 @@ static ssize_t sep_create_dcb_dmatables_context(struct sep_device *sep,
 end_function:
 	kfree(dcb_args);
 	return error;
-
 }
 
 /**
@@ -3511,7 +3481,6 @@ int sep_create_dcb_dmatables_context_kernel(struct sep_device *sep,
 
 end_function:
 	return error;
-
 }
 
 /**
@@ -3590,7 +3559,6 @@ end_function:
 	return error;
 }
 
-
 /**
  *	sep_read - Returns results of an operation for fastcall interface
  *	@filp: File pointer
@@ -3638,7 +3606,6 @@ static ssize_t sep_read(struct file *filp,
 		goto end_function_error;
 	}
 
-
 	/* Wait for SEP to finish */
 	wait_event(sep->event_interrupt,
 		   test_bit(SEP_WORKING_LOCK_BIT,
@@ -3711,7 +3678,6 @@ static inline ssize_t sep_fastcall_args_get(struct sep_device *sep,
 		goto end_function;
 	}
 
-
 	if (copy_from_user(args, buf_user, sizeof(struct sep_fastcall_hdr))) {
 		error = -EFAULT;
 		goto end_function;
@@ -3813,7 +3779,6 @@ static ssize_t sep_write(struct file *filp,
 		goto end_function_error;
 	}
 
-
 	/*
 	 * Prepare contents of the shared area regions for
 	 * the operation into temporary buffers
@@ -3923,6 +3888,7 @@ end_function:
 
 	return error;
 }
+
 /**
  *	sep_seek - Handler for seek system call
  *	@filp: File pointer
@@ -3937,8 +3903,6 @@ static loff_t sep_seek(struct file *filp, loff_t offset, int origin)
 	return -ENOSYS;
 }
 
-
-
 /**
  * sep_file_operations - file operation on sep device
  * @sep_ioctl:	ioctl handler from user space call
@@ -3991,7 +3955,6 @@ sep_sysfs_read(struct file *filp, struct kobject *kobj,
 	if (queue_num > SEP_DOUBLEBUF_USERS_LIMIT)
 		queue_num = SEP_DOUBLEBUF_USERS_LIMIT;
 
-
 	if (count < sizeof(queue_num)
 			+ (queue_num * sizeof(struct sep_queue_data))) {
 		spin_unlock_irqrestore(&sep->sep_queue_lock, lck_flags);
@@ -4061,7 +4024,6 @@ static int sep_register_driver_with_fs(struct sep_device *sep)
 	return ret_val;
 }
 
-
 /**
  *sep_probe - probe a matching PCI device
  *@pdev:	pci_device
@@ -4352,7 +4314,6 @@ static int sep_pci_suspend(struct device *dev)
  */
 static int sep_pm_runtime_resume(struct device *dev)
 {
-
 	u32 retval2;
 	u32 delay_count;
 	struct sep_device *sep = sep_dev;
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)
  2014-07-19 17:34 ` [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch) LABBE Corentin
@ 2014-07-19 18:00   ` Joe Perches
  2014-07-20  6:39     ` Corentin LABBE
  0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2014-07-19 18:00 UTC (permalink / raw)
  To: LABBE Corentin, mark.a.allyn, jayant.mangalampalli
  Cc: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack, devel, linux-kernel

(Adding Mark Allyn and Jayant Mangalampalli)

Is this still project still active?

On Sat, 2014-07-19 at 19:34 +0200, LABBE Corentin wrote:
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  drivers/staging/sep/sep_main.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
> index 177e4b9..1580d95f 100644
> --- a/drivers/staging/sep/sep_main.c
> +++ b/drivers/staging/sep/sep_main.c
> @@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
>  				if (is_kva) {
>  					error = -ENODEV;
>  					break;
> -				} else {
> -					error_temp = copy_to_user(
> +				}
> +				error_temp = copy_to_user(
>  						(void __user *)tail_pt,
>  						dcb_table_ptr->tail_data,
>  						dcb_table_ptr->tail_data_size);
> -				}
>  				if (error_temp) {
>  					/* Release the DMA resource */
>  					error = -EFAULT;

It'd be probably be better to rewrite the code to unindent
a level by using continue.  Something like below:

btw:

the is_kva test looks very odd and should probably be
moved outside the loop.

pt_hold should probably be void * not unsigned long
as it loses high order bits on x86-32.

definition:
	aligned_u64 out_vr_tail_pt;
use:
+			pt_hold = (unsigned long)dcb_table_ptr->
+				out_vr_tail_pt;

---
 drivers/staging/sep/sep_main.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 75ca15e..24b4a54 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -2871,26 +2871,23 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
 		 * Go over each DCB and see if
 		 * tail pointer must be updated
 		 */
-		for (i = 0; i < (*dma_ctx)->nr_dcb_creat;
-		     i++, dcb_table_ptr++) {
-			if (dcb_table_ptr->out_vr_tail_pt) {
-				pt_hold = (unsigned long)dcb_table_ptr->
-					out_vr_tail_pt;
-				tail_pt = (void *)pt_hold;
-				if (is_kva) {
-					error = -ENODEV;
-					break;
-				} else {
-					error_temp = copy_to_user(
-						(void __user *)tail_pt,
-						dcb_table_ptr->tail_data,
-						dcb_table_ptr->tail_data_size);
-				}
-				if (error_temp) {
-					/* Release the DMA resource */
-					error = -EFAULT;
-					break;
-				}
+		for (i = 0; i < (*dma_ctx)->nr_dcb_creat; i++, dcb_table_ptr++) {
+			if (!dcb_table_ptr->out_vr_tail_pt)
+				continue;
+			pt_hold = (unsigned long)dcb_table_ptr->
+				out_vr_tail_pt;
+			tail_pt = (void *)pt_hold;
+			if (is_kva) {
+				error = -ENODEV;
+				break;
+			}
+			error_temp = copy_to_user((void __user *)tail_pt,
+						  dcb_table_ptr->tail_data,
+						  dcb_table_ptr->tail_data_size);
+			if (error_temp) {
+				/* Release the DMA resource */
+				error = -EFAULT;
+				break;
 			}
 		}
 	}



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)
  2014-07-19 18:00   ` Joe Perches
@ 2014-07-20  6:39     ` Corentin LABBE
  0 siblings, 0 replies; 8+ messages in thread
From: Corentin LABBE @ 2014-07-20  6:39 UTC (permalink / raw)
  To: Joe Perches, mark.a.allyn, jayant.mangalampalli
  Cc: gregkh, monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack, devel, linux-kernel

Le 19/07/2014 20:00, Joe Perches a écrit :
> (Adding Mark Allyn and Jayant Mangalampalli)
> 
> Is this still project still active?

I do not know

> 
> On Sat, 2014-07-19 at 19:34 +0200, LABBE Corentin wrote:
>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>> ---
>>  drivers/staging/sep/sep_main.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
>> index 177e4b9..1580d95f 100644
>> --- a/drivers/staging/sep/sep_main.c
>> +++ b/drivers/staging/sep/sep_main.c
>> @@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
>>  				if (is_kva) {
>>  					error = -ENODEV;
>>  					break;
>> -				} else {
>> -					error_temp = copy_to_user(
>> +				}
>> +				error_temp = copy_to_user(
>>  						(void __user *)tail_pt,
>>  						dcb_table_ptr->tail_data,
>>  						dcb_table_ptr->tail_data_size);
>> -				}
>>  				if (error_temp) {
>>  					/* Release the DMA resource */
>>  					error = -EFAULT;
> 
> It'd be probably be better to rewrite the code to unindent
> a level by using continue.  Something like below:
> 
> btw:
> 
> the is_kva test looks very odd and should probably be
> moved outside the loop.
> 
> pt_hold should probably be void * not unsigned long
> as it loses high order bits on x86-32.
> 
> definition:
> 	aligned_u64 out_vr_tail_pt;
> use:
> +			pt_hold = (unsigned long)dcb_table_ptr->
> +				out_vr_tail_pt;
> 

As I said in the introduction email, I have done thoses patch for the Eudyptula challenge,
since I have not the hardware needed by this driver I cannot modify beyond simple style changes without testing

Regards


> ---
>  drivers/staging/sep/sep_main.c | 37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
> index 75ca15e..24b4a54 100644
> --- a/drivers/staging/sep/sep_main.c
> +++ b/drivers/staging/sep/sep_main.c
> @@ -2871,26 +2871,23 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
>  		 * Go over each DCB and see if
>  		 * tail pointer must be updated
>  		 */
> -		for (i = 0; i < (*dma_ctx)->nr_dcb_creat;
> -		     i++, dcb_table_ptr++) {
> -			if (dcb_table_ptr->out_vr_tail_pt) {
> -				pt_hold = (unsigned long)dcb_table_ptr->
> -					out_vr_tail_pt;
> -				tail_pt = (void *)pt_hold;
> -				if (is_kva) {
> -					error = -ENODEV;
> -					break;
> -				} else {
> -					error_temp = copy_to_user(
> -						(void __user *)tail_pt,
> -						dcb_table_ptr->tail_data,
> -						dcb_table_ptr->tail_data_size);
> -				}
> -				if (error_temp) {
> -					/* Release the DMA resource */
> -					error = -EFAULT;
> -					break;
> -				}
> +		for (i = 0; i < (*dma_ctx)->nr_dcb_creat; i++, dcb_table_ptr++) {
> +			if (!dcb_table_ptr->out_vr_tail_pt)
> +				continue;
> +			pt_hold = (unsigned long)dcb_table_ptr->
> +				out_vr_tail_pt;
> +			tail_pt = (void *)pt_hold;
> +			if (is_kva) {
> +				error = -ENODEV;
> +				break;
> +			}
> +			error_temp = copy_to_user((void __user *)tail_pt,
> +						  dcb_table_ptr->tail_data,
> +						  dcb_table_ptr->tail_data_size);
> +			if (error_temp) {
> +				/* Release the DMA resource */
> +				error = -EFAULT;
> +				break;
>  			}
>  		}
>  	}
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] staging: sep: Solves some problems reported by checkpatch
  2014-07-19 17:34 ` [PATCH 1/4] staging: sep: Solves some problems " LABBE Corentin
@ 2014-07-21 19:19   ` Greg KH
  0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2014-07-21 19:19 UTC (permalink / raw)
  To: LABBE Corentin
  Cc: monamagarwal123, paul.gortmaker, jg1.han, paulmck,
	valentina.manea.m, jack, devel, linux-kernel

On Sat, Jul 19, 2014 at 07:34:39PM +0200, LABBE Corentin wrote:
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  drivers/staging/sep/sep_main.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

_which_ specific problems are fixed here?

Please be specific and provide a valid changelog entry in the body of
the email, changelogs without any information other than the subject:
are generally frowned apon.

Can you please fix this up and resend?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-07-21 19:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-19 17:34 [PATCH] staging: sep: Fix issues reported by checkpatch LABBE Corentin
2014-07-19 17:34 ` [PATCH 1/4] staging: sep: Solves some problems " LABBE Corentin
2014-07-21 19:19   ` Greg KH
2014-07-19 17:34 ` [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch) LABBE Corentin
2014-07-19 18:00   ` Joe Perches
2014-07-20  6:39     ` Corentin LABBE
2014-07-19 17:34 ` [PATCH 3/4] staging: sep: Fix misceanellous problems reported by checkpatch LABBE Corentin
2014-07-19 17:34 ` [PATCH 4/4] staging: sep: Fix blank lines issue " LABBE Corentin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox