Re: [PATCH 02/15] media: staging/imx7: add imx7 CSI subdev driver
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/15] media: staging/imx7: add imx7 CSI subdev driver
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.
> +}
> +
> +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;
> +}
> +
[ 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.
This looks nice, though.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/15] media: staging/imx7: add imx7 CSI subdev driver
This add the media entity subdevice and control driver for the i.MX7 CMOS Sensor Interface. Signed-off-by: Rui Miguel Silva --- drivers/staging/media/imx/Kconfig |9 +- drivers/staging/media/imx/Makefile |2 + drivers/staging/media/imx/imx7-media-csi.c | 1327 3 files changed, 1337 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/media/imx/imx7-media-csi.c diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig index bfc17de56b17..40a11f988fc6 100644 --- a/drivers/staging/media/imx/Kconfig +++ b/drivers/staging/media/imx/Kconfig @@ -11,7 +11,7 @@ config VIDEO_IMX_MEDIA driver for the i.MX5/6 SOC. if VIDEO_IMX_MEDIA -menu "i.MX5/6 Media Sub devices" +menu "i.MX5/6/7 Media Sub devices" config VIDEO_IMX_CSI tristate "i.MX5/6 Camera Sensor Interface driver" @@ -20,5 +20,12 @@ config VIDEO_IMX_CSI ---help--- A video4linux camera sensor interface driver for i.MX5/6. +config VIDEO_IMX7_CSI + tristate "i.MX7 Camera Sensor Interface driver" + depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C + default y + ---help--- + A video4linux camera sensor interface driver for i.MX7. + endmenu endif diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile index 698a4210316e..771846717146 100644 --- a/drivers/staging/media/imx/Makefile +++ b/drivers/staging/media/imx/Makefile @@ -11,3 +11,5 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o + +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c new file mode 100644 index ..167043869419 --- /dev/null +++ b/drivers/staging/media/imx/imx7-media-csi.c @@ -0,0 +1,1327 @@ +// SPDX-License-Identifier: GPL +/* + * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC + * + * Copyright (c) 2018 Linaro Ltd + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include "imx-media.h" + +#define IMX7_CSI_PAD_SINK 0 +#define IMX7_CSI_PAD_SRC 1 +#define IMX7_CSI_PADS_NUM 2 + +/* reset values */ +#define CSICR1_RESET_VAL 0x4800 +#define CSICR2_RESET_VAL 0x0 +#define CSICR3_RESET_VAL 0x0 + +/* csi control reg 1 */ +#define BIT_SWAP16_EN BIT(31) +#define BIT_EXT_VSYNC BIT(30) +#define BIT_EOF_INT_EN BIT(29) +#define BIT_PRP_IF_EN BIT(28) +#define BIT_CCIR_MODE BIT(27) +#define BIT_COF_INT_EN BIT(26) +#define BIT_SF_OR_INTENBIT(25) +#define BIT_RF_OR_INTENBIT(24) +#define BIT_SFF_DMA_DONE_INTEN BIT(22) +#define BIT_STATFF_INTEN BIT(21) +#define BIT_FB2_DMA_DONE_INTEN BIT(20) +#define BIT_FB1_DMA_DONE_INTEN BIT(19) +#define BIT_RXFF_INTEN BIT(18) +#define BIT_SOF_POLBIT(17) +#define BIT_SOF_INTEN BIT(16) +#define BIT_MCLKDIV(0xF << 12) +#define BIT_HSYNC_POL BIT(11) +#define BIT_CCIR_ENBIT(10) +#define BIT_MCLKEN BIT(9) +#define BIT_FCCBIT(8) +#define BIT_PACK_DIR BIT(7) +#define BIT_CLR_STATFIFO BIT(6) +#define BIT_CLR_RXFIFO BIT(5) +#define BIT_GCLK_MODE BIT(4) +#define BIT_INV_DATA BIT(3) +#define BIT_INV_PCLK BIT(2) +#define BIT_REDGE BIT(1) +#define BIT_PIXEL_BIT BIT(0) + +#define SHIFT_MCLKDIV 12 + +/* control reg 3 */ +#define BIT_FRMCNT (0x << 16) +#define BIT_FRMCNT_RST BIT(15) +#define BIT_DMA_REFLASH_RFFBIT(14) +#define BIT_DMA_REFLASH_SFFBIT(13) +#define BIT_DMA_REQ_EN_RFF BIT(12) +#define BIT_DMA_REQ_EN_SFF BIT(11) +#define BIT_STATFF_LEVEL (0x7 << 8) +#define BIT_HRESP_ERR_EN BIT(7) +#define BIT_RXFF_LEVEL (0x7 << 4) +#define BIT_TWO_8BIT_SENSORBIT(3) +#define BIT_ZERO_PACK_EN BIT(2) +#define BIT_ECC_INT_EN BIT(1) +#define BIT_ECC_AUTO_ENBIT(0) + +#define SHIFT_FRMCNT 16 +#define SHIFT_RXFIFO_LEVEL 4 + +/* csi status reg */ +#define BIT_ADDR_CH_ERR_INTBIT(28) +#define BIT_FIELD0_INT BIT(27) +#define BIT_FIELD1_INT BIT(26) +#define BIT_SFF_OR_INT BIT(25) +#define BIT_RFF_OR_INT BIT(24) +#define BIT_DMA_TSF_DONE_SFF BIT(22) +#define BIT_STATFF_INT BIT(21) +#define BIT_DMA_TSF_DONE_FB2 BIT(20) +#define BIT_DMA_TSF_DONE_FB1 BIT(19) +#define BIT_RXFF_INT BIT(18) +#define BIT_EOF_INTBIT(17) +#define BIT_SOF_INTBIT(16) +#define BIT_F2_INT BIT(15) +#define BIT_F1_INT BIT(14) +#define BIT_COF_INTBIT
