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

[Dan Carpenter]

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