linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75
@ 2018-11-05  7:30 Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 01/11] media: ov7670: hook s_power onto v4l2 core Lubomir Rintel
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

Hi,

this patch set somewhat modernizes the Marvel MMP2 CCIC driver. Notably,
it ports it from the platform data (which seems unused as the board
support code never made it) to devicetree.

At the core of the rework is the move to asynchronous sensor discovery
and clock management with the standard clock framework. There are also
some straightforward fixes for bitrotten parts.

There's probably still room for improvement, but as it is, it seems to
work well on OLPC XO-1.75 and doesn't break OLPC XO-1 (I've tested on
both platforms).

Cheers,
Lubo



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

* [PATCH 01/11] media: ov7670: hook s_power onto v4l2 core
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 02/11] media: ov7670: control clock along with power Lubomir Rintel
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

The commit 71862f63f351 ("media: ov7670: Add the ov7670_s_power function")
added a s_power function. For some reason it didn't register it with v4l2,
only uses it internally. Fix this now.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/media/i2c/ov7670.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index bc68a3a5b4ec..d87f2362bf40 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1651,6 +1651,7 @@ static int ov7670_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 static const struct v4l2_subdev_core_ops ov7670_core_ops = {
 	.reset = ov7670_reset,
 	.init = ov7670_init,
+	.s_power = ov7670_s_power,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register = ov7670_g_register,
 	.s_register = ov7670_s_register,
-- 
2.19.1


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

* [PATCH 02/11] media: ov7670: control clock along with power
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 01/11] media: ov7670: hook s_power onto v4l2 core Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 03/11] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera Lubomir Rintel
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

This provides more power saving when the sensor is off.

While at that, do the delay on power/clock enable even if the sensor driver
itself doesn't control the GPIOs. This is required for the OLPC XO-1
platform, that lacks the proper power/reset properties in its DT, but
needs the delay after the sensor is clocked up.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/media/i2c/ov7670.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index d87f2362bf40..a3e72c62382c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -241,6 +241,7 @@ struct ov7670_info {
 	struct v4l2_mbus_framefmt format;
 	struct ov7670_format_struct *fmt;  /* Current format */
 	struct clk *clk;
+	int on;
 	struct gpio_desc *resetb_gpio;
 	struct gpio_desc *pwdn_gpio;
 	unsigned int mbus_config;	/* Media bus configuration flags */
@@ -1610,15 +1611,26 @@ static int ov7670_s_power(struct v4l2_subdev *sd, int on)
 {
 	struct ov7670_info *info = to_state(sd);
 
+	if (info->on == on)
+		return 0;
+
+	if (on)
+		clk_prepare_enable(info->clk);
+	else
+		clk_disable_unprepare(info->clk);
+
 	if (info->pwdn_gpio)
 		gpiod_set_value(info->pwdn_gpio, !on);
 	if (on && info->resetb_gpio) {
 		gpiod_set_value(info->resetb_gpio, 1);
 		usleep_range(500, 1000);
 		gpiod_set_value(info->resetb_gpio, 0);
-		usleep_range(3000, 5000);
 	}
 
+	if (on && (info->pwdn_gpio || info->resetb_gpio || info->clk))
+		usleep_range(3000, 5000);
+
+	info->on = on;
 	return 0;
 }
 
@@ -1817,24 +1829,20 @@ static int ov7670_probe(struct i2c_client *client,
 		else
 			return ret;
 	}
-	if (info->clk) {
-		ret = clk_prepare_enable(info->clk);
-		if (ret)
-			return ret;
+	ret = ov7670_init_gpio(client, info);
+	if (ret)
+		return ret;
+
+	ov7670_s_power(sd, 1);
 
+	if (info->clk) {
 		info->clock_speed = clk_get_rate(info->clk) / 1000000;
 		if (info->clock_speed < 10 || info->clock_speed > 48) {
 			ret = -EINVAL;
-			goto clk_disable;
+			goto power_off;
 		}
 	}
 
-	ret = ov7670_init_gpio(client, info);
-	if (ret)
-		goto clk_disable;
-
-	ov7670_s_power(sd, 1);
-
 	/* Make sure it's an ov7670 */
 	ret = ov7670_detect(sd);
 	if (ret) {
@@ -1913,6 +1921,7 @@ static int ov7670_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto entity_cleanup;
 
+	ov7670_s_power(sd, 0);
 	return 0;
 
 entity_cleanup:
@@ -1921,12 +1930,9 @@ static int ov7670_probe(struct i2c_client *client,
 	v4l2_ctrl_handler_free(&info->hdl);
 power_off:
 	ov7670_s_power(sd, 0);
-clk_disable:
-	clk_disable_unprepare(info->clk);
 	return ret;
 }
 
-
 static int ov7670_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1934,7 +1940,6 @@ static int ov7670_remove(struct i2c_client *client)
 
 	v4l2_async_unregister_subdev(sd);
 	v4l2_ctrl_handler_free(&info->hdl);
-	clk_disable_unprepare(info->clk);
 	media_entity_cleanup(&info->sd.entity);
 	ov7670_s_power(sd, 0);
 	return 0;
-- 
2.19.1


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

* [PATCH 03/11] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 01/11] media: ov7670: hook s_power onto v4l2 core Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 02/11] media: ov7670: control clock along with power Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 04/11] [media] marvell-ccic: fix DMA s/g desc number calculation Lubomir Rintel
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

Add Marvell MMP2 camera host interface.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../bindings/media/marvell,mmp2-ccic.txt      | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt

diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
new file mode 100644
index 000000000000..a9c536e58dda
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
@@ -0,0 +1,30 @@
+Marvell MMP2 camera host interface
+
+Required properties:
+ - compatible: Should be "marvell,mmp2-ccic"
+ - reg: register base and size
+ - interrupts: the interrupt number
+ - any required generic properties defined in video-interfaces.txt
+
+Optional properties:
+ - clocks: input clock (see clock-bindings.txt)
+ - clock-output-names: should contain the name of the clock driving the
+                       sensor master clock MCLK
+
+Example:
+
+	camera0: camera@d420a000 {
+		compatible = "marvell,mmp2-ccic";
+		reg = <0xd420a000 0x800>;
+		interrupts = <42>;
+		clocks = <&soc_clocks MMP2_CLK_CCIC0>;
+		clock-names = "CCICAXICLK";
+		#clock-cells = <0>;
+		clock-output-names = "mclk";
+
+		port {
+			camera0_0: endpoint {
+				remote-endpoint = <&ov7670_0>;
+			};
+		};
+	};
-- 
2.19.1


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

* [PATCH 04/11] [media] marvell-ccic: fix DMA s/g desc number calculation
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (2 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 03/11] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus Lubomir Rintel
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

The commit d790b7eda953 ("[media] vb2-dma-sg: move dma_(un)map_sg here")
left dma_desc_nent unset. It previously contained the number of DMA
descriptors as returned from dma_map_sg().

We can now (since the commit referred to above) obtain the same value from
the sg_table and drop dma_desc_nent altogether.

Tested on OLPC XO-1.75 machine. Doesn't affect the OLPC XO-1's Cafe
driver, since that one doesn't do DMA.

Fixes: d790b7eda953df474f470169ebdf111c02fa7a2d
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/media/platform/marvell-ccic/mcam-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index f1b301810260..d97f39bde9bd 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -200,7 +200,6 @@ struct mcam_vb_buffer {
 	struct list_head queue;
 	struct mcam_dma_desc *dma_desc;	/* Descriptor virtual address */
 	dma_addr_t dma_desc_pa;		/* Descriptor physical address */
-	int dma_desc_nent;		/* Number of mapped descriptors */
 };
 
 static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_v4l2_buffer *vb)
@@ -608,9 +607,11 @@ static void mcam_dma_contig_done(struct mcam_camera *cam, int frame)
 static void mcam_sg_next_buffer(struct mcam_camera *cam)
 {
 	struct mcam_vb_buffer *buf;
+	struct sg_table *sg_table;
 
 	buf = list_first_entry(&cam->buffers, struct mcam_vb_buffer, queue);
 	list_del_init(&buf->queue);
+	sg_table = vb2_dma_sg_plane_desc(&buf->vb_buf.vb2_buf, 0);
 	/*
 	 * Very Bad Not Good Things happen if you don't clear
 	 * C1_DESC_ENA before making any descriptor changes.
@@ -618,7 +619,7 @@ static void mcam_sg_next_buffer(struct mcam_camera *cam)
 	mcam_reg_clear_bit(cam, REG_CTRL1, C1_DESC_ENA);
 	mcam_reg_write(cam, REG_DMA_DESC_Y, buf->dma_desc_pa);
 	mcam_reg_write(cam, REG_DESC_LEN_Y,
-			buf->dma_desc_nent*sizeof(struct mcam_dma_desc));
+			sg_table->nents*sizeof(struct mcam_dma_desc));
 	mcam_reg_write(cam, REG_DESC_LEN_U, 0);
 	mcam_reg_write(cam, REG_DESC_LEN_V, 0);
 	mcam_reg_set_bit(cam, REG_CTRL1, C1_DESC_ENA);
-- 
2.19.1


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

* [PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (3 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 04/11] [media] marvell-ccic: fix DMA s/g desc number calculation Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  9:07   ` Pavel Machek
  2018-11-05  7:30 ` [PATCH 06/11] [media] marvell-ccic: drop ctlr_reset() Lubomir Rintel
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

The commit 05fed81625bf ("[media] marvell-ccic: add MIPI support for
marvell-ccic driver") that claimed to add CSI2 turned on C0_EOF_VSYNC for
parallel bus without a very good explanation.

That broke camera on OLPC XO-1.75 which precisely uses a sensor on a
parallel bus. Revert that chunk.

Tested on an OLPC XO-1.75.

Fixes: 05fed81625bf755cc67c5864cdfd18b69ea828d1
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/media/platform/marvell-ccic/mcam-core.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index d97f39bde9bd..d24e5b7a3bc5 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -792,12 +792,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 	 * Make sure it knows we want to use hsync/vsync.
 	 */
 	mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK);
-	/*
-	 * This field controls the generation of EOF(DVP only)
-	 */
-	if (cam->bus_type != V4L2_MBUS_CSI2_DPHY)
-		mcam_reg_set_bit(cam, REG_CTRL0,
-				C0_EOF_VSYNC | C0_VEDGE_CTRL);
 }
 
 
-- 
2.19.1


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

* [PATCH 06/11] [media] marvell-ccic: drop ctlr_reset()
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (4 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  9:11   ` Pavel Machek
  2018-11-05  7:30 ` [PATCH 07/11] [media] marvell-ccic: drop unused stuff Lubomir Rintel
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

This accesses the clock registers directly and thus is not too
devicetree friendly. If it's actually needed it needs to be done
differently.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../media/platform/marvell-ccic/mcam-core.c   |  6 -----
 .../media/platform/marvell-ccic/mcam-core.h   |  1 -
 .../media/platform/marvell-ccic/mmp-driver.c  | 23 -------------------
 3 files changed, 30 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index d24e5b7a3bc5..1b879035948c 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1154,12 +1154,6 @@ static void mcam_vb_stop_streaming(struct vb2_queue *vq)
 	if (cam->state != S_STREAMING)
 		return;
 	mcam_ctlr_stop_dma(cam);
-	/*
-	 * Reset the CCIC PHY after stopping streaming,
-	 * otherwise, the CCIC may be unstable.
-	 */
-	if (cam->ctlr_reset)
-		cam->ctlr_reset(cam);
 	/*
 	 * VB2 reclaims the buffers, so we need to forget
 	 * about them.
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index ad8955f9f0a1..f93f23faf364 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -137,7 +137,6 @@ struct mcam_camera {
 	int (*plat_power_up) (struct mcam_camera *cam);
 	void (*plat_power_down) (struct mcam_camera *cam);
 	void (*calc_dphy) (struct mcam_camera *cam);
-	void (*ctlr_reset) (struct mcam_camera *cam);
 
 	/*
 	 * Everything below here is private to the mcam core and
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 70a2833db0d1..92a79ad8a12c 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -183,28 +183,6 @@ static void mmpcam_power_down(struct mcam_camera *mcam)
 	mcam_clk_disable(mcam);
 }
 
-static void mcam_ctlr_reset(struct mcam_camera *mcam)
-{
-	unsigned long val;
-	struct mmp_camera *cam = mcam_to_cam(mcam);
-
-	if (mcam->ccic_id) {
-		/*
-		 * Using CCIC2
-		 */
-		val = ioread32(cam->power_regs + REG_CCIC2_CRCR);
-		iowrite32(val & ~0x2, cam->power_regs + REG_CCIC2_CRCR);
-		iowrite32(val | 0x2, cam->power_regs + REG_CCIC2_CRCR);
-	} else {
-		/*
-		 * Using CCIC1
-		 */
-		val = ioread32(cam->power_regs + REG_CCIC_CRCR);
-		iowrite32(val & ~0x2, cam->power_regs + REG_CCIC_CRCR);
-		iowrite32(val | 0x2, cam->power_regs + REG_CCIC_CRCR);
-	}
-}
-
 /*
  * calc the dphy register values
  * There are three dphy registers being used.
@@ -352,7 +330,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	mcam = &cam->mcam;
 	mcam->plat_power_up = mmpcam_power_up;
 	mcam->plat_power_down = mmpcam_power_down;
-	mcam->ctlr_reset = mcam_ctlr_reset;
 	mcam->calc_dphy = mmpcam_calc_dphy;
 	mcam->dev = &pdev->dev;
 	mcam->use_smbus = 0;
-- 
2.19.1


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

* [PATCH 07/11] [media] marvell-ccic: drop unused stuff
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (5 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 06/11] [media] marvell-ccic: drop ctlr_reset() Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  9:17   ` Pavel Machek
  2018-11-05  7:30 ` [PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers Lubomir Rintel
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

Remove structure members and headers that are not actually used. Saves
us from some noise in subsequent cleanup commits.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/media/platform/marvell-ccic/mcam-core.c  | 1 -
 drivers/media/platform/marvell-ccic/mcam-core.h  | 3 ---
 drivers/media/platform/marvell-ccic/mmp-driver.c | 3 ---
 include/linux/platform_data/media/mmp-camera.h   | 1 -
 4 files changed, 8 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 1b879035948c..0113b8d37d03 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1776,7 +1776,6 @@ int mccic_register(struct mcam_camera *cam)
 	 */
 	sensor_cfg.clock_speed = cam->clock_speed;
 	sensor_cfg.use_smbus = cam->use_smbus;
-	cam->sensor_addr = ov7670_info.addr;
 	cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
 			cam->i2c_adapter, &ov7670_info, NULL);
 	if (cam->sensor == NULL) {
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index f93f23faf364..b828b1bb59d3 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -112,11 +112,9 @@ struct mcam_camera {
 	short int use_smbus;	/* SMBUS or straight I2c? */
 	enum mcam_buffer_mode buffer_mode;
 
-	int mclk_min;	/* The minimal value of mclk */
 	int mclk_src;	/* which clock source the mclk derives from */
 	int mclk_div;	/* Clock Divider Value for MCLK */
 
-	int ccic_id;
 	enum v4l2_mbus_type bus_type;
 	/* MIPI support */
 	/* The dphy config value, allocated in board file
@@ -153,7 +151,6 @@ struct mcam_camera {
 	 */
 	struct video_device vdev;
 	struct v4l2_subdev *sensor;
-	unsigned short sensor_addr;
 
 	/* Videobuf2 stuff */
 	struct vb2_queue vb_queue;
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 92a79ad8a12c..9e988e527b0d 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -12,7 +12,6 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
-#include <linux/platform_data/i2c-gpio.h>
 #include <linux/interrupt.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
@@ -333,8 +332,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	mcam->calc_dphy = mmpcam_calc_dphy;
 	mcam->dev = &pdev->dev;
 	mcam->use_smbus = 0;
-	mcam->ccic_id = pdev->id;
-	mcam->mclk_min = pdata->mclk_min;
 	mcam->mclk_src = pdata->mclk_src;
 	mcam->mclk_div = pdata->mclk_div;
 	mcam->bus_type = pdata->bus_type;
diff --git a/include/linux/platform_data/media/mmp-camera.h b/include/linux/platform_data/media/mmp-camera.h
index d2d3a443eedf..4c3a80a45883 100644
--- a/include/linux/platform_data/media/mmp-camera.h
+++ b/include/linux/platform_data/media/mmp-camera.h
@@ -16,7 +16,6 @@ struct mmp_camera_platform_data {
 	int sensor_power_gpio;
 	int sensor_reset_gpio;
 	enum v4l2_mbus_type bus_type;
-	int mclk_min;	/* The minimal value of MCLK */
 	int mclk_src;	/* which clock source the MCLK derives from */
 	int mclk_div;	/* Clock Divider Value for MCLK */
 	/*
-- 
2.19.1


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

* [PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (6 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 07/11] [media] marvell-ccic: drop unused stuff Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  9:17   ` Pavel Machek
  2018-11-05  7:30 ` [PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support Lubomir Rintel
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

The access to REG_CLKCTRL or REG_CTRL1 without the clock enabled hangs
the machine. Enable the clock first.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/media/platform/marvell-ccic/mmp-driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 9e988e527b0d..9c0238f72c40 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -145,6 +145,7 @@ static int mmpcam_power_up(struct mcam_camera *mcam)
  * Turn on power and clocks to the controller.
  */
 	mmpcam_power_up_ctlr(cam);
+	mcam_clk_enable(mcam);
 /*
  * Provide power to the sensor.
  */
@@ -158,8 +159,6 @@ static int mmpcam_power_up(struct mcam_camera *mcam)
 	gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */
 	mdelay(5);
 
-	mcam_clk_enable(mcam);
-
 	return 0;
 }
 
-- 
2.19.1


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

* [PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (7 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  9:20   ` Pavel Machek
  2018-11-05  7:30 ` [PATCH 10/11] [media] marvell-ccic: use async notifier to get the sensor Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 11/11] [media] marvell-ccic: provide a clock for " Lubomir Rintel
  10 siblings, 1 reply; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

The platform data is actually not used anywhere (along with the CSI
support) and should be safe to remove.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../media/platform/marvell-ccic/mmp-driver.c  | 36 ++++++++++++++-----
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index 9c0238f72c40..ff1feca7bc9b 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -19,6 +19,8 @@
 #include <media/v4l2-device.h>
 #include <linux/platform_data/media/mmp-camera.h>
 #include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
 #include <linux/io.h>
@@ -197,6 +199,9 @@ static void mmpcam_calc_dphy(struct mcam_camera *mcam)
 	struct device *dev = &cam->pdev->dev;
 	unsigned long tx_clk_esc;
 
+	if (!pdata)
+		return;
+
 	/*
 	 * If CSI2_DPHY3 is calculated dynamically,
 	 * pdata->lane_clk should be already set
@@ -315,10 +320,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	struct mmp_camera_platform_data *pdata;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata)
-		return -ENODEV;
-
 	cam = devm_kzalloc(&pdev->dev, sizeof(*cam), GFP_KERNEL);
 	if (cam == NULL)
 		return -ENOMEM;
@@ -331,17 +332,29 @@ static int mmpcam_probe(struct platform_device *pdev)
 	mcam->calc_dphy = mmpcam_calc_dphy;
 	mcam->dev = &pdev->dev;
 	mcam->use_smbus = 0;
-	mcam->mclk_src = pdata->mclk_src;
-	mcam->mclk_div = pdata->mclk_div;
-	mcam->bus_type = pdata->bus_type;
-	mcam->dphy = pdata->dphy;
+	pdata = pdev->dev.platform_data;
+	if (pdata) {
+		mcam->mclk_src = pdata->mclk_src;
+		mcam->mclk_div = pdata->mclk_div;
+		mcam->bus_type = pdata->bus_type;
+		mcam->dphy = pdata->dphy;
+		mcam->lane = pdata->lane;
+	} else {
+		/*
+		 * This are values that used to be hardcoded in mcam-core and
+		 * work well on a OLPC XO 1.75 with a parallel bus sensor.
+		 * If it turns out other setups make sense, the values should
+		 * be obtained from the device tree.
+		 */
+		mcam->mclk_src = 3;
+		mcam->mclk_div = 2;
+	}
 	if (mcam->bus_type == V4L2_MBUS_CSI2_DPHY) {
 		cam->mipi_clk = devm_clk_get(mcam->dev, "mipi");
 		if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0))
 			return PTR_ERR(cam->mipi_clk);
 	}
 	mcam->mipi_enabled = false;
-	mcam->lane = pdata->lane;
 	mcam->chip_id = MCAM_ARMADA610;
 	mcam->buffer_mode = B_DMA_sg;
 	strscpy(mcam->bus_info, "platform:mmp-camera", sizeof(mcam->bus_info));
@@ -476,6 +489,10 @@ static int mmpcam_resume(struct platform_device *pdev)
 
 #endif
 
+static const struct of_device_id mmpcam_of_match[] = {
+	{ .compatible = "marvell,mmp2-ccic", },
+	{},
+};
 
 static struct platform_driver mmpcam_driver = {
 	.probe		= mmpcam_probe,
@@ -486,6 +503,7 @@ static struct platform_driver mmpcam_driver = {
 #endif
 	.driver = {
 		.name	= "mmp-camera",
+		.of_match_table = of_match_ptr(mmpcam_of_match),
 	}
 };
 
-- 
2.19.1


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

* [PATCH 10/11] [media] marvell-ccic: use async notifier to get the sensor
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (8 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05  7:30 ` [PATCH 11/11] [media] marvell-ccic: provide a clock for " Lubomir Rintel
  10 siblings, 0 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

An instance of a sensor on DT-based MMP2 platform is always going to be
created asynchronously.

Let's move the manual device creation away from the core to the Cafe
driver (used on OLPC XO-1, not present in DT) and set up appropriate
async matches: I2C on Cafe, FWNODE on MMP (OLPC XO-1.75).

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../media/platform/marvell-ccic/cafe-driver.c |  49 ++++--
 .../media/platform/marvell-ccic/mcam-core.c   | 157 ++++++++++++------
 .../media/platform/marvell-ccic/mcam-core.h   |   5 +-
 .../media/platform/marvell-ccic/mmp-driver.c  |  27 +--
 .../linux/platform_data/media/mmp-camera.h    |   1 -
 5 files changed, 162 insertions(+), 77 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 2986cb4b88d0..0164afc405d1 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -8,6 +8,7 @@
  *
  * Copyright 2006-11 One Laptop Per Child Association, Inc.
  * Copyright 2006-11 Jonathan Corbet <corbet@lwn.net>
+ * Copyright 2018 Lubomir Rintel <lkundrak@v3.sk>
  *
  * Written by Jonathan Corbet, corbet@lwn.net.
  *
@@ -27,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-device.h>
+#include <media/i2c/ov7670.h>
 #include <linux/device.h>
 #include <linux/wait.h>
 #include <linux/delay.h>
@@ -52,6 +54,7 @@ struct cafe_camera {
 	int registered;			/* Fully initialized? */
 	struct mcam_camera mcam;
 	struct pci_dev *pdev;
+	struct i2c_adapter *i2c_adapter;
 	wait_queue_head_t smbus_wait;	/* Waiting on i2c events */
 };
 
@@ -351,15 +354,15 @@ static int cafe_smbus_setup(struct cafe_camera *cam)
 		return ret;
 	}
 
-	cam->mcam.i2c_adapter = adap;
+	cam->i2c_adapter = adap;
 	cafe_smbus_enable_irq(cam);
 	return 0;
 }
 
 static void cafe_smbus_shutdown(struct cafe_camera *cam)
 {
-	i2c_del_adapter(cam->mcam.i2c_adapter);
-	kfree(cam->mcam.i2c_adapter);
+	i2c_del_adapter(cam->i2c_adapter);
+	kfree(cam->i2c_adapter);
 }
 
 
@@ -452,6 +455,29 @@ static irqreturn_t cafe_irq(int irq, void *data)
 	return IRQ_RETVAL(handled);
 }
 
+/* -------------------------------------------------------------------------- */
+
+static struct ov7670_config sensor_cfg = {
+	/*
+	 * Exclude QCIF mode, because it only captures a tiny portion
+	 * of the sensor FOV
+	 */
+	.min_width = 320,
+	.min_height = 240,
+
+	/*
+	 * Set the clock speed for the XO 1; I don't believe this
+	 * driver has ever run anywhere else.
+	 */
+	.clock_speed = 45,
+	.use_smbus = 1,
+};
+
+struct i2c_board_info ov7670_info = {
+	.type = "ov7670",
+	.addr = 0x42 >> 1,
+	.platform_data = &sensor_cfg,
+};
 
 /* -------------------------------------------------------------------------- */
 /*
@@ -481,12 +507,6 @@ static int cafe_pci_probe(struct pci_dev *pdev,
 	mcam->plat_power_down = cafe_ctlr_power_down;
 	mcam->dev = &pdev->dev;
 	snprintf(mcam->bus_info, sizeof(mcam->bus_info), "PCI:%s", pci_name(pdev));
-	/*
-	 * Set the clock speed for the XO 1; I don't believe this
-	 * driver has ever run anywhere else.
-	 */
-	mcam->clock_speed = 45;
-	mcam->use_smbus = 1;
 	/*
 	 * Vmalloc mode for buffers is traditional with this driver.
 	 * We *might* be able to run DMA_contig, especially on a system
@@ -527,12 +547,21 @@ static int cafe_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto out_pdown;
 
+	mcam->asd.match_type = V4L2_ASYNC_MATCH_I2C;
+	mcam->asd.match.i2c.adapter_id = i2c_adapter_id(cam->i2c_adapter);
+	mcam->asd.match.i2c.address = ov7670_info.addr;
+
 	ret = mccic_register(mcam);
-	if (ret == 0) {
+	if (ret)
+		goto out_smbus_shutdown;
+
+	if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
 		cam->registered = 1;
 		return 0;
 	}
 
+	cafe_shutdown(cam);
+out_smbus_shutdown:
 	cafe_smbus_shutdown(cam);
 out_pdown:
 	cafe_ctlr_power_down(mcam);
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 0113b8d37d03..87812b7287f0 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -4,6 +4,7 @@
  * so it needs platform-specific support outside of the core.
  *
  * Copyright 2011 Jonathan Corbet corbet@lwn.net
+ * Copyright 2018 Lubomir Rintel <lkundrak@v3.sk>
  */
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -26,7 +27,6 @@
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-event.h>
-#include <media/i2c/ov7670.h>
 #include <media/videobuf2-vmalloc.h>
 #include <media/videobuf2-dma-contig.h>
 #include <media/videobuf2-dma-sg.h>
@@ -93,6 +93,9 @@ MODULE_PARM_DESC(buffer_mode,
 #define sensor_call(cam, o, f, args...) \
 	v4l2_subdev_call(cam->sensor, o, f, ##args)
 
+#define notifier_to_mcam(notifier) \
+	container_of(notifier, struct mcam_camera, notifier)
+
 static struct mcam_format_struct {
 	__u8 *desc;
 	__u32 pixelformat;
@@ -1715,23 +1718,94 @@ EXPORT_SYMBOL_GPL(mccic_irq);
 /*
  * Registration and such.
  */
-static struct ov7670_config sensor_cfg = {
+
+static int mccic_notify_bound(struct v4l2_async_notifier *notifier,
+	struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd)
+{
+	struct mcam_camera *cam = notifier_to_mcam(notifier);
+	int ret;
+
+	mutex_lock(&cam->s_mutex);
+	if (cam->sensor) {
+		cam_err(cam, "sensor already bound\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
+	v4l2_set_subdev_hostdata(subdev, cam);
+	cam->sensor = subdev;
+
+	ret = mcam_cam_init(cam);
+	if (ret) {
+		cam->sensor = NULL;
+		goto out;
+	}
+
+	ret = mcam_setup_vb2(cam);
+	if (ret) {
+		cam->sensor = NULL;
+		goto out;
+	}
+
+	cam->vdev = mcam_v4l_template;
+	cam->vdev.v4l2_dev = &cam->v4l2_dev;
+	cam->vdev.lock = &cam->s_mutex;
+	cam->vdev.queue = &cam->vb_queue;
+	video_set_drvdata(&cam->vdev, cam);
+	ret = video_register_device(&cam->vdev, VFL_TYPE_GRABBER, -1);
+	if (ret) {
+		cam->sensor = NULL;
+		goto out;
+	}
+
+	cam_dbg(cam, "sensor %s bound\n", subdev->name);
+out:
+	mutex_unlock(&cam->s_mutex);
+	return ret;
+}
+
+static void mccic_notify_unbind(struct v4l2_async_notifier *notifier,
+	struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd)
+{
+	struct mcam_camera *cam = notifier_to_mcam(notifier);
+
+	mutex_lock(&cam->s_mutex);
+	if (cam->sensor != subdev) {
+		cam_err(cam, "sensor %s not bound\n", subdev->name);
+		goto out;
+	}
+
+	video_unregister_device(&cam->vdev);
+	cam->sensor = NULL;
+	cam_dbg(cam, "sensor %s unbound\n", subdev->name);
+
+out:
+	mutex_unlock(&cam->s_mutex);
+}
+
+static int mccic_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct mcam_camera *cam = notifier_to_mcam(notifier);
+	int ret;
+
 	/*
-	 * Exclude QCIF mode, because it only captures a tiny portion
-	 * of the sensor FOV
+	 * Get the v4l2 setup done.
 	 */
-	.min_width = 320,
-	.min_height = 240,
-};
+	ret = v4l2_ctrl_handler_init(&cam->ctrl_handler, 10);
+	if (!ret)
+		cam->v4l2_dev.ctrl_handler = &cam->ctrl_handler;
+
+	return ret;
+}
 
+static const struct v4l2_async_notifier_operations mccic_notify_ops = {
+	.bound = mccic_notify_bound,
+	.unbind = mccic_notify_unbind,
+	.complete = mccic_notify_complete,
+};
 
 int mccic_register(struct mcam_camera *cam)
 {
-	struct i2c_board_info ov7670_info = {
-		.type = "ov7670",
-		.addr = 0x42 >> 1,
-		.platform_data = &sensor_cfg,
-	};
 	int ret;
 
 	/*
@@ -1744,17 +1818,20 @@ int mccic_register(struct mcam_camera *cam)
 		printk(KERN_ERR "marvell-cam: Cafe can't do S/G I/O, attempting vmalloc mode instead\n");
 		cam->buffer_mode = B_vmalloc;
 	}
+
 	if (!mcam_buffer_mode_supported(cam->buffer_mode)) {
 		printk(KERN_ERR "marvell-cam: buffer mode %d unsupported\n",
 				cam->buffer_mode);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
+
 	/*
 	 * Register with V4L
 	 */
 	ret = v4l2_device_register(cam->dev, &cam->v4l2_dev);
 	if (ret)
-		return ret;
+		goto out;
 
 	mutex_init(&cam->s_mutex);
 	cam->state = S_NOTREADY;
@@ -1764,43 +1841,20 @@ int mccic_register(struct mcam_camera *cam)
 	mcam_ctlr_init(cam);
 
 	/*
-	 * Get the v4l2 setup done.
+	 * Register sensor notifier.
 	 */
-	ret = v4l2_ctrl_handler_init(&cam->ctrl_handler, 10);
-	if (ret)
-		goto out_unregister;
-	cam->v4l2_dev.ctrl_handler = &cam->ctrl_handler;
-
-	/*
-	 * Try to find the sensor.
-	 */
-	sensor_cfg.clock_speed = cam->clock_speed;
-	sensor_cfg.use_smbus = cam->use_smbus;
-	cam->sensor = v4l2_i2c_new_subdev_board(&cam->v4l2_dev,
-			cam->i2c_adapter, &ov7670_info, NULL);
-	if (cam->sensor == NULL) {
-		ret = -ENODEV;
-		goto out_unregister;
+	v4l2_async_notifier_init(&cam->notifier);
+	ret = v4l2_async_notifier_add_subdev(&cam->notifier, &cam->asd);
+	if (ret) {
+		cam_warn(cam, "failed to add subdev to a notifier");
+		goto out;
 	}
 
-	ret = mcam_cam_init(cam);
-	if (ret)
-		goto out_unregister;
-
-	ret = mcam_setup_vb2(cam);
-	if (ret)
-		goto out_unregister;
-
-	mutex_lock(&cam->s_mutex);
-	cam->vdev = mcam_v4l_template;
-	cam->vdev.v4l2_dev = &cam->v4l2_dev;
-	cam->vdev.lock = &cam->s_mutex;
-	cam->vdev.queue = &cam->vb_queue;
-	video_set_drvdata(&cam->vdev, cam);
-	ret = video_register_device(&cam->vdev, VFL_TYPE_GRABBER, -1);
-	if (ret) {
-		mutex_unlock(&cam->s_mutex);
-		goto out_unregister;
+	cam->notifier.ops = &mccic_notify_ops;
+	ret = v4l2_async_notifier_register(&cam->v4l2_dev, &cam->notifier);
+	if (ret < 0) {
+		cam_warn(cam, "failed to register a sensor notifier");
+		goto out;
 	}
 
 	/*
@@ -1811,11 +1865,10 @@ int mccic_register(struct mcam_camera *cam)
 			cam_warn(cam, "Unable to alloc DMA buffers at load will try again later.");
 	}
 
-	mutex_unlock(&cam->s_mutex);
 	return 0;
 
-out_unregister:
-	v4l2_ctrl_handler_free(&cam->ctrl_handler);
+out:
+	v4l2_async_notifier_unregister(&cam->notifier);
 	v4l2_device_unregister(&cam->v4l2_dev);
 	return ret;
 }
@@ -1835,8 +1888,8 @@ void mccic_shutdown(struct mcam_camera *cam)
 	}
 	if (cam->buffer_mode == B_vmalloc)
 		mcam_free_dma_bufs(cam);
-	video_unregister_device(&cam->vdev);
 	v4l2_ctrl_handler_free(&cam->ctrl_handler);
+	v4l2_async_notifier_unregister(&cam->notifier);
 	v4l2_device_unregister(&cam->v4l2_dev);
 }
 EXPORT_SYMBOL_GPL(mccic_shutdown);
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index b828b1bb59d3..4a72213aca1a 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -102,14 +102,11 @@ struct mcam_camera {
 	 * These fields should be set by the platform code prior to
 	 * calling mcam_register().
 	 */
-	struct i2c_adapter *i2c_adapter;
 	unsigned char __iomem *regs;
 	unsigned regs_size; /* size in bytes of the register space */
 	spinlock_t dev_lock;
 	struct device *dev; /* For messages, dma alloc */
 	enum mcam_chip_id chip_id;
-	short int clock_speed;	/* Sensor clock speed, default 30 */
-	short int use_smbus;	/* SMBUS or straight I2c? */
 	enum mcam_buffer_mode buffer_mode;
 
 	int mclk_src;	/* which clock source the mclk derives from */
@@ -150,6 +147,8 @@ struct mcam_camera {
 	 * Subsystem structures.
 	 */
 	struct video_device vdev;
+	struct v4l2_async_notifier notifier;
+	struct v4l2_async_subdev asd;
 	struct v4l2_subdev *sensor;
 
 	/* Videobuf2 stuff */
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index ff1feca7bc9b..cafb55f2b057 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -3,6 +3,7 @@
  * to work with the Armada 610 as used in the OLPC 1.75 system.
  *
  * Copyright 2011 Jonathan Corbet <corbet@lwn.net>
+ * Copyright 2018 Lubomir Rintel <lkundrak@v3.sk>
  *
  * This file may be distributed under the terms of the GNU General
  * Public License, version 2.
@@ -11,7 +12,6 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
@@ -317,6 +317,7 @@ static int mmpcam_probe(struct platform_device *pdev)
 	struct mmp_camera *cam;
 	struct mcam_camera *mcam;
 	struct resource *res;
+	struct fwnode_handle *ep;
 	struct mmp_camera_platform_data *pdata;
 	int ret;
 
@@ -331,7 +332,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	mcam->plat_power_down = mmpcam_power_down;
 	mcam->calc_dphy = mmpcam_calc_dphy;
 	mcam->dev = &pdev->dev;
-	mcam->use_smbus = 0;
 	pdata = pdev->dev.platform_data;
 	if (pdata) {
 		mcam->mclk_src = pdata->mclk_src;
@@ -375,15 +375,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	cam->power_regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(cam->power_regs))
 		return PTR_ERR(cam->power_regs);
-	/*
-	 * Find the i2c adapter.  This assumes, of course, that the
-	 * i2c bus is already up and functioning.
-	 */
-	mcam->i2c_adapter = platform_get_drvdata(pdata->i2c_device);
-	if (mcam->i2c_adapter == NULL) {
-		dev_err(&pdev->dev, "No i2c adapter\n");
-		return -ENODEV;
-	}
 	/*
 	 * Sensor GPIO pins.
 	 */
@@ -406,6 +397,19 @@ static int mmpcam_probe(struct platform_device *pdev)
 
 	mcam_init_clk(mcam);
 
+	/*
+	 * Create a match of the sensor against its OF node.
+	 */
+	ep = fwnode_graph_get_next_endpoint(of_fwnode_handle(pdev->dev.of_node),
+					    NULL);
+	if (!ep)
+		return -ENODEV;
+
+	mcam->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
+	mcam->asd.match.fwnode = fwnode_graph_get_remote_port_parent(ep);
+
+	fwnode_handle_put(ep);
+
 	/*
 	 * Power the device up and hand it off to the core.
 	 */
@@ -415,6 +419,7 @@ static int mmpcam_probe(struct platform_device *pdev)
 	ret = mccic_register(mcam);
 	if (ret)
 		goto out_power_down;
+
 	/*
 	 * Finally, set up our IRQ now that the core is ready to
 	 * deal with it.
diff --git a/include/linux/platform_data/media/mmp-camera.h b/include/linux/platform_data/media/mmp-camera.h
index 4c3a80a45883..c573ebc40035 100644
--- a/include/linux/platform_data/media/mmp-camera.h
+++ b/include/linux/platform_data/media/mmp-camera.h
@@ -12,7 +12,6 @@ enum dphy3_algo {
 };
 
 struct mmp_camera_platform_data {
-	struct platform_device *i2c_device;
 	int sensor_power_gpio;
 	int sensor_reset_gpio;
 	enum v4l2_mbus_type bus_type;
-- 
2.19.1


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

* [PATCH 11/11] [media] marvell-ccic: provide a clock for the sensor
  2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
                   ` (9 preceding siblings ...)
  2018-11-05  7:30 ` [PATCH 10/11] [media] marvell-ccic: use async notifier to get the sensor Lubomir Rintel
@ 2018-11-05  7:30 ` Lubomir Rintel
  2018-11-05 13:01   ` kbuild test robot
  2018-11-07  4:42   ` Dan Carpenter
  10 siblings, 2 replies; 19+ messages in thread
From: Lubomir Rintel @ 2018-11-05  7:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Jonathan Corbet, linux-media
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

The sensor needs the MCLK clock running when it's being probed. On
platforms where the sensor is instantiated from a DT (MMP2) it is going
to happen asynchronously.

Therefore, the current modus operandi, where the bridge driver fiddles
with the sensor power and clock itself is not going to fly. As the comments
wisely note, this doesn't even belong there.

Luckily, the ov7670 driver is already able to control its power and
reset lines, we can just drop the MMP platform glue altogether.

It also requests the clock via the standard clock subsystem. Good -- let's
set up a clock instance so that the sensor can ask us to enable the clock.
Note that this is pretty dumb at the moment: the clock is hardwired to a
particular frequency and parent. It was always the case.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../media/platform/marvell-ccic/cafe-driver.c |  11 +-
 .../media/platform/marvell-ccic/mcam-core.c   | 158 +++++++++++++++---
 .../media/platform/marvell-ccic/mcam-core.h   |   3 +
 .../media/platform/marvell-ccic/mmp-driver.c  | 153 ++---------------
 .../linux/platform_data/media/mmp-camera.h    |   2 -
 5 files changed, 161 insertions(+), 166 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 0164afc405d1..0e712bb941ba 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -33,6 +33,7 @@
 #include <linux/wait.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/clkdev.h>
 
 #include "mcam-core.h"
 
@@ -533,11 +534,10 @@ static int cafe_pci_probe(struct pci_dev *pdev,
 		goto out_iounmap;
 
 	/*
-	 * Initialize the controller and leave it powered up.  It will
-	 * stay that way until the sensor driver shows up.
+	 * Initialize the controller.
 	 */
 	cafe_ctlr_init(mcam);
-	cafe_ctlr_power_up(mcam);
+
 	/*
 	 * Set up I2C/SMBUS communications.  We have to drop the mutex here
 	 * because the sensor could attach in this call chain, leading to
@@ -555,12 +555,15 @@ static int cafe_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto out_smbus_shutdown;
 
+	clkdev_create(mcam->mclk, "xclk", "%d-%04x",
+		i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
+
 	if (i2c_new_device(cam->i2c_adapter, &ov7670_info)) {
 		cam->registered = 1;
 		return 0;
 	}
 
-	cafe_shutdown(cam);
+	mccic_shutdown(mcam);
 out_smbus_shutdown:
 	cafe_smbus_shutdown(cam);
 out_pdown:
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 87812b7287f0..4656eaee2f29 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -22,6 +22,7 @@
 #include <linux/vmalloc.h>
 #include <linux/io.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
@@ -303,9 +304,6 @@ static void mcam_enable_mipi(struct mcam_camera *mcam)
 		 */
 		mcam_reg_write(mcam, REG_CSI2_CTRL0,
 			CSI2_C0_MIPI_EN | CSI2_C0_ACT_LANE(mcam->lane));
-		mcam_reg_write(mcam, REG_CLKCTRL,
-			(mcam->mclk_src << 29) | mcam->mclk_div);
-
 		mcam->mipi_enabled = true;
 	}
 }
@@ -846,11 +844,6 @@ static void mcam_ctlr_init(struct mcam_camera *cam)
 	 * but it's good to be sure.
 	 */
 	mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE);
-	/*
-	 * Clock the sensor appropriately.  Controller clock should
-	 * be 48MHz, sensor "typical" value is half that.
-	 */
-	mcam_reg_write_mask(cam, REG_CLKCTRL, 2, CLK_DIV_MASK);
 	spin_unlock_irqrestore(&cam->dev_lock, flags);
 }
 
@@ -898,14 +891,15 @@ static int mcam_ctlr_power_up(struct mcam_camera *cam)
 	int ret;
 
 	spin_lock_irqsave(&cam->dev_lock, flags);
-	ret = cam->plat_power_up(cam);
-	if (ret) {
-		spin_unlock_irqrestore(&cam->dev_lock, flags);
-		return ret;
+	if (cam->plat_power_up) {
+		ret = cam->plat_power_up(cam);
+		if (ret) {
+			spin_unlock_irqrestore(&cam->dev_lock, flags);
+			return ret;
+		}
 	}
 	mcam_reg_clear_bit(cam, REG_CTRL1, C1_PWRDWN);
 	spin_unlock_irqrestore(&cam->dev_lock, flags);
-	msleep(5); /* Just to be sure */
 	return 0;
 }
 
@@ -920,10 +914,101 @@ static void mcam_ctlr_power_down(struct mcam_camera *cam)
 	 * power down routine.
 	 */
 	mcam_reg_set_bit(cam, REG_CTRL1, C1_PWRDWN);
-	cam->plat_power_down(cam);
+	if (cam->plat_power_down)
+		cam->plat_power_down(cam);
 	spin_unlock_irqrestore(&cam->dev_lock, flags);
 }
 
+/* ---------------------------------------------------------------------- */
+/*
+ * Controller clocks.
+ */
+static void mcam_clk_enable(struct mcam_camera *mcam)
+{
+	unsigned int i;
+
+	for (i = 0; i < NR_MCAM_CLK; i++) {
+		if (!IS_ERR(mcam->clk[i]))
+			clk_prepare_enable(mcam->clk[i]);
+	}
+}
+
+static void mcam_clk_disable(struct mcam_camera *mcam)
+{
+	int i;
+
+	for (i = NR_MCAM_CLK - 1; i >= 0; i--) {
+		if (!IS_ERR(mcam->clk[i]))
+			clk_disable_unprepare(mcam->clk[i]);
+	}
+}
+
+/* ---------------------------------------------------------------------- */
+/*
+ * Master sensor clock.
+ */
+static int mclk_prepare(struct clk_hw *hw)
+{
+	struct mcam_camera *cam = container_of(hw, struct mcam_camera, mclk_hw);
+
+	clk_prepare(cam->clk[0]);
+	return 0;
+}
+
+static void mclk_unprepare(struct clk_hw *hw)
+{
+	struct mcam_camera *cam = container_of(hw, struct mcam_camera, mclk_hw);
+
+	clk_unprepare(cam->clk[0]);
+}
+
+static int mclk_enable(struct clk_hw *hw)
+{
+	struct mcam_camera *cam = container_of(hw, struct mcam_camera, mclk_hw);
+	int mclk_src;
+	int mclk_div;
+
+	/*
+	 * Clock the sensor appropriately.  Controller clock should
+	 * be 48MHz, sensor "typical" value is half that.
+	 */
+	if (cam->bus_type == V4L2_MBUS_CSI2_DPHY) {
+		mclk_src = cam->mclk_src;
+		mclk_div = cam->mclk_div;
+	} else {
+		mclk_src = 3;
+		mclk_div = 2;
+	}
+
+	clk_enable(cam->clk[0]);
+	mcam_reg_write(cam, REG_CLKCTRL, (mclk_src << 29) | mclk_div);
+	mcam_ctlr_power_up(cam);
+
+	return 0;
+}
+
+static void mclk_disable(struct clk_hw *hw)
+{
+	struct mcam_camera *cam = container_of(hw, struct mcam_camera, mclk_hw);
+
+	mcam_ctlr_power_down(cam);
+	clk_disable(cam->clk[0]);
+}
+
+static unsigned long mclk_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	return 48000000;
+}
+
+static const struct clk_ops mclk_ops = {
+	.prepare = mclk_prepare,
+	.unprepare = mclk_unprepare,
+	.enable = mclk_enable,
+	.disable = mclk_disable,
+	.recalc_rate = mclk_recalc_rate,
+};
+
 /* -------------------------------------------------------------------- */
 /*
  * Communications with the sensor.
@@ -948,7 +1033,6 @@ static int mcam_cam_init(struct mcam_camera *cam)
 	ret = __mcam_cam_reset(cam);
 	/* Get/set parameters? */
 	cam->state = S_IDLE;
-	mcam_ctlr_power_down(cam);
 	return ret;
 }
 
@@ -1584,9 +1668,10 @@ static int mcam_v4l_open(struct file *filp)
 	if (ret)
 		goto out;
 	if (v4l2_fh_is_singular_file(filp)) {
-		ret = mcam_ctlr_power_up(cam);
+		ret = sensor_call(cam, core, s_power, 1);
 		if (ret)
-			goto out;
+			return ret;
+		mcam_clk_enable(cam);
 		__mcam_cam_reset(cam);
 		mcam_set_config_needed(cam, 1);
 	}
@@ -1608,7 +1693,8 @@ static int mcam_v4l_release(struct file *filp)
 	_vb2_fop_release(filp, NULL);
 	if (last_open) {
 		mcam_disable_mipi(cam);
-		mcam_ctlr_power_down(cam);
+		sensor_call(cam, core, s_power, 0);
+		mcam_clk_disable(cam);
 		if (cam->buffer_mode == B_vmalloc && alloc_bufs_at_read)
 			mcam_free_dma_bufs(cam);
 	}
@@ -1806,6 +1892,7 @@ static const struct v4l2_async_notifier_operations mccic_notify_ops = {
 
 int mccic_register(struct mcam_camera *cam)
 {
+	struct clk_init_data mclk_init = { };
 	int ret;
 
 	/*
@@ -1838,7 +1925,10 @@ int mccic_register(struct mcam_camera *cam)
 	mcam_set_config_needed(cam, 1);
 	cam->pix_format = mcam_def_pix_format;
 	cam->mbus_code = mcam_def_mbus_code;
-	mcam_ctlr_init(cam);
+
+	mcam_clk_enable(cam);
+	mcam_ctlr_init(cam); // XXX?
+	mcam_clk_disable(cam);
 
 	/*
 	 * Register sensor notifier.
@@ -1857,6 +1947,26 @@ int mccic_register(struct mcam_camera *cam)
 		goto out;
 	}
 
+	/*
+	 * Register sensor master clock.
+	 */
+	mclk_init.parent_names = NULL;
+	mclk_init.num_parents = 0;
+	mclk_init.ops = &mclk_ops;
+	mclk_init.name = "mclk";
+
+	of_property_read_string(cam->dev->of_node, "clock-output-names",
+							&mclk_init.name);
+
+	cam->mclk_hw.init = &mclk_init;
+
+	cam->mclk = devm_clk_register(cam->dev, &cam->mclk_hw);
+	if (IS_ERR(cam->mclk)) {
+		ret = PTR_ERR(cam->mclk);
+		dev_err(cam->dev, "can't register clock\n");
+		goto out;
+	}
+
 	/*
 	 * If so requested, try to get our DMA buffers now.
 	 */
@@ -1884,7 +1994,7 @@ void mccic_shutdown(struct mcam_camera *cam)
 	 */
 	if (!list_empty(&cam->vdev.fh_list)) {
 		cam_warn(cam, "Removing a device with users!\n");
-		mcam_ctlr_power_down(cam);
+		sensor_call(cam, core, s_power, 0);
 	}
 	if (cam->buffer_mode == B_vmalloc)
 		mcam_free_dma_bufs(cam);
@@ -1906,7 +2016,8 @@ void mccic_suspend(struct mcam_camera *cam)
 		enum mcam_state cstate = cam->state;
 
 		mcam_ctlr_stop_dma(cam);
-		mcam_ctlr_power_down(cam);
+		sensor_call(cam, core, s_power, 0);
+		mcam_clk_disable(cam);
 		cam->state = cstate;
 	}
 	mutex_unlock(&cam->s_mutex);
@@ -1919,14 +2030,15 @@ int mccic_resume(struct mcam_camera *cam)
 
 	mutex_lock(&cam->s_mutex);
 	if (!list_empty(&cam->vdev.fh_list)) {
-		ret = mcam_ctlr_power_up(cam);
+		mcam_clk_enable(cam);
+		ret = sensor_call(cam, core, s_power, 1);
 		if (ret) {
 			mutex_unlock(&cam->s_mutex);
 			return ret;
 		}
 		__mcam_cam_reset(cam);
 	} else {
-		mcam_ctlr_power_down(cam);
+		sensor_call(cam, core, s_power, 0);
 	}
 	mutex_unlock(&cam->s_mutex);
 
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index 4a72213aca1a..2e3a7567a76a 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -8,6 +8,7 @@
 #define _MCAM_CORE_H
 
 #include <linux/list.h>
+#include <linux/clk-provider.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-dev.h>
@@ -125,6 +126,8 @@ struct mcam_camera {
 
 	/* clock tree support */
 	struct clk *clk[NR_MCAM_CLK];
+	struct clk_hw mclk_hw;
+	struct clk *mclk;
 
 	/*
 	 * Callbacks from the core to the platform code.
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index cafb55f2b057..787984193ffe 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -22,9 +22,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
-#include <linux/gpio.h>
 #include <linux/io.h>
-#include <linux/delay.h>
 #include <linux/list.h>
 #include <linux/pm.h>
 #include <linux/clk.h>
@@ -38,7 +36,6 @@ MODULE_LICENSE("GPL");
 static char *mcam_clks[] = {"CCICAXICLK", "CCICFUNCLK", "CCICPHYCLK"};
 
 struct mmp_camera {
-	void __iomem *power_regs;
 	struct platform_device *pdev;
 	struct mcam_camera mcam;
 	struct list_head devlist;
@@ -94,95 +91,6 @@ static struct mmp_camera *mmpcam_find_device(struct platform_device *pdev)
 	return NULL;
 }
 
-
-
-
-/*
- * Power-related registers; this almost certainly belongs
- * somewhere else.
- *
- * ARMADA 610 register manual, sec 7.2.1, p1842.
- */
-#define CPU_SUBSYS_PMU_BASE	0xd4282800
-#define REG_CCIC_DCGCR		0x28	/* CCIC dyn clock gate ctrl reg */
-#define REG_CCIC_CRCR		0x50	/* CCIC clk reset ctrl reg	*/
-#define REG_CCIC2_CRCR		0xf4	/* CCIC2 clk reset ctrl reg	*/
-
-static void mcam_clk_enable(struct mcam_camera *mcam)
-{
-	unsigned int i;
-
-	for (i = 0; i < NR_MCAM_CLK; i++) {
-		if (!IS_ERR(mcam->clk[i]))
-			clk_prepare_enable(mcam->clk[i]);
-	}
-}
-
-static void mcam_clk_disable(struct mcam_camera *mcam)
-{
-	int i;
-
-	for (i = NR_MCAM_CLK - 1; i >= 0; i--) {
-		if (!IS_ERR(mcam->clk[i]))
-			clk_disable_unprepare(mcam->clk[i]);
-	}
-}
-
-/*
- * Power control.
- */
-static void mmpcam_power_up_ctlr(struct mmp_camera *cam)
-{
-	iowrite32(0x3f, cam->power_regs + REG_CCIC_DCGCR);
-	iowrite32(0x3805b, cam->power_regs + REG_CCIC_CRCR);
-	mdelay(1);
-}
-
-static int mmpcam_power_up(struct mcam_camera *mcam)
-{
-	struct mmp_camera *cam = mcam_to_cam(mcam);
-	struct mmp_camera_platform_data *pdata;
-
-/*
- * Turn on power and clocks to the controller.
- */
-	mmpcam_power_up_ctlr(cam);
-	mcam_clk_enable(mcam);
-/*
- * Provide power to the sensor.
- */
-	mcam_reg_write(mcam, REG_CLKCTRL, 0x60000002);
-	pdata = cam->pdev->dev.platform_data;
-	gpio_set_value(pdata->sensor_power_gpio, 1);
-	mdelay(5);
-	mcam_reg_clear_bit(mcam, REG_CTRL1, 0x10000000);
-	gpio_set_value(pdata->sensor_reset_gpio, 0); /* reset is active low */
-	mdelay(5);
-	gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */
-	mdelay(5);
-
-	return 0;
-}
-
-static void mmpcam_power_down(struct mcam_camera *mcam)
-{
-	struct mmp_camera *cam = mcam_to_cam(mcam);
-	struct mmp_camera_platform_data *pdata;
-/*
- * Turn off clocks and set reset lines
- */
-	iowrite32(0, cam->power_regs + REG_CCIC_DCGCR);
-	iowrite32(0, cam->power_regs + REG_CCIC_CRCR);
-/*
- * Shut down the sensor.
- */
-	pdata = cam->pdev->dev.platform_data;
-	gpio_set_value(pdata->sensor_power_gpio, 0);
-	gpio_set_value(pdata->sensor_reset_gpio, 0);
-
-	mcam_clk_disable(mcam);
-}
-
 /*
  * calc the dphy register values
  * There are three dphy registers being used.
@@ -328,8 +236,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&cam->devlist);
 
 	mcam = &cam->mcam;
-	mcam->plat_power_up = mmpcam_power_up;
-	mcam->plat_power_down = mmpcam_power_down;
 	mcam->calc_dphy = mmpcam_calc_dphy;
 	mcam->dev = &pdev->dev;
 	pdata = pdev->dev.platform_data;
@@ -367,33 +273,6 @@ static int mmpcam_probe(struct platform_device *pdev)
 	if (IS_ERR(mcam->regs))
 		return PTR_ERR(mcam->regs);
 	mcam->regs_size = resource_size(res);
-	/*
-	 * Power/clock memory is elsewhere; get it too.  Perhaps this
-	 * should really be managed outside of this driver?
-	 */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	cam->power_regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(cam->power_regs))
-		return PTR_ERR(cam->power_regs);
-	/*
-	 * Sensor GPIO pins.
-	 */
-	ret = devm_gpio_request(&pdev->dev, pdata->sensor_power_gpio,
-							"cam-power");
-	if (ret) {
-		dev_err(&pdev->dev, "Can't get sensor power gpio %d",
-				pdata->sensor_power_gpio);
-		return ret;
-	}
-	gpio_direction_output(pdata->sensor_power_gpio, 0);
-	ret = devm_gpio_request(&pdev->dev, pdata->sensor_reset_gpio,
-							"cam-reset");
-	if (ret) {
-		dev_err(&pdev->dev, "Can't get sensor reset gpio %d",
-				pdata->sensor_reset_gpio);
-		return ret;
-	}
-	gpio_direction_output(pdata->sensor_reset_gpio, 0);
 
 	mcam_init_clk(mcam);
 
@@ -411,14 +290,21 @@ static int mmpcam_probe(struct platform_device *pdev)
 	fwnode_handle_put(ep);
 
 	/*
-	 * Power the device up and hand it off to the core.
+	 * Register the device with the core.
 	 */
-	ret = mmpcam_power_up(mcam);
-	if (ret)
-		return ret;
 	ret = mccic_register(mcam);
 	if (ret)
-		goto out_power_down;
+		return ret;
+
+	/*
+	 * Add OF clock provider.
+	 */
+	ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get,
+								mcam->mclk);
+	if (ret) {
+		dev_err(&pdev->dev, "can't add DT clock provider\n");
+		goto out;
+	}
 
 	/*
 	 * Finally, set up our IRQ now that the core is ready to
@@ -427,7 +313,7 @@ static int mmpcam_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res == NULL) {
 		ret = -ENODEV;
-		goto out_unregister;
+		goto out;
 	}
 	cam->irq = res->start;
 	ret = devm_request_irq(&pdev->dev, cam->irq, mmpcam_irq, IRQF_SHARED,
@@ -437,10 +323,10 @@ static int mmpcam_probe(struct platform_device *pdev)
 		return 0;
 	}
 
-out_unregister:
+out:
+	fwnode_handle_put(mcam->asd.match.fwnode);
 	mccic_shutdown(mcam);
-out_power_down:
-	mmpcam_power_down(mcam);
+
 	return ret;
 }
 
@@ -451,7 +337,6 @@ static int mmpcam_remove(struct mmp_camera *cam)
 
 	mmpcam_remove_device(cam);
 	mccic_shutdown(mcam);
-	mmpcam_power_down(mcam);
 	return 0;
 }
 
@@ -483,12 +368,6 @@ static int mmpcam_resume(struct platform_device *pdev)
 {
 	struct mmp_camera *cam = mmpcam_find_device(pdev);
 
-	/*
-	 * Power up unconditionally just in case the core tries to
-	 * touch a register even if nothing was active before; trust
-	 * me, it's better this way.
-	 */
-	mmpcam_power_up_ctlr(cam);
 	return mccic_resume(&cam->mcam);
 }
 
diff --git a/include/linux/platform_data/media/mmp-camera.h b/include/linux/platform_data/media/mmp-camera.h
index c573ebc40035..53adaab64f28 100644
--- a/include/linux/platform_data/media/mmp-camera.h
+++ b/include/linux/platform_data/media/mmp-camera.h
@@ -12,8 +12,6 @@ enum dphy3_algo {
 };
 
 struct mmp_camera_platform_data {
-	int sensor_power_gpio;
-	int sensor_reset_gpio;
 	enum v4l2_mbus_type bus_type;
 	int mclk_src;	/* which clock source the MCLK derives from */
 	int mclk_div;	/* Clock Divider Value for MCLK */
-- 
2.19.1


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

* Re: [PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus
  2018-11-05  7:30 ` [PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus Lubomir Rintel
@ 2018-11-05  9:07   ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2018-11-05  9:07 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, linux-media, Rob Herring,
	Mark Rutland, devicetree, linux-kernel, James Cameron

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

On Mon 2018-11-05 08:30:48, Lubomir Rintel wrote:
> The commit 05fed81625bf ("[media] marvell-ccic: add MIPI support for
> marvell-ccic driver") that claimed to add CSI2 turned on C0_EOF_VSYNC for
> parallel bus without a very good explanation.
> 
> That broke camera on OLPC XO-1.75 which precisely uses a sensor on a
> parallel bus. Revert that chunk.
> 
> Tested on an OLPC XO-1.75.
> 
> Fixes: 05fed81625bf755cc67c5864cdfd18b69ea828d1
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

You should really Cc original author and people that signed off on
that patch.
									Pavel
		

>  drivers/media/platform/marvell-ccic/mcam-core.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
> index d97f39bde9bd..d24e5b7a3bc5 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
> @@ -792,12 +792,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
>  	 * Make sure it knows we want to use hsync/vsync.
>  	 */
>  	mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK);
> -	/*
> -	 * This field controls the generation of EOF(DVP only)
> -	 */
> -	if (cam->bus_type != V4L2_MBUS_CSI2_DPHY)
> -		mcam_reg_set_bit(cam, REG_CTRL0,
> -				C0_EOF_VSYNC | C0_VEDGE_CTRL);
>  }
>  
>  

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 06/11] [media] marvell-ccic: drop ctlr_reset()
  2018-11-05  7:30 ` [PATCH 06/11] [media] marvell-ccic: drop ctlr_reset() Lubomir Rintel
@ 2018-11-05  9:11   ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2018-11-05  9:11 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, linux-media, Rob Herring,
	Mark Rutland, devicetree, linux-kernel, James Cameron

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

On Mon 2018-11-05 08:30:49, Lubomir Rintel wrote:
> This accesses the clock registers directly and thus is not too
> devicetree friendly. If it's actually needed it needs to be done
> differently.

Device-tree unfriendly is bad, but you still don't want to cause
regression to people needing this.

(Perhaps noone uses this, but...)

> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Git blame says: 7c269f454 . Perhaps its authors should be Cced here,
too?
								Pavel

> ---
>  .../media/platform/marvell-ccic/mcam-core.c   |  6 -----
>  .../media/platform/marvell-ccic/mcam-core.h   |  1 -
>  .../media/platform/marvell-ccic/mmp-driver.c  | 23 -------------------
>  3 files changed, 30 deletions(-)
> 
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
> index d24e5b7a3bc5..1b879035948c 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
> @@ -1154,12 +1154,6 @@ static void mcam_vb_stop_streaming(struct vb2_queue *vq)
>  	if (cam->state != S_STREAMING)
>  		return;
>  	mcam_ctlr_stop_dma(cam);
> -	/*
> -	 * Reset the CCIC PHY after stopping streaming,
> -	 * otherwise, the CCIC may be unstable.
> -	 */
> -	if (cam->ctlr_reset)
> -		cam->ctlr_reset(cam);
>  	/*
>  	 * VB2 reclaims the buffers, so we need to forget
>  	 * about them.
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
> index ad8955f9f0a1..f93f23faf364 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.h
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h
> @@ -137,7 +137,6 @@ struct mcam_camera {
>  	int (*plat_power_up) (struct mcam_camera *cam);
>  	void (*plat_power_down) (struct mcam_camera *cam);
>  	void (*calc_dphy) (struct mcam_camera *cam);
> -	void (*ctlr_reset) (struct mcam_camera *cam);
>  
>  	/*
>  	 * Everything below here is private to the mcam core and
> diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
> index 70a2833db0d1..92a79ad8a12c 100644
> --- a/drivers/media/platform/marvell-ccic/mmp-driver.c
> +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
> @@ -183,28 +183,6 @@ static void mmpcam_power_down(struct mcam_camera *mcam)
>  	mcam_clk_disable(mcam);
>  }
>  
> -static void mcam_ctlr_reset(struct mcam_camera *mcam)
> -{
> -	unsigned long val;
> -	struct mmp_camera *cam = mcam_to_cam(mcam);
> -
> -	if (mcam->ccic_id) {
> -		/*
> -		 * Using CCIC2
> -		 */
> -		val = ioread32(cam->power_regs + REG_CCIC2_CRCR);
> -		iowrite32(val & ~0x2, cam->power_regs + REG_CCIC2_CRCR);
> -		iowrite32(val | 0x2, cam->power_regs + REG_CCIC2_CRCR);
> -	} else {
> -		/*
> -		 * Using CCIC1
> -		 */
> -		val = ioread32(cam->power_regs + REG_CCIC_CRCR);
> -		iowrite32(val & ~0x2, cam->power_regs + REG_CCIC_CRCR);
> -		iowrite32(val | 0x2, cam->power_regs + REG_CCIC_CRCR);
> -	}
> -}
> -
>  /*
>   * calc the dphy register values
>   * There are three dphy registers being used.
> @@ -352,7 +330,6 @@ static int mmpcam_probe(struct platform_device *pdev)
>  	mcam = &cam->mcam;
>  	mcam->plat_power_up = mmpcam_power_up;
>  	mcam->plat_power_down = mmpcam_power_down;
> -	mcam->ctlr_reset = mcam_ctlr_reset;
>  	mcam->calc_dphy = mmpcam_calc_dphy;
>  	mcam->dev = &pdev->dev;
>  	mcam->use_smbus = 0;

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 07/11] [media] marvell-ccic: drop unused stuff
  2018-11-05  7:30 ` [PATCH 07/11] [media] marvell-ccic: drop unused stuff Lubomir Rintel
@ 2018-11-05  9:17   ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2018-11-05  9:17 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, linux-media, Rob Herring,
	Mark Rutland, devicetree, linux-kernel, James Cameron

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

On Mon 2018-11-05 08:30:50, Lubomir Rintel wrote:
> Remove structure members and headers that are not actually used. Saves
> us from some noise in subsequent cleanup commits.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Pavel Machek <pavel@ucw.cz>

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers
  2018-11-05  7:30 ` [PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers Lubomir Rintel
@ 2018-11-05  9:17   ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2018-11-05  9:17 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, linux-media, Rob Herring,
	Mark Rutland, devicetree, linux-kernel, James Cameron

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Mon 2018-11-05 08:30:51, Lubomir Rintel wrote:
> The access to REG_CLKCTRL or REG_CTRL1 without the clock enabled hangs
> the machine. Enable the clock first.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Pavel Machek <pavel@ucw.cz>

> ---
>  drivers/media/platform/marvell-ccic/mmp-driver.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
> index 9e988e527b0d..9c0238f72c40 100644
> --- a/drivers/media/platform/marvell-ccic/mmp-driver.c
> +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
> @@ -145,6 +145,7 @@ static int mmpcam_power_up(struct mcam_camera *mcam)
>   * Turn on power and clocks to the controller.
>   */
>  	mmpcam_power_up_ctlr(cam);
> +	mcam_clk_enable(mcam);
>  /*
>   * Provide power to the sensor.
>   */
> @@ -158,8 +159,6 @@ static int mmpcam_power_up(struct mcam_camera *mcam)
>  	gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */
>  	mdelay(5);
>  
> -	mcam_clk_enable(mcam);
> -
>  	return 0;
>  }
>  

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support
  2018-11-05  7:30 ` [PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support Lubomir Rintel
@ 2018-11-05  9:20   ` Pavel Machek
  0 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2018-11-05  9:20 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, linux-media, Rob Herring,
	Mark Rutland, devicetree, linux-kernel, James Cameron

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On Mon 2018-11-05 08:30:52, Lubomir Rintel wrote:
> The platform data is actually not used anywhere (along with the CSI
> support) and should be safe to remove.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Pavel Machek <pavel@ucw.cz>


> +	} else {
> +		/*
> +		 * This are values that used to be hardcoded in mcam-core and

"These are values"? "and it"?

> +		 * work well on a OLPC XO 1.75 with a parallel bus sensor.
> +		 * If it turns out other setups make sense, the values should
> +		 * be obtained from the device tree.
> +		 */
> +		mcam->mclk_src = 3;
> +		mcam->mclk_div = 2;

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 11/11] [media] marvell-ccic: provide a clock for the sensor
  2018-11-05  7:30 ` [PATCH 11/11] [media] marvell-ccic: provide a clock for " Lubomir Rintel
@ 2018-11-05 13:01   ` kbuild test robot
  2018-11-07  4:42   ` Dan Carpenter
  1 sibling, 0 replies; 19+ messages in thread
From: kbuild test robot @ 2018-11-05 13:01 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: kbuild-all, Mauro Carvalho Chehab, Jonathan Corbet, linux-media,
	Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

[-- Attachment #1: Type: text/plain, Size: 11553 bytes --]

Hi Lubomir,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.20-rc1 next-20181105]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lubomir-Rintel/media-ov7670-hook-s_power-onto-v4l2-core/20181105-163336
base:   git://linuxtv.org/media_tree.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=ia64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/media/platform/marvell-ccic/cafe-driver.c:38:
>> drivers/media/platform/marvell-ccic/mcam-core.h:129:16: error: field 'mclk_hw' has incomplete type
     struct clk_hw mclk_hw;
                   ^~~~~~~
--
   In file included from drivers/media/platform/marvell-ccic/mmp-driver.c:30:
>> drivers/media/platform/marvell-ccic/mcam-core.h:129:16: error: field 'mclk_hw' has incomplete type
     struct clk_hw mclk_hw;
                   ^~~~~~~
   drivers/media/platform/marvell-ccic/mmp-driver.c: In function 'mmpcam_probe':
>> drivers/media/platform/marvell-ccic/mmp-driver.c:302:8: error: implicit declaration of function 'of_clk_add_provider'; did you mean 'of_clk_get_from_provider'? [-Werror=implicit-function-declaration]
     ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get,
           ^~~~~~~~~~~~~~~~~~~
           of_clk_get_from_provider
>> drivers/media/platform/marvell-ccic/mmp-driver.c:302:47: error: 'of_clk_src_simple_get' undeclared (first use in this function); did you mean 'ida_simple_get'?
     ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get,
                                                  ^~~~~~~~~~~~~~~~~~~~~
                                                  ida_simple_get
   drivers/media/platform/marvell-ccic/mmp-driver.c:302:47: note: each undeclared identifier is reported only once for each function it appears in
   cc1: some warnings being treated as errors
--
   In file included from drivers/media/platform/marvell-ccic/mcam-core.c:35:
>> drivers/media/platform/marvell-ccic/mcam-core.h:129:16: error: field 'mclk_hw' has incomplete type
     struct clk_hw mclk_hw;
                   ^~~~~~~
   In file included from include/linux/kernel.h:10,
                    from drivers/media/platform/marvell-ccic/mcam-core.c:9:
   drivers/media/platform/marvell-ccic/mcam-core.c: In function 'mclk_prepare':
   include/linux/kernel.h:997:32: error: dereferencing pointer to incomplete type 'struct clk_hw'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                                   ^~~~~~
   include/linux/compiler.h:353:9: note: in definition of macro '__compiletime_assert'
      if (!(condition))     \
            ^~~~~~~~~
   include/linux/compiler.h:373:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:997:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:997:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:952:28: note: in expansion of macro 'container_of'
     struct mcam_camera *cam = container_of(hw, struct mcam_camera, mclk_hw);
                               ^~~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c: At top level:
>> drivers/media/platform/marvell-ccic/mcam-core.c:1004:21: error: variable 'mclk_ops' has initializer but incomplete type
    static const struct clk_ops mclk_ops = {
                        ^~~~~~~
>> drivers/media/platform/marvell-ccic/mcam-core.c:1005:3: error: 'const struct clk_ops' has no member named 'prepare'
     .prepare = mclk_prepare,
      ^~~~~~~
>> drivers/media/platform/marvell-ccic/mcam-core.c:1005:13: warning: excess elements in struct initializer
     .prepare = mclk_prepare,
                ^~~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1005:13: note: (near initialization for 'mclk_ops')
>> drivers/media/platform/marvell-ccic/mcam-core.c:1006:3: error: 'const struct clk_ops' has no member named 'unprepare'
     .unprepare = mclk_unprepare,
      ^~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1006:15: warning: excess elements in struct initializer
     .unprepare = mclk_unprepare,
                  ^~~~~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1006:15: note: (near initialization for 'mclk_ops')
>> drivers/media/platform/marvell-ccic/mcam-core.c:1007:3: error: 'const struct clk_ops' has no member named 'enable'
     .enable = mclk_enable,
      ^~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1007:12: warning: excess elements in struct initializer
     .enable = mclk_enable,
               ^~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1007:12: note: (near initialization for 'mclk_ops')
>> drivers/media/platform/marvell-ccic/mcam-core.c:1008:3: error: 'const struct clk_ops' has no member named 'disable'
     .disable = mclk_disable,
      ^~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1008:13: warning: excess elements in struct initializer
     .disable = mclk_disable,
                ^~~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1008:13: note: (near initialization for 'mclk_ops')
>> drivers/media/platform/marvell-ccic/mcam-core.c:1009:3: error: 'const struct clk_ops' has no member named 'recalc_rate'
     .recalc_rate = mclk_recalc_rate,
      ^~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1009:17: warning: excess elements in struct initializer
     .recalc_rate = mclk_recalc_rate,
                    ^~~~~~~~~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c:1009:17: note: (near initialization for 'mclk_ops')
   drivers/media/platform/marvell-ccic/mcam-core.c: In function 'mccic_register':
>> drivers/media/platform/marvell-ccic/mcam-core.c:1895:9: error: variable 'mclk_init' has initializer but incomplete type
     struct clk_init_data mclk_init = { };
            ^~~~~~~~~~~~~
>> drivers/media/platform/marvell-ccic/mcam-core.c:1895:23: error: storage size of 'mclk_init' isn't known
     struct clk_init_data mclk_init = { };
                          ^~~~~~~~~
>> drivers/media/platform/marvell-ccic/mcam-core.c:1963:14: error: implicit declaration of function 'devm_clk_register'; did you mean 'device_register'? [-Werror=implicit-function-declaration]
     cam->mclk = devm_clk_register(cam->dev, &cam->mclk_hw);
                 ^~~~~~~~~~~~~~~~~
                 device_register
   drivers/media/platform/marvell-ccic/mcam-core.c:1895:23: warning: unused variable 'mclk_init' [-Wunused-variable]
     struct clk_init_data mclk_init = { };
                          ^~~~~~~~~
   drivers/media/platform/marvell-ccic/mcam-core.c: At top level:
>> drivers/media/platform/marvell-ccic/mcam-core.c:1004:29: error: storage size of 'mclk_ops' isn't known
    static const struct clk_ops mclk_ops = {
                                ^~~~~~~~
   cc1: some warnings being treated as errors

vim +/mclk_hw +129 drivers/media/platform/marvell-ccic/mcam-core.h

    94	
    95	/*
    96	 * A description of one of our devices.
    97	 * Locking: controlled by s_mutex.  Certain fields, however, require
    98	 *          the dev_lock spinlock; they are marked as such by comments.
    99	 *          dev_lock is also required for access to device registers.
   100	 */
   101	struct mcam_camera {
   102		/*
   103		 * These fields should be set by the platform code prior to
   104		 * calling mcam_register().
   105		 */
   106		unsigned char __iomem *regs;
   107		unsigned regs_size; /* size in bytes of the register space */
   108		spinlock_t dev_lock;
   109		struct device *dev; /* For messages, dma alloc */
   110		enum mcam_chip_id chip_id;
   111		enum mcam_buffer_mode buffer_mode;
   112	
   113		int mclk_src;	/* which clock source the mclk derives from */
   114		int mclk_div;	/* Clock Divider Value for MCLK */
   115	
   116		enum v4l2_mbus_type bus_type;
   117		/* MIPI support */
   118		/* The dphy config value, allocated in board file
   119		 * dphy[0]: DPHY3
   120		 * dphy[1]: DPHY5
   121		 * dphy[2]: DPHY6
   122		 */
   123		int *dphy;
   124		bool mipi_enabled;	/* flag whether mipi is enabled already */
   125		int lane;			/* lane number */
   126	
   127		/* clock tree support */
   128		struct clk *clk[NR_MCAM_CLK];
 > 129		struct clk_hw mclk_hw;
   130		struct clk *mclk;
   131	
   132		/*
   133		 * Callbacks from the core to the platform code.
   134		 */
   135		int (*plat_power_up) (struct mcam_camera *cam);
   136		void (*plat_power_down) (struct mcam_camera *cam);
   137		void (*calc_dphy) (struct mcam_camera *cam);
   138	
   139		/*
   140		 * Everything below here is private to the mcam core and
   141		 * should not be touched by the platform code.
   142		 */
   143		struct v4l2_device v4l2_dev;
   144		struct v4l2_ctrl_handler ctrl_handler;
   145		enum mcam_state state;
   146		unsigned long flags;		/* Buffer status, mainly (dev_lock) */
   147	
   148		struct mcam_frame_state frame_state;	/* Frame state counter */
   149		/*
   150		 * Subsystem structures.
   151		 */
   152		struct video_device vdev;
   153		struct v4l2_async_notifier notifier;
   154		struct v4l2_async_subdev asd;
   155		struct v4l2_subdev *sensor;
   156	
   157		/* Videobuf2 stuff */
   158		struct vb2_queue vb_queue;
   159		struct list_head buffers;	/* Available frames */
   160	
   161		unsigned int nbufs;		/* How many are alloc'd */
   162		int next_buf;			/* Next to consume (dev_lock) */
   163	
   164		char bus_info[32];		/* querycap bus_info */
   165	
   166		/* DMA buffers - vmalloc mode */
   167	#ifdef MCAM_MODE_VMALLOC
   168		unsigned int dma_buf_size;	/* allocated size */
   169		void *dma_bufs[MAX_DMA_BUFS];	/* Internal buffer addresses */
   170		dma_addr_t dma_handles[MAX_DMA_BUFS]; /* Buffer bus addresses */
   171		struct tasklet_struct s_tasklet;
   172	#endif
   173		unsigned int sequence;		/* Frame sequence number */
   174		unsigned int buf_seq[MAX_DMA_BUFS]; /* Sequence for individual bufs */
   175	
   176		/* DMA buffers - DMA modes */
   177		struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
   178	
   179		/* Mode-specific ops, set at open time */
   180		void (*dma_setup)(struct mcam_camera *cam);
   181		void (*frame_complete)(struct mcam_camera *cam, int frame);
   182	
   183		/* Current operating parameters */
   184		struct v4l2_pix_format pix_format;
   185		u32 mbus_code;
   186	
   187		/* Locks */
   188		struct mutex s_mutex; /* Access to this structure */
   189	};
   190	
   191	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52267 bytes --]

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

* Re: [PATCH 11/11] [media] marvell-ccic: provide a clock for the sensor
  2018-11-05  7:30 ` [PATCH 11/11] [media] marvell-ccic: provide a clock for " Lubomir Rintel
  2018-11-05 13:01   ` kbuild test robot
@ 2018-11-07  4:42   ` Dan Carpenter
  1 sibling, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2018-11-07  4:42 UTC (permalink / raw)
  To: kbuild, Lubomir Rintel
  Cc: kbuild-all, Mauro Carvalho Chehab, Jonathan Corbet, linux-media,
	Rob Herring, Mark Rutland, devicetree, linux-kernel,
	Lubomir Rintel, James Cameron, Pavel Machek

Hi Lubomir,

url:    https://github.com/0day-ci/linux/commits/Lubomir-Rintel/media-ov7670-hook-s_power-onto-v4l2-core/20181105-163336
base:   git://linuxtv.org/media_tree.git master

smatch warnings:
drivers/media/platform/marvell-ccic/mcam-core.c:1682 mcam_v4l_open() warn: inconsistent returns 'mutex:&cam->s_mutex'.
  Locked on:   line 1673
  Unlocked on: line 1682

# https://github.com/0day-ci/linux/commit/a4f7d692c7067355da433bbb534531a4e1a55ac6
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a4f7d692c7067355da433bbb534531a4e1a55ac6
vim +1682 drivers/media/platform/marvell-ccic/mcam-core.c

abfa3df3 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-11  1656  
abfa3df3 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-11  1657  /* ---------------------------------------------------------------------- */
abfa3df3 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-11  1658  /*
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1659   * Our various file operations.
abfa3df3 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-11  1660   */
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1661  static int mcam_v4l_open(struct file *filp)
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1662  {
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1663  	struct mcam_camera *cam = video_drvdata(filp);
949bd408 drivers/media/platform/marvell-ccic/mcam-core.c Hans Verkuil    2015-03-05  1664  	int ret;
abfa3df3 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-11  1665  
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1666  	mutex_lock(&cam->s_mutex);
949bd408 drivers/media/platform/marvell-ccic/mcam-core.c Hans Verkuil    2015-03-05  1667  	ret = v4l2_fh_open(filp);
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1668  	if (ret)
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1669  		goto out;
949bd408 drivers/media/platform/marvell-ccic/mcam-core.c Hans Verkuil    2015-03-05  1670  	if (v4l2_fh_is_singular_file(filp)) {
a4f7d692 drivers/media/platform/marvell-ccic/mcam-core.c Lubomir Rintel  2018-11-05  1671  		ret = sensor_call(cam, core, s_power, 1);
05fed816 drivers/media/platform/marvell-ccic/mcam-core.c Libin Yang      2013-07-03  1672  		if (ret)
a4f7d692 drivers/media/platform/marvell-ccic/mcam-core.c Lubomir Rintel  2018-11-05  1673  			return ret;
^^^^^^^^^^
This should be a goto out;

a4f7d692 drivers/media/platform/marvell-ccic/mcam-core.c Lubomir Rintel  2018-11-05  1674  		mcam_clk_enable(cam);
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1675  		__mcam_cam_reset(cam);
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1676  		mcam_set_config_needed(cam, 1);
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1677  	}
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1678  out:
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08  1679  	mutex_unlock(&cam->s_mutex);
44fbcb10 drivers/media/platform/marvell-ccic/mcam-core.c Hans Verkuil    2015-03-05  1680  	if (ret)
44fbcb10 drivers/media/platform/marvell-ccic/mcam-core.c Hans Verkuil    2015-03-05  1681  		v4l2_fh_release(filp);
d43dae75 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-07-08 @1682  	return ret;
a9b36e85 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-20  1683  }
abfa3df3 drivers/media/video/marvell-ccic/mcam-core.c    Jonathan Corbet 2011-06-11  1684  

:::::: The code at line 1682 was first introduced by commit
:::::: d43dae75cc1140bf27a59aa6d8e8bc7a00f009cc [media] marvell-cam: core code reorganization

:::::: TO: Jonathan Corbet <corbet@lwn.net>
:::::: CC: Mauro Carvalho Chehab <mchehab@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

end of thread, other threads:[~2018-11-07  4:42 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05  7:30 [PATCH 0/11] media: make Marvell camera work on DT-based OLPC XO-1.75 Lubomir Rintel
2018-11-05  7:30 ` [PATCH 01/11] media: ov7670: hook s_power onto v4l2 core Lubomir Rintel
2018-11-05  7:30 ` [PATCH 02/11] media: ov7670: control clock along with power Lubomir Rintel
2018-11-05  7:30 ` [PATCH 03/11] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera Lubomir Rintel
2018-11-05  7:30 ` [PATCH 04/11] [media] marvell-ccic: fix DMA s/g desc number calculation Lubomir Rintel
2018-11-05  7:30 ` [PATCH 05/11] [media] marvell-ccic: don't generate EOF on parallel bus Lubomir Rintel
2018-11-05  9:07   ` Pavel Machek
2018-11-05  7:30 ` [PATCH 06/11] [media] marvell-ccic: drop ctlr_reset() Lubomir Rintel
2018-11-05  9:11   ` Pavel Machek
2018-11-05  7:30 ` [PATCH 07/11] [media] marvell-ccic: drop unused stuff Lubomir Rintel
2018-11-05  9:17   ` Pavel Machek
2018-11-05  7:30 ` [PATCH 08/11] [media] marvell-ccic/mmp: enable clock before accessing registers Lubomir Rintel
2018-11-05  9:17   ` Pavel Machek
2018-11-05  7:30 ` [PATCH 09/11] [media] marvell-ccic/mmp: add devicetree support Lubomir Rintel
2018-11-05  9:20   ` Pavel Machek
2018-11-05  7:30 ` [PATCH 10/11] [media] marvell-ccic: use async notifier to get the sensor Lubomir Rintel
2018-11-05  7:30 ` [PATCH 11/11] [media] marvell-ccic: provide a clock for " Lubomir Rintel
2018-11-05 13:01   ` kbuild test robot
2018-11-07  4:42   ` Dan Carpenter

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).