[{"id":1773465,"web_url":"http://patchwork.ozlabs.org/comment/1773465/","msgid":"<927e6bd8-bdcd-4415-8d41-422a6da1220a@xs4all.nl>","list_archive_url":null,"date":"2017-09-22T10:37:00","subject":"Re: [PATCH v3 3/4] [media] bcm2835-unicam: Driver for CCP2/CSI2\n\tcamera interface","submitter":{"id":723,"url":"http://patchwork.ozlabs.org/api/people/723/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"Hi Dave,\n\nThank you for working on this!\n\nSee my review below:\n\nOn 20/09/17 18:07, Dave Stevenson wrote:\n> Add driver for the Unicam camera receiver block on\n> BCM283x processors.\n> \n> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>\n> ---\n> \n> Changes from v2.\n> - Don't store the irq value as it isn't used outside the probe.\n> - Change PIX_FMT_ALL_ defines to avoid potential clashes with 4CCs.\n> - Clock now called \"lp\" and not \"lp_clock\".\n> - Minor description changes.\n> \n>  drivers/media/platform/Kconfig                   |    1 +\n>  drivers/media/platform/Makefile                  |    1 +\n>  drivers/media/platform/bcm2835/Kconfig           |   14 +\n>  drivers/media/platform/bcm2835/Makefile          |    3 +\n>  drivers/media/platform/bcm2835/bcm2835-unicam.c  | 2192 ++++++++++++++++++++++\n>  drivers/media/platform/bcm2835/vc4-regs-unicam.h |  264 +++\n>  6 files changed, 2475 insertions(+)\n>  create mode 100644 drivers/media/platform/bcm2835/Kconfig\n>  create mode 100644 drivers/media/platform/bcm2835/Makefile\n>  create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c\n>  create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h\n> \n> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig\n> index 7e7cc49..1e5f004 100644\n> --- a/drivers/media/platform/Kconfig\n> +++ b/drivers/media/platform/Kconfig\n> @@ -150,6 +150,7 @@ source \"drivers/media/platform/am437x/Kconfig\"\n>  source \"drivers/media/platform/xilinx/Kconfig\"\n>  source \"drivers/media/platform/rcar-vin/Kconfig\"\n>  source \"drivers/media/platform/atmel/Kconfig\"\n> +source \"drivers/media/platform/bcm2835/Kconfig\"\n>  \n>  config VIDEO_TI_CAL\n>  \ttristate \"TI CAL (Camera Adaptation Layer) driver\"\n> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile\n> index c1ef946..045de3f 100644\n> --- a/drivers/media/platform/Makefile\n> +++ b/drivers/media/platform/Makefile\n> @@ -90,3 +90,4 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)\t\t+= qcom/camss-8x16/\n>  obj-$(CONFIG_VIDEO_QCOM_VENUS)\t\t+= qcom/venus/\n>  \n>  obj-y\t\t\t\t\t+= meson/\n> +obj-y\t\t\t\t\t+= bcm2835/\n> diff --git a/drivers/media/platform/bcm2835/Kconfig b/drivers/media/platform/bcm2835/Kconfig\n> new file mode 100644\n> index 0000000..6a74842\n> --- /dev/null\n> +++ b/drivers/media/platform/bcm2835/Kconfig\n> @@ -0,0 +1,14 @@\n> +# Broadcom VideoCore4 V4L2 camera support\n> +\n> +config VIDEO_BCM2835_UNICAM\n> +\ttristate \"Broadcom BCM2835 Unicam video capture driver\"\n> +\tdepends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API\n> +\tdepends on ARCH_BCM2835 || COMPILE_TEST\n> +\tselect VIDEOBUF2_DMA_CONTIG\n> +\tselect V4L2_FWNODE\n> +\t---help---\n> +\t  Say Y here to enable V4L2 subdevice for CSI2 receiver.\n> +\t  This is a V4L2 subdevice that interfaces directly to the VC4 peripheral.\n> +\n> +\t   To compile this driver as a module, choose M here. The module\n> +\t   will be called bcm2835-unicam.\n> diff --git a/drivers/media/platform/bcm2835/Makefile b/drivers/media/platform/bcm2835/Makefile\n> new file mode 100644\n> index 0000000..a98aba0\n> --- /dev/null\n> +++ b/drivers/media/platform/bcm2835/Makefile\n> @@ -0,0 +1,3 @@\n> +# Makefile for BCM2835 Unicam driver\n> +\n> +obj-$(CONFIG_VIDEO_BCM2835_UNICAM) += bcm2835-unicam.o\n> diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c b/drivers/media/platform/bcm2835/bcm2835-unicam.c\n> new file mode 100644\n> index 0000000..93831fb\n> --- /dev/null\n> +++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c\n> @@ -0,0 +1,2192 @@\n> +/*\n> + * BCM2835 Unicam capture Driver\n> + *\n> + * Copyright (C) 2017 - Raspberry Pi (Trading) Ltd.\n> + *\n> + * Dave Stevenson <dave.stevenson@raspberrypi.org>\n> + *\n> + * Based on TI am437x driver by Benoit Parrot and Lad, Prabhakar and\n> + * TI CAL camera interface driver by Benoit Parrot.\n> + *\n> + *\n> + * There are two camera drivers in the kernel for BCM283x - this one\n> + * and bcm2835-camera (currently in staging).\n> + *\n> + * This driver directly controls the Unicam peripheral - there is no\n> + * involvement with the VideoCore firmware. Unicam receives CSI-2 or\n> + * CCP2 data and writes it into SDRAM. The only processing options are\n> + * to repack Bayer data into an alternate format, and applying windowing\n> + * (currently not implemented).\n> + * It should be possible to connect it to any sensor with a\n> + * suitable output interface and V4L2 subdevice driver.\n> + *\n> + * bcm2835-camera uses the VideoCore firmware to control the sensor,\n> + * Unicam, ISP, and all tuner control loops. Fully processed frames are\n> + * delivered to the driver by the firmware. It only has sensor drivers\n> + * for Omnivision OV5647, and Sony IMX219 sensors.\n> + *\n> + * The two drivers are mutually exclusive for the same Unicam instance.\n> + * The VideoCore firmware checks the device tree configuration during boot.\n> + * If it finds device tree nodes called csi0 or csi1 it will block the\n> + * firmware from accessing the peripheral, and bcm2835-camera will\n> + * not be able to stream data.\n> + *\n> + *\n> + * This program is free software; you may redistribute it and/or modify\n> + * it under the terms of the GNU General Public License as published by\n> + * the Free Software Foundation; version 2 of the License.\n> + *\n> + * THE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND,\n> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF\n> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND\n> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS\n> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN\n> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN\n> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\n> + * SOFTWARE.\n> + */\n> +\n> +#include <linux/clk.h>\n> +#include <linux/delay.h>\n> +#include <linux/device.h>\n> +#include <linux/err.h>\n> +#include <linux/init.h>\n> +#include <linux/interrupt.h>\n> +#include <linux/io.h>\n> +#include <linux/module.h>\n> +#include <linux/of_device.h>\n> +#include <linux/of_graph.h>\n> +#include <linux/pinctrl/consumer.h>\n> +#include <linux/platform_device.h>\n> +#include <linux/pm_runtime.h>\n> +#include <linux/slab.h>\n> +#include <linux/uaccess.h>\n> +#include <linux/videodev2.h>\n> +\n> +#include <media/v4l2-common.h>\n> +#include <media/v4l2-ctrls.h>\n> +#include <media/v4l2-dev.h>\n> +#include <media/v4l2-device.h>\n> +#include <media/v4l2-dv-timings.h>\n> +#include <media/v4l2-event.h>\n> +#include <media/v4l2-ioctl.h>\n> +#include <media/v4l2-fwnode.h>\n> +#include <media/videobuf2-v4l2.h>\n\nYou can drop this include, videobuf2-dma-contig.h pulls it in for you.\n\n> +#include <media/videobuf2-dma-contig.h>\n> +\n> +#include \"vc4-regs-unicam.h\"\n> +\n> +#define UNICAM_MODULE_NAME\t\"unicam\"\n> +#define UNICAM_VERSION\t\t\"0.1.0\"\n> +\n> +static int debug;\n> +module_param(debug, int, 0644);\n> +MODULE_PARM_DESC(debug, \"Debug level 0-3\");\n> +\n> +#define unicam_dbg(level, dev, fmt, arg...)\t\\\n> +\t\tv4l2_dbg(level, debug, &(dev)->v4l2_dev, fmt, ##arg)\n> +#define unicam_info(dev, fmt, arg...)\t\\\n> +\t\tv4l2_info(&(dev)->v4l2_dev, fmt, ##arg)\n> +#define unicam_err(dev, fmt, arg...)\t\\\n> +\t\tv4l2_err(&(dev)->v4l2_dev, fmt, ##arg)\n> +\n> +/*\n> + * Stride is a 16 bit register, but also has to be a multiple of 16.\n> + */\n> +#define BPL_ALIGNMENT\t\t16\n> +#define MAX_BYTESPERLINE\t((1 << 16) - BPL_ALIGNMENT)\n> +/*\n> + * Max width is therefore determined by the max stride divided by\n> + * the number of bits per pixel. Take 32bpp as a\n> + * worst case.\n> + * No imposed limit on the height, so adopt a square image for want\n> + * of anything better.\n> + */\n> +#define MAX_WIDTH\t(MAX_BYTESPERLINE / 4)\n> +#define MAX_HEIGHT\tMAX_WIDTH\n> +/* Define a nominal minimum image size */\n> +#define MIN_WIDTH\t16\n> +#define MIN_HEIGHT\t16\n> +/*\n> + * Whilst Unicam doesn't require any additional padding on the image\n> + * height, various other parts of the BCM283x frameworks require a multiple\n> + * of 16.\n> + * Seeing as image buffers are significantly larger than this extra\n> + * padding, add it in order to simplify integration.\n> + */\n> +#define HEIGHT_ALIGNMENT\t16\n> +\n> +/*\n> + * struct unicam_fmt - Unicam media bus format information\n> + * @pixelformat: V4L2 pixel format FCC identifier.\n> + * @code: V4L2 media bus format code.\n> + * @depth: Bits per pixel (when stored in memory).\n> + * @csi_dt: CSI data type.\n> + */\n> +struct unicam_fmt {\n> +\tu32\tfourcc;\n> +\tu32\tcode;\n> +\tu8\tdepth;\n> +\tu8\tcsi_dt;\n> +};\n> +\n> +/*\n> + * The peripheral can unpack and repack between several of\n> + * the Bayer raw formats, so any Bayer format can be advertised\n> + * as the same Bayer order in each of the supported bit depths.\n> + * These values are used as flags to denote that all Bayer formats\n> + * in the specified order should be advertised. They are not\n> + * used outside of this driver but must avoid collisions with\n> + * the standard V4L2_PIX_FMT_ values, hence 0xFFFFFFxx.\n> + */\n> +#define PIX_FMT_ALL_BGGR  0xFFFFFFF0\n> +#define PIX_FMT_ALL_RGGB  0xFFFFFFF1\n> +#define PIX_FMT_ALL_GBRG  0xFFFFFFF2\n> +#define PIX_FMT_ALL_GRBG  0xFFFFFFF3\n\nUse lower case 'f' (linux coding style)\n\n> +\n> +static const struct unicam_fmt formats[] = {\n> +\t/* YUV Formats */\n> +\t{\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_YUYV,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_YUYV8_2X8,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_UYVY,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_UYVY8_2X8,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_YVYU,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_YVYU8_2X8,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_VYUY,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_VYUY8_2X8,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_YUYV,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_YUYV8_1X16,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_UYVY,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_UYVY8_1X16,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_YVYU,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_YVYU8_1X16,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_VYUY,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_VYUY8_1X16,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x1e,\n> +\t}, {\n> +\t/* RGB Formats */\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_RGB565, /* gggbbbbb rrrrrggg */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_RGB565_2X8_LE,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x22,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_RGB565X, /* rrrrrggg gggbbbbb */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_RGB565_2X8_BE,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x22\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_RGB555, /* gggbbbbb arrrrrgg */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x21,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_RGB555X, /* arrrrrgg gggbbbbb */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE,\n> +\t\t.depth\t\t= 16,\n> +\t\t.csi_dt\t\t= 0x21,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_RGB24, /* rgb */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_RGB888_1X24,\n> +\t\t.depth\t\t= 24,\n> +\t\t.csi_dt\t\t= 0x24,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_BGR24, /* bgr */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_BGR888_1X24,\n> +\t\t.depth\t\t= 24,\n> +\t\t.csi_dt\t\t= 0x24,\n> +\t}, {\n> +\t\t.fourcc\t\t= V4L2_PIX_FMT_RGB32, /* argb */\n> +\t\t.code\t\t= MEDIA_BUS_FMT_ARGB8888_1X32,\n> +\t\t.depth\t\t= 32,\n> +\t\t.csi_dt\t\t= 0x0,\n> +\t}, {\n> +\t/* Bayer Formats */\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_BGGR,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SBGGR8_1X8,\n> +\t\t.depth\t\t= 8,\n> +\t\t.csi_dt\t\t= 0x2a,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GBRG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGBRG8_1X8,\n> +\t\t.depth\t\t= 8,\n> +\t\t.csi_dt\t\t= 0x2a,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GRBG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGRBG8_1X8,\n> +\t\t.depth\t\t= 8,\n> +\t\t.csi_dt\t\t= 0x2a,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_RGGB,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SRGGB8_1X8,\n> +\t\t.depth\t\t= 8,\n> +\t\t.csi_dt\t\t= 0x2a,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_BGGR,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SBGGR10_1X10,\n> +\t\t.depth\t\t= 10,\n> +\t\t.csi_dt\t\t= 0x2b,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GBRG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGBRG10_1X10,\n> +\t\t.depth\t\t= 10,\n> +\t\t.csi_dt\t\t= 0x2b,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GRBG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGRBG10_1X10,\n> +\t\t.depth\t\t= 10,\n> +\t\t.csi_dt\t\t= 0x2b,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_RGGB,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SRGGB10_1X10,\n> +\t\t.depth\t\t= 10,\n> +\t\t.csi_dt\t\t= 0x2b,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_BGGR,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SBGGR12_1X12,\n> +\t\t.depth\t\t= 12,\n> +\t\t.csi_dt\t\t= 0x2c,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GBRG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGBRG12_1X12,\n> +\t\t.depth\t\t= 12,\n> +\t\t.csi_dt\t\t= 0x2c,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GRBG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGRBG12_1X12,\n> +\t\t.depth\t\t= 12,\n> +\t\t.csi_dt\t\t= 0x2c,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_RGGB,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SRGGB12_1X12,\n> +\t\t.depth\t\t= 12,\n> +\t\t.csi_dt\t\t= 0x2c,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_BGGR,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SBGGR14_1X14,\n> +\t\t.depth\t\t= 14,\n> +\t\t.csi_dt\t\t= 0x2d,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GBRG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGBRG14_1X14,\n> +\t\t.depth\t\t= 14,\n> +\t\t.csi_dt\t\t= 0x2d,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_GRBG,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SGRBG14_1X14,\n> +\t\t.depth\t\t= 14,\n> +\t\t.csi_dt\t\t= 0x2d,\n> +\t}, {\n> +\t\t.fourcc\t\t= PIX_FMT_ALL_RGGB,\n> +\t\t.code\t\t= MEDIA_BUS_FMT_SRGGB14_1X14,\n> +\t\t.depth\t\t= 14,\n> +\t\t.csi_dt\t\t= 0x2d,\n> +\t},\n> +};\n> +\n> +struct bayer_fmt {\n> +\tu32 fourcc;\n> +\tu8 depth;\n> +};\n> +\n> +const struct bayer_fmt all_bayer_bggr[] = {\n> +\t{V4L2_PIX_FMT_SBGGR8,\t8},\n> +\t{V4L2_PIX_FMT_SBGGR10P,\t10},\n> +\t{V4L2_PIX_FMT_SBGGR12,\t12},\n> +\t{V4L2_PIX_FMT_SBGGR16,\t16},\n> +\t{0,\t\t\t0}\n> +};\n> +\n> +const struct bayer_fmt all_bayer_rggb[] = {\n> +\t{V4L2_PIX_FMT_SRGGB8,\t8},\n> +\t{V4L2_PIX_FMT_SRGGB10P,\t10},\n> +\t{V4L2_PIX_FMT_SRGGB12,\t12},\n> +\t{V4L2_PIX_FMT_SRGGB16,\t16},\n> +\t{0,\t\t\t0}\n> +};\n> +\n> +const struct bayer_fmt all_bayer_gbrg[] = {\n> +\t{V4L2_PIX_FMT_SGBRG8,\t8},\n> +\t{V4L2_PIX_FMT_SGBRG10P,\t10},\n> +\t{V4L2_PIX_FMT_SGBRG12,\t12},\n> +\t{V4L2_PIX_FMT_SGBRG16,\t16},\n> +\t{0,\t\t\t0}\n> +};\n> +\n> +const struct bayer_fmt all_bayer_grbg[] = {\n> +\t{V4L2_PIX_FMT_SGRBG8,\t8},\n> +\t{V4L2_PIX_FMT_SGRBG10P,\t10},\n> +\t{V4L2_PIX_FMT_SGRBG12,\t12},\n> +\t{V4L2_PIX_FMT_SGRBG16,\t16},\n> +\t{0,\t\t\t0}\n> +};\n> +\n> +struct unicam_dmaqueue {\n> +\tstruct list_head\tactive;\n> +};\n> +\n> +struct unicam_buffer {\n> +\tstruct vb2_v4l2_buffer vb;\n> +\tstruct list_head list;\n> +};\n> +\n> +struct unicam_cfg {\n> +\t/* peripheral base address */\n> +\tvoid __iomem *base;\n> +\t/* clock gating base address */\n> +\tvoid __iomem *clk_gate_base;\n> +};\n> +\n> +#define MAX_POSSIBLE_FMTS \\\n> +\t\t(ARRAY_SIZE(formats) + \\\n> +\t\tARRAY_SIZE(all_bayer_bggr) + \\\n> +\t\tARRAY_SIZE(all_bayer_rggb) + \\\n> +\t\tARRAY_SIZE(all_bayer_grbg) + \\\n> +\t\tARRAY_SIZE(all_bayer_gbrg))\n\nYou're counting the bayer formats in the formats[] array as well, but\nthat's not really correct, is it? Those shouldn't be counted since the\nall_bayer_* arrays take care of those formats. I'm not sure what the best\noption is here. One is to split off the bayer formats into their own\nbayer_formats array.\n\nBTW, I assume we're talking MAX_POSSIBLE_PIX_FMTS (might be a better name).\n\n> +\n> +struct unicam_device {\n> +\t/* V4l2 specific parameters */\n> +\t/* Identifies video device for this channel */\n> +\tstruct video_device video_dev;\n> +\tstruct v4l2_ctrl_handler ctrl_handler;\n> +\n> +\tstruct v4l2_fwnode_endpoint endpoint;\n> +\n> +\tstruct v4l2_async_subdev asd;\n> +\n> +\t/* unicam cfg */\n> +\tstruct unicam_cfg cfg;\n> +\t/* clock handle */\n> +\tstruct clk *clock;\n> +\t/* V4l2 device */\n> +\tstruct v4l2_device v4l2_dev;\n> +\t/* parent device */\n> +\tstruct platform_device *pdev;\n> +\t/* subdevice async Notifier */\n> +\tstruct v4l2_async_notifier notifier;\n> +\tunsigned int sequence;\n> +\n> +\t/* ptr to  sub device */\n> +\tstruct v4l2_subdev *sensor;\n> +\t/* Pad config for the sensor */\n> +\tstruct v4l2_subdev_pad_config *sensor_config;\n> +\t/* current input at the sub device */\n> +\tint current_input;\n> +\n> +\t/* Pointer pointing to current v4l2_buffer */\n> +\tstruct unicam_buffer *cur_frm;\n> +\t/* Pointer pointing to next v4l2_buffer */\n> +\tstruct unicam_buffer *next_frm;\n> +\n> +\t/* video capture */\n> +\tconst struct unicam_fmt\t*fmt;\n> +\t/* Used to store current pixel format */\n> +\tstruct v4l2_format v_fmt;\n> +\t/* Used to store current mbus frame format */\n> +\tstruct v4l2_mbus_framefmt m_fmt;\n> +\n> +\tstruct unicam_fmt active_fmts[MAX_POSSIBLE_FMTS];\n> +\tint num_active_fmt;\n> +\tunsigned int virtual_channel;\n> +\tenum v4l2_mbus_type bus_type;\n> +\t/*\n> +\t * Stores bus.mipi_csi2.flags for CSI2 sensors, or\n> +\t * bus.mipi_csi1.strobe for CCP2.\n> +\t */\n> +\tunsigned int bus_flags;\n> +\tunsigned int max_data_lanes;\n> +\tunsigned int active_data_lanes;\n> +\n> +\tstruct v4l2_rect crop;\n> +\n> +\t/* Currently selected input on subdev */\n> +\tint input;\n> +\n> +\t/* Buffer queue used in video-buf */\n> +\tstruct vb2_queue buffer_queue;\n> +\t/* Queue of filled frames */\n> +\tstruct unicam_dmaqueue dma_queue;\n> +\t/* IRQ lock for DMA queue */\n> +\tspinlock_t dma_queue_lock;\n> +\t/* lock used to access this structure */\n> +\tstruct mutex lock;\n> +\t/* Flag to denote that we are processing buffers */\n> +\tint streaming;\n> +};\n> +\n> +/* Hardware access */\n> +#define clk_write(dev, val) writel((val) | 0x5a000000, (dev)->clk_gate_base)\n> +#define clk_read(dev) readl((dev)->clk_gate_base)\n> +\n> +#define reg_read(dev, offset) readl((dev)->base + (offset))\n> +#define reg_write(dev, offset, val) writel(val, (dev)->base + (offset))\n> +\n> +#define reg_read_field(dev, offset, mask) get_field(reg_read((dev), (offset), \\\n> +\t\t\t\t\t\t    mask))\n> +\n> +static inline int get_field(u32 value, u32 mask)\n> +{\n> +\treturn (value & mask) >> __ffs(mask);\n> +}\n> +\n> +static inline void set_field(u32 *valp, u32 field, u32 mask)\n> +{\n> +\tu32 val = *valp;\n> +\n> +\tval &= ~mask;\n> +\tval |= (field << __ffs(mask)) & mask;\n> +\t*valp = val;\n> +}\n> +\n> +static inline void reg_write_field(struct unicam_cfg *dev, u32 offset,\n> +\t\t\t\t   u32 field, u32 mask)\n> +{\n> +\tu32 val = reg_read((dev), (offset));\n> +\n> +\tset_field(&val, field, mask);\n> +\treg_write((dev), (offset), val);\n> +}\n> +\n> +/* Power management functions */\n> +static inline int unicam_runtime_get(struct unicam_device *dev)\n> +{\n> +\tint r;\n> +\n> +\tr = pm_runtime_get_sync(&dev->pdev->dev);\n> +\n> +\treturn r;\n> +}\n> +\n> +static inline void unicam_runtime_put(struct unicam_device *dev)\n> +{\n> +\tpm_runtime_put_sync(&dev->pdev->dev);\n> +}\n> +\n> +/* Format setup functions */\n> +static int find_mbus_depth_by_code(u32 code)\n> +{\n> +\tconst struct unicam_fmt *fmt;\n> +\tunsigned int k;\n> +\n> +\tfor (k = 0; k < ARRAY_SIZE(formats); k++) {\n> +\t\tfmt = &formats[k];\n> +\t\tif (fmt->code == code)\n> +\t\t\treturn fmt->depth;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct unicam_fmt *find_format_by_code(struct unicam_device *dev,\n> +\t\t\t\t\t\t    u32 code)\n> +{\n> +\tconst struct unicam_fmt *fmt;\n> +\tunsigned int k;\n> +\n> +\tfor (k = 0; k < dev->num_active_fmt; k++) {\n> +\t\tfmt = &dev->active_fmts[k];\n> +\t\tif (fmt->code == code)\n> +\t\t\treturn fmt;\n> +\t}\n> +\n> +\treturn NULL;\n> +}\n> +\n> +static const struct unicam_fmt *find_format_by_pix(struct unicam_device *dev,\n> +\t\t\t\t\t\t   u32 pixelformat)\n> +{\n> +\tconst struct unicam_fmt *fmt;\n> +\tunsigned int k;\n> +\n> +\tfor (k = 0; k < dev->num_active_fmt; k++) {\n> +\t\tfmt = &dev->active_fmts[k];\n> +\t\tif (fmt->fourcc == pixelformat)\n> +\t\t\treturn fmt;\n> +\t}\n> +\n> +\treturn NULL;\n> +}\n> +\n> +static void dump_active_formats(struct unicam_device *dev)\n> +{\n> +\tint i;\n> +\tchar fourcc_str[V4L2_FOURCC_MAX_SIZE];\n\nSwap these lines. A short line followed by a long line looks odd and there\nis no reason for it in this case.\n\n> +\n> +\tfor (i = 0; i < dev->num_active_fmt; i++) {\n> +\t\tunicam_dbg(3, dev, \"active_fmt[%d] (%p) is code %04X, fourcc %s, depth %d\\n\",\n> +\t\t\t   i, &dev->active_fmts[i], dev->active_fmts[i].code,\n> +\t\t\t   v4l2_fourcc2s(dev->active_fmts[i].fourcc,\n> +\t\t\t\t\t fourcc_str),\n> +\t\t\t   dev->active_fmts[i].depth);\n> +\t}\n> +}\n> +\n> +static inline unsigned int bytes_per_line(u32 width,\n> +\t\t\t\t\t  const struct unicam_fmt *fmt)\n> +{\n> +\treturn ALIGN((width * fmt->depth) >> 3,  BPL_ALIGNMENT);\n\nTwo spaces before BPL, should be one.\n\n> +}\n> +\n> +static int __subdev_get_format(struct unicam_device *dev,\n> +\t\t\t       struct v4l2_mbus_framefmt *fmt)\n> +{\n> +\tstruct v4l2_subdev_format sd_fmt = {0};\n> +\tstruct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;\n> +\tint ret;\n> +\n> +\tsd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> +\tsd_fmt.pad = 0;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, pad, get_fmt, dev->sensor_config,\n> +\t\t\t       &sd_fmt);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tif (mbus_fmt->field != V4L2_FIELD_NONE) {\n> +\t\t/*\n> +\t\t * No support for receiving interlaced video, so try to\n> +\t\t * disable it if reported.\n> +\t\t */\n> +\t\tmbus_fmt->field = V4L2_FIELD_NONE;\n> +\t\tret = v4l2_subdev_call(dev->sensor, pad, set_fmt,\n> +\t\t\t\t       dev->sensor_config,\n> +\t\t\t\t       &sd_fmt);\n\nYou're not checking ret here?\n\nAnyway, this is wrong. get_fmt should never change the format of the\nsubdev. The initial sensor format that is chosen by the driver when it\nis loaded should never be interlaced, and when enumerating formats all\ninterlaced formats must be skipped.\n\n> +\t}\n> +\t*fmt = *mbus_fmt;\n> +\n> +\tunicam_dbg(1, dev, \"%s %dx%d code:%04X\\n\", __func__,\n> +\t\t   fmt->width, fmt->height, fmt->code);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int __subdev_set_format(struct unicam_device *dev,\n> +\t\t\t       struct v4l2_mbus_framefmt *fmt)\n> +{\n> +\tstruct v4l2_subdev_format sd_fmt = {\n> +\t\t.which = V4L2_SUBDEV_FORMAT_ACTIVE,\n> +\t};\n> +\tstruct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;\n> +\tint ret;\n> +\n> +\t*mbus_fmt = *fmt;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, pad, set_fmt, dev->sensor_config,\n> +\t\t\t       &sd_fmt);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tunicam_dbg(1, dev, \"%s %dx%d code:%04X\\n\", __func__,\n> +\t\t   fmt->width, fmt->height, fmt->code);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_calc_format_size_bpl(struct unicam_device *dev,\n> +\t\t\t\t       const struct unicam_fmt *fmt,\n> +\t\t\t\t       struct v4l2_format *f)\n> +{\n> +\tunsigned int min_bytesperline;\n> +\tchar fourcc_str[V4L2_FOURCC_MAX_SIZE];\n\nSwap these two lines.\n\n> +\n> +\tif (!fmt) {\n> +\t\tunicam_dbg(3, dev, \"No unicam_fmt provided!\\n\");\n> +\t\treturn -EINVAL;\n\nCan this ever happen?\n\n> +\t}\n> +\n> +\tv4l_bound_align_image(&f->fmt.pix.width, MIN_WIDTH, MAX_WIDTH, 2,\n> +\t\t\t      &f->fmt.pix.height, MIN_HEIGHT, MAX_HEIGHT, 0,\n> +\t\t\t      0);\n> +\n> +\tmin_bytesperline = bytes_per_line(f->fmt.pix.width, fmt);\n> +\n> +\tif (f->fmt.pix.bytesperline > min_bytesperline &&\n> +\t    f->fmt.pix.bytesperline <= MAX_BYTESPERLINE)\n> +\t\tf->fmt.pix.bytesperline = ALIGN(f->fmt.pix.bytesperline,\n> +\t\t\t\t\t\tBPL_ALIGNMENT);\n> +\telse\n> +\t\tf->fmt.pix.bytesperline = min_bytesperline;\n> +\n> +\t/* Align height up for compatibility with other hardware blocks */\n> +\tf->fmt.pix.sizeimage = ALIGN(f->fmt.pix.height, HEIGHT_ALIGNMENT) *\n> +\t\t\t       f->fmt.pix.bytesperline;\n> +\n> +\tunicam_dbg(3, dev, \"%s: fourcc: %s size: %dx%d bpl:%d img_size:%d\\n\",\n> +\t\t   __func__, v4l2_fourcc2s(f->fmt.pix.pixelformat, fourcc_str),\n> +\t\t   f->fmt.pix.width, f->fmt.pix.height,\n> +\t\t   f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_reset_format(struct unicam_device *dev)\n> +{\n> +\tstruct v4l2_mbus_framefmt mbus_fmt;\n> +\tint ret;\n> +\n> +\tret = __subdev_get_format(dev, &mbus_fmt);\n> +\tif (ret) {\n> +\t\tunicam_err(dev, \"Failed to get_format - ret %d\\n\", ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (mbus_fmt.code != dev->fmt->code) {\n> +\t\tunicam_err(dev, \"code mismatch - fmt->code %08X, mbus_fmt.code %08X\\n\",\n\nWhy 'X' instead of 'x'?\n\n> +\t\t\t   dev->fmt->code, mbus_fmt.code);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tv4l2_fill_pix_format(&dev->v_fmt.fmt.pix, &mbus_fmt);\n> +\tdev->v_fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;\n> +\n> +\tunicam_calc_format_size_bpl(dev, dev->fmt, &dev->v_fmt);\n> +\n> +\tdev->m_fmt = mbus_fmt;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static void unicam_wr_dma_addr(struct unicam_device *dev, unsigned int dmaaddr)\n> +{\n> +\tunicam_dbg(1, dev, \"wr_dma_addr %08X-%08X\\n\",\n> +\t\t   dmaaddr, dmaaddr + dev->v_fmt.fmt.pix.sizeimage);\n> +\treg_write(&dev->cfg, UNICAM_IBSA0, dmaaddr);\n> +\treg_write(&dev->cfg,\n> +\t\t  UNICAM_IBEA0,\n\nThis can be joined with the reg_write line. No need to put this on a separate line.\n\n> +\t\t  dmaaddr + dev->v_fmt.fmt.pix.sizeimage);\n> +\treg_write(&dev->cfg, UNICAM_DBSA0, (uint32_t)dmaaddr);\n> +\treg_write(&dev->cfg, UNICAM_DBEA0, (uint32_t)dmaaddr + (16 << 10));\n\n16 << 10 is a bit magic. Should it be a define? Or at least add a comment.\n\n> +}\n> +\n> +static inline void unicam_schedule_next_buffer(struct unicam_device *dev)\n> +{\n> +\tstruct unicam_dmaqueue *dma_q = &dev->dma_queue;\n> +\tstruct unicam_buffer *buf;\n> +\tunsigned long addr;\n\nUse dma_addr_t\n\n> +\n> +\tbuf = list_entry(dma_q->active.next, struct unicam_buffer, list);\n> +\tdev->next_frm = buf;\n> +\tlist_del(&buf->list);\n> +\n> +\taddr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);\n> +\tunicam_wr_dma_addr(dev, addr);\n> +}\n> +\n> +static inline void unicam_process_buffer_complete(struct unicam_device *dev)\n> +{\n> +\tdev->cur_frm->vb.field = dev->m_fmt.field;\n> +\tdev->cur_frm->vb.sequence = dev->sequence++;\n> +\n> +\tvb2_buffer_done(&dev->cur_frm->vb.vb2_buf, VB2_BUF_STATE_DONE);\n> +\tdev->cur_frm = dev->next_frm;\n> +}\n> +\n> +/*\n> + * unicam_isr : ISR handler for unicam capture\n> + * @irq: irq number\n> + * @dev_id: dev_id ptr\n> + *\n> + * It changes status of the captured buffer, takes next buffer from the queue\n> + * and sets its address in unicam registers\n> + */\n> +static irqreturn_t unicam_isr(int irq, void *dev)\n> +{\n> +\tstruct unicam_device *unicam = (struct unicam_device *)dev;\n> +\tint ista, sta;\n> +\tstruct unicam_cfg *cfg = &unicam->cfg;\n> +\tstruct unicam_dmaqueue *dma_q = &unicam->dma_queue;\n> +\n> +\t/*\n> +\t * Don't service interrupts if not streaming.\n> +\t * Avoids issues if the VPU should enable the\n> +\t * peripheral without the kernel knowing (that\n> +\t * shouldn't happen, but causes issues if it does).\n> +\t */\n> +\tif (!unicam->streaming)\n> +\t\treturn IRQ_HANDLED;\n> +\n> +\tsta = reg_read(cfg, UNICAM_STA);\n> +\t/* Write value back to clear the interrupts */\n> +\treg_write(cfg, UNICAM_STA, sta);\n> +\n> +\tista = reg_read(cfg, UNICAM_ISTA);\n> +\t/* Write value back to clear the interrupts */\n> +\treg_write(cfg, UNICAM_ISTA, ista);\n> +\n> +\tif (!(sta && (UNICAM_IS | UNICAM_PI0)))\n> +\t\treturn IRQ_HANDLED;\n> +\n> +\tif (ista & UNICAM_FSI) {\n> +\t\t/*\n> +\t\t * Timestamp is to be when the first data byte was captured,\n> +\t\t * aka frame start.\n> +\t\t */\n> +\t\tif (unicam->cur_frm)\n> +\t\t\tunicam->cur_frm->vb.vb2_buf.timestamp = ktime_get_ns();\n> +\t}\n> +\tif (ista & UNICAM_FEI || sta & UNICAM_PI0) {\n> +\t\t/*\n> +\t\t * Ensure we have swapped buffers already as we can't\n> +\t\t * stop the peripheral. Overwrite the frame we've just\n> +\t\t * captured instead.\n> +\t\t */\n> +\t\tif (unicam->cur_frm &&\n> +\t\t    unicam->cur_frm != unicam->next_frm)\n\nI'd join these lines. Or does this get over 80 cols?\n\n> +\t\t\tunicam_process_buffer_complete(unicam);\n> +\t}\n> +\n> +\tif (ista & (UNICAM_FSI | UNICAM_LCI)) {\n> +\t\tspin_lock(&unicam->dma_queue_lock);\n> +\t\tif (!list_empty(&dma_q->active) &&\n> +\t\t    unicam->cur_frm == unicam->next_frm)\n> +\t\t\tunicam_schedule_next_buffer(unicam);\n> +\t\tspin_unlock(&unicam->dma_queue_lock);\n> +\t}\n> +\n> +\tif (reg_read(&unicam->cfg, UNICAM_ICTL) & UNICAM_FCM) {\n> +\t\t/* Switch out of trigger mode if selected */\n> +\t\treg_write_field(&unicam->cfg, UNICAM_ICTL, 1, UNICAM_TFC);\n> +\t\treg_write_field(&unicam->cfg, UNICAM_ICTL, 0, UNICAM_FCM);\n> +\t}\n> +\treturn IRQ_HANDLED;\n> +}\n> +\n> +static int unicam_querycap(struct file *file, void *priv,\n> +\t\t\t   struct v4l2_capability *cap)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\tstrlcpy(cap->driver, UNICAM_MODULE_NAME, sizeof(cap->driver));\n> +\tstrlcpy(cap->card, UNICAM_MODULE_NAME, sizeof(cap->card));\n> +\n> +\tsnprintf(cap->bus_info, sizeof(cap->bus_info),\n> +\t\t \"platform:%s\", dev->v4l2_dev.name);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_enum_fmt_vid_cap(struct file *file, void  *priv,\n> +\t\t\t\t   struct v4l2_fmtdesc *f)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\tconst struct unicam_fmt *fmt = NULL;\n> +\n> +\tif (f->index >= dev->num_active_fmt)\n> +\t\treturn -EINVAL;\n> +\n> +\tfmt = &dev->active_fmts[f->index];\n> +\n> +\tf->pixelformat = fmt->fourcc;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_g_fmt_vid_cap(struct file *file, void *priv,\n> +\t\t\t\tstruct v4l2_format *f)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\t*f = dev->v_fmt;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_try_fmt_vid_cap(struct file *file, void *priv,\n> +\t\t\t\t  struct v4l2_format *f)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\tconst struct unicam_fmt *fmt;\n> +\tstruct v4l2_subdev_format sd_fmt = {\n> +\t\t.which = V4L2_SUBDEV_FORMAT_TRY,\n> +\t};\n> +\tstruct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;\n> +\tint ret;\n> +\n> +\tfmt = find_format_by_pix(dev, f->fmt.pix.pixelformat);\n> +\tif (!fmt) {\n> +\t\tunicam_dbg(3, dev, \"Fourcc format (0x%08x) not found. Use default of %08X\\n\",\n> +\t\t\t   f->fmt.pix.pixelformat, dev->active_fmts[0].fourcc);\n> +\n> +\t\t/* Just get the first one enumerated */\n> +\t\tfmt = &dev->active_fmts[0];\n> +\t\tf->fmt.pix.pixelformat = fmt->fourcc;\n> +\t}\n> +\n> +\tv4l2_fill_mbus_format(mbus_fmt, &f->fmt.pix, fmt->code);\n> +\t/*\n> +\t * No support for receiving interlaced video, so never\n> +\t * request it from the sensor subdev.\n> +\t */\n> +\tmbus_fmt->field = V4L2_FIELD_NONE;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, pad, set_fmt, dev->sensor_config,\n> +\t\t\t       &sd_fmt);\n> +\tif (ret && ret != -ENOIOCTLCMD && ret != -ENODEV)\n> +\t\treturn ret;\n> +\n> +\tif (mbus_fmt->field != V4L2_FIELD_NONE)\n> +\t\tunicam_info(dev, \"Sensor trying to send interlaced video - results may be unpredictable\\n\");\n> +\n> +\tv4l2_fill_pix_format(&f->fmt.pix, &sd_fmt.format);\n> +\t/*\n> +\t * Use current colorspace for now, it will get\n> +\t * updated properly during s_fmt\n> +\t */\n> +\tf->fmt.pix.colorspace = dev->v_fmt.fmt.pix.colorspace;\n> +\treturn unicam_calc_format_size_bpl(dev, fmt, f);\n> +}\n> +\n> +static int unicam_s_fmt_vid_cap(struct file *file, void *priv,\n> +\t\t\t\tstruct v4l2_format *f)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\tstruct vb2_queue *q = &dev->buffer_queue;\n> +\tconst struct unicam_fmt *fmt;\n> +\tstruct v4l2_mbus_framefmt mbus_fmt = {0};\n> +\tint ret;\n> +\tchar fourcc_str[V4L2_FOURCC_MAX_SIZE];\n\nSwap these two lines.\n\n> +\n> +\tif (vb2_is_busy(q))\n> +\t\treturn -EBUSY;\n> +\n> +\tret = unicam_try_fmt_vid_cap(file, priv, f);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tfmt = find_format_by_pix(dev, f->fmt.pix.pixelformat);\n> +\tif (!fmt) {\n> +\t\t/* Unknown pixel format - adopt a default */\n> +\t\tfmt = &dev->active_fmts[0];\n> +\t\tf->fmt.pix.pixelformat = fmt->fourcc;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tv4l2_fill_mbus_format(&mbus_fmt, &f->fmt.pix, fmt->code);\n> +\n> +\tret = __subdev_set_format(dev, &mbus_fmt);\n> +\tif (ret) {\n> +\t\tunicam_dbg(3, dev,\n> +\t\t\t   \"%s __subdev_set_format failed %d\\n\",\n\nCan these two lines be joined?\n\n> +\t\t\t   __func__, ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* Just double check nothing has gone wrong */\n> +\tif (mbus_fmt.code != fmt->code) {\n> +\t\tunicam_dbg(3, dev,\n> +\t\t\t   \"%s subdev changed format on us, this should not happen\\n\",\n> +\t\t\t   __func__);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tdev->fmt = fmt;\n> +\tdev->v_fmt.fmt.pix.pixelformat = f->fmt.pix.pixelformat;\n> +\tdev->v_fmt.fmt.pix.bytesperline = f->fmt.pix.bytesperline;\n> +\tunicam_reset_format(dev);\n> +\n> +\tunicam_dbg(3, dev,\n> +\t\t   \"%s %dx%d, mbus_fmt %08X, V4L2 pix %s.\\n\",\n\nDitto.\n\n> +\t\t   __func__,\n> +\t\t   dev->v_fmt.fmt.pix.width,\n> +\t\t   dev->v_fmt.fmt.pix.height,\n\nDitto.\n\n> +\t\t   mbus_fmt.code,\n> +\t\t   v4l2_fourcc2s(dev->v_fmt.fmt.pix.pixelformat,\n> +\t\t\t\t fourcc_str));\n\nDitto.\n\n> +\n> +\t*f = dev->v_fmt;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_queue_setup(struct vb2_queue *vq,\n> +\t\t\t      unsigned int *nbuffers,\n> +\t\t\t      unsigned int *nplanes,\n> +\t\t\t      unsigned int sizes[],\n> +\t\t\t      struct device *alloc_devs[])\n> +{\n> +\tstruct unicam_device *dev = vb2_get_drv_priv(vq);\n> +\tunsigned int size = dev->v_fmt.fmt.pix.sizeimage;\n> +\n> +\tif (vq->num_buffers + *nbuffers < 3)\n> +\t\t*nbuffers = 3 - vq->num_buffers;\n> +\n> +\tif (*nplanes) {\n> +\t\tif (sizes[0] < size) {\n> +\t\t\tunicam_err(dev, \"sizes[0] %i < size %u\\n\",\n> +\t\t\t\t   sizes[0], size);\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t\tsize = sizes[0];\n> +\t}\n> +\n> +\t*nplanes = 1;\n> +\tsizes[0] = size;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_buffer_prepare(struct vb2_buffer *vb)\n> +{\n> +\tstruct unicam_device *dev = vb2_get_drv_priv(vb->vb2_queue);\n> +\tstruct unicam_buffer *buf = container_of(vb, struct unicam_buffer,\n> +\t\t\t\t\t      vb.vb2_buf);\n> +\tunsigned long size;\n> +\n> +\tif (WARN_ON(!dev->fmt))\n> +\t\treturn -EINVAL;\n> +\n> +\tsize = dev->v_fmt.fmt.pix.sizeimage;\n> +\tif (vb2_plane_size(vb, 0) < size) {\n> +\t\tunicam_err(dev, \"data will not fit into plane (%lu < %lu)\\n\",\n> +\t\t\t   vb2_plane_size(vb, 0), size);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tvb2_set_plane_payload(&buf->vb.vb2_buf, 0, size);\n> +\treturn 0;\n> +}\n> +\n> +static void unicam_buffer_queue(struct vb2_buffer *vb)\n> +{\n> +\tstruct unicam_device *dev = vb2_get_drv_priv(vb->vb2_queue);\n> +\tstruct unicam_buffer *buf = container_of(vb, struct unicam_buffer,\n> +\t\t\t\t\t      vb.vb2_buf);\n> +\tstruct unicam_dmaqueue *dma_queue = &dev->dma_queue;\n> +\tunsigned long flags = 0;\n> +\n> +\t/* recheck locking */\n> +\tspin_lock_irqsave(&dev->dma_queue_lock, flags);\n> +\tlist_add_tail(&buf->list, &dma_queue->active);\n> +\tspin_unlock_irqrestore(&dev->dma_queue_lock, flags);\n> +}\n> +\n> +static void unicam_wr_dma_config(struct unicam_device *dev,\n> +\t\t\t\t unsigned int stride)\n> +{\n> +\treg_write(&dev->cfg, UNICAM_IBLS, stride);\n> +}\n> +\n> +static void unicam_set_packing_config(struct unicam_device *dev)\n> +{\n> +\tint pack, unpack;\n> +\tu32 val;\n> +\tint mbus_depth = find_mbus_depth_by_code(dev->fmt->code);\n> +\tint v4l2_depth = dev->fmt->depth;\n> +\n> +\tif (mbus_depth == v4l2_depth) {\n> +\t\tunpack = UNICAM_PUM_NONE;\n> +\t\tpack = UNICAM_PPM_NONE;\n> +\t} else {\n> +\t\tswitch (mbus_depth) {\n> +\t\tcase 8:\n> +\t\t\tunpack = UNICAM_PUM_UNPACK8;\n> +\t\t\tbreak;\n> +\t\tcase 10:\n> +\t\t\tunpack = UNICAM_PUM_UNPACK10;\n> +\t\t\tbreak;\n> +\t\tcase 12:\n> +\t\t\tunpack = UNICAM_PUM_UNPACK12;\n> +\t\t\tbreak;\n> +\t\tcase 14:\n> +\t\t\tunpack = UNICAM_PUM_UNPACK14;\n> +\t\t\tbreak;\n> +\t\tcase 16:\n> +\t\t\tunpack = UNICAM_PUM_UNPACK16;\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tunpack = UNICAM_PUM_NONE;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tswitch (v4l2_depth) {\n> +\t\tcase 8:\n> +\t\t\tpack = UNICAM_PPM_PACK8;\n> +\t\t\tbreak;\n> +\t\tcase 10:\n> +\t\t\tpack = UNICAM_PPM_PACK10;\n> +\t\t\tbreak;\n> +\t\tcase 12:\n> +\t\t\tpack = UNICAM_PPM_PACK12;\n> +\t\t\tbreak;\n> +\t\tcase 14:\n> +\t\t\tpack = UNICAM_PPM_PACK14;\n> +\t\t\tbreak;\n> +\t\tcase 16:\n> +\t\t\tpack = UNICAM_PPM_PACK16;\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tpack = UNICAM_PPM_NONE;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tval = 0;\n> +\tset_field(&val, 2, UNICAM_DEBL_MASK);\n> +\tset_field(&val, unpack, UNICAM_PUM_MASK);\n> +\tset_field(&val, pack, UNICAM_PPM_MASK);\n> +\treg_write(&dev->cfg, UNICAM_IPIPE, val);\n> +}\n> +\n> +static void unicam_cfg_image_id(struct unicam_device *dev)\n> +{\n> +\tstruct unicam_cfg *cfg = &dev->cfg;\n> +\n> +\tif (dev->bus_type == V4L2_MBUS_CSI2) {\n> +\t\t/* CSI2 mode */\n> +\t\treg_write(cfg, UNICAM_IDI0,\n> +\t\t\t  (dev->virtual_channel << 6) |\n> +\t\t\t  dev->fmt->csi_dt);\n> +\t} else {\n> +\t\t/* CCP2 mode */\n> +\t\treg_write(cfg, UNICAM_IDI0,\n> +\t\t\t  (0x80 | dev->fmt->csi_dt));\n\nI'm sure these two lines can be joined as well. I'm not going to comment\non this any further, just take a look for unnecessary line breaks for v4.\n\n> +\t}\n> +}\n> +\n> +void unicam_start_rx(struct unicam_device *dev, unsigned long addr)\n> +{\n> +\tu32 val;\n> +\tunsigned int i;\n> +\tstruct unicam_cfg *cfg = &dev->cfg;\n> +\tint line_int_freq = dev->v_fmt.fmt.pix.height >> 2;\n> +\n> +\tif (line_int_freq < 128)\n> +\t\tline_int_freq = 128;\n> +\n> +\t/* Enable lane clocks */\n> +\tval = 1;\n> +\tfor (i = 0; i < dev->active_data_lanes; i++)\n> +\t\tval = val << 2 | 1;\n> +\tclk_write(cfg, val);\n> +\n> +\t/* Basic init */\n> +\treg_write(cfg, UNICAM_CTRL, UNICAM_MEM);\n> +\n> +\t/* Enable analogue control, and leave in reset. */\n> +\tval = UNICAM_AR;\n> +\tset_field(&val, 7, UNICAM_CTATADJ_MASK);\n> +\tset_field(&val, 7, UNICAM_PTATADJ_MASK);\n> +\treg_write(cfg, UNICAM_ANA, val);\n> +\tusleep_range(1000, 2000);\n> +\n> +\t/* Come out of reset */\n> +\treg_write_field(cfg, UNICAM_ANA, 0, UNICAM_AR);\n> +\n> +\t/* Peripheral reset */\n> +\treg_write_field(cfg, UNICAM_CTRL, 1, UNICAM_CPR);\n> +\treg_write_field(cfg, UNICAM_CTRL, 0, UNICAM_CPR);\n> +\n> +\treg_write_field(cfg, UNICAM_CTRL, 0, UNICAM_CPE);\n> +\n> +\t/* Enable Rx control. */\n> +\tval = reg_read(cfg, UNICAM_CTRL);\n> +\tif (dev->bus_type == V4L2_MBUS_CSI2) {\n> +\t\tset_field(&val, UNICAM_CPM_CSI2, UNICAM_CPM_MASK);\n> +\t\tset_field(&val, UNICAM_DCM_STROBE, UNICAM_DCM_MASK);\n> +\t} else {\n> +\t\tset_field(&val, UNICAM_CPM_CCP2, UNICAM_CPM_MASK);\n> +\t\tset_field(&val, dev->bus_flags, UNICAM_DCM_MASK);\n> +\t}\n> +\tset_field(&val, 0xF, UNICAM_PFT_MASK);\n> +\tset_field(&val, 128, UNICAM_OET_MASK);\n> +\treg_write(cfg, UNICAM_CTRL, val);\n> +\n> +\treg_write(cfg, UNICAM_IHWIN, 0);\n> +\treg_write(cfg, UNICAM_IVWIN, 0);\n> +\n> +\tval = reg_read(&dev->cfg, UNICAM_PRI);\n> +\tset_field(&val, 0, UNICAM_BL_MASK);\n> +\tset_field(&val, 0, UNICAM_BS_MASK);\n> +\tset_field(&val, 0xE, UNICAM_PP_MASK);\n> +\tset_field(&val, 8, UNICAM_NP_MASK);\n> +\tset_field(&val, 2, UNICAM_PT_MASK);\n> +\tset_field(&val, 1, UNICAM_PE);\n\nLots of magic numbers here. At the very least add some comments.\n\n> +\treg_write(cfg, UNICAM_PRI, val);\n> +\n> +\treg_write_field(cfg, UNICAM_ANA, 0, UNICAM_DDL);\n> +\n> +\t/* Always start in trigger frame capture mode (UNICAM_FCM set) */\n> +\tval = UNICAM_FSIE | UNICAM_FEIE | UNICAM_FCM;\n> +\tset_field(&val,  line_int_freq, UNICAM_LCIE_MASK);\n> +\treg_write(cfg, UNICAM_ICTL, val);\n> +\treg_write(cfg, UNICAM_STA, UNICAM_STA_MASK_ALL);\n> +\treg_write(cfg, UNICAM_ISTA, UNICAM_ISTA_MASK_ALL);\n> +\n> +\t/* tclk_term_en */\n> +\treg_write_field(cfg, UNICAM_CLT, 2, UNICAM_CLT1_MASK);\n> +\t/* tclk_settle */\n> +\treg_write_field(cfg, UNICAM_CLT, 6, UNICAM_CLT2_MASK);\n> +\t/* td_term_en */\n> +\treg_write_field(cfg, UNICAM_DLT, 2, UNICAM_DLT1_MASK);\n> +\t/* ths_settle */\n> +\treg_write_field(cfg, UNICAM_DLT, 6, UNICAM_DLT2_MASK);\n> +\t/* trx_enable */\n> +\treg_write_field(cfg, UNICAM_DLT, 0, UNICAM_DLT3_MASK);\n> +\n> +\treg_write_field(cfg, UNICAM_CTRL, 0, UNICAM_SOE);\n> +\n> +\tval = 0;\n> +\tset_field(&val, 1, UNICAM_PCE);\n> +\tset_field(&val, 1, UNICAM_GI);\n> +\tset_field(&val, 1, UNICAM_CPH);\n> +\tset_field(&val, 0, UNICAM_PCVC_MASK);\n> +\tset_field(&val, 1, UNICAM_PCDT_MASK);\n> +\treg_write(cfg, UNICAM_CMP0, val);\n> +\n> +\t/* Enable clock lane */\n> +\tval = 0;\n> +\tif (dev->bus_type == V4L2_MBUS_CSI2) {\n> +\t\t/* CSI2 */\n> +\t\tset_field(&val, 1, UNICAM_CLE);\n> +\t\tset_field(&val, 1, UNICAM_CLLPE);\n> +\t\tif (dev->bus_flags & V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) {\n> +\t\t\tset_field(&val, 1, UNICAM_CLTRE);\n> +\t\t\tset_field(&val, 1, UNICAM_CLHSE);\n> +\t\t}\n> +\t} else {\n> +\t\t/* CCP2 */\n> +\t\tset_field(&val, 1, UNICAM_CLE);\n> +\t\tset_field(&val, 1, UNICAM_CLHSE);\n> +\t\tset_field(&val, 1, UNICAM_CLTRE);\n> +\t}\n> +\treg_write(cfg, UNICAM_CLK, val);\n> +\n> +\t/* Enable required data lanes */\n> +\tval = 0;\n> +\tif (dev->bus_type == V4L2_MBUS_CSI2) {\n> +\t\t/* CSI2 */\n> +\t\tset_field(&val, 1, UNICAM_DLE);\n> +\t\tset_field(&val, 1, UNICAM_DLLPE);\n> +\t\tif (dev->bus_flags & V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) {\n> +\t\t\tset_field(&val, 1, UNICAM_DLTRE);\n> +\t\t\tset_field(&val, 1, UNICAM_DLHSE);\n> +\t\t}\n> +\t} else {\n> +\t\t/* CCP2 */\n> +\t\tset_field(&val, 1, UNICAM_DLE);\n> +\t\tset_field(&val, 1, UNICAM_DLHSE);\n> +\t\tset_field(&val, 1, UNICAM_DLTRE);\n> +\t}\n> +\treg_write(cfg, UNICAM_DAT0, val);\n> +\n> +\tif (dev->active_data_lanes == 1)\n> +\t\tval = 0;\n> +\treg_write(cfg, UNICAM_DAT1, val);\n> +\n> +\tif (dev->max_data_lanes > 2) {\n> +\t\tif (dev->active_data_lanes == 2)\n> +\t\t\tval = 0;\n> +\t\treg_write(cfg, UNICAM_DAT2, val);\n> +\n> +\t\tif (dev->active_data_lanes == 3)\n> +\t\t\tval = 0;\n> +\t\treg_write(cfg, UNICAM_DAT3, val);\n> +\t}\n> +\n> +\tunicam_wr_dma_config(dev, dev->v_fmt.fmt.pix.bytesperline);\n> +\tunicam_wr_dma_addr(dev, addr);\n> +\tunicam_set_packing_config(dev);\n> +\tunicam_cfg_image_id(dev);\n> +\n> +\tval = 0;\n> +\tset_field(&val, 0, UNICAM_EDL_MASK);\n> +\treg_write(cfg, UNICAM_DCS, val);\n> +\n> +\tval = reg_read(cfg, UNICAM_MISC);\n> +\tset_field(&val, 1, UNICAM_FL0);\n> +\tset_field(&val, 1, UNICAM_FL1);\n> +\treg_write(cfg, UNICAM_MISC, val);\n> +\n> +\treg_write_field(cfg, UNICAM_CTRL, 1, UNICAM_CPE);\n> +\n> +\treg_write_field(cfg, UNICAM_ICTL, 1, UNICAM_LIP_MASK);\n> +\n> +\treg_write_field(cfg, UNICAM_DCS, 1, UNICAM_LDP);\n> +\n> +\t/*\n> +\t * Enable trigger only for the first frame to\n> +\t * sync correctly to the FS from the source.\n> +\t */\n> +\treg_write_field(cfg, UNICAM_ICTL, 1, UNICAM_TFC);\n\nThis function really could do with some more comments.\n\n> +}\n> +\n> +static void unicam_disable(struct unicam_device *dev)\n> +{\n> +\tstruct unicam_cfg *cfg = &dev->cfg;\n> +\n> +\t/* Analogue lane control disable */\n> +\treg_write_field(cfg, UNICAM_ANA, 1, UNICAM_DDL);\n> +\n> +\t/* Stop the output engine */\n> +\treg_write_field(cfg, UNICAM_CTRL, 1, UNICAM_SOE);\n> +\n> +\t/* Disable the data lanes. */\n> +\treg_write(cfg, UNICAM_DAT0, 0);\n> +\treg_write(cfg, UNICAM_DAT1, 0);\n> +\n> +\tif (dev->max_data_lanes > 2) {\n> +\t\treg_write(cfg, UNICAM_DAT2, 0);\n> +\t\treg_write(cfg, UNICAM_DAT3, 0);\n> +\t}\n> +\n> +\t/* Peripheral reset */\n> +\treg_write_field(cfg, UNICAM_CTRL, 1, UNICAM_CPR);\n> +\tusleep_range(50, 100);\n> +\treg_write_field(cfg, UNICAM_CTRL, 0, UNICAM_CPR);\n> +\n> +\t/* Disable peripheral */\n> +\treg_write_field(cfg, UNICAM_CTRL, 0, UNICAM_CPE);\n> +\n> +\t/* Disable all lane clocks */\n> +\tclk_write(cfg, 0);\n> +}\n> +\n> +static int unicam_start_streaming(struct vb2_queue *vq, unsigned int count)\n> +{\n> +\tstruct unicam_device *dev = vb2_get_drv_priv(vq);\n> +\tstruct unicam_dmaqueue *dma_q = &dev->dma_queue;\n> +\tstruct unicam_buffer *buf, *tmp;\n> +\tunsigned long addr = 0;\n> +\tunsigned long flags;\n> +\tint ret;\n> +\n> +\tspin_lock_irqsave(&dev->dma_queue_lock, flags);\n> +\tbuf = list_entry(dma_q->active.next, struct unicam_buffer, list);\n> +\tdev->cur_frm = buf;\n> +\tdev->next_frm = buf;\n> +\tlist_del(&buf->list);\n> +\tspin_unlock_irqrestore(&dev->dma_queue_lock, flags);\n> +\n> +\taddr = vb2_dma_contig_plane_dma_addr(&dev->cur_frm->vb.vb2_buf, 0);\n> +\tdev->sequence = 0;\n> +\n> +\tret = unicam_runtime_get(dev);\n> +\tif (ret < 0) {\n> +\t\tunicam_dbg(3, dev, \"unicam_runtime_get failed\\n\");\n> +\t\tgoto err_release_buffers;\n> +\t}\n> +\n> +\tdev->active_data_lanes = dev->max_data_lanes;\n> +\tif (dev->bus_type == V4L2_MBUS_CSI2 &&\n> +\t    v4l2_subdev_has_op(dev->sensor, video, g_mbus_config)) {\n> +\t\tstruct v4l2_mbus_config mbus_config;\n> +\n> +\t\tret = v4l2_subdev_call(dev->sensor, video, g_mbus_config,\n> +\t\t\t\t       &mbus_config);\n> +\t\tif (ret < 0) {\n> +\t\t\tunicam_dbg(3, dev, \"g_mbus_config failed\\n\");\n> +\t\t\tgoto err_pm_put;\n> +\t\t}\n> +\n> +\t\tswitch (mbus_config.flags & V4L2_MBUS_CSI2_LANES) {\n> +\t\tcase V4L2_MBUS_CSI2_1_LANE:\n> +\t\t\tdev->active_data_lanes = 1;\n> +\t\t\tbreak;\n> +\t\tcase V4L2_MBUS_CSI2_2_LANE:\n> +\t\t\tdev->active_data_lanes = 2;\n> +\t\t\tbreak;\n> +\t\tcase V4L2_MBUS_CSI2_3_LANE:\n> +\t\t\tdev->active_data_lanes = 3;\n> +\t\t\tbreak;\n> +\t\tcase V4L2_MBUS_CSI2_4_LANE:\n> +\t\t\tdev->active_data_lanes = 4;\n> +\t\t\tbreak;\n\nI assume this will change in v4, using Philipp Zabel's new V4L2_MBUS_CSI2_LANE_MASK\ndefine. The only thing you should use from mbus_config is V4L2_MBUS_CSI2_LANE_MASK,\neverything else should be ignored since it comes from the DT.\n\n> +\t\tdefault:\n> +\t\t\tunicam_err(dev, \"Invalid CSI2 lane flag value - %X\\n\",\n> +\t\t\t\t   mbus_config.flags & V4L2_MBUS_CSI2_LANES);\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif ((mbus_config.flags ^ dev->bus_flags) &\n> +\t\t\t\t\t(V4L2_MBUS_CSI2_CONTINUOUS_CLOCK |\n> +\t\t\t\t\t V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK))\n> +\t\t\tunicam_err(dev, \"g_mbus_format returned different clocking mode to DT\\n\");\n> +\t}\n> +\tif (dev->active_data_lanes > dev->max_data_lanes) {\n> +\t\tunicam_err(dev, \"Device has requested %u data lanes, which is >%u configured in DT\\n\",\n> +\t\t\t   dev->active_data_lanes,\n> +\t\t\t   dev->max_data_lanes);\n> +\t\tret = -EINVAL;\n> +\t\tgoto err_pm_put;\n> +\t}\n> +\n> +\tunicam_dbg(1, dev, \"Running with %u data lanes\\n\",\n> +\t\t   dev->active_data_lanes);\n> +\n> +\tret = clk_set_rate(dev->clock, 100 * 1000 * 1000);\n> +\tif (ret) {\n> +\t\tunicam_err(dev, \"failed to set up clock\\n\");\n> +\t\tgoto err_pm_put;\n> +\t}\n> +\n> +\tret = clk_prepare_enable(dev->clock);\n> +\tif (ret) {\n> +\t\tunicam_err(dev, \"Failed to enable CSI clock: %d\\n\", ret);\n> +\t\tgoto err_pm_put;\n> +\t}\n> +\tret = v4l2_subdev_call(dev->sensor, core, s_power, 1);\n> +\tif (ret < 0 && ret != -ENOIOCTLCMD) {\n> +\t\tunicam_err(dev, \"power on failed in subdev\\n\");\n> +\t\tgoto err_clock_unprepare;\n> +\t}\n> +\tdev->streaming = 1;\n> +\n> +\tunicam_start_rx(dev, addr);\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, video, s_stream, 1);\n> +\tif (ret < 0) {\n> +\t\tunicam_err(dev, \"stream on failed in subdev\\n\");\n> +\t\tgoto err_disable_unicam;\n> +\t}\n> +\n> +\treturn 0;\n> +\n> +err_disable_unicam:\n> +\tunicam_disable(dev);\n> +\tv4l2_subdev_call(dev->sensor, core, s_power, 0);\n> +err_clock_unprepare:\n> +\tclk_disable_unprepare(dev->clock);\n> +err_pm_put:\n> +\tunicam_runtime_put(dev);\n> +err_release_buffers:\n> +\tlist_for_each_entry_safe(buf, tmp, &dma_q->active, list) {\n> +\t\tlist_del(&buf->list);\n> +\t\tvb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);\n> +\t}\n> +\tif (dev->cur_frm != dev->next_frm)\n> +\t\tvb2_buffer_done(&dev->next_frm->vb.vb2_buf,\n> +\t\t\t\tVB2_BUF_STATE_QUEUED);\n> +\tvb2_buffer_done(&dev->cur_frm->vb.vb2_buf, VB2_BUF_STATE_QUEUED);\n> +\tdev->next_frm = NULL;\n> +\tdev->cur_frm = NULL;\n> +\n> +\treturn ret;\n> +}\n> +\n> +static void unicam_stop_streaming(struct vb2_queue *vq)\n> +{\n> +\tstruct unicam_device *dev = vb2_get_drv_priv(vq);\n> +\tstruct unicam_dmaqueue *dma_q = &dev->dma_queue;\n> +\tstruct unicam_buffer *buf, *tmp;\n> +\tunsigned long flags;\n> +\n> +\tif (v4l2_subdev_call(dev->sensor, video, s_stream, 0) < 0)\n> +\t\tunicam_err(dev, \"stream off failed in subdev\\n\");\n> +\n> +\tunicam_disable(dev);\n> +\n> +\t/* Release all active buffers */\n> +\tspin_lock_irqsave(&dev->dma_queue_lock, flags);\n> +\tlist_for_each_entry_safe(buf, tmp, &dma_q->active, list) {\n> +\t\tlist_del(&buf->list);\n> +\t\tvb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);\n> +\t}\n> +\n> +\tif (dev->cur_frm == dev->next_frm) {\n> +\t\tvb2_buffer_done(&dev->cur_frm->vb.vb2_buf, VB2_BUF_STATE_ERROR);\n> +\t} else {\n> +\t\tvb2_buffer_done(&dev->cur_frm->vb.vb2_buf, VB2_BUF_STATE_ERROR);\n> +\t\tvb2_buffer_done(&dev->next_frm->vb.vb2_buf,\n> +\t\t\t\tVB2_BUF_STATE_ERROR);\n> +\t}\n> +\tdev->cur_frm = NULL;\n> +\tdev->next_frm = NULL;\n> +\tspin_unlock_irqrestore(&dev->dma_queue_lock, flags);\n> +\n> +\tif (v4l2_subdev_has_op(dev->sensor, core, s_power)) {\n> +\t\tif (v4l2_subdev_call(dev->sensor, core, s_power, 0) < 0)\n> +\t\t\tunicam_err(dev, \"power off failed in subdev\\n\");\n> +\t}\n> +\n> +\tclk_disable_unprepare(dev->clock);\n> +\tunicam_runtime_put(dev);\n> +}\n> +\n> +static int unicam_enum_input(struct file *file, void *priv,\n> +\t\t\t     struct v4l2_input *inp)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\tif (inp->index != 0)\n> +\t\treturn -EINVAL;\n> +\n> +\tinp->type = V4L2_INPUT_TYPE_CAMERA;\n> +\tif (v4l2_subdev_has_op(dev->sensor, video, s_dv_timings)) {\n> +\t\tinp->capabilities = V4L2_IN_CAP_DV_TIMINGS;\n> +\t\tinp->std = 0;\n> +\t} else if (v4l2_subdev_has_op(dev->sensor, video, s_std)) {\n> +\t\tinp->capabilities = V4L2_IN_CAP_STD;\n> +\t\tif (v4l2_subdev_call(dev->sensor, video, g_tvnorms, &inp->std)\n> +\t\t\t\t\t< 0)\n> +\t\t\tinp->std = V4L2_STD_ALL;\n> +\t} else {\n> +\t\tinp->capabilities = 0;\n> +\t\tinp->std = 0;\n> +\t}\n> +\tsprintf(inp->name, \"Camera 0\");\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_g_input(struct file *file, void *priv, unsigned int *i)\n> +{\n> +\t*i = 0;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_s_input(struct file *file, void *priv, unsigned int i)\n> +{\n> +\t/*\n> +\t * FIXME: Ideally we would like to be able to query the sensor\n\nYou probably mean 'subdev' instead of 'sensor'. A sensor has no 'inputs'\nas such.\n\n> +\t * for information over the input connectors it supports, and\n> +\t * map that through in to a call to video_ops->s_routing.\n> +\t * There is no infrastructure support for defining that within\n> +\t * devicetree at present. Until that is implemented we can't\n> +\t * map a user physical connector number to s_routing input number.\n> +\t */\n> +\tif (i > 0)\n> +\t\treturn -EINVAL;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_querystd(struct file *file, void *priv,\n> +\t\t\t   v4l2_std_id *std)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, video, querystd, std);\n> +}\n> +\n> +static int unicam_g_std(struct file *file, void *priv, v4l2_std_id *std)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, video, g_std, std);\n> +}\n> +\n> +static int unicam_s_std(struct file *file, void *priv, v4l2_std_id std)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\tint ret;\n> +\tv4l2_std_id current_std;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, video, g_std, &current_std);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tif (std == current_std)\n> +\t\treturn 0;\n> +\n> +\tif (vb2_is_busy(&dev->buffer_queue))\n> +\t\treturn -EBUSY;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, video, s_std, std);\n> +\n> +\t/* Force recomputation of bytesperline */\n> +\tdev->v_fmt.fmt.pix.bytesperline = 0;\n> +\n> +\tunicam_reset_format(dev);\n> +\n> +\treturn ret;\n> +}\n> +\n> +static int unicam_s_edid(struct file *file, void *priv, struct v4l2_edid *edid)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, pad, set_edid, edid);\n> +}\n> +\n> +static int unicam_g_edid(struct file *file, void *priv, struct v4l2_edid *edid)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, pad, get_edid, edid);\n> +}\n> +\n> +static int unicam_g_dv_timings(struct file *file, void *priv,\n> +\t\t\t       struct v4l2_dv_timings *timings)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, video, g_dv_timings, timings);\n> +}\n> +\n> +static int unicam_s_dv_timings(struct file *file, void *priv,\n> +\t\t\t       struct v4l2_dv_timings *timings)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\tstruct v4l2_dv_timings current_timings;\n> +\tint ret;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, video, g_dv_timings,\n> +\t\t\t       &current_timings);\n> +\n> +\tif (v4l2_match_dv_timings(timings, &current_timings, 0, false))\n> +\t\treturn 0;\n> +\n> +\tif (vb2_is_busy(&dev->buffer_queue))\n> +\t\treturn -EBUSY;\n> +\n> +\tret = v4l2_subdev_call(dev->sensor, video, s_dv_timings, timings);\n> +\n> +\t/* Force recomputation of bytesperline */\n> +\tdev->v_fmt.fmt.pix.bytesperline = 0;\n> +\n> +\tunicam_reset_format(dev);\n> +\n> +\treturn ret;\n> +}\n> +\n> +static int unicam_query_dv_timings(struct file *file, void *priv,\n> +\t\t\t\t   struct v4l2_dv_timings *timings)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, video, query_dv_timings, timings);\n> +}\n> +\n> +static int unicam_enum_dv_timings(struct file *file, void *priv,\n> +\t\t\t\t  struct v4l2_enum_dv_timings *timings)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, pad, enum_dv_timings, timings);\n> +}\n> +\n> +static int unicam_dv_timings_cap(struct file *file, void *priv,\n> +\t\t\t\t struct v4l2_dv_timings_cap *cap)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n> +\n> +\treturn v4l2_subdev_call(dev->sensor, pad, dv_timings_cap, cap);\n> +}\n> +\n> +static int unicam_subscribe_event(struct v4l2_fh *fh,\n> +\t\t\t\t  const struct v4l2_event_subscription *sub)\n> +{\n> +\tswitch (sub->type) {\n> +\tcase V4L2_EVENT_SOURCE_CHANGE:\n> +\t\treturn v4l2_event_subscribe(fh, sub, 4, NULL);\n> +\t}\n> +\n> +\treturn v4l2_ctrl_subscribe_event(fh, sub);\n> +}\n> +\n> +static int unicam_log_status(struct file *file, void *fh)\n> +{\n> +\tstruct unicam_device *dev = video_drvdata(file);\n\nAdd newline.\n\n> +\t/* status for sub devices */\n> +\tv4l2_device_call_all(&dev->v4l2_dev, 0, core, log_status);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static void unicam_notify(struct v4l2_subdev *sd,\n> +\t\t\t  unsigned int notification, void *arg)\n> +{\n> +\tstruct unicam_device *dev =\n> +\t\tcontainer_of(sd->v4l2_dev, struct unicam_device, v4l2_dev);\n> +\n> +\tswitch (notification) {\n> +\tcase V4L2_DEVICE_NOTIFY_EVENT:\n> +\t\tv4l2_event_queue(&dev->video_dev, arg);\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +static const struct vb2_ops unicam_video_qops = {\n> +\t.wait_prepare\t\t= vb2_ops_wait_prepare,\n> +\t.wait_finish\t\t= vb2_ops_wait_finish,\n> +\t.queue_setup\t\t= unicam_queue_setup,\n> +\t.buf_prepare\t\t= unicam_buffer_prepare,\n> +\t.buf_queue\t\t= unicam_buffer_queue,\n> +\t.start_streaming\t= unicam_start_streaming,\n> +\t.stop_streaming\t\t= unicam_stop_streaming,\n> +};\n> +\n> +/* unicam capture driver file operations */\n> +static const struct v4l2_file_operations unicam_fops = {\n> +\t.owner\t\t= THIS_MODULE,\n> +\t.open           = v4l2_fh_open,\n> +\t.release        = vb2_fop_release,\n> +\t.read\t\t= vb2_fop_read,\n> +\t.poll\t\t= vb2_fop_poll,\n> +\t.unlocked_ioctl\t= video_ioctl2,\n> +\t.mmap\t\t= vb2_fop_mmap,\n> +};\n> +\n> +/* unicam capture ioctl operations */\n> +static const struct v4l2_ioctl_ops unicam_ioctl_ops = {\n> +\t.vidioc_querycap\t\t= unicam_querycap,\n> +\t.vidioc_enum_fmt_vid_cap\t= unicam_enum_fmt_vid_cap,\n> +\t.vidioc_g_fmt_vid_cap\t\t= unicam_g_fmt_vid_cap,\n> +\t.vidioc_s_fmt_vid_cap\t\t= unicam_s_fmt_vid_cap,\n> +\t.vidioc_try_fmt_vid_cap\t\t= unicam_try_fmt_vid_cap,\n> +\n> +\t.vidioc_enum_input\t\t= unicam_enum_input,\n> +\t.vidioc_g_input\t\t\t= unicam_g_input,\n> +\t.vidioc_s_input\t\t\t= unicam_s_input,\n> +\n> +\t.vidioc_querystd\t\t= unicam_querystd,\n> +\t.vidioc_s_std\t\t\t= unicam_s_std,\n> +\t.vidioc_g_std\t\t\t= unicam_g_std,\n> +\n> +\t.vidioc_g_edid\t\t\t= unicam_g_edid,\n> +\t.vidioc_s_edid\t\t\t= unicam_s_edid,\n> +\n> +\t.vidioc_s_dv_timings\t\t= unicam_s_dv_timings,\n> +\t.vidioc_g_dv_timings\t\t= unicam_g_dv_timings,\n> +\t.vidioc_query_dv_timings\t= unicam_query_dv_timings,\n> +\t.vidioc_enum_dv_timings\t\t= unicam_enum_dv_timings,\n> +\t.vidioc_dv_timings_cap\t\t= unicam_dv_timings_cap,\n> +\n> +\t.vidioc_reqbufs\t\t\t= vb2_ioctl_reqbufs,\n> +\t.vidioc_create_bufs\t\t= vb2_ioctl_create_bufs,\n> +\t.vidioc_prepare_buf\t\t= vb2_ioctl_prepare_buf,\n> +\t.vidioc_querybuf\t\t= vb2_ioctl_querybuf,\n> +\t.vidioc_qbuf\t\t\t= vb2_ioctl_qbuf,\n> +\t.vidioc_dqbuf\t\t\t= vb2_ioctl_dqbuf,\n> +\t.vidioc_expbuf\t\t\t= vb2_ioctl_expbuf,\n> +\t.vidioc_streamon\t\t= vb2_ioctl_streamon,\n> +\t.vidioc_streamoff\t\t= vb2_ioctl_streamoff,\n> +\n> +\t.vidioc_log_status\t\t= unicam_log_status,\n> +\t.vidioc_subscribe_event\t\t= unicam_subscribe_event,\n> +\t.vidioc_unsubscribe_event\t= v4l2_event_unsubscribe,\n> +};\n> +\n> +static int\n> +unicam_async_bound(struct v4l2_async_notifier *notifier,\n> +\t\t   struct v4l2_subdev *subdev,\n> +\t\t   struct v4l2_async_subdev *asd)\n> +{\n> +\tstruct unicam_device *unicam = container_of(notifier->v4l2_dev,\n> +\t\t\t\t\t       struct unicam_device, v4l2_dev);\n> +\tstruct v4l2_subdev_mbus_code_enum mbus_code;\n> +\tint j;\n> +\tstruct unicam_fmt *active_fmt;\n> +\tint ret = 0;\n> +\n> +\tif (unicam->sensor) {\n> +\t\tunicam_info(unicam, \"Rejecting subdev %s (Already set!!)\",\n> +\t\t\t    subdev->name);\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tunicam->sensor = subdev;\n> +\tunicam_dbg(1, unicam, \"Using sensor %s for capture\\n\", subdev->name);\n> +\n> +\t/* Enumerate sub device formats and enable all matching local formats */\n> +\tunicam->num_active_fmt = 0;\n> +\tactive_fmt = &unicam->active_fmts[0];\n> +\tunicam_dbg(2, unicam, \"Get supported formats...\\n\");\n> +\tfor (j = 0; ret != -EINVAL && ret != -ENOIOCTLCMD; ++j) {\n> +\t\tconst struct unicam_fmt *fmt = NULL;\n> +\t\tint k;\n> +\n> +\t\tmemset(&mbus_code, 0, sizeof(mbus_code));\n> +\t\tmbus_code.index = j;\n> +\t\tret = v4l2_subdev_call(subdev, pad, enum_mbus_code,\n> +\t\t\t\t       NULL, &mbus_code);\n> +\t\tif (ret < 0) {\n> +\t\t\tunicam_dbg(2, unicam,\n> +\t\t\t\t   \"subdev->enum_mbus_code idx %d returned %d - continue\\n\",\n> +\t\t\t\t   j, ret);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tunicam_dbg(2, unicam,\n> +\t\t\t   \"subdev %s: code: %04x idx: %d\\n\",\n> +\t\t\t   subdev->name, mbus_code.code, j);\n> +\n> +\t\tfor (k = 0; k < ARRAY_SIZE(formats); k++) {\n> +\t\t\tif (mbus_code.code == formats[k].code) {\n> +\t\t\t\tfmt = &formats[k];\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t\tunicam_dbg(2, unicam, \"fmt %04x returned as %p, V4L2 FOURCC %04x, csi_dt %02X\\n\",\n> +\t\t\t   mbus_code.code, fmt, fmt ? fmt->fourcc : 0,\n> +\t\t\t   fmt ? fmt->csi_dt : 0);\n> +\t\tif (fmt) {\n> +\t\t\tconst struct bayer_fmt *fmt_list = NULL;\n> +\n> +\t\t\tswitch (fmt->fourcc) {\n> +\t\t\tcase PIX_FMT_ALL_BGGR:\n> +\t\t\t\tfmt_list = all_bayer_bggr;\n> +\t\t\t\tbreak;\n> +\t\t\tcase PIX_FMT_ALL_RGGB:\n> +\t\t\t\tfmt_list = all_bayer_rggb;\n> +\t\t\t\tbreak;\n> +\t\t\tcase PIX_FMT_ALL_GBRG:\n> +\t\t\t\tfmt_list = all_bayer_gbrg;\n> +\t\t\t\tbreak;\n> +\t\t\tcase PIX_FMT_ALL_GRBG:\n> +\t\t\t\tfmt_list = all_bayer_grbg;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t\tif (fmt_list) {\n> +\t\t\t\tfor (k = 0; fmt_list[k].fourcc; k++) {\n> +\t\t\t\t\tchar fourcc_str[V4L2_FOURCC_MAX_SIZE];\n> +\n> +\t\t\t\t\t*active_fmt = *fmt;\n> +\t\t\t\t\tactive_fmt->fourcc = fmt_list[k].fourcc;\n> +\t\t\t\t\tactive_fmt->depth = fmt_list[k].depth;\n> +\t\t\t\t\tunicam_dbg(2, unicam,\n> +\t\t\t\t\t\t   \"matched fourcc: %s: code: %04x idx: %d\\n\",\n> +\t\t\t\t\t\t   v4l2_fourcc2s(fmt->fourcc,\n> +\t\t\t\t\t\t\t\t fourcc_str),\n> +\t\t\t\t\t\t   fmt->code,\n> +\t\t\t\t\t\t   unicam->num_active_fmt);\n> +\t\t\t\t\tunicam->num_active_fmt++;\n> +\t\t\t\t\tactive_fmt++;\n> +\t\t\t\t}\n> +\t\t\t} else {\n> +\t\t\t\tchar fourcc_str[V4L2_FOURCC_MAX_SIZE];\n> +\n> +\t\t\t\t*active_fmt = *fmt;\n> +\t\t\t\tunicam_dbg(2, unicam,\n> +\t\t\t\t\t   \"matched fourcc: %s: code: %04x idx: %d\\n\",\n> +\t\t\t\t\t   v4l2_fourcc2s(fmt->fourcc,\n> +\t\t\t\t\t\t\t fourcc_str),\n> +\t\t\t\t\t   fmt->code,\n> +\t\t\t\t\t   unicam->num_active_fmt);\n> +\t\t\t\tunicam->num_active_fmt++;\n> +\t\t\t\tactive_fmt++;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\tunicam_dbg(2, unicam,\n> +\t\t   \"Done all formats\\n\");\n> +\tdump_active_formats(unicam);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_probe_complete(struct unicam_device *unicam)\n> +{\n> +\tstruct video_device *vdev;\n> +\tstruct vb2_queue *q;\n> +\tstruct v4l2_mbus_framefmt mbus_fmt = {0};\n> +\tconst struct unicam_fmt *fmt;\n> +\tint ret;\n> +\n> +\tv4l2_set_subdev_hostdata(unicam->sensor, unicam);\n> +\n> +\tunicam->v4l2_dev.notify = unicam_notify;\n> +\n> +\tunicam->sensor_config = v4l2_subdev_alloc_pad_config(unicam->sensor);\n> +\tif (!unicam->sensor_config)\n> +\t\treturn -ENOMEM;\n> +\n> +\tret = __subdev_get_format(unicam, &mbus_fmt);\n> +\tif (ret) {\n> +\t\tunicam_err(unicam, \"Failed to get_format - ret %d\\n\", ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tfmt = find_format_by_code(unicam, mbus_fmt.code);\n> +\tif (!fmt) {\n> +\t\tunicam_dbg(3, unicam, \"mbus code format (0x%08x) not found.\\n\",\n> +\t\t\t   mbus_fmt.code);\n> +\t\treturn -EINVAL;\n> +\t}\n\nThis is weird code. What typically happens at this time is that the first\nvalid format (i.e. one that both the subdev driver and this driver support)\nis chosen and set. What seems to happen here is that you get the current\nsubdev format and try to select that, but that format may not be valid\ncausing this probe to return an error.\n\nInstead, just pick the first of the active formats and select that.\n\nOnly if there are no active formats at all (i.e. no matches were found)\nwould you return an error.\n\n> +\tunicam->fmt = fmt;\n> +\tunicam->v_fmt.fmt.pix.pixelformat  = fmt->fourcc;\n> +\n> +\t/* Read current subdev format */\n> +\tunicam_reset_format(unicam);\n> +\n> +\tif (v4l2_subdev_has_op(unicam->sensor, video, s_std)) {\n> +\t\tv4l2_std_id tvnorms;\n> +\n> +\t\tif (WARN_ON(!v4l2_subdev_has_op(unicam->sensor, video,\n> +\t\t\t\t\t\tg_tvnorms)))\n> +\t\t\t/*\n> +\t\t\t * Subdevice should not advertise querystd but not\n> +\t\t\t * g_tvnorms\n\nYou probably mean s_std instead of querystd?\n\n> +\t\t\t */\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tret = v4l2_subdev_call(unicam->sensor, video,\n> +\t\t\t\t       g_tvnorms, &tvnorms);\n> +\t\tif (WARN_ON(ret))\n> +\t\t\treturn -EINVAL;\n> +\t\tunicam->video_dev.tvnorms |= tvnorms;\n> +\t}\n> +\n> +\tspin_lock_init(&unicam->dma_queue_lock);\n> +\tmutex_init(&unicam->lock);\n> +\n> +\t/* Add controls from the subdevice */\n> +\tret = v4l2_ctrl_add_handler(&unicam->ctrl_handler,\n> +\t\t\t\t    unicam->sensor->ctrl_handler, NULL);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tq = &unicam->buffer_queue;\n> +\tq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;\n> +\tq->io_modes = VB2_MMAP | VB2_DMABUF | VB2_READ;\n> +\tq->drv_priv = unicam;\n> +\tq->ops = &unicam_video_qops;\n> +\tq->mem_ops = &vb2_dma_contig_memops;\n> +\tq->buf_struct_size = sizeof(struct unicam_buffer);\n> +\tq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;\n> +\tq->lock = &unicam->lock;\n> +\tq->min_buffers_needed = 2;\n> +\tq->dev = &unicam->pdev->dev;\n> +\n> +\tret = vb2_queue_init(q);\n> +\tif (ret) {\n> +\t\tunicam_err(unicam, \"vb2_queue_init() failed\\n\");\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tINIT_LIST_HEAD(&unicam->dma_queue.active);\n> +\n> +\tvdev = &unicam->video_dev;\n> +\tstrlcpy(vdev->name, UNICAM_MODULE_NAME, sizeof(vdev->name));\n> +\tvdev->release = video_device_release_empty;\n> +\tvdev->fops = &unicam_fops;\n> +\tvdev->ioctl_ops = &unicam_ioctl_ops;\n> +\tvdev->v4l2_dev = &unicam->v4l2_dev;\n> +\tvdev->vfl_dir = VFL_DIR_RX;\n> +\tvdev->queue = q;\n> +\tvdev->lock = &unicam->lock;\n> +\tvdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |\n> +\t\t\t    V4L2_CAP_READWRITE;\n> +\n> +\tvideo_set_drvdata(vdev, unicam);\n> +\tret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);\n> +\tif (ret) {\n> +\t\tunicam_err(unicam,\n> +\t\t\t   \"Unable to register video device.\\n\");\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (!v4l2_subdev_has_op(unicam->sensor, video, s_std)) {\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_S_STD);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_G_STD);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_ENUMSTD);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_QUERYSTD);\n\nI would move QUERYSTD out of this if and test for it separately.\n\nQUERYSTD is optional in SDTV receivers (unfortunately), so it may not\nbe there, even if S_STD is.\n\n> +\t}\n> +\tif (!v4l2_subdev_has_op(unicam->sensor, video, s_dv_timings)) {\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_S_EDID);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_G_EDID);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_DV_TIMINGS_CAP);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_G_DV_TIMINGS);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_S_DV_TIMINGS);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_ENUM_DV_TIMINGS);\n> +\t\tv4l2_disable_ioctl(&unicam->video_dev, VIDIOC_QUERY_DV_TIMINGS);\n> +\t}\n> +\n> +\tret = v4l2_device_register_subdev_nodes(&unicam->v4l2_dev);\n> +\tif (ret) {\n> +\t\tunicam_err(unicam,\n> +\t\t\t   \"Unable to register subdev nodes.\\n\");\n> +\t\tvideo_unregister_device(&unicam->video_dev);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int unicam_async_complete(struct v4l2_async_notifier *notifier)\n> +{\n> +\tstruct unicam_device *unicam = container_of(notifier->v4l2_dev,\n> +\t\t\t\t\tstruct unicam_device, v4l2_dev);\n> +\n> +\treturn unicam_probe_complete(unicam);\n> +}\n> +\n> +static int of_unicam_connect_subdevs(struct unicam_device *dev)\n> +{\n> +\tstruct platform_device *pdev = dev->pdev;\n> +\tstruct device_node *parent, *ep_node = NULL, *remote_ep = NULL,\n> +\t\t\t*sensor_node = NULL;\n> +\tstruct v4l2_fwnode_endpoint *ep;\n> +\tstruct v4l2_async_subdev *asd;\n> +\tint ret = -EINVAL;\n> +\tunsigned int lane;\n> +\tstruct v4l2_async_subdev **subdevs = NULL;\n> +\tunsigned int peripheral_data_lanes;\n> +\n> +\tparent = pdev->dev.of_node;\n> +\n> +\tasd = &dev->asd;\n> +\tep = &dev->endpoint;\n> +\n> +\tep_node = of_graph_get_next_endpoint(parent, NULL);\n> +\tif (!ep_node) {\n> +\t\tunicam_dbg(3, dev, \"can't get next endpoint\\n\");\n> +\t\tgoto cleanup_exit;\n> +\t}\n> +\n> +\tunicam_dbg(3, dev, \"ep_node is %s\\n\",\n> +\t\t   ep_node->name);\n> +\n> +\tv4l2_fwnode_endpoint_parse(of_fwnode_handle(ep_node), ep);\n> +\n> +\tfor (lane = 0; lane < ep->bus.mipi_csi2.num_data_lanes; lane++) {\n> +\t\tif (ep->bus.mipi_csi2.data_lanes[lane] != lane + 1) {\n> +\t\t\tunicam_err(dev, \"Local endpoint - data lane reordering not supported\\n\");\n> +\t\t\tgoto cleanup_exit;\n> +\t\t}\n> +\t}\n> +\n> +\tperipheral_data_lanes = ep->bus.mipi_csi2.num_data_lanes;\n> +\n> +\tsensor_node = of_graph_get_remote_port_parent(ep_node);\n> +\tif (!sensor_node) {\n> +\t\tunicam_dbg(3, dev, \"can't get remote parent\\n\");\n> +\t\tgoto cleanup_exit;\n> +\t}\n> +\tunicam_dbg(3, dev, \"sensor_node is %s\\n\",\n> +\t\t   sensor_node->name);\n> +\tasd->match_type = V4L2_ASYNC_MATCH_FWNODE;\n> +\tasd->match.fwnode.fwnode = of_fwnode_handle(sensor_node);\n> +\n> +\tremote_ep = of_graph_get_remote_endpoint(ep_node);\n> +\tif (!remote_ep) {\n> +\t\tunicam_dbg(3, dev, \"can't get remote-endpoint\\n\");\n> +\t\tgoto cleanup_exit;\n> +\t}\n> +\tunicam_dbg(3, dev, \"remote_ep is %s\\n\",\n> +\t\t   remote_ep->name);\n> +\tv4l2_fwnode_endpoint_parse(of_fwnode_handle(remote_ep), ep);\n> +\tunicam_dbg(3, dev, \"parsed remote_ep to endpoint. nr_of_link_frequencies %u, bus_type %u\\n\",\n> +\t\t   ep->nr_of_link_frequencies, ep->bus_type);\n> +\n> +\tswitch (ep->bus_type) {\n> +\tcase V4L2_MBUS_CSI2:\n> +\t\tif (ep->bus.mipi_csi2.num_data_lanes >\n> +\t\t\t\tperipheral_data_lanes) {\n> +\t\t\tunicam_err(dev, \"Subdevice %s wants too many data lanes (%u > %u)\\n\",\n> +\t\t\t\t   sensor_node->name,\n> +\t\t\t\t   ep->bus.mipi_csi2.num_data_lanes,\n> +\t\t\t\t   peripheral_data_lanes);\n> +\t\t\tgoto cleanup_exit;\n> +\t\t}\n> +\t\tfor (lane = 0;\n> +\t\t     lane < ep->bus.mipi_csi2.num_data_lanes;\n> +\t\t     lane++) {\n> +\t\t\tif (ep->bus.mipi_csi2.data_lanes[lane] != lane + 1) {\n> +\t\t\t\tunicam_err(dev, \"Subdevice %s - incompatible data lane config\\n\",\n> +\t\t\t\t\t   sensor_node->name);\n> +\t\t\t\tgoto cleanup_exit;\n> +\t\t\t}\n> +\t\t}\n> +\t\tdev->max_data_lanes = ep->bus.mipi_csi2.num_data_lanes;\n> +\t\tdev->bus_flags = ep->bus.mipi_csi2.flags;\n> +\t\tbreak;\n> +\tcase V4L2_MBUS_CCP2:\n> +\t\tif (ep->bus.mipi_csi1.clock_lane != 0 ||\n> +\t\t    ep->bus.mipi_csi1.data_lane != 1) {\n> +\t\t\tunicam_err(dev, \"Subdevice %s incompatible lane config\\n\",\n> +\t\t\t\t   sensor_node->name);\n> +\t\t\tgoto cleanup_exit;\n> +\t\t}\n> +\t\tdev->max_data_lanes = 1;\n> +\t\tdev->bus_flags = ep->bus.mipi_csi1.strobe;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\t/* Unsupported bus type */\n> +\t\tunicam_err(dev, \"sub-device %s is not a CSI2 or CCP2 device %d\\n\",\n> +\t\t\t   sensor_node->name, ep->bus_type);\n> +\t\tgoto cleanup_exit;\n> +\t}\n> +\n> +\t/* Store bus type - CSI2 or CCP2 */\n> +\tdev->bus_type = ep->bus_type;\n> +\tunicam_dbg(3, dev, \"bus_type is %d\\n\", dev->bus_type);\n> +\n> +\t/* Store Virtual Channel number */\n> +\tdev->virtual_channel = ep->base.id;\n> +\n> +\tunicam_dbg(3, dev, \"v4l2-endpoint: %s\\n\",\n> +\t\t   dev->bus_type == V4L2_MBUS_CSI2 ? \"CSI2\" : \"CCP2\");\n> +\tunicam_dbg(3, dev, \"Virtual Channel=%d\\n\", dev->virtual_channel);\n> +\tif (dev->bus_type == V4L2_MBUS_CSI2)\n> +\t\tunicam_dbg(3, dev, \"flags=0x%08x\\n\",\n> +\t\t\t   ep->bus.mipi_csi2.flags);\n> +\tunicam_dbg(3, dev, \"num_data_lanes=%d\\n\", dev->max_data_lanes);\n> +\n> +\tunicam_dbg(1, dev, \"found sub-device %s\\n\",\n> +\t\t   sensor_node->name);\n> +\n> +\tsubdevs = devm_kzalloc(&dev->pdev->dev, sizeof(*subdevs), GFP_KERNEL);\n> +\tif (!subdevs) {\n> +\t\tret = -ENOMEM;\n> +\t\tgoto cleanup_exit;\n> +\t}\n> +\tsubdevs[0] = asd;\n> +\tdev->notifier.subdevs = subdevs;\n> +\tdev->notifier.num_subdevs = 1;\n> +\tdev->notifier.bound = unicam_async_bound;\n> +\tdev->notifier.complete = unicam_async_complete;\n> +\tret = v4l2_async_notifier_register(&dev->v4l2_dev,\n> +\t\t\t\t\t   &dev->notifier);\n> +\tif (ret) {\n> +\t\tunicam_err(dev, \"Error registering async notifier - ret %d\\n\",\n> +\t\t\t   ret);\n> +\t\tret = -EINVAL;\n> +\t}\n> +\n> +cleanup_exit:\n> +\tif (remote_ep)\n> +\t\tof_node_put(remote_ep);\n> +\tif (sensor_node)\n> +\t\tof_node_put(sensor_node);\n> +\tif (ep_node)\n> +\t\tof_node_put(ep_node);\n> +\n> +\treturn ret;\n> +}\n> +\n> +static int unicam_probe(struct platform_device *pdev)\n> +{\n> +\tstruct unicam_cfg *unicam_cfg;\n> +\tstruct unicam_device *unicam;\n> +\tstruct v4l2_ctrl_handler *hdl;\n> +\tstruct resource\t*res;\n> +\tint ret;\n> +\n> +\tunicam = devm_kzalloc(&pdev->dev, sizeof(*unicam), GFP_KERNEL);\n> +\tif (!unicam)\n> +\t\treturn -ENOMEM;\n> +\n> +\tunicam->pdev = pdev;\n> +\tunicam_cfg = &unicam->cfg;\n> +\n> +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> +\tunicam_cfg->base = devm_ioremap_resource(&pdev->dev, res);\n> +\tif (IS_ERR(unicam_cfg->base)) {\n> +\t\tunicam_err(unicam, \"Failed to get main io block\\n\");\n> +\t\treturn PTR_ERR(unicam_cfg->base);\n> +\t}\n> +\n> +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 1);\n> +\tunicam_cfg->clk_gate_base = devm_ioremap_resource(&pdev->dev, res);\n> +\tif (IS_ERR(unicam_cfg->clk_gate_base)) {\n> +\t\tunicam_err(unicam, \"Failed to get 2nd io block\\n\");\n> +\t\treturn PTR_ERR(unicam_cfg->clk_gate_base);\n> +\t}\n> +\n> +\tunicam->clock = devm_clk_get(&pdev->dev, \"lp\");\n> +\tif (IS_ERR(unicam->clock)) {\n> +\t\tunicam_err(unicam, \"Failed to get clock\\n\");\n> +\t\treturn PTR_ERR(unicam->clock);\n> +\t}\n> +\n> +\tret = platform_get_irq(pdev, 0);\n> +\tif (ret <= 0) {\n> +\t\tdev_err(&pdev->dev, \"No IRQ resource\\n\");\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tret = devm_request_irq(&pdev->dev, ret, unicam_isr, 0,\n> +\t\t\t       \"unicam_capture0\", unicam);\n> +\tif (ret) {\n> +\t\tdev_err(&pdev->dev, \"Unable to request interrupt\\n\");\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tret = v4l2_device_register(&pdev->dev, &unicam->v4l2_dev);\n> +\tif (ret) {\n> +\t\tunicam_err(unicam,\n> +\t\t\t   \"Unable to register v4l2 device.\\n\");\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* Reserve space for the controls */\n> +\thdl = &unicam->ctrl_handler;\n> +\tret = v4l2_ctrl_handler_init(hdl, 16);\n> +\tif (ret < 0)\n> +\t\tgoto probe_out_v4l2_unregister;\n> +\tunicam->v4l2_dev.ctrl_handler = hdl;\n> +\n> +\t/* set the driver data in platform device */\n> +\tplatform_set_drvdata(pdev, unicam);\n> +\n> +\tret = of_unicam_connect_subdevs(unicam);\n> +\tif (ret) {\n> +\t\tdev_err(&pdev->dev, \"Failed to connect subdevs\\n\");\n> +\t\tgoto free_hdl;\n> +\t}\n> +\n> +\t/* Enable the block power domain */\n> +\tpm_runtime_enable(&pdev->dev);\n> +\n> +\treturn 0;\n> +\n> +free_hdl:\n> +\tv4l2_ctrl_handler_free(hdl);\n> +probe_out_v4l2_unregister:\n> +\tv4l2_device_unregister(&unicam->v4l2_dev);\n> +\treturn ret;\n> +}\n> +\n> +static int unicam_remove(struct platform_device *pdev)\n> +{\n> +\tstruct unicam_device *unicam = platform_get_drvdata(pdev);\n> +\n> +\tunicam_dbg(2, unicam, \"%s\\n\", __func__);\n> +\n> +\tpm_runtime_disable(&pdev->dev);\n> +\n> +\tv4l2_async_notifier_unregister(&unicam->notifier);\n> +\tv4l2_ctrl_handler_free(&unicam->ctrl_handler);\n> +\tv4l2_device_unregister(&unicam->v4l2_dev);\n> +\tvideo_unregister_device(&unicam->video_dev);\n> +\tif (unicam->sensor_config)\n> +\t\tv4l2_subdev_free_pad_config(unicam->sensor_config);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct of_device_id unicam_of_match[] = {\n> +\t{ .compatible = \"brcm,bcm2835-unicam\", },\n> +\t{ /* sentinel */ },\n> +};\n> +MODULE_DEVICE_TABLE(of, unicam_of_match);\n> +\n> +static struct platform_driver unicam_driver = {\n> +\t.probe\t\t= unicam_probe,\n> +\t.remove\t\t= unicam_remove,\n> +\t.driver = {\n> +\t\t.name\t= UNICAM_MODULE_NAME,\n> +\t\t.of_match_table = of_match_ptr(unicam_of_match),\n> +\t},\n> +};\n> +\n> +module_platform_driver(unicam_driver);\n> +\n> +MODULE_AUTHOR(\"Dave Stevenson <dave.stevenson@raspberrypi.org>\");\n> +MODULE_DESCRIPTION(\"BCM2835 Unicam driver\");\n> +MODULE_LICENSE(\"GPL\");\n> +MODULE_VERSION(UNICAM_VERSION);\n> diff --git a/drivers/media/platform/bcm2835/vc4-regs-unicam.h b/drivers/media/platform/bcm2835/vc4-regs-unicam.h\n> new file mode 100644\n> index 0000000..28e9a75\n> --- /dev/null\n> +++ b/drivers/media/platform/bcm2835/vc4-regs-unicam.h\n> @@ -0,0 +1,264 @@\n> +/*\n> + * Copyright (C) 2017 Raspberry Pi Trading.\n> + * Dave Stevenson <dave.stevenson@raspberrypi.org>\n> + *\n> + * This program is free software; you can redistribute it and/or modify\n> + * it under the terms of the GNU General Public License version 2 as\n> + * published by the Free Software Foundation.\n> + *\n> + * THE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND,\n> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF\n> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND\n> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS\n> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN\n> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN\n> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\n> + * SOFTWARE.\n> + */\n> +\n> +#ifndef VC4_REGS_UNICAM_H\n> +#define VC4_REGS_UNICAM_H\n> +\n> +/*\n> + * The following values are taken from files found within the code drop\n> + * made by Broadcom for the BCM21553 Graphics Driver, predominantly in\n> + * brcm_usrlib/dag/vmcsx/vcinclude/hardware_vc4.h.\n> + * They have been modified to be only the register offset.\n> + */\n> +#define UNICAM_CTRL\t0x000\n> +#define UNICAM_STA\t0x004\n> +#define UNICAM_ANA\t0x008\n> +#define UNICAM_PRI\t0x00c\n> +#define UNICAM_CLK\t0x010\n> +#define UNICAM_CLT\t0x014\n> +#define UNICAM_DAT0\t0x018\n> +#define UNICAM_DAT1\t0x01c\n> +#define UNICAM_DAT2\t0x020\n> +#define UNICAM_DAT3\t0x024\n> +#define UNICAM_DLT\t0x028\n> +#define UNICAM_CMP0\t0x02c\n> +#define UNICAM_CMP1\t0x030\n> +#define UNICAM_CAP0\t0x034\n> +#define UNICAM_CAP1\t0x038\n> +#define UNICAM_ICTL\t0x100\n> +#define UNICAM_ISTA\t0x104\n> +#define UNICAM_IDI0\t0x108\n> +#define UNICAM_IPIPE\t0x10c\n> +#define UNICAM_IBSA0\t0x110\n> +#define UNICAM_IBEA0\t0x114\n> +#define UNICAM_IBLS\t0x118\n> +#define UNICAM_IBWP\t0x11c\n> +#define UNICAM_IHWIN\t0x120\n> +#define UNICAM_IHSTA\t0x124\n> +#define UNICAM_IVWIN\t0x128\n> +#define UNICAM_IVSTA\t0x12c\n> +#define UNICAM_ICC\t0x130\n> +#define UNICAM_ICS\t0x134\n> +#define UNICAM_IDC\t0x138\n> +#define UNICAM_IDPO\t0x13c\n> +#define UNICAM_IDCA\t0x140\n> +#define UNICAM_IDCD\t0x144\n> +#define UNICAM_IDS\t0x148\n> +#define UNICAM_DCS\t0x200\n> +#define UNICAM_DBSA0\t0x204\n> +#define UNICAM_DBEA0\t0x208\n> +#define UNICAM_DBWP\t0x20c\n> +#define UNICAM_DBCTL\t0x300\n> +#define UNICAM_IBSA1\t0x304\n> +#define UNICAM_IBEA1\t0x308\n> +#define UNICAM_IDI1\t0x30c\n> +#define UNICAM_DBSA1\t0x310\n> +#define UNICAM_DBEA1\t0x314\n> +#define UNICAM_MISC\t0x400\n> +\n> +/*\n> + * The following bitmasks are from the kernel released by Broadcom\n> + * for Android - https://android.googlesource.com/kernel/bcm/\n> + * The Rhea, Hawaii, and Java chips all contain the same VideoCore4\n> + * Unicam block as BCM2835, as defined in eg\n> + * arch/arm/mach-rhea/include/mach/rdb_A0/brcm_rdb_cam.h and similar.\n> + * Values reworked to use the kernel BIT and GENMASK macros.\n> + *\n> + * Some of the bit mnenomics have been amended to match the datasheet.\n> + */\n> +/* UNICAM_CTRL Register */\n> +#define UNICAM_CPE\t\tBIT(0)\n> +#define UNICAM_MEM\t\tBIT(1)\n> +#define UNICAM_CPR\t\tBIT(2)\n> +#define UNICAM_CPM_MASK\t\tGENMASK(3, 3)\n> +#define UNICAM_CPM_CSI2\t\t0\n> +#define UNICAM_CPM_CCP2\t\t1\n> +#define UNICAM_SOE\t\tBIT(4)\n> +#define UNICAM_DCM_MASK\t\tGENMASK(5, 5)\n> +#define UNICAM_DCM_STROBE\t0\n> +#define UNICAM_DCM_DATA\t\t1\n> +#define UNICAM_SLS\t\tBIT(6)\n> +#define UNICAM_PFT_MASK\t\tGENMASK(11, 8)\n> +#define UNICAM_OET_MASK\t\tGENMASK(20, 12)\n> +\n> +/* UNICAM_STA Register */\n> +#define UNICAM_SYN\t\tBIT(0)\n> +#define UNICAM_CS\t\tBIT(1)\n> +#define UNICAM_SBE\t\tBIT(2)\n> +#define UNICAM_PBE\t\tBIT(3)\n> +#define UNICAM_HOE\t\tBIT(4)\n> +#define UNICAM_PLE\t\tBIT(5)\n> +#define UNICAM_SSC\t\tBIT(6)\n> +#define UNICAM_CRCE\t\tBIT(7)\n> +#define UNICAM_OES\t\tBIT(8)\n> +#define UNICAM_IFO\t\tBIT(9)\n> +#define UNICAM_OFO\t\tBIT(10)\n> +#define UNICAM_BFO\t\tBIT(11)\n> +#define UNICAM_DL\t\tBIT(12)\n> +#define UNICAM_PS\t\tBIT(13)\n> +#define UNICAM_IS\t\tBIT(14)\n> +#define UNICAM_PI0\t\tBIT(15)\n> +#define UNICAM_PI1\t\tBIT(16)\n> +#define UNICAM_FSI_S\t\tBIT(17)\n> +#define UNICAM_FEI_S\t\tBIT(18)\n> +#define UNICAM_LCI_S\t\tBIT(19)\n> +#define UNICAM_BUF0_RDY\t\tBIT(20)\n> +#define UNICAM_BUF0_NO\t\tBIT(21)\n> +#define UNICAM_BUF1_RDY\t\tBIT(22)\n> +#define UNICAM_BUF1_NO\t\tBIT(23)\n> +#define UNICAM_DI\t\tBIT(24)\n> +\n> +#define UNICAM_STA_MASK_ALL \\\n> +\t\t(UNICAM_DL + \\\n> +\t\tUNICAM_SBE + \\\n> +\t\tUNICAM_PBE + \\\n> +\t\tUNICAM_HOE + \\\n> +\t\tUNICAM_PLE + \\\n> +\t\tUNICAM_SSC + \\\n> +\t\tUNICAM_CRCE + \\\n> +\t\tUNICAM_IFO + \\\n> +\t\tUNICAM_OFO + \\\n> +\t\tUNICAM_PS + \\\n> +\t\tUNICAM_PI0 + \\\n> +\t\tUNICAM_PI1)\n> +\n> +/* UNICAM_ANA Register */\n> +#define UNICAM_APD\t\tBIT(0)\n> +#define UNICAM_BPD\t\tBIT(1)\n> +#define UNICAM_AR\t\tBIT(2)\n> +#define UNICAM_DDL\t\tBIT(3)\n> +#define UNICAM_CTATADJ_MASK\tGENMASK(7, 4)\n> +#define UNICAM_PTATADJ_MASK\tGENMASK(11, 8)\n> +\n> +/* UNICAM_PRI Register */\n> +#define UNICAM_PE\t\tBIT(0)\n> +#define UNICAM_PT_MASK\t\tGENMASK(2, 1)\n> +#define UNICAM_NP_MASK\t\tGENMASK(7, 4)\n> +#define UNICAM_PP_MASK\t\tGENMASK(11, 8)\n> +#define UNICAM_BS_MASK\t\tGENMASK(15, 12)\n> +#define UNICAM_BL_MASK\t\tGENMASK(17, 16)\n> +\n> +/* UNICAM_CLK Register */\n> +#define UNICAM_CLE\t\tBIT(0)\n> +#define UNICAM_CLPD\t\tBIT(1)\n> +#define UNICAM_CLLPE\t\tBIT(2)\n> +#define UNICAM_CLHSE\t\tBIT(3)\n> +#define UNICAM_CLTRE\t\tBIT(4)\n> +#define UNICAM_CLAC_MASK\tGENMASK(8, 5)\n> +#define UNICAM_CLSTE\t\tBIT(29)\n> +\n> +/* UNICAM_CLT Register */\n> +#define UNICAM_CLT1_MASK\tGENMASK(7, 0)\n> +#define UNICAM_CLT2_MASK\tGENMASK(15, 8)\n> +\n> +/* UNICAM_DATn Registers */\n> +#define UNICAM_DLE\t\tBIT(0)\n> +#define UNICAM_DLPD\t\tBIT(1)\n> +#define UNICAM_DLLPE\t\tBIT(2)\n> +#define UNICAM_DLHSE\t\tBIT(3)\n> +#define UNICAM_DLTRE\t\tBIT(4)\n> +#define UNICAM_DLSM\t\tBIT(5)\n> +#define UNICAM_DLFO\t\tBIT(28)\n> +#define UNICAM_DLSTE\t\tBIT(29)\n> +\n> +#define UNICAM_DAT_MASK_ALL (UNICAM_DLSTE + UNICAM_DLFO)\n> +\n> +/* UNICAM_DLT Register */\n> +#define UNICAM_DLT1_MASK\tGENMASK(7, 0)\n> +#define UNICAM_DLT2_MASK\tGENMASK(15, 8)\n> +#define UNICAM_DLT3_MASK\tGENMASK(23, 16)\n> +\n> +/* UNICAM_ICTL Register */\n> +#define UNICAM_FSIE\t\tBIT(0)\n> +#define UNICAM_FEIE\t\tBIT(1)\n> +#define UNICAM_IBOB\t\tBIT(2)\n> +#define UNICAM_FCM\t\tBIT(3)\n> +#define UNICAM_TFC\t\tBIT(4)\n> +#define UNICAM_LIP_MASK\t\tGENMASK(6, 5)\n> +#define UNICAM_LCIE_MASK\tGENMASK(28, 16)\n> +\n> +/* UNICAM_IDI0/1 Register */\n> +#define UNICAM_ID0_MASK\t\tGENMASK(7, 0)\n> +#define UNICAM_ID1_MASK\t\tGENMASK(15, 8)\n> +#define UNICAM_ID2_MASK\t\tGENMASK(23, 16)\n> +#define UNICAM_ID3_MASK\t\tGENMASK(31, 24)\n> +\n> +/* UNICAM_ISTA Register */\n> +#define UNICAM_FSI\t\tBIT(0)\n> +#define UNICAM_FEI\t\tBIT(1)\n> +#define UNICAM_LCI\t\tBIT(2)\n> +\n> +#define UNICAM_ISTA_MASK_ALL (UNICAM_FSI + UNICAM_FEI + UNICAM_LCI)\n> +\n> +/* UNICAM_IPIPE Register */\n> +#define UNICAM_PUM_MASK\t\tGENMASK(2, 0)\n> +\t\t/* Unpacking modes */\n> +\t\t#define UNICAM_PUM_NONE\t\t0\n> +\t\t#define UNICAM_PUM_UNPACK6\t1\n> +\t\t#define UNICAM_PUM_UNPACK7\t2\n> +\t\t#define UNICAM_PUM_UNPACK8\t3\n> +\t\t#define UNICAM_PUM_UNPACK10\t4\n> +\t\t#define UNICAM_PUM_UNPACK12\t5\n> +\t\t#define UNICAM_PUM_UNPACK14\t6\n> +\t\t#define UNICAM_PUM_UNPACK16\t7\n> +#define UNICAM_DDM_MASK\t\tGENMASK(6, 3)\n> +#define UNICAM_PPM_MASK\t\tGENMASK(9, 7)\n> +\t\t/* Packing modes */\n> +\t\t#define UNICAM_PPM_NONE\t\t0\n> +\t\t#define UNICAM_PPM_PACK8\t1\n> +\t\t#define UNICAM_PPM_PACK10\t2\n> +\t\t#define UNICAM_PPM_PACK12\t3\n> +\t\t#define UNICAM_PPM_PACK14\t4\n> +\t\t#define UNICAM_PPM_PACK16\t5\n> +#define UNICAM_DEM_MASK\t\tGENMASK(11, 10)\n> +#define UNICAM_DEBL_MASK\tGENMASK(14, 12)\n> +#define UNICAM_ICM_MASK\t\tGENMASK(16, 15)\n> +#define UNICAM_IDM_MASK\t\tGENMASK(17, 17)\n> +\n> +/* UNICAM_ICC Register */\n> +#define UNICAM_ICFL_MASK\tGENMASK(4, 0)\n> +#define UNICAM_ICFH_MASK\tGENMASK(9, 5)\n> +#define UNICAM_ICST_MASK\tGENMASK(12, 10)\n> +#define UNICAM_ICLT_MASK\tGENMASK(15, 13)\n> +#define UNICAM_ICLL_MASK\tGENMASK(31, 16)\n> +\n> +/* UNICAM_DCS Register */\n> +#define UNICAM_DIE\t\tBIT(0)\n> +#define UNICAM_DIM\t\tBIT(1)\n> +#define UNICAM_DBOB\t\tBIT(3)\n> +#define UNICAM_FDE\t\tBIT(4)\n> +#define UNICAM_LDP\t\tBIT(5)\n> +#define UNICAM_EDL_MASK\t\tGENMASK(15, 8)\n> +\n> +/* UNICAM_DBCTL Register */\n> +#define UNICAM_DBEN\t\tBIT(0)\n> +#define UNICAM_BUF0_IE\t\tBIT(1)\n> +#define UNICAM_BUF1_IE\t\tBIT(2)\n> +\n> +/* UNICAM_CMP[0,1] register */\n> +#define UNICAM_PCE\t\tBIT(31)\n> +#define UNICAM_GI\t\tBIT(9)\n> +#define UNICAM_CPH\t\tBIT(8)\n> +#define UNICAM_PCVC_MASK\tGENMASK(7, 6)\n> +#define UNICAM_PCDT_MASK\tGENMASK(5, 0)\n> +\n> +/* UNICAM_MISC register */\n> +#define UNICAM_FL0\t\tBIT(6)\n> +#define UNICAM_FL1\t\tBIT(9)\n> +\n> +#endif\n> \n\nRegards,\n\n\tHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xz9050KS7z9sNc\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tFri, 22 Sep 2017 20:37:08 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751912AbdIVKhG (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tFri, 22 Sep 2017 06:37:06 -0400","from lb1-smtp-cloud7.xs4all.net ([194.109.24.24]:33918 \"EHLO\n\tlb1-smtp-cloud7.xs4all.net\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751903AbdIVKhE (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Fri, 22 Sep 2017 06:37:04 -0400","from [192.168.1.10] ([80.101.105.217])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid vLKOdZAh4G5oqvLKPdIRrh; Fri, 22 Sep 2017 12:37:02 +0200"],"Subject":"Re: [PATCH v3 3/4] [media] bcm2835-unicam: Driver for CCP2/CSI2\n\tcamera interface","To":"Dave Stevenson <dave.stevenson@raspberrypi.org>,\n\tMauro Carvalho Chehab <mchehab@kernel.org>,\n\tRob Herring <robh+dt@kernel.org>, Hans Verkuil <hans.verkuil@cisco.com>, \n\tEric Anholt <eric@anholt.net>, Stefan Wahren <stefan.wahren@i2se.com>,\n\tSakari Ailus <sakari.ailus@iki.fi>,\n\tlinux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,\n\tdevicetree@vger.kernel.org","References":"<cover.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<b58d96778205cbfd35bb0f302df7f93bdabe4af0.1505916622.git.dave.stevenson@raspberrypi.org>","From":"Hans Verkuil <hverkuil@xs4all.nl>","Message-ID":"<927e6bd8-bdcd-4415-8d41-422a6da1220a@xs4all.nl>","Date":"Fri, 22 Sep 2017 12:37:00 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.7.1","MIME-Version":"1.0","In-Reply-To":"<b58d96778205cbfd35bb0f302df7f93bdabe4af0.1505916622.git.dave.stevenson@raspberrypi.org>","Content-Type":"text/plain; charset=windows-1252","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfHSHpCwXq60jWj2/3ilPUPXCsn20vcsWk7cYrktCg7vZ+X0xUllf93qqtZQnzAHFZ+vzDcd+XTo3BKHH3KPrRFmw/k3uK1CWfDIY1mcuNkVrRfuVAug+\n\trxfKSIfRLRKU7S3AnmCcgh9sNObmxCnIXgSm0lzkfJ6HBwzZ3QTS8qFV6QUzsqsGV8Qtn4XE2AidvZymZz/JyeKz6orTcoJSKBtBQrbIpc1wWmgUbjsvV+WI\n\tZO5txWEoRFI4MVa++hoIUXjXpy7IYMVlQ4U3ybM31csiqwNVjTDlSimIyI8VLg045Wum1z9967/C2V5F6RCwogWibz0QoNT8BQbVhjVHKZEn1Ib/mgtwBB26\n\t13UHG0C4ibijYrAlexjKMOdiSs9T0MwlNEwH619lEQMrhAM/4SshU9A8tCzN+zuGRRCcHXjiFZqKUIaVF+XNwgmehk5VyiBczZPZiwYnjcaKTxMUJTHMeIqi\n\t9wbzXrtu6yGOuhW4Q/F53OkKvXRcCNjWgq9+NFzC262RrDEai0GZUCl4w/HCuZXm/xQ+0ncdE+5YUxxCPnd5EryyyCHdQktUpwBFjaJGavCx06dT+fF3HgR2\n\tIi4=","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1786676,"web_url":"http://patchwork.ozlabs.org/comment/1786676/","msgid":"<20171013210937.pzgmozz7elsb3yo5@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-10-13T21:09:37","subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Dave,\n\nOn Wed, Sep 20, 2017 at 05:07:54PM +0100, Dave Stevenson wrote:\n> New helper function char *v4l2_fourcc2s(u32 fourcc, char *buf)\n> that converts a fourcc into a nice printable version.\n> \n> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>\n> ---\n> \n> No changes from v2 to v3\n> \n>  drivers/media/v4l2-core/v4l2-common.c | 18 ++++++++++++++++++\n>  include/media/v4l2-common.h           |  3 +++\n>  2 files changed, 21 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c\n> index a5ea1f5..0219895 100644\n> --- a/drivers/media/v4l2-core/v4l2-common.c\n> +++ b/drivers/media/v4l2-core/v4l2-common.c\n> @@ -405,3 +405,21 @@ void v4l2_get_timestamp(struct timeval *tv)\n>  \ttv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;\n>  }\n>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);\n> +\n> +char *v4l2_fourcc2s(u32 fourcc, char *buf)\n> +{\n> +\tbuf[0] = fourcc & 0x7f;\n> +\tbuf[1] = (fourcc >> 8) & 0x7f;\n> +\tbuf[2] = (fourcc >> 16) & 0x7f;\n> +\tbuf[3] = (fourcc >> 24) & 0x7f;\n> +\tif (fourcc & (1 << 31)) {\n> +\t\tbuf[4] = '-';\n> +\t\tbuf[5] = 'B';\n> +\t\tbuf[6] = 'E';\n> +\t\tbuf[7] = '\\0';\n> +\t} else {\n> +\t\tbuf[4] = '\\0';\n> +\t}\n> +\treturn buf;\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_fourcc2s);\n> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h\n> index aac8b7b..5b0fff9 100644\n> --- a/include/media/v4l2-common.h\n> +++ b/include/media/v4l2-common.h\n> @@ -264,4 +264,7 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(\n>  \n>  void v4l2_get_timestamp(struct timeval *tv);\n>  \n> +#define V4L2_FOURCC_MAX_SIZE 8\n> +char *v4l2_fourcc2s(u32 fourcc, char *buf);\n> +\n>  #endif /* V4L2_COMMON_H_ */\n\nI like the idea but the use of a character pointer and assuming a length\nlooks a bit scary.\n\nAs this seems to be used uniquely for printing stuff, a couple of macros\ncould be used instead. Something like\n\n#define V4L2_FOURCC_CONV \"%c%c%c%c%s\"\n#define V4L2_FOURCC_TO_CONV(fourcc) \\\n\tfourcc & 0x7f, (fourcc >> 8) & 0x7f, (fourcc >> 16) & 0x7f, \\\n\t(fourcc >> 24) & 0x7f, fourcc & BIT(31) ? \"-BE\" : \"\"\n\nYou could use it with printk-style functions, e.g.\n\n\tprintk(\"blah fourcc \" V4L2_FOURCC_CONV \" is a nice format\",\n\t       V4L2_FOURCC_TO_CONV(fourcc));\n\nI guess it'd be better to add more parentheses around \"fourcc\" but it'd be\nless clear here that way.","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yDL2G73C1z9t2S\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tSat, 14 Oct 2017 08:09:42 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752288AbdJMVJl (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tFri, 13 Oct 2017 17:09:41 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:34924 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S1752195AbdJMVJk (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Fri, 13 Oct 2017 17:09:40 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 636BC600EE;\n\tSat, 14 Oct 2017 00:09:38 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakari.ailus@retiisi.org.uk>)\n\tid 1e37D7-0002y3-Ta; Sat, 14 Oct 2017 00:09:37 +0300"],"Date":"Sat, 14 Oct 2017 00:09:37 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Dave Stevenson <dave.stevenson@raspberrypi.org>","Cc":"Mauro Carvalho Chehab <mchehab@kernel.org>,\n\tRob Herring <robh+dt@kernel.org>, Hans Verkuil <hans.verkuil@cisco.com>, \n\tEric Anholt <eric@anholt.net>, Stefan Wahren <stefan.wahren@i2se.com>,\n\tlinux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,\n\tdevicetree@vger.kernel.org","Subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","Message-ID":"<20171013210937.pzgmozz7elsb3yo5@valkosipuli.retiisi.org.uk>","References":"<cover.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<e6dfbe4afd3f1db4c3f8a81c0813dc418896f5e1.1505916622.git.dave.stevenson@raspberrypi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<e6dfbe4afd3f1db4c3f8a81c0813dc418896f5e1.1505916622.git.dave.stevenson@raspberrypi.org>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1788139,"web_url":"http://patchwork.ozlabs.org/comment/1788139/","msgid":"<CAAoAYcN441pVUqCu00hbKmEQWyNaK4jdwkufpJ2P8iXkcQG5KA@mail.gmail.com>","list_archive_url":null,"date":"2017-10-17T09:17:07","subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","submitter":{"id":72357,"url":"http://patchwork.ozlabs.org/api/people/72357/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.org"},"content":"Hi Sakari.\n\nOn 13 October 2017 at 22:09, Sakari Ailus <sakari.ailus@iki.fi> wrote:\n> Hi Dave,\n>\n> On Wed, Sep 20, 2017 at 05:07:54PM +0100, Dave Stevenson wrote:\n>> New helper function char *v4l2_fourcc2s(u32 fourcc, char *buf)\n>> that converts a fourcc into a nice printable version.\n>>\n>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>\n>> ---\n>>\n>> No changes from v2 to v3\n>>\n>>  drivers/media/v4l2-core/v4l2-common.c | 18 ++++++++++++++++++\n>>  include/media/v4l2-common.h           |  3 +++\n>>  2 files changed, 21 insertions(+)\n>>\n>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c\n>> index a5ea1f5..0219895 100644\n>> --- a/drivers/media/v4l2-core/v4l2-common.c\n>> +++ b/drivers/media/v4l2-core/v4l2-common.c\n>> @@ -405,3 +405,21 @@ void v4l2_get_timestamp(struct timeval *tv)\n>>       tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;\n>>  }\n>>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);\n>> +\n>> +char *v4l2_fourcc2s(u32 fourcc, char *buf)\n>> +{\n>> +     buf[0] = fourcc & 0x7f;\n>> +     buf[1] = (fourcc >> 8) & 0x7f;\n>> +     buf[2] = (fourcc >> 16) & 0x7f;\n>> +     buf[3] = (fourcc >> 24) & 0x7f;\n>> +     if (fourcc & (1 << 31)) {\n>> +             buf[4] = '-';\n>> +             buf[5] = 'B';\n>> +             buf[6] = 'E';\n>> +             buf[7] = '\\0';\n>> +     } else {\n>> +             buf[4] = '\\0';\n>> +     }\n>> +     return buf;\n>> +}\n>> +EXPORT_SYMBOL_GPL(v4l2_fourcc2s);\n>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h\n>> index aac8b7b..5b0fff9 100644\n>> --- a/include/media/v4l2-common.h\n>> +++ b/include/media/v4l2-common.h\n>> @@ -264,4 +264,7 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(\n>>\n>>  void v4l2_get_timestamp(struct timeval *tv);\n>>\n>> +#define V4L2_FOURCC_MAX_SIZE 8\n>> +char *v4l2_fourcc2s(u32 fourcc, char *buf);\n>> +\n>>  #endif /* V4L2_COMMON_H_ */\n>\n> I like the idea but the use of a character pointer and assuming a length\n> looks a bit scary.\n>\n> As this seems to be used uniquely for printing stuff, a couple of macros\n> could be used instead. Something like\n>\n> #define V4L2_FOURCC_CONV \"%c%c%c%c%s\"\n> #define V4L2_FOURCC_TO_CONV(fourcc) \\\n>         fourcc & 0x7f, (fourcc >> 8) & 0x7f, (fourcc >> 16) & 0x7f, \\\n>         (fourcc >> 24) & 0x7f, fourcc & BIT(31) ? \"-BE\" : \"\"\n>\n> You could use it with printk-style functions, e.g.\n>\n>         printk(\"blah fourcc \" V4L2_FOURCC_CONV \" is a nice format\",\n>                V4L2_FOURCC_TO_CONV(fourcc));\n>\n> I guess it'd be better to add more parentheses around \"fourcc\" but it'd be\n> less clear here that way.\n\nI was following Hans' suggestion made in\nhttps://www.spinics.net/lists/linux-media/msg117046.html\n\nHans: Do you agree with Sakari here to make it to a macro?\n\n  Dave\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tsecure) header.d=raspberrypi.org header.i=@raspberrypi.org\n\theader.b=\"T1kugIU3\"; \n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi-org.20150623.gappssmtp.com\n\theader.i=@raspberrypi-org.20150623.gappssmtp.com header.b=\"EvjX+cbs\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yGV2W1N5cz9s83\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue, 17 Oct 2017 20:17:23 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1760202AbdJQJRR (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tTue, 17 Oct 2017 05:17:17 -0400","from mx07-00252a01.pphosted.com ([62.209.51.214]:39799 \"EHLO\n\tmx07-00252a01.pphosted.com\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1760199AbdJQJRN (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Tue, 17 Oct 2017 05:17:13 -0400","from pps.filterd (m0102628.ppops.net [127.0.0.1])\n\tby mx07-00252a01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv9H9DFVT020910\n\tfor <devicetree@vger.kernel.org>; Tue, 17 Oct 2017 10:17:11 +0100","from mail-pf0-f197.google.com (mail-pf0-f197.google.com\n\t[209.85.192.197])\n\tby mx07-00252a01.pphosted.com with ESMTP id 2dk8001dah-1\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128\n\tverify=OK)\n\tfor <devicetree@vger.kernel.org>; Tue, 17 Oct 2017 10:17:11 +0100","by mail-pf0-f197.google.com with SMTP id p2so927252pfk.13\n\tfor <devicetree@vger.kernel.org>;\n\tTue, 17 Oct 2017 02:17:10 -0700 (PDT)","by 10.100.229.6 with HTTP; Tue, 17 Oct 2017 02:17:07 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.org;\n\th=mime-version :\n\tin-reply-to : references : from : date : message-id : subject : to :\n\tcc :\n\tcontent-type; s=pp; bh=lP0mqBq87E0DhR2fLf+s2Ba6ylHsQfafYWf2VGnX7x8=; \n\tb=T1kugIU3rc8XY4hqiMi/Q+djyssSLoIkbVV1wx2m3QeOk6mnWPUvLYiu+57jkzmUxghk\n\tkJgC76/rRvTBA743rUEqNup6yTr4rSJc6X0BcoaTdVknD0/GaGhKEOW6i+QOsOn1bmtr\n\t5QKVOS632DoMaYXm5nLhFy6mnTGBftRkTYjmkTYVZaIV89fbC3pFJKhDCKRBgzQzPrd5\n\tLaA/Aja6Z9fknsGWpqyrjysVSPN9ggrm2hhfTACavFXcChbJ19XDWiEx54onaTZ1KlfY\n\tDCKy3MisRu0sBV58VkfMcy53P22Nq0teRQT1ziRtKuTHaEXltfNbWD2iA7b1FTRjHNUH\n\t0Q== ","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi-org.20150623.gappssmtp.com; s=20150623;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=lP0mqBq87E0DhR2fLf+s2Ba6ylHsQfafYWf2VGnX7x8=;\n\tb=EvjX+cbslgn0LwsmPPDTaYeonh8NULEoPulZ7bqTdmT+0kRd4wXgvANjCwRofp9mbi\n\tsFcy1dIUUXFoqF3aRWvkO1ouz9EwDKZz79/QIfg9RQsq3uA/Fb1CNhiBkdpltOTMwimT\n\tj7U/smJVKvy3l99ObMAT7Rm1M81yRTio5E37Jd15sqDb3m2aZSLLo4mQoQKiL/z1R5GI\n\trxSCKmKFDz9SstQgfftkgrEFM8Rq7hZ1RzDNloFP5slJToWDsXglUxA0VA1JztBOXJNB\n\t3VJiT021VZUPzkK16csl2Z23rmvlA09PV52LC26WFoe5l8un0Us8OyWYcLF623NcyMT8\n\t3xpg=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=lP0mqBq87E0DhR2fLf+s2Ba6ylHsQfafYWf2VGnX7x8=;\n\tb=ExOcP3Q2tYsx7Urh/N/MSGUvGUyz4DWjD30VuwIdZ/n5dJqhUSdtGtK/dQuc2+chP7\n\tCxPmLR6z5JaM1lFxtLCEg0r6ZaZdEcZLFXxCPmW2hPMcg0+Lapdd8KI+S2qXURey469X\n\tTb3C1Gtdhkqgw88ulTcXta4o+BjmjBgtBw7H7kN6+j5XwlqstKNAbopgn275Xigm0KmT\n\tjwiIuoLvfQdUoS0+HtUs+/EERAg79BlBuEhb3svMqIFd+MZcbjtw/+Shl6jCa7nSCets\n\t4T3Een9BaRltRbgkqe6B42rXxoUSlmOwhUN5Bm1S4t4/RzTtOPYDOojBpfKFdlmv5J7F\n\t43zA==","X-Gm-Message-State":"AMCzsaXulsrIpAfqxUHfLRDKin24GmNj5Ol5ExzcA5D187suyUj3+8XX\n\tM5P+PpJDSFJ/6rrmzrie1ptoJ0bcHE+FlkdbQ625m3AsFtFZkLPIbQhPQRvfTOgLEKfVdBqUxs9\n\teqjitDytMNnW3mCOx5g5eWni3IXjT/dB36FzZ","X-Received":["by 10.99.37.193 with SMTP id l184mr10843619pgl.14.1508231828832; \n\tTue, 17 Oct 2017 02:17:08 -0700 (PDT)","by 10.99.37.193 with SMTP id l184mr10843599pgl.14.1508231828475; \n\tTue, 17 Oct 2017 02:17:08 -0700 (PDT)"],"X-Google-Smtp-Source":"AOwi7QBlIyACMu6hpM3XPTBe4zeI0vPL7XuDx7FyYRtU1+8OmIFzojvGGiqRjN1G0jUZElQjyCtZMHJKpJ2+VUOL9zo=","MIME-Version":"1.0","In-Reply-To":"<20171013210937.pzgmozz7elsb3yo5@valkosipuli.retiisi.org.uk>","References":"<cover.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<e6dfbe4afd3f1db4c3f8a81c0813dc418896f5e1.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<20171013210937.pzgmozz7elsb3yo5@valkosipuli.retiisi.org.uk>","From":"Dave Stevenson <dave.stevenson@raspberrypi.org>","Date":"Tue, 17 Oct 2017 10:17:07 +0100","Message-ID":"<CAAoAYcN441pVUqCu00hbKmEQWyNaK4jdwkufpJ2P8iXkcQG5KA@mail.gmail.com>","Subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","To":"Sakari Ailus <sakari.ailus@iki.fi>, Hans Verkuil <hans.verkuil@cisco.com>","Cc":"Mauro Carvalho Chehab <mchehab@kernel.org>,\n\tRob Herring <robh+dt@kernel.org>, Eric Anholt <eric@anholt.net>,\n\tStefan Wahren <stefan.wahren@i2se.com>,\n\tlinux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,\n\tdevicetree@vger.kernel.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-10-17_06:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_spam_notspam policy=outbound_spam\n\tscore=0 priorityscore=1501\n\tmalwarescore=0 suspectscore=1 phishscore=0 bulkscore=0 spamscore=0\n\tclxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0\n\tclassifier=spam adjust=0 reason=mlx scancount=1\n\tengine=8.0.1-1707230000\n\tdefinitions=main-1710170130","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1788148,"web_url":"http://patchwork.ozlabs.org/comment/1788148/","msgid":"<8cd44cbf-6751-c3de-1729-c8c1d54e8b00@cisco.com>","list_archive_url":null,"date":"2017-10-17T09:28:46","subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","submitter":{"id":26081,"url":"http://patchwork.ozlabs.org/api/people/26081/","name":"Hans Verkuil (hansverk)","email":"hansverk@cisco.com"},"content":"On 10/17/17 11:17, Dave Stevenson wrote:\n> Hi Sakari.\n> \n> On 13 October 2017 at 22:09, Sakari Ailus <sakari.ailus@iki.fi> wrote:\n>> Hi Dave,\n>>\n>> On Wed, Sep 20, 2017 at 05:07:54PM +0100, Dave Stevenson wrote:\n>>> New helper function char *v4l2_fourcc2s(u32 fourcc, char *buf)\n>>> that converts a fourcc into a nice printable version.\n>>>\n>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>\n>>> ---\n>>>\n>>> No changes from v2 to v3\n>>>\n>>>  drivers/media/v4l2-core/v4l2-common.c | 18 ++++++++++++++++++\n>>>  include/media/v4l2-common.h           |  3 +++\n>>>  2 files changed, 21 insertions(+)\n>>>\n>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c\n>>> index a5ea1f5..0219895 100644\n>>> --- a/drivers/media/v4l2-core/v4l2-common.c\n>>> +++ b/drivers/media/v4l2-core/v4l2-common.c\n>>> @@ -405,3 +405,21 @@ void v4l2_get_timestamp(struct timeval *tv)\n>>>       tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;\n>>>  }\n>>>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);\n>>> +\n>>> +char *v4l2_fourcc2s(u32 fourcc, char *buf)\n>>> +{\n>>> +     buf[0] = fourcc & 0x7f;\n>>> +     buf[1] = (fourcc >> 8) & 0x7f;\n>>> +     buf[2] = (fourcc >> 16) & 0x7f;\n>>> +     buf[3] = (fourcc >> 24) & 0x7f;\n>>> +     if (fourcc & (1 << 31)) {\n>>> +             buf[4] = '-';\n>>> +             buf[5] = 'B';\n>>> +             buf[6] = 'E';\n>>> +             buf[7] = '\\0';\n>>> +     } else {\n>>> +             buf[4] = '\\0';\n>>> +     }\n>>> +     return buf;\n>>> +}\n>>> +EXPORT_SYMBOL_GPL(v4l2_fourcc2s);\n>>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h\n>>> index aac8b7b..5b0fff9 100644\n>>> --- a/include/media/v4l2-common.h\n>>> +++ b/include/media/v4l2-common.h\n>>> @@ -264,4 +264,7 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(\n>>>\n>>>  void v4l2_get_timestamp(struct timeval *tv);\n>>>\n>>> +#define V4L2_FOURCC_MAX_SIZE 8\n>>> +char *v4l2_fourcc2s(u32 fourcc, char *buf);\n>>> +\n>>>  #endif /* V4L2_COMMON_H_ */\n>>\n>> I like the idea but the use of a character pointer and assuming a length\n>> looks a bit scary.\n>>\n>> As this seems to be used uniquely for printing stuff, a couple of macros\n>> could be used instead. Something like\n>>\n>> #define V4L2_FOURCC_CONV \"%c%c%c%c%s\"\n>> #define V4L2_FOURCC_TO_CONV(fourcc) \\\n>>         fourcc & 0x7f, (fourcc >> 8) & 0x7f, (fourcc >> 16) & 0x7f, \\\n>>         (fourcc >> 24) & 0x7f, fourcc & BIT(31) ? \"-BE\" : \"\"\n\n'fourcc' should be in parenthesis '(fourcc)'.\n\n>>\n>> You could use it with printk-style functions, e.g.\n>>\n>>         printk(\"blah fourcc \" V4L2_FOURCC_CONV \" is a nice format\",\n>>                V4L2_FOURCC_TO_CONV(fourcc));\n>>\n>> I guess it'd be better to add more parentheses around \"fourcc\" but it'd be\n>> less clear here that way.\n> \n> I was following Hans' suggestion made in\n> https://www.spinics.net/lists/linux-media/msg117046.html\n> \n> Hans: Do you agree with Sakari here to make it to a macro?\n\nNot a bad idea. But I think we should add it to the public videodev2.h\nheader in that case, allowing it to be used by applications as well.\n\nHow about calling the defines V4L2_FOURCC_FMT and V4L2_FOURCC_FMT_ARGS?\n\nRegards,\n\n\tHans\n--\nTo unsubscribe from this list: send the line \"unsubscribe devicetree\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=cisco.com header.i=@cisco.com\n\theader.b=\"CsWEHX2t\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yGVHm1Xtgz9sRm\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue, 17 Oct 2017 20:28:52 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753565AbdJQJ2u (ORCPT\n\t<rfc822;incoming-dt@patchwork.ozlabs.org>);\n\tTue, 17 Oct 2017 05:28:50 -0400","from aer-iport-4.cisco.com ([173.38.203.54]:30580 \"EHLO\n\taer-iport-4.cisco.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754716AbdJQJ2t (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Tue, 17 Oct 2017 05:28:49 -0400","from aer-iport-nat.cisco.com (HELO aer-core-3.cisco.com)\n\t([173.38.203.22])\n\tby aer-iport-4.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t17 Oct 2017 09:28:47 +0000","from [10.47.79.81] ([10.47.79.81]) (authenticated bits=0)\n\tby aer-core-3.cisco.com (8.14.5/8.14.5) with ESMTP id v9H9Sk6r011654\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO);\n\tTue, 17 Oct 2017 09:28:47 GMT"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=cisco.com; i=@cisco.com; l=3255; q=dns/txt; s=iport;\n\tt=1508232528; x=1509442128;\n\th=subject:to:cc:references:from:message-id:date:\n\tmime-version:in-reply-to:content-transfer-encoding;\n\tbh=oRIMtXJbqnmm5jdJsAKbx96/UIrXgY3kZmfOJ9AQ7ko=;\n\tb=CsWEHX2tm8fTO+EBpga4bpQTyd/oN29BXATS25xjpwkbqP3O7f6NZM4o\n\tucAdEMKPz7bcdoLfkI65k7ZzKXpor9kt7kmkqvHGH8/tw3siMz0K0e2Hz\n\tvZGaMx33tT/+SXIWE8igJx2KrQ9nbWaogDkByiLPXiBDfc0D8vU57djCz o=;","X-IronPort-AV":"E=Sophos;i=\"5.43,390,1503360000\"; d=\"scan'208\";a=\"658126419\"","Subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","To":"Dave Stevenson <dave.stevenson@raspberrypi.org>,\n\tSakari Ailus <sakari.ailus@iki.fi>, Hans Verkuil <hans.verkuil@cisco.com>","Cc":"Mauro Carvalho Chehab <mchehab@kernel.org>,\n\tRob Herring <robh+dt@kernel.org>, Eric Anholt <eric@anholt.net>,\n\tStefan Wahren <stefan.wahren@i2se.com>,\n\tlinux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,\n\tdevicetree@vger.kernel.org","References":"<cover.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<e6dfbe4afd3f1db4c3f8a81c0813dc418896f5e1.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<20171013210937.pzgmozz7elsb3yo5@valkosipuli.retiisi.org.uk>\n\t<CAAoAYcN441pVUqCu00hbKmEQWyNaK4jdwkufpJ2P8iXkcQG5KA@mail.gmail.com>","From":"Hans Verkuil <hansverk@cisco.com>","Message-ID":"<8cd44cbf-6751-c3de-1729-c8c1d54e8b00@cisco.com>","Date":"Tue, 17 Oct 2017 11:28:46 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<CAAoAYcN441pVUqCu00hbKmEQWyNaK4jdwkufpJ2P8iXkcQG5KA@mail.gmail.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Authenticated-User":"hansverk","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}},{"id":1788187,"web_url":"http://patchwork.ozlabs.org/comment/1788187/","msgid":"<20171017100851.lr4vmrwqucq4q46f@valkosipuli.retiisi.org.uk>","list_archive_url":null,"date":"2017-10-17T10:08:51","subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","submitter":{"id":1593,"url":"http://patchwork.ozlabs.org/api/people/1593/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Hans,\n\nOn Tue, Oct 17, 2017 at 11:28:46AM +0200, Hans Verkuil wrote:\n> On 10/17/17 11:17, Dave Stevenson wrote:\n> > Hi Sakari.\n> > \n> > On 13 October 2017 at 22:09, Sakari Ailus <sakari.ailus@iki.fi> wrote:\n> >> Hi Dave,\n> >>\n> >> On Wed, Sep 20, 2017 at 05:07:54PM +0100, Dave Stevenson wrote:\n> >>> New helper function char *v4l2_fourcc2s(u32 fourcc, char *buf)\n> >>> that converts a fourcc into a nice printable version.\n> >>>\n> >>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>\n> >>> ---\n> >>>\n> >>> No changes from v2 to v3\n> >>>\n> >>>  drivers/media/v4l2-core/v4l2-common.c | 18 ++++++++++++++++++\n> >>>  include/media/v4l2-common.h           |  3 +++\n> >>>  2 files changed, 21 insertions(+)\n> >>>\n> >>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c\n> >>> index a5ea1f5..0219895 100644\n> >>> --- a/drivers/media/v4l2-core/v4l2-common.c\n> >>> +++ b/drivers/media/v4l2-core/v4l2-common.c\n> >>> @@ -405,3 +405,21 @@ void v4l2_get_timestamp(struct timeval *tv)\n> >>>       tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;\n> >>>  }\n> >>>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);\n> >>> +\n> >>> +char *v4l2_fourcc2s(u32 fourcc, char *buf)\n> >>> +{\n> >>> +     buf[0] = fourcc & 0x7f;\n> >>> +     buf[1] = (fourcc >> 8) & 0x7f;\n> >>> +     buf[2] = (fourcc >> 16) & 0x7f;\n> >>> +     buf[3] = (fourcc >> 24) & 0x7f;\n> >>> +     if (fourcc & (1 << 31)) {\n> >>> +             buf[4] = '-';\n> >>> +             buf[5] = 'B';\n> >>> +             buf[6] = 'E';\n> >>> +             buf[7] = '\\0';\n> >>> +     } else {\n> >>> +             buf[4] = '\\0';\n> >>> +     }\n> >>> +     return buf;\n> >>> +}\n> >>> +EXPORT_SYMBOL_GPL(v4l2_fourcc2s);\n> >>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h\n> >>> index aac8b7b..5b0fff9 100644\n> >>> --- a/include/media/v4l2-common.h\n> >>> +++ b/include/media/v4l2-common.h\n> >>> @@ -264,4 +264,7 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(\n> >>>\n> >>>  void v4l2_get_timestamp(struct timeval *tv);\n> >>>\n> >>> +#define V4L2_FOURCC_MAX_SIZE 8\n> >>> +char *v4l2_fourcc2s(u32 fourcc, char *buf);\n> >>> +\n> >>>  #endif /* V4L2_COMMON_H_ */\n> >>\n> >> I like the idea but the use of a character pointer and assuming a length\n> >> looks a bit scary.\n> >>\n> >> As this seems to be used uniquely for printing stuff, a couple of macros\n> >> could be used instead. Something like\n> >>\n> >> #define V4L2_FOURCC_CONV \"%c%c%c%c%s\"\n> >> #define V4L2_FOURCC_TO_CONV(fourcc) \\\n> >>         fourcc & 0x7f, (fourcc >> 8) & 0x7f, (fourcc >> 16) & 0x7f, \\\n> >>         (fourcc >> 24) & 0x7f, fourcc & BIT(31) ? \"-BE\" : \"\"\n> \n> 'fourcc' should be in parenthesis '(fourcc)'.\n\nYes, I omitted those for readability here.\n\n> \n> >>\n> >> You could use it with printk-style functions, e.g.\n> >>\n> >>         printk(\"blah fourcc \" V4L2_FOURCC_CONV \" is a nice format\",\n> >>                V4L2_FOURCC_TO_CONV(fourcc));\n> >>\n> >> I guess it'd be better to add more parentheses around \"fourcc\" but it'd be\n> >> less clear here that way.\n> > \n> > I was following Hans' suggestion made in\n> > https://www.spinics.net/lists/linux-media/msg117046.html\n> > \n> > Hans: Do you agree with Sakari here to make it to a macro?\n> \n> Not a bad idea. But I think we should add it to the public videodev2.h\n> header in that case, allowing it to be used by applications as well.\n\nSounds good.\n\n> \n> How about calling the defines V4L2_FOURCC_FMT and V4L2_FOURCC_FMT_ARGS?\n\nprintf(3) describes conversion specifiers (%...) and I thought of using\n\"CONV\" there for that purpose to make it explicit. Fourcc, implicitly,\nalready is about formats. What would you think of V4L2_FOURCC_CONV and\nV4L2_FOURCC_CONV_ARGS (or just V4L2_FOURCC_ARGS)?","headers":{"Return-Path":"<devicetree-owner@vger.kernel.org>","X-Original-To":"incoming-dt@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-dt@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=devicetree-owner@vger.kernel.org; receiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yGWB52S1gz9s72\n\tfor <incoming-dt@patchwork.ozlabs.org>;\n\tTue, 17 Oct 2017 21:09:01 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S934947AbdJQKI5 (ORCPT <rfc822; incoming-dt@patchwork.ozlabs.org>);\n\tTue, 17 Oct 2017 06:08:57 -0400","from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:37536 \"EHLO\n\thillosipuli.retiisi.org.uk\" rhost-flags-OK-OK-OK-FAIL)\n\tby vger.kernel.org with ESMTP id S933518AbdJQKIz (ORCPT\n\t<rfc822; devicetree@vger.kernel.org>); Tue, 17 Oct 2017 06:08:55 -0400","from valkosipuli.localdomain (valkosipuli.retiisi.org.uk\n\t[IPv6:2001:1bc8:1a6:d3d5::80:2])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id A9EA760108;\n\tTue, 17 Oct 2017 13:08:52 +0300 (EEST)","from sakke by valkosipuli.localdomain with local (Exim 4.89)\n\t(envelope-from <sakari.ailus@retiisi.org.uk>)\n\tid 1e4Onr-0001Rg-TC; Tue, 17 Oct 2017 13:08:51 +0300"],"Date":"Tue, 17 Oct 2017 13:08:51 +0300","From":"Sakari Ailus <sakari.ailus@iki.fi>","To":"Hans Verkuil <hansverk@cisco.com>","Cc":"Dave Stevenson <dave.stevenson@raspberrypi.org>,\n\tHans Verkuil <hans.verkuil@cisco.com>,\n\tMauro Carvalho Chehab <mchehab@kernel.org>,\n\tRob Herring <robh+dt@kernel.org>, Eric Anholt <eric@anholt.net>,\n\tStefan Wahren <stefan.wahren@i2se.com>,\n\tlinux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,\n\tdevicetree@vger.kernel.org","Subject":"Re: [PATCH v3 1/4] [media] v4l2-common: Add helper function for\n\tfourcc to string","Message-ID":"<20171017100851.lr4vmrwqucq4q46f@valkosipuli.retiisi.org.uk>","References":"<cover.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<e6dfbe4afd3f1db4c3f8a81c0813dc418896f5e1.1505916622.git.dave.stevenson@raspberrypi.org>\n\t<20171013210937.pzgmozz7elsb3yo5@valkosipuli.retiisi.org.uk>\n\t<CAAoAYcN441pVUqCu00hbKmEQWyNaK4jdwkufpJ2P8iXkcQG5KA@mail.gmail.com>\n\t<8cd44cbf-6751-c3de-1729-c8c1d54e8b00@cisco.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<8cd44cbf-6751-c3de-1729-c8c1d54e8b00@cisco.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"devicetree-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<devicetree.vger.kernel.org>","X-Mailing-List":"devicetree@vger.kernel.org"}}]