Re: [PATCH v2 15/19] media: imx: Add MIPI CSI-2 Receiver subdev driver

2017-01-06 Thread Steve Longerbeam



On 01/04/2017 07:05 AM, Vladimir Zapolskiy wrote:

On 01/03/2017 10:57 PM, Steve Longerbeam wrote:

Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
for sensors with a MIPI CSI2 interface.

Signed-off-by: Steve Longerbeam 
---
  drivers/staging/media/imx/Makefile|   1 +
  drivers/staging/media/imx/imx-mipi-csi2.c | 509 ++
  2 files changed, 510 insertions(+)
  create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c

diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index fe9e992..0decef7 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
+obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
b/drivers/staging/media/imx/imx-mipi-csi2.c
new file mode 100644
index 000..84df16e
--- /dev/null
+++ b/drivers/staging/media/imx/imx-mipi-csi2.c
@@ -0,0 +1,509 @@
+/*
+ * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
+ *
+ * Copyright (c) 2012-2014 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

Please sort the list of headers alphabetically.


done.




+#include 

Why do you need to include this header?


good question. In fact this include list was in need of a lot of pruning,
so I paired it down to the essentials.


+static int imxcsi2_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+   int i, ret = 0;
+
+   if (!csi2->src_sd)
+   return -EPIPE;
+   for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
+   if (csi2->sink_sd[i])
+   break;
+   }
+   if (i >= CSI2_NUM_SRC_PADS)
+   return -EPIPE;
+
+   v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
+
+   if (enable && !csi2->stream_on) {
+   clk_prepare_enable(csi2->pix_clk);

It can complicate the design for you, but in general clk_prepare_enable()
can return an error.


I added an error check and reorganized a little.


+
+static int imxcsi2_parse_endpoints(struct imxcsi2_dev *csi2)
+{
+   struct device_node *node = csi2->dev->of_node;
+   struct device_node *epnode;
+   struct v4l2_of_endpoint ep;
+   int ret = 0;
+
+   epnode = of_graph_get_next_endpoint(node, NULL);
+   if (!epnode) {
+   v4l2_err(>sd, "failed to get endpoint node\n");
+   return -EINVAL;
+   }
+
+   v4l2_of_parse_endpoint(epnode, );

Do of_node_put(epnode) here and remove 'out' goto label.


done.


+static const struct of_device_id imxcsi2_dt_ids[] = {
+   { .compatible = "fsl,imx-mipi-csi2", },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imxcsi2_dt_ids);
+
+static struct platform_driver imxcsi2_driver = {
+   .driver = {
+   .name = DEVICE_NAME,
+   .owner = THIS_MODULE,

Please drop .owner assignment.


done.


Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 15/19] media: imx: Add MIPI CSI-2 Receiver subdev driver

2017-01-04 Thread Vladimir Zapolskiy
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile|   1 +
>  drivers/staging/media/imx/imx-mipi-csi2.c | 509 
> ++
>  2 files changed, 510 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index fe9e992..0decef7 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
> b/drivers/staging/media/imx/imx-mipi-csi2.c
> new file mode 100644
> index 000..84df16e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
> @@ -0,0 +1,509 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
> + *
> + * Copyright (c) 2012-2014 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please sort the list of headers alphabetically.

> +#include 

Why do you need to include this header?

> +#include 
> +#include "imx-media.h"
> +

[snip]

> +static int imxcsi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> + int i, ret = 0;
> +
> + if (!csi2->src_sd)
> + return -EPIPE;
> + for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
> + if (csi2->sink_sd[i])
> + break;
> + }
> + if (i >= CSI2_NUM_SRC_PADS)
> + return -EPIPE;
> +
> + v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
> +
> + if (enable && !csi2->stream_on) {
> + clk_prepare_enable(csi2->pix_clk);

It can complicate the design for you, but in general clk_prepare_enable()
can return an error.

> + ret = imxcsi2_dphy_wait(csi2);
> + if (ret)
> + clk_disable_unprepare(csi2->pix_clk);
> + } else if (!enable && csi2->stream_on) {
> + clk_disable_unprepare(csi2->pix_clk);
> + }
> +
> + if (!ret)
> + csi2->stream_on = enable;
> + return ret;
> +}
> +

[snip]

> +
> +static int imxcsi2_parse_endpoints(struct imxcsi2_dev *csi2)
> +{
> + struct device_node *node = csi2->dev->of_node;
> + struct device_node *epnode;
> + struct v4l2_of_endpoint ep;
> + int ret = 0;
> +
> + epnode = of_graph_get_next_endpoint(node, NULL);
> + if (!epnode) {
> + v4l2_err(>sd, "failed to get endpoint node\n");
> + return -EINVAL;
> + }
> +
> + v4l2_of_parse_endpoint(epnode, );

Do of_node_put(epnode) here and remove 'out' goto label.

> + if (ep.bus_type != V4L2_MBUS_CSI2) {
> + v4l2_err(>sd, "invalid bus type, must be MIPI CSI2\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + csi2->bus = ep.bus.mipi_csi2;
> +
> + v4l2_info(>sd, "data lanes: %d\n", csi2->bus.num_data_lanes);
> + v4l2_info(>sd, "flags: 0x%08x\n", csi2->bus.flags);
> +out:
> + of_node_put(epnode);
> + return ret;
> +}
> +

[snip]

> +static const struct of_device_id imxcsi2_dt_ids[] = {
> + { .compatible = "fsl,imx-mipi-csi2", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imxcsi2_dt_ids);
> +
> +static struct platform_driver imxcsi2_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .owner = THIS_MODULE,

Please drop .owner assignment.

> + .of_match_table = imxcsi2_dt_ids,
> + },
> + .probe = imxcsi2_probe,
> + .remove = imxcsi2_remove,
> +};
> +
> +module_platform_driver(imxcsi2_driver);
> +
> +MODULE_DESCRIPTION("i.MX5/6 MIPI CSI-2 Receiver driver");
> +MODULE_AUTHOR("Steve Longerbeam ");
> +MODULE_LICENSE("GPL");
> +
> 

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 15/19] media: imx: Add MIPI CSI-2 Receiver subdev driver

2017-01-03 Thread Steve Longerbeam
Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
for sensors with a MIPI CSI2 interface.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/Makefile|   1 +
 drivers/staging/media/imx/imx-mipi-csi2.c | 509 ++
 2 files changed, 510 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c

diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index fe9e992..0decef7 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
+obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
b/drivers/staging/media/imx/imx-mipi-csi2.c
new file mode 100644
index 000..84df16e
--- /dev/null
+++ b/drivers/staging/media/imx/imx-mipi-csi2.c
@@ -0,0 +1,509 @@
+/*
+ * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
+ *
+ * Copyright (c) 2012-2014 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "imx-media.h"
+
+/*
+ * there must be 5 pads: 1 input pad from sensor, and
+ * the 4 virtual channel output pads
+ */
+#define CSI2_NUM_SINK_PADS  1
+#define CSI2_NUM_SRC_PADS   4
+#define CSI2_NUM_PADS   5
+
+struct imxcsi2_dev {
+   struct device  *dev;
+   struct imx_media_dev   *md;
+   struct v4l2_subdev  sd;
+   struct media_pad   pad[CSI2_NUM_PADS];
+   struct v4l2_mbus_framefmt format_mbus;
+   struct v4l2_subdev *src_sd;
+   struct v4l2_subdev *sink_sd[CSI2_NUM_SRC_PADS];
+   intinput_pad;
+   struct clk *dphy_clk;
+   struct clk *cfg_clk;
+   struct clk *pix_clk; /* what is this? */
+   void __iomem   *base;
+   int intr1;
+   int intr2;
+   struct v4l2_of_bus_mipi_csi2 bus;
+   boolon;
+   boolstream_on;
+};
+
+#define DEVICE_NAME "imx-mipi-csi2"
+
+/* Register offsets */
+#define CSI2_VERSION0x000
+#define CSI2_N_LANES0x004
+#define CSI2_PHY_SHUTDOWNZ  0x008
+#define CSI2_DPHY_RSTZ  0x00c
+#define CSI2_RESETN 0x010
+#define CSI2_PHY_STATE  0x014
+#define CSI2_DATA_IDS_1 0x018
+#define CSI2_DATA_IDS_2 0x01c
+#define CSI2_ERR1   0x020
+#define CSI2_ERR2   0x024
+#define CSI2_MSK1   0x028
+#define CSI2_MSK2   0x02c
+#define CSI2_PHY_TST_CTRL0  0x030
+#define CSI2_PHY_TST_CTRL1  0x034
+#define CSI2_SFT_RESET  0xf00
+
+static inline struct imxcsi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
+{
+   return container_of(sdev, struct imxcsi2_dev, sd);
+}
+
+static inline u32 imxcsi2_read(struct imxcsi2_dev *csi2, unsigned int regoff)
+{
+   return readl(csi2->base + regoff);
+}
+
+static inline void imxcsi2_write(struct imxcsi2_dev *csi2, u32 val,
+unsigned int regoff)
+{
+   writel(val, csi2->base + regoff);
+}
+
+static void imxcsi2_set_lanes(struct imxcsi2_dev *csi2)
+{
+   int lanes = csi2->bus.num_data_lanes;
+
+   imxcsi2_write(csi2, lanes - 1, CSI2_N_LANES);
+}
+
+static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
+{
+   if (enable) {
+   imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
+   imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
+   imxcsi2_write(csi2, 0x, CSI2_RESETN);
+   } else {
+   imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
+   imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
+   imxcsi2_write(csi2, 0x0, CSI2_RESETN);
+   }
+}
+
+static void imxcsi2_reset(struct imxcsi2_dev *csi2)
+{
+   imxcsi2_enable(csi2, false);
+
+   imxcsi2_write(csi2, 0x0001, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x00010044, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x0014, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
+