linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
@ 2020-06-22 17:18 Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 01/25] dt-bindings: media: ov5647: Document pwdn-gpios Jacopo Mondi
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

Hello,
  this series improves and expand the existing ov5647 sensor driver to
the same feature level as found in RaspberryPi BSP.

It is based on recent media tree master and the sensor bindings conversion
to dt-schema I sent out a few days ago:
"[PATCH 0/2] dt-bidings: media: ov5647 bindings + small fix"

The first patches in the series has been sent by Roman as part of
"[PATCH v2 0/6] ov5647 driver improvement"
I took his patches in, addressed review comments and rebased on top
of the new dt-schema bindings. I kept the extensive receiver list he had
in his series for this reason.

The series continues by polishing and cleaning up the driver and expanding
its functionalities to support multiple modes and image formats.

The series has been tested with libcamera and the driver functionalities
compared with the BSP driver ones, and tested stand-alone by capturing
raw frames with yavta.

Thanks
   j

Dave Stevenson (8):
  media: ov5647: Add support for PWDN GPIO.
  media: ov5647: Add support for non-continuous clock mode
  media: ov5647: Add set_fmt and get_fmt calls.
  media: ov5647: Add support for get_selection()
  media: ov5647: Set V4L2_SUBDEV_FL_HAS_EVENTS flag
  media: ov5647: Support V4L2_CID_PIXEL_RATE
  media: ov5647: Support V4L2_CID_VBLANK control
  media: ov5647: Advertise the correct exposure range

David Plowman (1):
  media: ov5647: Support gain, exposure and AWB controls

Jacopo Mondi (16):
  dt-bindings: media: ov5647: Document pwdn-gpios
  dt-bindings: media: ov5647: Document clock-noncontinuous
  media: ov5647: Fix format initialization
  media: ov5647: Fix style issues
  media: ov5647: Replace license with SPDX identifier
  media: ov5647: Fix return value from read/write
  media: ov5647: Program mode at s_stream(1) time
  media: ov5647: Implement enum_frame_size()
  media: ov5647: Protect s_stream() with mutex
  media: ov5647: Rationalize driver structure name
  media: ov5647: Break out format handling
  media: ov5647: Rename SBGGR8 VGA mode
  media: ov5647: Add SGGBR10_1X10 modes
  media: ov5647: Implement set_fmt pad operation
  media: ov5647: Program mode only if it has changed
  media: ov5647: Support V4L2_CID_HBLANK control

 .../devicetree/bindings/media/i2c/ov5647.yaml |   11 +
 drivers/media/i2c/ov5647.c                    | 1267 +++++++++++++++--
 2 files changed, 1126 insertions(+), 152 deletions(-)

--
2.27.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/25] dt-bindings: media: ov5647: Document pwdn-gpios
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 02/25] dt-bindings: media: ov5647: Document clock-noncontinuous Jacopo Mondi
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

Document in dt-schema bindings for the ov5647 sensor the optional
'pwdn-gpios' property.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 Documentation/devicetree/bindings/media/i2c/ov5647.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.yaml b/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
index 067e222e0c7c3..58d64a69e9640 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
@@ -25,6 +25,10 @@ properties:
     description: Reference to the xclk clock
     maxItems: 1
 
+  pwdn-gpios:
+    description: Reference to the GPIO connected to the pwdn pin. Active high.
+    maxItems: 1
+
   port:
     type: object
     description: |-
@@ -61,6 +65,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
 
     i2c {
         #address-cells = <1>;
@@ -70,6 +75,7 @@ examples:
             compatible = "ovti,ov5647";
             reg = <0x36>;
             clocks = <&camera_clk>;
+            pwdn-gpios = <&pioE 29 GPIO_ACTIVE_HIGH>;
 
             port {
                 camera_out: endpoint {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/25] dt-bindings: media: ov5647: Document clock-noncontinuous
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 01/25] dt-bindings: media: ov5647: Document pwdn-gpios Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 03/25] media: ov5647: Add support for PWDN GPIO Jacopo Mondi
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

Document the optional clock-noncontinuous endpoint property that
allows enabling MIPI CSI-2 non-continuous clock operations.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 Documentation/devicetree/bindings/media/i2c/ov5647.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.yaml b/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
index 58d64a69e9640..68d6998d7180c 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
@@ -45,6 +45,11 @@ properties:
             description: |-
               phandle to the video receiver input port
 
+          clock-noncontinuous:
+            type: boolean
+            description: |-
+              Set to true to allow MIPI CSI-2 non-continuous clock operations
+
         required:
           - remote-endpoint
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/25] media: ov5647: Add support for PWDN GPIO.
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 01/25] dt-bindings: media: ov5647: Document pwdn-gpios Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 02/25] dt-bindings: media: ov5647: Document clock-noncontinuous Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 04/25] media: ov5647: Add support for non-continuous clock mode Jacopo Mondi
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Add support for an optional GPIO connected to PWDN on the sensor. This
allows the use of hardware standby mode where internal device clock
and circuit activities are halted.

Please note that power is off when PWDN is high.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index e7d2e5b4ad4b9..105ff7f899b34 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -21,6 +21,7 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -35,6 +36,13 @@
 
 #define SENSOR_NAME "ov5647"
 
+/*
+ * From the datasheet, "20ms after PWDN goes low or 20ms after RESETB goes
+ * high if reset is inserted after PWDN goes high, host can access sensor's
+ * SCCB to initialize sensor."
+ */
+#define PWDN_ACTIVE_DELAY_MS	20
+
 #define MIPI_CTRL00_CLOCK_LANE_GATE		BIT(5)
 #define MIPI_CTRL00_BUS_IDLE			BIT(2)
 #define MIPI_CTRL00_CLOCK_LANE_DISABLE		BIT(0)
@@ -86,6 +94,7 @@ struct ov5647 {
 	unsigned int			height;
 	int				power_count;
 	struct clk			*xclk;
+	struct gpio_desc		*pwdn;
 };
 
 static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
@@ -355,6 +364,11 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 	if (on && !ov5647->power_count)	{
 		dev_dbg(&client->dev, "OV5647 power on\n");
 
+		if (ov5647->pwdn) {
+			gpiod_set_value_cansleep(ov5647->pwdn, 0);
+			msleep(PWDN_ACTIVE_DELAY_MS);
+		}
+
 		ret = clk_prepare_enable(ov5647->xclk);
 		if (ret < 0) {
 			dev_err(&client->dev, "clk prepare enable failed\n");
@@ -392,6 +406,8 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 			dev_dbg(&client->dev, "soft stby failed\n");
 
 		clk_disable_unprepare(ov5647->xclk);
+
+		gpiod_set_value_cansleep(ov5647->pwdn, 1);
 	}
 
 	/* Update the power count. */
@@ -581,6 +597,10 @@ static int ov5647_probe(struct i2c_client *client)
 		return -EINVAL;
 	}
 
+	/* Request the power down GPIO asserted */
+	sensor->pwdn = devm_gpiod_get_optional(&client->dev, "pwdn",
+					       GPIOD_OUT_HIGH);
+
 	mutex_init(&sensor->lock);
 
 	sd = &sensor->sd;
@@ -594,7 +614,15 @@ static int ov5647_probe(struct i2c_client *client)
 	if (ret < 0)
 		goto mutex_remove;
 
+	if (sensor->pwdn) {
+		gpiod_set_value_cansleep(sensor->pwdn, 0);
+		msleep(PWDN_ACTIVE_DELAY_MS);
+	}
+
 	ret = ov5647_detect(sd);
+
+	gpiod_set_value_cansleep(sensor->pwdn, 1);
+
 	if (ret < 0)
 		goto error;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/25] media: ov5647: Add support for non-continuous clock mode
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (2 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 03/25] media: ov5647: Add support for PWDN GPIO Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 05/25] media: ov5647: Add set_fmt and get_fmt calls Jacopo Mondi
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Add support for optional non-continuous clock mode to the ov5647
sensor driver.

Non-continuous clock saves a small amount of power and on some SoCs
is easier to interface with.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 105ff7f899b34..2d69cd97142d7 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -44,6 +44,7 @@
 #define PWDN_ACTIVE_DELAY_MS	20
 
 #define MIPI_CTRL00_CLOCK_LANE_GATE		BIT(5)
+#define MIPI_CTRL00_LINE_SYNC_ENABLE		BIT(4)
 #define MIPI_CTRL00_BUS_IDLE			BIT(2)
 #define MIPI_CTRL00_CLOCK_LANE_DISABLE		BIT(0)
 
@@ -95,6 +96,7 @@ struct ov5647 {
 	int				power_count;
 	struct clk			*xclk;
 	struct gpio_desc		*pwdn;
+	bool				clock_ncont;
 };
 
 static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
@@ -269,9 +271,15 @@ static int ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
 
 static int ov5647_stream_on(struct v4l2_subdev *sd)
 {
+	struct ov5647 *ov5647 = to_state(sd);
+	u8 val = MIPI_CTRL00_BUS_IDLE;
 	int ret;
 
-	ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, MIPI_CTRL00_BUS_IDLE);
+	if (ov5647->clock_ncont)
+		val |= MIPI_CTRL00_CLOCK_LANE_GATE |
+		       MIPI_CTRL00_LINE_SYNC_ENABLE;
+
+	ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, val);
 	if (ret < 0)
 		return ret;
 
@@ -546,9 +554,11 @@ static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
 	.open = ov5647_open,
 };
 
-static int ov5647_parse_dt(struct device_node *np)
+static int ov5647_parse_dt(struct ov5647 *sensor, struct device_node *np)
 {
-	struct v4l2_fwnode_endpoint bus_cfg = { .bus_type = 0 };
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY,
+	};
 	struct device_node *ep;
 
 	int ret;
@@ -558,7 +568,13 @@ static int ov5647_parse_dt(struct device_node *np)
 		return -EINVAL;
 
 	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &bus_cfg);
+	if (ret)
+		goto out;
 
+	sensor->clock_ncont = bus_cfg.bus.mipi_csi2.flags &
+			      V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
+
+out:
 	of_node_put(ep);
 	return ret;
 }
@@ -577,7 +593,7 @@ static int ov5647_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	if (IS_ENABLED(CONFIG_OF) && np) {
-		ret = ov5647_parse_dt(np);
+		ret = ov5647_parse_dt(sensor, np);
 		if (ret) {
 			dev_err(dev, "DT parsing error: %d\n", ret);
 			return ret;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/25] media: ov5647: Add set_fmt and get_fmt calls.
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (3 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 04/25] media: ov5647: Add support for non-continuous clock mode Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 06/25] media: ov5647: Fix format initialization Jacopo Mondi
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

There's no way to query the subdevice for the supported
resolutions. Add set_fmt and get_fmt implementations. Since there's
only one format supported set_fmt does nothing and get returns single
format.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 2d69cd97142d7..43fecf0ca58f3 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -487,8 +487,27 @@ static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_pad_config *cfg,
+			      struct v4l2_subdev_format *format)
+{
+	struct v4l2_mbus_framefmt *fmt = &format->format;
+
+	/* Only one format is supported, so return that */
+	memset(fmt, 0, sizeof(*fmt));
+	fmt->code = MEDIA_BUS_FMT_SBGGR8_1X8;
+	fmt->colorspace = V4L2_COLORSPACE_SRGB;
+	fmt->field = V4L2_FIELD_NONE;
+	fmt->width = 640;
+	fmt->height = 480;
+
+	return 0;
+}
+
 static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {
 	.enum_mbus_code = ov5647_enum_mbus_code,
+	.set_fmt =	  ov5647_set_get_fmt,
+	.get_fmt =	  ov5647_set_get_fmt,
 };
 
 static const struct v4l2_subdev_ops ov5647_subdev_ops = {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/25] media: ov5647: Fix format initialization
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (4 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 05/25] media: ov5647: Add set_fmt and get_fmt calls Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 07/25] media: ov5647: Fix style issues Jacopo Mondi
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

The driver currently support a single format. Fix its initialization to
use the only supported resolution.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 43fecf0ca58f3..c92856d3aa81c 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -560,9 +560,8 @@ static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 	crop->height = OV5647_WINDOW_HEIGHT_DEF;
 
 	format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
-
-	format->width = OV5647_WINDOW_WIDTH_DEF;
-	format->height = OV5647_WINDOW_HEIGHT_DEF;
+	format->width = 640;
+	format->height = 480;
 	format->field = V4L2_FIELD_NONE;
 	format->colorspace = V4L2_COLORSPACE_SRGB;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/25] media: ov5647: Fix style issues
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (5 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 06/25] media: ov5647: Fix format initialization Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 08/25] media: ov5647: Replace license with SPDX identifier Jacopo Mondi
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

The driver has some obvious style issues which are worth fixing before
expanding the driver capabilities.

Fix:
- Variable declaration order
- Function parameters alignment
- Multi-line comments and spurious line breaks
- Use lowercase for hexadecimal values

Cosmetic change, no functional changes intended.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 101 +++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 56 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index c92856d3aa81c..e9679382623f9 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -34,8 +34,6 @@
 #include <media/v4l2-image-sizes.h>
 #include <media/v4l2-mediabus.h>
 
-#define SENSOR_NAME "ov5647"
-
 /*
  * From the datasheet, "20ms after PWDN goes low or 20ms after RESETB goes
  * high if reset is inserted after PWDN goes high, host can access sensor's
@@ -50,9 +48,9 @@
 
 #define OV5647_SW_STANDBY		0x0100
 #define OV5647_SW_RESET			0x0103
-#define OV5647_REG_CHIPID_H		0x300A
-#define OV5647_REG_CHIPID_L		0x300B
-#define OV5640_REG_PAD_OUT		0x300D
+#define OV5647_REG_CHIPID_H		0x300a
+#define OV5647_REG_CHIPID_L		0x300b
+#define OV5640_REG_PAD_OUT		0x300d
 #define OV5647_REG_FRAME_OFF_NUMBER	0x4202
 #define OV5647_REG_MIPI_CTRL00		0x4800
 #define OV5647_REG_MIPI_CTRL14		0x4814
@@ -158,7 +156,7 @@ static struct regval_list ov5647_640x480[] = {
 	{0x3808, 0x02},
 	{0x3809, 0x80},
 	{0x380a, 0x01},
-	{0x380b, 0xE0},
+	{0x380b, 0xe0},
 	{0x3801, 0x00},
 	{0x3802, 0x00},
 	{0x3803, 0x00},
@@ -209,9 +207,9 @@ static struct regval_list ov5647_640x480[] = {
 
 static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
 {
-	int ret;
 	unsigned char data[3] = { reg >> 8, reg & 0xff, val};
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	int ret;
 
 	ret = i2c_master_send(client, data, 3);
 	if (ret < 0)
@@ -223,9 +221,9 @@ static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
 
 static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
 {
-	int ret;
 	unsigned char data_w[2] = { reg >> 8, reg & 0xff };
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	int ret;
 
 	ret = i2c_master_send(client, data_w, 2);
 	if (ret < 0) {
@@ -243,7 +241,7 @@ static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
 }
 
 static int ov5647_write_array(struct v4l2_subdev *sd,
-				struct regval_list *regs, int array_size)
+			      struct regval_list *regs, int array_size)
 {
 	int i, ret;
 
@@ -266,6 +264,7 @@ static int ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
 		return ret;
 
 	channel_id &= ~(3 << 6);
+
 	return ov5647_write(sd, OV5647_REG_MIPI_CTRL14, channel_id | (channel << 6));
 }
 
@@ -294,8 +293,8 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
 {
 	int ret;
 
-	ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, MIPI_CTRL00_CLOCK_LANE_GATE
-			   | MIPI_CTRL00_BUS_IDLE | MIPI_CTRL00_CLOCK_LANE_DISABLE);
+	ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, MIPI_CTRL00_CLOCK_LANE_GATE |
+			       MIPI_CTRL00_BUS_IDLE | MIPI_CTRL00_CLOCK_LANE_DISABLE);
 	if (ret < 0)
 		return ret;
 
@@ -325,16 +324,16 @@ static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
 
 static int __sensor_init(struct v4l2_subdev *sd)
 {
-	int ret;
-	u8 resetval, rdval;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	u8 resetval, rdval;
+	int ret;
 
 	ret = ov5647_read(sd, OV5647_SW_STANDBY, &rdval);
 	if (ret < 0)
 		return ret;
 
 	ret = ov5647_write_array(sd, ov5647_640x480,
-					ARRAY_SIZE(ov5647_640x480));
+				 ARRAY_SIZE(ov5647_640x480));
 	if (ret < 0) {
 		dev_err(&client->dev, "write sensor default regs error\n");
 		return ret;
@@ -355,17 +354,15 @@ static int __sensor_init(struct v4l2_subdev *sd)
 			return ret;
 	}
 
-	/*
-	 * stream off to make the clock lane into LP-11 state.
-	 */
+	/* Stream off to make the clock lane into LP-11 state. */
 	return ov5647_stream_off(sd);
 }
 
 static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 {
-	int ret = 0;
-	struct ov5647 *ov5647 = to_state(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ov5647 *ov5647 = to_state(sd);
+	int ret = 0;
 
 	mutex_lock(&ov5647->lock);
 
@@ -384,7 +381,7 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 		}
 
 		ret = ov5647_write_array(sd, sensor_oe_enable_regs,
-				ARRAY_SIZE(sensor_oe_enable_regs));
+					 ARRAY_SIZE(sensor_oe_enable_regs));
 		if (ret < 0) {
 			clk_disable_unprepare(ov5647->xclk);
 			dev_err(&client->dev,
@@ -403,18 +400,15 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 		dev_dbg(&client->dev, "OV5647 power off\n");
 
 		ret = ov5647_write_array(sd, sensor_oe_disable_regs,
-				ARRAY_SIZE(sensor_oe_disable_regs));
-
+					 ARRAY_SIZE(sensor_oe_disable_regs));
 		if (ret < 0)
 			dev_dbg(&client->dev, "disable oe failed\n");
 
 		ret = set_sw_standby(sd, true);
-
 		if (ret < 0)
 			dev_dbg(&client->dev, "soft stby failed\n");
 
 		clk_disable_unprepare(ov5647->xclk);
-
 		gpiod_set_value_cansleep(ov5647->pwdn, 1);
 	}
 
@@ -430,10 +424,10 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 static int ov5647_sensor_get_register(struct v4l2_subdev *sd,
-				struct v4l2_dbg_register *reg)
+				      struct v4l2_dbg_register *reg)
 {
-	u8 val;
 	int ret;
+	u8 val;
 
 	ret = ov5647_read(sd, reg->reg & 0xff, &val);
 	if (ret < 0)
@@ -446,15 +440,13 @@ static int ov5647_sensor_get_register(struct v4l2_subdev *sd,
 }
 
 static int ov5647_sensor_set_register(struct v4l2_subdev *sd,
-				const struct v4l2_dbg_register *reg)
+				      const struct v4l2_dbg_register *reg)
 {
 	return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
 }
 #endif
 
-/*
- * Subdev core operations registration
- */
+/* Subdev core operations registration */
 static const struct v4l2_subdev_core_ops ov5647_subdev_core_ops = {
 	.s_power		= ov5647_sensor_power,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -476,8 +468,8 @@ static const struct v4l2_subdev_video_ops ov5647_subdev_video_ops = {
 };
 
 static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
-				struct v4l2_subdev_pad_config *cfg,
-				struct v4l2_subdev_mbus_code_enum *code)
+				 struct v4l2_subdev_pad_config *cfg,
+				 struct v4l2_subdev_mbus_code_enum *code)
 {
 	if (code->index > 0)
 		return -EINVAL;
@@ -493,7 +485,7 @@ static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
 {
 	struct v4l2_mbus_framefmt *fmt = &format->format;
 
-	/* Only one format is supported, so return that */
+	/* Only one format is supported, so return that. */
 	memset(fmt, 0, sizeof(*fmt));
 	fmt->code = MEDIA_BUS_FMT_SBGGR8_1X8;
 	fmt->colorspace = V4L2_COLORSPACE_SRGB;
@@ -518,9 +510,9 @@ static const struct v4l2_subdev_ops ov5647_subdev_ops = {
 
 static int ov5647_detect(struct v4l2_subdev *sd)
 {
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	u8 read;
 	int ret;
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 
 	ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
 	if (ret < 0)
@@ -549,10 +541,8 @@ static int ov5647_detect(struct v4l2_subdev *sd)
 
 static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 {
-	struct v4l2_mbus_framefmt *format =
-				v4l2_subdev_get_try_format(sd, fh->pad, 0);
-	struct v4l2_rect *crop =
-				v4l2_subdev_get_try_crop(sd, fh->pad, 0);
+	struct v4l2_mbus_framefmt *format = v4l2_subdev_get_try_format(sd, fh->pad, 0);
+	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, fh->pad, 0);
 
 	crop->left = OV5647_COLUMN_START_DEF;
 	crop->top = OV5647_ROW_START_DEF;
@@ -578,7 +568,6 @@ static int ov5647_parse_dt(struct ov5647 *sensor, struct device_node *np)
 		.bus_type = V4L2_MBUS_CSI2_DPHY,
 	};
 	struct device_node *ep;
-
 	int ret;
 
 	ep = of_graph_get_next_endpoint(np, NULL);
@@ -594,17 +583,18 @@ static int ov5647_parse_dt(struct ov5647 *sensor, struct device_node *np)
 
 out:
 	of_node_put(ep);
+
 	return ret;
 }
 
 static int ov5647_probe(struct i2c_client *client)
 {
+	struct device_node *np = client->dev.of_node;
 	struct device *dev = &client->dev;
 	struct ov5647 *sensor;
-	int ret;
 	struct v4l2_subdev *sd;
-	struct device_node *np = client->dev.of_node;
 	u32 xclk_freq;
+	int ret;
 
 	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
 	if (!sensor)
@@ -618,7 +608,6 @@ static int ov5647_probe(struct i2c_client *client)
 		}
 	}
 
-	/* get system clock (xclk) */
 	sensor->xclk = devm_clk_get(dev, NULL);
 	if (IS_ERR(sensor->xclk)) {
 		dev_err(dev, "could not get xclk");
@@ -631,22 +620,21 @@ static int ov5647_probe(struct i2c_client *client)
 		return -EINVAL;
 	}
 
-	/* Request the power down GPIO asserted */
-	sensor->pwdn = devm_gpiod_get_optional(&client->dev, "pwdn",
-					       GPIOD_OUT_HIGH);
+	/* Request the power down GPIO asserted. */
+	sensor->pwdn = devm_gpiod_get_optional(dev, "pwdn", GPIOD_OUT_HIGH);
 
 	mutex_init(&sensor->lock);
 
 	sd = &sensor->sd;
 	v4l2_i2c_subdev_init(sd, client, &ov5647_subdev_ops);
-	sensor->sd.internal_ops = &ov5647_subdev_internal_ops;
-	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->internal_ops = &ov5647_subdev_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
 	sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
 	ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
 	if (ret < 0)
-		goto mutex_remove;
+		goto mutex_destroy;
 
 	if (sensor->pwdn) {
 		gpiod_set_value_cansleep(sensor->pwdn, 0);
@@ -654,22 +642,23 @@ static int ov5647_probe(struct i2c_client *client)
 	}
 
 	ret = ov5647_detect(sd);
-
 	gpiod_set_value_cansleep(sensor->pwdn, 1);
-
 	if (ret < 0)
-		goto error;
+		goto entity_cleanup;
 
 	ret = v4l2_async_register_subdev(sd);
 	if (ret < 0)
-		goto error;
+		goto entity_cleanup;
 
 	dev_dbg(dev, "OmniVision OV5647 camera driver probed\n");
+
 	return 0;
-error:
+
+entity_cleanup:
 	media_entity_cleanup(&sd->entity);
-mutex_remove:
+mutex_destroy:
 	mutex_destroy(&sensor->lock);
+
 	return ret;
 }
 
@@ -688,7 +677,7 @@ static int ov5647_remove(struct i2c_client *client)
 
 static const struct i2c_device_id ov5647_id[] = {
 	{ "ov5647", 0 },
-	{ }
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, ov5647_id);
 
@@ -703,7 +692,7 @@ MODULE_DEVICE_TABLE(of, ov5647_of_match);
 static struct i2c_driver ov5647_driver = {
 	.driver = {
 		.of_match_table = of_match_ptr(ov5647_of_match),
-		.name	= SENSOR_NAME,
+		.name	= "ov5647",
 	},
 	.probe_new	= ov5647_probe,
 	.remove		= ov5647_remove,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/25] media: ov5647: Replace license with SPDX identifier
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (6 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 07/25] media: ov5647: Fix style issues Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 09/25] media: ov5647: Fix return value from read/write Jacopo Mondi
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

Replace the boilerplate license text with the SPDX identifier.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index e9679382623f9..61aa86e507b32 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * A V4L2 driver for OmniVision OV5647 cameras.
  *
@@ -8,15 +9,6 @@
  * Copyright (C) 2006-7 Jonathan Corbet <corbet@lwn.net>
  *
  * Copyright (C) 2016, Synopsys, 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 version 2.
- *
- * This program is distributed .as is. WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include <linux/clk.h>
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/25] media: ov5647: Fix return value from read/write
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (7 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 08/25] media: ov5647: Replace license with SPDX identifier Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:18 ` [PATCH 10/25] media: ov5647: Program mode at s_stream(1) time Jacopo Mondi
  2020-06-22 17:26 ` [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

The ov5647_read()/ov5647_write() return in case of success the number
of bytes read or written respectively. This requires callers to check
if the return value is less than zero to detect an error. Unfortunately,
in several places, callers directly return the result of a read/write
call, causing issues when the returned valued is checked to be different
from zero to detect an error.

Fix this by returning zero if i2c_master_send() and i2c_master_read()
return a positive value (the number of bytes written or read).

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 61aa86e507b32..0c88f682de9b9 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -204,11 +204,13 @@ static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
 	int ret;
 
 	ret = i2c_master_send(client, data, 3);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
 				__func__, reg);
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
@@ -225,11 +227,13 @@ static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
 	}
 
 	ret = i2c_master_recv(client, val, 1);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
 				__func__, reg);
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static int ov5647_write_array(struct v4l2_subdev *sd,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/25] media: ov5647: Program mode at s_stream(1) time
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (8 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 09/25] media: ov5647: Fix return value from read/write Jacopo Mondi
@ 2020-06-22 17:18 ` Jacopo Mondi
  2020-06-22 17:26 ` [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
  10 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:18 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

Rename __sensor_init() function to ov5647_set_mode() as the function
is a regular one and the double underscores prefix shall be removed, and
then move it to program the mode at s_stream(1) time, not at sensor power
up.

Break out from __sensor_init() the stream_off() operation call at sensor
power up to coax the lanes in LP-11 state.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 81 +++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 0c88f682de9b9..bb9ff77f49fe0 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -264,12 +264,54 @@ static int ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
 	return ov5647_write(sd, OV5647_REG_MIPI_CTRL14, channel_id | (channel << 6));
 }
 
+static int ov5647_set_mode(struct v4l2_subdev *sd)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	u8 resetval, rdval;
+	int ret;
+
+	ret = ov5647_read(sd, OV5647_SW_STANDBY, &rdval);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5647_write_array(sd, ov5647_640x480,
+				 ARRAY_SIZE(ov5647_640x480));
+	if (ret < 0) {
+		dev_err(&client->dev, "write sensor default regs error\n");
+		return ret;
+	}
+
+	ret = ov5647_set_virtual_channel(sd, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5647_read(sd, OV5647_SW_STANDBY, &resetval);
+	if (ret < 0)
+		return ret;
+
+	if (!(resetval & 0x01)) {
+		dev_err(&client->dev, "Device was in SW standby");
+		ret = ov5647_write(sd, OV5647_SW_STANDBY, 0x01);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int ov5647_stream_on(struct v4l2_subdev *sd)
 {
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct ov5647 *ov5647 = to_state(sd);
 	u8 val = MIPI_CTRL00_BUS_IDLE;
 	int ret;
 
+	ret = ov5647_set_mode(sd);
+	if (ret) {
+		dev_err(&client->dev, "Failed to program sensor mode: %d\n", ret);
+		return ret;
+	}
+
 	if (ov5647->clock_ncont)
 		val |= MIPI_CTRL00_CLOCK_LANE_GATE |
 		       MIPI_CTRL00_LINE_SYNC_ENABLE;
@@ -318,42 +360,6 @@ static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
 	return ov5647_write(sd, OV5647_SW_STANDBY, rdval);
 }
 
-static int __sensor_init(struct v4l2_subdev *sd)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	u8 resetval, rdval;
-	int ret;
-
-	ret = ov5647_read(sd, OV5647_SW_STANDBY, &rdval);
-	if (ret < 0)
-		return ret;
-
-	ret = ov5647_write_array(sd, ov5647_640x480,
-				 ARRAY_SIZE(ov5647_640x480));
-	if (ret < 0) {
-		dev_err(&client->dev, "write sensor default regs error\n");
-		return ret;
-	}
-
-	ret = ov5647_set_virtual_channel(sd, 0);
-	if (ret < 0)
-		return ret;
-
-	ret = ov5647_read(sd, OV5647_SW_STANDBY, &resetval);
-	if (ret < 0)
-		return ret;
-
-	if (!(resetval & 0x01)) {
-		dev_err(&client->dev, "Device was in SW standby");
-		ret = ov5647_write(sd, OV5647_SW_STANDBY, 0x01);
-		if (ret < 0)
-			return ret;
-	}
-
-	/* Stream off to make the clock lane into LP-11 state. */
-	return ov5647_stream_off(sd);
-}
-
 static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -385,7 +391,8 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
 			goto out;
 		}
 
-		ret = __sensor_init(sd);
+		/* Stream off to coax lanes into LP-11 state. */
+		ret = ov5647_stream_off(sd);
 		if (ret < 0) {
 			clk_disable_unprepare(ov5647->xclk);
 			dev_err(&client->dev,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
  2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
                   ` (9 preceding siblings ...)
  2020-06-22 17:18 ` [PATCH 10/25] media: ov5647: Program mode at s_stream(1) time Jacopo Mondi
@ 2020-06-22 17:26 ` Jacopo Mondi
  2020-06-23 10:30   ` Eugeniu Rosca
  10 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-22 17:26 UTC (permalink / raw)
  To: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

My ISP has rejected the rest of the series: too many emails :(
Has it ever happened to anyone else ? How did you solved this ?

I'll send the rest of the series tomorrow. Sorry about this.

On Mon, Jun 22, 2020 at 07:18:45PM +0200, Jacopo Mondi wrote:
> Hello,
>   this series improves and expand the existing ov5647 sensor driver to
> the same feature level as found in RaspberryPi BSP.
>
> It is based on recent media tree master and the sensor bindings conversion
> to dt-schema I sent out a few days ago:
> "[PATCH 0/2] dt-bidings: media: ov5647 bindings + small fix"
>
> The first patches in the series has been sent by Roman as part of
> "[PATCH v2 0/6] ov5647 driver improvement"
> I took his patches in, addressed review comments and rebased on top
> of the new dt-schema bindings. I kept the extensive receiver list he had
> in his series for this reason.
>
> The series continues by polishing and cleaning up the driver and expanding
> its functionalities to support multiple modes and image formats.
>
> The series has been tested with libcamera and the driver functionalities
> compared with the BSP driver ones, and tested stand-alone by capturing
> raw frames with yavta.
>
> Thanks
>    j
>
> Dave Stevenson (8):
>   media: ov5647: Add support for PWDN GPIO.
>   media: ov5647: Add support for non-continuous clock mode
>   media: ov5647: Add set_fmt and get_fmt calls.
>   media: ov5647: Add support for get_selection()
>   media: ov5647: Set V4L2_SUBDEV_FL_HAS_EVENTS flag
>   media: ov5647: Support V4L2_CID_PIXEL_RATE
>   media: ov5647: Support V4L2_CID_VBLANK control
>   media: ov5647: Advertise the correct exposure range
>
> David Plowman (1):
>   media: ov5647: Support gain, exposure and AWB controls
>
> Jacopo Mondi (16):
>   dt-bindings: media: ov5647: Document pwdn-gpios
>   dt-bindings: media: ov5647: Document clock-noncontinuous
>   media: ov5647: Fix format initialization
>   media: ov5647: Fix style issues
>   media: ov5647: Replace license with SPDX identifier
>   media: ov5647: Fix return value from read/write
>   media: ov5647: Program mode at s_stream(1) time
>   media: ov5647: Implement enum_frame_size()
>   media: ov5647: Protect s_stream() with mutex
>   media: ov5647: Rationalize driver structure name
>   media: ov5647: Break out format handling
>   media: ov5647: Rename SBGGR8 VGA mode
>   media: ov5647: Add SGGBR10_1X10 modes
>   media: ov5647: Implement set_fmt pad operation
>   media: ov5647: Program mode only if it has changed
>   media: ov5647: Support V4L2_CID_HBLANK control
>
>  .../devicetree/bindings/media/i2c/ov5647.yaml |   11 +
>  drivers/media/i2c/ov5647.c                    | 1267 +++++++++++++++--
>  2 files changed, 1126 insertions(+), 152 deletions(-)
>
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
  2020-06-22 17:26 ` [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
@ 2020-06-23 10:30   ` Eugeniu Rosca
  2020-06-23 10:49     ` Jacopo Mondi
  0 siblings, 1 reply; 18+ messages in thread
From: Eugeniu Rosca @ 2020-06-23 10:30 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush, mrodin, hugues.fruchet,
	mripard, aford173, sudipi, andrew_gabbasov, erosca, linux-media,
	libcamera-devel, Eugeniu Rosca

Hi Jacopo,

Many thanks for your precious contribution.

On Mon, Jun 22, 2020 at 07:26:14PM +0200, Jacopo Mondi wrote:
> My ISP has rejected the rest of the series: too many emails :(
> Has it ever happened to anyone else ? How did you solved this ?

I guess leaving 5-10 seconds between sending individual patches should
overcome this? I wonder if git provides a built-in command for that?

-- 
Best regards,
Eugeniu Rosca

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
  2020-06-23 10:30   ` Eugeniu Rosca
@ 2020-06-23 10:49     ` Jacopo Mondi
  2020-06-23 12:17       ` Eugeniu Rosca
  0 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-23 10:49 UTC (permalink / raw)
  To: Eugeniu Rosca
  Cc: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush, mrodin, hugues.fruchet,
	mripard, aford173, sudipi, andrew_gabbasov, linux-media,
	libcamera-devel, Eugeniu Rosca

Hi Eugeniu,

On Tue, Jun 23, 2020 at 12:30:02PM +0200, Eugeniu Rosca wrote:
> Hi Jacopo,
>
> Many thanks for your precious contribution.
>
> On Mon, Jun 22, 2020 at 07:26:14PM +0200, Jacopo Mondi wrote:
> > My ISP has rejected the rest of the series: too many emails :(
> > Has it ever happened to anyone else ? How did you solved this ?
>
> I guess leaving 5-10 seconds between sending individual patches should
> overcome this? I wonder if git provides a built-in command for that?
>

git send-email does provide the --batch-size --relogin-delay options,
as Ezequiel suggested me in #v4l.

I tried re-sending with a 10 email batch and a 5 seconds delay but I
got the same failure. I was not able to find any description of the
email number limits for the SMTP server I'm using, so I could only go
and try. I think the extensive CC list of this series which I got from
Roman's series plays a role, so I can't try just by sending to
myself... I wonder if I should send the series in chunks, the first 10
patches went out (2 times '-.- ) already...

> --
> Best regards,
> Eugeniu Rosca

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
  2020-06-23 10:49     ` Jacopo Mondi
@ 2020-06-23 12:17       ` Eugeniu Rosca
  2020-06-24  7:47         ` Jacopo Mondi
  0 siblings, 1 reply; 18+ messages in thread
From: Eugeniu Rosca @ 2020-06-23 12:17 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Eugeniu Rosca, lolivei, mchehab, sakari.ailus, hverkuil,
	laurent.pinchart, roman.kovalivskyi, dave.stevenson, naush,
	mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, linux-media, libcamera-devel, Eugeniu Rosca

Hi Jacopo,

On Tue, Jun 23, 2020 at 12:49:03PM +0200, Jacopo Mondi wrote:
> On Tue, Jun 23, 2020 at 12:30:02PM +0200, Eugeniu Rosca wrote:
> > On Mon, Jun 22, 2020 at 07:26:14PM +0200, Jacopo Mondi wrote:
> > > My ISP has rejected the rest of the series: too many emails :(
> > > Has it ever happened to anyone else ? How did you solved this ?
> >
> > I guess leaving 5-10 seconds between sending individual patches should
> > overcome this? I wonder if git provides a built-in command for that?
> >
> 
> git send-email does provide the --batch-size --relogin-delay options,
> as Ezequiel suggested me in #v4l.
> 
> I tried re-sending with a 10 email batch and a 5 seconds delay but I
> got the same failure. I was not able to find any description of the
> email number limits for the SMTP server I'm using, so I could only go
> and try. I think the extensive CC list of this series which I got from
> Roman's series plays a role, so I can't try just by sending to
> myself... I wonder if I should send the series in chunks, the first 10
> patches went out (2 times '-.- ) already...

Any time you send a new batch, please call 'git send-email' with
'--in-reply-to=<cover-letter-id>' to allow LKML front-ends identify
the patches as belonging to one topic and make it less of a pain
for people to go through these discussions later on.

-- 
Best regards,
Eugeniu Rosca

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
  2020-06-23 12:17       ` Eugeniu Rosca
@ 2020-06-24  7:47         ` Jacopo Mondi
  2020-06-24  8:04           ` Eugeniu Rosca
  0 siblings, 1 reply; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-24  7:47 UTC (permalink / raw)
  To: Eugeniu Rosca
  Cc: lolivei, mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush, mrodin, hugues.fruchet,
	mripard, aford173, sudipi, andrew_gabbasov, linux-media,
	libcamera-devel, Eugeniu Rosca

Hi Eugeniu,

On Tue, Jun 23, 2020 at 02:17:49PM +0200, Eugeniu Rosca wrote:
> Hi Jacopo,
>
> On Tue, Jun 23, 2020 at 12:49:03PM +0200, Jacopo Mondi wrote:
> > On Tue, Jun 23, 2020 at 12:30:02PM +0200, Eugeniu Rosca wrote:
> > > On Mon, Jun 22, 2020 at 07:26:14PM +0200, Jacopo Mondi wrote:
> > > > My ISP has rejected the rest of the series: too many emails :(
> > > > Has it ever happened to anyone else ? How did you solved this ?
> > >
> > > I guess leaving 5-10 seconds between sending individual patches should
> > > overcome this? I wonder if git provides a built-in command for that?
> > >
> >
> > git send-email does provide the --batch-size --relogin-delay options,
> > as Ezequiel suggested me in #v4l.
> >
> > I tried re-sending with a 10 email batch and a 5 seconds delay but I
> > got the same failure. I was not able to find any description of the
> > email number limits for the SMTP server I'm using, so I could only go
> > and try. I think the extensive CC list of this series which I got from
> > Roman's series plays a role, so I can't try just by sending to
> > myself... I wonder if I should send the series in chunks, the first 10
> > patches went out (2 times '-.- ) already...
>
> Any time you send a new batch, please call 'git send-email' with
> '--in-reply-to=<cover-letter-id>' to allow LKML front-ends identify
> the patches as belonging to one topic and make it less of a pain
> for people to go through these discussions later on.

Thanks for the suggestion, I hope I got it right ;) The series has now
been resent.

Thanks
   j

>
> --
> Best regards,
> Eugeniu Rosca

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1
  2020-06-24  7:47         ` Jacopo Mondi
@ 2020-06-24  8:04           ` Eugeniu Rosca
  0 siblings, 0 replies; 18+ messages in thread
From: Eugeniu Rosca @ 2020-06-24  8:04 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Eugeniu Rosca, lolivei, mchehab, sakari.ailus, hverkuil,
	laurent.pinchart, roman.kovalivskyi, dave.stevenson, naush,
	mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, linux-media, libcamera-devel, Eugeniu Rosca

Hi Jacopo,

On Wed, Jun 24, 2020 at 09:47:49AM +0200, Jacopo Mondi wrote:
> On Tue, Jun 23, 2020 at 02:17:49PM +0200, Eugeniu Rosca wrote:
> > Hi Jacopo,
> >
> > On Tue, Jun 23, 2020 at 12:49:03PM +0200, Jacopo Mondi wrote:
> > > On Tue, Jun 23, 2020 at 12:30:02PM +0200, Eugeniu Rosca wrote:
> > > > On Mon, Jun 22, 2020 at 07:26:14PM +0200, Jacopo Mondi wrote:
> > > > > My ISP has rejected the rest of the series: too many emails :(
> > > > > Has it ever happened to anyone else ? How did you solved this ?
> > > >
> > > > I guess leaving 5-10 seconds between sending individual patches should
> > > > overcome this? I wonder if git provides a built-in command for that?
> > > >
> > >
> > > git send-email does provide the --batch-size --relogin-delay options,
> > > as Ezequiel suggested me in #v4l.
> > >
> > > I tried re-sending with a 10 email batch and a 5 seconds delay but I
> > > got the same failure. I was not able to find any description of the
> > > email number limits for the SMTP server I'm using, so I could only go
> > > and try. I think the extensive CC list of this series which I got from
> > > Roman's series plays a role, so I can't try just by sending to
> > > myself... I wonder if I should send the series in chunks, the first 10
> > > patches went out (2 times '-.- ) already...
> >
> > Any time you send a new batch, please call 'git send-email' with
> > '--in-reply-to=<cover-letter-id>' to allow LKML front-ends identify
> > the patches as belonging to one topic and make it less of a pain
> > for people to go through these discussions later on.
> 
> Thanks for the suggestion, I hope I got it right ;) The series has now
> been resent.

That's right. I now see all 25 patches appearing under the same umbrella
of https://patchwork.linuxtv.org/cover/64822/ (via "Related"), even if
the last 15 have been sent out at a later point in time.

-- 
Best regards,
Eugeniu Rosca

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 04/25] media: ov5647: Add support for non-continuous clock mode
  2020-06-23 10:07 Jacopo Mondi
@ 2020-06-23 10:07 ` Jacopo Mondi
  0 siblings, 0 replies; 18+ messages in thread
From: Jacopo Mondi @ 2020-06-23 10:07 UTC (permalink / raw)
  To: mchehab, sakari.ailus, hverkuil, laurent.pinchart,
	roman.kovalivskyi, dave.stevenson, naush
  Cc: Jacopo Mondi, mrodin, hugues.fruchet, mripard, aford173, sudipi,
	andrew_gabbasov, erosca, linux-media, libcamera-devel

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Add support for optional non-continuous clock mode to the ov5647
sensor driver.

Non-continuous clock saves a small amount of power and on some SoCs
is easier to interface with.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 105ff7f899b34..2d69cd97142d7 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -44,6 +44,7 @@
 #define PWDN_ACTIVE_DELAY_MS	20
 
 #define MIPI_CTRL00_CLOCK_LANE_GATE		BIT(5)
+#define MIPI_CTRL00_LINE_SYNC_ENABLE		BIT(4)
 #define MIPI_CTRL00_BUS_IDLE			BIT(2)
 #define MIPI_CTRL00_CLOCK_LANE_DISABLE		BIT(0)
 
@@ -95,6 +96,7 @@ struct ov5647 {
 	int				power_count;
 	struct clk			*xclk;
 	struct gpio_desc		*pwdn;
+	bool				clock_ncont;
 };
 
 static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
@@ -269,9 +271,15 @@ static int ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
 
 static int ov5647_stream_on(struct v4l2_subdev *sd)
 {
+	struct ov5647 *ov5647 = to_state(sd);
+	u8 val = MIPI_CTRL00_BUS_IDLE;
 	int ret;
 
-	ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, MIPI_CTRL00_BUS_IDLE);
+	if (ov5647->clock_ncont)
+		val |= MIPI_CTRL00_CLOCK_LANE_GATE |
+		       MIPI_CTRL00_LINE_SYNC_ENABLE;
+
+	ret = ov5647_write(sd, OV5647_REG_MIPI_CTRL00, val);
 	if (ret < 0)
 		return ret;
 
@@ -546,9 +554,11 @@ static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
 	.open = ov5647_open,
 };
 
-static int ov5647_parse_dt(struct device_node *np)
+static int ov5647_parse_dt(struct ov5647 *sensor, struct device_node *np)
 {
-	struct v4l2_fwnode_endpoint bus_cfg = { .bus_type = 0 };
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY,
+	};
 	struct device_node *ep;
 
 	int ret;
@@ -558,7 +568,13 @@ static int ov5647_parse_dt(struct device_node *np)
 		return -EINVAL;
 
 	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &bus_cfg);
+	if (ret)
+		goto out;
 
+	sensor->clock_ncont = bus_cfg.bus.mipi_csi2.flags &
+			      V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
+
+out:
 	of_node_put(ep);
 	return ret;
 }
@@ -577,7 +593,7 @@ static int ov5647_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	if (IS_ENABLED(CONFIG_OF) && np) {
-		ret = ov5647_parse_dt(np);
+		ret = ov5647_parse_dt(sensor, np);
 		if (ret) {
 			dev_err(dev, "DT parsing error: %d\n", ret);
 			return ret;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-06-24  8:05 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 17:18 [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
2020-06-22 17:18 ` [PATCH 01/25] dt-bindings: media: ov5647: Document pwdn-gpios Jacopo Mondi
2020-06-22 17:18 ` [PATCH 02/25] dt-bindings: media: ov5647: Document clock-noncontinuous Jacopo Mondi
2020-06-22 17:18 ` [PATCH 03/25] media: ov5647: Add support for PWDN GPIO Jacopo Mondi
2020-06-22 17:18 ` [PATCH 04/25] media: ov5647: Add support for non-continuous clock mode Jacopo Mondi
2020-06-22 17:18 ` [PATCH 05/25] media: ov5647: Add set_fmt and get_fmt calls Jacopo Mondi
2020-06-22 17:18 ` [PATCH 06/25] media: ov5647: Fix format initialization Jacopo Mondi
2020-06-22 17:18 ` [PATCH 07/25] media: ov5647: Fix style issues Jacopo Mondi
2020-06-22 17:18 ` [PATCH 08/25] media: ov5647: Replace license with SPDX identifier Jacopo Mondi
2020-06-22 17:18 ` [PATCH 09/25] media: ov5647: Fix return value from read/write Jacopo Mondi
2020-06-22 17:18 ` [PATCH 10/25] media: ov5647: Program mode at s_stream(1) time Jacopo Mondi
2020-06-22 17:26 ` [PATCH 00/25] media: ov5647: Support RaspberryPi Camera Module v1 Jacopo Mondi
2020-06-23 10:30   ` Eugeniu Rosca
2020-06-23 10:49     ` Jacopo Mondi
2020-06-23 12:17       ` Eugeniu Rosca
2020-06-24  7:47         ` Jacopo Mondi
2020-06-24  8:04           ` Eugeniu Rosca
2020-06-23 10:07 Jacopo Mondi
2020-06-23 10:07 ` [PATCH 04/25] media: ov5647: Add support for non-continuous clock mode Jacopo Mondi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).