Hi Alain
On Tue, Sep 26, 2023 at 11:28:20AM +0200, Alain Volmat wrote:
> Addition of support for the Galaxy Core GC2145 XVGA sensor.
> The sensor supports both DVP and CSI-2 interfaces however for
> the time being only CSI-2 is implemented.
>
> Configurations is currently based on initialization scripts
> coming from Galaxy Core and for that purpose only 3 static
> resolutions are supported with static framerates.
> - 640x480 (30fps)
> - 1280x720 (30fps)
> - 1600x1200 (20fps)
>
> Signed-off-by: Alain Volmat <alain.volmat@xxxxxxxxxxx>
> ---
> MAINTAINERS | 8 +
> drivers/media/i2c/Kconfig | 12 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/gc2145.c | 1591 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 1612 insertions(+)
> create mode 100644 drivers/media/i2c/gc2145.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 90f13281d297..c595335812c7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8600,6 +8600,14 @@ F: kernel/futex/*
> F: tools/perf/bench/futex*
> F: tools/testing/selftests/futex/
>
> +GALAXYCORE GC2145 SENSOR DRIVER
> +M: Alain Volmat <alain.volmat@xxxxxxxxxxx>
> +L: linux-media@xxxxxxxxxxxxxxx
> +S: Maintained
> +T: git git://linuxtv.org/media_tree.git
> +F: Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml
> +F: drivers/media/i2c/gc2145.c
> +
> GATEWORKS SYSTEM CONTROLLER (GSC) DRIVER
> M: Tim Harvey <tharvey@xxxxxxxxxxxxx>
> S: Maintained
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 74ff833ff48c..78e5c8cbf916 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -50,6 +50,18 @@ config VIDEO_AR0521
> To compile this driver as a module, choose M here: the
> module will be called ar0521.
>
> +config VIDEO_GC2145
> + tristate "GalaxyCore GC2145 sensor support"
> + depends on I2C && VIDEO_DEV
> + select VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
The top-level menu already:
select V4L2_FWNODE
select VIDEO_V4L2_SUBDEV_API
you can drop these two.
> + help
> + This is a V4L2 sensor-level driver for GalaxyCore GC2145
> + 2 Mpixel camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called gc2145.
> +
> config VIDEO_HI556
> tristate "Hynix Hi-556 sensor support"
> help
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 80b00d39b48f..e74eded89428 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_VIDEO_DW9719) += dw9719.o
> obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
> +obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
> obj-$(CONFIG_VIDEO_HI556) += hi556.o
> obj-$(CONFIG_VIDEO_HI846) += hi846.o
> obj-$(CONFIG_VIDEO_HI847) += hi847.o
> diff --git a/drivers/media/i2c/gc2145.c b/drivers/media/i2c/gc2145.c
> new file mode 100644
> index 000000000000..d0b065011732
> --- /dev/null
> +++ b/drivers/media/i2c/gc2145.c
> @@ -0,0 +1,1591 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * A V4L2 driver for Galaxycore GC2145 camera.
> + * Copyright (C) 2022, STMicroelectronics SA
2023 ?
> + *
> + * Inspired from the imx219.c driver
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
I would have added an empy line here. Up to you
> +#include <media/mipi-csi2.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mediabus.h>
> +
> +/* Chip ID */
> +#define GC2145_CHIP_ID 0x2145
> +
> +/* Page 0 */
> +#define GC2145_REG_HBLANK_HIGH 0x05
> +#define GC2145_REG_HBLANK_LOW 0x06
> +#define GC2145_REG_VBLANK_HIGH 0x07
> +#define GC2145_REG_VBLANK_LOW 0x08
> +#define GC2145_REG_ROW_START_HIGH 0x09
> +#define GC2145_REG_ROW_START_LOW 0x0a
> +#define GC2145_REG_COL_START_HIGH 0x0b
> +#define GC2145_REG_COL_START_LOW 0x0c
> +#define GC2145_REG_WIN_HEIGHT_HIGH 0x0d
> +#define GC2145_REG_WIN_HEIGHT_LOW 0x0e
> +#define GC2145_REG_WIN_WIDTH_HIGH 0x0f
> +#define GC2145_REG_WIN_WIDTH_LOW 0x10
> +#define GC2145_REG_ANALOG_MODE1 0x17
> +#define GC2145_REG_OUTPUT_FMT 0x84
> +#define GC2145_REG_SYNC_MODE 0x86
> +#define GC2145_SYNC_MODE_COL_SWITCH BIT(4)
> +#define GC2145_SYNC_MODE_ROW_SWITCH BIT(5)
> +#define GC2145_REG_DEBUG_MODE2 0x8c
> +#define GC2145_REG_DEBUG_MODE3 0x8d
> +#define GC2145_REG_CROP_ENABLE 0x90
> +#define GC2145_REG_CROP_Y_HIGH 0x91
> +#define GC2145_REG_CROP_Y_LOW 0x92
> +#define GC2145_REG_CROP_X_HIGH 0x93
> +#define GC2145_REG_CROP_X_LOW 0x94
> +#define GC2145_REG_CROP_HEIGHT_HIGH 0x95
> +#define GC2145_REG_CROP_HEIGHT_LOW 0x96
> +#define GC2145_REG_CROP_WIDTH_HIGH 0x97
> +#define GC2145_REG_CROP_WIDTH_LOW 0x98
> +#define GC2145_REG_CHIP_ID 0xf0
> +#define GC2145_REG_PAD_IO 0xf2
> +#define GC2145_REG_PAGE_SELECT 0xfe
> +/* Page 3 */
> +#define GC2145_REG_FIFO_FULL_LVL_LOW 0x04
> +#define GC2145_REG_FIFO_FULL_LVL_HIGH 0x05
> +#define GC2145_REG_MIPI_DT 0x11
> +#define GC2145_REG_LWC_LOW 0x12
> +#define GC2145_REG_LWC_HIGH 0x13
> +#define GC2145_REG_FIFO_GATE_MODE 0x17
> +
> +/* External clock frequency is 24.0MHz */
> +#define GC2145_XCLK_FREQ (24 * HZ_PER_MHZ)
> +
> +#define GC2145_NATIVE_WIDTH 1616U
> +#define GC2145_NATIVE_HEIGHT 1232U
> +
> +struct gc2145_reg {
> + unsigned char address;
> + unsigned char val;
> +};
> +
> +struct gc2145_reg_list {
> + unsigned int num_of_regs;
> + const struct gc2145_reg *regs;
> +};
> +
> +/**
> + * struct gc2145_mode - GC2145 mode description
> + * @width: frame width (in pixel)
> + * @height: frame height (in pixel)
> + * @frame_interval: interval (fractionnal) between 2 frames
> + * @reg_list: registers config sequence to enter into the mode
> + * @pixel_rate: pixel_rate associated with the mode
> + * @crop: window area captured
> + * @hblank: horizontal blanking setting of this mode
> + */
> +struct gc2145_mode {
> + unsigned int width;
> + unsigned int height;
> + struct v4l2_fract frame_interval;
> + struct gc2145_reg_list reg_list;
> + unsigned long pixel_rate;
> + struct v4l2_rect crop;
> + u32 hblank;
> +};
> +
> +static const struct gc2145_reg common_regs[] = {
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + /* SH Delay */
> + {0x12, 0x2e},
> + /* Flip */
> + {GC2145_REG_ANALOG_MODE1, 0x14},
> + /* Analog Conf */
> + {0x18, 0x22}, {0x19, 0x0e}, {0x1a, 0x01}, {0x1b, 0x4b},
> + {0x1c, 0x07}, {0x1d, 0x10}, {0x1e, 0x88}, {0x1f, 0x78},
> + {0x20, 0x03}, {0x21, 0x40}, {0x22, 0xa0}, {0x24, 0x16},
> + {0x25, 0x01}, {0x26, 0x10}, {0x2d, 0x60}, {0x30, 0x01},
> + {0x31, 0x90}, {0x33, 0x06}, {0x34, 0x01},
> + {0x80, 0x7f}, {0x81, 0x26}, {0x82, 0xfa}, {0x83, 0x00},
> + {0x84, 0x02}, {0x86, 0x02}, {0x88, 0x03}, {0x89, 0x03},
> + {0x85, 0x08}, {0x8a, 0x00}, {0x8b, 0x00}, {0xb0, 0x55},
> + {0xc3, 0x00}, {0xc4, 0x80}, {0xc5, 0x90}, {0xc6, 0x3b},
> + {0xc7, 0x46},
> + /* BLK */
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {0x40, 0x42}, {0x41, 0x00}, {0x43, 0x5b}, {0x5e, 0x00},
> + {0x5f, 0x00}, {0x60, 0x00}, {0x61, 0x00}, {0x62, 0x00},
> + {0x63, 0x00}, {0x64, 0x00}, {0x65, 0x00}, {0x66, 0x20},
> + {0x67, 0x20}, {0x68, 0x20}, {0x69, 0x20}, {0x76, 0x00},
> + {0x6a, 0x08}, {0x6b, 0x08}, {0x6c, 0x08}, {0x6d, 0x08},
> + {0x6e, 0x08}, {0x6f, 0x08}, {0x70, 0x08}, {0x71, 0x08},
> + {0x76, 0x00}, {0x72, 0xf0}, {0x7e, 0x3c}, {0x7f, 0x00},
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x48, 0x15}, {0x49, 0x00}, {0x4b, 0x0b},
> + /* AEC */
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {0x03, 0x04}, {0x04, 0xe2},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0x01, 0x04}, {0x02, 0xc0}, {0x03, 0x04}, {0x04, 0x90},
> + {0x05, 0x30}, {0x06, 0x90}, {0x07, 0x30}, {0x08, 0x80},
> + {0x09, 0x00}, {0x0a, 0x82}, {0x0b, 0x11}, {0x0c, 0x10},
> + {0x11, 0x10}, {0x13, 0x7b}, {0x17, 0x00}, {0x1c, 0x11},
> + {0x1e, 0x61}, {0x1f, 0x35}, {0x20, 0x40}, {0x22, 0x40},
> + {0x23, 0x20},
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x0f, 0x04},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0x12, 0x35}, {0x15, 0xb0}, {0x10, 0x31}, {0x3e, 0x28},
> + {0x3f, 0xb0}, {0x40, 0x90}, {0x41, 0x0f},
> + /* INTPEE */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x90, 0x6c}, {0x91, 0x03}, {0x92, 0xcb}, {0x94, 0x33},
> + {0x95, 0x84}, {0x97, 0x65}, {0xa2, 0x11},
> + /* DNDD */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x80, 0xc1}, {0x81, 0x08}, {0x82, 0x05}, {0x83, 0x08},
> + {0x84, 0x0a}, {0x86, 0xf0}, {0x87, 0x50}, {0x88, 0x15},
> + {0x89, 0xb0}, {0x8a, 0x30}, {0x8b, 0x10},
> + /* ASDE */
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0x21, 0x04},
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0xa3, 0x50}, {0xa4, 0x20}, {0xa5, 0x40}, {0xa6, 0x80},
> + {0xab, 0x40}, {0xae, 0x0c}, {0xb3, 0x46}, {0xb4, 0x64},
> + {0xb6, 0x38}, {0xb7, 0x01}, {0xb9, 0x2b}, {0x3c, 0x04},
> + {0x3d, 0x15}, {0x4b, 0x06}, {0x4c, 0x20},
> + /* Gamma */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x10, 0x09}, {0x11, 0x0d}, {0x12, 0x13}, {0x13, 0x19},
> + {0x14, 0x27}, {0x15, 0x37}, {0x16, 0x45}, {0x17, 0x53},
> + {0x18, 0x69}, {0x19, 0x7d}, {0x1a, 0x8f}, {0x1b, 0x9d},
> + {0x1c, 0xa9}, {0x1d, 0xbd}, {0x1e, 0xcd}, {0x1f, 0xd9},
> + {0x20, 0xe3}, {0x21, 0xea}, {0x22, 0xef}, {0x23, 0xf5},
> + {0x24, 0xf9}, {0x25, 0xff},
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {0xc6, 0x20}, {0xc7, 0x2b},
> + /* Gamma 2 */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x26, 0x0f}, {0x27, 0x14}, {0x28, 0x19}, {0x29, 0x1e},
> + {0x2a, 0x27}, {0x2b, 0x33}, {0x2c, 0x3b}, {0x2d, 0x45},
> + {0x2e, 0x59}, {0x2f, 0x69}, {0x30, 0x7c}, {0x31, 0x89},
> + {0x32, 0x98}, {0x33, 0xae}, {0x34, 0xc0}, {0x35, 0xcf},
> + {0x36, 0xda}, {0x37, 0xe2}, {0x38, 0xe9}, {0x39, 0xf3},
> + {0x3a, 0xf9}, {0x3b, 0xff},
> + /* YCP */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0xd1, 0x32}, {0xd2, 0x32}, {0xd3, 0x40}, {0xd6, 0xf0},
> + {0xd7, 0x10}, {0xd8, 0xda}, {0xdd, 0x14}, {0xde, 0x86},
> + {0xed, 0x80}, {0xee, 0x00}, {0xef, 0x3f}, {0xd8, 0xd8},
> + /* ABS */
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0x9f, 0x40},
> + /* LSC */
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0xc2, 0x14}, {0xc3, 0x0d}, {0xc4, 0x0c}, {0xc8, 0x15},
> + {0xc9, 0x0d}, {0xca, 0x0a}, {0xbc, 0x24}, {0xbd, 0x10},
> + {0xbe, 0x0b}, {0xb6, 0x25}, {0xb7, 0x16}, {0xb8, 0x15},
> + {0xc5, 0x00}, {0xc6, 0x00}, {0xc7, 0x00}, {0xcb, 0x00},
> + {0xcc, 0x00}, {0xcd, 0x00}, {0xbf, 0x07}, {0xc0, 0x00},
> + {0xc1, 0x00}, {0xb9, 0x00}, {0xba, 0x00}, {0xbb, 0x00},
> + {0xaa, 0x01}, {0xab, 0x01}, {0xac, 0x00}, {0xad, 0x05},
> + {0xae, 0x06}, {0xaf, 0x0e}, {0xb0, 0x0b}, {0xb1, 0x07},
> + {0xb2, 0x06}, {0xb3, 0x17}, {0xb4, 0x0e}, {0xb5, 0x0e},
> + {0xd0, 0x09}, {0xd1, 0x00}, {0xd2, 0x00}, {0xd6, 0x08},
> + {0xd7, 0x00}, {0xd8, 0x00}, {0xd9, 0x00}, {0xda, 0x00},
> + {0xdb, 0x00}, {0xd3, 0x0a}, {0xd4, 0x00}, {0xd5, 0x00},
> + {0xa4, 0x00}, {0xa5, 0x00}, {0xa6, 0x77}, {0xa7, 0x77},
> + {0xa8, 0x77}, {0xa9, 0x77}, {0xa1, 0x80}, {0xa2, 0x80},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0xdf, 0x0d}, {0xdc, 0x25}, {0xdd, 0x30}, {0xe0, 0x77},
> + {0xe1, 0x80}, {0xe2, 0x77}, {0xe3, 0x90}, {0xe6, 0x90},
> + {0xe7, 0xa0}, {0xe8, 0x90}, {0xe9, 0xa0},
> + /* AWB */
> + /* measure window */
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {0xec, 0x06}, {0xed, 0x04}, {0xee, 0x60}, {0xef, 0x90},
> + {0xb6, 0x01},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0x4f, 0x00}, {0x4f, 0x00}, {0x4b, 0x01}, {0x4f, 0x00},
> + {0x4c, 0x01}, {0x4d, 0x71}, {0x4e, 0x01},
> + {0x4c, 0x01}, {0x4d, 0x91}, {0x4e, 0x01},
> + {0x4c, 0x01}, {0x4d, 0x70}, {0x4e, 0x01},
> + {0x4c, 0x01}, {0x4d, 0x90}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0xb0}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0x8f}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0x6f}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0xaf}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0xd0}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0xf0}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0xcf}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0xef}, {0x4e, 0x02},
> + {0x4c, 0x01}, {0x4d, 0x6e}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x8e}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xae}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xce}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x4d}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x6d}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x8d}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xad}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xcd}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x4c}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x6c}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x8c}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xac}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xcc}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xcb}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x4b}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x6b}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x8b}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0xab}, {0x4e, 0x03},
> + {0x4c, 0x01}, {0x4d, 0x8a}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0xaa}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0xca}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0xca}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0xc9}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0x8a}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0x89}, {0x4e, 0x04},
> + {0x4c, 0x01}, {0x4d, 0xa9}, {0x4e, 0x04},
> + {0x4c, 0x02}, {0x4d, 0x0b}, {0x4e, 0x05},
> + {0x4c, 0x02}, {0x4d, 0x0a}, {0x4e, 0x05},
> + {0x4c, 0x01}, {0x4d, 0xeb}, {0x4e, 0x05},
> + {0x4c, 0x01}, {0x4d, 0xea}, {0x4e, 0x05},
> + {0x4c, 0x02}, {0x4d, 0x09}, {0x4e, 0x05},
> + {0x4c, 0x02}, {0x4d, 0x29}, {0x4e, 0x05},
> + {0x4c, 0x02}, {0x4d, 0x2a}, {0x4e, 0x05},
> + {0x4c, 0x02}, {0x4d, 0x4a}, {0x4e, 0x05},
> + {0x4c, 0x02}, {0x4d, 0x8a}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0x49}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0x69}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0x89}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0xa9}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0x48}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0x68}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0x69}, {0x4e, 0x06},
> + {0x4c, 0x02}, {0x4d, 0xca}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xc9}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xe9}, {0x4e, 0x07},
> + {0x4c, 0x03}, {0x4d, 0x09}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xc8}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xe8}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xa7}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xc7}, {0x4e, 0x07},
> + {0x4c, 0x02}, {0x4d, 0xe7}, {0x4e, 0x07},
> + {0x4c, 0x03}, {0x4d, 0x07}, {0x4e, 0x07},
> + {0x4f, 0x01},
> + {0x50, 0x80}, {0x51, 0xa8}, {0x52, 0x47}, {0x53, 0x38},
> + {0x54, 0xc7}, {0x56, 0x0e}, {0x58, 0x08}, {0x5b, 0x00},
> + {0x5c, 0x74}, {0x5d, 0x8b}, {0x61, 0xdb}, {0x62, 0xb8},
> + {0x63, 0x86}, {0x64, 0xc0}, {0x65, 0x04}, {0x67, 0xa8},
> + {0x68, 0xb0}, {0x69, 0x00}, {0x6a, 0xa8}, {0x6b, 0xb0},
> + {0x6c, 0xaf}, {0x6d, 0x8b}, {0x6e, 0x50}, {0x6f, 0x18},
> + {0x73, 0xf0}, {0x70, 0x0d}, {0x71, 0x60}, {0x72, 0x80},
> + {0x74, 0x01}, {0x75, 0x01}, {0x7f, 0x0c}, {0x76, 0x70},
> + {0x77, 0x58}, {0x78, 0xa0}, {0x79, 0x5e}, {0x7a, 0x54},
> + {0x7b, 0x58},
> + /* CC */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0xc0, 0x01}, {0xc1, 0x44}, {0xc2, 0xfd}, {0xc3, 0x04},
> + {0xc4, 0xf0}, {0xc5, 0x48}, {0xc6, 0xfd}, {0xc7, 0x46},
> + {0xc8, 0xfd}, {0xc9, 0x02}, {0xca, 0xe0}, {0xcb, 0x45},
> + {0xcc, 0xec}, {0xcd, 0x48}, {0xce, 0xf0}, {0xcf, 0xf0},
> + {0xe3, 0x0c}, {0xe4, 0x4b}, {0xe5, 0xe0},
> + /* ABS */
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + {0x9f, 0x40},
> + /* Dark sun */
> + {GC2145_REG_PAGE_SELECT, 0x02},
> + {0x40, 0xbf}, {0x46, 0xcf},
> +};
> +
> +#define GC2145_640_480_PIXELRATE (60 * HZ_PER_MHZ)
> +static const struct gc2145_reg mode_640_480_regs[] = {
> + {GC2145_REG_PAGE_SELECT, 0xf0}, {GC2145_REG_PAGE_SELECT, 0xf0},
> + {GC2145_REG_PAGE_SELECT, 0xf0}, {0xfc, 0x06},
> + {0xf6, 0x00}, {0xf7, 0x1d}, {0xf8, 0x86}, {0xfa, 0x00},
> + {0xf9, 0x8e},
> + /* Disable PAD IO */
> + {GC2145_REG_PAD_IO, 0x00},
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + /* Row/Col start - 0/0 */
> + {GC2145_REG_ROW_START_HIGH, 0x00}, {GC2145_REG_ROW_START_LOW, 0x00},
> + {GC2145_REG_COL_START_HIGH, 0x00}, {GC2145_REG_COL_START_LOW, 0x00},
> + /* Window size 1216/1618 */
> + {GC2145_REG_WIN_HEIGHT_HIGH, 0x04}, {GC2145_REG_WIN_HEIGHT_LOW, 0xc0},
> + {GC2145_REG_WIN_WIDTH_HIGH, 0x06}, {GC2145_REG_WIN_WIDTH_LOW, 0x52},
> + /* Scalar more */
> + {0xfd, 0x01}, {0xfa, 0x00},
> + /* Crop 640-480@0-0 */
> + {GC2145_REG_CROP_ENABLE, 0x01},
> + {GC2145_REG_CROP_Y_HIGH, 0x00}, {GC2145_REG_CROP_Y_LOW, 0x00},
> + {GC2145_REG_CROP_X_HIGH, 0x00}, {GC2145_REG_CROP_X_LOW, 0x00},
> + {GC2145_REG_CROP_HEIGHT_HIGH, 0x01}, {GC2145_REG_CROP_HEIGHT_LOW, 0xe0},
> + {GC2145_REG_CROP_WIDTH_HIGH, 0x02}, {GC2145_REG_CROP_WIDTH_LOW, 0x80},
> + /* Subsampling configuration */
> + {0x99, 0x55}, {0x9a, 0x06}, {0x9b, 0x01},
> + {0x9c, 0x23}, {0x9d, 0x00}, {0x9e, 0x00}, {0x9f, 0x01},
> + {0xa0, 0x23}, {0xa1, 0x00}, {0xa2, 0x00},
> + /* Framerate */
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {GC2145_REG_HBLANK_HIGH, 0x01}, {GC2145_REG_HBLANK_LOW, 0x30},
> + {GC2145_REG_VBLANK_HIGH, 0x00}, {GC2145_REG_VBLANK_LOW, 0x0c},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + /* AEC anti-flicker */
> + {0x25, 0x01}, {0x26, 0x75},
> + /* AEC exposure level 1-5 */
> + {0x27, 0x04}, {0x28, 0x5f}, {0x29, 0x04}, {0x2a, 0x5f},
> + {0x2b, 0x04}, {0x2c, 0x5f}, {0x2d, 0x04}, {0x2e, 0x5f},
> +};
> +
> +#define GC2145_1280_720_PIXELRATE (96 * HZ_PER_MHZ)
> +static const struct gc2145_reg mode_1280_720_regs[] = {
> + {GC2145_REG_PAGE_SELECT, 0xf0}, {GC2145_REG_PAGE_SELECT, 0xf0},
> + {GC2145_REG_PAGE_SELECT, 0xf0}, {0xfc, 0x06},
> + {0xf6, 0x00}, {0xf7, 0x1d}, {0xf8, 0x83}, {0xfa, 0x00},
> + {0xf9, 0x8e},
> + /* Disable PAD IO */
> + {GC2145_REG_PAD_IO, 0x00},
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + /* Row/Col start - 240/160 */
> + {GC2145_REG_ROW_START_HIGH, 0x00}, {GC2145_REG_ROW_START_LOW, 0xf0},
> + {GC2145_REG_COL_START_HIGH, 0x00}, {GC2145_REG_COL_START_LOW, 0xa0},
> + /* Window size 736/1296 */
> + {GC2145_REG_WIN_HEIGHT_HIGH, 0x02}, {GC2145_REG_WIN_HEIGHT_LOW, 0xe0},
> + {GC2145_REG_WIN_WIDTH_HIGH, 0x05}, {GC2145_REG_WIN_WIDTH_LOW, 0x10},
> + /* Crop 1280-720@0-0 */
> + {GC2145_REG_CROP_ENABLE, 0x01},
> + {GC2145_REG_CROP_Y_HIGH, 0x00}, {GC2145_REG_CROP_Y_LOW, 0x00},
> + {GC2145_REG_CROP_X_HIGH, 0x00}, {GC2145_REG_CROP_X_LOW, 0x00},
> + {GC2145_REG_CROP_HEIGHT_HIGH, 0x02}, {GC2145_REG_CROP_HEIGHT_LOW, 0xd0},
> + {GC2145_REG_CROP_WIDTH_HIGH, 0x05}, {GC2145_REG_CROP_WIDTH_LOW, 0x00},
> + /* Framerate */
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {GC2145_REG_HBLANK_HIGH, 0x01}, {GC2145_REG_HBLANK_LOW, 0x56},
> + {GC2145_REG_VBLANK_HIGH, 0x00}, {GC2145_REG_VBLANK_LOW, 0x11},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + /* AEC anti-flicker */
> + {0x25, 0x00}, {0x26, 0xe6},
> + /* AEC exposure level 1-5 */
> + {0x27, 0x02}, {0x28, 0xb2}, {0x29, 0x02}, {0x2a, 0xb2},
> + {0x2b, 0x02}, {0x2c, 0xb2}, {0x2d, 0x02}, {0x2e, 0xb2},
> +};
> +
> +#define GC2145_1600_1200_PIXELRATE (72 * HZ_PER_MHZ)
> +static const struct gc2145_reg mode_1600_1200_regs[] = {
> + {GC2145_REG_PAGE_SELECT, 0xf0}, {GC2145_REG_PAGE_SELECT, 0xf0},
> + {GC2145_REG_PAGE_SELECT, 0xf0}, {0xfc, 0x06},
> + {0xf6, 0x00}, {0xf7, 0x1d}, {0xf8, 0x84}, {0xfa, 0x00},
> + {0xf9, 0x8e},
> + /* Disable PAD IO */
> + {GC2145_REG_PAD_IO, 0x00},
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + /* Row/Col start - 0/0 */
> + {GC2145_REG_ROW_START_HIGH, 0x00}, {GC2145_REG_ROW_START_LOW, 0x00},
> + {GC2145_REG_COL_START_HIGH, 0x00}, {GC2145_REG_COL_START_LOW, 0x00},
> + /* Window size: 1216/1618 */
> + {GC2145_REG_WIN_HEIGHT_HIGH, 0x04}, {GC2145_REG_WIN_HEIGHT_LOW, 0xc0},
> + {GC2145_REG_WIN_WIDTH_HIGH, 0x06}, {GC2145_REG_WIN_WIDTH_LOW, 0x52},
> + /* Crop 1600-1200@0-0 */
> + {GC2145_REG_CROP_ENABLE, 0x01},
> + {GC2145_REG_CROP_Y_HIGH, 0x00}, {GC2145_REG_CROP_Y_LOW, 0x00},
> + {GC2145_REG_CROP_X_HIGH, 0x00}, {GC2145_REG_CROP_X_LOW, 0x00},
> + {GC2145_REG_CROP_HEIGHT_HIGH, 0x04}, {GC2145_REG_CROP_HEIGHT_LOW, 0xb0},
> + {GC2145_REG_CROP_WIDTH_HIGH, 0x06}, {GC2145_REG_CROP_WIDTH_LOW, 0x40},
> + /* Framerate */
> + {GC2145_REG_PAGE_SELECT, 0x00},
> + {GC2145_REG_HBLANK_HIGH, 0x01}, {GC2145_REG_HBLANK_LOW, 0x56},
> + {GC2145_REG_VBLANK_HIGH, 0x00}, {GC2145_REG_VBLANK_LOW, 0x10},
> + {GC2145_REG_PAGE_SELECT, 0x01},
> + /* AEC anti-flicker */
> + {0x25, 0x00}, {0x26, 0xfa},
> + /* AEC exposure level 1-5 */
> + {0x27, 0x04}, {0x28, 0xe2}, {0x29, 0x04}, {0x2a, 0xe2},
> + {0x2b, 0x04}, {0x2c, 0xe2}, {0x2d, 0x04}, {0x2e, 0xe2},
> +};
> +
> +/* Regulators supplies */
> +static const char * const gc2145_supply_name[] = {
> + "IOVDD", /* Digital I/O (1.7-3V) suppply */
> + "AVDD", /* Analog (2.7-3V) supply */
> + "DVDD", /* Digital Core (1.7-1.9V) supply */
> +};
> +
> +#define GC2145_NUM_SUPPLIES ARRAY_SIZE(gc2145_supply_name)
> +
> +/* Mode configs */
> +#define GC2145_MODE_640X480 0
> +#define GC2145_MODE_1280X720 1
> +#define GC2145_MODE_1600X1200 2
> +static const struct gc2145_mode supported_modes[] = {
> + {
> + /* 640x480 30fps mode */
> + .width = 640,
> + .height = 480,
> + .frame_interval = {
> + .numerator = 1,
> + .denominator = 30,
> + },
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_640_480_regs),
> + .regs = mode_640_480_regs,
> + },
> + .pixel_rate = GC2145_640_480_PIXELRATE,
> + .crop = {
> + .top = 0,
> + .left = 0,
> + .width = 640,
> + .height = 480,
> + },
> + .hblank = 0x130,
> + },
> + {
> + /* 1280x720 30fps mode */
> + .width = 1280,
> + .height = 720,
> + .frame_interval = {
> + .numerator = 1,
> + .denominator = 30,
> + },
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_1280_720_regs),
> + .regs = mode_1280_720_regs,
> + },
> + .pixel_rate = GC2145_1280_720_PIXELRATE,
> + .crop = {
> + .top = 160,
> + .left = 240,
> + .width = 1280,
> + .height = 720,
> + },
> + .hblank = 0x156,
> + },
> + {
> + /* 1600x1200 20fps mode */
> + .width = 1600,
> + .height = 1200,
> + .frame_interval = {
> + .numerator = 1,
> + .denominator = 20,
> + },
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(mode_1600_1200_regs),
> + .regs = mode_1600_1200_regs,
> + },
> + .pixel_rate = GC2145_1600_1200_PIXELRATE,
> + .crop = {
> + .top = 0,
> + .left = 0,
> + .width = 1600,
> + .height = 1200,
> + },
> + .hblank = 0x156,
> + },
> +};
> +
> +/**
> + * struct gc2145_format - GC2145 pixel format description
> + * @code: media bus (MBUS) associated code
> + * @colorspace: V4L2 colospace
> + * @datatype: MIPI CSI2 data type
> + * @output_fmt: GC2145 output format
> + * @row_col_switch: control of GC2145 row/col switch feature
> + */
> +struct gc2145_format {
> + unsigned int code;
> + unsigned int colorspace;
> + unsigned char datatype;
> + unsigned char output_fmt;
> + unsigned char row_col_switch;
Never initialized in the supported_formats[] list. Is this intentional ?
> +};
> +
> +/* All supported formats */
> +#define GC2145_DEFAULT_MBUS_FORMAT MEDIA_BUS_FMT_YUYV8_2X8
Seems like this is not used and you use MEDIA_BUS_FMT_RGB565_2X8_BE
in init_cfg()
> +static const struct gc2145_format supported_formats[] = {
> + {
> + .code = MEDIA_BUS_FMT_UYVY8_2X8,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .datatype = MIPI_CSI2_DT_YUV422_8B,
> + .output_fmt = 0x00,
> + },
> + {
> + .code = MEDIA_BUS_FMT_VYUY8_2X8,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .datatype = MIPI_CSI2_DT_YUV422_8B,
> + .output_fmt = 0x01,
> + },
> + {
> + .code = MEDIA_BUS_FMT_YUYV8_2X8,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .datatype = MIPI_CSI2_DT_YUV422_8B,
> + .output_fmt = 0x02,
> + },
> + {
> + .code = MEDIA_BUS_FMT_YVYU8_2X8,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .datatype = MIPI_CSI2_DT_YUV422_8B,
> + .output_fmt = 0x03,
> + },
> + {
> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .datatype = MIPI_CSI2_DT_RGB565,
> + .output_fmt = 0x06,
> + },
> +};
> +
> +struct gc2145_ctrls {
> + struct v4l2_ctrl_handler handler;
> + struct v4l2_ctrl *pixel_rate;
> + struct v4l2_ctrl *hblank;
> + struct v4l2_ctrl *test_pattern;
> + struct v4l2_ctrl *hflip;
> + struct v4l2_ctrl *vflip;
> +};
> +
> +struct gc2145 {
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> +
> + struct clk *xclk;
> +
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *powerdown_gpio;
> + struct regulator_bulk_data supplies[GC2145_NUM_SUPPLIES];
> +
> + /* V4L2 controls */
> + struct gc2145_ctrls ctrls;
> +
> + /* Current mode */
> + const struct gc2145_mode *mode;
> +
> + /* Streaming on/off */
> + bool streaming;
> +};
> +
> +static inline struct gc2145 *to_gc2145(struct v4l2_subdev *_sd)
> +{
> + return container_of(_sd, struct gc2145, sd);
> +}
> +
> +static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
> +{
> + return &container_of(ctrl->handler, struct gc2145,
> + ctrls.handler)->sd;
> +}
> +
> +static int gc2145_read_reg(struct gc2145 *gc2145, u8 addr, u8 *data,
> + int data_size)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + struct i2c_msg msg[2];
> + int ret;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = client->flags;
> + msg[0].buf = &addr;
> + msg[0].len = 1;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = client->flags | I2C_M_RD;
> + msg[1].buf = data;
> + msg[1].len = data_size;
> +
> + ret = i2c_transfer(client->adapter, msg, 2);
> + if (ret < 0) {
> + dev_err(&client->dev, "%s: error %d: start_index=%x, data_size=%d\n",
> + __func__, ret, (u32)addr, data_size);
> + return ret;
> + }
> +
> + dev_dbg(&client->dev, "[rd %02x] => %*ph\n", (u32)addr, data_size,
> + data);
> +
> + return 0;
> +}
> +
> +static int gc2145_write_reg(struct gc2145 *gc2145, u8 addr, u8 data)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + struct i2c_msg msg;
> + u8 buf[] = { addr, data };
> + int ret;
> +
> + msg.addr = client->addr;
> + msg.flags = client->flags;
> + msg.buf = buf;
> + msg.len = sizeof(buf);
> +
> + dev_dbg(&client->dev, "[wr %02x] <= %02xh\n", (u32)addr, data);
> +
> + ret = i2c_transfer(client->adapter, &msg, 1);
> + if (ret < 0) {
> + dev_err(&client->dev, "%s: error %d: addr=%x, data=%02xh\n",
> + __func__, ret, (u32)addr, data);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int gc2145_mod_reg(struct gc2145 *gc2145, u16 reg, u8 mask, u8 val)
> +{
> + u8 readval;
> + int ret;
> +
> + ret = gc2145_read_reg(gc2145, reg, &readval, 1);
> + if (ret)
> + return ret;
> +
> + readval &= ~mask;
> + val &= mask;
> + val |= readval;
> +
> + return gc2145_write_reg(gc2145, reg, val);
> +}
> +
> +/* Write a list of registers */
> +static int gc2145_write_regs(struct gc2145 *gc2145,
> + const struct gc2145_reg *regs, u32 len)
> +{
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < len; i++) {
> + ret = gc2145_write_reg(gc2145, regs[i].address, regs[i].val);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
>From a quick look at the documentation, this sensor seems to be
compatible with MIPI CCI. The newly merged CCI helpers would provide
read/write functions for you. If possible I would suggest to use them
> +
> +static const struct gc2145_format *
> +gc2145_get_format_code(struct gc2145 *gc2145, u32 code)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(supported_formats); i++)
> + if (supported_formats[i].code == code)
> + break;
> +
> + if (i >= ARRAY_SIZE(supported_formats))
> + i = 0;
> +
> + return &supported_formats[i];
> +}
> +
> +static void gc2145_update_pad_format(struct gc2145 *gc2145,
> + const struct gc2145_mode *mode,
> + struct v4l2_mbus_framefmt *fmt, u32 code)
> +{
> + fmt->code = code;
> + fmt->width = mode->width;
> + fmt->height = mode->height;
> + fmt->field = V4L2_FIELD_NONE;
> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
> + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
> + fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, fmt->colorspace,
> + fmt->ycbcr_enc);
> + fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
> +}
> +
> +static int gc2145_init_cfg(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + struct v4l2_mbus_framefmt *format;
> + struct v4l2_rect *crop;
> +
> + /* Initialize try_fmt */
> + format = v4l2_subdev_get_pad_format(sd, state, 0);
> + gc2145_update_pad_format(gc2145,
> + &supported_modes[0], format,
> + MEDIA_BUS_FMT_RGB565_2X8_BE);
> +
> + /* Initialize crop rectangle. */
> + crop = v4l2_subdev_get_pad_crop(sd, state, 0);
> + *crop = supported_modes[0].crop;
> +
> + return 0;
> +}
> +
> +static int gc2145_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_selection *sel)
> +{
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP: {
> + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state, 0);
> + return 0;
> + }
Do you need {} ?
> +
> + case V4L2_SEL_TGT_NATIVE_SIZE:
> + sel->r.top = 0;
> + sel->r.left = 0;
> + sel->r.width = GC2145_NATIVE_WIDTH;
> + sel->r.height = GC2145_NATIVE_HEIGHT;
> +
> + return 0;
> +
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + sel->r.top = 0;
> + sel->r.left = 0;
> + sel->r.width = 1600;
> + sel->r.height = 1200;
> +
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int gc2145_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + if (code->index >= ARRAY_SIZE(supported_formats))
> + return -EINVAL;
> +
> + code->code = supported_formats[code->index].code;
> + return 0;
> +}
> +
> +static int gc2145_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + const struct gc2145_format *gc2145_format;
> + u32 code;
> +
> + if (fse->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + gc2145_format = gc2145_get_format_code(gc2145, fse->code);
> + code = gc2145_format->code;
> + if (fse->code != code)
> + return -EINVAL;
Is this correct ? Should you check if fse->code matches the currently
applied format code or should you just check if fse->code is supported
at al ? I recall the API doc was not super-clear about this, am I
wrong ?
> +
> + fse->min_width = supported_modes[fse->index].width;
> + fse->max_width = fse->min_width;
> + fse->min_height = supported_modes[fse->index].height;
> + fse->max_height = fse->min_height;
> +
> + return 0;
> +}
> +
> +static int
> +gc2145_enum_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_interval_enum *fie)
> +{
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + const struct gc2145_format *gc2145_format;
> + u32 code, i;
> +
> + /* Only supporting a unique framerate per resolution currently */
> + if (fie->index > 0)
> + return -EINVAL;
> +
> + gc2145_format = gc2145_get_format_code(gc2145, fie->code);
> + code = gc2145_format->code;
> + if (fie->code != code)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(supported_modes); i++)
> + if (supported_modes[i].width == fie->width &&
> + supported_modes[i].height == fie->height)
> + break;
> +
> + if (i >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + fie->interval.numerator = supported_modes[i].frame_interval.numerator;
> + fie->interval.denominator =
> + supported_modes[i].frame_interval.denominator;
> +
> + return 0;
> +}
> +
> +static int gc2145_set_pad_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + const struct gc2145_mode *mode;
> + const struct gc2145_format *gc2145_fmt;
> + struct v4l2_mbus_framefmt *framefmt;
> + struct gc2145_ctrls *ctrls = &gc2145->ctrls;
> + struct v4l2_rect *crop;
> +
> + gc2145_fmt = gc2145_get_format_code(gc2145, fmt->format.code);
> + mode = v4l2_find_nearest_size(supported_modes,
> + ARRAY_SIZE(supported_modes),
> + width, height,
> + fmt->format.width, fmt->format.height);
> +
> + /* In RAW mode, VGA is not possible so use 720p instead */
> + if (gc2145_fmt->colorspace == V4L2_COLORSPACE_RAW &&
> + mode == &supported_modes[GC2145_MODE_640X480])
> + mode = &supported_modes[GC2145_MODE_1280X720];
> +
> + gc2145_update_pad_format(gc2145, mode, &fmt->format, gc2145_fmt->code);
> + framefmt = v4l2_subdev_get_pad_format(sd, sd_state, fmt->pad);
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
> + (gc2145->mode != mode || framefmt->code != fmt->format.code)) {
For consistency see 6cb042908b3e ("media: i2c: imx219: Perform a full
mode set unconditionally")
> + gc2145->mode = mode;
> + /* Update pixel_rate based on the mode */
> + __v4l2_ctrl_s_ctrl_int64(ctrls->pixel_rate, mode->pixel_rate);
> + /* Update hblank based on the mode */
> + __v4l2_ctrl_modify_range(ctrls->hblank, mode->hblank,
> + mode->hblank, 1, mode->hblank);
> + __v4l2_ctrl_s_ctrl(ctrls->hblank, mode->hblank);
> + }
> + *framefmt = fmt->format;
> + crop = v4l2_subdev_get_pad_crop(sd, sd_state, fmt->pad);
> + *crop = mode->crop;
> +
> + return 0;
> +}
> +
> +static const struct gc2145_reg common_mipi_regs[] = {
> + {GC2145_REG_PAGE_SELECT, 0x03},
> + {0x01, 0x87}, {0x02, 0x22}, {0x03, 0x10},
> + {0x06, 0x88}, {0x10, 0x95}, {0x15, 0x10}, {0x22, 0x04},
> + {0x23, 0x10}, {0x24, 0x10}, {0x25, 0x10}, {0x26, 0x05},
> + {0x21, 0x10}, {0x29, 0x03}, {0x2a, 0x0a}, {0x2b, 0x06},
I supposed these are undocumented in your documentation as well, right ?
> +};
> +
> +static int gc2145_config_mipi_mode(struct gc2145 *gc2145,
> + const struct gc2145_format *gc2145_format)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + u16 lwc, fifo_full_lvl, fifo_gate_mode;
> + int ret;
> +
> + /* Common MIPI settings */
> + ret = gc2145_write_regs(gc2145, common_mipi_regs,
> + ARRAY_SIZE(common_mipi_regs));
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set mode - MIPI\n",
> + __func__);
> + return ret;
> + }
> +
> + /*
> + * Adjust the MIPI buffer settings.
> + * For YUV/RGB, LWC = image width * 2
> + * For RAW8, LWC = image width
> + * For RAW10, LWC = image width * 1.25
The multiplication factor is then just bpp expressed in bytes, right ?
> + */
> + if (gc2145_format->colorspace != V4L2_COLORSPACE_RAW)
> + lwc = gc2145->mode->width * 2;
> + else if (gc2145_format->code == MEDIA_BUS_FMT_SGRBG8_1X8 ||
> + gc2145_format->code == MEDIA_BUS_FMT_SRGGB8_1X8 ||
> + gc2145_format->code == MEDIA_BUS_FMT_SBGGR8_1X8 ||
> + gc2145_format->code == MEDIA_BUS_FMT_SGBRG8_1X8)
> + lwc = gc2145->mode->width * 1;
The driver does not support RAW formats, right ?
> + else
> + lwc = gc2145->mode->width + (gc2145->mode->width / 4);
> +
> + ret = gc2145_write_reg(gc2145, GC2145_REG_LWC_HIGH, lwc >> 8);
> + if (ret)
> + return ret;
> + ret = gc2145_write_reg(gc2145, GC2145_REG_LWC_LOW, (lwc & 0xff));
> + if (ret)
> + return ret;
> +
> + /*
> + * Adjust the MIPI Fifo Full Level
> + * 640x480 RGB: 0x0190
> + * 1280x720 / 1600x1200 (aka no scaler) non RAW: 0x0001
> + * 1600x1200 RAW: 0x0190
> + */
> + if (gc2145_format->colorspace != V4L2_COLORSPACE_RAW) {
Ditto, no RAW formats are supported. This applies to all other uses of
V4L2_COLORSPACE_RAW in the driver
> + if (gc2145->mode->width == 1280 || gc2145->mode->width == 1600)
> + fifo_full_lvl = 0x0001;
> + else
> + fifo_full_lvl = 0x0190;
> + } else {
> + fifo_full_lvl = 0x0190;
> + }
> +
> + ret = gc2145_write_reg(gc2145, GC2145_REG_FIFO_FULL_LVL_HIGH,
> + fifo_full_lvl >> 8);
> + if (ret)
> + return ret;
> + ret = gc2145_write_reg(gc2145, GC2145_REG_FIFO_FULL_LVL_LOW,
> + fifo_full_lvl & 0xff);
> + if (ret)
> + return ret;
> +
> + /* Set the fifo gate mode / MIPI wdiv set */
> + if (gc2145_format->colorspace == V4L2_COLORSPACE_RAW)
> + fifo_gate_mode = 0xf1;
> + else
> + fifo_gate_mode = 0xf0;
> + ret = gc2145_write_reg(gc2145, GC2145_REG_FIFO_GATE_MODE,
> + fifo_gate_mode);
> + if (ret)
> + return ret;
> +
> + /* Set the MIPI data type */
> + ret = gc2145_write_reg(gc2145, GC2145_REG_MIPI_DT,
> + gc2145_format->datatype);
> + return ret;
> +}
> +
> +static int gc2145_start_streaming(struct gc2145 *gc2145,
> + struct v4l2_subdev_state *state)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + const struct gc2145_format *gc2145_format;
> + struct v4l2_mbus_framefmt *fmt;
> + u8 sync_mode;
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(&client->dev);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * TODO - configuration of the sensor will have to be moved into
> + * gc2145_set_pad_format once we have a way to configure without
> + * starting the sensor. In such case, only streaming start will
> + * be done here.
> + */
Are you sure ? It seems correct to me to update the sensor
configuration before starting the streaming, isn't it ?
> +
> + /* Apply default values of current mode */
> + ret = gc2145_write_regs(gc2145, gc2145->mode->reg_list.regs,
> + gc2145->mode->reg_list.num_of_regs);
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set mode - pre ISP\n",
> + __func__);
> + goto err_rpm_put;
> + }
> +
> + ret = gc2145_write_regs(gc2145, common_regs,
> + ARRAY_SIZE(common_regs));
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set common regs\n",
> + __func__);
> + goto err_rpm_put;
> + }
> +
> + fmt = v4l2_subdev_get_pad_format(&gc2145->sd, state, 0);
> + gc2145_format = gc2145_get_format_code(gc2145, fmt->code);
> +
> + /* Set the output format */
> + ret = gc2145_write_reg(gc2145, GC2145_REG_PAGE_SELECT, 0x00);
> + if (ret)
> + return ret;
> +
> + ret = gc2145_write_reg(gc2145, GC2145_REG_OUTPUT_FMT,
> + gc2145_format->output_fmt);
> + if (ret)
> + return ret;
> +
> + ret = gc2145_read_reg(gc2145, GC2145_REG_SYNC_MODE, &sync_mode, 1);
> + if (ret)
> + return ret;
> +
> + sync_mode &= ~(GC2145_SYNC_MODE_COL_SWITCH |
> + GC2145_SYNC_MODE_ROW_SWITCH);
> + sync_mode |= gc2145_format->row_col_switch;
As said, this is fixed to 0. Did you get what COL/ROW_SWITCH is meant
to be used for ?
> +
> + ret = gc2145_write_reg(gc2145, GC2145_REG_SYNC_MODE, sync_mode);
> + if (ret)
> + return ret;
> +
> + /* Perform MIPI specific configuration */
> + ret = gc2145_config_mipi_mode(gc2145, gc2145_format);
> + if (ret)
> + return ret;
> +
> + /* Come back on page 0 by default */
> + ret = gc2145_write_reg(gc2145, GC2145_REG_PAGE_SELECT, 0x00);
> + if (ret)
> + return ret;
> +
> + /* Apply customized values from user */
> + ret = __v4l2_ctrl_handler_setup(&gc2145->ctrls.handler);
> + if (ret)
> + goto err_rpm_put;
> +
> + return 0;
> +
> +err_rpm_put:
> + pm_runtime_put(&client->dev);
> + return ret;
> +}
> +
> +static void gc2145_stop_streaming(struct gc2145 *gc2145)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> +
> + /*
> + * TODO - once we have a way to turn off only streaming of the
> + * sensor, we will have to do it here.
Right, what register write "starts" the streaming ?
> + */
> +
> + pm_runtime_put(&client->dev);
> +}
> +
> +static int gc2145_g_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_frame_interval *fi)
> +{
> + struct gc2145 *gc2145 = to_gc2145(sd);
> +
> + fi->interval = gc2145->mode->frame_interval;
> +
> + return 0;
> +}
> +
> +static int gc2145_set_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + struct v4l2_subdev_state *state;
> + int ret = 0;
> +
> + state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + if (gc2145->streaming == enable)
> + goto unlock;
> +
> + if (enable) {
> + /*
> + * Apply default & customized values
> + * and then start streaming.
> + */
> + ret = gc2145_start_streaming(gc2145, state);
> + if (ret)
> + goto unlock;
> + } else {
> + gc2145_stop_streaming(gc2145);
> + }
> +
> + gc2145->streaming = enable;
> +
> +unlock:
> + v4l2_subdev_unlock_state(state);
> +
> + return ret;
> +}
> +
> +/* Power/clock management functions */
> +static int gc2145_power_on(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(GC2145_NUM_SUPPLIES, gc2145->supplies);
> + if (ret) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(gc2145->xclk);
> + if (ret) {
> + dev_err(dev, "failed to enable clock\n");
> + goto reg_off;
> + }
> +
> + /* TODO - Following delays should be tuned */
> + usleep_range(10000, 12000);
wow, 10 to 12 msec ?
The datasheet version I have reports "TBD" as suggested delays :/
> + gpiod_set_value_cansleep(gc2145->powerdown_gpio, 0);
> + usleep_range(10000, 12000);
> + gpiod_set_value_cansleep(gc2145->reset_gpio, 0);
> + usleep_range(40000, 50000);
> +
> + return 0;
> +
> +reg_off:
> + regulator_bulk_disable(GC2145_NUM_SUPPLIES, gc2145->supplies);
> +
> + return ret;
> +}
> +
> +static int gc2145_power_off(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct gc2145 *gc2145 = to_gc2145(sd);
> +
> + gpiod_set_value_cansleep(gc2145->powerdown_gpio, 1);
> + gpiod_set_value_cansleep(gc2145->reset_gpio, 1);
> + regulator_bulk_disable(GC2145_NUM_SUPPLIES, gc2145->supplies);
> + clk_disable_unprepare(gc2145->xclk);
7.2.2 Power Off Sequence shows clock being stopped before disabling
regulators.
> +
> + return 0;
> +}
> +
> +static int gc2145_suspend(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct gc2145 *gc2145 = to_gc2145(sd);
> +
> + if (gc2145->streaming)
> + gc2145_stop_streaming(gc2145);
> +
> + return 0;
> +}
> +
> +static int gc2145_resume(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + struct v4l2_subdev_state *state;
> + int ret;
> +
> + if (gc2145->streaming) {
> + state = v4l2_subdev_lock_and_get_active_state(sd);
> + ret = gc2145_start_streaming(gc2145, state);
> + v4l2_subdev_unlock_state(state);
> + if (ret)
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + gc2145_stop_streaming(gc2145);
> + gc2145->streaming = false;
> +
> + return ret;
> +}
> +
> +static int gc2145_get_regulators(struct gc2145 *gc2145)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + unsigned int i;
> +
> + for (i = 0; i < GC2145_NUM_SUPPLIES; i++)
> + gc2145->supplies[i].supply = gc2145_supply_name[i];
> +
> + return devm_regulator_bulk_get(&client->dev, GC2145_NUM_SUPPLIES,
> + gc2145->supplies);
> +}
> +
> +/* Verify chip ID */
> +static int gc2145_identify_module(struct gc2145 *gc2145)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + int ret;
> + u16 chip_id;
> +
> + ret = gc2145_read_reg(gc2145, GC2145_REG_CHIP_ID, (u8 *)&chip_id, 2);
> + if (ret) {
> + dev_err(&client->dev, "failed to read chip id (%d)\n", ret);
> + return ret;
> + }
> + chip_id = be16_to_cpu(chip_id);
> +
> + if (chip_id != GC2145_CHIP_ID) {
> + dev_err(&client->dev, "chip id mismatch: %x!=%x\n",
> + GC2145_CHIP_ID, chip_id);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static const char * const test_pattern_menu[] = {
> + "Disabled",
> + "Colored patterns",
> + "Uniform white",
> + "Uniform yellow",
> + "Uniform cyan",
> + "Uniform green",
> + "Uniform magenta",
> + "Uniform red",
> + "Uniform black",
> +};
> +
> +#define GC2145_TEST_PATTERN_ENABLE BIT(0)
> +#define GC2145_TEST_PATTERN_UXGA BIT(3)
> +
> +#define GC2145_TEST_UNIFORM BIT(3)
> +#define GC2145_TEST_WHITE (4 << 4)
> +#define GC2145_TEST_YELLOW (8 << 4)
> +#define GC2145_TEST_CYAN (9 << 4)
> +#define GC2145_TEST_GREEN (6 << 4)
> +#define GC2145_TEST_MAGENTA (10 << 4)
> +#define GC2145_TEST_RED (5 << 4)
> +#define GC2145_TEST_BLACK (0)
> +
> +static const u8 test_pattern_val[] = {
> + 0,
> + GC2145_TEST_PATTERN_ENABLE,
> + GC2145_TEST_UNIFORM | GC2145_TEST_WHITE,
> + GC2145_TEST_UNIFORM | GC2145_TEST_YELLOW,
> + GC2145_TEST_UNIFORM | GC2145_TEST_CYAN,
> + GC2145_TEST_UNIFORM | GC2145_TEST_GREEN,
> + GC2145_TEST_UNIFORM | GC2145_TEST_MAGENTA,
> + GC2145_TEST_UNIFORM | GC2145_TEST_RED,
> + GC2145_TEST_UNIFORM | GC2145_TEST_BLACK,
> +};
> +
> +static const struct v4l2_subdev_core_ops gc2145_core_ops = {
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> +static const struct v4l2_subdev_video_ops gc2145_video_ops = {
> + .g_frame_interval = gc2145_g_frame_interval,
> + /*
> + * Currently frame_interval can't be changed hence s_frame_interval
> + * does same as g_frame_interval
> + */
> + .s_frame_interval = gc2145_g_frame_interval,
> + .s_stream = gc2145_set_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops gc2145_pad_ops = {
> + .init_cfg = gc2145_init_cfg,
> + .enum_mbus_code = gc2145_enum_mbus_code,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = gc2145_set_pad_format,
> + .get_selection = gc2145_get_selection,
> + .enum_frame_size = gc2145_enum_frame_size,
> + .enum_frame_interval = gc2145_enum_frame_interval,
> +};
> +
> +static const struct v4l2_subdev_ops gc2145_subdev_ops = {
> + .core = &gc2145_core_ops,
> + .video = &gc2145_video_ops,
> + .pad = &gc2145_pad_ops,
> +};
> +
> +static int gc2145_set_ctrl_test_pattern(struct gc2145 *gc2145, int value)
> +{
> + int ret;
> +
> + if (!value) {
> + /* Disable test pattern */
> + ret = gc2145_write_reg(gc2145, GC2145_REG_DEBUG_MODE2, 0);
> + if (ret)
> + return ret;
> +
> + return gc2145_write_reg(gc2145, GC2145_REG_DEBUG_MODE3, 0);
> + }
> +
> + /* Enable test pattern, colored or uniform */
> + ret = gc2145_write_reg(gc2145, GC2145_REG_DEBUG_MODE2,
> + GC2145_TEST_PATTERN_ENABLE |
> + GC2145_TEST_PATTERN_UXGA);
> + if (ret)
> + return ret;
> +
> + if (!(test_pattern_val[value] & GC2145_TEST_UNIFORM))
> + return gc2145_write_reg(gc2145, GC2145_REG_DEBUG_MODE3, 0);
> +
> + /* Uniform */
> + return gc2145_write_reg(gc2145, GC2145_REG_DEBUG_MODE3,
> + test_pattern_val[value]);
> +}
> +
> +static int gc2145_set_ctrl_hflip(struct gc2145 *gc2145, int value)
> +{
> + return gc2145_mod_reg(gc2145, GC2145_REG_ANALOG_MODE1,
> + BIT(0), (value ? BIT(0) : 0));
> +}
> +
> +static int gc2145_set_ctrl_vflip(struct gc2145 *gc2145, int value)
> +{
> + return gc2145_mod_reg(gc2145, GC2145_REG_ANALOG_MODE1,
> + BIT(1), (value ? BIT(1) : 0));
> +}
> +
> +static int gc2145_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + struct gc2145 *gc2145 = to_gc2145(sd);
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(&client->dev);
You shouldn't resume the sensor when setting controls but rather check
if the sensor is powered up with:
if (pm_runtime_get_if_in_use(&client->dev) == 0)
return 0;
and as you call __v4l2_ctrl_handler_setup() already at s_stream(1)
time, controls will be applied before streaming start
> + if (ret < 0)
> + return ret;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_TEST_PATTERN:
> + ret = gc2145_set_ctrl_test_pattern(gc2145, ctrl->val);
> + break;
> + case V4L2_CID_HFLIP:
> + ret = gc2145_set_ctrl_hflip(gc2145, ctrl->val);
> + break;
> + case V4L2_CID_VFLIP:
> + ret = gc2145_set_ctrl_vflip(gc2145, ctrl->val);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + pm_runtime_put(&client->dev);
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops gc2145_ctrl_ops = {
> + .s_ctrl = gc2145_s_ctrl,
> +};
> +
> +/* Initialize control handlers */
> +static int gc2145_init_controls(struct gc2145 *gc2145)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
> + const struct v4l2_ctrl_ops *ops = &gc2145_ctrl_ops;
> + struct gc2145_ctrls *ctrls = &gc2145->ctrls;
> + struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> + struct v4l2_fwnode_device_properties props;
> + int ret;
> +
> + ret = v4l2_ctrl_handler_init(hdl, 12);
> + if (ret)
> + return ret;
> +
> + ctrls->pixel_rate = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE,
> + 0, INT_MAX, 1,
You know the supported pixel rates. Why use INT_MAX ?
> + supported_modes[0].pixel_rate);
> + ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
isn't PIXEL_RATE RO by default ?
> +
> + ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK,
> + supported_modes[0].hblank,
> + supported_modes[0].hblank, 1,
> + supported_modes[0].hblank);
Register P0:0x05 "HBlanking" is 12 bits long.
Is the max value BIT(13) - 1 ?
I have not been able to find a min value.
However registering hblank without vblank kind of defeats the purpose
of using the two controls to compute the frame duration..
> + ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + ctrls->test_pattern =
> + v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(test_pattern_menu) - 1,
> + 0, 0, test_pattern_menu);
> + ctrls->hflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP,
> + 0, 1, 1, 0);
> + ctrls->vflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP,
> + 0, 1, 1, 0);
> +
> + if (hdl->error) {
> + ret = hdl->error;
> + dev_err(&client->dev, "control init failed (%d)\n", ret);
> + goto error;
> + }
> +
> + ret = v4l2_fwnode_device_parse(&client->dev, &props);
> + if (ret)
> + goto error;
> +
> + ret = v4l2_ctrl_new_fwnode_properties(hdl, &gc2145_ctrl_ops,
> + &props);
> + if (ret)
> + goto error;
> +
> + gc2145->sd.ctrl_handler = hdl;
> +
> + return 0;
> +
> +error:
> + v4l2_ctrl_handler_free(hdl);
> +
> + return ret;
> +}
> +
> +static void gc2145_free_controls(struct gc2145 *gc2145)
> +{
> + v4l2_ctrl_handler_free(&gc2145->ctrls.handler);
One liners could be called directly maybe..
> +}
> +
> +static int gc2145_check_hwcfg(struct device *dev)
> +{
> + struct fwnode_handle *endpoint;
> + struct v4l2_fwnode_endpoint ep_cfg = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
> + int ret = -EINVAL;
> +
> + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> + if (!endpoint) {
> + dev_err(dev, "endpoint node not found\n");
> + return -EINVAL;
> + }
> +
> + if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg)) {
> + dev_err(dev, "could not parse endpoint\n");
> + goto error_out;
no need to v4l2_fwnode_endpoint_free() if
v4l2_fwnode_endpoint_alloc_parse() fails.
This could be
ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg);
fwnode_handle_put(endpoint);
if (ret)
return ret;
> + }
> +
> + /* Check the number of MIPI CSI2 data lanes */
> + if (ep_cfg.bus.mipi_csi2.num_data_lanes != 2) {
> + dev_err(dev, "only 2 data lanes are currently supported\n");
> + goto error_out;
> + }
> +
> + ret = 0;
> +
> +error_out:
> + v4l2_fwnode_endpoint_free(&ep_cfg);
> + fwnode_handle_put(endpoint);
> +
> + return ret;
> +}
> +
> +static int gc2145_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct gc2145 *gc2145;
> + int ret;
> + unsigned int xclk_freq;
You can easily declare this 2 lines up (I know, reverse-xmas-tree
nitpicking)
> +
> + gc2145 = devm_kzalloc(&client->dev, sizeof(*gc2145), GFP_KERNEL);
> + if (!gc2145)
> + return -ENOMEM;
> +
> + v4l2_i2c_subdev_init(&gc2145->sd, client, &gc2145_subdev_ops);
> +
> + /* Check the hardware configuration in device tree */
> + if (gc2145_check_hwcfg(dev))
> + return -EINVAL;
> +
> + /* Get system clock (xclk) */
> + gc2145->xclk = devm_clk_get(dev, NULL);
> + if (IS_ERR(gc2145->xclk)) {
> + dev_err(dev, "failed to get xclk\n");
> + return PTR_ERR(gc2145->xclk);
> + }
> +
> + xclk_freq = clk_get_rate(gc2145->xclk);
> + if (xclk_freq != GC2145_XCLK_FREQ) {
> + dev_err(dev, "xclk frequency not supported: %d Hz\n",
> + xclk_freq);
> + return -EINVAL;
> + }
> +
> + ret = gc2145_get_regulators(gc2145);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
Can dev_err_probe help here ?
> + return ret;
> + }
> +
> + /* Request optional enable pin */
> + gc2145->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> + GPIOD_OUT_HIGH);
> +
> + /* Request optional enable pin */
> + gc2145->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> + GPIOD_OUT_HIGH);
> +
> + /*
> + * The sensor must be powered for gc2145_identify_module()
> + * to be able to read the CHIP_ID register
> + */
> + ret = gc2145_power_on(dev);
> + if (ret)
> + return ret;
> +
> + ret = gc2145_identify_module(gc2145);
> + if (ret)
> + goto error_power_off;
> +
> + /* Set default mode */
> + gc2145->mode = &supported_modes[0];
> +
> + ret = gc2145_init_controls(gc2145);
> + if (ret)
> + goto error_power_off;
> +
> + /* Initialize subdev */
> + gc2145->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_HAS_EVENTS;
> + gc2145->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> + /* Initialize source pad */
> + gc2145->pad.flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&gc2145->sd.entity, 1, &gc2145->pad);
> + if (ret) {
> + dev_err(dev, "failed to init entity pads: %d\n", ret);
> + goto error_handler_free;
> + }
> +
> + gc2145->sd.state_lock = gc2145->ctrls.handler.lock;
> + ret = v4l2_subdev_init_finalize(&gc2145->sd);
> + if (ret < 0) {
> + dev_err(dev, "subdev init error: %d\n", ret);
> + goto error_media_entity;
> + }
> +
> + ret = v4l2_async_register_subdev_sensor(&gc2145->sd);
> + if (ret < 0) {
> + dev_err(dev, "failed to register sensor sub-device: %d\n", ret);
> + goto error_subdev_cleanup;
> + }
> +
> + /* Enable runtime PM and turn off the device */
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_idle(dev);
> +
> + return 0;
> +
> +error_subdev_cleanup:
> + v4l2_subdev_cleanup(&gc2145->sd);
> +
> +error_media_entity:
> + media_entity_cleanup(&gc2145->sd.entity);
> +
> +error_handler_free:
> + gc2145_free_controls(gc2145);
> +
> +error_power_off:
> + gc2145_power_off(dev);
> +
> + return ret;
> +}
> +
> +static void gc2145_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct gc2145 *gc2145 = to_gc2145(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
Do you need to call
v4l2_subdev_cleanup(&gc2145->sd);
here as well ?
> + gc2145_free_controls(gc2145);
> +
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + gc2145_power_off(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +}
> +
> +static const struct of_device_id gc2145_dt_ids[] = {
> + { .compatible = "galaxycore,gc2145" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, gc2145_dt_ids);
> +
> +static const struct dev_pm_ops gc2145_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(gc2145_suspend, gc2145_resume)
You can drop these function and the "streaming" flag
Thanks
j
See: [PATCH 00/57] media: i2c: Reduce cargo cult
> + SET_RUNTIME_PM_OPS(gc2145_power_off, gc2145_power_on, NULL)
> +};
> +
> +static struct i2c_driver gc2145_i2c_driver = {
> + .driver = {
> + .name = "gc2145",
> + .of_match_table = gc2145_dt_ids,
> + .pm = pm_ptr(&gc2145_pm_ops),
> + },
> + .probe = gc2145_probe,
> + .remove = gc2145_remove,
> +};
> +
> +module_i2c_driver(gc2145_i2c_driver);
> +
> +MODULE_AUTHOR("Alain Volmat <alain.volmat@xxxxxxxxxxx");
> +MODULE_DESCRIPTION("GalaxyCore GC2145 sensor driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
