Re: [PATCH v5 02/12] media: staging/imx7: add imx7 CSI subdev driver

[Philipp Zabel <p.zabel@pengutronix.de>]

On Fri, 2018-05-18 at 10:27 +0100, Rui Miguel Silva wrote:
> This add the media entity subdevice and control driver for the i.MX7
> CMOS Sensor Interface.
> 
> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> ---
[...]
> +static int imx7_csi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct imx7_csi *csi;
> +	struct resource *res;
> +	int ret;
> +
> +	csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);
> +	if (!csi)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, &csi->sd);
> +	csi->dev = dev;
> +
> +	ret = imx7_csi_parse_dt(csi);
> +	if (ret < 0)
> +		return -ENODEV;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	csi->irq = platform_get_irq(pdev, 0);
> +	if (!res || csi->irq < 0) {
> +		dev_err(dev, "Missing platform resources data\n");
> +		return -ENODEV;
> +	}
> +
> +	csi->regbase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(csi->regbase)) {
> +		dev_err(dev, "Failed platform resources map\n");
> +		return -ENODEV;
> +	}
> +
> +	spin_lock_init(&csi->irqlock);
> +	mutex_init(&csi->lock);
> +
> +	/* install interrupt handler */
> +	ret = devm_request_irq(dev, csi->irq, imx7_csi_irq_handler, 0, "csi",
> +			       (void *)csi);
> +	if (ret < 0) {
> +		dev_err(dev, "Request CSI IRQ failed.\n");
> +		return -ENODEV;
> +	}
> +
> +	/* add media device */
> +	csi->imxmd = imx_media_dev_init(dev, false);
> +	if (IS_ERR(csi->imxmd))
> +		return PTR_ERR(csi->imxmd);
> +
> +	ret = imx_media_of_add_csi(csi->imxmd, node);
> +	if (ret < 0)
> +		goto media_cleanup;
> +
> +	ret = imx_media_dev_notifier_register(csi->imxmd);
> +	if (ret < 0)
> +		goto media_cleanup;
> +
> +	v4l2_subdev_init(&csi->sd, &imx7_csi_subdev_ops);
> +	v4l2_set_subdevdata(&csi->sd, csi);
> +	csi->sd.internal_ops = &imx7_csi_internal_ops;
> +	csi->sd.entity.ops = &imx7_csi_entity_ops;
> +	csi->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	csi->sd.dev = &pdev->dev;
> +	csi->sd.owner = THIS_MODULE;
> +	csi->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	csi->sd.grp_id = IMX_MEDIA_GRP_ID_CSI0;

See my comments on the first patch. Since grp_id specifies an IPU CSI0,
it would be better to use a different id here to make clear this is a
standalone CSI as opposed to an IPU CSI.

regards
Philipp