[v3,1/4] staging: vc04_services: Drop vchiq_log_error() in favour of dev_err
Commit Message
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(-)
Comments
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;
> }
>
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;
>> }
>>
>
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
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
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
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
>
@@ -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);
}
@@ -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;
@@ -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;
}
@@ -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)
@@ -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;
}