Thread (31 messages) 31 messages, 5 authors, 2022-02-08

Re: [RFC PATCH v4 05/12] media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface

From: Stefan Wahren <hidden>
Date: 2022-02-03 20:44:50
Also in: linux-devicetree, linux-media, lkml

Hi Jean-Michel,

Am 03.02.22 um 18:50 schrieb Jean-Michel Hautbois:
quoted hunk ↗ jump to hunk
Add driver for the Unicam camera receiver block on BCM283x processors.
It is represented as two video device nodes: unicam-image and
unicam-embedded which are connected to an internal subdev (named
unicam-subdev) in order to manage streams routing.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Naushir Patuck <redacted>
Signed-off-by: Jean-Michel Hautbois <redacted>

---
v3 main changes:
  - Change code organization
  - Remove unused variables
  - Correct the fmt_meta functions
  - Rewrite the start/stop streaming
    - You can now start the image node alone, but not the metadata one
    - The buffers are allocated per-node
    - only the required stream is started, if the route exists and is
      enabled
  - Prefix the macros with UNICAM_ to not have too generic names
  - Drop colorspace support
    -> This is causing issues in the try-fmt v4l2-compliance test
  test VIDIOC_G_FMT: OK
	fail: v4l2-test-formats.cpp(363): colorspace >= 0xff
	fail: v4l2-test-formats.cpp(465): testColorspace(!node->is_io_mc, pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization)
  test VIDIOC_TRY_FMT: FAIL
	fail: v4l2-test-formats.cpp(363): colorspace >= 0xff
	fail: v4l2-test-formats.cpp(465): testColorspace(!node->is_io_mc, pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization)
  test VIDIOC_S_FMT: FAIL

v2: Remove the unicam_{info,debug,error} macros and use
dev_dbg/dev_err instead.
---
 MAINTAINERS                                   |    1 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    2 +
 drivers/media/platform/bcm2835/Kconfig        |   21 +
 drivers/media/platform/bcm2835/Makefile       |    3 +
 .../media/platform/bcm2835/bcm2835-unicam.c   | 2586 +++++++++++++++++
 .../media/platform/bcm2835/vc4-regs-unicam.h  |  253 ++
 7 files changed, 2867 insertions(+)
 create mode 100644 drivers/media/platform/bcm2835/Kconfig
 create mode 100644 drivers/media/platform/bcm2835/Makefile
 create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
 create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 740cf86c56dc..ef2cdb37e06f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3676,6 +3676,7 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
 F:	arch/arm/boot/dts/bcm283x*
+F:	drivers/media/platform/bcm2835/
 
 BROADCOM BCM47XX MIPS ARCHITECTURE
 M:	Hauke Mehrtens <hauke@hauke-m.de>
Please move this change to the "single" MAINTAINER patch
quoted hunk ↗ jump to hunk
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9fbdba0fd1e7..033b0358fbb8 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -170,6 +170,7 @@ source "drivers/media/platform/am437x/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
 source "drivers/media/platform/rcar-vin/Kconfig"
 source "drivers/media/platform/atmel/Kconfig"
+source "drivers/media/platform/bcm2835/Kconfig"
 source "drivers/media/platform/sunxi/Kconfig"
 
 config VIDEO_TI_CAL
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 28eb4aadbf45..18894fc586aa 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -86,6 +86,8 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)		+= qcom/camss/
 
 obj-$(CONFIG_VIDEO_QCOM_VENUS)		+= qcom/venus/
 
+obj-$(CONFIG_VIDEO_BCM2835_UNICAM)	+= bcm2835/
+
 obj-y					+= sunxi/
 
 obj-$(CONFIG_VIDEO_MESON_GE2D)		+= meson/ge2d/
diff --git a/drivers/media/platform/bcm2835/Kconfig b/drivers/media/platform/bcm2835/Kconfig
new file mode 100644
index 000000000000..1691541da905
--- /dev/null
+++ b/drivers/media/platform/bcm2835/Kconfig
@@ -0,0 +1,21 @@
+# Broadcom VideoCore4 V4L2 camera support
+
+config VIDEO_BCM2835_UNICAM
+	tristate "Broadcom BCM283x/BCM271x Unicam video capture driver"
+	depends on ARCH_BCM2835 || COMPILE_TEST
+	depends on VIDEO_V4L2
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select V4L2_FWNODE
+	select VIDEOBUF2_DMA_CONTIG
+	help
+	  Say Y here to enable support for the BCM283x/BCM271x CSI-2 receiver.
+	  This is a V4L2 driver that controls the CSI-2 receiver directly,
+	  independently from the VC4 firmware.
+	  This driver is mutually exclusive with the use of bcm2835-camera. The
+	  firmware will disable all access to the peripheral from within the
+	  firmware if it finds a DT node using it, and bcm2835-camera will
+	  therefore fail to probe.
+
+	  To compile this driver as a module, choose M here. The module will be
+	  called bcm2835-unicam.
diff --git a/drivers/media/platform/bcm2835/Makefile b/drivers/media/platform/bcm2835/Makefile
new file mode 100644
index 000000000000..a98aba03598a
--- /dev/null
+++ b/drivers/media/platform/bcm2835/Makefile
@@ -0,0 +1,3 @@
+# Makefile for BCM2835 Unicam driver
+
+obj-$(CONFIG_VIDEO_BCM2835_UNICAM) += bcm2835-unicam.o
diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c b/drivers/media/platform/bcm2835/bcm2835-unicam.c
new file mode 100644
index 000000000000..95e12fdb90f5
--- /dev/null
+++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c
@@ -0,0 +1,2586 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * BCM283x / BCM271x Unicam Capture Driver
+ *
+ * Copyright (C) 2017-2020 - Raspberry Pi (Trading) Ltd.
+ *
+ * Dave Stevenson <dave.stevenson@raspberrypi.com>
+ *
+ * Based on TI am437x driver by
+ *   Benoit Parrot <bparrot@ti.com>
+ *   Lad, Prabhakar <prabhakar.csengg@gmail.com>
+ *
+ * and TI CAL camera interface driver by
+ *    Benoit Parrot <bparrot@ti.com>
+ *
+ *
+ * There are two camera drivers in the kernel for BCM283x - this one
+ * and bcm2835-camera (currently in staging).
+ *
+ * This driver directly controls the Unicam peripheral - there is no
+ * involvement with the VideoCore firmware. Unicam receives CSI-2 or
+ * CCP2 data and writes it into SDRAM.
+ * The only potential processing options are to repack Bayer data into an
+ * alternate format, and applying windowing.
+ * The repacking does not shift the data, so can repack V4L2_PIX_FMT_Sxxxx10P
+ * to V4L2_PIX_FMT_Sxxxx10, or V4L2_PIX_FMT_Sxxxx12P to V4L2_PIX_FMT_Sxxxx12,
+ * but not generically up to V4L2_PIX_FMT_Sxxxx16. The driver will add both
+ * formats where the relevant formats are defined, and will automatically
+ * configure the repacking as required.
+ * Support for windowing may be added later.
+ *
+ * It should be possible to connect this driver to any sensor with a
+ * suitable output interface and V4L2 subdevice driver.
+ *
+ * bcm2835-camera uses the VideoCore firmware to control the sensor,
+ * Unicam, ISP, and all tuner control loops. Fully processed frames are
+ * delivered to the driver by the firmware. It only has sensor drivers
+ * for Omnivision OV5647, and Sony IMX219 sensors.
+ *
+ * The two drivers are mutually exclusive for the same Unicam instance.
+ * The VideoCore firmware checks the device tree configuration during boot.
+ * If it finds device tree nodes called csi0 or csi1 it will block the
+ * firmware from accessing the peripheral, and bcm2835-camera will
+ * not be able to stream data.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-common.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-dv-timings.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/videobuf2-dma-contig.h>
+
+#include "vc4-regs-unicam.h"
+
+#define UNICAM_MODULE_NAME	"unicam"
+
+/*
+ * Unicam must request a minimum of 250Mhz from the VPU clock.
+ * Otherwise the input FIFOs overrun and cause image corruption.
+ */
+#define UNICAM_MIN_VPU_CLOCK_RATE (250 * 1000 * 1000)
+
+/*
+ * Stride is a 16 bit register, but also has to be a multiple of 32.
+ */
+#define UNICAM_BPL_ALIGNMENT		32
+#define UNICAM_MAX_BYTESPERLINE		((1 << 16) - UNICAM_BPL_ALIGNMENT)
+/*
+ * Max width is therefore determined by the max stride divided by
+ * the number of bits per pixel. Take 32bpp as a
+ * worst case.
+ * No imposed limit on the height, so adopt a square image for want
+ * of anything better.
+ */
+#define UNICAM_MAX_WIDTH		(UNICAM_MAX_BYTESPERLINE / 4)
+#define UNICAM_MAX_HEIGHT		UNICAM_MAX_WIDTH
+/* Define a nominal minimum image size */
+#define UNICAM_MIN_WIDTH		16
+#define UNICAM_MIN_HEIGHT		16
+/* Default size of the embedded buffer */
+#define UNICAM_EMBEDDED_SIZE	16384
+
+/*
+ * Size of the dummy buffer. Can be any size really, but the DMA
+ * allocation works in units of page sizes.
+ */
+#define UNICAM_DUMMY_BUF_SIZE		PAGE_SIZE
+
+#define UNICAM_SD_PAD_SINK		0
+#define UNICAM_SD_PAD_SOURCE_IMAGE	1
+#define UNICAM_SD_PAD_SOURCE_METADATA	2
+#define UNICAM_SD_NUM_PADS		(1 + UNICAM_SD_PAD_SOURCE_METADATA)
+
+static inline bool unicam_sd_pad_is_sink(u32 pad)
+{
+	/* Camera RX has 1 sink pad, and N source pads */
+	return pad == 0;
+}
+
+static inline bool unicam_sd_pad_is_source(u32 pad)
+{
+	/* Camera RX has 1 sink pad, and N source pads */
+	return pad != UNICAM_SD_PAD_SINK;
+}
+
+enum unicam_node_type {
+	UNICAM_IMAGE_NODE,
+	UNICAM_METADATA_NODE,
+	UNICAM_MAX_NODES
+};
+
+/* -----------------------------------------------------------------------------
+ * Structure definitions
+ */
Please remove those unnecessary comments ...
+
+/*
+ * struct unicam_fmt - Unicam media bus format information
+ * @fourcc: V4L2 pixel format FCC identifier. 0 if n/a.
+ * @unpacked_fourcc: V4L2 pixel format FCC identifier if the data is expanded
+ * out to 16bpp. 0 if n/a.
+ * @code: V4L2 media bus format code.
+ * @depth: Bits per pixel as delivered from the source.
+ * @csi_dt: CSI data type.
+ * @metadata_fmt: This format only applies to the metadata pad.
+ */
+struct unicam_fmt {
+	u32	fourcc;
+	u32	unpacked_fourcc;
+	u32	code;
+	u8	depth;
+	u8	csi_dt;
+	u8	metadata_fmt:1;
+};
+
+static const struct unicam_fmt formats[] = {
+	/* YUV Formats */
+	{
+		.fourcc		= V4L2_PIX_FMT_YUYV,
+		.code		= MEDIA_BUS_FMT_YUYV8_1X16,
+		.depth		= 16,
+		.csi_dt		= 0x1e,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_UYVY,
+		.code		= MEDIA_BUS_FMT_UYVY8_1X16,
+		.depth		= 16,
+		.csi_dt		= 0x1e,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_YVYU,
+		.code		= MEDIA_BUS_FMT_YVYU8_1X16,
+		.depth		= 16,
+		.csi_dt		= 0x1e,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_VYUY,
+		.code		= MEDIA_BUS_FMT_VYUY8_1X16,
+		.depth		= 16,
+		.csi_dt		= 0x1e,
+	}, {
+	/* RGB Formats */
+		.fourcc		= V4L2_PIX_FMT_RGB565, /* gggbbbbb rrrrrggg */
+		.code		= MEDIA_BUS_FMT_RGB565_1X16,
+		.depth		= 16,
+		.csi_dt		= 0x22,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_RGB24, /* rgb */
+		.code		= MEDIA_BUS_FMT_RGB888_1X24,
+		.depth		= 24,
+		.csi_dt		= 0x24,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_BGR24, /* bgr */
+		.code		= MEDIA_BUS_FMT_BGR888_1X24,
+		.depth		= 24,
+		.csi_dt		= 0x24,
+	}, {
+	/* Bayer Formats */
+		.fourcc		= V4L2_PIX_FMT_SBGGR8,
+		.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
+		.depth		= 8,
+		.csi_dt		= 0x2a,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGBRG8,
+		.code		= MEDIA_BUS_FMT_SGBRG8_1X8,
+		.depth		= 8,
+		.csi_dt		= 0x2a,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGRBG8,
+		.code		= MEDIA_BUS_FMT_SGRBG8_1X8,
+		.depth		= 8,
+		.csi_dt		= 0x2a,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SRGGB8,
+		.code		= MEDIA_BUS_FMT_SRGGB8_1X8,
+		.depth		= 8,
+		.csi_dt		= 0x2a,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SBGGR10P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SBGGR10,
+		.code		= MEDIA_BUS_FMT_SBGGR10_1X10,
+		.depth		= 10,
+		.csi_dt		= 0x2b,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGBRG10P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SGBRG10,
+		.code		= MEDIA_BUS_FMT_SGBRG10_1X10,
+		.depth		= 10,
+		.csi_dt		= 0x2b,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGRBG10P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SGRBG10,
+		.code		= MEDIA_BUS_FMT_SGRBG10_1X10,
+		.depth		= 10,
+		.csi_dt		= 0x2b,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SRGGB10P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SRGGB10,
+		.code		= MEDIA_BUS_FMT_SRGGB10_1X10,
+		.depth		= 10,
+		.csi_dt		= 0x2b,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SBGGR12P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SBGGR12,
+		.code		= MEDIA_BUS_FMT_SBGGR12_1X12,
+		.depth		= 12,
+		.csi_dt		= 0x2c,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGBRG12P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SGBRG12,
+		.code		= MEDIA_BUS_FMT_SGBRG12_1X12,
+		.depth		= 12,
+		.csi_dt		= 0x2c,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGRBG12P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SGRBG12,
+		.code		= MEDIA_BUS_FMT_SGRBG12_1X12,
+		.depth		= 12,
+		.csi_dt		= 0x2c,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SRGGB12P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SRGGB12,
+		.code		= MEDIA_BUS_FMT_SRGGB12_1X12,
+		.depth		= 12,
+		.csi_dt		= 0x2c,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SBGGR14P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SBGGR14,
+		.code		= MEDIA_BUS_FMT_SBGGR14_1X14,
+		.depth		= 14,
+		.csi_dt		= 0x2d,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGBRG14P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SGBRG14,
+		.code		= MEDIA_BUS_FMT_SGBRG14_1X14,
+		.depth		= 14,
+		.csi_dt		= 0x2d,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SGRBG14P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SGRBG14,
+		.code		= MEDIA_BUS_FMT_SGRBG14_1X14,
+		.depth		= 14,
+		.csi_dt		= 0x2d,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_SRGGB14P,
+		.unpacked_fourcc = V4L2_PIX_FMT_SRGGB14,
+		.code		= MEDIA_BUS_FMT_SRGGB14_1X14,
+		.depth		= 14,
+		.csi_dt		= 0x2d,
+	}, {
+	/*
+	 * 16 bit Bayer formats could be supported, but there is no CSI2
+	 * data_type defined for raw 16, and no sensors that produce it at
+	 * present.
+	 */
+
+	/* Greyscale formats */
+		.fourcc		= V4L2_PIX_FMT_GREY,
+		.code		= MEDIA_BUS_FMT_Y8_1X8,
+		.depth		= 8,
+		.csi_dt		= 0x2a,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_Y10P,
+		.unpacked_fourcc = V4L2_PIX_FMT_Y10,
+		.code		= MEDIA_BUS_FMT_Y10_1X10,
+		.depth		= 10,
+		.csi_dt		= 0x2b,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_Y12P,
+		.unpacked_fourcc = V4L2_PIX_FMT_Y12,
+		.code		= MEDIA_BUS_FMT_Y12_1X12,
+		.depth		= 12,
+		.csi_dt		= 0x2c,
+	}, {
+		.fourcc		= V4L2_PIX_FMT_Y14P,
+		.unpacked_fourcc = V4L2_PIX_FMT_Y14,
+		.code		= MEDIA_BUS_FMT_Y14_1X14,
+		.depth		= 14,
+		.csi_dt		= 0x2d,
+	},
+	/* Embedded data format */
+	{
+		.fourcc		= V4L2_META_FMT_8,
+		.code		= MEDIA_BUS_FMT_METADATA_8,
+		.depth		= 8,
+		.metadata_fmt	= 1,
+	}
+};
+
+struct unicam_buffer {
+	struct vb2_v4l2_buffer vb;
+	struct list_head list;
+};
+
+static inline struct unicam_buffer *to_unicam_buffer(struct vb2_buffer *vb)
+{
+	return container_of(vb, struct unicam_buffer, vb.vb2_buf);
+}
+
+struct unicam_node {
+	bool registered;
+	bool streaming;
+	unsigned int node_id;
+	/* Source pad id on the subdev for this node */
+	unsigned int src_pad_id;
+	/* Pointer pointing to current v4l2_buffer */
+	struct unicam_buffer *cur_frm;
+	/* Pointer pointing to next v4l2_buffer */
+	struct unicam_buffer *next_frm;
+	/* video capture */
+	const struct unicam_fmt *fmt;
+	/* Used to store current pixel format */
+	struct v4l2_format v_fmt;
+	/* Used to store current mbus frame format */
+	struct v4l2_mbus_framefmt m_fmt;
+	/* Buffer queue used in video-buf */
+	struct vb2_queue buffer_queue;
+	/* Queue of filled frames */
+	struct list_head dma_queue;
+	/* IRQ lock for DMA queue */
+	spinlock_t dma_queue_lock;
+	/* lock used to access this structure */
+	struct mutex lock;
+	/* Identifies video device for this channel */
+	struct video_device video_dev;
+	/* Pointer to the parent handle */
+	struct unicam_device *dev;
+	struct media_pad pad;
+	struct media_pipeline pipe;
+	/*
+	 * Dummy buffer intended to be used by unicam
+	 * if we have no other queued buffers to swap to.
+	 */
+	void *dummy_buf_cpu_addr;
+	dma_addr_t dummy_buf_dma_addr;
+
+	/* Stream connected */
+	u32 stream;
+};
+
+struct unicam_device {
+	struct kref kref;
+
+	/* V4l2 specific parameters */
+	struct v4l2_async_subdev asd;
+
+	/* peripheral base address */
+	void __iomem *base;
+	/* clock gating base address */
+	void __iomem *clk_gate_base;
+	/* lp clock handle */
+	struct clk *clock;
+	/* vpu clock handle */
+	struct clk *vpu_clock;
+	/* V4l2 device */
+	struct v4l2_device v4l2_dev;
+	struct media_device mdev;
+
+	/* parent device */
+	struct device *pdev;
+	/* subdevice async Notifier */
+	struct v4l2_async_notifier notifier;
+	unsigned int sequence;
+
+	/* ptr to  sub device */
+	struct v4l2_subdev *sensor;
+
+	/* Internal subdev */
+	struct {
+		struct v4l2_subdev sd;
+		struct media_pad pads[UNICAM_SD_NUM_PADS];
+		bool streaming;
+	} subdev;
+
+	/* Remote pad pointer */
+	struct media_pad *remote_pad;
+
+	enum v4l2_mbus_type bus_type;
+	/*
+	 * Stores bus.mipi_csi2.flags for CSI2 sensors, or
+	 * bus.mipi_csi1.strobe for CCP2.
+	 */
+	unsigned int bus_flags;
+	unsigned int max_data_lanes;
+	unsigned int active_data_lanes;
+	bool sensor_embedded_data;
+
+	struct unicam_node node[UNICAM_MAX_NODES];
+};
+
+static inline struct unicam_device *
+v4l2_device_to_unicam_device(struct v4l2_device *v4l2_dev)
+{
+	return container_of(v4l2_dev, struct unicam_device, v4l2_dev);
+}
+
+static inline struct unicam_device *
+sd_to_unicam_device(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct unicam_device, subdev.sd);
+}
+
+/* -----------------------------------------------------------------------------
+ * Misc helper functions
+ */
+
+static inline bool is_metadata_node(struct unicam_node *node)
+{
+	return node->video_dev.device_caps & V4L2_CAP_META_CAPTURE;
+}
+
+static inline bool is_image_node(struct unicam_node *node)
+{
+	return node->video_dev.device_caps & V4L2_CAP_VIDEO_CAPTURE;
+}
+
+/* Hardware access */
+static inline void unicam_clk_write(struct unicam_device *dev, u32 val)
+{
+	writel(val | 0x5a000000, dev->clk_gate_base);
This magic number has a special meaning for the clock manager. Look at
drivers/clk/bcm/clk-bcm2835.c. Either we use the define CM_PASSWORD or
add at least a comment.

...
+
+static int of_unicam_connect_subdevs(struct unicam_device *dev)
+{
+	struct device *pdev = dev->pdev;
+	struct v4l2_fwnode_endpoint ep = { };
+	struct fwnode_handle *ep_handle;
+	struct v4l2_async_subdev *asd;
+	unsigned int lane;
+	int ret = -EINVAL;
+
+	if (of_property_read_u32(pdev->of_node, "num-data-lanes",
After adding the vendor prefix in the binding, i think this should be
updated?
+				 &dev->max_data_lanes) < 0) {
+		dev_err(dev->v4l2_dev.dev, "number of data lanes not set\n");
+		return -EINVAL;
+	}
+
...
+
+static int unicam_probe(struct platform_device *pdev)
+{
+	struct unicam_device *unicam;
+	int ret = 0;
+
+	unicam = kzalloc(sizeof(*unicam), GFP_KERNEL);
+	if (!unicam)
+		return -ENOMEM;
+
+	kref_init(&unicam->kref);
+	unicam->pdev = &pdev->dev;
+
+	unicam->base = devm_platform_ioremap_resource(pdev, 0);
After adding the reg-names to the binding the driver can access the
register by name:

devm_platform_ioremap_resource_byname()

This is easier to read/understand.

Best regards



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help