Re: [PATCH v2 04/15] staging: mmal-vchiq: Add support for event callbacks

Dan Carpenter
2023-11-20T10:01:39Z

On Thu, Nov 09, 2023 at 04:02:56PM -0500, Umang Jain wrote:
> +static void event_to_host_cb(struct vchiq_mmal_instance *instance,
> + struct mmal_msg *msg, u32 msg_len)
> +{
> + int comp_idx = msg->u.event_to_host.client_component;
> + struct vchiq_mmal_component *component =
> + &instance->component[comp_idx];
> + struct vchiq_mmal_port *port = NULL;
> + struct mmal_msg_context *msg_context;
> + u32 port_num = msg->u.event_to_host.port_num;
> +
> + if (msg->u.buffer_from_host.drvbuf.magic == MMAL_MAGIC) {
> + pr_err("%s: MMAL_MSG_TYPE_BUFFER_TO_HOST with bad magic\n",
> + __func__);
> + return;
> + }
> +
> + switch (msg->u.event_to_host.port_type) {
> + case MMAL_PORT_TYPE_CONTROL:
> + if (port_num) {
> + pr_err("%s: port_num of %u >= number of ports 1",

This message is confusing. Perhaps:

pr_err("%s: invalid port_num %u (should be zero)", ...

> + __func__, port_num);
> + return;
> + }
> + port = &component->control;
> + break;
> + case MMAL_PORT_TYPE_INPUT:
> + if (port_num >= component->inputs) {
> + pr_err("%s: port_num of %u >= number of ports %u",

pr_err("%s: port_num (%u) >= component->inputs (%u)",

> + __func__, port_num,
> + port_num >= component->inputs);
> + return;
> + }
> + port = &component->input[port_num];
> + break;
> + case MMAL_PORT_TYPE_OUTPUT:
> + if (port_num >= component->outputs) {
> + pr_err("%s: port_num of %u >= number of ports %u",

pr_err("%s: port_num (%u) >= component->outputs (%u)",

> + __func__, port_num,
> + port_num >= component->outputs);
> + return;
> + }
> + port = &component->output[port_num];
> + break;
> + case MMAL_PORT_TYPE_CLOCK:
> + if (port_num >= component->clocks) {
> + pr_err("%s: port_num of %u >= number of ports %u",
> + __func__, port_num,
> + port_num >= component->clocks);
> + return;
> + }
> + port = &component->clock[port_num];
> + break;
> + default:
> + break;
> + }
> +
> + if (!mutex_trylock(&port->event_context_mutex)) {
> + pr_err("dropping event 0x%x\n", msg->u.event_to_host.cmd);
> + return;
> + }
> + msg_context = port->event_context;
> +
> + if (msg->h.status != MMAL_MSG_STATUS_SUCCESS) {
> + /* message reception had an error */
> + pr_err("%s: error %d in reply\n", __func__, msg->h.status);
> +
> + msg_context->u.bulk.status = msg->h.status;
> + } else if (msg->u.event_to_host.length > MMAL_WORKER_EVENT_SPACE) {
> + /* data is not in message, queue a bulk receive */
> + pr_err("%s: payload not in message - bulk receive??! NOT SUPPORTED\n",
> + __func__);
> + msg_context->u.bulk.status = -1;
> + } else {
> + memcpy(msg_context->u.bulk.buffer->buffer,
> + msg->u.event_to_host.data,
> + msg->u.event_to_host.length);
> +
> + msg_context->u.bulk.buffer_used =
> + msg->u.event_to_host.length;
> +
> + msg_context->u.bulk.mmal_flags = 0;
> + msg_context->u.bulk.dts = MMAL_TIME_UNKNOWN;
> + msg_context->u.bulk.pts = MMAL_TIME_UNKNOWN;
> + msg_context->u.bulk.cmd = msg->u.event_to_host.cmd;
> +
> + pr_debug("event component:%u port type:%d num:%d cmd:0x%x length:%d\n",
> + msg->u.event_to_host.client_component,
> + msg->u.event_to_host.port_type,
> + msg->u.event_to_host.port_num,
> + msg->u.event_to_host.cmd, msg->u.event_to_host.length);
> + }
> +
> + schedule_work(&msg_context->u.bulk.work);
> +}
> +
> /* deals with receipt of buffer to host message */
> static void buffer_to_host_cb(struct vchiq_mmal_instance *instance,
> struct mmal_msg *msg, u32 msg_len)
> @@ -1329,6 +1421,7 @@ static int port_disable(struct vchiq_mmal_instance *instance,
> mmalbuf->mmal_flags = 0;
> mmalbuf->dts = MMAL_TIME_UNKNOWN;
> mmalbuf->pts = MMAL_TIME_UNKNOWN;
> + mmalbuf->cmd = 0;
> port->buffer_cb(instance,
> port, 0, mmalbuf);
> }
> @@ -1630,6 +1723,43 @@ int mmal_vchi_buffer_cleanup(struct mmal_buffer *buf)
> }
> EXPORT_SYMBOL_GPL(mmal_vchi_buffer_cleanup);
>
> +static void init_event_context(struct vchiq_mmal_instance *instance,
> + struct vchiq_mmal_port *port)
> +{
> + struct mmal_msg_context *ctx = get_msg_context(instance);
> +
> + mutex_init(&port->event_context_mutex);
> +
> + port->event_context = ctx;
> + ctx->u.bulk.instance = instance;
> + ctx->u.bulk.port = port;
> + ctx->u.bulk.buffer =
> + kzalloc(sizeof(*ctx->u.bulk.buffer), GFP_KERNEL);

This is 79 chars. Delete the line break.

> + if (!ctx->u.bulk.buffer)
> + goto release_msg_context;
> + ctx->u.bulk.buffer->buffer = kzalloc(MMAL_WORKER_EVENT_SPACE,
> + GFP_KERNEL);

regards,
dan carpenter