Re: [PATCH v1 1/4] staging: vc04: Convert vchiq_log_error() to use dynamic debug

Kieran Bingham
2023-10-19T13:32:58Z

Quoting Umang Jain (2023-10-19 09:21:44)
> Move vchiq_log_error() custom logging wrapper based on printk,
> to use dynamic debug. To categorise, enum vchiq_log_category
> has been introduced, which will become the part of the error
> string format that will be logged to dynamic debug (for grep).
>
> All the vchiq_log_error() calls are adjusted to use the
> dynamic debug wrapper. vchiq_loud_error_*() has been removed
> as a part of this patch and replaced with dev_err (so that
> they directly end up in kernel log, even if dynamic debug
> isn't enabled), which serves the purpose.

So far that looks quite reasonable to me. They all look like they are
'real error' messages indeed.

>
> For cases where struct device pointer `dev` is not available,
> NULL can be passed to vchiq_log_error() to log to dynamic debug
> using pr_debug().

That sounds reasonable.

I see two uses below. Maybe they could be inlined, but this way keeps
the category strings which may help a developer when grepping...

>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 36 ++++---
> .../interface/vchiq_arm/vchiq_connected.c | 2 +-
> .../interface/vchiq_arm/vchiq_core.c | 100 +++++++-----------
> .../interface/vchiq_arm/vchiq_core.h | 33 ++++--
> .../interface/vchiq_arm/vchiq_dev.c | 14 +--
> 5 files changed, 91 insertions(+), 94 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 eef9c8c06e66..fa4463bfc117 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -690,7 +690,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
> usleep_range(500, 600);
> }
> if (i == VCHIQ_INIT_RETRIES) {
> - vchiq_log_error(vchiq_core_log_level, "%s: videocore not initialized\n", __func__);
> + vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not initialized\n",
> + __func__);
> ret = -ENOTCONN;
> goto failed;
> } else if (i > 0) {
> @@ -700,7 +701,7 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
>
> instance = kzalloc(sizeof(*instance), GFP_KERNEL);
> if (!instance) {
> - vchiq_log_error(vchiq_core_log_level,
> + vchiq_log_error(state->dev, VCHIQ_CORE,
> "%s: error allocating vchiq instance\n", __func__);
> ret = -ENOMEM;
> goto failed;
> @@ -966,7 +967,8 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
> } else {
> waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
> if (!waiter) {
> - vchiq_log_error(vchiq_core_log_level, "%s - out of memory", __func__);
> + vchiq_log_error(service->state->dev, VCHIQ_CORE,
> + "%s - out of memory", __func__);
> return -ENOMEM;
> }
> }
> @@ -1333,7 +1335,8 @@ vchiq_keepalive_vchiq_callback(struct vchiq_instance *instance,
> struct vchiq_header *header,
> unsigned int service_user, void *bulk_user)
> {
> - vchiq_log_error(vchiq_susp_log_level, "%s callback reason %d", __func__, reason);
> + vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND,
> + "%s callback reason %d", __func__, reason);
> return 0;
> }
>
> @@ -1357,22 +1360,22 @@ vchiq_keepalive_thread_func(void *v)
>
> ret = vchiq_initialise(&instance);
> if (ret) {
> - vchiq_log_error(vchiq_susp_log_level, "%s vchiq_initialise failed %d", __func__,
> - ret);
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> + "%s vchiq_initialise failed %d", __func__, ret);
> goto exit;
> }
>
> status = vchiq_connect(instance);
> if (status) {
> - vchiq_log_error(vchiq_susp_log_level, "%s vchiq_connect failed %d", __func__,
> - status);
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> + "%s vchiq_connect failed %d", __func__, status);
> goto shutdown;
> }
>
> status = vchiq_add_service(instance, &params, &ka_handle);
> if (status) {
> - vchiq_log_error(vchiq_susp_log_level, "%s vchiq_open_service failed %d", __func__,
> - status);
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> + "%s vchiq_open_service failed %d", __func__, status);
> goto shutdown;
> }
>
> @@ -1380,7 +1383,8 @@ vchiq_keepalive_thread_func(void *v)
> long rc = 0, uc = 0;
>
> if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
> - vchiq_log_error(vchiq_susp_log_level, "%s interrupted", __func__);
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> + "%s interrupted", __func__);
> flush_signals(current);
> continue;
> }
> @@ -1400,14 +1404,14 @@ 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(vchiq_susp_log_level,
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> "%s vchiq_use_service error %d", __func__, status);
> }
> }
> while (rc--) {
> status = vchiq_release_service(instance, ka_handle);
> if (status) {
> - vchiq_log_error(vchiq_susp_log_level,
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> "%s vchiq_release_service error %d", __func__,
> status);
> }
> @@ -1444,7 +1448,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(vchiq_susp_log_level, "%s null service ptr", __func__);
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND, "%s null service ptr", __func__);
> ret = -EINVAL;
> goto out;
> }
> @@ -1725,7 +1729,7 @@ vchiq_check_service(struct vchiq_service *service)
> read_unlock_bh(&arm_state->susp_res_lock);
>
> if (ret) {
> - vchiq_log_error(vchiq_susp_log_level,
> + vchiq_log_error(service->state->dev, VCHIQ_SUSPEND,
> "%s ERROR - %c%c%c%c:%d service count %d, state count %d", __func__,
> VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), service->client_id,
> service->service_use_count, arm_state->videocore_use_count);
> @@ -1761,7 +1765,7 @@ void vchiq_platform_conn_state_changed(struct vchiq_state *state,
> (void *)state,
> threadname);
> if (IS_ERR(arm_state->ka_thread)) {
> - vchiq_log_error(vchiq_susp_log_level,
> + vchiq_log_error(state->dev, VCHIQ_SUSPEND,
> "vchiq: FATAL: couldn't create thread %s",
> threadname);
> } else {
> 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 bdb0ab617d8b..b3928bd8c9c6 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> @@ -39,7 +39,7 @@ void vchiq_add_connected_callback(void (*callback)(void))
> callback();
> } else {
> if (g_num_deferred_callbacks >= MAX_CALLBACKS) {
> - vchiq_log_error(vchiq_core_log_level,
> + vchiq_log_error(NULL, VCHIQ_CORE,
> "There already %d callback registered - please increase MAX_CALLBACKS",
> g_num_deferred_callbacks);

Aha, here's a second use where we don't have access to a dev pointer.

> } else {
> 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 596894338cb4..f6032c6022cb 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -751,7 +751,7 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
> */
> complete(&quota->quota_event);
> } else if (count == 0) {
> - vchiq_log_error(vchiq_core_log_level,
> + 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);
> @@ -776,7 +776,7 @@ process_free_data_message(struct vchiq_state *state, u32 *service_found,
> vchiq_log_trace(vchiq_core_log_level, "%d: pfq:%d %x@%pK - slot_use->%d",
> state->id, port, header->size, header, count - 1);
> } else {
> - vchiq_log_error(vchiq_core_log_level,
> + 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);
> WARN(1, "bad slot use count\n");
> @@ -841,7 +841,7 @@ process_free_queue(struct vchiq_state *state, u32 *service_found,
>
> pos += calc_stride(header->size);
> if (pos > VCHIQ_SLOT_SIZE) {
> - vchiq_log_error(vchiq_core_log_level,
> + 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);
> WARN(1, "invalid slot position\n");
> @@ -1178,7 +1178,7 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
> int oldmsgid = header->msgid;
>
> if (oldmsgid != VCHIQ_MSGID_PADDING)
> - vchiq_log_error(vchiq_core_log_level, "%d: qms - msgid %x, not PADDING",
> + vchiq_log_error(state->dev, VCHIQ_CORE, "%d: qms - msgid %x, not PADDING",
> state->id, oldmsgid);
> }
>
> @@ -1512,11 +1512,9 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
>
> if ((service->version < version_min) || (version < service->version_min)) {
> /* Version mismatch */
> - vchiq_loud_error_header();
> - vchiq_loud_error("%d: service %d (%c%c%c%c) version mismatch - local (%d, min %d) vs. remote (%d, min %d)",
> - state->id, service->localport, VCHIQ_FOURCC_AS_4CHARS(fourcc),
> - service->version, service->version_min, version, version_min);
> - vchiq_loud_error_footer();
> + dev_err(state->dev, "%d: service %d (%c%c%c%c) version mismatch - local (%d, min %d) vs. remote (%d, min %d)",
> + state->id, service->localport, VCHIQ_FOURCC_AS_4CHARS(fourcc),
> + service->version, service->version_min, version, version_min);
> vchiq_service_put(service);
> service = NULL;
> goto fail_open;
> @@ -1631,7 +1629,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> }
>
> if (!service) {
> - vchiq_log_error(vchiq_core_log_level,
> + 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);
> @@ -1658,7 +1656,7 @@ 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(vchiq_core_log_level,
> + vchiq_log_error(state->dev, VCHIQ_CORE,
> "header %pK (msgid %x) - size %x too big for slot",
> header, (unsigned int)msgid, (unsigned int)size);
> WARN(1, "oversized for slot\n");
> @@ -1685,7 +1683,7 @@ 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(vchiq_core_log_level, "OPENACK received in state %s",
> + vchiq_log_error(state->dev, VCHIQ_CORE, "OPENACK received in state %s",
> srvstate_names[service->srvstate]);
> }
> break;
> @@ -1756,7 +1754,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> }
> if ((int)(queue->remote_insert -
> queue->local_insert) >= 0) {
> - vchiq_log_error(vchiq_core_log_level,
> + 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,
> @@ -1805,8 +1803,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> vchiq_log_trace(vchiq_core_log_level, "%d: prs PAUSE@%pK,%x",
> state->id, header, size);
> if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
> - vchiq_log_error(vchiq_core_log_level, "%d: PAUSE received in state PAUSED",
> - state->id);
> + vchiq_log_error(state->dev, VCHIQ_CORE,
> + "%d: PAUSE received in state PAUSED", state->id);
> break;
> }
> if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
> @@ -1836,7 +1834,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
> break;
>
> default:
> - vchiq_log_error(vchiq_core_log_level, "%d: prs invalid msgid %x@%pK,%x",
> + vchiq_log_error(state->dev, VCHIQ_CORE, "%d: prs invalid msgid %x@%pK,%x",
> state->id, msgid, header, size);
> WARN(1, "invalid message\n");
> break;
> @@ -1947,7 +1945,7 @@ handle_poll(struct vchiq_state *state)
> * since the PAUSE should have flushed
> * through outstanding messages.
> */
> - vchiq_log_error(vchiq_core_log_level, "Failed to send RESUME message");
> + vchiq_log_error(state->dev, VCHIQ_CORE, "Failed to send RESUME message");
> }
> break;
> default:
> @@ -2046,7 +2044,7 @@ sync_func(void *v)
> service = find_service_by_port(state, localport);
>
> if (!service) {
> - vchiq_log_error(vchiq_sync_log_level,
> + 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);
> @@ -2096,14 +2094,14 @@ sync_func(void *v)
> (service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
> if (make_service_callback(service, VCHIQ_MESSAGE_AVAILABLE, header,
> NULL) == -EAGAIN)
> - vchiq_log_error(vchiq_sync_log_level,
> + vchiq_log_error(state->dev, VCHIQ_SYNC,
> "synchronous callback to service %d returns -EAGAIN",
> localport);
> }
> break;
>
> default:
> - vchiq_log_error(vchiq_sync_log_level, "%d: sf unexpected msgid %x@%pK,%x",
> + vchiq_log_error(state->dev, VCHIQ_SYNC, "%d: sf unexpected msgid %x@%pK,%x",
> state->id, msgid, header, size);
> release_message_sync(state, header);
> break;
> @@ -2137,7 +2135,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
> num_slots -= first_data_slot;
>
> if (num_slots < 4) {
> - vchiq_log_error(vchiq_core_log_level, "%s - insufficient memory %x bytes",
> + vchiq_log_error(NULL, VCHIQ_CORE, "%s - insufficient memory %x bytes",
> __func__, mem_size);

Is this the only call without a 'dev' pointer?
I wonder if this could just as easily be a pr_error directly.
<edit - no it's not the only one>

> return NULL;
> }
> @@ -2174,12 +2172,11 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero, s
> remote = &slot_zero->master;
>
> if (local->initialised) {
> - vchiq_loud_error_header();
> if (remote->initialised)
> - vchiq_loud_error("local state has already been initialised");
> + dev_err(dev, "local state has already been initialised\n");
> else
> - vchiq_loud_error("master/slave mismatch two slaves");
> - vchiq_loud_error_footer();
> + dev_err(dev, "master/slave mismatch two slaves\n");
> +
> return -EINVAL;
> }
>
> @@ -2257,9 +2254,7 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero, s
> state->slot_handler_thread = kthread_create(&slot_handler_func, (void *)state, threadname);
>
> if (IS_ERR(state->slot_handler_thread)) {
> - vchiq_loud_error_header();
> - vchiq_loud_error("couldn't create thread %s", threadname);
> - vchiq_loud_error_footer();
> + dev_err(state->dev, "couldn't create thread %s\n", threadname);
> return PTR_ERR(state->slot_handler_thread);
> }
> set_user_nice(state->slot_handler_thread, -19);
> @@ -2267,9 +2262,7 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero, s
> snprintf(threadname, sizeof(threadname), "vchiq-recy/%d", state->id);
> state->recycle_thread = kthread_create(&recycle_func, (void *)state, threadname);
> if (IS_ERR(state->recycle_thread)) {
> - vchiq_loud_error_header();
> - vchiq_loud_error("couldn't create thread %s", threadname);
> - vchiq_loud_error_footer();
> + dev_err(state->dev, "couldn't create thread %s\n", threadname);
> ret = PTR_ERR(state->recycle_thread);
> goto fail_free_handler_thread;
> }
> @@ -2278,9 +2271,7 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero, s
> snprintf(threadname, sizeof(threadname), "vchiq-sync/%d", state->id);
> state->sync_thread = kthread_create(&sync_func, (void *)state, threadname);
> if (IS_ERR(state->sync_thread)) {
> - vchiq_loud_error_header();
> - vchiq_loud_error("couldn't create thread %s", threadname);
> - vchiq_loud_error_footer();
> + dev_err(state->dev, "couldn't create thread %s\n", threadname);
> ret = PTR_ERR(state->sync_thread);
> goto fail_free_recycle_thread;
> }
> @@ -2353,10 +2344,11 @@ struct vchiq_header *vchiq_msg_hold(struct vchiq_instance *instance, unsigned in
> }
> EXPORT_SYMBOL(vchiq_msg_hold);
>
> -static int vchiq_validate_params(const struct vchiq_service_params_kernel *params)
> +static int vchiq_validate_params(struct vchiq_state *state,
> + const struct vchiq_service_params_kernel *params)
> {
> if (!params->callback || !params->fourcc) {
> - vchiq_loud_error("Can't add service, invalid params\n");
> + dev_err(state->dev, "Can't add service, invalid params\n");
> return -EINVAL;
> }
>
> @@ -2376,7 +2368,7 @@ vchiq_add_service_internal(struct vchiq_state *state,
> int ret;
> int i;
>
> - ret = vchiq_validate_params(params);
> + ret = vchiq_validate_params(state, params);
> if (ret)
> return NULL;
>
> @@ -2525,7 +2517,7 @@ 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(vchiq_core_log_level,
> + vchiq_log_error(service->state->dev, VCHIQ_CORE,
> "%d: osi - srvstate = %s (ref %u)",
> service->state->id,
> srvstate_names[service->srvstate],
> @@ -2589,7 +2581,7 @@ release_service_messages(struct vchiq_service *service)
> }
> pos += calc_stride(header->size);
> if (pos > VCHIQ_SLOT_SIZE) {
> - vchiq_log_error(vchiq_core_log_level,
> + 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);
> WARN(1, "invalid slot position\n");
> @@ -2645,8 +2637,8 @@ close_service_complete(struct vchiq_service *service, int failstate)
> case VCHIQ_SRVSTATE_LISTENING:
> break;
> default:
> - vchiq_log_error(vchiq_core_log_level, "%s(%x) called in state %s", __func__,
> - service->handle, srvstate_names[service->srvstate]);
> + vchiq_log_error(service->state->dev, VCHIQ_CORE, "%s(%x) called in state %s",
> + __func__, service->handle, srvstate_names[service->srvstate]);
> WARN(1, "%s in unexpected state\n", __func__);
> return -EINVAL;
> }
> @@ -2701,7 +2693,7 @@ 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(vchiq_core_log_level, "%s(1) called in state %s",
> + vchiq_log_error(state->dev, VCHIQ_CORE, "%s(1) called in state %s",
> __func__, srvstate_names[service->srvstate]);
> } else if (is_server) {
> if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
> @@ -2789,7 +2781,7 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
> break;
>
> default:
> - vchiq_log_error(vchiq_core_log_level, "%s(%d) called in state %s", __func__,
> + vchiq_log_error(state->dev, VCHIQ_CORE, "%s(%d) called in state %s", __func__,
> close_recvd, srvstate_names[service->srvstate]);
> break;
> }
> @@ -2828,7 +2820,7 @@ vchiq_free_service_internal(struct vchiq_service *service)
> case VCHIQ_SRVSTATE_CLOSEWAIT:
> break;
> default:
> - vchiq_log_error(vchiq_core_log_level, "%d: fsi - (%d) in state %s", state->id,
> + vchiq_log_error(state->dev, VCHIQ_CORE, "%d: fsi - (%d) in state %s", state->id,
> service->localport, srvstate_names[service->srvstate]);
> return;
> }
> @@ -3628,26 +3620,6 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
> return err;
> }
>
> -void
> -vchiq_loud_error_header(void)
> -{
> - vchiq_log_error(vchiq_core_log_level,
> - "============================================================================");
> - vchiq_log_error(vchiq_core_log_level,
> - "============================================================================");
> - vchiq_log_error(vchiq_core_log_level, "=====");
> -}
> -
> -void
> -vchiq_loud_error_footer(void)
> -{
> - vchiq_log_error(vchiq_core_log_level, "=====");
> - vchiq_log_error(vchiq_core_log_level,
> - "============================================================================");
> - vchiq_log_error(vchiq_core_log_level,
> - "============================================================================");
> -}
> -
> int vchiq_send_remote_use(struct vchiq_state *state)
> {
> if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
> 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 ec1a3caefaea..c74c3647ef60 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -6,6 +6,7 @@
>
> #include <linux/mutex.h>
> #include <linux/completion.h>
> +#include <linux/dev_printk.h>
> #include <linux/kthread.h>
> #include <linux/kref.h>
> #include <linux/rcupdate.h>
> @@ -39,10 +40,33 @@
>
> #define VCHIQ_LOG_PREFIX KERN_INFO "vchiq: "
>
> +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_error
> -#define vchiq_log_error(cat, fmt, ...) \
> - do { if (cat >= VCHIQ_LOG_ERROR) \
> - printk(VCHIQ_LOG_PREFIX fmt "\n", ##__VA_ARGS__); } while (0)
> +#define vchiq_log_error(dev, cat, fmt, ...) \
> + do { if (dev != NULL) \
> + dev_dbg(dev, "%s error: " fmt, log_category_str(cat), ##__VA_ARGS__); \
> + else \
> + pr_debug("%s error: " fmt, log_category_str(cat), ##__VA_ARGS__); } while (0)
> #endif
> #ifndef vchiq_log_warning
> #define vchiq_log_warning(cat, fmt, ...) \
> @@ -60,9 +84,6 @@
> printk(VCHIQ_LOG_PREFIX fmt "\n", ##__VA_ARGS__); } while (0)
> #endif
>
> -#define vchiq_loud_error(...) \
> - vchiq_log_error(vchiq_core_log_level, "===== " __VA_ARGS__)
> -
> #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 841e1a535642..c89dc6d82fd2 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> @@ -271,7 +271,7 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
> ret = -EFAULT;
> }
> } else {
> - vchiq_log_error(vchiq_arm_log_level,
> + vchiq_log_error(service->state->dev, VCHIQ_ARM,
> "header %pK: bufsize %x < size %x",
> header, args->bufsize, header->size);
> WARN(1, "invalid size\n");
> @@ -318,7 +318,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
> }
> mutex_unlock(&instance->bulk_waiter_list_mutex);
> if (!waiter) {
> - vchiq_log_error(vchiq_arm_log_level,
> + vchiq_log_error(service->state->dev, VCHIQ_ARM,
> "no bulk_waiter found for pid %d", current->pid);
> ret = -ESRCH;
> goto out;
> @@ -501,7 +501,7 @@ 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(vchiq_arm_log_level,
> + vchiq_log_error(service->state->dev, VCHIQ_ARM,
> "header %pK: msgbufsize %x < msglen %x",
> header, args->msgbufsize, msglen);
> WARN(1, "invalid message size\n");
> @@ -618,7 +618,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> }
> rc = mutex_lock_killable(&instance->state->mutex);
> if (rc) {
> - vchiq_log_error(vchiq_arm_log_level,
> + vchiq_log_error(instance->state->dev, VCHIQ_ARM,
> "vchiq: connect: could not lock mutex for state %d: %d",
> instance->state->id, rc);
> ret = -EINTR;
> @@ -630,7 +630,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> if (!status)
> instance->connected = 1;
> else
> - vchiq_log_error(vchiq_arm_log_level,
> + vchiq_log_error(instance->state->dev, VCHIQ_ARM,
> "vchiq: could not connect: %d", status);
> break;
>
> @@ -700,7 +700,7 @@ 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(vchiq_susp_log_level,
> + vchiq_log_error(service->state->dev, VCHIQ_SUSPEND,
> "%s: cmd %s returned error %ld for service %c%c%c%c:%03d",
> __func__, (cmd == VCHIQ_IOC_USE_SERVICE) ?
> "VCHIQ_IOC_USE_SERVICE" :
> @@ -1173,7 +1173,7 @@ static int vchiq_open(struct inode *inode, struct file *file)
> vchiq_log_info(vchiq_arm_log_level, "vchiq_open");
>
> if (!state) {
> - vchiq_log_error(vchiq_arm_log_level,
> + vchiq_log_error(state->dev, VCHIQ_ARM,
> "vchiq has no connection to VideoCore");
> return -ENOTCONN;
> }
> --
> 2.40.1
>