[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZU/UX9Jjk288ELUx@duo.ucw.cz>
Date: Sat, 11 Nov 2023 20:22:07 +0100
From: Pavel Machek <pavel@....cz>
To: Alain Volmat <alain.volmat@...s.st.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Jacopo Mondi <jacopo.mondi@...asonboard.com>,
Kieran Bingham <kieran.bingham@...asonboard.com>,
Rob Herring <robh@...nel.org>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] media: i2c: gc2145: Galaxy Core GC2145 sensor
support
Hi!
> 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
"are"?
> coming from Galaxy Core and for that purpose only 3 static
"and so"?
> resolutions are supported.
"supported:"?
> - 640x480
> - 1280x720
> - 1600x1200
> --- /dev/null
> +++ b/drivers/media/i2c/gc2145.c
> @@ -0,0 +1,1404 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * A V4L2 driver for Galaxycore GC2145 camera.
> + * Copyright (C) 2023, STMicroelectronics SA
> + *
> + * Inspired from the imx219.c driver
"by the"?
Link to some kind of datasheet / documentation /... would be welcome
here.
> +/**
> + * struct gc2145_mode - GC2145 mode description
> + * @width: frame width (in pixel)
> + * @height: frame height (in pixel)
"in pixels".
> +static const struct gc2145_mode supported_modes[] = {
...
> + {
> + /* 1280x720 30fps mode */
> + .width = 1280,
> + .height = 720,
> + .reg_seq = gc2145_mode_1280_720_regs,
> + .reg_seq_size = ARRAY_SIZE(gc2145_mode_1280_720_regs),
> + .pixel_rate = GC2145_1280_720_PIXELRATE,
> + .crop = {
> + .top = 160,
> + .left = 240,
> + .width = 1280,
> + .height = 720,
> + },
> + .hblank = GC2145_1280_720_HBLANK,
> + .vblank = GC2145_1280_720_VBLANK,
> + },
Won't this result in 1120x480 mode due to crop?
> +/* All supported formats */
> +static const struct gc2145_format supported_formats[] = {
> + {
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> + .code = MEDIA_BUS_FMT_VYUY8_1X16,
> + .code = MEDIA_BUS_FMT_YUYV8_1X16,
> + .code = MEDIA_BUS_FMT_YVYU8_1X16,
> + .code = MEDIA_BUS_FMT_RGB565_1X16,
> +};
So ... the hardware can do 10bit ADC, but we don't actually have a
mode exposing that?
> + * Adjust the MIPI buffer settings.
> + * For YUV/RGB, LWC = image width * 2
> + * For RAW8, LWC = image width
> + * For RAW10, LWC = image width * 1.25
> + */
> + lwc = gc2145->mode->width * 2;
> + cci_write(gc2145->regmap, GC2145_REG_LWC_HIGH, lwc >> 8, &ret);
> + cci_write(gc2145->regmap, GC2145_REG_LWC_LOW, lwc & 0xff, &ret);
> +
> + /*
> + * Adjust the MIPI Fifo Full Level
Fifo -> FIFO?
> + /*
> + * Set the fifo gate mode / MIPI wdiv set:
> + * 0xf1 in case of RAW mode and 0xf0 otherwise
> + */
fifo -> FIFO?
> + /*
> + * Datasheet doesn't mention timing between PWDN/RESETB control and
> + * i2c access however experimentation shows that a rather big delay is
> + * needed
> + */
"however," "needed."
> +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)
> +{
> + ret = v4l2_ctrl_handler_init(hdl, 12);
> + if (ret)
> + return ret;
> +
> + ctrls->pixel_rate = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE,
> + GC2145_640_480_PIXELRATE,
> + GC2145_1280_720_PIXELRATE, 1,
Should the second pixelrate be one from 1600x1200?
> +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;
This "ret" value is unused. Not sure if something will warn about this.
> +MODULE_AUTHOR("Alain Volmat <alain.volmat@...s.st.com");
">" is missing at the end of address.
The driver looks good, thank you!
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)
Powered by blists - more mailing lists