Hi,
On Thu 19 Apr 2018 at 12:22, Dan Carpenter wrote:
On Thu, Apr 19, 2018 at 11:17:59AM +0100, Rui Miguel Silva
wrote:
+static int imx7_csi_link_setup(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32
flags)
+{
+ struct v4l2_subdev *sd =
media_entity_to_v4l2_subdev(entity);
+ struct imx7_csi *csi = v4l2_get_subdevdata(sd);
+ struct v4l2_subdev *remote_sd;
+ int ret = 0;
+
+ dev_dbg(csi->dev, "link setup %s -> %s\n",
remote->entity->name,
+ local->entity->name);
+
+ mutex_lock(&csi->lock);
+
+ if (local->flags & MEDIA_PAD_FL_SINK) {
+ if (!is_media_entity_v4l2_subdev(remote->entity))
{
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ remote_sd =
media_entity_to_v4l2_subdev(remote->entity);
+
+ if (flags & MEDIA_LNK_FL_ENABLED) {
+ if (csi->src_sd) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+ csi->src_sd = remote_sd;
+ } else {
+ csi->src_sd = NULL;
+ }
+
+ goto init;
+ }
+
+ /* source pad */
+ if (flags & MEDIA_LNK_FL_ENABLED) {
+ if (csi->sink) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+ csi->sink = remote->entity;
+ } else {
+ v4l2_ctrl_handler_free(&csi->ctrl_hdlr);
+ v4l2_ctrl_handler_init(&csi->ctrl_hdlr, 0);
+ csi->sink = NULL;
+ }
+
+init:
+ if (csi->sink || csi->src_sd)
+ imx7_csi_init(csi);
+ else
+ imx7_csi_deinit(csi);
+
+unlock:
+ mutex_unlock(&csi->lock);
+
+ return 0;
This should be "return ret;" because the failure paths go
through here
as well.
Agree.
+}
+
+static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
+ struct media_link *link,
+ struct v4l2_subdev_format
*source_fmt,
+ struct v4l2_subdev_format
*sink_fmt)
+{
+ struct imx7_csi *csi = v4l2_get_subdevdata(sd);
+ struct v4l2_fwnode_endpoint upstream_ep;
+ int ret;
+
+ ret = v4l2_subdev_link_validate_default(sd, link,
source_fmt, sink_fmt);
+ if (ret)
+ return ret;
+
+ ret = imx7_csi_get_upstream_endpoint(csi, &upstream_ep,
true);
+ if (ret) {
+ v4l2_err(&csi->sd, "failed to find upstream
endpoint\n");
+ return ret;
+ }
+
+ mutex_lock(&csi->lock);
+
+ csi->upstream_ep = upstream_ep;
+ csi->is_csi2 = (upstream_ep.bus_type == V4L2_MBUS_CSI2);
+
+ mutex_unlock(&csi->lock);
+
+ return ret;
return 0;
ack.
+}
+
[ snip ]
+
+static int imx7_csi_remove(struct platform_device *pdev)
+{
+ return 0;
+}
There is no need for this empty (struct
platform_driver)->remove()
function. See platform_drv_remove() for how it's called.
right.
This looks nice, though.
Thanks,
---
Cheers,
Rui
