* [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-05 8:41 [PATCH v3 0/4] staging: vc04_services: Drop custom logging Umang Jain
@ 2023-12-05 8:41 ` Umang Jain
2023-12-05 19:32 ` Stefan Wahren
2023-12-05 8:41 ` [PATCH v3 2/4] staging: vc04_services: Drop vchiq_log_warning() in favour of dev_warn Umang Jain
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Umang Jain @ 2023-12-05 8:41 UTC (permalink / raw)
To: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere, Umang Jain
Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the usage
of dev_err() directly.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
.../interface/vchiq_arm/vchiq_arm.c | 50 ++++----
.../interface/vchiq_arm/vchiq_connected.c | 6 +-
.../interface/vchiq_arm/vchiq_core.c | 113 +++++++++---------
.../interface/vchiq_arm/vchiq_core.h | 4 -
.../interface/vchiq_arm/vchiq_dev.c | 45 ++++---
5 files changed, 101 insertions(+), 117 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 7978fb6dc4fb..b403400edd8e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -683,8 +683,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
usleep_range(500, 600);
}
if (i == VCHIQ_INIT_RETRIES) {
- vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not initialized\n",
- __func__);
+ dev_err(state->dev, "core: %s: Videocore not initialized\n", __func__);
ret = -ENOTCONN;
goto failed;
} else if (i > 0) {
@@ -694,8 +693,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
instance = kzalloc(sizeof(*instance), GFP_KERNEL);
if (!instance) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "%s: error allocating vchiq instance\n", __func__);
+ dev_err(state->dev, "core: %s: Cannot allocate vchiq instance\n", __func__);
ret = -ENOMEM;
goto failed;
}
@@ -967,8 +965,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
} else {
waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
if (!waiter) {
- vchiq_log_error(service->state->dev, VCHIQ_CORE,
- "%s - out of memory", __func__);
+ dev_err(service->state->dev, "core: %s: - Out of memory\n", __func__);
return -ENOMEM;
}
}
@@ -1290,8 +1287,8 @@ vchiq_keepalive_vchiq_callback(struct vchiq_instance *instance,
struct vchiq_header *header,
unsigned int service_user, void *bulk_user)
{
- vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
- "%s callback reason %d", __func__, reason);
+ dev_err(instance->state->dev, "suspend: %s: callback reason %d\n",
+ __func__, reason);
return 0;
}
@@ -1315,22 +1312,20 @@ vchiq_keepalive_thread_func(void *v)
ret = vchiq_initialise(&instance);
if (ret) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "%s vchiq_initialise failed %d", __func__, ret);
+ dev_err(state->dev, "suspend: %s: vchiq_initialise failed %d\n", __func__, ret);
goto exit;
}
status = vchiq_connect(instance);
if (status) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "%s vchiq_connect failed %d", __func__, status);
+ dev_err(state->dev, "suspend: %s: vchiq_connect failed %d\n", __func__, status);
goto shutdown;
}
status = vchiq_add_service(instance, ¶ms, &ka_handle);
if (status) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "%s vchiq_open_service failed %d", __func__, status);
+ dev_err(state->dev, "suspend: %s: vchiq_open_service failed %d\n",
+ __func__, status);
goto shutdown;
}
@@ -1338,8 +1333,7 @@ vchiq_keepalive_thread_func(void *v)
long rc = 0, uc = 0;
if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "%s interrupted", __func__);
+ dev_err(state->dev, "suspend: %s: interrupted\n", __func__);
flush_signals(current);
continue;
}
@@ -1359,16 +1353,15 @@ vchiq_keepalive_thread_func(void *v)
atomic_inc(&arm_state->ka_use_ack_count);
status = vchiq_use_service(instance, ka_handle);
if (status) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "%s vchiq_use_service error %d", __func__, status);
+ dev_err(state->dev, "suspend: %s: vchiq_use_service error %d\n",
+ __func__, status);
}
}
while (rc--) {
status = vchiq_release_service(instance, ka_handle);
if (status) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "%s vchiq_release_service error %d", __func__,
- status);
+ dev_err(state->dev, "suspend: %s: vchiq_release_service error %d\n",
+ __func__, status);
}
}
}
@@ -1403,7 +1396,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
service->client_id);
entity_uc = &service->service_use_count;
} else {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND, "%s null service ptr", __func__);
+ dev_err(state->dev, "suspend: %s: null service ptr\n", __func__);
ret = -EINVAL;
goto out;
}
@@ -1686,10 +1679,10 @@ vchiq_check_service(struct vchiq_service *service)
read_unlock_bh(&arm_state->susp_res_lock);
if (ret) {
- vchiq_log_error(service->state->dev, VCHIQ_SUSPEND,
- "%s ERROR - %p4cc:%d service count %d, state count %d", __func__,
- &service->base.fourcc, service->client_id,
- service->service_use_count, arm_state->videocore_use_count);
+ dev_err(service->state->dev,
+ "suspend: %s: %p4cc:%d service count %d, state count %d\n",
+ __func__, &service->base.fourcc, service->client_id,
+ service->service_use_count, arm_state->videocore_use_count);
vchiq_dump_service_use_state(service->state);
}
out:
@@ -1722,9 +1715,8 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
(void *)state,
threadname);
if (IS_ERR(arm_state->ka_thread)) {
- vchiq_log_error(state->dev, VCHIQ_SUSPEND,
- "vchiq: FATAL: couldn't create thread %s",
- threadname);
+ dev_err(state->dev, "suspend: Couldn't create thread %s\n",
+ threadname);
} else {
wake_up_process(arm_state->ka_thread);
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
index 21f9fa1a1713..3cad13f09e37 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
@@ -39,9 +39,9 @@ void vchiq_add_connected_callback(struct vchiq_device *device, void (*callback)(
callback();
} else {
if (g_num_deferred_callbacks >= MAX_CALLBACKS) {
- vchiq_log_error(&device->dev, VCHIQ_CORE,
- "There already %d callback registered - please increase MAX_CALLBACKS",
- g_num_deferred_callbacks);
+ dev_err(&device->dev,
+ "core: There already %d callback registered - please increase MAX_CALLBACKS\n",
+ g_num_deferred_callbacks);
} else {
g_deferred_callback[g_num_deferred_callbacks] =
callback;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index e0022acb4c58..63f412815a32 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -741,10 +741,10 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
*/
complete("a->quota_event);
} else if (count == 0) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
- port, quota->message_use_count, header, msgid, header->msgid,
- header->size);
+ dev_err(state->dev,
+ "core: service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)\n",
+ port, quota->message_use_count, header, msgid,
+ header->msgid, header->size);
WARN(1, "invalid message use count\n");
}
if (!BITSET_IS_SET(service_found, port)) {
@@ -766,9 +766,9 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq:%d %x@%pK - slot_use->%d",
state->id, port, header->size, header, count - 1);
} else {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
- port, count, header, msgid, header->msgid, header->size);
+ dev_err(state->dev,
+ "core: service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)\n",
+ port, count, header, msgid, header->msgid, header->size);
WARN(1, "bad slot use count\n");
}
}
@@ -831,9 +831,9 @@ process_free_queue(struct vchiq_state *state, u32 *service_found,
pos += calc_stride(header->size);
if (pos > VCHIQ_SLOT_SIZE) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "pfq - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x",
- pos, header, msgid, header->msgid, header->size);
+ dev_err(state->dev,
+ "core: pfq - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x\n",
+ pos, header, msgid, header->msgid, header->size);
WARN(1, "invalid slot position\n");
}
}
@@ -1167,8 +1167,8 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
int oldmsgid = header->msgid;
if (oldmsgid != VCHIQ_MSGID_PADDING)
- vchiq_log_error(state->dev, VCHIQ_CORE, "%d: qms - msgid %x, not PADDING",
- state->id, oldmsgid);
+ dev_err(state->dev, "core: %d: qms - msgid %x, not PADDING\n",
+ state->id, oldmsgid);
}
vchiq_log_debug(state->dev, VCHIQ_SYNC,
@@ -1616,10 +1616,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
}
if (!service) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "%d: prs %s@%pK (%d->%d) - invalid/closed service %d",
- state->id, msg_type_str(type), header, remoteport,
- localport, localport);
+ dev_err(state->dev,
+ "core: %d: prs %s@%pK (%d->%d) - invalid/closed service %d\n",
+ state->id, msg_type_str(type), header, remoteport,
+ localport, localport);
goto skip_message;
}
break;
@@ -1640,9 +1640,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
if (((unsigned long)header & VCHIQ_SLOT_MASK) +
calc_stride(size) > VCHIQ_SLOT_SIZE) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "header %pK (msgid %x) - size %x too big for slot",
- header, (unsigned int)msgid, (unsigned int)size);
+ dev_err(state->dev, "core: header %pK (msgid %x) - size %x too big for slot\n",
+ header, (unsigned int)msgid, (unsigned int)size);
WARN(1, "oversized for slot\n");
}
@@ -1668,8 +1667,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
set_service_state(service, VCHIQ_SRVSTATE_OPEN);
complete(&service->remove_event);
} else {
- vchiq_log_error(state->dev, VCHIQ_CORE, "OPENACK received in state %s",
- srvstate_names[service->srvstate]);
+ dev_err(state->dev, "core: OPENACK received in state %s\n",
+ srvstate_names[service->srvstate]);
}
break;
case VCHIQ_MSG_CLOSE:
@@ -1740,11 +1739,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
}
if ((int)(queue->remote_insert -
queue->local_insert) >= 0) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "%d: prs %s@%pK (%d->%d) unexpected (ri=%d,li=%d)",
- state->id, msg_type_str(type), header, remoteport,
- localport, queue->remote_insert,
- queue->local_insert);
+ dev_err(state->dev,
+ "core: %d: prs %s@%pK (%d->%d) unexpected (ri=%d,li=%d)\n",
+ state->id, msg_type_str(type), header, remoteport,
+ localport, queue->remote_insert, queue->local_insert);
mutex_unlock(&service->bulk_mutex);
break;
}
@@ -1790,8 +1788,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs PAUSE@%pK,%x",
state->id, header, size);
if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "%d: PAUSE received in state PAUSED", state->id);
+ dev_err(state->dev, "core: %d: PAUSE received in state PAUSED\n",
+ state->id);
break;
}
if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
@@ -1821,8 +1819,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
break;
default:
- vchiq_log_error(state->dev, VCHIQ_CORE, "%d: prs invalid msgid %x@%pK,%x",
- state->id, msgid, header, size);
+ dev_err(state->dev, "core: %d: prs invalid msgid %x@%pK,%x\n",
+ state->id, msgid, header, size);
WARN(1, "invalid message\n");
break;
}
@@ -1932,7 +1930,7 @@ handle_poll(struct vchiq_state *state)
* since the PAUSE should have flushed
* through outstanding messages.
*/
- vchiq_log_error(state->dev, VCHIQ_CORE, "Failed to send RESUME message");
+ dev_err(state->dev, "core: Failed to send RESUME message\n");
}
break;
default:
@@ -2032,10 +2030,10 @@ sync_func(void *v)
service = find_service_by_port(state, localport);
if (!service) {
- vchiq_log_error(state->dev, VCHIQ_SYNC,
- "%d: sf %s@%pK (%d->%d) - invalid/closed service %d",
- state->id, msg_type_str(type), header,
- remoteport, localport, localport);
+ dev_err(state->dev,
+ "sync: %d: sf %s@%pK (%d->%d) - invalid/closed service %d\n",
+ state->id, msg_type_str(type), header, remoteport,
+ localport, localport);
release_message_sync(state, header);
continue;
}
@@ -2077,15 +2075,15 @@ sync_func(void *v)
(service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
if (make_service_callback(service, VCHIQ_MESSAGE_AVAILABLE, header,
NULL) == -EAGAIN)
- vchiq_log_error(state->dev, VCHIQ_SYNC,
- "synchronous callback to service %d returns -EAGAIN",
- localport);
+ dev_err(state->dev,
+ "sync: error: synchronous callback to service %d returns -EAGAIN\n",
+ localport);
}
break;
default:
- vchiq_log_error(state->dev, VCHIQ_SYNC, "%d: sf unexpected msgid %x@%pK,%x",
- state->id, msgid, header, size);
+ dev_err(state->dev, "sync: error: %d: sf unexpected msgid %x@%pK,%x\n",
+ state->id, msgid, header, size);
release_message_sync(state, header);
break;
}
@@ -2118,8 +2116,8 @@ vchiq_init_slots(struct device *dev, void *mem_base, int mem_size)
num_slots -= first_data_slot;
if (num_slots < 4) {
- vchiq_log_error(dev, VCHIQ_CORE, "%s - insufficient memory %x bytes",
- __func__, mem_size);
+ dev_err(dev, "core: %s: Insufficient memory %x bytes\n",
+ __func__, mem_size);
return NULL;
}
@@ -2500,11 +2498,10 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)
} else if ((service->srvstate != VCHIQ_SRVSTATE_OPEN) &&
(service->srvstate != VCHIQ_SRVSTATE_OPENSYNC)) {
if (service->srvstate != VCHIQ_SRVSTATE_CLOSEWAIT)
- vchiq_log_error(service->state->dev, VCHIQ_CORE,
- "%d: osi - srvstate = %s (ref %u)",
- service->state->id,
- srvstate_names[service->srvstate],
- kref_read(&service->ref_count));
+ dev_err(service->state->dev,
+ "core: %d: osi - srvstate = %s (ref %u)\n",
+ service->state->id, srvstate_names[service->srvstate],
+ kref_read(&service->ref_count));
status = -EINVAL;
VCHIQ_SERVICE_STATS_INC(service, error_count);
vchiq_release_service_internal(service);
@@ -2565,9 +2562,9 @@ release_service_messages(struct vchiq_service *service)
}
pos += calc_stride(header->size);
if (pos > VCHIQ_SLOT_SIZE) {
- vchiq_log_error(state->dev, VCHIQ_CORE,
- "fsi - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x",
- pos, header, msgid, header->msgid, header->size);
+ dev_err(state->dev,
+ "core: fsi - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x\n",
+ pos, header, msgid, header->msgid, header->size);
WARN(1, "invalid slot position\n");
}
}
@@ -2621,8 +2618,8 @@ close_service_complete(struct vchiq_service *service, int failstate)
case VCHIQ_SRVSTATE_LISTENING:
break;
default:
- vchiq_log_error(service->state->dev, VCHIQ_CORE, "%s(%x) called in state %s",
- __func__, service->handle, srvstate_names[service->srvstate]);
+ dev_err(service->state->dev, "core: (%x) called in state %s\n",
+ service->handle, srvstate_names[service->srvstate]);
WARN(1, "%s in unexpected state\n", __func__);
return -EINVAL;
}
@@ -2677,8 +2674,8 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
case VCHIQ_SRVSTATE_LISTENING:
case VCHIQ_SRVSTATE_CLOSEWAIT:
if (close_recvd) {
- vchiq_log_error(state->dev, VCHIQ_CORE, "%s(1) called in state %s",
- __func__, srvstate_names[service->srvstate]);
+ dev_err(state->dev, "core: (1) called in state %s\n",
+ srvstate_names[service->srvstate]);
} else if (is_server) {
if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
status = -EINVAL;
@@ -2765,8 +2762,8 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
break;
default:
- vchiq_log_error(state->dev, VCHIQ_CORE, "%s(%d) called in state %s", __func__,
- close_recvd, srvstate_names[service->srvstate]);
+ dev_err(state->dev, "core: (%d) called in state %s\n",
+ close_recvd, srvstate_names[service->srvstate]);
break;
}
@@ -2805,8 +2802,8 @@ vchiq_free_service_internal(struct vchiq_service *service)
case VCHIQ_SRVSTATE_CLOSEWAIT:
break;
default:
- vchiq_log_error(state->dev, VCHIQ_CORE, "%d: fsi - (%d) in state %s", state->id,
- service->localport, srvstate_names[service->srvstate]);
+ dev_err(state->dev, "core: %d: fsi - (%d) in state %s\n",
+ state->id, service->localport, srvstate_names[service->srvstate]);
return;
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 564b5c707267..d7dcd17e4bff 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -53,10 +53,6 @@ static inline const char *log_category_str(enum vchiq_log_category c)
return strings[c];
};
-#ifndef vchiq_log_error
-#define vchiq_log_error(dev, cat, fmt, ...) \
- do { dev_dbg(dev, "%s error: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
-#endif
#ifndef vchiq_log_warning
#define vchiq_log_warning(dev, cat, fmt, ...) \
do { dev_dbg(dev, "%s warning: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index 307a2d76cbb7..ba287cb4c87b 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -271,9 +271,9 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
ret = -EFAULT;
}
} else {
- vchiq_log_error(service->state->dev, VCHIQ_ARM,
- "header %pK: bufsize %x < size %x",
- header, args->bufsize, header->size);
+ dev_err(service->state->dev,
+ "arm: header %pK: bufsize %x < size %x\n",
+ header, args->bufsize, header->size);
WARN(1, "invalid size\n");
ret = -EMSGSIZE;
}
@@ -318,8 +318,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
if (!waiter) {
- vchiq_log_error(service->state->dev, VCHIQ_ARM,
- "no bulk_waiter found for pid %d", current->pid);
+ dev_err(service->state->dev,
+ "arm: no bulk_waiter found for pid %d\n", current->pid);
ret = -ESRCH;
goto out;
}
@@ -501,10 +501,10 @@ static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
msglen = header->size + sizeof(struct vchiq_header);
/* This must be a VCHIQ-style service */
if (args->msgbufsize < msglen) {
- vchiq_log_error(service->state->dev, VCHIQ_ARM,
- "header %pK: msgbufsize %x < msglen %x",
- header, args->msgbufsize, msglen);
- WARN(1, "invalid message size\n");
+ dev_err(service->state->dev,
+ "arm: header %pK: msgbufsize %x < msglen %x\n",
+ header, args->msgbufsize, msglen);
+ WARN(1, "invalid message size\n");
if (ret == 0)
ret = -EMSGSIZE;
break;
@@ -618,9 +618,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
}
rc = mutex_lock_killable(&instance->state->mutex);
if (rc) {
- vchiq_log_error(instance->state->dev, VCHIQ_ARM,
- "vchiq: connect: could not lock mutex for state %d: %d",
- instance->state->id, rc);
+ dev_err(instance->state->dev,
+ "arm: vchiq: connect: could not lock mutex for state %d: %d\n",
+ instance->state->id, rc);
ret = -EINTR;
break;
}
@@ -630,8 +630,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (!status)
instance->connected = 1;
else
- vchiq_log_error(instance->state->dev, VCHIQ_ARM,
- "vchiq: could not connect: %d", status);
+ dev_err(instance->state->dev,
+ "arm: vchiq: could not connect: %d\n", status);
break;
case VCHIQ_IOC_CREATE_SERVICE: {
@@ -700,13 +700,13 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
vchiq_use_service_internal(service) :
vchiq_release_service_internal(service);
if (ret) {
- vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
- "%s: cmd %s returned error %ld for service %p4cc:%03d",
- __func__, (cmd == VCHIQ_IOC_USE_SERVICE) ?
- "VCHIQ_IOC_USE_SERVICE" :
- "VCHIQ_IOC_RELEASE_SERVICE",
- ret, &service->base.fourcc,
- service->client_id);
+ dev_err(instance->state->dev,
+ "suspend: cmd %s returned error %ld for service %p4cc:%03d\n",
+ (cmd == VCHIQ_IOC_USE_SERVICE) ?
+ "VCHIQ_IOC_USE_SERVICE" :
+ "VCHIQ_IOC_RELEASE_SERVICE",
+ ret, &service->base.fourcc,
+ service->client_id);
}
} else {
ret = -EINVAL;
@@ -1173,8 +1173,7 @@ static int vchiq_open(struct inode *inode, struct file *file)
vchiq_log_debug(state->dev, VCHIQ_ARM, "vchiq_open");
if (!state) {
- vchiq_log_error(state->dev, VCHIQ_ARM,
- "vchiq has no connection to VideoCore");
+ dev_err(state->dev, "arm: vchiq has no connection to VideoCore\n");
return -ENOTCONN;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-05 8:41 ` [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err Umang Jain
@ 2023-12-05 19:32 ` Stefan Wahren
2023-12-06 5:27 ` Umang Jain
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Wahren @ 2023-12-05 19:32 UTC (permalink / raw)
To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Umang,
Am 05.12.23 um 09:41 schrieb Umang Jain:
> Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the usage
> of dev_err() directly.
sorry, i missed to review the last change. So the change won't be that
trivial.
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 50 ++++----
> .../interface/vchiq_arm/vchiq_connected.c | 6 +-
> .../interface/vchiq_arm/vchiq_core.c | 113 +++++++++---------
> .../interface/vchiq_arm/vchiq_core.h | 4 -
> .../interface/vchiq_arm/vchiq_dev.c | 45 ++++---
> 5 files changed, 101 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 7978fb6dc4fb..b403400edd8e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -683,8 +683,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
> usleep_range(500, 600);
> }
> if (i == VCHIQ_INIT_RETRIES) {
> - vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not initialized\n",
> - __func__);
> + dev_err(state->dev, "core: %s: Videocore not initialized\n", __func__);
> ret = -ENOTCONN;
> goto failed;
> } else if (i > 0) {
> @@ -694,8 +693,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
>
> instance = kzalloc(sizeof(*instance), GFP_KERNEL);
> if (!instance) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "%s: error allocating vchiq instance\n", __func__);
> + dev_err(state->dev, "core: %s: Cannot allocate vchiq instance\n", __func__);
please drop because kzalloc already report such errors
> ret = -ENOMEM;
> goto failed;
> }
> @@ -967,8 +965,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
> } else {
> waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
> if (!waiter) {
> - vchiq_log_error(service->state->dev, VCHIQ_CORE,
> - "%s - out of memory", __func__);
> + dev_err(service->state->dev, "core: %s: - Out of memory\n", __func__);
ditto
> return -ENOMEM;
> }
> }
> @@ -1290,8 +1287,8 @@ vchiq_keepalive_vchiq_callback(struct vchiq_instance *instance,
> struct vchiq_header *header,
> unsigned int service_user, void *bulk_user)
> {
> - vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
> - "%s callback reason %d", __func__, reason);
> + dev_err(instance->state->dev, "suspend: %s: callback reason %d\n",
> + __func__, reason);
> return 0;
> }
>
> @@ -1315,22 +1312,20 @@ vchiq_keepalive_thread_func(void *v)
>
> ret = vchiq_initialise(&instance);
> if (ret) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "%s vchiq_initialise failed %d", __func__, ret);
> + dev_err(state->dev, "suspend: %s: vchiq_initialise failed %d\n", __func__, ret);
> goto exit;
> }
>
> status = vchiq_connect(instance);
> if (status) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "%s vchiq_connect failed %d", __func__, status);
> + dev_err(state->dev, "suspend: %s: vchiq_connect failed %d\n", __func__, status);
> goto shutdown;
> }
>
> status = vchiq_add_service(instance, ¶ms, &ka_handle);
> if (status) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "%s vchiq_open_service failed %d", __func__, status);
> + dev_err(state->dev, "suspend: %s: vchiq_open_service failed %d\n",
> + __func__, status);
> goto shutdown;
> }
>
> @@ -1338,8 +1333,7 @@ vchiq_keepalive_thread_func(void *v)
> long rc = 0, uc = 0;
>
> if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "%s interrupted", __func__);
> + dev_err(state->dev, "suspend: %s: interrupted\n", __func__);
I think this isn't an error and we should use dev_dbg here.
> flush_signals(current);
> continue;
> }
> @@ -1359,16 +1353,15 @@ vchiq_keepalive_thread_func(void *v)
> atomic_inc(&arm_state->ka_use_ack_count);
> status = vchiq_use_service(instance, ka_handle);
> if (status) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "%s vchiq_use_service error %d", __func__, status);
> + dev_err(state->dev, "suspend: %s: vchiq_use_service error %d\n",
> + __func__, status);
> }
> }
> while (rc--) {
> status = vchiq_release_service(instance, ka_handle);
> if (status) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "%s vchiq_release_service error %d", __func__,
> - status);
> + dev_err(state->dev, "suspend: %s: vchiq_release_service error %d\n",
> + __func__, status);
> }
> }
> }
> @@ -1403,7 +1396,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
> service->client_id);
> entity_uc = &service->service_use_count;
> } else {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND, "%s null service ptr", __func__);
> + dev_err(state->dev, "suspend: %s: null service ptr\n", __func__);
Instead of dev_err() i would suggest a WARN() here
> ret = -EINVAL;
> goto out;
> }
> @@ -1686,10 +1679,10 @@ vchiq_check_service(struct vchiq_service *service)
> read_unlock_bh(&arm_state->susp_res_lock);
>
> if (ret) {
> - vchiq_log_error(service->state->dev, VCHIQ_SUSPEND,
> - "%s ERROR - %p4cc:%d service count %d, state count %d", __func__,
> - &service->base.fourcc, service->client_id,
> - service->service_use_count, arm_state->videocore_use_count);
> + dev_err(service->state->dev,
> + "suspend: %s: %p4cc:%d service count %d, state count %d\n",
> + __func__, &service->base.fourcc, service->client_id,
> + service->service_use_count, arm_state->videocore_use_count);
> vchiq_dump_service_use_state(service->state);
> }
> out:
> @@ -1722,9 +1715,8 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
> (void *)state,
> threadname);
> if (IS_ERR(arm_state->ka_thread)) {
> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> - "vchiq: FATAL: couldn't create thread %s",
> - threadname);
> + dev_err(state->dev, "suspend: Couldn't create thread %s\n",
> + threadname);
> } else {
> wake_up_process(arm_state->ka_thread);
> }
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> index 21f9fa1a1713..3cad13f09e37 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> @@ -39,9 +39,9 @@ void vchiq_add_connected_callback(struct vchiq_device *device, void (*callback)(
> callback();
> } else {
> if (g_num_deferred_callbacks >= MAX_CALLBACKS) {
> - vchiq_log_error(&device->dev, VCHIQ_CORE,
> - "There already %d callback registered - please increase MAX_CALLBACKS",
> - g_num_deferred_callbacks);
> + dev_err(&device->dev,
> + "core: There already %d callback registered - please increase MAX_CALLBACKS\n",
> + g_num_deferred_callbacks);
> } else {
> g_deferred_callback[g_num_deferred_callbacks] =
> callback;
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index e0022acb4c58..63f412815a32 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -741,10 +741,10 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
> */
> complete("a->quota_event);
> } else if (count == 0) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
> - port, quota->message_use_count, header, msgid, header->msgid,
> - header->size);
> + dev_err(state->dev,
> + "core: service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)\n",
> + port, quota->message_use_count, header, msgid,
> + header->msgid, header->size);
> WARN(1, "invalid message use count\n");
> }
> if (!BITSET_IS_SET(service_found, port)) {
> @@ -766,9 +766,9 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
> vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq:%d %x@%pK - slot_use->%d",
> state->id, port, header->size, header, count - 1);
> } else {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
> - port, count, header, msgid, header->msgid, header->size);
> + dev_err(state->dev,
> + "core: service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)\n",
> + port, count, header, msgid, header->msgid, header->size);
> WARN(1, "bad slot use count\n");
> }
> }
> @@ -831,9 +831,9 @@ process_free_queue(struct vchiq_state *state, u32 *service_found,
>
> pos += calc_stride(header->size);
> if (pos > VCHIQ_SLOT_SIZE) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "pfq - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x",
> - pos, header, msgid, header->msgid, header->size);
> + dev_err(state->dev,
> + "core: pfq - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x\n",
> + pos, header, msgid, header->msgid, header->size);
> WARN(1, "invalid slot position\n");
> }
> }
> @@ -1167,8 +1167,8 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
> int oldmsgid = header->msgid;
>
> if (oldmsgid != VCHIQ_MSGID_PADDING)
> - vchiq_log_error(state->dev, VCHIQ_CORE, "%d: qms - msgid %x, not PADDING",
> - state->id, oldmsgid);
> + dev_err(state->dev, "core: %d: qms - msgid %x, not PADDING\n",
> + state->id, oldmsgid);
> }
>
> vchiq_log_debug(state->dev, VCHIQ_SYNC,
> @@ -1616,10 +1616,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> }
>
> if (!service) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "%d: prs %s@%pK (%d->%d) - invalid/closed service %d",
> - state->id, msg_type_str(type), header, remoteport,
> - localport, localport);
> + dev_err(state->dev,
> + "core: %d: prs %s@%pK (%d->%d) - invalid/closed service %d\n",
> + state->id, msg_type_str(type), header, remoteport,
> + localport, localport);
> goto skip_message;
> }
> break;
> @@ -1640,9 +1640,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
>
> if (((unsigned long)header & VCHIQ_SLOT_MASK) +
> calc_stride(size) > VCHIQ_SLOT_SIZE) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "header %pK (msgid %x) - size %x too big for slot",
> - header, (unsigned int)msgid, (unsigned int)size);
> + dev_err(state->dev, "core: header %pK (msgid %x) - size %x too big for slot\n",
> + header, (unsigned int)msgid, (unsigned int)size);
> WARN(1, "oversized for slot\n");
> }
>
> @@ -1668,8 +1667,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> set_service_state(service, VCHIQ_SRVSTATE_OPEN);
> complete(&service->remove_event);
> } else {
> - vchiq_log_error(state->dev, VCHIQ_CORE, "OPENACK received in state %s",
> - srvstate_names[service->srvstate]);
> + dev_err(state->dev, "core: OPENACK received in state %s\n",
> + srvstate_names[service->srvstate]);
> }
> break;
> case VCHIQ_MSG_CLOSE:
> @@ -1740,11 +1739,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> }
> if ((int)(queue->remote_insert -
> queue->local_insert) >= 0) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "%d: prs %s@%pK (%d->%d) unexpected (ri=%d,li=%d)",
> - state->id, msg_type_str(type), header, remoteport,
> - localport, queue->remote_insert,
> - queue->local_insert);
> + dev_err(state->dev,
> + "core: %d: prs %s@%pK (%d->%d) unexpected (ri=%d,li=%d)\n",
> + state->id, msg_type_str(type), header, remoteport,
> + localport, queue->remote_insert, queue->local_insert);
> mutex_unlock(&service->bulk_mutex);
> break;
> }
> @@ -1790,8 +1788,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs PAUSE@%pK,%x",
> state->id, header, size);
> if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "%d: PAUSE received in state PAUSED", state->id);
> + dev_err(state->dev, "core: %d: PAUSE received in state PAUSED\n",
> + state->id);
> break;
> }
> if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
> @@ -1821,8 +1819,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> break;
>
> default:
> - vchiq_log_error(state->dev, VCHIQ_CORE, "%d: prs invalid msgid %x@%pK,%x",
> - state->id, msgid, header, size);
> + dev_err(state->dev, "core: %d: prs invalid msgid %x@%pK,%x\n",
> + state->id, msgid, header, size);
> WARN(1, "invalid message\n");
> break;
> }
> @@ -1932,7 +1930,7 @@ handle_poll(struct vchiq_state *state)
> * since the PAUSE should have flushed
> * through outstanding messages.
> */
> - vchiq_log_error(state->dev, VCHIQ_CORE, "Failed to send RESUME message");
> + dev_err(state->dev, "core: Failed to send RESUME message\n");
> }
> break;
> default:
> @@ -2032,10 +2030,10 @@ sync_func(void *v)
> service = find_service_by_port(state, localport);
>
> if (!service) {
> - vchiq_log_error(state->dev, VCHIQ_SYNC,
> - "%d: sf %s@%pK (%d->%d) - invalid/closed service %d",
> - state->id, msg_type_str(type), header,
> - remoteport, localport, localport);
> + dev_err(state->dev,
> + "sync: %d: sf %s@%pK (%d->%d) - invalid/closed service %d\n",
> + state->id, msg_type_str(type), header, remoteport,
> + localport, localport);
> release_message_sync(state, header);
> continue;
> }
> @@ -2077,15 +2075,15 @@ sync_func(void *v)
> (service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
> if (make_service_callback(service, VCHIQ_MESSAGE_AVAILABLE, header,
> NULL) == -EAGAIN)
> - vchiq_log_error(state->dev, VCHIQ_SYNC,
> - "synchronous callback to service %d returns -EAGAIN",
> - localport);
> + dev_err(state->dev,
> + "sync: error: synchronous callback to service %d returns -EAGAIN\n",
> + localport);
> }
> break;
>
> default:
> - vchiq_log_error(state->dev, VCHIQ_SYNC, "%d: sf unexpected msgid %x@%pK,%x",
> - state->id, msgid, header, size);
> + dev_err(state->dev, "sync: error: %d: sf unexpected msgid %x@%pK,%x\n",
> + state->id, msgid, header, size);
> release_message_sync(state, header);
> break;
> }
> @@ -2118,8 +2116,8 @@ vchiq_init_slots(struct device *dev, void *mem_base, int mem_size)
> num_slots -= first_data_slot;
>
> if (num_slots < 4) {
> - vchiq_log_error(dev, VCHIQ_CORE, "%s - insufficient memory %x bytes",
> - __func__, mem_size);
> + dev_err(dev, "core: %s: Insufficient memory %x bytes\n",
> + __func__, mem_size);
> return NULL;
> }
>
> @@ -2500,11 +2498,10 @@ vchiq_open_service_internal(struct vchiq_service *service, int client_id)
> } else if ((service->srvstate != VCHIQ_SRVSTATE_OPEN) &&
> (service->srvstate != VCHIQ_SRVSTATE_OPENSYNC)) {
> if (service->srvstate != VCHIQ_SRVSTATE_CLOSEWAIT)
> - vchiq_log_error(service->state->dev, VCHIQ_CORE,
> - "%d: osi - srvstate = %s (ref %u)",
> - service->state->id,
> - srvstate_names[service->srvstate],
> - kref_read(&service->ref_count));
> + dev_err(service->state->dev,
> + "core: %d: osi - srvstate = %s (ref %u)\n",
> + service->state->id, srvstate_names[service->srvstate],
> + kref_read(&service->ref_count));
> status = -EINVAL;
> VCHIQ_SERVICE_STATS_INC(service, error_count);
> vchiq_release_service_internal(service);
> @@ -2565,9 +2562,9 @@ release_service_messages(struct vchiq_service *service)
> }
> pos += calc_stride(header->size);
> if (pos > VCHIQ_SLOT_SIZE) {
> - vchiq_log_error(state->dev, VCHIQ_CORE,
> - "fsi - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x",
> - pos, header, msgid, header->msgid, header->size);
> + dev_err(state->dev,
> + "core: fsi - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x\n",
> + pos, header, msgid, header->msgid, header->size);
> WARN(1, "invalid slot position\n");
> }
> }
> @@ -2621,8 +2618,8 @@ close_service_complete(struct vchiq_service *service, int failstate)
> case VCHIQ_SRVSTATE_LISTENING:
> break;
> default:
> - vchiq_log_error(service->state->dev, VCHIQ_CORE, "%s(%x) called in state %s",
> - __func__, service->handle, srvstate_names[service->srvstate]);
> + dev_err(service->state->dev, "core: (%x) called in state %s\n",
> + service->handle, srvstate_names[service->srvstate]);
> WARN(1, "%s in unexpected state\n", __func__);
> return -EINVAL;
> }
> @@ -2677,8 +2674,8 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
> case VCHIQ_SRVSTATE_LISTENING:
> case VCHIQ_SRVSTATE_CLOSEWAIT:
> if (close_recvd) {
> - vchiq_log_error(state->dev, VCHIQ_CORE, "%s(1) called in state %s",
> - __func__, srvstate_names[service->srvstate]);
> + dev_err(state->dev, "core: (1) called in state %s\n",
> + srvstate_names[service->srvstate]);
> } else if (is_server) {
> if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
> status = -EINVAL;
> @@ -2765,8 +2762,8 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
> break;
>
> default:
> - vchiq_log_error(state->dev, VCHIQ_CORE, "%s(%d) called in state %s", __func__,
> - close_recvd, srvstate_names[service->srvstate]);
> + dev_err(state->dev, "core: (%d) called in state %s\n",
> + close_recvd, srvstate_names[service->srvstate]);
> break;
> }
>
> @@ -2805,8 +2802,8 @@ vchiq_free_service_internal(struct vchiq_service *service)
> case VCHIQ_SRVSTATE_CLOSEWAIT:
> break;
> default:
> - vchiq_log_error(state->dev, VCHIQ_CORE, "%d: fsi - (%d) in state %s", state->id,
> - service->localport, srvstate_names[service->srvstate]);
> + dev_err(state->dev, "core: %d: fsi - (%d) in state %s\n",
> + state->id, service->localport, srvstate_names[service->srvstate]);
> return;
> }
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> index 564b5c707267..d7dcd17e4bff 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -53,10 +53,6 @@ static inline const char *log_category_str(enum vchiq_log_category c)
> return strings[c];
> };
>
> -#ifndef vchiq_log_error
> -#define vchiq_log_error(dev, cat, fmt, ...) \
> - do { dev_dbg(dev, "%s error: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
> -#endif
> #ifndef vchiq_log_warning
> #define vchiq_log_warning(dev, cat, fmt, ...) \
> do { dev_dbg(dev, "%s warning: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> index 307a2d76cbb7..ba287cb4c87b 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> @@ -271,9 +271,9 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
> ret = -EFAULT;
> }
> } else {
> - vchiq_log_error(service->state->dev, VCHIQ_ARM,
> - "header %pK: bufsize %x < size %x",
> - header, args->bufsize, header->size);
> + dev_err(service->state->dev,
> + "arm: header %pK: bufsize %x < size %x\n",
> + header, args->bufsize, header->size);
> WARN(1, "invalid size\n");
> ret = -EMSGSIZE;
> }
> @@ -318,8 +318,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
> }
> mutex_unlock(&instance->bulk_waiter_list_mutex);
> if (!waiter) {
> - vchiq_log_error(service->state->dev, VCHIQ_ARM,
> - "no bulk_waiter found for pid %d", current->pid);
> + dev_err(service->state->dev,
> + "arm: no bulk_waiter found for pid %d\n", current->pid);
> ret = -ESRCH;
> goto out;
> }
> @@ -501,10 +501,10 @@ static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
> msglen = header->size + sizeof(struct vchiq_header);
> /* This must be a VCHIQ-style service */
> if (args->msgbufsize < msglen) {
> - vchiq_log_error(service->state->dev, VCHIQ_ARM,
> - "header %pK: msgbufsize %x < msglen %x",
> - header, args->msgbufsize, msglen);
> - WARN(1, "invalid message size\n");
> + dev_err(service->state->dev,
> + "arm: header %pK: msgbufsize %x < msglen %x\n",
> + header, args->msgbufsize, msglen);
> + WARN(1, "invalid message size\n");
> if (ret == 0)
> ret = -EMSGSIZE;
> break;
> @@ -618,9 +618,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> }
> rc = mutex_lock_killable(&instance->state->mutex);
> if (rc) {
> - vchiq_log_error(instance->state->dev, VCHIQ_ARM,
> - "vchiq: connect: could not lock mutex for state %d: %d",
> - instance->state->id, rc);
> + dev_err(instance->state->dev,
> + "arm: vchiq: connect: could not lock mutex for state %d: %d\n",
> + instance->state->id, rc);
I think this should be dev_dbg
> ret = -EINTR;
> break;
> }
> @@ -630,8 +630,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> if (!status)
> instance->connected = 1;
> else
> - vchiq_log_error(instance->state->dev, VCHIQ_ARM,
> - "vchiq: could not connect: %d", status);
> + dev_err(instance->state->dev,
> + "arm: vchiq: could not connect: %d\n", status);
> break;
>
> case VCHIQ_IOC_CREATE_SERVICE: {
> @@ -700,13 +700,13 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> vchiq_use_service_internal(service) :
> vchiq_release_service_internal(service);
> if (ret) {
> - vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
> - "%s: cmd %s returned error %ld for service %p4cc:%03d",
> - __func__, (cmd == VCHIQ_IOC_USE_SERVICE) ?
> - "VCHIQ_IOC_USE_SERVICE" :
> - "VCHIQ_IOC_RELEASE_SERVICE",
> - ret, &service->base.fourcc,
> - service->client_id);
> + dev_err(instance->state->dev,
> + "suspend: cmd %s returned error %ld for service %p4cc:%03d\n",
> + (cmd == VCHIQ_IOC_USE_SERVICE) ?
> + "VCHIQ_IOC_USE_SERVICE" :
> + "VCHIQ_IOC_RELEASE_SERVICE",
> + ret, &service->base.fourcc,
> + service->client_id);
> }
> } else {
> ret = -EINVAL;
> @@ -1173,8 +1173,7 @@ static int vchiq_open(struct inode *inode, struct file *file)
> vchiq_log_debug(state->dev, VCHIQ_ARM, "vchiq_open");
>
> if (!state) {
> - vchiq_log_error(state->dev, VCHIQ_ARM,
> - "vchiq has no connection to VideoCore");
> + dev_err(state->dev, "arm: vchiq has no connection to VideoCore\n");
> return -ENOTCONN;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-05 19:32 ` Stefan Wahren
@ 2023-12-06 5:27 ` Umang Jain
2023-12-06 21:34 ` Stefan Wahren
2023-12-07 2:39 ` Greg Kroah-Hartman
0 siblings, 2 replies; 16+ messages in thread
From: Umang Jain @ 2023-12-06 5:27 UTC (permalink / raw)
To: Stefan Wahren, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Stefan
On 12/6/23 1:02 AM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 05.12.23 um 09:41 schrieb Umang Jain:
>> Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the usage
>> of dev_err() directly.
> sorry, i missed to review the last change. So the change won't be that
> trivial.
All the changes you said are valid and I have taken a note of it.
However, I think it would best that if we can address them in a separate
series on top of this (and I will do it). It would seem to be
streamlined on top of this.
>>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>> .../interface/vchiq_arm/vchiq_arm.c | 50 ++++----
>> .../interface/vchiq_arm/vchiq_connected.c | 6 +-
>> .../interface/vchiq_arm/vchiq_core.c | 113 +++++++++---------
>> .../interface/vchiq_arm/vchiq_core.h | 4 -
>> .../interface/vchiq_arm/vchiq_dev.c | 45 ++++---
>> 5 files changed, 101 insertions(+), 117 deletions(-)
>>
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> index 7978fb6dc4fb..b403400edd8e 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> @@ -683,8 +683,7 @@ int vchiq_initialise(struct vchiq_instance
>> **instance_out)
>> usleep_range(500, 600);
>> }
>> if (i == VCHIQ_INIT_RETRIES) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not
>> initialized\n",
>> - __func__);
>> + dev_err(state->dev, "core: %s: Videocore not initialized\n",
>> __func__);
>> ret = -ENOTCONN;
>> goto failed;
>> } else if (i > 0) {
>> @@ -694,8 +693,7 @@ int vchiq_initialise(struct vchiq_instance
>> **instance_out)
>>
>> instance = kzalloc(sizeof(*instance), GFP_KERNEL);
>> if (!instance) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "%s: error allocating vchiq instance\n", __func__);
>> + dev_err(state->dev, "core: %s: Cannot allocate vchiq
>> instance\n", __func__);
> please drop because kzalloc already report such errors
>> ret = -ENOMEM;
>> goto failed;
>> }
>> @@ -967,8 +965,7 @@ vchiq_blocking_bulk_transfer(struct
>> vchiq_instance *instance, unsigned int handl
>> } else {
>> waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
>> if (!waiter) {
>> - vchiq_log_error(service->state->dev, VCHIQ_CORE,
>> - "%s - out of memory", __func__);
>> + dev_err(service->state->dev, "core: %s: - Out of
>> memory\n", __func__);
> ditto
>> return -ENOMEM;
>> }
>> }
>> @@ -1290,8 +1287,8 @@ vchiq_keepalive_vchiq_callback(struct
>> vchiq_instance *instance,
>> struct vchiq_header *header,
>> unsigned int service_user, void *bulk_user)
>> {
>> - vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
>> - "%s callback reason %d", __func__, reason);
>> + dev_err(instance->state->dev, "suspend: %s: callback reason %d\n",
>> + __func__, reason);
>> return 0;
>> }
>>
>> @@ -1315,22 +1312,20 @@ vchiq_keepalive_thread_func(void *v)
>>
>> ret = vchiq_initialise(&instance);
>> if (ret) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "%s vchiq_initialise failed %d", __func__, ret);
>> + dev_err(state->dev, "suspend: %s: vchiq_initialise failed
>> %d\n", __func__, ret);
>> goto exit;
>> }
>>
>> status = vchiq_connect(instance);
>> if (status) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "%s vchiq_connect failed %d", __func__, status);
>> + dev_err(state->dev, "suspend: %s: vchiq_connect failed
>> %d\n", __func__, status);
>> goto shutdown;
>> }
>>
>> status = vchiq_add_service(instance, ¶ms, &ka_handle);
>> if (status) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "%s vchiq_open_service failed %d", __func__, status);
>> + dev_err(state->dev, "suspend: %s: vchiq_open_service failed
>> %d\n",
>> + __func__, status);
>> goto shutdown;
>> }
>>
>> @@ -1338,8 +1333,7 @@ vchiq_keepalive_thread_func(void *v)
>> long rc = 0, uc = 0;
>>
>> if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "%s interrupted", __func__);
>> + dev_err(state->dev, "suspend: %s: interrupted\n",
>> __func__);
> I think this isn't an error and we should use dev_dbg here.
>> flush_signals(current);
>> continue;
>> }
>> @@ -1359,16 +1353,15 @@ vchiq_keepalive_thread_func(void *v)
>> atomic_inc(&arm_state->ka_use_ack_count);
>> status = vchiq_use_service(instance, ka_handle);
>> if (status) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "%s vchiq_use_service error %d", __func__,
>> status);
>> + dev_err(state->dev, "suspend: %s: vchiq_use_service
>> error %d\n",
>> + __func__, status);
>> }
>> }
>> while (rc--) {
>> status = vchiq_release_service(instance, ka_handle);
>> if (status) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "%s vchiq_release_service error %d", __func__,
>> - status);
>> + dev_err(state->dev, "suspend: %s:
>> vchiq_release_service error %d\n",
>> + __func__, status);
>> }
>> }
>> }
>> @@ -1403,7 +1396,7 @@ vchiq_use_internal(struct vchiq_state *state,
>> struct vchiq_service *service,
>> service->client_id);
>> entity_uc = &service->service_use_count;
>> } else {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND, "%s null service
>> ptr", __func__);
>> + dev_err(state->dev, "suspend: %s: null service ptr\n",
>> __func__);
> Instead of dev_err() i would suggest a WARN() here
>> ret = -EINVAL;
>> goto out;
>> }
>> @@ -1686,10 +1679,10 @@ vchiq_check_service(struct vchiq_service
>> *service)
>> read_unlock_bh(&arm_state->susp_res_lock);
>>
>> if (ret) {
>> - vchiq_log_error(service->state->dev, VCHIQ_SUSPEND,
>> - "%s ERROR - %p4cc:%d service count %d, state count
>> %d", __func__,
>> - &service->base.fourcc, service->client_id,
>> - service->service_use_count,
>> arm_state->videocore_use_count);
>> + dev_err(service->state->dev,
>> + "suspend: %s: %p4cc:%d service count %d, state count
>> %d\n",
>> + __func__, &service->base.fourcc, service->client_id,
>> + service->service_use_count,
>> arm_state->videocore_use_count);
>> vchiq_dump_service_use_state(service->state);
>> }
>> out:
>> @@ -1722,9 +1715,8 @@ void vchiq_platform_conn_state_changed(struct
>> vchiq_state *state,
>> (void *)state,
>> threadname);
>> if (IS_ERR(arm_state->ka_thread)) {
>> - vchiq_log_error(state->dev, VCHIQ_SUSPEND,
>> - "vchiq: FATAL: couldn't create thread %s",
>> - threadname);
>> + dev_err(state->dev, "suspend: Couldn't create thread %s\n",
>> + threadname);
>> } else {
>> wake_up_process(arm_state->ka_thread);
>> }
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
>> index 21f9fa1a1713..3cad13f09e37 100644
>> ---
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
>> +++
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
>> @@ -39,9 +39,9 @@ void vchiq_add_connected_callback(struct
>> vchiq_device *device, void (*callback)(
>> callback();
>> } else {
>> if (g_num_deferred_callbacks >= MAX_CALLBACKS) {
>> - vchiq_log_error(&device->dev, VCHIQ_CORE,
>> - "There already %d callback registered - please
>> increase MAX_CALLBACKS",
>> - g_num_deferred_callbacks);
>> + dev_err(&device->dev,
>> + "core: There already %d callback registered - please
>> increase MAX_CALLBACKS\n",
>> + g_num_deferred_callbacks);
>> } else {
>> g_deferred_callback[g_num_deferred_callbacks] =
>> callback;
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> index e0022acb4c58..63f412815a32 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> @@ -741,10 +741,10 @@ process_free_data_message(struct vchiq_state
>> *state, u32 *service_found,
>> */
>> complete("a->quota_event);
>> } else if (count == 0) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "service %d message_use_count=%d (header %pK, msgid
>> %x, header->msgid %x, header->size %x)",
>> - port, quota->message_use_count, header, msgid,
>> header->msgid,
>> - header->size);
>> + dev_err(state->dev,
>> + "core: service %d message_use_count=%d (header %pK,
>> msgid %x, header->msgid %x, header->size %x)\n",
>> + port, quota->message_use_count, header, msgid,
>> + header->msgid, header->size);
>> WARN(1, "invalid message use count\n");
>> }
>> if (!BITSET_IS_SET(service_found, port)) {
>> @@ -766,9 +766,9 @@ process_free_data_message(struct vchiq_state
>> *state, u32 *service_found,
>> vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq:%d
>> %x@%pK - slot_use->%d",
>> state->id, port, header->size, header, count - 1);
>> } else {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "service %d slot_use_count=%d (header %pK, msgid
>> %x, header->msgid %x, header->size %x)",
>> - port, count, header, msgid, header->msgid,
>> header->size);
>> + dev_err(state->dev,
>> + "core: service %d slot_use_count=%d (header %pK,
>> msgid %x, header->msgid %x, header->size %x)\n",
>> + port, count, header, msgid, header->msgid,
>> header->size);
>> WARN(1, "bad slot use count\n");
>> }
>> }
>> @@ -831,9 +831,9 @@ process_free_queue(struct vchiq_state *state, u32
>> *service_found,
>>
>> pos += calc_stride(header->size);
>> if (pos > VCHIQ_SLOT_SIZE) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "pfq - pos %x: header %pK, msgid %x,
>> header->msgid %x, header->size %x",
>> - pos, header, msgid, header->msgid,
>> header->size);
>> + dev_err(state->dev,
>> + "core: pfq - pos %x: header %pK, msgid %x,
>> header->msgid %x, header->size %x\n",
>> + pos, header, msgid, header->msgid, header->size);
>> WARN(1, "invalid slot position\n");
>> }
>> }
>> @@ -1167,8 +1167,8 @@ queue_message_sync(struct vchiq_state *state,
>> struct vchiq_service *service,
>> int oldmsgid = header->msgid;
>>
>> if (oldmsgid != VCHIQ_MSGID_PADDING)
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "%d: qms - msgid
>> %x, not PADDING",
>> - state->id, oldmsgid);
>> + dev_err(state->dev, "core: %d: qms - msgid %x, not
>> PADDING\n",
>> + state->id, oldmsgid);
>> }
>>
>> vchiq_log_debug(state->dev, VCHIQ_SYNC,
>> @@ -1616,10 +1616,10 @@ parse_message(struct vchiq_state *state,
>> struct vchiq_header *header)
>> }
>>
>> if (!service) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "%d: prs %s@%pK (%d->%d) - invalid/closed
>> service %d",
>> - state->id, msg_type_str(type), header, remoteport,
>> - localport, localport);
>> + dev_err(state->dev,
>> + "core: %d: prs %s@%pK (%d->%d) - invalid/closed
>> service %d\n",
>> + state->id, msg_type_str(type), header, remoteport,
>> + localport, localport);
>> goto skip_message;
>> }
>> break;
>> @@ -1640,9 +1640,8 @@ parse_message(struct vchiq_state *state, struct
>> vchiq_header *header)
>>
>> if (((unsigned long)header & VCHIQ_SLOT_MASK) +
>> calc_stride(size) > VCHIQ_SLOT_SIZE) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "header %pK (msgid %x) - size %x too big for slot",
>> - header, (unsigned int)msgid, (unsigned int)size);
>> + dev_err(state->dev, "core: header %pK (msgid %x) - size %x
>> too big for slot\n",
>> + header, (unsigned int)msgid, (unsigned int)size);
>> WARN(1, "oversized for slot\n");
>> }
>>
>> @@ -1668,8 +1667,8 @@ parse_message(struct vchiq_state *state, struct
>> vchiq_header *header)
>> set_service_state(service, VCHIQ_SRVSTATE_OPEN);
>> complete(&service->remove_event);
>> } else {
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "OPENACK
>> received in state %s",
>> - srvstate_names[service->srvstate]);
>> + dev_err(state->dev, "core: OPENACK received in state %s\n",
>> + srvstate_names[service->srvstate]);
>> }
>> break;
>> case VCHIQ_MSG_CLOSE:
>> @@ -1740,11 +1739,10 @@ parse_message(struct vchiq_state *state,
>> struct vchiq_header *header)
>> }
>> if ((int)(queue->remote_insert -
>> queue->local_insert) >= 0) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "%d: prs %s@%pK (%d->%d) unexpected
>> (ri=%d,li=%d)",
>> - state->id, msg_type_str(type), header,
>> remoteport,
>> - localport, queue->remote_insert,
>> - queue->local_insert);
>> + dev_err(state->dev,
>> + "core: %d: prs %s@%pK (%d->%d) unexpected
>> (ri=%d,li=%d)\n",
>> + state->id, msg_type_str(type), header, remoteport,
>> + localport, queue->remote_insert,
>> queue->local_insert);
>> mutex_unlock(&service->bulk_mutex);
>> break;
>> }
>> @@ -1790,8 +1788,8 @@ parse_message(struct vchiq_state *state, struct
>> vchiq_header *header)
>> vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs
>> PAUSE@%pK,%x",
>> state->id, header, size);
>> if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "%d: PAUSE received in state PAUSED", state->id);
>> + dev_err(state->dev, "core: %d: PAUSE received in state
>> PAUSED\n",
>> + state->id);
>> break;
>> }
>> if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
>> @@ -1821,8 +1819,8 @@ parse_message(struct vchiq_state *state, struct
>> vchiq_header *header)
>> break;
>>
>> default:
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "%d: prs invalid
>> msgid %x@%pK,%x",
>> - state->id, msgid, header, size);
>> + dev_err(state->dev, "core: %d: prs invalid msgid %x@%pK,%x\n",
>> + state->id, msgid, header, size);
>> WARN(1, "invalid message\n");
>> break;
>> }
>> @@ -1932,7 +1930,7 @@ handle_poll(struct vchiq_state *state)
>> * since the PAUSE should have flushed
>> * through outstanding messages.
>> */
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "Failed to send
>> RESUME message");
>> + dev_err(state->dev, "core: Failed to send RESUME
>> message\n");
>> }
>> break;
>> default:
>> @@ -2032,10 +2030,10 @@ sync_func(void *v)
>> service = find_service_by_port(state, localport);
>>
>> if (!service) {
>> - vchiq_log_error(state->dev, VCHIQ_SYNC,
>> - "%d: sf %s@%pK (%d->%d) - invalid/closed service
>> %d",
>> - state->id, msg_type_str(type), header,
>> - remoteport, localport, localport);
>> + dev_err(state->dev,
>> + "sync: %d: sf %s@%pK (%d->%d) - invalid/closed
>> service %d\n",
>> + state->id, msg_type_str(type), header, remoteport,
>> + localport, localport);
>> release_message_sync(state, header);
>> continue;
>> }
>> @@ -2077,15 +2075,15 @@ sync_func(void *v)
>> (service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
>> if (make_service_callback(service,
>> VCHIQ_MESSAGE_AVAILABLE, header,
>> NULL) == -EAGAIN)
>> - vchiq_log_error(state->dev, VCHIQ_SYNC,
>> - "synchronous callback to service %d
>> returns -EAGAIN",
>> - localport);
>> + dev_err(state->dev,
>> + "sync: error: synchronous callback to
>> service %d returns -EAGAIN\n",
>> + localport);
>> }
>> break;
>>
>> default:
>> - vchiq_log_error(state->dev, VCHIQ_SYNC, "%d: sf
>> unexpected msgid %x@%pK,%x",
>> - state->id, msgid, header, size);
>> + dev_err(state->dev, "sync: error: %d: sf unexpected
>> msgid %x@%pK,%x\n",
>> + state->id, msgid, header, size);
>> release_message_sync(state, header);
>> break;
>> }
>> @@ -2118,8 +2116,8 @@ vchiq_init_slots(struct device *dev, void
>> *mem_base, int mem_size)
>> num_slots -= first_data_slot;
>>
>> if (num_slots < 4) {
>> - vchiq_log_error(dev, VCHIQ_CORE, "%s - insufficient memory
>> %x bytes",
>> - __func__, mem_size);
>> + dev_err(dev, "core: %s: Insufficient memory %x bytes\n",
>> + __func__, mem_size);
>> return NULL;
>> }
>>
>> @@ -2500,11 +2498,10 @@ vchiq_open_service_internal(struct
>> vchiq_service *service, int client_id)
>> } else if ((service->srvstate != VCHIQ_SRVSTATE_OPEN) &&
>> (service->srvstate != VCHIQ_SRVSTATE_OPENSYNC)) {
>> if (service->srvstate != VCHIQ_SRVSTATE_CLOSEWAIT)
>> - vchiq_log_error(service->state->dev, VCHIQ_CORE,
>> - "%d: osi - srvstate = %s (ref %u)",
>> - service->state->id,
>> - srvstate_names[service->srvstate],
>> - kref_read(&service->ref_count));
>> + dev_err(service->state->dev,
>> + "core: %d: osi - srvstate = %s (ref %u)\n",
>> + service->state->id, srvstate_names[service->srvstate],
>> + kref_read(&service->ref_count));
>> status = -EINVAL;
>> VCHIQ_SERVICE_STATS_INC(service, error_count);
>> vchiq_release_service_internal(service);
>> @@ -2565,9 +2562,9 @@ release_service_messages(struct vchiq_service
>> *service)
>> }
>> pos += calc_stride(header->size);
>> if (pos > VCHIQ_SLOT_SIZE) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE,
>> - "fsi - pos %x: header %pK, msgid %x,
>> header->msgid %x, header->size %x",
>> - pos, header, msgid, header->msgid,
>> header->size);
>> + dev_err(state->dev,
>> + "core: fsi - pos %x: header %pK, msgid %x,
>> header->msgid %x, header->size %x\n",
>> + pos, header, msgid, header->msgid, header->size);
>> WARN(1, "invalid slot position\n");
>> }
>> }
>> @@ -2621,8 +2618,8 @@ close_service_complete(struct vchiq_service
>> *service, int failstate)
>> case VCHIQ_SRVSTATE_LISTENING:
>> break;
>> default:
>> - vchiq_log_error(service->state->dev, VCHIQ_CORE, "%s(%x)
>> called in state %s",
>> - __func__, service->handle,
>> srvstate_names[service->srvstate]);
>> + dev_err(service->state->dev, "core: (%x) called in state %s\n",
>> + service->handle, srvstate_names[service->srvstate]);
>> WARN(1, "%s in unexpected state\n", __func__);
>> return -EINVAL;
>> }
>> @@ -2677,8 +2674,8 @@ vchiq_close_service_internal(struct
>> vchiq_service *service, int close_recvd)
>> case VCHIQ_SRVSTATE_LISTENING:
>> case VCHIQ_SRVSTATE_CLOSEWAIT:
>> if (close_recvd) {
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "%s(1) called in
>> state %s",
>> - __func__, srvstate_names[service->srvstate]);
>> + dev_err(state->dev, "core: (1) called in state %s\n",
>> + srvstate_names[service->srvstate]);
>> } else if (is_server) {
>> if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
>> status = -EINVAL;
>> @@ -2765,8 +2762,8 @@ vchiq_close_service_internal(struct
>> vchiq_service *service, int close_recvd)
>> break;
>>
>> default:
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "%s(%d) called in
>> state %s", __func__,
>> - close_recvd, srvstate_names[service->srvstate]);
>> + dev_err(state->dev, "core: (%d) called in state %s\n",
>> + close_recvd, srvstate_names[service->srvstate]);
>> break;
>> }
>>
>> @@ -2805,8 +2802,8 @@ vchiq_free_service_internal(struct
>> vchiq_service *service)
>> case VCHIQ_SRVSTATE_CLOSEWAIT:
>> break;
>> default:
>> - vchiq_log_error(state->dev, VCHIQ_CORE, "%d: fsi - (%d) in
>> state %s", state->id,
>> - service->localport, srvstate_names[service->srvstate]);
>> + dev_err(state->dev, "core: %d: fsi - (%d) in state %s\n",
>> + state->id, service->localport,
>> srvstate_names[service->srvstate]);
>> return;
>> }
>>
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> index 564b5c707267..d7dcd17e4bff 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> @@ -53,10 +53,6 @@ static inline const char *log_category_str(enum
>> vchiq_log_category c)
>> return strings[c];
>> };
>>
>> -#ifndef vchiq_log_error
>> -#define vchiq_log_error(dev, cat, fmt, ...) \
>> - do { dev_dbg(dev, "%s error: " fmt, log_category_str(cat),
>> ##__VA_ARGS__); } while (0)
>> -#endif
>> #ifndef vchiq_log_warning
>> #define vchiq_log_warning(dev, cat, fmt, ...) \
>> do { dev_dbg(dev, "%s warning: " fmt, log_category_str(cat),
>> ##__VA_ARGS__); } while (0)
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> index 307a2d76cbb7..ba287cb4c87b 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> @@ -271,9 +271,9 @@ static int vchiq_ioc_dequeue_message(struct
>> vchiq_instance *instance,
>> ret = -EFAULT;
>> }
>> } else {
>> - vchiq_log_error(service->state->dev, VCHIQ_ARM,
>> - "header %pK: bufsize %x < size %x",
>> - header, args->bufsize, header->size);
>> + dev_err(service->state->dev,
>> + "arm: header %pK: bufsize %x < size %x\n",
>> + header, args->bufsize, header->size);
>> WARN(1, "invalid size\n");
>> ret = -EMSGSIZE;
>> }
>> @@ -318,8 +318,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct
>> vchiq_instance *instance,
>> }
>> mutex_unlock(&instance->bulk_waiter_list_mutex);
>> if (!waiter) {
>> - vchiq_log_error(service->state->dev, VCHIQ_ARM,
>> - "no bulk_waiter found for pid %d", current->pid);
>> + dev_err(service->state->dev,
>> + "arm: no bulk_waiter found for pid %d\n",
>> current->pid);
>> ret = -ESRCH;
>> goto out;
>> }
>> @@ -501,10 +501,10 @@ static int vchiq_ioc_await_completion(struct
>> vchiq_instance *instance,
>> msglen = header->size + sizeof(struct vchiq_header);
>> /* This must be a VCHIQ-style service */
>> if (args->msgbufsize < msglen) {
>> - vchiq_log_error(service->state->dev, VCHIQ_ARM,
>> - "header %pK: msgbufsize %x < msglen %x",
>> - header, args->msgbufsize, msglen);
>> - WARN(1, "invalid message size\n");
>> + dev_err(service->state->dev,
>> + "arm: header %pK: msgbufsize %x < msglen %x\n",
>> + header, args->msgbufsize, msglen);
>> + WARN(1, "invalid message size\n");
>> if (ret == 0)
>> ret = -EMSGSIZE;
>> break;
>> @@ -618,9 +618,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
>> unsigned long arg)
>> }
>> rc = mutex_lock_killable(&instance->state->mutex);
>> if (rc) {
>> - vchiq_log_error(instance->state->dev, VCHIQ_ARM,
>> - "vchiq: connect: could not lock mutex for state
>> %d: %d",
>> - instance->state->id, rc);
>> + dev_err(instance->state->dev,
>> + "arm: vchiq: connect: could not lock mutex for state
>> %d: %d\n",
>> + instance->state->id, rc);
> I think this should be dev_dbg
>> ret = -EINTR;
>> break;
>> }
>> @@ -630,8 +630,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
>> unsigned long arg)
>> if (!status)
>> instance->connected = 1;
>> else
>> - vchiq_log_error(instance->state->dev, VCHIQ_ARM,
>> - "vchiq: could not connect: %d", status);
>> + dev_err(instance->state->dev,
>> + "arm: vchiq: could not connect: %d\n", status);
>> break;
>>
>> case VCHIQ_IOC_CREATE_SERVICE: {
>> @@ -700,13 +700,13 @@ vchiq_ioctl(struct file *file, unsigned int
>> cmd, unsigned long arg)
>> vchiq_use_service_internal(service) :
>> vchiq_release_service_internal(service);
>> if (ret) {
>> - vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
>> - "%s: cmd %s returned error %ld for service
>> %p4cc:%03d",
>> - __func__, (cmd == VCHIQ_IOC_USE_SERVICE) ?
>> - "VCHIQ_IOC_USE_SERVICE" :
>> - "VCHIQ_IOC_RELEASE_SERVICE",
>> - ret, &service->base.fourcc,
>> - service->client_id);
>> + dev_err(instance->state->dev,
>> + "suspend: cmd %s returned error %ld for service
>> %p4cc:%03d\n",
>> + (cmd == VCHIQ_IOC_USE_SERVICE) ?
>> + "VCHIQ_IOC_USE_SERVICE" :
>> + "VCHIQ_IOC_RELEASE_SERVICE",
>> + ret, &service->base.fourcc,
>> + service->client_id);
>> }
>> } else {
>> ret = -EINVAL;
>> @@ -1173,8 +1173,7 @@ static int vchiq_open(struct inode *inode,
>> struct file *file)
>> vchiq_log_debug(state->dev, VCHIQ_ARM, "vchiq_open");
>>
>> if (!state) {
>> - vchiq_log_error(state->dev, VCHIQ_ARM,
>> - "vchiq has no connection to VideoCore");
>> + dev_err(state->dev, "arm: vchiq has no connection to
>> VideoCore\n");
>> return -ENOTCONN;
>> }
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-06 5:27 ` Umang Jain
@ 2023-12-06 21:34 ` Stefan Wahren
2023-12-07 8:41 ` Umang Jain
2023-12-07 2:39 ` Greg Kroah-Hartman
1 sibling, 1 reply; 16+ messages in thread
From: Stefan Wahren @ 2023-12-06 21:34 UTC (permalink / raw)
To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Umang,
Am 06.12.23 um 06:27 schrieb Umang Jain:
> Hi Stefan
>
> On 12/6/23 1:02 AM, Stefan Wahren wrote:
>> Hi Umang,
>>
>> Am 05.12.23 um 09:41 schrieb Umang Jain:
>>> Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the usage
>>> of dev_err() directly.
>> sorry, i missed to review the last change. So the change won't be that
>> trivial.
>
> All the changes you said are valid and I have taken a note of it.
>
> However, I think it would best that if we can address them in a
> separate series on top of this (and I will do it). It would seem to be
> streamlined on top of this.
i would address the comments as separate patches and then finally
convert the trivial rest with such a patch within one series. This
avoids unnecessary changes and reviewes. But that's just my opinion.
Best regards
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-06 21:34 ` Stefan Wahren
@ 2023-12-07 8:41 ` Umang Jain
2023-12-07 22:28 ` Stefan Wahren
0 siblings, 1 reply; 16+ messages in thread
From: Umang Jain @ 2023-12-07 8:41 UTC (permalink / raw)
To: Stefan Wahren, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Stefan
On 12/7/23 3:04 AM, Stefan Wahren wrote:
> Hi Umang,
>
> Am 06.12.23 um 06:27 schrieb Umang Jain:
>> Hi Stefan
>>
>> On 12/6/23 1:02 AM, Stefan Wahren wrote:
>>> Hi Umang,
>>>
>>> Am 05.12.23 um 09:41 schrieb Umang Jain:
>>>> Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the
>>>> usage
>>>> of dev_err() directly.
>>> sorry, i missed to review the last change. So the change won't be that
>>> trivial.
>>
>> All the changes you said are valid and I have taken a note of it.
>>
>> However, I think it would best that if we can address them in a
>> separate series on top of this (and I will do it). It would seem to be
>> streamlined on top of this.
> i would address the comments as separate patches and then finally
> convert the trivial rest with such a patch within one series. This
> avoids unnecessary changes and reviewes. But that's just my opinion.
>
Series is merged by Greg KH (Thank you).
Do you have more comments on 3/4 and 4/4 or are they waiting on your
review queue. Let me know so that I can prepare a follow up series in
-one- go.
thanks!
> Best regards
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-07 8:41 ` Umang Jain
@ 2023-12-07 22:28 ` Stefan Wahren
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Wahren @ 2023-12-07 22:28 UTC (permalink / raw)
To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Umang,
Am 07.12.23 um 09:41 schrieb Umang Jain:
> Hi Stefan
>
> On 12/7/23 3:04 AM, Stefan Wahren wrote:
>> Hi Umang,
>>
>> Am 06.12.23 um 06:27 schrieb Umang Jain:
>>> Hi Stefan
>>>
>>> On 12/6/23 1:02 AM, Stefan Wahren wrote:
>>>> Hi Umang,
>>>>
>>>> Am 05.12.23 um 09:41 schrieb Umang Jain:
>>>>> Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the
>>>>> usage
>>>>> of dev_err() directly.
>>>> sorry, i missed to review the last change. So the change won't be that
>>>> trivial.
>>>
>>> All the changes you said are valid and I have taken a note of it.
>>>
>>> However, I think it would best that if we can address them in a
>>> separate series on top of this (and I will do it). It would seem to be
>>> streamlined on top of this.
>> i would address the comments as separate patches and then finally
>> convert the trivial rest with such a patch within one series. This
>> avoids unnecessary changes and reviewes. But that's just my opinion.
>>
>
> Series is merged by Greg KH (Thank you).
>
> Do you have more comments on 3/4 and 4/4 or are they waiting on your
> review queue. Let me know so that I can prepare a follow up series in
> -one- go.
i was busy. I will look at the rest tomorrow.
Best regards
>
> thanks!
>> Best regards
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
2023-12-06 5:27 ` Umang Jain
2023-12-06 21:34 ` Stefan Wahren
@ 2023-12-07 2:39 ` Greg Kroah-Hartman
1 sibling, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2023-12-07 2:39 UTC (permalink / raw)
To: Umang Jain
Cc: Stefan Wahren, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel, Stefan Wahren, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
On Wed, Dec 06, 2023 at 10:57:59AM +0530, Umang Jain wrote:
> Hi Stefan
>
> On 12/6/23 1:02 AM, Stefan Wahren wrote:
> > Hi Umang,
> >
> > Am 05.12.23 um 09:41 schrieb Umang Jain:
> > > Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the usage
> > > of dev_err() directly.
> > sorry, i missed to review the last change. So the change won't be that
> > trivial.
>
> All the changes you said are valid and I have taken a note of it.
>
> However, I think it would best that if we can address them in a separate
> series on top of this (and I will do it). It would seem to be streamlined on
> top of this.
Agreed, I'll merge this now so you can make forward progress here,
thanks!
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/4] staging: vc04_services: Drop vchiq_log_warning() in favour of dev_warn
2023-12-05 8:41 [PATCH v3 0/4] staging: vc04_services: Drop custom logging Umang Jain
2023-12-05 8:41 ` [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err Umang Jain
@ 2023-12-05 8:41 ` Umang Jain
2023-12-05 19:37 ` Stefan Wahren
2023-12-05 8:41 ` [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg Umang Jain
2023-12-05 8:41 ` [PATCH v3 4/4] staging: vc04_services: Drop vchiq_log_debug() " Umang Jain
3 siblings, 1 reply; 16+ messages in thread
From: Umang Jain @ 2023-12-05 8:41 UTC (permalink / raw)
To: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere, Umang Jain
Drop vchiq_log_warning() macro which wraps dev_dbg(). Introduce the usage
of dev_warn() directly.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
.../interface/vchiq_arm/vchiq_arm.c | 30 +++++++++----------
.../interface/vchiq_arm/vchiq_core.c | 30 +++++++++----------
.../interface/vchiq_arm/vchiq_core.h | 4 ---
3 files changed, 29 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index b403400edd8e..56326b59a718 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -687,8 +687,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
ret = -ENOTCONN;
goto failed;
} else if (i > 0) {
- vchiq_log_warning(state->dev, VCHIQ_CORE,
- "%s: videocore initialized after %d retries\n", __func__, i);
+ dev_warn(state->dev, "core: %s: videocore initialized after %d retries\n",
+ __func__, i);
}
instance = kzalloc(sizeof(*instance), GFP_KERNEL);
@@ -1644,20 +1644,19 @@ vchiq_dump_service_use_state(struct vchiq_state *state)
read_unlock_bh(&arm_state->susp_res_lock);
if (only_nonzero)
- vchiq_log_warning(state->dev, VCHIQ_SUSPEND,
- "Too many active services (%d). Only dumping up to first %d services with non-zero use-count",
- active_services, found);
+ dev_warn(state->dev,
+ "suspend: Too many active services (%d). Only dumping up to first %d services with non-zero use-count\n",
+ active_services, found);
for (i = 0; i < found; i++) {
- vchiq_log_warning(state->dev, VCHIQ_SUSPEND,
- "%p4cc:%d service count %d %s",
- &service_data[i].fourcc,
- service_data[i].clientid, service_data[i].use_count,
- service_data[i].use_count ? nz : "");
+ dev_warn(state->dev,
+ "suspend: %p4cc:%d service count %d %s\n",
+ &service_data[i].fourcc,
+ service_data[i].clientid, service_data[i].use_count,
+ service_data[i].use_count ? nz : "");
}
- vchiq_log_warning(state->dev, VCHIQ_SUSPEND, "VCHIQ use count %d", peer_count);
- vchiq_log_warning(state->dev, VCHIQ_SUSPEND, "Overall vchiq instance use count %d",
- vc_use_count);
+ dev_warn(state->dev, "suspend: VCHIQ use count %d\n", peer_count);
+ dev_warn(state->dev, "suspend: Overall vchiq instance use count %d\n", vc_use_count);
kfree(service_data);
}
@@ -1771,8 +1770,7 @@ static int vchiq_probe(struct platform_device *pdev)
*/
err = vchiq_register_chrdev(&pdev->dev);
if (err) {
- vchiq_log_warning(&pdev->dev, VCHIQ_ARM,
- "Failed to initialize vchiq cdev");
+ dev_warn(&pdev->dev, "arm: Failed to initialize vchiq cdev\n");
goto error_exit;
}
@@ -1782,7 +1780,7 @@ static int vchiq_probe(struct platform_device *pdev)
return 0;
failed_platform_init:
- vchiq_log_warning(&pdev->dev, VCHIQ_ARM, "could not initialize vchiq platform");
+ dev_warn(&pdev->dev, "arm: Could not initialize vchiq platform\n");
error_exit:
return err;
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 63f412815a32..2fd793954001 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -465,9 +465,9 @@ make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
status = service->base.callback(service->instance, reason, header, service->handle,
bulk_userdata);
if (status && (status != -EAGAIN)) {
- vchiq_log_warning(service->state->dev, VCHIQ_CORE,
- "%d: ignoring ERROR from callback to service %x",
- service->state->id, service->handle);
+ dev_warn(service->state->dev,
+ "core: %d: ignoring ERROR from callback to service %x\n",
+ service->state->id, service->handle);
status = 0;
}
@@ -1609,10 +1609,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
vchiq_service_put(service);
service = get_connected_service(state, remoteport);
if (service)
- vchiq_log_warning(state->dev, VCHIQ_CORE,
- "%d: prs %s@%pK (%d->%d) - found connected service %d",
- state->id, msg_type_str(type), header,
- remoteport, localport, service->localport);
+ dev_warn(state->dev,
+ "core: %d: prs %s@%pK (%d->%d) - found connected service %d\n",
+ state->id, msg_type_str(type), header,
+ remoteport, localport, service->localport);
}
if (!service) {
@@ -2903,10 +2903,10 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
(service->srvstate == VCHIQ_SRVSTATE_OPEN))
break;
- vchiq_log_warning(service->state->dev, VCHIQ_CORE,
- "%d: close_service:%d - waiting in state %s",
- service->state->id, service->localport,
- srvstate_names[service->srvstate]);
+ dev_warn(service->state->dev,
+ "core: %d: close_service:%d - waiting in state %s\n",
+ service->state->id, service->localport,
+ srvstate_names[service->srvstate]);
}
if (!status &&
@@ -2964,10 +2964,10 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
(service->srvstate == VCHIQ_SRVSTATE_OPEN))
break;
- vchiq_log_warning(service->state->dev, VCHIQ_CORE,
- "%d: remove_service:%d - waiting in state %s",
- service->state->id, service->localport,
- srvstate_names[service->srvstate]);
+ dev_warn(service->state->dev,
+ "core: %d: remove_service:%d - waiting in state %s\n",
+ service->state->id, service->localport,
+ srvstate_names[service->srvstate]);
}
if (!status && (service->srvstate != VCHIQ_SRVSTATE_FREE))
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index d7dcd17e4bff..bc980a001711 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -53,10 +53,6 @@ static inline const char *log_category_str(enum vchiq_log_category c)
return strings[c];
};
-#ifndef vchiq_log_warning
-#define vchiq_log_warning(dev, cat, fmt, ...) \
- do { dev_dbg(dev, "%s warning: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
-#endif
#ifndef vchiq_log_debug
#define vchiq_log_debug(dev, cat, fmt, ...) \
do { dev_dbg(dev, "%s debug: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 2/4] staging: vc04_services: Drop vchiq_log_warning() in favour of dev_warn
2023-12-05 8:41 ` [PATCH v3 2/4] staging: vc04_services: Drop vchiq_log_warning() in favour of dev_warn Umang Jain
@ 2023-12-05 19:37 ` Stefan Wahren
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Wahren @ 2023-12-05 19:37 UTC (permalink / raw)
To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Umang,
Am 05.12.23 um 09:41 schrieb Umang Jain:
> Drop vchiq_log_warning() macro which wraps dev_dbg(). Introduce the usage
> of dev_warn() directly.
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 30 +++++++++----------
> .../interface/vchiq_arm/vchiq_core.c | 30 +++++++++----------
> .../interface/vchiq_arm/vchiq_core.h | 4 ---
> 3 files changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index b403400edd8e..56326b59a718 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -687,8 +687,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
> ret = -ENOTCONN;
> goto failed;
> } else if (i > 0) {
> - vchiq_log_warning(state->dev, VCHIQ_CORE,
> - "%s: videocore initialized after %d retries\n", __func__, i);
> + dev_warn(state->dev, "core: %s: videocore initialized after %d retries\n",
> + __func__, i);
> }
>
> instance = kzalloc(sizeof(*instance), GFP_KERNEL);
> @@ -1644,20 +1644,19 @@ vchiq_dump_service_use_state(struct vchiq_state *state)
> read_unlock_bh(&arm_state->susp_res_lock);
>
> if (only_nonzero)
> - vchiq_log_warning(state->dev, VCHIQ_SUSPEND,
> - "Too many active services (%d). Only dumping up to first %d services with non-zero use-count",
> - active_services, found);
> + dev_warn(state->dev,
> + "suspend: Too many active services (%d). Only dumping up to first %d services with non-zero use-count\n",
> + active_services, found);
>
> for (i = 0; i < found; i++) {
> - vchiq_log_warning(state->dev, VCHIQ_SUSPEND,
> - "%p4cc:%d service count %d %s",
> - &service_data[i].fourcc,
> - service_data[i].clientid, service_data[i].use_count,
> - service_data[i].use_count ? nz : "");
> + dev_warn(state->dev,
> + "suspend: %p4cc:%d service count %d %s\n",
> + &service_data[i].fourcc,
> + service_data[i].clientid, service_data[i].use_count,
> + service_data[i].use_count ? nz : "");
> }
> - vchiq_log_warning(state->dev, VCHIQ_SUSPEND, "VCHIQ use count %d", peer_count);
> - vchiq_log_warning(state->dev, VCHIQ_SUSPEND, "Overall vchiq instance use count %d",
> - vc_use_count);
> + dev_warn(state->dev, "suspend: VCHIQ use count %d\n", peer_count);
> + dev_warn(state->dev, "suspend: Overall vchiq instance use count %d\n", vc_use_count);
>
> kfree(service_data);
> }
> @@ -1771,8 +1770,7 @@ static int vchiq_probe(struct platform_device *pdev)
> */
> err = vchiq_register_chrdev(&pdev->dev);
> if (err) {
> - vchiq_log_warning(&pdev->dev, VCHIQ_ARM,
> - "Failed to initialize vchiq cdev");
> + dev_warn(&pdev->dev, "arm: Failed to initialize vchiq cdev\n");
I think a dev_err() would fit better here.
> goto error_exit;
> }
>
> @@ -1782,7 +1780,7 @@ static int vchiq_probe(struct platform_device *pdev)
> return 0;
>
> failed_platform_init:
> - vchiq_log_warning(&pdev->dev, VCHIQ_ARM, "could not initialize vchiq platform");
> + dev_warn(&pdev->dev, "arm: Could not initialize vchiq platform\n");
ditto
> error_exit:
> return err;
> }
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 63f412815a32..2fd793954001 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -465,9 +465,9 @@ make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
> status = service->base.callback(service->instance, reason, header, service->handle,
> bulk_userdata);
> if (status && (status != -EAGAIN)) {
> - vchiq_log_warning(service->state->dev, VCHIQ_CORE,
> - "%d: ignoring ERROR from callback to service %x",
> - service->state->id, service->handle);
> + dev_warn(service->state->dev,
> + "core: %d: ignoring ERROR from callback to service %x\n",
> + service->state->id, service->handle);
> status = 0;
> }
>
> @@ -1609,10 +1609,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> vchiq_service_put(service);
> service = get_connected_service(state, remoteport);
> if (service)
> - vchiq_log_warning(state->dev, VCHIQ_CORE,
> - "%d: prs %s@%pK (%d->%d) - found connected service %d",
> - state->id, msg_type_str(type), header,
> - remoteport, localport, service->localport);
> + dev_warn(state->dev,
> + "core: %d: prs %s@%pK (%d->%d) - found connected service %d\n",
> + state->id, msg_type_str(type), header,
> + remoteport, localport, service->localport);
> }
>
> if (!service) {
> @@ -2903,10 +2903,10 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
> (service->srvstate == VCHIQ_SRVSTATE_OPEN))
> break;
>
> - vchiq_log_warning(service->state->dev, VCHIQ_CORE,
> - "%d: close_service:%d - waiting in state %s",
> - service->state->id, service->localport,
> - srvstate_names[service->srvstate]);
> + dev_warn(service->state->dev,
> + "core: %d: close_service:%d - waiting in state %s\n",
> + service->state->id, service->localport,
> + srvstate_names[service->srvstate]);
> }
>
> if (!status &&
> @@ -2964,10 +2964,10 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
> (service->srvstate == VCHIQ_SRVSTATE_OPEN))
> break;
>
> - vchiq_log_warning(service->state->dev, VCHIQ_CORE,
> - "%d: remove_service:%d - waiting in state %s",
> - service->state->id, service->localport,
> - srvstate_names[service->srvstate]);
> + dev_warn(service->state->dev,
> + "core: %d: remove_service:%d - waiting in state %s\n",
> + service->state->id, service->localport,
> + srvstate_names[service->srvstate]);
> }
>
> if (!status && (service->srvstate != VCHIQ_SRVSTATE_FREE))
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> index d7dcd17e4bff..bc980a001711 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -53,10 +53,6 @@ static inline const char *log_category_str(enum vchiq_log_category c)
> return strings[c];
> };
>
> -#ifndef vchiq_log_warning
> -#define vchiq_log_warning(dev, cat, fmt, ...) \
> - do { dev_dbg(dev, "%s warning: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
> -#endif
> #ifndef vchiq_log_debug
> #define vchiq_log_debug(dev, cat, fmt, ...) \
> do { dev_dbg(dev, "%s debug: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg
2023-12-05 8:41 [PATCH v3 0/4] staging: vc04_services: Drop custom logging Umang Jain
2023-12-05 8:41 ` [PATCH v3 1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err Umang Jain
2023-12-05 8:41 ` [PATCH v3 2/4] staging: vc04_services: Drop vchiq_log_warning() in favour of dev_warn Umang Jain
@ 2023-12-05 8:41 ` Umang Jain
2023-12-08 12:08 ` Stefan Wahren
2023-12-05 8:41 ` [PATCH v3 4/4] staging: vc04_services: Drop vchiq_log_debug() " Umang Jain
3 siblings, 1 reply; 16+ messages in thread
From: Umang Jain @ 2023-12-05 8:41 UTC (permalink / raw)
To: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere, Umang Jain
Drop vchiq_log_trace() macro which wraps dev_dbg(). Introduce the usage
of dev_dbg() directly.
Meanwhile at it, drop the usage of __func__ from the logs.
Dynamic debug supports this via the 'f' decorator flag.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
.../interface/vchiq_arm/vchiq_arm.c | 53 ++++-----
.../interface/vchiq_arm/vchiq_core.c | 101 +++++++++---------
.../interface/vchiq_arm/vchiq_core.h | 4 -
.../interface/vchiq_arm/vchiq_dev.c | 15 ++-
4 files changed, 78 insertions(+), 95 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 56326b59a718..e337b8387647 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -255,8 +255,7 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
pagelist = dma_alloc_coherent(instance->state->dev, pagelist_size, &dma_addr,
GFP_KERNEL);
- vchiq_log_trace(instance->state->dev, VCHIQ_ARM,
- "%s - %pK", __func__, pagelist);
+ dev_dbg(instance->state->dev, "arm: %pK\n", pagelist);
if (!pagelist)
return NULL;
@@ -407,8 +406,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
struct page **pages = pagelistinfo->pages;
unsigned int num_pages = pagelistinfo->num_pages;
- vchiq_log_trace(instance->state->dev, VCHIQ_ARM,
- "%s - %pK, %d", __func__, pagelistinfo->pagelist, actual);
+ dev_dbg(instance->state->dev, "arm: %pK, %d\n", pagelistinfo->pagelist, actual);
/*
* NOTE: dma_unmap_sg must be called before the
@@ -708,8 +706,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
ret = 0;
failed:
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%s(%p): returning %d", __func__, instance, ret);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
return ret;
}
@@ -742,8 +739,7 @@ int vchiq_shutdown(struct vchiq_instance *instance)
mutex_unlock(&state->mutex);
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%s(%p): returning %d", __func__, instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
free_bulk_waiter(instance);
kfree(instance);
@@ -763,8 +759,8 @@ int vchiq_connect(struct vchiq_instance *instance)
struct vchiq_state *state = instance->state;
if (mutex_lock_killable(&state->mutex)) {
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%s: call to mutex_lock failed", __func__);
+ dev_dbg(state->dev,
+ "core: call to mutex_lock failed\n");
status = -EAGAIN;
goto failed;
}
@@ -776,8 +772,7 @@ int vchiq_connect(struct vchiq_instance *instance)
mutex_unlock(&state->mutex);
failed:
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%s(%p): returning %d", __func__, instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
return status;
}
@@ -808,8 +803,7 @@ vchiq_add_service(struct vchiq_instance *instance,
status = -EINVAL;
}
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%s(%p): returning %d", __func__, instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
return status;
}
@@ -840,8 +834,7 @@ vchiq_open_service(struct vchiq_instance *instance,
}
failed:
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%s(%p): returning %d", __func__, instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
return status;
}
@@ -1010,8 +1003,7 @@ 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(instance->state->dev, VCHIQ_CORE,
- "%s - completion queue full", __func__);
+ dev_dbg(instance->state->dev, "core: completion queue full\n");
DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
if (wait_for_completion_interruptible(&instance->remove_event)) {
vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
@@ -1099,11 +1091,11 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
vchiq_service_get(service);
rcu_read_unlock();
- vchiq_log_trace(service->state->dev, VCHIQ_ARM,
- "%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);
+ dev_dbg(service->state->dev,
+ "arm: service %lx(%d,%p), reason %d, header %lx, instance %lx, bulk_userdata %lx\n",
+ (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);
@@ -1112,8 +1104,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
spin_unlock(&msg_queue_spinlock);
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
- vchiq_log_trace(service->state->dev, VCHIQ_ARM,
- "%s - msg queue full", __func__);
+ dev_dbg(service->state->dev, "arm: msg queue full\n");
/*
* If there is no MESSAGE_AVAILABLE in the completion
* queue, add one
@@ -1405,8 +1396,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
local_uc = ++arm_state->videocore_use_count;
++(*entity_uc);
- vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s %s count %d, state count %d",
- __func__, entity, *entity_uc, local_uc);
+ dev_dbg(state->dev, "suspend: %s count %d, state count %d\n",
+ entity, *entity_uc, local_uc);
write_unlock_bh(&arm_state->susp_res_lock);
@@ -1425,7 +1416,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
}
out:
- vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s exit %d", __func__, ret);
+ dev_dbg(state->dev, "suspend: exit %d\n", ret);
return ret;
}
@@ -1463,14 +1454,14 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service)
--arm_state->videocore_use_count;
--(*entity_uc);
- vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s %s count %d, state count %d",
- __func__, entity, *entity_uc, arm_state->videocore_use_count);
+ dev_dbg(state->dev, "suspend: %s count %d, state count %d\n",
+ entity, *entity_uc, arm_state->videocore_use_count);
unlock:
write_unlock_bh(&arm_state->susp_res_lock);
out:
- vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s exit %d", __func__, ret);
+ dev_dbg(state->dev, "suspend: exit %d\n", ret);
return ret;
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 2fd793954001..d197e4504bd4 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -459,9 +459,9 @@ make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
{
int status;
- vchiq_log_trace(service->state->dev, VCHIQ_CORE, "%d: callback:%d (%s, %pK, %pK)",
- service->state->id, service->localport, reason_names[reason],
- header, bulk_userdata);
+ dev_dbg(service->state->dev, "core: %d: callback:%d (%s, %pK, %pK)\n",
+ service->state->id, service->localport, reason_names[reason],
+ header, bulk_userdata);
status = service->base.callback(service->instance, reason, header, service->handle,
bulk_userdata);
if (status && (status != -EAGAIN)) {
@@ -763,8 +763,8 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
* it has dropped below its quota
*/
complete("a->quota_event);
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq:%d %x@%pK - slot_use->%d",
- state->id, port, header->size, header, count - 1);
+ dev_dbg(state->dev, "core: %d: pfq:%d %x@%pK - slot_use->%d\n",
+ state->id, port, header->size, header, count - 1);
} else {
dev_err(state->dev,
"core: service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)\n",
@@ -809,9 +809,9 @@ process_free_queue(struct vchiq_state *state, u32 *service_found,
*/
rmb();
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq %d=%pK %x %x",
- state->id, slot_index, data, local->slot_queue_recycle,
- slot_queue_available);
+ dev_dbg(state->dev, "core: %d: pfq %d=%pK %x %x\n",
+ state->id, slot_index, data, local->slot_queue_recycle,
+ slot_queue_available);
/* Initialise the bitmask for services which have used this slot */
memset(service_found, 0, length);
@@ -980,10 +980,10 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
((tx_end_index != quota->previous_tx_index) &&
(quota->slot_use_count == quota->slot_quota))) {
spin_unlock("a_spinlock);
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%d: qm:%d %s,%zx - quota stall (msg %d, slot %d)",
- state->id, service->localport, msg_type_str(type), size,
- quota->message_use_count, quota->slot_use_count);
+ dev_dbg(state->dev,
+ "core: %d: qm:%d %s,%zx - quota stall (msg %d, slot %d)\n",
+ state->id, service->localport, msg_type_str(type), size,
+ quota->message_use_count, quota->slot_use_count);
VCHIQ_SERVICE_STATS_INC(service, quota_stalls);
mutex_unlock(&state->slot_mutex);
if (wait_for_completion_interruptible("a->quota_event))
@@ -1073,10 +1073,9 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
spin_unlock("a_spinlock);
if (slot_use_count)
- vchiq_log_trace(state->dev, VCHIQ_CORE,
- "%d: qm:%d %s,%zx - slot_use->%d (hdr %p)", state->id,
- service->localport, msg_type_str(VCHIQ_MSG_TYPE(msgid)),
- size, slot_use_count, header);
+ dev_dbg(state->dev, "core: %d: qm:%d %s,%zx - slot_use->%d (hdr %p)\n",
+ state->id, service->localport, msg_type_str(VCHIQ_MSG_TYPE(msgid)),
+ size, slot_use_count, header);
VCHIQ_SERVICE_STATS_INC(service, ctrl_tx_count);
VCHIQ_SERVICE_STATS_ADD(service, ctrl_tx_bytes, size);
@@ -1205,11 +1204,11 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
svc_fourcc = service ? service->base.fourcc
: VCHIQ_MAKE_FOURCC('?', '?', '?', '?');
- vchiq_log_trace(state->dev, VCHIQ_SYNC,
- "Sent Sync Msg %s(%u) to %p4cc s:%u d:%d len:%d",
- msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid),
- &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid),
- VCHIQ_MSG_DSTPORT(msgid), size);
+ dev_dbg(state->dev,
+ "sync: Sent Sync Msg %s(%u) to %p4cc s:%u d:%d len:%d\n",
+ msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid),
+ &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid),
+ VCHIQ_MSG_DSTPORT(msgid), size);
remote_event_signal(&state->remote->sync_trigger);
@@ -1298,11 +1297,11 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
{
int status = 0;
- vchiq_log_trace(service->state->dev, VCHIQ_CORE,
- "%d: nb:%d %cx - p=%x rn=%x r=%x",
- service->state->id, service->localport,
- (queue == &service->bulk_tx) ? 't' : 'r',
- queue->process, queue->remote_notify, queue->remove);
+ dev_dbg(service->state->dev,
+ "core: %d: nb:%d %cx - p=%x rn=%x r=%x\n",
+ service->state->id, service->localport,
+ (queue == &service->bulk_tx) ? 't' : 'r',
+ queue->process, queue->remote_notify, queue->remove);
queue->remote_notify = queue->process;
@@ -1425,11 +1424,11 @@ abort_outstanding_bulks(struct vchiq_service *service,
{
int is_tx = (queue == &service->bulk_tx);
- vchiq_log_trace(service->state->dev, VCHIQ_CORE,
- "%d: aob:%d %cx - li=%x ri=%x p=%x",
- service->state->id, service->localport,
- is_tx ? 't' : 'r', queue->local_insert,
- queue->remote_insert, queue->process);
+ dev_dbg(service->state->dev,
+ "core: %d: aob:%d %cx - li=%x ri=%x p=%x\n",
+ service->state->id, service->localport,
+ is_tx ? 't' : 'r', queue->local_insert,
+ queue->remote_insert, queue->process);
WARN_ON((int)(queue->local_insert - queue->process) < 0);
WARN_ON((int)(queue->remote_insert - queue->process) < 0);
@@ -1764,10 +1763,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
state->id, msg_type_str(type), header, remoteport,
localport, bulk->actual, &bulk->data);
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs:%d %cx li=%x ri=%x p=%x",
- state->id, localport,
- (type == VCHIQ_MSG_BULK_RX_DONE) ? 'r' : 't',
- queue->local_insert, queue->remote_insert, queue->process);
+ dev_dbg(state->dev, "core: %d: prs:%d %cx li=%x ri=%x p=%x\n",
+ state->id, localport,
+ (type == VCHIQ_MSG_BULK_RX_DONE) ? 'r' : 't',
+ queue->local_insert, queue->remote_insert, queue->process);
DEBUG_TRACE(PARSE_LINE);
WARN_ON(queue->process == queue->local_insert);
@@ -1780,13 +1779,13 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
}
break;
case VCHIQ_MSG_PADDING:
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs PADDING@%pK,%x",
- state->id, header, size);
+ dev_dbg(state->dev, "core: %d: prs PADDING@%pK,%x\n",
+ state->id, header, size);
break;
case VCHIQ_MSG_PAUSE:
/* If initiated, signal the application thread */
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs PAUSE@%pK,%x",
- state->id, header, size);
+ dev_dbg(state->dev, "core: %d: prs PAUSE@%pK,%x\n",
+ state->id, header, size);
if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
dev_err(state->dev, "core: %d: PAUSE received in state PAUSED\n",
state->id);
@@ -1802,8 +1801,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSED);
break;
case VCHIQ_MSG_RESUME:
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs RESUME@%pK,%x",
- state->id, header, size);
+ dev_dbg(state->dev, "core: %d: prs RESUME@%pK,%x\n",
+ state->id, header, size);
/* Release the slot mutex */
mutex_unlock(&state->slot_mutex);
vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
@@ -2040,10 +2039,8 @@ sync_func(void *v)
svc_fourcc = service->base.fourcc;
- vchiq_log_trace(state->dev, VCHIQ_SYNC,
- "Rcvd Msg %s from %p4cc s:%d d:%d len:%d",
- msg_type_str(type), &svc_fourcc,
- remoteport, localport, size);
+ dev_dbg(state->dev, "sync: Rcvd Msg %s from %p4cc s:%d d:%d len:%d\n",
+ msg_type_str(type), &svc_fourcc, remoteport, localport, size);
if (size > 0)
vchiq_log_dump_mem(state->dev, "Rcvd", 0, header->data, min(16, size));
@@ -2068,8 +2065,8 @@ sync_func(void *v)
break;
case VCHIQ_MSG_DATA:
- vchiq_log_trace(state->dev, VCHIQ_SYNC, "%d: sf DATA@%pK,%x (%d->%d)",
- state->id, header, size, remoteport, localport);
+ dev_dbg(state->dev, "sync: %d: sf DATA@%pK,%x (%d->%d)\n",
+ state->id, header, size, remoteport, localport);
if ((service->remoteport == remoteport) &&
(service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
@@ -3111,9 +3108,9 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
mutex_unlock(&state->slot_mutex);
mutex_unlock(&service->bulk_mutex);
- vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: bt:%d %cx li=%x ri=%x p=%x",
- state->id, service->localport, dir_char, queue->local_insert,
- queue->remote_insert, queue->process);
+ dev_dbg(state->dev, "core: %d: bt:%d %cx li=%x ri=%x p=%x\n",
+ state->id, service->localport, dir_char, queue->local_insert,
+ queue->remote_insert, queue->process);
waiting:
vchiq_service_put(service);
@@ -3579,9 +3576,9 @@ void vchiq_log_dump_mem(struct device *dev, const char *label, u32 addr,
*s++ = '\0';
if (label && (*label != '\0'))
- vchiq_log_trace(dev, VCHIQ_CORE, "%s: %08x: %s", label, addr, line_buf);
+ dev_dbg(dev, "core: %s: %08x: %s\n", label, addr, line_buf);
else
- vchiq_log_trace(dev, VCHIQ_CORE, "%s: %08x: %s", label, addr, line_buf);
+ dev_dbg(dev, "core: %s: %08x: %s\n", label, addr, line_buf);
addr += 16;
mem += 16;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index bc980a001711..36e34a7fc628 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -57,10 +57,6 @@ static inline const char *log_category_str(enum vchiq_log_category c)
#define vchiq_log_debug(dev, cat, fmt, ...) \
do { dev_dbg(dev, "%s debug: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
#endif
-#ifndef vchiq_log_trace
-#define vchiq_log_trace(dev, cat, fmt, ...) \
- do { dev_dbg(dev, "%s trace: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
-#endif
#define VCHIQ_SLOT_MASK (VCHIQ_SLOT_SIZE - 1)
#define VCHIQ_SLOT_QUEUE_MASK (VCHIQ_MAX_SLOTS_PER_SIDE - 1)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index ba287cb4c87b..38841c9d8ef9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -582,10 +582,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
long ret = 0;
int i, rc;
- vchiq_log_trace(instance->state->dev, VCHIQ_ARM,
- "%s - instance %pK, cmd %s, arg %lx", __func__, instance,
- ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) && (_IOC_NR(cmd) <= VCHIQ_IOC_MAX)) ?
- ioctl_names[_IOC_NR(cmd)] : "<invalid>", arg);
+ dev_dbg(instance->state->dev, "arm: instance %pK, cmd %s, arg %lx\n", instance,
+ ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) && (_IOC_NR(cmd) <= VCHIQ_IOC_MAX)) ?
+ ioctl_names[_IOC_NR(cmd)] : "<invalid>", arg);
switch (cmd) {
case VCHIQ_IOC_SHUTDOWN:
@@ -873,10 +872,10 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
instance, (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
ioctl_names[_IOC_NR(cmd)] : "<invalid>", status, ret);
} else {
- vchiq_log_trace(instance->state->dev, VCHIQ_ARM,
- " ioctl instance %pK, cmd %s -> status %d, %ld",
- instance, (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
- ioctl_names[_IOC_NR(cmd)] : "<invalid>", status, ret);
+ dev_dbg(instance->state->dev,
+ "arm: ioctl instance %pK, cmd %s -> status %d\n, %ld\n",
+ instance, (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
+ ioctl_names[_IOC_NR(cmd)] : "<invalid>", status, ret);
}
return ret;
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg
2023-12-05 8:41 ` [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg Umang Jain
@ 2023-12-08 12:08 ` Stefan Wahren
2023-12-09 6:27 ` Dan Carpenter
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Wahren @ 2023-12-08 12:08 UTC (permalink / raw)
To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Umang,
Am 05.12.23 um 09:41 schrieb Umang Jain:
> Drop vchiq_log_trace() macro which wraps dev_dbg(). Introduce the usage
> of dev_dbg() directly.
>
> Meanwhile at it, drop the usage of __func__ from the logs.
> Dynamic debug supports this via the 'f' decorator flag.
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
in general i'm fine with this patch:
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
but there some improvements, which could be addressed/improved later
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 53 ++++-----
> .../interface/vchiq_arm/vchiq_core.c | 101 +++++++++---------
> .../interface/vchiq_arm/vchiq_core.h | 4 -
> .../interface/vchiq_arm/vchiq_dev.c | 15 ++-
> 4 files changed, 78 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 56326b59a718..e337b8387647 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -255,8 +255,7 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
> pagelist = dma_alloc_coherent(instance->state->dev, pagelist_size, &dma_addr,
> GFP_KERNEL);
>
> - vchiq_log_trace(instance->state->dev, VCHIQ_ARM,
> - "%s - %pK", __func__, pagelist);
> + dev_dbg(instance->state->dev, "arm: %pK\n", pagelist);
>
> if (!pagelist)
> return NULL;
> @@ -407,8 +406,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> struct page **pages = pagelistinfo->pages;
> unsigned int num_pages = pagelistinfo->num_pages;
>
> - vchiq_log_trace(instance->state->dev, VCHIQ_ARM,
> - "%s - %pK, %d", __func__, pagelistinfo->pagelist, actual);
> + dev_dbg(instance->state->dev, "arm: %pK, %d\n", pagelistinfo->pagelist, actual);
>
> /*
> * NOTE: dma_unmap_sg must be called before the
> @@ -708,8 +706,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
> ret = 0;
>
> failed:
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%s(%p): returning %d", __func__, instance, ret);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
>
> return ret;
> }
> @@ -742,8 +739,7 @@ int vchiq_shutdown(struct vchiq_instance *instance)
>
> mutex_unlock(&state->mutex);
>
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%s(%p): returning %d", __func__, instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
>
> free_bulk_waiter(instance);
> kfree(instance);
> @@ -763,8 +759,8 @@ int vchiq_connect(struct vchiq_instance *instance)
> struct vchiq_state *state = instance->state;
>
> if (mutex_lock_killable(&state->mutex)) {
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%s: call to mutex_lock failed", __func__);
> + dev_dbg(state->dev,
> + "core: call to mutex_lock failed\n");
> status = -EAGAIN;
> goto failed;
> }
> @@ -776,8 +772,7 @@ int vchiq_connect(struct vchiq_instance *instance)
> mutex_unlock(&state->mutex);
>
> failed:
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%s(%p): returning %d", __func__, instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
>
> return status;
> }
> @@ -808,8 +803,7 @@ vchiq_add_service(struct vchiq_instance *instance,
> status = -EINVAL;
> }
>
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%s(%p): returning %d", __func__, instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
>
> return status;
> }
> @@ -840,8 +834,7 @@ vchiq_open_service(struct vchiq_instance *instance,
> }
>
> failed:
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%s(%p): returning %d", __func__, instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
>
> return status;
> }
> @@ -1010,8 +1003,7 @@ 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(instance->state->dev, VCHIQ_CORE,
> - "%s - completion queue full", __func__);
> + dev_dbg(instance->state->dev, "core: completion queue full\n");
> DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
> if (wait_for_completion_interruptible(&instance->remove_event)) {
> vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> @@ -1099,11 +1091,11 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
> vchiq_service_get(service);
> rcu_read_unlock();
>
> - vchiq_log_trace(service->state->dev, VCHIQ_ARM,
> - "%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);
> + dev_dbg(service->state->dev,
> + "arm: service %lx(%d,%p), reason %d, header %lx, instance %lx, bulk_userdata %lx\n",
> + (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);
> @@ -1112,8 +1104,7 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
> spin_unlock(&msg_queue_spinlock);
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
> - vchiq_log_trace(service->state->dev, VCHIQ_ARM,
> - "%s - msg queue full", __func__);
> + dev_dbg(service->state->dev, "arm: msg queue full\n");
> /*
> * If there is no MESSAGE_AVAILABLE in the completion
> * queue, add one
> @@ -1405,8 +1396,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
> local_uc = ++arm_state->videocore_use_count;
> ++(*entity_uc);
>
> - vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s %s count %d, state count %d",
> - __func__, entity, *entity_uc, local_uc);
> + dev_dbg(state->dev, "suspend: %s count %d, state count %d\n",
> + entity, *entity_uc, local_uc);
>
> write_unlock_bh(&arm_state->susp_res_lock);
>
> @@ -1425,7 +1416,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
> }
>
> out:
> - vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s exit %d", __func__, ret);
> + dev_dbg(state->dev, "suspend: exit %d\n", ret);
In case we want to keep this kind of debug messages, i prefer to have a
common format:
returning %d vs exit %d
> return ret;
> }
>
> @@ -1463,14 +1454,14 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service)
> --arm_state->videocore_use_count;
> --(*entity_uc);
>
> - vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s %s count %d, state count %d",
> - __func__, entity, *entity_uc, arm_state->videocore_use_count);
> + dev_dbg(state->dev, "suspend: %s count %d, state count %d\n",
> + entity, *entity_uc, arm_state->videocore_use_count);
>
> unlock:
> write_unlock_bh(&arm_state->susp_res_lock);
>
> out:
> - vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s exit %d", __func__, ret);
> + dev_dbg(state->dev, "suspend: exit %d\n", ret);
ditto
> return ret;
> }
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 2fd793954001..d197e4504bd4 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -459,9 +459,9 @@ make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
> {
> int status;
>
> - vchiq_log_trace(service->state->dev, VCHIQ_CORE, "%d: callback:%d (%s, %pK, %pK)",
> - service->state->id, service->localport, reason_names[reason],
> - header, bulk_userdata);
> + dev_dbg(service->state->dev, "core: %d: callback:%d (%s, %pK, %pK)\n",
> + service->state->id, service->localport, reason_names[reason],
> + header, bulk_userdata);
> status = service->base.callback(service->instance, reason, header, service->handle,
> bulk_userdata);
> if (status && (status != -EAGAIN)) {
> @@ -763,8 +763,8 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
> * it has dropped below its quota
> */
> complete("a->quota_event);
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq:%d %x@%pK - slot_use->%d",
> - state->id, port, header->size, header, count - 1);
> + dev_dbg(state->dev, "core: %d: pfq:%d %x@%pK - slot_use->%d\n",
> + state->id, port, header->size, header, count - 1);
> } else {
> dev_err(state->dev,
> "core: service %d slot_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)\n",
> @@ -809,9 +809,9 @@ process_free_queue(struct vchiq_state *state, u32 *service_found,
> */
> rmb();
>
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: pfq %d=%pK %x %x",
> - state->id, slot_index, data, local->slot_queue_recycle,
> - slot_queue_available);
> + dev_dbg(state->dev, "core: %d: pfq %d=%pK %x %x\n",
> + state->id, slot_index, data, local->slot_queue_recycle,
> + slot_queue_available);
>
> /* Initialise the bitmask for services which have used this slot */
> memset(service_found, 0, length);
> @@ -980,10 +980,10 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
> ((tx_end_index != quota->previous_tx_index) &&
> (quota->slot_use_count == quota->slot_quota))) {
> spin_unlock("a_spinlock);
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%d: qm:%d %s,%zx - quota stall (msg %d, slot %d)",
> - state->id, service->localport, msg_type_str(type), size,
> - quota->message_use_count, quota->slot_use_count);
> + dev_dbg(state->dev,
> + "core: %d: qm:%d %s,%zx - quota stall (msg %d, slot %d)\n",
> + state->id, service->localport, msg_type_str(type), size,
> + quota->message_use_count, quota->slot_use_count);
> VCHIQ_SERVICE_STATS_INC(service, quota_stalls);
> mutex_unlock(&state->slot_mutex);
> if (wait_for_completion_interruptible("a->quota_event))
> @@ -1073,10 +1073,9 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
> spin_unlock("a_spinlock);
>
> if (slot_use_count)
> - vchiq_log_trace(state->dev, VCHIQ_CORE,
> - "%d: qm:%d %s,%zx - slot_use->%d (hdr %p)", state->id,
> - service->localport, msg_type_str(VCHIQ_MSG_TYPE(msgid)),
> - size, slot_use_count, header);
> + dev_dbg(state->dev, "core: %d: qm:%d %s,%zx - slot_use->%d (hdr %p)\n",
> + state->id, service->localport, msg_type_str(VCHIQ_MSG_TYPE(msgid)),
> + size, slot_use_count, header);
>
> VCHIQ_SERVICE_STATS_INC(service, ctrl_tx_count);
> VCHIQ_SERVICE_STATS_ADD(service, ctrl_tx_bytes, size);
> @@ -1205,11 +1204,11 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
> svc_fourcc = service ? service->base.fourcc
> : VCHIQ_MAKE_FOURCC('?', '?', '?', '?');
>
> - vchiq_log_trace(state->dev, VCHIQ_SYNC,
> - "Sent Sync Msg %s(%u) to %p4cc s:%u d:%d len:%d",
> - msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid),
> - &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid),
> - VCHIQ_MSG_DSTPORT(msgid), size);
> + dev_dbg(state->dev,
> + "sync: Sent Sync Msg %s(%u) to %p4cc s:%u d:%d len:%d\n",
> + msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid),
> + &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid),
> + VCHIQ_MSG_DSTPORT(msgid), size);
>
> remote_event_signal(&state->remote->sync_trigger);
>
> @@ -1298,11 +1297,11 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
> {
> int status = 0;
>
> - vchiq_log_trace(service->state->dev, VCHIQ_CORE,
> - "%d: nb:%d %cx - p=%x rn=%x r=%x",
> - service->state->id, service->localport,
> - (queue == &service->bulk_tx) ? 't' : 'r',
> - queue->process, queue->remote_notify, queue->remove);
> + dev_dbg(service->state->dev,
> + "core: %d: nb:%d %cx - p=%x rn=%x r=%x\n",
> + service->state->id, service->localport,
> + (queue == &service->bulk_tx) ? 't' : 'r',
> + queue->process, queue->remote_notify, queue->remove);
>
> queue->remote_notify = queue->process;
>
> @@ -1425,11 +1424,11 @@ abort_outstanding_bulks(struct vchiq_service *service,
> {
> int is_tx = (queue == &service->bulk_tx);
>
> - vchiq_log_trace(service->state->dev, VCHIQ_CORE,
> - "%d: aob:%d %cx - li=%x ri=%x p=%x",
> - service->state->id, service->localport,
> - is_tx ? 't' : 'r', queue->local_insert,
> - queue->remote_insert, queue->process);
> + dev_dbg(service->state->dev,
> + "core: %d: aob:%d %cx - li=%x ri=%x p=%x\n",
> + service->state->id, service->localport,
> + is_tx ? 't' : 'r', queue->local_insert,
> + queue->remote_insert, queue->process);
>
> WARN_ON((int)(queue->local_insert - queue->process) < 0);
> WARN_ON((int)(queue->remote_insert - queue->process) < 0);
> @@ -1764,10 +1763,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> state->id, msg_type_str(type), header, remoteport,
> localport, bulk->actual, &bulk->data);
>
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs:%d %cx li=%x ri=%x p=%x",
> - state->id, localport,
> - (type == VCHIQ_MSG_BULK_RX_DONE) ? 'r' : 't',
> - queue->local_insert, queue->remote_insert, queue->process);
> + dev_dbg(state->dev, "core: %d: prs:%d %cx li=%x ri=%x p=%x\n",
> + state->id, localport,
> + (type == VCHIQ_MSG_BULK_RX_DONE) ? 'r' : 't',
> + queue->local_insert, queue->remote_insert, queue->process);
>
> DEBUG_TRACE(PARSE_LINE);
> WARN_ON(queue->process == queue->local_insert);
> @@ -1780,13 +1779,13 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> }
> break;
> case VCHIQ_MSG_PADDING:
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs PADDING@%pK,%x",
> - state->id, header, size);
> + dev_dbg(state->dev, "core: %d: prs PADDING@%pK,%x\n",
> + state->id, header, size);
> break;
> case VCHIQ_MSG_PAUSE:
> /* If initiated, signal the application thread */
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs PAUSE@%pK,%x",
> - state->id, header, size);
> + dev_dbg(state->dev, "core: %d: prs PAUSE@%pK,%x\n",
> + state->id, header, size);
> if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
> dev_err(state->dev, "core: %d: PAUSE received in state PAUSED\n",
> state->id);
> @@ -1802,8 +1801,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSED);
> break;
> case VCHIQ_MSG_RESUME:
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: prs RESUME@%pK,%x",
> - state->id, header, size);
> + dev_dbg(state->dev, "core: %d: prs RESUME@%pK,%x\n",
> + state->id, header, size);
> /* Release the slot mutex */
> mutex_unlock(&state->slot_mutex);
> vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
> @@ -2040,10 +2039,8 @@ sync_func(void *v)
>
> svc_fourcc = service->base.fourcc;
>
> - vchiq_log_trace(state->dev, VCHIQ_SYNC,
> - "Rcvd Msg %s from %p4cc s:%d d:%d len:%d",
> - msg_type_str(type), &svc_fourcc,
> - remoteport, localport, size);
> + dev_dbg(state->dev, "sync: Rcvd Msg %s from %p4cc s:%d d:%d len:%d\n",
> + msg_type_str(type), &svc_fourcc, remoteport, localport, size);
> if (size > 0)
> vchiq_log_dump_mem(state->dev, "Rcvd", 0, header->data, min(16, size));
>
> @@ -2068,8 +2065,8 @@ sync_func(void *v)
> break;
>
> case VCHIQ_MSG_DATA:
> - vchiq_log_trace(state->dev, VCHIQ_SYNC, "%d: sf DATA@%pK,%x (%d->%d)",
> - state->id, header, size, remoteport, localport);
> + dev_dbg(state->dev, "sync: %d: sf DATA@%pK,%x (%d->%d)\n",
> + state->id, header, size, remoteport, localport);
>
> if ((service->remoteport == remoteport) &&
> (service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
> @@ -3111,9 +3108,9 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
> mutex_unlock(&state->slot_mutex);
> mutex_unlock(&service->bulk_mutex);
>
> - vchiq_log_trace(state->dev, VCHIQ_CORE, "%d: bt:%d %cx li=%x ri=%x p=%x",
> - state->id, service->localport, dir_char, queue->local_insert,
> - queue->remote_insert, queue->process);
> + dev_dbg(state->dev, "core: %d: bt:%d %cx li=%x ri=%x p=%x\n",
> + state->id, service->localport, dir_char, queue->local_insert,
> + queue->remote_insert, queue->process);
>
> waiting:
> vchiq_service_put(service);
> @@ -3579,9 +3576,9 @@ void vchiq_log_dump_mem(struct device *dev, const char *label, u32 addr,
> *s++ = '\0';
>
> if (label && (*label != '\0'))
> - vchiq_log_trace(dev, VCHIQ_CORE, "%s: %08x: %s", label, addr, line_buf);
> + dev_dbg(dev, "core: %s: %08x: %s\n", label, addr, line_buf);
> else
> - vchiq_log_trace(dev, VCHIQ_CORE, "%s: %08x: %s", label, addr, line_buf);
> + dev_dbg(dev, "core: %s: %08x: %s\n", label, addr, line_buf);
This looks like a open-coded version of a hexdump. AFAIR the kernel
already has such logging functions.
Best regards
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg
2023-12-08 12:08 ` Stefan Wahren
@ 2023-12-09 6:27 ` Dan Carpenter
2023-12-09 9:33 ` Stefan Wahren
0 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2023-12-09 6:27 UTC (permalink / raw)
To: Stefan Wahren
Cc: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel, Stefan Wahren, Greg Kroah-Hartman,
Dan Carpenter, Kieran Bingham, Ricardo B . Marliere
On Fri, Dec 08, 2023 at 01:08:04PM +0100, Stefan Wahren wrote:
> > @@ -1425,7 +1416,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
> > }
> >
> > out:
> > - vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s exit %d", __func__, ret);
> > + dev_dbg(state->dev, "suspend: exit %d\n", ret);
> In case we want to keep this kind of debug messages, i prefer to have a
> common format:
>
> returning %d vs exit %d
Someone was trying to make ftrace track return values... Whatever
happened with that work?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg
2023-12-09 6:27 ` Dan Carpenter
@ 2023-12-09 9:33 ` Stefan Wahren
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Wahren @ 2023-12-09 9:33 UTC (permalink / raw)
To: Dan Carpenter
Cc: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel, Stefan Wahren, Greg Kroah-Hartman,
Dan Carpenter, Kieran Bingham, Ricardo B . Marliere
Hi Dan,
Am 09.12.23 um 07:27 schrieb Dan Carpenter:
> On Fri, Dec 08, 2023 at 01:08:04PM +0100, Stefan Wahren wrote:
>>> @@ -1425,7 +1416,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
>>> }
>>>
>>> out:
>>> - vchiq_log_trace(state->dev, VCHIQ_SUSPEND, "%s exit %d", __func__, ret);
>>> + dev_dbg(state->dev, "suspend: exit %d\n", ret);
>> In case we want to keep this kind of debug messages, i prefer to have a
>> common format:
>>
>> returning %d vs exit %d
> Someone was trying to make ftrace track return values... Whatever
> happened with that work?
this feature has been merged, but it doesn't seem to be available for arm:
find -name Kconfig | xargs grep HAVE_FUNCTION_GRAPH_RETVAL
./kernel/trace/Kconfig:config HAVE_FUNCTION_GRAPH_RETVAL
./kernel/trace/Kconfig: depends on HAVE_FUNCTION_GRAPH_RETVAL
./arch/x86/Kconfig: select HAVE_FUNCTION_GRAPH_RETVAL if
HAVE_FUNCTION_GRAPH_TRACER
./arch/arm64/Kconfig: select HAVE_FUNCTION_GRAPH_RETVAL if
HAVE_FUNCTION_GRAPH_TRACER
./arch/s390/Kconfig: select HAVE_FUNCTION_GRAPH_RETVAL
./arch/riscv/Kconfig: select HAVE_FUNCTION_GRAPH_RETVAL if
HAVE_FUNCTION_GRAPH_TRACER
./arch/loongarch/Kconfig: select HAVE_FUNCTION_GRAPH_RETVAL if
HAVE_FUNCTION_GRAPH_TRACER
These debug messages are only in the error path, so i'm not sure if
someone what to hunt bugs by searching all that return values without
knowing which function is possibly affected.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 4/4] staging: vc04_services: Drop vchiq_log_debug() in favour of dev_dbg
2023-12-05 8:41 [PATCH v3 0/4] staging: vc04_services: Drop custom logging Umang Jain
` (2 preceding siblings ...)
2023-12-05 8:41 ` [PATCH v3 3/4] staging: vc04_services: Drop vchiq_log_trace() in favour of dev_dbg Umang Jain
@ 2023-12-05 8:41 ` Umang Jain
2023-12-08 13:00 ` Stefan Wahren
3 siblings, 1 reply; 16+ messages in thread
From: Umang Jain @ 2023-12-05 8:41 UTC (permalink / raw)
To: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere, Umang Jain
Drop vchiq_log_debug() macro which wraps dev_dbg(). Introduce the usage
of dev_dbg() directly.
Meanwhile at it, drop the usage of __func__ from the logs.
Dynamic debug supports this via the 'f' decorator flag.
Remove the entry from TODO regarding custom logging. VC04 is now
aligned according to the standard kernel logging mechanisms.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
drivers/staging/vc04_services/interface/TODO | 5 -
.../interface/vchiq_arm/vchiq_arm.c | 45 +++--
.../interface/vchiq_arm/vchiq_core.c | 159 ++++++++----------
.../interface/vchiq_arm/vchiq_core.h | 26 ---
.../interface/vchiq_arm/vchiq_dev.c | 32 ++--
5 files changed, 106 insertions(+), 161 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
index 6d9d4a800aa7..05eb5140d096 100644
--- a/drivers/staging/vc04_services/interface/TODO
+++ b/drivers/staging/vc04_services/interface/TODO
@@ -23,11 +23,6 @@ should properly handle a module unload. This also includes that all
resources must be freed (kthreads, debugfs entries, ...) and global
variables avoided.
-* Cleanup logging mechanism
-
-The driver should probably be using the standard kernel logging mechanisms
-such as dev_info, dev_dbg, and friends.
-
* Documentation
A short top-down description of this driver's architecture (function of
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index e337b8387647..4b4ff469d3a3 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -310,9 +310,8 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
type == PAGELIST_READ, pages);
if (actual_pages != num_pages) {
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "%s - only %d/%d pages locked",
- __func__, actual_pages, num_pages);
+ dev_dbg(instance->state->dev, "arm: Only %d/%d pages locked\n",
+ actual_pages, num_pages);
/* This is probably due to the process being killed */
if (actual_pages > 0)
@@ -554,8 +553,8 @@ static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state
return -ENXIO;
}
- vchiq_log_debug(&pdev->dev, VCHIQ_ARM, "vchiq_init - done (slots %pK, phys %pad)",
- vchiq_slot_zero, &slot_phys);
+ dev_dbg(&pdev->dev, "arm: vchiq_init - done (slots %pK, phys %pad)\n",
+ vchiq_slot_zero, &slot_phys);
vchiq_call_connected_callbacks();
@@ -719,9 +718,9 @@ 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_debug(instance->state->dev, VCHIQ_ARM,
- "bulk_waiter - cleaned up %pK for pid %d",
- waiter, waiter->pid);
+ dev_dbg(instance->state->dev,
+ "arm: bulk_waiter - cleaned up %pK for pid %d\n",
+ waiter, waiter->pid);
kfree(waiter);
}
}
@@ -981,9 +980,8 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
mutex_lock(&instance->bulk_waiter_list_mutex);
list_add(&waiter->list, &instance->bulk_waiter_list);
mutex_unlock(&instance->bulk_waiter_list_mutex);
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "saved bulk_waiter %pK for pid %d", waiter,
- current->pid);
+ dev_dbg(instance->state->dev, "arm: saved bulk_waiter %pK for pid %d\n",
+ waiter, current->pid);
}
return status;
@@ -1006,12 +1004,10 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
dev_dbg(instance->state->dev, "core: completion queue full\n");
DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
if (wait_for_completion_interruptible(&instance->remove_event)) {
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "service_callback interrupted");
+ dev_dbg(instance->state->dev, "arm: service_callback interrupted\n");
return -EAGAIN;
} else if (instance->closing) {
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "service_callback closing");
+ dev_dbg(instance->state->dev, "arm: service_callback closing\n");
return 0;
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
@@ -1113,8 +1109,8 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
instance->completion_remove) < 0) {
int status;
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "Inserting extra MESSAGE_AVAILABLE");
+ dev_dbg(instance->state->dev,
+ "arm: Inserting extra MESSAGE_AVAILABLE\n");
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
status = add_completion(instance, reason, NULL, user_service,
bulk_userdata);
@@ -1127,14 +1123,12 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
if (wait_for_completion_interruptible(&user_service->remove_event)) {
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "%s interrupted", __func__);
+ dev_dbg(instance->state->dev, "arm: interrupted\n");
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
return -EAGAIN;
} else if (instance->closing) {
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "%s closing", __func__);
+ dev_dbg(instance->state->dev, "arm: closing\n");
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
return -EINVAL;
@@ -1686,8 +1680,8 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);
char threadname[16];
- vchiq_log_debug(state->dev, VCHIQ_SUSPEND, "%d: %s->%s", state->id,
- get_conn_state_name(oldstate), get_conn_state_name(newstate));
+ dev_dbg(state->dev, "suspend: %d: %s->%s\n",
+ state->id, get_conn_state_name(oldstate), get_conn_state_name(newstate));
if (state->conn_state != VCHIQ_CONNSTATE_CONNECTED)
return;
@@ -1751,9 +1745,8 @@ static int vchiq_probe(struct platform_device *pdev)
vchiq_debugfs_init();
- vchiq_log_debug(&pdev->dev, VCHIQ_ARM,
- "vchiq: platform initialised - version %d (min %d)",
- VCHIQ_VERSION, VCHIQ_VERSION_MIN);
+ dev_dbg(&pdev->dev, "arm: platform initialised - version %d (min %d)\n",
+ VCHIQ_VERSION, VCHIQ_VERSION_MIN);
/*
* Simply exit on error since the function handles cleanup in
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index d197e4504bd4..76c27778154a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -217,10 +217,10 @@ static const char *msg_type_str(unsigned int msg_type)
static inline void
set_service_state(struct vchiq_service *service, int newstate)
{
- vchiq_log_debug(service->state->dev, VCHIQ_CORE, "%d: srv:%d %s->%s",
- service->state->id, service->localport,
- srvstate_names[service->srvstate],
- srvstate_names[newstate]);
+ dev_dbg(service->state->dev, "core: %d: srv:%d %s->%s\n",
+ service->state->id, service->localport,
+ srvstate_names[service->srvstate],
+ srvstate_names[newstate]);
service->srvstate = newstate;
}
@@ -245,8 +245,7 @@ find_service_by_handle(struct vchiq_instance *instance, unsigned int handle)
return service;
}
rcu_read_unlock();
- vchiq_log_debug(instance->state->dev, VCHIQ_CORE,
- "Invalid service handle 0x%x", handle);
+ dev_dbg(instance->state->dev, "core: Invalid service handle 0x%x\n", handle);
return NULL;
}
@@ -266,8 +265,7 @@ find_service_by_port(struct vchiq_state *state, unsigned int localport)
}
rcu_read_unlock();
}
- vchiq_log_debug(state->dev, VCHIQ_CORE,
- "Invalid port %u", localport);
+ dev_dbg(state->dev, "core: Invalid port %u\n", localport);
return NULL;
}
@@ -287,8 +285,7 @@ find_service_for_instance(struct vchiq_instance *instance, unsigned int handle)
return service;
}
rcu_read_unlock();
- vchiq_log_debug(instance->state->dev, VCHIQ_CORE,
- "Invalid service handle 0x%x", handle);
+ dev_dbg(instance->state->dev, "core: Invalid service handle 0x%x\n", handle);
return NULL;
}
@@ -310,8 +307,7 @@ find_closed_service_for_instance(struct vchiq_instance *instance, unsigned int h
return service;
}
rcu_read_unlock();
- vchiq_log_debug(instance->state->dev, VCHIQ_CORE,
- "Invalid service handle 0x%x", handle);
+ dev_dbg(instance->state->dev, "core: Invalid service handle 0x%x\n", handle);
return service;
}
@@ -482,8 +478,8 @@ vchiq_set_conn_state(struct vchiq_state *state, enum vchiq_connstate newstate)
{
enum vchiq_connstate oldstate = state->conn_state;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: %s->%s", state->id, conn_state_names[oldstate],
- conn_state_names[newstate]);
+ dev_dbg(state->dev, "core: %d: %s->%s\n",
+ state->id, conn_state_names[oldstate], conn_state_names[newstate]);
state->conn_state = newstate;
vchiq_platform_conn_state_changed(state, oldstate, newstate);
}
@@ -1023,9 +1019,9 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
int tx_end_index;
int slot_use_count;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: qm %s@%pK,%zx (%d->%d)", state->id,
- msg_type_str(VCHIQ_MSG_TYPE(msgid)), header, size,
- VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid));
+ dev_dbg(state->dev, "core: %d: qm %s@%pK,%zx (%d->%d)\n",
+ state->id, msg_type_str(VCHIQ_MSG_TYPE(msgid)), header, size,
+ VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid));
WARN_ON(flags & (QMFLAGS_NO_MUTEX_LOCK |
QMFLAGS_NO_MUTEX_UNLOCK));
@@ -1080,9 +1076,9 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
VCHIQ_SERVICE_STATS_INC(service, ctrl_tx_count);
VCHIQ_SERVICE_STATS_ADD(service, ctrl_tx_bytes, size);
} else {
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: qm %s@%pK,%zx (%d->%d)", state->id,
- msg_type_str(VCHIQ_MSG_TYPE(msgid)), header, size,
- VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid));
+ dev_dbg(state->dev, "core: %d: qm %s@%pK,%zx (%d->%d)\n",
+ state->id, msg_type_str(VCHIQ_MSG_TYPE(msgid)), header, size,
+ VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid));
if (size != 0) {
/*
* It is assumed for now that this code path
@@ -1110,11 +1106,9 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
? service->base.fourcc
: VCHIQ_MAKE_FOURCC('?', '?', '?', '?');
- vchiq_log_debug(state->dev, VCHIQ_CORE_MSG,
- "Sent Msg %s(%u) to %p4cc s:%u d:%d len:%zu",
- msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid),
- &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid),
- VCHIQ_MSG_DSTPORT(msgid), size);
+ dev_dbg(state->dev, "core_msg: Sent Msg %s(%u) to %p4cc s:%u d:%d len:%zu\n",
+ msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid),
+ &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid), size);
}
/* Make sure the new header is visible to the peer. */
@@ -1170,11 +1164,9 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
state->id, oldmsgid);
}
- vchiq_log_debug(state->dev, VCHIQ_SYNC,
- "%d: qms %s@%pK,%x (%d->%d)", state->id,
- msg_type_str(VCHIQ_MSG_TYPE(msgid)),
- header, size, VCHIQ_MSG_SRCPORT(msgid),
- VCHIQ_MSG_DSTPORT(msgid));
+ dev_dbg(state->dev, "sync: %d: qms %s@%pK,%x (%d->%d)\n",
+ state->id, msg_type_str(VCHIQ_MSG_TYPE(msgid)), header, size,
+ VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid));
callback_result =
copy_message_data(copy_callback, context,
@@ -1260,9 +1252,9 @@ release_slot(struct vchiq_state *state, struct vchiq_slot_info *slot_info,
VCHIQ_SLOT_QUEUE_MASK] =
SLOT_INDEX_FROM_INFO(state, slot_info);
state->remote->slot_queue_recycle = slot_queue_recycle + 1;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: %s %d - recycle->%x",
- state->id, __func__, SLOT_INDEX_FROM_INFO(state, slot_info),
- state->remote->slot_queue_recycle);
+ dev_dbg(state->dev, "core: %d: %d - recycle->%x\n",
+ state->id, SLOT_INDEX_FROM_INFO(state, slot_info),
+ state->remote->slot_queue_recycle);
/*
* A write barrier is necessary, but remote_event_signal
@@ -1381,8 +1373,8 @@ poll_services_of_group(struct vchiq_state *state, int group)
service_flags = atomic_xchg(&service->poll_flags, 0);
if (service_flags & BIT(VCHIQ_POLL_REMOVE)) {
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: ps - remove %d<->%d",
- state->id, service->localport, service->remoteport);
+ dev_dbg(state->dev, "core: %d: ps - remove %d<->%d\n",
+ state->id, service->localport, service->remoteport);
/*
* Make it look like a client, because
@@ -1394,8 +1386,8 @@ poll_services_of_group(struct vchiq_state *state, int group)
if (vchiq_close_service_internal(service, NO_CLOSE_RECVD))
request_poll(state, service, VCHIQ_POLL_REMOVE);
} else if (service_flags & BIT(VCHIQ_POLL_TERMINATE)) {
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: ps - terminate %d<->%d",
- state->id, service->localport, service->remoteport);
+ dev_dbg(state->dev, "core: %d: ps - terminate %d<->%d\n",
+ state->id, service->localport, service->remoteport);
if (vchiq_close_service_internal(service, NO_CLOSE_RECVD))
request_poll(state, service, VCHIQ_POLL_TERMINATE);
}
@@ -1447,11 +1439,11 @@ abort_outstanding_bulks(struct vchiq_service *service,
if (queue->process != queue->local_insert) {
vchiq_complete_bulk(service->instance, bulk);
- vchiq_log_debug(service->state->dev, VCHIQ_CORE_MSG,
- "%s %p4cc d:%d ABORTED - tx len:%d, rx len:%d",
- is_tx ? "Send Bulk to" : "Recv Bulk from",
- &service->base.fourcc,
- service->remoteport, bulk->size, bulk->remote_size);
+ dev_dbg(service->state->dev,
+ "core_msg: %s %p4cc d:%d ABORTED - tx len:%d, rx len:%d\n",
+ is_tx ? "Send Bulk to" : "Recv Bulk from",
+ &service->base.fourcc,
+ service->remoteport, bulk->size, bulk->remote_size);
} else {
/* fabricate a matching dummy bulk */
bulk->data = 0;
@@ -1484,8 +1476,8 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
payload = (struct vchiq_open_payload *)header->data;
fourcc = payload->fourcc;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: prs OPEN@%pK (%d->'%p4cc')",
- state->id, header, localport, &fourcc);
+ dev_dbg(state->dev, "core: %d: prs OPEN@%pK (%d->'%p4cc')\n",
+ state->id, header, localport, &fourcc);
service = get_listening_service(state, fourcc);
if (!service)
@@ -1630,10 +1622,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
svc_fourcc = service ? service->base.fourcc
: VCHIQ_MAKE_FOURCC('?', '?', '?', '?');
- vchiq_log_debug(state->dev, VCHIQ_CORE_MSG,
- "Rcvd Msg %s(%u) from %p4cc s:%d d:%d len:%d",
- msg_type_str(type), type, &svc_fourcc,
- remoteport, localport, size);
+ dev_dbg(state->dev, "core_msg: Rcvd Msg %s(%u) from %p4cc s:%d d:%d len:%d\n",
+ msg_type_str(type), type, &svc_fourcc, remoteport, localport, size);
if (size > 0)
vchiq_log_dump_mem(state->dev, "Rcvd", 0, header->data, min(16, size));
@@ -1657,10 +1647,10 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
header->data;
service->peer_version = payload->version;
}
- vchiq_log_debug(state->dev, VCHIQ_CORE,
- "%d: prs OPENACK@%pK,%x (%d->%d) v:%d",
- state->id, header, size, remoteport, localport,
- service->peer_version);
+ dev_dbg(state->dev,
+ "core: %d: prs OPENACK@%pK,%x (%d->%d) v:%d\n",
+ state->id, header, size, remoteport, localport,
+ service->peer_version);
if (service->srvstate == VCHIQ_SRVSTATE_OPENING) {
service->remoteport = remoteport;
set_service_state(service, VCHIQ_SRVSTATE_OPEN);
@@ -1673,21 +1663,20 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
case VCHIQ_MSG_CLOSE:
WARN_ON(size); /* There should be no data */
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: prs CLOSE@%pK (%d->%d)",
- state->id, header, remoteport, localport);
+ dev_dbg(state->dev, "core: %d: prs CLOSE@%pK (%d->%d)\n",
+ state->id, header, remoteport, localport);
mark_service_closing_internal(service, 1);
if (vchiq_close_service_internal(service, CLOSE_RECVD) == -EAGAIN)
goto bail_not_ready;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "Close Service %p4cc s:%u d:%d",
- &service->base.fourcc,
- service->localport, service->remoteport);
+ dev_dbg(state->dev, "core: Close Service %p4cc s:%u d:%d\n",
+ &service->base.fourcc, service->localport, service->remoteport);
break;
case VCHIQ_MSG_DATA:
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: prs DATA@%pK,%x (%d->%d)",
- state->id, header, size, remoteport, localport);
+ dev_dbg(state->dev, "core: %d: prs DATA@%pK,%x (%d->%d)\n",
+ state->id, header, size, remoteport, localport);
if ((service->remoteport == remoteport) &&
(service->srvstate == VCHIQ_SRVSTATE_OPEN)) {
@@ -1706,8 +1695,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
}
break;
case VCHIQ_MSG_CONNECT:
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: prs CONNECT@%pK",
- state->id, header);
+ dev_dbg(state->dev, "core: %d: prs CONNECT@%pK\n",
+ state->id, header);
state->version_common = ((struct vchiq_slot_zero *)
state->slot_data)->version;
complete(&state->connect);
@@ -1758,10 +1747,9 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
bulk->actual = *(int *)header->data;
queue->remote_insert++;
- vchiq_log_debug(state->dev, VCHIQ_CORE,
- "%d: prs %s@%pK (%d->%d) %x@%pad",
- state->id, msg_type_str(type), header, remoteport,
- localport, bulk->actual, &bulk->data);
+ dev_dbg(state->dev, "core: %d: prs %s@%pK (%d->%d) %x@%pad\n",
+ state->id, msg_type_str(type), header, remoteport,
+ localport, bulk->actual, &bulk->data);
dev_dbg(state->dev, "core: %d: prs:%d %cx li=%x ri=%x p=%x\n",
state->id, localport,
@@ -2052,9 +2040,9 @@ sync_func(void *v)
header->data;
service->peer_version = payload->version;
}
- vchiq_log_debug(state->dev, VCHIQ_SYNC, "%d: sf OPENACK@%pK,%x (%d->%d) v:%d",
- state->id, header, size, remoteport, localport,
- service->peer_version);
+ dev_err(state->dev, "sync: %d: sf OPENACK@%pK,%x (%d->%d) v:%d\n",
+ state->id, header, size, remoteport, localport,
+ service->peer_version);
if (service->srvstate == VCHIQ_SRVSTATE_OPENING) {
service->remoteport = remoteport;
set_service_state(service, VCHIQ_SRVSTATE_OPENSYNC);
@@ -2456,9 +2444,9 @@ vchiq_add_service_internal(struct vchiq_state *state,
/* Bring this service online */
set_service_state(service, srvstate);
- vchiq_log_debug(state->dev, VCHIQ_CORE_MSG, "%s Service %p4cc SrcPort:%d",
- (srvstate == VCHIQ_SRVSTATE_OPENING) ? "Open" : "Add",
- ¶ms->fourcc, service->localport);
+ dev_dbg(state->dev, "core_msg: %s Service %p4cc SrcPort:%d\n",
+ (srvstate == VCHIQ_SRVSTATE_OPENING) ? "Open" : "Add",
+ ¶ms->fourcc, service->localport);
/* Don't unlock the service - leave it with a ref_count of 1. */
@@ -2553,8 +2541,7 @@ release_service_messages(struct vchiq_service *service)
int port = VCHIQ_MSG_DSTPORT(msgid);
if ((port == service->localport) && (msgid & VCHIQ_MSGID_CLAIMED)) {
- vchiq_log_debug(state->dev, VCHIQ_CORE,
- " fsi - hdr %pK", header);
+ dev_dbg(state->dev, "core: fsi - hdr %pK\n", header);
release_slot(state, slot_info, header, NULL);
}
pos += calc_stride(header->size);
@@ -2662,8 +2649,9 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
int close_id = MAKE_CLOSE(service->localport,
VCHIQ_MSG_DSTPORT(service->remoteport));
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: csi:%d,%d (%s)", service->state->id,
- service->localport, close_recvd, srvstate_names[service->srvstate]);
+ dev_dbg(state->dev, "core: %d: csi:%d,%d (%s)\n",
+ service->state->id, service->localport, close_recvd,
+ srvstate_names[service->srvstate]);
switch (service->srvstate) {
case VCHIQ_SRVSTATE_CLOSED:
@@ -2773,8 +2761,8 @@ vchiq_terminate_service_internal(struct vchiq_service *service)
{
struct vchiq_state *state = service->state;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: tsi - (%d<->%d)", state->id,
- service->localport, service->remoteport);
+ dev_dbg(state->dev, "core: %d: tsi - (%d<->%d)\n",
+ state->id, service->localport, service->remoteport);
mark_service_closing(service);
@@ -2788,8 +2776,7 @@ vchiq_free_service_internal(struct vchiq_service *service)
{
struct vchiq_state *state = service->state;
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: fsi - (%d)",
- state->id, service->localport);
+ dev_dbg(state->dev, "core: %d: fsi - (%d)\n", state->id, service->localport);
switch (service->srvstate) {
case VCHIQ_SRVSTATE_OPENING:
@@ -2869,8 +2856,8 @@ vchiq_close_service(struct vchiq_instance *instance, unsigned int handle)
if (!service)
return -EINVAL;
- vchiq_log_debug(service->state->dev, VCHIQ_CORE, "%d: close_service:%d",
- service->state->id, service->localport);
+ dev_dbg(service->state->dev, "core: %d: close_service:%d\n",
+ service->state->id, service->localport);
if ((service->srvstate == VCHIQ_SRVSTATE_FREE) ||
(service->srvstate == VCHIQ_SRVSTATE_LISTENING) ||
@@ -2927,8 +2914,8 @@ vchiq_remove_service(struct vchiq_instance *instance, unsigned int handle)
if (!service)
return -EINVAL;
- vchiq_log_debug(service->state->dev, VCHIQ_CORE, "%d: remove_service:%d",
- service->state->id, service->localport);
+ dev_dbg(service->state->dev, "core: %d: remove_service:%d\n",
+ service->state->id, service->localport);
if (service->srvstate == VCHIQ_SRVSTATE_FREE) {
vchiq_service_put(service);
@@ -3071,9 +3058,9 @@ int vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle,
*/
wmb();
- vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: bt (%d->%d) %cx %x@%pad %pK",
- state->id, service->localport, service->remoteport,
- dir_char, size, &bulk->data, userdata);
+ dev_dbg(state->dev, "core: %d: bt (%d->%d) %cx %x@%pad %pK\n",
+ state->id, service->localport, service->remoteport,
+ dir_char, size, &bulk->data, userdata);
/*
* The slot mutex must be held when the service is being closed, so
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 36e34a7fc628..c8527551b58c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -32,32 +32,6 @@
#define VCHIQ_SLOT_SIZE 4096
#define VCHIQ_MAX_MSG_SIZE (VCHIQ_SLOT_SIZE - sizeof(struct vchiq_header))
-enum vchiq_log_category {
- VCHIQ_ARM,
- VCHIQ_CORE,
- VCHIQ_CORE_MSG,
- VCHIQ_SYNC,
- VCHIQ_SUSPEND,
-};
-
-static inline const char *log_category_str(enum vchiq_log_category c)
-{
- static const char * const strings[] = {
- "vchiq_arm",
- "vchiq_core",
- "vchiq_core_msg",
- "vchiq_sync",
- "vchiq_suspend",
- };
-
- return strings[c];
-};
-
-#ifndef vchiq_log_debug
-#define vchiq_log_debug(dev, cat, fmt, ...) \
- do { dev_dbg(dev, "%s debug: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
-#endif
-
#define VCHIQ_SLOT_MASK (VCHIQ_SLOT_SIZE - 1)
#define VCHIQ_SLOT_QUEUE_MASK (VCHIQ_MAX_SLOTS_PER_SIDE - 1)
#define VCHIQ_SLOT_ZERO_SLOTS DIV_ROUND_UP(sizeof(struct vchiq_slot_zero), \
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index 38841c9d8ef9..68c8897c504c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -47,9 +47,8 @@ user_service_free(void *userdata)
static void close_delivered(struct user_service *user_service)
{
- vchiq_log_debug(user_service->service->state->dev, VCHIQ_ARM,
- "%s(handle=%x)",
- __func__, user_service->service->handle);
+ dev_dbg(user_service->service->state->dev,
+ "arm: (handle=%x)\n", user_service->service->handle);
if (user_service->close_pending) {
/* Allow the underlying service to be culled */
@@ -235,8 +234,7 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
spin_unlock(&msg_queue_spinlock);
DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
if (wait_for_completion_interruptible(&user_service->insert_event)) {
- vchiq_log_debug(service->state->dev, VCHIQ_ARM,
- "DEQUEUE_MESSAGE interrupted");
+ dev_dbg(service->state->dev, "arm: DEQUEUE_MESSAGE interrupted\n");
ret = -EINTR;
break;
}
@@ -323,8 +321,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
ret = -ESRCH;
goto out;
}
- vchiq_log_debug(service->state->dev, VCHIQ_ARM,
- "found bulk_waiter %pK for pid %d", waiter, current->pid);
+ dev_dbg(service->state->dev, "arm: found bulk_waiter %pK for pid %d\n",
+ waiter, current->pid);
userdata = &waiter->bulk_waiter;
} else {
userdata = args->userdata;
@@ -355,8 +353,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
mutex_lock(&instance->bulk_waiter_list_mutex);
list_add(&waiter->list, &instance->bulk_waiter_list);
mutex_unlock(&instance->bulk_waiter_list_mutex);
- vchiq_log_debug(service->state->dev, VCHIQ_ARM,
- "saved bulk_waiter %pK for pid %d", waiter, current->pid);
+ dev_dbg(service->state->dev, "arm: saved bulk_waiter %pK for pid %d\n",
+ waiter, current->pid);
ret = put_user(mode_waiting, mode);
}
@@ -455,8 +453,7 @@ static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
mutex_lock(&instance->completion_mutex);
if (rc) {
DEBUG_TRACE(AWAIT_COMPLETION_LINE);
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- "AWAIT_COMPLETION interrupted");
+ dev_dbg(instance->state->dev, "arm: AWAIT_COMPLETION interrupted\n");
ret = -EINTR;
goto out;
}
@@ -867,10 +864,10 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
}
if (!status && (ret < 0) && (ret != -EINTR) && (ret != -EWOULDBLOCK)) {
- vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
- " ioctl instance %pK, cmd %s -> status %d, %ld",
- instance, (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
- ioctl_names[_IOC_NR(cmd)] : "<invalid>", status, ret);
+ dev_dbg(instance->state->dev,
+ "arm: ioctl instance %pK, cmd %s -> status %d, %ld\n",
+ instance, (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
+ ioctl_names[_IOC_NR(cmd)] : "<invalid>", status, ret);
} else {
dev_dbg(instance->state->dev,
"arm: ioctl instance %pK, cmd %s -> status %d\n, %ld\n",
@@ -1169,7 +1166,7 @@ static int vchiq_open(struct inode *inode, struct file *file)
struct vchiq_state *state = vchiq_get_state();
struct vchiq_instance *instance;
- vchiq_log_debug(state->dev, VCHIQ_ARM, "vchiq_open");
+ dev_dbg(state->dev, "arm: vchiq open\n");
if (!state) {
dev_err(state->dev, "arm: vchiq has no connection to VideoCore\n");
@@ -1204,8 +1201,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
int ret = 0;
int i;
- vchiq_log_debug(state->dev, VCHIQ_ARM, "%s: instance=%lx", __func__,
- (unsigned long)instance);
+ dev_dbg(state->dev, "arm: instance=%lx\n", (unsigned long)instance);
if (!state) {
ret = -EPERM;
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 4/4] staging: vc04_services: Drop vchiq_log_debug() in favour of dev_dbg
2023-12-05 8:41 ` [PATCH v3 4/4] staging: vc04_services: Drop vchiq_log_debug() " Umang Jain
@ 2023-12-08 13:00 ` Stefan Wahren
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Wahren @ 2023-12-08 13:00 UTC (permalink / raw)
To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media,
linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Ricardo B . Marliere
Hi Umang,
Am 05.12.23 um 09:41 schrieb Umang Jain:
> Drop vchiq_log_debug() macro which wraps dev_dbg(). Introduce the usage
> of dev_dbg() directly.
>
> Meanwhile at it, drop the usage of __func__ from the logs.
> Dynamic debug supports this via the 'f' decorator flag.
>
> Remove the entry from TODO regarding custom logging. VC04 is now
> aligned according to the standard kernel logging mechanisms.
personally i would have mentioned that this patch also drop the now
unused logging categories. Except of this i'm fine with the patch.
Possibled future improvements are mentioned below
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> drivers/staging/vc04_services/interface/TODO | 5 -
> .../interface/vchiq_arm/vchiq_arm.c | 45 +++--
> .../interface/vchiq_arm/vchiq_core.c | 159 ++++++++----------
> .../interface/vchiq_arm/vchiq_core.h | 26 ---
> .../interface/vchiq_arm/vchiq_dev.c | 32 ++--
> 5 files changed, 106 insertions(+), 161 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
> index 6d9d4a800aa7..05eb5140d096 100644
> --- a/drivers/staging/vc04_services/interface/TODO
> +++ b/drivers/staging/vc04_services/interface/TODO
> @@ -23,11 +23,6 @@ should properly handle a module unload. This also includes that all
> resources must be freed (kthreads, debugfs entries, ...) and global
> variables avoided.
>
> -* Cleanup logging mechanism
> -
> -The driver should probably be using the standard kernel logging mechanisms
> -such as dev_info, dev_dbg, and friends.
> -
> * Documentation
>
> A short top-down description of this driver's architecture (function of
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index e337b8387647..4b4ff469d3a3 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -310,9 +310,8 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
> type == PAGELIST_READ, pages);
>
> if (actual_pages != num_pages) {
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "%s - only %d/%d pages locked",
> - __func__, actual_pages, num_pages);
> + dev_dbg(instance->state->dev, "arm: Only %d/%d pages locked\n",
> + actual_pages, num_pages);
>
> /* This is probably due to the process being killed */
> if (actual_pages > 0)
> @@ -554,8 +553,8 @@ static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state
> return -ENXIO;
> }
>
> - vchiq_log_debug(&pdev->dev, VCHIQ_ARM, "vchiq_init - done (slots %pK, phys %pad)",
> - vchiq_slot_zero, &slot_phys);
> + dev_dbg(&pdev->dev, "arm: vchiq_init - done (slots %pK, phys %pad)\n",
> + vchiq_slot_zero, &slot_phys);
>
> vchiq_call_connected_callbacks();
>
> @@ -719,9 +718,9 @@ 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_debug(instance->state->dev, VCHIQ_ARM,
> - "bulk_waiter - cleaned up %pK for pid %d",
> - waiter, waiter->pid);
> + dev_dbg(instance->state->dev,
> + "arm: bulk_waiter - cleaned up %pK for pid %d\n",
> + waiter, waiter->pid);
> kfree(waiter);
> }
> }
> @@ -981,9 +980,8 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
> mutex_lock(&instance->bulk_waiter_list_mutex);
> list_add(&waiter->list, &instance->bulk_waiter_list);
> mutex_unlock(&instance->bulk_waiter_list_mutex);
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "saved bulk_waiter %pK for pid %d", waiter,
> - current->pid);
> + dev_dbg(instance->state->dev, "arm: saved bulk_waiter %pK for pid %d\n",
> + waiter, current->pid);
> }
>
> return status;
> @@ -1006,12 +1004,10 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
> dev_dbg(instance->state->dev, "core: completion queue full\n");
> DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
> if (wait_for_completion_interruptible(&instance->remove_event)) {
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "service_callback interrupted");
> + dev_dbg(instance->state->dev, "arm: service_callback interrupted\n");
> return -EAGAIN;
> } else if (instance->closing) {
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "service_callback closing");
> + dev_dbg(instance->state->dev, "arm: service_callback closing\n");
> return 0;
> }
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> @@ -1113,8 +1109,8 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
> instance->completion_remove) < 0) {
> int status;
>
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "Inserting extra MESSAGE_AVAILABLE");
> + dev_dbg(instance->state->dev,
> + "arm: Inserting extra MESSAGE_AVAILABLE\n");
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> status = add_completion(instance, reason, NULL, user_service,
> bulk_userdata);
> @@ -1127,14 +1123,12 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
>
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> if (wait_for_completion_interruptible(&user_service->remove_event)) {
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "%s interrupted", __func__);
> + dev_dbg(instance->state->dev, "arm: interrupted\n");
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> vchiq_service_put(service);
> return -EAGAIN;
> } else if (instance->closing) {
> - vchiq_log_debug(instance->state->dev, VCHIQ_ARM,
> - "%s closing", __func__);
> + dev_dbg(instance->state->dev, "arm: closing\n");
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> vchiq_service_put(service);
> return -EINVAL;
> @@ -1686,8 +1680,8 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
> struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);
> char threadname[16];
>
> - vchiq_log_debug(state->dev, VCHIQ_SUSPEND, "%d: %s->%s", state->id,
> - get_conn_state_name(oldstate), get_conn_state_name(newstate));
> + dev_dbg(state->dev, "suspend: %d: %s->%s\n",
> + state->id, get_conn_state_name(oldstate), get_conn_state_name(newstate));
> if (state->conn_state != VCHIQ_CONNSTATE_CONNECTED)
> return;
>
> @@ -1751,9 +1745,8 @@ static int vchiq_probe(struct platform_device *pdev)
>
> vchiq_debugfs_init();
>
> - vchiq_log_debug(&pdev->dev, VCHIQ_ARM,
> - "vchiq: platform initialised - version %d (min %d)",
> - VCHIQ_VERSION, VCHIQ_VERSION_MIN);
> + dev_dbg(&pdev->dev, "arm: platform initialised - version %d (min %d)\n",
> + VCHIQ_VERSION, VCHIQ_VERSION_MIN);
>
> /*
> * Simply exit on error since the function handles cleanup in
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index d197e4504bd4..76c27778154a 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -217,10 +217,10 @@ static const char *msg_type_str(unsigned int msg_type)
> static inline void
> set_service_state(struct vchiq_service *service, int newstate)
> {
> - vchiq_log_debug(service->state->dev, VCHIQ_CORE, "%d: srv:%d %s->%s",
> - service->state->id, service->localport,
> - srvstate_names[service->srvstate],
> - srvstate_names[newstate]);
> + dev_dbg(service->state->dev, "core: %d: srv:%d %s->%s\n",
> + service->state->id, service->localport,
> + srvstate_names[service->srvstate],
> + srvstate_names[newstate]);
> service->srvstate = newstate;
> }
>
> @@ -245,8 +245,7 @@ find_service_by_handle(struct vchiq_instance *instance, unsigned int handle)
> return service;
> }
> rcu_read_unlock();
> - vchiq_log_debug(instance->state->dev, VCHIQ_CORE,
> - "Invalid service handle 0x%x", handle);
> + dev_dbg(instance->state->dev, "core: Invalid service handle 0x%x\n", handle);
From my PoV this is a real error, so maybe a WARN_ONCE. But looking
deeper at this shows that the error handling must be reworked first :-(
^ permalink raw reply [flat|nested] 16+ messages in thread