Skip to contentSkip to conversation

staging: vchiq_arm: replace sleep() with usleep_range()

PATCH In review Gaston Gonzalez 5 participants 13 messages Sep 14, 2021 – Sep 15, 2021

Subsystems (2)
Staging Broadcom BCM2711/BCM2835 Arm architecture
lore
Share & tools
lore

Gaston Gonzalez

author
#0 lore raw

PATCH 1/8 staging: vchiq_arm: replace sleep() with usleep_range()

usleep_range() should be used instead of sleep() when sleepings range from 10 us to 20 ms, [1].

Reported by checkpatch.pl

[1] Documentation/timers/timers-howto.txt

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +2 -2
index b25369a13452..0214ae37e01f 100644
@@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size,
 		if (status != VCHIQ_RETRY)
 			break;
 
-		msleep(1);
+		usleep_range(1000, 1100);
 	}
 
 	return status;

@@ -861,7 +861,7 @@ enum vchiq_status vchiq_bulk_receive(unsigned int handle, void *data,
 		if (status != VCHIQ_RETRY)
 			break;
 
-		msleep(1);
+		usleep_range(1000, 1100);
 	}
 
 	return status;
Signature · 1 line
2.33.0
author
#1 lore raw

PATCH 2/8 staging: vchiq_arm: cleanup code alignment issues

Fix code alignment issues.

Reported by checkpatch.pl

---
 .../interface/vchiq_arm/vchiq_arm.c           | 208 +++++++-----------
 1 file changed, 79 insertions(+), 129 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +79 -129
index 0214ae37e01f..435f1bd68ba3 100644
@@ -115,7 +115,7 @@ static DEFINE_SEMAPHORE(g_free_fragments_mutex);
 
 static enum vchiq_status
 vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
-	unsigned int size, enum vchiq_bulk_dir dir);
+			     unsigned int size, enum vchiq_bulk_dir dir);
 
 static irqreturn_t
 vchiq_doorbell_irq(int irq, void *dev_id)

@@ -251,11 +251,8 @@ create_pagelist(char *buf, char __user *ubuf,
 		}
 		/* do not try and release vmalloc pages */
 	} else {
-		actual_pages = pin_user_pages_fast(
-					  (unsigned long)ubuf & PAGE_MASK,
-					  num_pages,
-					  type == PAGELIST_READ,
-					  pages);
+		actual_pages = pin_user_pages_fast((unsigned long)ubuf & PAGE_MASK, num_pages,
+						   type == PAGELIST_READ, pages);
 
 		if (actual_pages != num_pages) {
 			vchiq_log_info(vchiq_arm_log_level,

@@ -325,9 +322,9 @@ create_pagelist(char *buf, char __user *ubuf,
 
 	/* Partial cache lines (fragments) require special measures */
 	if ((type == PAGELIST_READ) &&
-		((pagelist->offset & (g_cache_line_size - 1)) ||
-		((pagelist->offset + pagelist->length) &
-		(g_cache_line_size - 1)))) {
+	    ((pagelist->offset & (g_cache_line_size - 1)) ||
+	    ((pagelist->offset + pagelist->length) &
+	    (g_cache_line_size - 1)))) {
 		char *fragments;
 
 		if (down_interruptible(&g_free_fragments_sema)) {

@@ -391,7 +388,7 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
 			kunmap(pages[0]);
 		}
 		if ((actual >= 0) && (head_bytes < actual) &&
-			(tail_bytes != 0)) {
+		    (tail_bytes != 0)) {
 			memcpy((char *)kmap(pages[num_pages - 1]) +
 				((pagelist->offset + actual) &
 				(PAGE_SIZE - 1) & ~(g_cache_line_size - 1)),

@@ -504,9 +501,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 	}
 
 	g_dev = dev;
-	vchiq_log_info(vchiq_arm_log_level,
-		"vchiq_init - done (slots %pK, phys %pad)",
-		vchiq_slot_zero, &slot_phys);
+	vchiq_log_info(vchiq_arm_log_level, "vchiq_init - done (slots %pK, phys %pad)",
+		       vchiq_slot_zero, &slot_phys);
 
 	vchiq_call_connected_callbacks();
 

@@ -593,8 +589,7 @@ int vchiq_dump_platform_state(void *dump_context)
 	char buf[80];
 	int len;
 
-	len = snprintf(buf, sizeof(buf),
-		"  Platform: 2835 (VC master)");
+	len = snprintf(buf, sizeof(buf), "  Platform: 2835 (VC master)");
 	return vchiq_dump(dump_context, buf, len + 1);
 }
 

@@ -617,20 +612,18 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
 		usleep_range(500, 600);
 	}
 	if (i == VCHIQ_INIT_RETRIES) {
-		vchiq_log_error(vchiq_core_log_level,
-			"%s: videocore not initialized\n", __func__);
+		vchiq_log_error(vchiq_core_log_level, "%s: videocore not initialized\n", __func__);
 		ret = -ENOTCONN;
 		goto failed;
 	} else if (i > 0) {
 		vchiq_log_warning(vchiq_core_log_level,
-			"%s: videocore initialized after %d retries\n",
-			__func__, i);
+				  "%s: videocore initialized after %d retries\n", __func__, i);
 	}
 
 	instance = kzalloc(sizeof(*instance), GFP_KERNEL);
 	if (!instance) {
 		vchiq_log_error(vchiq_core_log_level,
-			"%s: error allocating vchiq instance\n", __func__);
+				"%s: error allocating vchiq instance\n", __func__);
 		ret = -ENOMEM;
 		goto failed;
 	}

@@ -645,8 +638,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
 	ret = 0;
 
 failed:
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p): returning %d", __func__, instance, ret);
+	vchiq_log_trace(vchiq_core_log_level, "%s(%p): returning %d", __func__, instance, ret);
 
 	return ret;
 }

@@ -659,9 +651,8 @@ void free_bulk_waiter(struct vchiq_instance *instance)
 	list_for_each_entry_safe(waiter, next,
 				 &instance->bulk_waiter_list, list) {
 		list_del(&waiter->list);
-		vchiq_log_info(vchiq_arm_log_level,
-				"bulk_waiter - cleaned up %pK for pid %d",
-				waiter, waiter->pid);
+		vchiq_log_info(vchiq_arm_log_level, "bulk_waiter - cleaned up %pK for pid %d",
+			       waiter, waiter->pid);
 		kfree(waiter);
 	}
 }

@@ -679,8 +670,7 @@ enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance)
 
 	mutex_unlock(&state->mutex);
 
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p): returning %d", __func__, instance, status);
+	vchiq_log_trace(vchiq_core_log_level, "%s(%p): returning %d", __func__, instance, status);
 
 	free_bulk_waiter(instance);
 	kfree(instance);

@@ -700,8 +690,7 @@ enum vchiq_status vchiq_connect(struct vchiq_instance *instance)
 	struct vchiq_state *state = instance->state;
 
 	if (mutex_lock_killable(&state->mutex)) {
-		vchiq_log_trace(vchiq_core_log_level,
-			"%s: call to mutex_lock failed", __func__);
+		vchiq_log_trace(vchiq_core_log_level, "%s: call to mutex_lock failed", __func__);
 		status = VCHIQ_RETRY;
 		goto failed;
 	}

@@ -713,8 +702,7 @@ enum vchiq_status vchiq_connect(struct vchiq_instance *instance)
 	mutex_unlock(&state->mutex);
 
 failed:
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p): returning %d", __func__, instance, status);
+	vchiq_log_trace(vchiq_core_log_level, "%s(%p): returning %d", __func__, instance, status);
 
 	return status;
 }

@@ -736,12 +724,7 @@ vchiq_add_service(struct vchiq_instance *instance,
 		? VCHIQ_SRVSTATE_LISTENING
 		: VCHIQ_SRVSTATE_HIDDEN;
 
-	service = vchiq_add_service_internal(
-		state,
-		params,
-		srvstate,
-		instance,
-		NULL);
+	service = vchiq_add_service_internal(state, params, srvstate, instance, NULL);
 
 	if (service) {
 		*phandle = service->handle;

@@ -750,8 +733,7 @@ vchiq_add_service(struct vchiq_instance *instance,
 		status = VCHIQ_ERROR;
 	}
 
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p): returning %d", __func__, instance, status);
+	vchiq_log_trace(vchiq_core_log_level, "%s(%p): returning %d", __func__, instance, status);
 
 	return status;
 }

@@ -770,11 +752,7 @@ vchiq_open_service(struct vchiq_instance *instance,
 	if (!vchiq_is_connected(instance))
 		goto failed;
 
-	service = vchiq_add_service_internal(state,
-		params,
-		VCHIQ_SRVSTATE_OPENING,
-		instance,
-		NULL);
+	service = vchiq_add_service_internal(state, params, VCHIQ_SRVSTATE_OPENING, instance, NULL);
 
 	if (service) {
 		*phandle = service->handle;

@@ -786,8 +764,7 @@ vchiq_open_service(struct vchiq_instance *instance,
 	}
 
 failed:
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p): returning %d", __func__, instance, status);
+	vchiq_log_trace(vchiq_core_log_level, "%s(%p): returning %d", __func__, instance, status);
 
 	return status;
 }

@@ -809,8 +786,8 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size,
 						     VCHIQ_BULK_TRANSMIT);
 			break;
 		case VCHIQ_BULK_MODE_BLOCKING:
-			status = vchiq_blocking_bulk_transfer(handle,
-				(void *)data, size, VCHIQ_BULK_TRANSMIT);
+			status = vchiq_blocking_bulk_transfer(handle, (void *)data, size,
+							      VCHIQ_BULK_TRANSMIT);
 			break;
 		default:
 			return VCHIQ_ERROR;

@@ -846,8 +823,8 @@ enum vchiq_status vchiq_bulk_receive(unsigned int handle, void *data,
 						     mode, VCHIQ_BULK_RECEIVE);
 			break;
 		case VCHIQ_BULK_MODE_BLOCKING:
-			status = vchiq_blocking_bulk_transfer(handle,
-				(void *)data, size, VCHIQ_BULK_RECEIVE);
+			status = vchiq_blocking_bulk_transfer(handle, (void *)data, size,
+							      VCHIQ_BULK_RECEIVE);
 			break;
 		default:
 			return VCHIQ_ERROR;

@@ -902,8 +879,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data, unsigned int size,
 		if (bulk) {
 			/* This thread has an outstanding bulk transfer. */
 			/* FIXME: why compare a dma address to a pointer? */
-			if ((bulk->data != (dma_addr_t)(uintptr_t)data) ||
-				(bulk->size != size)) {
+			if ((bulk->data != (dma_addr_t)(uintptr_t)data) || (bulk->size != size)) {
 				/*
 				 * This is not a retry of the previous one.
 				 * Cancel the signal when the transfer completes.

@@ -916,8 +892,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data, unsigned int size,
 	} else {
 		waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
 		if (!waiter) {
-			vchiq_log_error(vchiq_core_log_level,
-				"%s - out of memory", __func__);
+			vchiq_log_error(vchiq_core_log_level, "%s - out of memory", __func__);
 			return VCHIQ_ERROR;
 		}
 	}

@@ -925,8 +900,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data, unsigned int size,
 	status = vchiq_bulk_transfer(handle, data, NULL, size,
 				     &waiter->bulk_waiter,
 				     VCHIQ_BULK_MODE_BLOCKING, dir);
-	if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
-		!waiter->bulk_waiter.bulk) {
+	if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
 		struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
 
 		if (bulk) {

@@ -941,9 +915,8 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data, unsigned int size,
 		mutex_lock(&instance->bulk_waiter_list_mutex);
 		list_add(&waiter->list, &instance->bulk_waiter_list);
 		mutex_unlock(&instance->bulk_waiter_list_mutex);
-		vchiq_log_info(vchiq_arm_log_level,
-				"saved bulk_waiter %pK for pid %d",
-				waiter, current->pid);
+		vchiq_log_info(vchiq_arm_log_level, "saved bulk_waiter %pK for pid %d", waiter,
+			       current->pid);
 	}
 
 	return status;

@@ -963,17 +936,13 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
 	while ((insert - instance->completion_remove) >= MAX_COMPLETIONS) {
 		/* Out of space - wait for the client */
 		DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-		vchiq_log_trace(vchiq_arm_log_level,
-			"%s - completion queue full", __func__);
+		vchiq_log_trace(vchiq_arm_log_level, "%s - completion queue full", __func__);
 		DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
-		if (wait_for_completion_interruptible(
-					&instance->remove_event)) {
-			vchiq_log_info(vchiq_arm_log_level,
-				"service_callback interrupted");
+		if (wait_for_completion_interruptible(&instance->remove_event)) {
+			vchiq_log_info(vchiq_arm_log_level, "service_callback interrupted");
 			return VCHIQ_RETRY;
 		} else if (instance->closing) {
-			vchiq_log_info(vchiq_arm_log_level,
-				"service_callback closing");
+			vchiq_log_info(vchiq_arm_log_level, "service_callback closing");
 			return VCHIQ_SUCCESS;
 		}
 		DEBUG_TRACE(SERVICE_CALLBACK_LINE);

@@ -1044,11 +1013,10 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 		return VCHIQ_SUCCESS;
 
 	vchiq_log_trace(vchiq_arm_log_level,
-		"%s - service %lx(%d,%p), reason %d, header %lx, instance %lx, bulk_userdata %lx",
-		__func__, (unsigned long)user_service,
-		service->localport, user_service->userdata,
-		reason, (unsigned long)header,
-		(unsigned long)instance, (unsigned long)bulk_userdata);
+			"%s - service %lx(%d,%p), reason %d, header %lx, instance %lx, bulk_userdata %lx",
+			__func__, (unsigned long)user_service, service->localport,
+			user_service->userdata, reason, (unsigned long)header,
+			(unsigned long)instance, (unsigned long)bulk_userdata);
 
 	if (header && user_service->is_vchi) {
 		spin_lock(&msg_queue_spinlock);

@@ -1057,8 +1025,7 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 			spin_unlock(&msg_queue_spinlock);
 			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 			DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
-			vchiq_log_trace(vchiq_arm_log_level,
-				"service_callback - msg queue full");
+			vchiq_log_trace(vchiq_arm_log_level, "service_callback - msg queue full");
 			/*
 			 * If there is no MESSAGE_AVAILABLE in the completion
 			 * queue, add one

@@ -1068,10 +1035,10 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 				enum vchiq_status status;
 
 				vchiq_log_info(vchiq_arm_log_level,
-					"Inserting extra MESSAGE_AVAILABLE");
+					       "Inserting extra MESSAGE_AVAILABLE");
 				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-				status = add_completion(instance, reason,
-					NULL, user_service, bulk_userdata);
+				status = add_completion(instance, reason, NULL, user_service,
+							bulk_userdata);
 				if (status != VCHIQ_SUCCESS) {
 					DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 					return status;

@@ -1079,15 +1046,12 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 			}
 
 			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-			if (wait_for_completion_interruptible(
-						&user_service->remove_event)) {
-				vchiq_log_info(vchiq_arm_log_level,
-					"%s interrupted", __func__);
+			if (wait_for_completion_interruptible(&user_service->remove_event)) {
+				vchiq_log_info(vchiq_arm_log_level, "%s interrupted", __func__);
 				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 				return VCHIQ_RETRY;
 			} else if (instance->closing) {
-				vchiq_log_info(vchiq_arm_log_level,
-					"%s closing", __func__);
+				vchiq_log_info(vchiq_arm_log_level, "%s closing", __func__);
 				DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 				return VCHIQ_ERROR;
 			}

@@ -1238,12 +1202,10 @@ int vchiq_dump_platform_service_state(void *dump_context,
 
 	len = scnprintf(buf, sizeof(buf), "  instance %pK", service->instance);
 
-	if ((service->base.callback == service_callback) &&
-		user_service->is_vchi) {
-		len += scnprintf(buf + len, sizeof(buf) - len,
-			", %d/%d messages",
-			user_service->msg_insert - user_service->msg_remove,
-			MSG_QUEUE_SIZE);
+	if ((service->base.callback == service_callback) && user_service->is_vchi) {
+		len += scnprintf(buf + len, sizeof(buf) - len, ", %d/%d messages",
+				 user_service->msg_insert - user_service->msg_remove,
+				 MSG_QUEUE_SIZE);
 
 		if (user_service->dequeue_pending)
 			len += scnprintf(buf + len, sizeof(buf) - len,

@@ -1276,8 +1238,7 @@ vchiq_keepalive_vchiq_callback(enum vchiq_reason reason,
 			       struct vchiq_header *header,
 			       unsigned int service_user, void *bulk_user)
 {
-	vchiq_log_error(vchiq_susp_log_level,
-		"%s callback reason %d", __func__, reason);
+	vchiq_log_error(vchiq_susp_log_level, "%s callback reason %d", __func__, reason);
 	return 0;
 }
 

@@ -1301,22 +1262,22 @@ vchiq_keepalive_thread_func(void *v)
 
 	ret = vchiq_initialise(&instance);
 	if (ret) {
-		vchiq_log_error(vchiq_susp_log_level,
-			"%s vchiq_initialise failed %d", __func__, ret);
+		vchiq_log_error(vchiq_susp_log_level, "%s vchiq_initialise failed %d", __func__,
+				ret);
 		goto exit;
 	}
 
 	status = vchiq_connect(instance);
 	if (status != VCHIQ_SUCCESS) {
-		vchiq_log_error(vchiq_susp_log_level,
-			"%s vchiq_connect failed %d", __func__, status);
+		vchiq_log_error(vchiq_susp_log_level, "%s vchiq_connect failed %d", __func__,
+				status);
 		goto shutdown;
 	}
 
 	status = vchiq_add_service(instance, &params, &ka_handle);
 	if (status != VCHIQ_SUCCESS) {
-		vchiq_log_error(vchiq_susp_log_level,
-			"%s vchiq_open_service failed %d", __func__, status);
+		vchiq_log_error(vchiq_susp_log_level, "%s vchiq_open_service failed %d", __func__,
+				status);
 		goto shutdown;
 	}
 

@@ -1324,8 +1285,7 @@ vchiq_keepalive_thread_func(void *v)
 		long rc = 0, uc = 0;
 
 		if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
-			vchiq_log_error(vchiq_susp_log_level,
-				"%s interrupted", __func__);
+			vchiq_log_error(vchiq_susp_log_level, "%s interrupted", __func__);
 			flush_signals(current);
 			continue;
 		}

@@ -1346,16 +1306,15 @@ vchiq_keepalive_thread_func(void *v)
 			status = vchiq_use_service(ka_handle);
 			if (status != VCHIQ_SUCCESS) {
 				vchiq_log_error(vchiq_susp_log_level,
-					"%s vchiq_use_service error %d",
-					__func__, status);
+						"%s vchiq_use_service error %d", __func__, status);
 			}
 		}
 		while (rc--) {
 			status = vchiq_release_service(ka_handle);
 			if (status != VCHIQ_SUCCESS) {
 				vchiq_log_error(vchiq_susp_log_level,
-					"%s vchiq_release_service error %d",
-					__func__, status);
+						"%s vchiq_release_service error %d", __func__,
+						status);
 			}
 		}
 	}

@@ -1417,9 +1376,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
 	local_uc = ++arm_state->videocore_use_count;
 	++(*entity_uc);
 
-	vchiq_log_trace(vchiq_susp_log_level,
-		"%s %s count %d, state count %d",
-		__func__, entity, *entity_uc, local_uc);
+	vchiq_log_trace(vchiq_susp_log_level, "%s %s count %d, state count %d", __func__, entity,
+			*entity_uc, local_uc);
 
 	write_unlock_bh(&arm_state->susp_res_lock);
 

@@ -1433,8 +1391,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
 			if (status == VCHIQ_SUCCESS)
 				ack_cnt--;
 			else
-				atomic_add(ack_cnt,
-					&arm_state->ka_use_ack_count);
+				atomic_add(ack_cnt, &arm_state->ka_use_ack_count);
 		}
 	}
 

@@ -1477,10 +1434,8 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service)
 	--arm_state->videocore_use_count;
 	--(*entity_uc);
 
-	vchiq_log_trace(vchiq_susp_log_level,
-		"%s %s count %d, state count %d",
-		__func__, entity, *entity_uc,
-		arm_state->videocore_use_count);
+	vchiq_log_trace(vchiq_susp_log_level, "%s %s count %d, state count %d", __func__, entity,
+			*entity_uc, arm_state->videocore_use_count);
 
 unlock:
 	write_unlock_bh(&arm_state->susp_res_lock);

@@ -1575,8 +1530,7 @@ vchiq_use_service(unsigned int handle)
 	struct vchiq_service *service = find_service_by_handle(handle);
 
 	if (service) {
-		ret = vchiq_use_internal(service->state, service,
-				USE_TYPE_SERVICE);
+		ret = vchiq_use_internal(service->state, service, USE_TYPE_SERVICE);
 		vchiq_service_put(service);
 	}
 	return ret;

@@ -1666,17 +1620,14 @@ vchiq_dump_service_use_state(struct vchiq_state *state)
 			"with non-zero use-count", active_services, found);
 
 	for (i = 0; i < found; i++) {
-		vchiq_log_warning(vchiq_susp_log_level,
-			"----- %c%c%c%c:%d service count %d %s",
-			VCHIQ_FOURCC_AS_4CHARS(service_data[i].fourcc),
-			service_data[i].clientid,
-			service_data[i].use_count,
-			service_data[i].use_count ? nz : "");
+		vchiq_log_warning(vchiq_susp_log_level, "----- %c%c%c%c:%d service count %d %s",
+				  VCHIQ_FOURCC_AS_4CHARS(service_data[i].fourcc),
+				  service_data[i].clientid, service_data[i].use_count,
+				  service_data[i].use_count ? nz : "");
 	}
-	vchiq_log_warning(vchiq_susp_log_level,
-		"----- VCHIQ use count count %d", peer_count);
-	vchiq_log_warning(vchiq_susp_log_level,
-		"--- Overall vchiq instance use count %d", vc_use_count);
+	vchiq_log_warning(vchiq_susp_log_level, "----- VCHIQ use count count %d", peer_count);
+	vchiq_log_warning(vchiq_susp_log_level, "--- Overall vchiq instance use count %d",
+			  vc_use_count);
 
 	kfree(service_data);
 }

@@ -1699,10 +1650,9 @@ vchiq_check_service(struct vchiq_service *service)
 
 	if (ret == VCHIQ_ERROR) {
 		vchiq_log_error(vchiq_susp_log_level,
-			"%s ERROR - %c%c%c%c:%d service count %d, state count %d", __func__,
-			VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
-			service->client_id, service->service_use_count,
-			arm_state->videocore_use_count);
+				"%s ERROR - %c%c%c%c:%d service count %d, state count %d", __func__,
+				VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), service->client_id,
+				service->service_use_count, arm_state->videocore_use_count);
 		vchiq_dump_service_use_state(service->state);
 	}
 out:

@@ -1717,7 +1667,7 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
 	char threadname[16];
 
 	vchiq_log_info(vchiq_susp_log_level, "%d: %s->%s", state->id,
-		get_conn_state_name(oldstate), get_conn_state_name(newstate));
+		       get_conn_state_name(oldstate), get_conn_state_name(newstate));
 	if (state->conn_state != VCHIQ_CONNSTATE_CONNECTED)
 		return;
Signature · 1 line
2.33.0
author
#2 lore raw

PATCH 3/8 staging: vchiq_arm: remove unnecessary space in cast

In reply to Gaston Gonzalez

As reported by checkpatch.pl, no space is necessary after a cast.

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
index 435f1bd68ba3..f8b3997125e4 100644
@@ -337,7 +337,7 @@ create_pagelist(char *buf, char __user *ubuf,
 		down(&g_free_fragments_mutex);
 		fragments = g_free_fragments;
 		WARN_ON(!fragments);
-		g_free_fragments = *(char **) g_free_fragments;
+		g_free_fragments = *(char **)g_free_fragments;
 		up(&g_free_fragments_mutex);
 		pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
 			(fragments - g_fragments_base) / g_fragments_size;
Signature · 1 line
2.33.0
author
#3 lore raw

PATCH 4/8 staging: vchiq_arm: clarify multiplication expressions

In reply to Gaston Gonzalez

Add spaces around '*' in multiplication expressions to enhance readability.

Reported by checkpatch.pl

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +2 -2
index f8b3997125e4..bb4035f2de65 100644
@@ -466,8 +466,8 @@ int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 
 	g_free_fragments = g_fragments_base;
 	for (i = 0; i < (MAX_FRAGMENTS - 1); i++) {
-		*(char **)&g_fragments_base[i*g_fragments_size] =
-			&g_fragments_base[(i + 1)*g_fragments_size];
+		*(char **)&g_fragments_base[i * g_fragments_size] =
+			&g_fragments_base[(i + 1) * g_fragments_size];
 	}
 	*(char **)&g_fragments_base[i * g_fragments_size] = NULL;
 	sema_init(&g_free_fragments_sema, MAX_FRAGMENTS);
Signature · 1 line
2.33.0
author
#4 lore raw

PATCH 5/8 staging: vchiq_arm: cleanup blank lines

In reply to Gaston Gonzalez

Remove unnecessary blank lines after open braces and before close braces.

Reported by checkpatch.pl

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 --
 1 file changed, 2 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +0 -2
index bb4035f2de65..ffacf1c7212b 100644
@@ -1218,7 +1218,6 @@ int vchiq_dump_platform_service_state(void *dump_context,
 struct vchiq_state *
 vchiq_get_state(void)
 {
-
 	if (!g_state.remote)
 		pr_err("%s: g_state.remote == NULL\n", __func__);
 	else if (g_state.remote->initialised != 1)

@@ -1339,7 +1338,6 @@ vchiq_arm_init_state(struct vchiq_state *state,
 
 		arm_state->state = state;
 		arm_state->first_connect = 0;
-
 	}
 }
Signature · 1 line
2.33.0
author
#5 lore raw

PATCH 6/8 staging: vchiq_arm: fix quoted strings split across lines

In reply to Gaston Gonzalez

Quoted strings should not be split across lines. Quoting Documentation/process/coding-style.rst: "never break user-visible strings such as printk messages because that breaks the ability to grep for them."

Reported by checkpatch.pl

---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c    | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +2 -3
index ffacf1c7212b..72d9a6e37ae9 100644
@@ -1613,9 +1613,8 @@ vchiq_dump_service_use_state(struct vchiq_state *state)
 	read_unlock_bh(&arm_state->susp_res_lock);
 
 	if (only_nonzero)
-		vchiq_log_warning(vchiq_susp_log_level, "Too many active "
-			"services (%d).  Only dumping up to first %d services "
-			"with non-zero use-count", active_services, found);
+		vchiq_log_warning(vchiq_susp_log_level, "Too many active services (%d). Only dumping up to first %d services with non-zero use-count",
+				  active_services, found);
 
 	for (i = 0; i < found; i++) {
 		vchiq_log_warning(vchiq_susp_log_level, "----- %c%c%c%c:%d service count %d %s",
Signature · 1 line
2.33.0
author
#6 lore raw

PATCH 7/8 staging: vchiq_arm: remove extra blank line

In reply to Gaston Gonzalez

Remove unnecessary blank line.

Reported by checkpatch.pl

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h | 1 -
 1 file changed, 1 deletion(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h +0 -1
index e8e39a154c74..8f5182df17b6 100644
@@ -151,7 +151,6 @@ vchiq_dump_service_use_state(struct vchiq_state *state);
 extern struct vchiq_arm_state*
 vchiq_platform_get_arm_state(struct vchiq_state *state);
 
-
 extern enum vchiq_status
 vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
 		   enum USE_TYPE_E use_type);
Signature · 1 line
2.33.0
author
#7 lore raw

PATCH 8/8 staging: vchiq_arm: use __func__ to get function name in debug message

In reply to Gaston Gonzalez

Avoid hardcoded function name using "%s", __func__. This prevents potential naming conflict if the function is eventually renamed.

Reported by checkpatch.pl

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
index 72d9a6e37ae9..2d498fb0d19f 100644
@@ -1025,7 +1025,7 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 			spin_unlock(&msg_queue_spinlock);
 			DEBUG_TRACE(SERVICE_CALLBACK_LINE);
 			DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
-			vchiq_log_trace(vchiq_arm_log_level, "service_callback - msg queue full");
+			vchiq_log_trace(vchiq_arm_log_level, "%s - msg queue full", __func__);
 			/*
 			 * If there is no MESSAGE_AVAILABLE in the completion
 			 * queue, add one
Signature · 1 line
2.33.0
#8 lore raw
In reply to Gaston Gonzalez

Hi,

Am 14.09.21 um 23:35 schrieb Gaston Gonzalez:

Quoted 15 lines

usleep_range() should be used instead of sleep() when sleepings range from 10 us to 20 ms, [1].

Reported by checkpatch.pl

[1] Documentation/timers/timers-howto.txt

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +0 -0
index b25369a13452..0214ae37e01f 100644

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size, · line 827

Quoted 5 lines
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
@@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size,
 		if (status != VCHIQ_RETRY)
 			break;
 
-		msleep(1);
+		usleep_range(1000, 1100);

from my understanding the usage of usleep_range() and hrtimers isn't necessary here. The intention is to sleep a little bit and not "exactly" 1 ms.

@Phil Elwell: what is your opinion?

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c enum vchiq_status vchiq_bulk_receive(unsigned int handle, void *data, · line 864

Quoted 9 lines
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
 	}
 
 	return status;

@@ -861,7 +861,7 @@ enum vchiq_status vchiq_bulk_receive(unsigned int handle, void *data,
 		if (status != VCHIQ_RETRY)
 			break;
 
-		msleep(1);
+		usleep_range(1000, 1100);

dito

Quoted 3 lines
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +0 -0
 	}
 
 	return status;
#9 lore raw

Hi Stefan,

Quoted 21 lines · Stefan Wahren

Hi,

Am 14.09.21 um 23:35 schrieb Gaston Gonzalez:

Quoted 18 lines

usleep_range() should be used instead of sleep() when sleepings range from 10 us to 20 ms, [1].

Reported by checkpatch.pl

[1] Documentation/timers/timers-howto.txt

---
  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +0 -0
index b25369a13452..0214ae37e01f 100644
@@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size,
  		if (status != VCHIQ_RETRY)
  			break;
Quoted 8 lines

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size, · line 827

Quoted 2 lines
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
-		msleep(1);
+		usleep_range(1000, 1100);

from my understanding the usage of usleep_range() and hrtimers isn't necessary here. The intention is to sleep a little bit and not "exactly" 1 ms.

@Phil Elwell: what is your opinion?

Exactly - the aim is just to stop it spinning.

Phil

#10 lore raw
Quoted 32 lines · Phil Elwell

Hi Stefan,

Quoted 28 lines · Stefan Wahren

Hi,

Am 14.09.21 um 23:35 schrieb Gaston Gonzalez:

Quoted 15 lines

usleep_range() should be used instead of sleep() when sleepings range from 10 us to 20 ms, [1].

Reported by checkpatch.pl

[1] Documentation/timers/timers-howto.txt

---
  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +0 -0
index b25369a13452..0214ae37e01f 100644

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size, · line 827

Quoted 5 lines
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +1 -1
@@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size,
             if (status != VCHIQ_RETRY)
                     break;
 
-            msleep(1);
+            usleep_range(1000, 1100);

from my understanding the usage of usleep_range() and hrtimers isn't necessary here. The intention is to sleep a little bit and not "exactly" 1 ms.

Quoted 3 lines
Quoted 1 line

@Phil Elwell: what is your opinion?

Exactly - the aim is just to stop it spinning.

This is usually a sign for something else being wrong in the thing it's waiting for. I can see multiple 'return VCHIQ_RETRY' statements in vchiq_bulk_transfer(), however these all happen when the task has received a signal and wait_for_completion_interruptible() or mutex_lock_killable() has returned an error.

I don't see why one of them would return on any signal and the other one only fatal signals, as you usually want those conditions to be the same, but in either case, the loop is broken because as soon as you get a signal, those interfaces will keep returning the same error and you can never break out of the loop any more.

I don't know how to properly fix it, but it's clear that vchiq_bulk_transmit() needs to propagate the -EINTR or -ERESTSTARTSYS back to user space to let the calling task handle the signal before retrying.

Arnd

#11 lore raw
In reply to Gaston Gonzalez
Quoted 6 lines · Gaston Gonzalez

usleep_range() should be used instead of sleep() when sleepings range from 10 us to 20 ms, [1].

Reported by checkpatch.pl

[1] Documentation/timers/timers-howto.txt

For this particular warning, you should probably just ignore it, if you can't test it...

You need a Signed-off-by. Please run checkpatch.pl on your patches.

regards,
dan carpenter

author
#12 lore raw
Quoted 4 lines · Dan Carpenter
Quoted 3 lines · Gaston Gonzalez

usleep_range() should be used instead of sleep() when sleepings range from 10 us to 20 ms, [1].

Quoted 9 lines
Quoted 3 lines

Reported by checkpatch.pl

[1] Documentation/timers/timers-howto.txt

For this particular warning, you should probably just ignore it, if you can't test it...

You need a Signed-off-by. Please run checkpatch.pl on your patches.

Yes, my bad...

Will drop this one and resend the rest of the series.

Thanks,

Gaston

Quoted 3 lines

regards,
dan carpenter