Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580926 > unrolled thread

[PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-14 23:40 +0100
    [PATCH 4/4] v4l: split lane parsing code Pavel Machek <pavel@ucw.cz> - 2017-02-14 23:40 +0100
    [PATCH 2/4] Core changes for CCP2/CSI1 support. Pavel Machek <pavel@ucw.cz> - 2017-02-14 23:40 +0100
    [PATCH 3/4] smiapp: add CCP2 support Pavel Machek <pavel@ucw.cz> - 2017-02-14 23:40 +0100
    Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-20 11:40 +0100
      Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-20 14:20 +0100
        Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-20 15:00 +0100
          Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-21 12:10 +0100
            Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-21 12:20 +0100
              Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-24 00:00 +0100
              Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-25 01:10 +0100
                Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-25 14:50 +0100
                  camera subdevice support was Re: [PATCH 1/4] v4l2:  device_register_subdev_nodes: allow calling multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-25 23:00 +0100
                    Re: camera subdevice support was Re: [PATCH 1/4] v4l2:  device_register_subdev_nodes: allow calling multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-26 00:10 +0100
                    Re: camera subdevice support was Re: [PATCH 1/4] v4l2:  device_register_subdev_nodes: allow calling multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-26 00:20 +0100
                      Re: camera subdevice support was Re: [PATCH 1/4] v4l2:  device_register_subdev_nodes: allow calling multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-26 09:40 +0100
                        Re: camera subdevice support was Re: [PATCH 1/4] v4l2:  device_register_subdev_nodes: allow calling multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-26 22:40 +0100
              Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-25 23:20 +0100
                Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-27 21:10 +0100
                Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-27 22:00 +0100
                  Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling  multiple times Pavel Machek <pavel@ucw.cz> - 2017-02-28 10:20 +0100
                  [PATCH] omap3isp: Parse CSI1 configuration from the device tree. Pavel Machek <pavel@ucw.cz> - 2017-02-28 12:40 +0100

Page 1 of 2  [1] 2  Next page →

#1580926 — [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

[Multipart message — attachments visible in raw view] — view raw

From: Sebastian Reichel <sre@kernel.org>

If v4l2_device_register_subdev_nodes() is called multiple times, it is
better to return early than corrupt memory.

Without this, exposure / gain controls do not work in the camera
application on N900.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/media/v4l2-core/v4l2-device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index f364cc1..937c6de 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
 		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
 			continue;
 
+		if (sd->devnode)
+			continue;
+
 		vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
 		if (!vdev) {
 			err = -ENOMEM;
-- 
2.1.4


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [next] | [standalone]

#1580927 — [PATCH 4/4] v4l: split lane parsing code

[Multipart message — attachments visible in raw view] — view raw

From: Sakari Ailus <sakari.ailus@iki.fi>

The function to parse CSI2 bus parameters was called
v4l2_of_parse_csi_bus(), rename it as v4l2_of_parse_csi2_bus() in
anticipation of CSI1/CCP2 support.

Obtain data bus type from bus-type property. Only try parsing bus
specific properties in this case.

Separate lane parsing from CSI-2 bus parameter parsing. The CSI-1 will
need these as well, separate them into a different
function. have_clk_lane and num_data_lanes arguments may be NULL; the
CSI-1 bus will have no use for them.

Add support for parsing of CSI-1 and CCP2 bus related properties
documented in video-interfaces.txt.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/media/v4l2-core/v4l2-of.c | 138 ++++++++++++++++++++++++++++++--------
 1 file changed, 111 insertions(+), 27 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index 4f59f44..85629de 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -20,64 +20,103 @@
 
 #include <media/v4l2-of.h>
 
-static int v4l2_of_parse_csi_bus(const struct device_node *node,
-				 struct v4l2_of_endpoint *endpoint)
+/* This has to match values in the device tree. */
+
+enum v4l2_of_bus_type {
+	V4L2_OF_BUS_TYPE_GUESS = 0, /* CSI-2 D-PHY, parallel or Bt.656 */
+	V4L2_OF_BUS_TYPE_CSI2_CPHY,
+	V4L2_OF_BUS_TYPE_CSI1,
+	V4L2_OF_BUS_TYPE_CCP2,
+};
+
+static int v4l2_of_parse_lanes(const struct device_node *node,
+			       unsigned char *clock_lane,
+			       bool *have_clk_lane,
+			       unsigned char *data_lanes,
+			       bool *lane_polarities,
+			       unsigned short *__num_data_lanes,
+			       unsigned int max_data_lanes)
 {
-	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
 	struct property *prop;
-	bool have_clk_lane = false;
-	unsigned int flags = 0, lanes_used = 0;
+	unsigned int lanes_used = 0;
+	short num_data_lanes = 0;
 	u32 v;
 
 	prop = of_find_property(node, "data-lanes", NULL);
 	if (prop) {
 		const __be32 *lane = NULL;
-		unsigned int i;
 
-		for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) {
+		for (num_data_lanes = 0; num_data_lanes < max_data_lanes;
+		     num_data_lanes++) {
 			lane = of_prop_next_u32(prop, lane, &v);
 			if (!lane)
 				break;
+			data_lanes[num_data_lanes] = v;
 
 			if (lanes_used & BIT(v))
 				pr_warn("%s: duplicated lane %u in data-lanes\n",
 					node->full_name, v);
 			lanes_used |= BIT(v);
-
-			bus->data_lanes[i] = v;
 		}
-		bus->num_data_lanes = i;
 	}
+	if (__num_data_lanes)
+		*__num_data_lanes = num_data_lanes;
 
 	prop = of_find_property(node, "lane-polarities", NULL);
 	if (prop) {
 		const __be32 *polarity = NULL;
 		unsigned int i;
 
-		for (i = 0; i < ARRAY_SIZE(bus->lane_polarities); i++) {
+		for (i = 0; i < 1 + max_data_lanes; i++) {
 			polarity = of_prop_next_u32(prop, polarity, &v);
 			if (!polarity)
 				break;
-			bus->lane_polarities[i] = v;
+			lane_polarities[i] = v;
 		}
 
-		if (i < 1 + bus->num_data_lanes /* clock + data */) {
+		if (i < 1 + num_data_lanes /* clock + data */) {
 			pr_warn("%s: too few lane-polarities entries (need %u, got %u)\n",
-				node->full_name, 1 + bus->num_data_lanes, i);
+				node->full_name, 1 + num_data_lanes, i);
 			return -EINVAL;
 		}
 	}
 
+	if (have_clk_lane)
+		*have_clk_lane = false;
+
 	if (!of_property_read_u32(node, "clock-lanes", &v)) {
+		*clock_lane = v;
+		if (have_clk_lane)
+			*have_clk_lane = true;
+
 		if (lanes_used & BIT(v))
 			pr_warn("%s: duplicated lane %u in clock-lanes\n",
 				node->full_name, v);
 		lanes_used |= BIT(v);
-
-		bus->clock_lane = v;
-		have_clk_lane = true;
 	}
 
+	return 0;
+}
+
+static int v4l2_of_parse_csi2_bus(const struct device_node *node,
+				 struct v4l2_of_endpoint *endpoint)
+{
+	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
+	bool have_clk_lane = false;
+	unsigned int flags = 0;
+	int rval;
+	u32 v;
+
+	rval = v4l2_of_parse_lanes(node, &bus->clock_lane, &have_clk_lane,
+				   bus->data_lanes, bus->lane_polarities,
+				   &bus->num_data_lanes,
+				   ARRAY_SIZE(bus->data_lanes));
+	if (rval)
+		return rval;
+
+	BUILD_BUG_ON(1 + ARRAY_SIZE(bus->data_lanes)
+		       != ARRAY_SIZE(bus->lane_polarities));
+
 	if (of_get_property(node, "clock-noncontinuous", &v))
 		flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
 	else if (have_clk_lane || bus->num_data_lanes > 0)
@@ -139,6 +178,35 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
 
 }
 
+void v4l2_of_parse_csi1_bus(const struct device_node *node,
+			    struct v4l2_of_endpoint *endpoint,
+			    enum v4l2_of_bus_type bus_type)
+{
+	struct v4l2_of_bus_mipi_csi1 *bus = &endpoint->bus.mipi_csi1;
+	u32 v;
+
+	v4l2_of_parse_lanes(node, &bus->clock_lane, NULL,
+			    &bus->data_lane, bus->lane_polarity,
+			    NULL, 1);
+
+	if (!of_property_read_u32(node, "clock-inv", &v))
+		bus->clock_inv = v;
+
+	if (!of_property_read_u32(node, "strobe", &v))
+		bus->strobe = v;
+
+	if (!of_property_read_u32(node, "data-lane", &v))
+		bus->data_lane = v;
+
+	if (!of_property_read_u32(node, "clock-lane", &v))
+		bus->clock_lane = v;
+
+	if (bus_type == V4L2_OF_BUS_TYPE_CSI1)
+		endpoint->bus_type = V4L2_MBUS_CSI1;
+	else
+		endpoint->bus_type = V4L2_MBUS_CCP2;
+}
+
 /**
  * v4l2_of_parse_endpoint() - parse all endpoint node properties
  * @node: pointer to endpoint device_node
@@ -162,6 +230,7 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
 int v4l2_of_parse_endpoint(const struct device_node *node,
 			   struct v4l2_of_endpoint *endpoint)
 {
+	u32 bus_type = 0;
 	int rval;
 
 	of_graph_parse_endpoint(node, &endpoint->base);
@@ -169,17 +238,32 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
 	memset(&endpoint->bus_type, 0, sizeof(*endpoint) -
 	       offsetof(typeof(*endpoint), bus_type));
 
-	rval = v4l2_of_parse_csi_bus(node, endpoint);
-	if (rval)
-		return rval;
-	/*
-	 * Parse the parallel video bus properties only if none
-	 * of the MIPI CSI-2 specific properties were found.
-	 */
-	if (endpoint->bus.mipi_csi2.flags == 0)
-		v4l2_of_parse_parallel_bus(node, endpoint);
+	rval = of_property_read_u32(node, "bus-type", &bus_type);
+	if (bus_type == 0) {
+		endpoint->bus_type = 0;
+		rval = v4l2_of_parse_csi2_bus(node, endpoint);
+		if (rval)
+			return rval;
+		/*
+		 * Parse the parallel video bus properties only if none
+		 * of the MIPI CSI-2 specific properties were found.
+		 */
+		if (endpoint->bus.mipi_csi2.flags == 0)
+			v4l2_of_parse_parallel_bus(node, endpoint);
+
+		return 0;
+	}
 
-	return 0;
+	switch (bus_type) {
+	case V4L2_OF_BUS_TYPE_CSI1:
+	case V4L2_OF_BUS_TYPE_CCP2:
+		v4l2_of_parse_csi1_bus(node, endpoint, bus_type);
+		return 0;
+	default:
+		pr_warn("bad bus-type %u, device_node \"%s\"\n",
+			bus_type, node->full_name);
+		return -EINVAL;
+	}
 }
 EXPORT_SYMBOL(v4l2_of_parse_endpoint);
 
-- 
2.1.4


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1580928 — [PATCH 2/4] Core changes for CCP2/CSI1 support.

[Multipart message — attachments visible in raw view] — view raw

From: Sakari Ailus <sakari.ailus@iki.fi>

CCP2, or CSI-1, is an older single data lane serial bus. Add core
support neccessary for it.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 include/media/v4l2-mediabus.h |  4 ++++
 include/media/v4l2-of.h       | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 34cc99e..315c167 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -69,11 +69,15 @@
  * @V4L2_MBUS_PARALLEL:	parallel interface with hsync and vsync
  * @V4L2_MBUS_BT656:	parallel interface with embedded synchronisation, can
  *			also be used for BT.1120
+ * @V4L2_MBUS_CSI1:	MIPI CSI-1 serial interface
+ * @V4L2_MBUS_CCP2:	CCP2 (Compact Camera Port 2)
  * @V4L2_MBUS_CSI2:	MIPI CSI-2 serial interface
  */
 enum v4l2_mbus_type {
 	V4L2_MBUS_PARALLEL,
 	V4L2_MBUS_BT656,
+	V4L2_MBUS_CSI1,
+	V4L2_MBUS_CCP2,
 	V4L2_MBUS_CSI2,
 };
 
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 4dc34b2..63a52ee 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -53,6 +53,22 @@ struct v4l2_of_bus_parallel {
 };
 
 /**
+ * struct v4l2_of_bus_csi1 - CSI-1/CCP2 data bus structure
+ * @clock_inv: polarity of clock/strobe signal
+ *	       false - not inverted, true - inverted
+ * @strobe: false - data/clock, true - data/strobe
+ * @data_lane: the number of the data lane
+ * @clock_lane: the number of the clock lane
+ */
+struct v4l2_of_bus_mipi_csi1 {
+	bool clock_inv;
+	bool strobe;
+	bool lane_polarity[2];
+	unsigned char data_lane;
+	unsigned char clock_lane;
+};
+
+/**
  * struct v4l2_of_endpoint - the endpoint data structure
  * @base: struct of_endpoint containing port, id, and local of_node
  * @bus_type: bus type
@@ -66,6 +82,7 @@ struct v4l2_of_endpoint {
 	enum v4l2_mbus_type bus_type;
 	union {
 		struct v4l2_of_bus_parallel parallel;
+		struct v4l2_of_bus_mipi_csi1 mipi_csi1;
 		struct v4l2_of_bus_mipi_csi2 mipi_csi2;
 	} bus;
 	u64 *link_frequencies;
-- 
2.1.4


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1580931 — [PATCH 3/4] smiapp: add CCP2 support

[Multipart message — attachments visible in raw view] — view raw

Add support for CCP2 connected SMIA sensors as found
on the Nokia N900.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index f4e92bd..212293f 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2807,13 +2807,19 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 	switch (bus_cfg->bus_type) {
 	case V4L2_MBUS_CSI2:
 		hwcfg->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
+		hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
+		break;
+	case V4L2_MBUS_CCP2:
+		hwcfg->csi_signalling_mode = (bus_cfg->bus.mipi_csi1.strobe) ?
+		SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_STROBE :
+		SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_CLOCK;
+		hwcfg->lanes = 1;
 		break;
-		/* FIXME: add CCP2 support. */
 	default:
+		dev_err(dev, "unknown bus protocol\n");
 		goto out_err;
 	}
 
-	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
 	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
 	/* NVM size is not mandatory */
@@ -2827,8 +2833,8 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 		goto out_err;
 	}
 
-	dev_dbg(dev, "nvm %d, clk %d, csi %d\n", hwcfg->nvm_size,
-		hwcfg->ext_clk, hwcfg->csi_signalling_mode);
+	dev_dbg(dev, "nvm %d, clk %d, mode %d\n",
+		hwcfg->nvm_size, hwcfg->ext_clk, hwcfg->csi_signalling_mode);
 
 	if (!bus_cfg->nr_of_link_frequencies) {
 		dev_warn(dev, "no link frequencies defined\n");
-- 
2.1.4


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1584496

[Multipart message — attachments visible in raw view] — view raw

Hi!

On Tue 2017-02-14 23:38:49, Pavel Machek wrote:
> From: Sebastian Reichel <sre@kernel.org>
> 
> If v4l2_device_register_subdev_nodes() is called multiple times, it is
> better to return early than corrupt memory.
> 
> Without this, exposure / gain controls do not work in the camera
> application on N900.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>

Can I get some updates/feedback here?

You liked this one and whole series should be ready...

Best regards,
									Pavel



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1584606

Hi Pavel,

On Mon, Feb 20, 2017 at 11:31:14AM +0100, Pavel Machek wrote:
> Hi!
> 
> On Tue 2017-02-14 23:38:49, Pavel Machek wrote:
> > From: Sebastian Reichel <sre@kernel.org>
> > 
> > If v4l2_device_register_subdev_nodes() is called multiple times, it is
> > better to return early than corrupt memory.
> > 
> > Without this, exposure / gain controls do not work in the camera
> > application on N900.
> > 
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> Can I get some updates/feedback here?
> 
> You liked this one and whole series should be ready...

:-)

I was just rebasing the CCP2 support on the fwnode patchset.

I'm just pushing the result here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2>

I've tested ACPI, will test DT soon...

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1584628

On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> I've tested ACPI, will test DT soon...

DT case works, too (Nokia N9).

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1585222

[Multipart message — attachments visible in raw view] — view raw

On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > I've tested ACPI, will test DT soon...
> 
> DT case works, too (Nokia N9).

Hmm. Good to know. Now to figure out how to get N900 case to work...

AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
seem to be in, so I'll need to figure out which, and will still need
omap3isp modifications...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1585232

On Tue, Feb 21, 2017 at 12:07:21PM +0100, Pavel Machek wrote:
> On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > I've tested ACPI, will test DT soon...
> > 
> > DT case works, too (Nokia N9).
> 
> Hmm. Good to know. Now to figure out how to get N900 case to work...
> 
> AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> seem to be in, so I'll need to figure out which, and will still need
> omap3isp modifications...

Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.

It's essentially the functionality in the four patches. The data-lane and
clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
plural) to match the property documentation.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1587156

[Multipart message — attachments visible in raw view] — view raw

Hi!

On Tue 2017-02-21 13:11:04, Sakari Ailus wrote:
> On Tue, Feb 21, 2017 at 12:07:21PM +0100, Pavel Machek wrote:
> > On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > > I've tested ACPI, will test DT soon...
> > > 
> > > DT case works, too (Nokia N9).
> > 
> > Hmm. Good to know. Now to figure out how to get N900 case to work...
> > 
> > AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> > seem to be in, so I'll need to figure out which, and will still need
> > omap3isp modifications...
> 
> Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.
> 
> It's essentially the functionality in the four patches. The data-lane and
> clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
> plural) to match the property documentation.

Ok, thanks, I got CSI-1 support to compile.

I'm now fighting with subdevices support. Camera flash and autofocus
coil really should be subdevices of the ISP, right?

Do you have any solution for that? [I need it for my userspace to
work, and porting the old one looks like lot of fun (tm) :-(].

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588012

[Multipart message — attachments visible in raw view] — view raw

On Tue 2017-02-21 13:11:04, Sakari Ailus wrote:
> On Tue, Feb 21, 2017 at 12:07:21PM +0100, Pavel Machek wrote:
> > On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > > I've tested ACPI, will test DT soon...
> > > 
> > > DT case works, too (Nokia N9).
> > 
> > Hmm. Good to know. Now to figure out how to get N900 case to work...
> > 
> > AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> > seem to be in, so I'll need to figure out which, and will still need
> > omap3isp modifications...
> 
> Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.
> 
> It's essentially the functionality in the four patches. The data-lane and
> clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
> plural) to match the property documentation.

Ok, I got the camera sensor to work. No subdevices support, so I don't
have focus (etc) working, but that's a start. I also had to remove
video-bus-switch support; but I guess it will be easier to use
video-multiplexer patches... 

I'll have patches over weekend.

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588141

On Sat, Feb 25, 2017 at 01:09:18AM +0100, Pavel Machek wrote:
> On Tue 2017-02-21 13:11:04, Sakari Ailus wrote:
> > On Tue, Feb 21, 2017 at 12:07:21PM +0100, Pavel Machek wrote:
> > > On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > > > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > > > I've tested ACPI, will test DT soon...
> > > > 
> > > > DT case works, too (Nokia N9).
> > > 
> > > Hmm. Good to know. Now to figure out how to get N900 case to work...
> > > 
> > > AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> > > seem to be in, so I'll need to figure out which, and will still need
> > > omap3isp modifications...
> > 
> > Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.
> > 
> > It's essentially the functionality in the four patches. The data-lane and
> > clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
> > plural) to match the property documentation.
> 
> Ok, I got the camera sensor to work. No subdevices support, so I don't
> have focus (etc) working, but that's a start. I also had to remove
> video-bus-switch support; but I guess it will be easier to use
> video-multiplexer patches... 
> 
> I'll have patches over weekend.

I briefly looked at what's there --- you do miss the video nodes for the
non-sensor sub-devices, and they also don't show up in the media graph,
right?

I guess they don't end up matching in the async list.

I think we need to make the non-sensor sub-device support more generic;
it's not just the OMAP 3 ISP that needs it. I think we need to document
the property for the flash phandle as well; I can write one, or refresh
an existing one that I believe already exists.

How about calling it either simply "flash" or "camera-flash"? Similarly
for lens: "lens" or "camera-lens". I have a vague feeling the "camera-"
prefix is somewhat redundant, so I'd just go for "flash" or "lens".

At the very least the property names must be generic (not hardware
dependent) as this kind of functionality should be present in the
framework rather than in individual drivers. That'll be for later though.

Making the sub-device bus configuration a pointer should be in a separate
patch. It makes sense since the entire configuration is not valid for all
sub-devices attached to the ISP anymore. I think it originally was a
separate patch, but they probably have been merged at some point. I can't
find it right now anyway.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1588241 — camera subdevice support was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > Ok, I got the camera sensor to work. No subdevices support, so I don't
> > have focus (etc) working, but that's a start. I also had to remove
> > video-bus-switch support; but I guess it will be easier to use
> > video-multiplexer patches... 
> > 
> > I'll have patches over weekend.
> 
> I briefly looked at what's there --- you do miss the video nodes for the
> non-sensor sub-devices, and they also don't show up in the media graph,
> right?

Yes.

> I guess they don't end up matching in the async list.

How should they get to the async list?

> I think we need to make the non-sensor sub-device support more generic;
> it's not just the OMAP 3 ISP that needs it. I think we need to document
> the property for the flash phandle as well; I can write one, or refresh
> an existing one that I believe already exists.
> 
> How about calling it either simply "flash" or "camera-flash"? Similarly
> for lens: "lens" or "camera-lens". I have a vague feeling the "camera-"
> prefix is somewhat redundant, so I'd just go for "flash" or "lens".

Actually, I'd go for "flash" and "focus-coil". There may be other
lens properties, such as zoom, mirror movement, lens identification,
...

> At the very least the property names must be generic (not hardware
> dependent) as this kind of functionality should be present in the
> framework rather than in individual drivers. That'll be for later
> though.

Agreed, that would be nice.

> Making the sub-device bus configuration a pointer should be in a separate
> patch. It makes sense since the entire configuration is not valid for all
> sub-devices attached to the ISP anymore. I think it originally was a
> separate patch, but they probably have been merged at some point. I can't
> find it right now anyway.

I believe I can find the patch. But I'm not sure if I can port it to
the fwnode infrastructure anytime soon...

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588244 — Re: camera subdevice support was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > Making the sub-device bus configuration a pointer should be in a separate
> > patch. It makes sense since the entire configuration is not valid for all
> > sub-devices attached to the ISP anymore. I think it originally was a
> > separate patch, but they probably have been merged at some point. I can't
> > find it right now anyway.
> 
> I believe I can find the patch. But I'm not sure if I can port it to
> the fwnode infrastructure anytime soon...

Here is the (unported) patch.

https://git.kernel.org/cgit/linux/kernel/git/pavel/linux-n900.git/commit/?h=camera-sa-5&id=e705712683b99fec282f87ed3e80bb58c95cf726

Maybe this helps,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588245 — Re: camera subdevice support was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

Moi! :-)

On Sat, Feb 25, 2017 at 10:53:22PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > Ok, I got the camera sensor to work. No subdevices support, so I don't
> > > have focus (etc) working, but that's a start. I also had to remove
> > > video-bus-switch support; but I guess it will be easier to use
> > > video-multiplexer patches... 
> > > 
> > > I'll have patches over weekend.
> > 
> > I briefly looked at what's there --- you do miss the video nodes for the
> > non-sensor sub-devices, and they also don't show up in the media graph,
> > right?
> 
> Yes.
> 
> > I guess they don't end up matching in the async list.
> 
> How should they get to the async list?

The patch you referred to does that. The problem is, it does make the bus
configuration a pointer as well. There should be two patches. That's not a
lot of work to separate them though. But it should be done.

> 
> > I think we need to make the non-sensor sub-device support more generic;
> > it's not just the OMAP 3 ISP that needs it. I think we need to document
> > the property for the flash phandle as well; I can write one, or refresh
> > an existing one that I believe already exists.
> > 
> > How about calling it either simply "flash" or "camera-flash"? Similarly
> > for lens: "lens" or "camera-lens". I have a vague feeling the "camera-"
> > prefix is somewhat redundant, so I'd just go for "flash" or "lens".
> 
> Actually, I'd go for "flash" and "focus-coil". There may be other
> lens properties, such as zoom, mirror movement, lens identification,
> ...

Good point. Still there may be other ways to move the lens than the voice
coil (which sure is cheap), so how about "flash" and "lens-focus"?

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1588301 — Re: camera subdevice support was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

[Multipart message — attachments visible in raw view] — view raw

Ahoj! :-)

> > > > Ok, I got the camera sensor to work. No subdevices support, so I don't
> > > > have focus (etc) working, but that's a start. I also had to remove
> > > > video-bus-switch support; but I guess it will be easier to use
> > > > video-multiplexer patches... 
> > > > 
> > > > I'll have patches over weekend.
> > > 
> > > I briefly looked at what's there --- you do miss the video nodes for the
> > > non-sensor sub-devices, and they also don't show up in the media graph,
> > > right?
> > 
> > Yes.
> > 
> > > I guess they don't end up matching in the async list.
> > 
> > How should they get to the async list?
> 
> The patch you referred to does that. The problem is, it does make the bus
> configuration a pointer as well. There should be two patches. That's not a
> lot of work to separate them though. But it should be done.

Well... This is the line I'm fighting with:

+ of_parse_phandle(dev->of_node, "ti,camera-flashes",
+							flash++)

If someone told me its fwnode equivalent, I might be able to get it to
work. Knowing what group_id is and if I could ignore it would help a
bit, too :-).

(Also, I'll be glad to test patches :-))

> > > I think we need to make the non-sensor sub-device support more generic;
> > > it's not just the OMAP 3 ISP that needs it. I think we need to document
> > > the property for the flash phandle as well; I can write one, or refresh
> > > an existing one that I believe already exists.
> > > 
> > > How about calling it either simply "flash" or "camera-flash"? Similarly
> > > for lens: "lens" or "camera-lens". I have a vague feeling the "camera-"
> > > prefix is somewhat redundant, so I'd just go for "flash" or "lens".
> > 
> > Actually, I'd go for "flash" and "focus-coil". There may be other
> > lens properties, such as zoom, mirror movement, lens identification,
> > ...
> 
> Good point. Still there may be other ways to move the lens than the voice
> coil (which sure is cheap), so how about "flash" and "lens-focus"?

Sounds good to me.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588402 — Re: camera subdevice support was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

Hyvää iltaa!

On Sun, Feb 26, 2017 at 09:38:51AM +0100, Pavel Machek wrote:
> Ahoj! :-)
> 
> > > > > Ok, I got the camera sensor to work. No subdevices support, so I don't
> > > > > have focus (etc) working, but that's a start. I also had to remove
> > > > > video-bus-switch support; but I guess it will be easier to use
> > > > > video-multiplexer patches... 
> > > > > 
> > > > > I'll have patches over weekend.
> > > > 
> > > > I briefly looked at what's there --- you do miss the video nodes for the
> > > > non-sensor sub-devices, and they also don't show up in the media graph,
> > > > right?
> > > 
> > > Yes.
> > > 
> > > > I guess they don't end up matching in the async list.
> > > 
> > > How should they get to the async list?
> > 
> > The patch you referred to does that. The problem is, it does make the bus
> > configuration a pointer as well. There should be two patches. That's not a
> > lot of work to separate them though. But it should be done.
> 
> Well... This is the line I'm fighting with:
> 
> + of_parse_phandle(dev->of_node, "ti,camera-flashes",
> +							flash++)
> 
> If someone told me its fwnode equivalent, I might be able to get it to
> work. Knowing what group_id is and if I could ignore it would help a
> bit, too :-).

Right.

ACPI does not have equivalents for OF phandles. That's the background of the
problem. The port and endpoint references are a bit special: there'a a
device reference and indices of the port and the endpoint nodes.

I think you can just use the OF API for the time being until we define how
to describe flash devices with ACPI. The difference with ACPI will be
visible there almost no matter what do you do there, which is one more
reason to have that functionality in the framework (and not drivers).

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1588242

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > > I've tested ACPI, will test DT soon...
> > > 
> > > DT case works, too (Nokia N9).
> > 
> > Hmm. Good to know. Now to figure out how to get N900 case to work...
> > 
> > AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> > seem to be in, so I'll need to figure out which, and will still need
> > omap3isp modifications...
> 
> Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.
> 
> It's essentially the functionality in the four patches. The data-lane and
> clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
> plural) to match the property documentation.

Yes, it seems to work.

Here's a patch. It has checkpatch issues, I can fix them.  More
support is needed on the ispcsiphy.c side... Could you take (fixed)
version of this to your fwnode branch?

Thanks,
									Pavel




---

omap3isp: add support for CSI1 bus
    
Signed-off-by: Pavel Machek <pavel@ucw.cz>
    
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 245225a..4b10cfe 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2032,6 +2034,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
 	struct v4l2_fwnode_endpoint vfwn;
 	unsigned int i;
 	int ret;
+	int csi1 = 0;
 
 	ret = v4l2_fwnode_endpoint_parse(fwn, &vfwn);
 	if (ret)
@@ -2059,38 +2062,82 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
 
 	case ISP_OF_PHY_CSIPHY1:
 	case ISP_OF_PHY_CSIPHY2:
-		/* FIXME: always assume CSI-2 for now. */
+		switch (vfwn.bus_type) {
+		case V4L2_MBUS_CSI2:
+			dev_dbg(dev, "csi2 configuration\n");
+			csi1 = 0;
+			break;
+		case V4L2_MBUS_CCP2:
+		case V4L2_MBUS_CSI1:
+			dev_dbg(dev, "csi1 configuration\n");
+			csi1 = 1;
+			break;
+		default:
+			dev_err(dev, "unkonwn bus type\n");
+		}
+
 		switch (vfwn.base.port) {
 		case ISP_OF_PHY_CSIPHY1:
-			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+			if (csi1)
+				buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
+			else
+				buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
 			break;
 		case ISP_OF_PHY_CSIPHY2:
-			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+			if (csi1)
+				buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
+			else
+				buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
 			break;
+		default:
+			dev_err(dev, "bad port\n");
 		}
-		buscfg->bus.csi2.lanecfg.clk.pos = vfwn.bus.mipi_csi2.clock_lane;
-		buscfg->bus.csi2.lanecfg.clk.pol =
-			vfwn.bus.mipi_csi2.lane_polarities[0];
-		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
-			buscfg->bus.csi2.lanecfg.clk.pol,
-			buscfg->bus.csi2.lanecfg.clk.pos);
-
-		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
-			buscfg->bus.csi2.lanecfg.data[i].pos =
-				vfwn.bus.mipi_csi2.data_lanes[i];
-			buscfg->bus.csi2.lanecfg.data[i].pol =
-				vfwn.bus.mipi_csi2.lane_polarities[i + 1];
+		if (csi1) {
+			buscfg->bus.ccp2.lanecfg.clk.pos = vfwn.bus.mipi_csi1.clock_lane;
+			buscfg->bus.ccp2.lanecfg.clk.pol =
+				vfwn.bus.mipi_csi1.lane_polarity[0];
+			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+				buscfg->bus.ccp2.lanecfg.clk.pol,
+				buscfg->bus.ccp2.lanecfg.clk.pos);
+
+			buscfg->bus.ccp2.lanecfg.data[0].pos = 1;
+			buscfg->bus.ccp2.lanecfg.data[0].pol = 0;
+
 			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
-				buscfg->bus.csi2.lanecfg.data[i].pol,
-				buscfg->bus.csi2.lanecfg.data[i].pos);
+				buscfg->bus.ccp2.lanecfg.data[0].pol,
+				buscfg->bus.ccp2.lanecfg.data[0].pos);
+
+			buscfg->bus.ccp2.strobe_clk_pol = vfwn.bus.mipi_csi1.clock_inv;
+			buscfg->bus.ccp2.phy_layer = vfwn.bus.mipi_csi1.strobe;
+			buscfg->bus.ccp2.ccp2_mode = vfwn.bus_type == V4L2_MBUS_CCP2;
+			buscfg->bus.ccp2.vp_clk_pol = 1;
+			
+			buscfg->bus.ccp2.crc = 1;		
+		} else {
+			buscfg->bus.csi2.lanecfg.clk.pos = vfwn.bus.mipi_csi2.clock_lane;
+			buscfg->bus.csi2.lanecfg.clk.pol =
+				vfwn.bus.mipi_csi2.lane_polarities[0];
+			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+				buscfg->bus.csi2.lanecfg.clk.pol,
+				buscfg->bus.csi2.lanecfg.clk.pos);
+
+			for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
+				buscfg->bus.csi2.lanecfg.data[i].pos =
+					vfwn.bus.mipi_csi2.data_lanes[i];
+				buscfg->bus.csi2.lanecfg.data[i].pol =
+					vfwn.bus.mipi_csi2.lane_polarities[i + 1];
+				dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
+					buscfg->bus.csi2.lanecfg.data[i].pol,
+					buscfg->bus.csi2.lanecfg.data[i].pos);
+			}
+			/*
+			 * FIXME: now we assume the CRC is always there.
+			 * Implement a way to obtain this information from the
+			 * sensor. Frame descriptors, perhaps?
+			 */
+
+			buscfg->bus.csi2.crc = 1;
 		}
-
-		/*
-		 * FIXME: now we assume the CRC is always there.
-		 * Implement a way to obtain this information from the
-		 * sensor. Frame descriptors, perhaps?
-		 */
-		buscfg->bus.csi2.crc = 1;
 		break;
 
 	default:


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588906

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > > On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > > > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > > > I've tested ACPI, will test DT soon...
> > > > 
> > > > DT case works, too (Nokia N9).
> > > 
> > > Hmm. Good to know. Now to figure out how to get N900 case to work...
> > > 
> > > AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> > > seem to be in, so I'll need to figure out which, and will still need
> > > omap3isp modifications...
> > 
> > Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.
> > 
> > It's essentially the functionality in the four patches. The data-lane and
> > clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
> > plural) to match the property documentation.
> 
> Yes, it seems to work.
> 
> Here's a patch. It has checkpatch issues, I can fix them.  More
> support is needed on the ispcsiphy.c side... Could you take (fixed)
> version of this to your fwnode branch?

Any feedback would be welcome :-)
									Pavel


> omap3isp: add support for CSI1 bus
>     
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>     
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index 245225a..4b10cfe 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2032,6 +2034,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
>  	struct v4l2_fwnode_endpoint vfwn;
>  	unsigned int i;
>  	int ret;
> +	int csi1 = 0;
>  
>  	ret = v4l2_fwnode_endpoint_parse(fwn, &vfwn);
>  	if (ret)
> @@ -2059,38 +2062,82 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
>  
>  	case ISP_OF_PHY_CSIPHY1:
>  	case ISP_OF_PHY_CSIPHY2:
> -		/* FIXME: always assume CSI-2 for now. */
> +		switch (vfwn.bus_type) {
> +		case V4L2_MBUS_CSI2:
> +			dev_dbg(dev, "csi2 configuration\n");
> +			csi1 = 0;
> +			break;
> +		case V4L2_MBUS_CCP2:
> +		case V4L2_MBUS_CSI1:
> +			dev_dbg(dev, "csi1 configuration\n");
> +			csi1 = 1;
> +			break;
> +		default:
> +			dev_err(dev, "unkonwn bus type\n");
> +		}
> +
>  		switch (vfwn.base.port) {
>  		case ISP_OF_PHY_CSIPHY1:
> -			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> +			if (csi1)
> +				buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
> +			else
> +				buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
>  			break;
>  		case ISP_OF_PHY_CSIPHY2:
> -			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> +			if (csi1)
> +				buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
> +			else
> +				buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
>  			break;
> +		default:
> +			dev_err(dev, "bad port\n");
>  		}
> -		buscfg->bus.csi2.lanecfg.clk.pos = vfwn.bus.mipi_csi2.clock_lane;
> -		buscfg->bus.csi2.lanecfg.clk.pol =
> -			vfwn.bus.mipi_csi2.lane_polarities[0];
> -		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> -			buscfg->bus.csi2.lanecfg.clk.pol,
> -			buscfg->bus.csi2.lanecfg.clk.pos);
> -
> -		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> -			buscfg->bus.csi2.lanecfg.data[i].pos =
> -				vfwn.bus.mipi_csi2.data_lanes[i];
> -			buscfg->bus.csi2.lanecfg.data[i].pol =
> -				vfwn.bus.mipi_csi2.lane_polarities[i + 1];
> +		if (csi1) {
> +			buscfg->bus.ccp2.lanecfg.clk.pos = vfwn.bus.mipi_csi1.clock_lane;
> +			buscfg->bus.ccp2.lanecfg.clk.pol =
> +				vfwn.bus.mipi_csi1.lane_polarity[0];
> +			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +				buscfg->bus.ccp2.lanecfg.clk.pol,
> +				buscfg->bus.ccp2.lanecfg.clk.pos);
> +
> +			buscfg->bus.ccp2.lanecfg.data[0].pos = 1;
> +			buscfg->bus.ccp2.lanecfg.data[0].pol = 0;
> +
>  			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
> -				buscfg->bus.csi2.lanecfg.data[i].pol,
> -				buscfg->bus.csi2.lanecfg.data[i].pos);
> +				buscfg->bus.ccp2.lanecfg.data[0].pol,
> +				buscfg->bus.ccp2.lanecfg.data[0].pos);
> +
> +			buscfg->bus.ccp2.strobe_clk_pol = vfwn.bus.mipi_csi1.clock_inv;
> +			buscfg->bus.ccp2.phy_layer = vfwn.bus.mipi_csi1.strobe;
> +			buscfg->bus.ccp2.ccp2_mode = vfwn.bus_type == V4L2_MBUS_CCP2;
> +			buscfg->bus.ccp2.vp_clk_pol = 1;
> +			
> +			buscfg->bus.ccp2.crc = 1;		
> +		} else {
> +			buscfg->bus.csi2.lanecfg.clk.pos = vfwn.bus.mipi_csi2.clock_lane;
> +			buscfg->bus.csi2.lanecfg.clk.pol =
> +				vfwn.bus.mipi_csi2.lane_polarities[0];
> +			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +				buscfg->bus.csi2.lanecfg.clk.pol,
> +				buscfg->bus.csi2.lanecfg.clk.pos);
> +
> +			for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> +				buscfg->bus.csi2.lanecfg.data[i].pos =
> +					vfwn.bus.mipi_csi2.data_lanes[i];
> +				buscfg->bus.csi2.lanecfg.data[i].pol =
> +					vfwn.bus.mipi_csi2.lane_polarities[i + 1];
> +				dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
> +					buscfg->bus.csi2.lanecfg.data[i].pol,
> +					buscfg->bus.csi2.lanecfg.data[i].pos);
> +			}
> +			/*
> +			 * FIXME: now we assume the CRC is always there.
> +			 * Implement a way to obtain this information from the
> +			 * sensor. Frame descriptors, perhaps?
> +			 */
> +
> +			buscfg->bus.csi2.crc = 1;
>  		}
> -
> -		/*
> -		 * FIXME: now we assume the CRC is always there.
> -		 * Implement a way to obtain this information from the
> -		 * sensor. Frame descriptors, perhaps?
> -		 */
> -		buscfg->bus.csi2.crc = 1;
>  		break;
>  
>  	default:
> 
> 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1588944

Hi Pavel,

Please find my comments below.

On Sat, Feb 25, 2017 at 11:12:55PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > On Mon 2017-02-20 15:56:36, Sakari Ailus wrote:
> > > > On Mon, Feb 20, 2017 at 03:09:13PM +0200, Sakari Ailus wrote:
> > > > > I've tested ACPI, will test DT soon...
> > > > 
> > > > DT case works, too (Nokia N9).
> > > 
> > > Hmm. Good to know. Now to figure out how to get N900 case to work...
> > > 
> > > AFAICT N9 has CSI2, not CSI1 support, right? Some of the core changes
> > > seem to be in, so I'll need to figure out which, and will still need
> > > omap3isp modifications...
> > 
> > Indeed, I've only tested for CSI-2 as I have no functional CSI-1 devices.
> > 
> > It's essentially the functionality in the four patches. The data-lane and
> > clock-name properties have been renamed as data-lanes and clock-lanes (i.e.
> > plural) to match the property documentation.
> 
> Yes, it seems to work.
> 
> Here's a patch. It has checkpatch issues, I can fix them.  More
> support is needed on the ispcsiphy.c side... Could you take (fixed)
> version of this to your fwnode branch?
> 
> Thanks,
> 									Pavel
> 
> 
> 
> 
> ---
> 
> omap3isp: add support for CSI1 bus
>     
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>     
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index 245225a..4b10cfe 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2032,6 +2034,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
>  	struct v4l2_fwnode_endpoint vfwn;
>  	unsigned int i;
>  	int ret;
> +	int csi1 = 0;
>  
>  	ret = v4l2_fwnode_endpoint_parse(fwn, &vfwn);
>  	if (ret)
> @@ -2059,38 +2062,82 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn,
>  
>  	case ISP_OF_PHY_CSIPHY1:
>  	case ISP_OF_PHY_CSIPHY2:
> -		/* FIXME: always assume CSI-2 for now. */
> +		switch (vfwn.bus_type) {
> +		case V4L2_MBUS_CSI2:
> +			dev_dbg(dev, "csi2 configuration\n");
> +			csi1 = 0;
> +			break;
> +		case V4L2_MBUS_CCP2:
> +		case V4L2_MBUS_CSI1:
> +			dev_dbg(dev, "csi1 configuration\n");
> +			csi1 = 1;
> +			break;
> +		default:
> +			dev_err(dev, "unkonwn bus type\n");
> +		}
> +
>  		switch (vfwn.base.port) {
>  		case ISP_OF_PHY_CSIPHY1:
> -			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> +			if (csi1)

You could compare vfwn.bus_type == V4L2_MBUS_CSI2 for this. But if you
choose the local variable, please make it bool instead.

> +				buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
> +			else
> +				buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
>  			break;
>  		case ISP_OF_PHY_CSIPHY2:
> -			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> +			if (csi1)
> +				buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
> +			else
> +				buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
>  			break;
> +		default:
> +			dev_err(dev, "bad port\n");
>  		}
> -		buscfg->bus.csi2.lanecfg.clk.pos = vfwn.bus.mipi_csi2.clock_lane;
> -		buscfg->bus.csi2.lanecfg.clk.pol =
> -			vfwn.bus.mipi_csi2.lane_polarities[0];
> -		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> -			buscfg->bus.csi2.lanecfg.clk.pol,
> -			buscfg->bus.csi2.lanecfg.clk.pos);
> -
> -		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> -			buscfg->bus.csi2.lanecfg.data[i].pos =
> -				vfwn.bus.mipi_csi2.data_lanes[i];
> -			buscfg->bus.csi2.lanecfg.data[i].pol =
> -				vfwn.bus.mipi_csi2.lane_polarities[i + 1];
> +		if (csi1) {
> +			buscfg->bus.ccp2.lanecfg.clk.pos = vfwn.bus.mipi_csi1.clock_lane;

Wrap after "="?

> +			buscfg->bus.ccp2.lanecfg.clk.pol =
> +				vfwn.bus.mipi_csi1.lane_polarity[0];
> +			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +				buscfg->bus.ccp2.lanecfg.clk.pol,
> +				buscfg->bus.ccp2.lanecfg.clk.pos);
> +
> +			buscfg->bus.ccp2.lanecfg.data[0].pos = 1;

Shouldn't this be vfwn.bus.mipi_csi1.data_lane ?

> +			buscfg->bus.ccp2.lanecfg.data[0].pol = 0;

And this one is vfwn.bus.mipi_csi1.lane_polarity[1] .

> +
>  			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
> -				buscfg->bus.csi2.lanecfg.data[i].pol,
> -				buscfg->bus.csi2.lanecfg.data[i].pos);
> +				buscfg->bus.ccp2.lanecfg.data[0].pol,
> +				buscfg->bus.ccp2.lanecfg.data[0].pos);
> +
> +			buscfg->bus.ccp2.strobe_clk_pol = vfwn.bus.mipi_csi1.clock_inv;
> +			buscfg->bus.ccp2.phy_layer = vfwn.bus.mipi_csi1.strobe;
> +			buscfg->bus.ccp2.ccp2_mode = vfwn.bus_type == V4L2_MBUS_CCP2;

The lines over 80 characters should be wrapped.

> +			buscfg->bus.ccp2.vp_clk_pol = 1;
> +			
> +			buscfg->bus.ccp2.crc = 1;		
> +		} else {
> +			buscfg->bus.csi2.lanecfg.clk.pos = vfwn.bus.mipi_csi2.clock_lane;
> +			buscfg->bus.csi2.lanecfg.clk.pol =
> +				vfwn.bus.mipi_csi2.lane_polarities[0];
> +			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +				buscfg->bus.csi2.lanecfg.clk.pol,
> +				buscfg->bus.csi2.lanecfg.clk.pos);
> +
> +			for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> +				buscfg->bus.csi2.lanecfg.data[i].pos =
> +					vfwn.bus.mipi_csi2.data_lanes[i];
> +				buscfg->bus.csi2.lanecfg.data[i].pol =
> +					vfwn.bus.mipi_csi2.lane_polarities[i + 1];
> +				dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
> +					buscfg->bus.csi2.lanecfg.data[i].pol,
> +					buscfg->bus.csi2.lanecfg.data[i].pos);
> +			}
> +			/*
> +			 * FIXME: now we assume the CRC is always there.
> +			 * Implement a way to obtain this information from the
> +			 * sensor. Frame descriptors, perhaps?
> +			 */
> +
> +			buscfg->bus.csi2.crc = 1;
>  		}
> -
> -		/*
> -		 * FIXME: now we assume the CRC is always there.
> -		 * Implement a way to obtain this information from the
> -		 * sensor. Frame descriptors, perhaps?
> -		 */
> -		buscfg->bus.csi2.crc = 1;
>  		break;
>  
>  	default:
> 
> 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web