lkml.org 
[lkml]   [2022]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH v4 05/12] media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface
    From
    Date
    Hi Jean-Michel,

    Am 03.02.22 um 18:50 schrieb Jean-Michel Hautbois:
    > 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 <naush@raspberrypi.com>
    > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
    >
    > ---
    > 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
    > 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


    \
     
     \ /
      Last update: 2022-02-03 21:46    [W:3.029 / U:1.924 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site