Re: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung,
Thanks for the patch. Please see my comments below.
On Tue, Jul 10, 2018 at 03:15:36PM +0800, Ping-chung Chen wrote:
> From: "Chen, Ping-chung"
>
> Add a V4L2 sub-device driver for the Sony IMX208 image sensor.
> This is a camera sensor using the I2C bus for control and the
> CSI-2 bus for data.
>
> Signed-off-by: Ping-Chung Chen
> ---
> drivers/media/i2c/Kconfig | 11 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/imx208.c | 953
> +
> 3 files changed, 965 insertions(+)
> create mode 100644 drivers/media/i2c/imx208.c
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 8669853..1c739f4 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -595,6 +595,17 @@ config VIDEO_APTINA_PLL
> config VIDEO_SMIAPP_PLL
> tristate
>
> +config VIDEO_IMX208
> + tristate "Sony IMX208 sensor support"
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> + This is a Video4Linux2 sensor-level driver for the Sony
s/-level//
> + IMX208 camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called imx208.
> +
> config VIDEO_IMX258
> tristate "Sony IMX258 sensor support"
> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 837c428..47604c2 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -104,6 +104,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
> obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> +obj-$(CONFIG_VIDEO_IMX208) += imx208.o
> obj-$(CONFIG_VIDEO_IMX258) += imx258.o
> obj-$(CONFIG_VIDEO_IMX274) += imx274.o
>
> diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c
> new file mode 100644
> index 000..9af9043
> --- /dev/null
> +++ b/drivers/media/i2c/imx208.c
> @@ -0,0 +1,953 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2018 Intel Corporation
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#ifndef V4L2_CID_DIGITAL_GAIN
> +#define V4L2_CID_DIGITAL_GAIN V4L2_CID_GAIN
> +#endif
Please drop this.
> +
> +#define IMX208_REG_VALUE_08BIT 1
> +#define IMX208_REG_VALUE_16BIT 2
> +#define IMX208_REG_VALUE_24BIT 3
The last one is unused.
> +
> +#define IMX208_REG_MODE_SELECT 0x0100
> +#define IMX208_MODE_STANDBY 0x00
> +#define IMX208_MODE_STREAMING0x01
> +
> +/* Chip ID */
> +#define IMX208_REG_CHIP_ID 0x
> +#define IMX208_CHIP_ID 0x0208
> +
> +/* V_TIMING internal */
> +#define IMX208_REG_VTS 0x0340
> +#define IMX208_VTS_60FPS 0x0472
> +#define IMX208_VTS_BINNING 0x0239
> +#define IMX208_VTS_60FPS_MIN 0x0458
> +#define IMX208_VTS_BINNING_MIN 0x0230
> +#define IMX208_VTS_MAX 0x
> +
> +/* HBLANK control - read only */
> +#define IMX208_PPL_384MHZ1124
> +#define IMX208_PPL_96MHZ 1124
> +
> +/* Exposure control */
> +#define IMX208_REG_EXPOSURE 0x0202
> +#define IMX208_EXPOSURE_MIN 4
> +#define IMX208_EXPOSURE_STEP 1
> +#define IMX208_EXPOSURE_DEFAULT 0x190
> +#define IMX208_EXPOSURE_MAX 65535
> +
> +/* Analog gain control */
> +#define IMX208_REG_ANALOG_GAIN 0x0204
> +#define IMX208_ANA_GAIN_MIN 0
> +#define IMX208_ANA_GAIN_MAX 0x00e0
> +#define IMX208_ANA_GAIN_STEP 1
> +#define IMX208_ANA_GAIN_DEFAULT 0x0
> +
> +/* Digital gain control */
> +#define IMX208_REG_GR_DIGITAL_GAIN 0x020e
> +#define IMX208_REG_R_DIGITAL_GAIN0x0210
> +#define IMX208_REG_B_DIGITAL_GAIN0x0212
> +#define IMX208_REG_GB_DIGITAL_GAIN 0x0214
> +#define IMX208_DGTL_GAIN_MIN 0
> +#define IMX208_DGTL_GAIN_MAX 4096 /* Max = 0xFFF */
> +#define IMX208_DGTL_GAIN_DEFAULT 0x100
> +#define IMX208_DGTL_GAIN_STEP 1
> +
> +/* Orientation */
> +#define REG_MIRROR_FLIP_CONTROL 0x0101
> +#define REG_CONFIG_MIRROR_FLIP 0x03
> +
> +#define IMX208_REG_TEST_PATTERN_MODE 0x0600
> +
> +struct imx208_reg {
> + u16 address;
> + u8 val;
> +};
> +
> +struct imx208_reg_list {
> + u32 num_of_regs;
> + const struct imx208_reg *regs;
> +};
> +
> +/* Link frequency config */
> +struct imx208_link_freq_config {
> + u32 pixels_per_line;
> +
> + /* PLL registers for this link frequency */
> + struct imx208_reg_list reg_list;
> +};
> +
> +/* Mode : resolution and related config&values */
> +struct imx208_mode {
> + /* Frame width */
> + u32 width;
> + /* Frame heigh
Re: [PATCH] media: imx208: Add imx208 camera sensor driver
On Tue, Jul 10, 2018 at 07:37:54AM +, Yeh, Andy wrote:
> Hi PC,
>
> Thanks for the patch.
>
> Cc in Grant, and Intel Jim/Jason
>
> > -Original Message-
> > From: Chen, Ping-chung
> > Sent: Tuesday, July 10, 2018 3:16 PM
> > To: linux-media@vger.kernel.org
> > Cc: sakari.ai...@linux.intel.com; Yeh, Andy ;
> > tf...@chromium.org; Chen, Ping-chung
> > Subject: [PATCH] media: imx208: Add imx208 camera sensor driver
> > +};
> > +
> > +static int imx208_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code) {
> > + /* Only one bayer order(GRBG) is supported */
> > + if (code->index > 0)
> > + return -EINVAL;
> > +
>
> There is no limitation on using GRBG bayer order now. You can refer to imx355
> driver as well.
It seems the rest of the driver uses RGGB.
The enumeration should only contain what is possible using the current
flipping configuration.
--
Sakari Ailus
sakari.ai...@linux.intel.com
RE: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Sakari, Some of suggestions below has been added in to [PATCH v2] or [PATCH v3]. We will fix others in PATCH v4 soon. Thanks, PC -Original Message- From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] Sent: Monday, July 16, 2018 11:59 PM To: Chen, Ping-chung Cc: linux-media@vger.kernel.org; Yeh, Andy ; tf...@chromium.org Subject: Re: [PATCH] media: imx208: Add imx208 camera sensor driver Hi Ping-chung, Thanks for the patch. Please see my comments below. On Tue, Jul 10, 2018 at 03:15:36PM +0800, Ping-chung Chen wrote: > From: "Chen, Ping-chung" > > Add a V4L2 sub-device driver for the Sony IMX208 image sensor. > This is a camera sensor using the I2C bus for control and the > CSI-2 bus for data. > > Signed-off-by: Ping-Chung Chen > --- > drivers/media/i2c/Kconfig | 11 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/imx208.c | 953 > + > 3 files changed, 965 insertions(+) > create mode 100644 drivers/media/i2c/imx208.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 8669853..1c739f4 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -595,6 +595,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL > tristate > > +config VIDEO_IMX208 > + tristate "Sony IMX208 sensor support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + depends on MEDIA_CAMERA_SUPPORT > + ---help--- > + This is a Video4Linux2 sensor-level driver for the Sony s/-level// > + IMX208 camera. > + > + To compile this driver as a module, choose M here: the > + module will be called imx208. > + > config VIDEO_IMX258 > tristate "Sony IMX258 sensor support" > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff --git > a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index > 837c428..47604c2 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -104,6 +104,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o > obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o > obj-$(CONFIG_VIDEO_OV2659) += ov2659.o > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o > +obj-$(CONFIG_VIDEO_IMX208) += imx208.o > obj-$(CONFIG_VIDEO_IMX258) += imx258.o > obj-$(CONFIG_VIDEO_IMX274) += imx274.o > > diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c > new file mode 100644 index 000..9af9043 > --- /dev/null > +++ b/drivers/media/i2c/imx208.c > @@ -0,0 +1,953 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (C) 2018 Intel Corporation > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#ifndef V4L2_CID_DIGITAL_GAIN > +#define V4L2_CID_DIGITAL_GAIN V4L2_CID_GAIN #endif Please drop this. > + > +#define IMX208_REG_VALUE_08BIT 1 > +#define IMX208_REG_VALUE_16BIT 2 > +#define IMX208_REG_VALUE_24BIT 3 The last one is unused. > + > +#define IMX208_REG_MODE_SELECT 0x0100 > +#define IMX208_MODE_STANDBY 0x00 > +#define IMX208_MODE_STREAMING0x01 > + > +/* Chip ID */ > +#define IMX208_REG_CHIP_ID 0x > +#define IMX208_CHIP_ID 0x0208 > + > +/* V_TIMING internal */ > +#define IMX208_REG_VTS 0x0340 > +#define IMX208_VTS_60FPS 0x0472 > +#define IMX208_VTS_BINNING 0x0239 > +#define IMX208_VTS_60FPS_MIN 0x0458 > +#define IMX208_VTS_BINNING_MIN 0x0230 > +#define IMX208_VTS_MAX 0x > + > +/* HBLANK control - read only */ > +#define IMX208_PPL_384MHZ1124 > +#define IMX208_PPL_96MHZ 1124 > + > +/* Exposure control */ > +#define IMX208_REG_EXPOSURE 0x0202 > +#define IMX208_EXPOSURE_MIN 4 > +#define IMX208_EXPOSURE_STEP 1 > +#define IMX208_EXPOSURE_DEFAULT 0x190 > +#define IMX208_EXPOSURE_MAX 65535 > + > +/* Analog gain control */ > +#define IMX208_REG_ANALOG_GAIN 0x0204 > +#define IMX208_ANA_GAIN_MIN 0 > +#define IMX208_ANA_GAIN_MAX 0x00e0 > +#define IMX208_ANA_GAIN_STEP 1 > +#define IMX208_ANA_GAIN_DEFAULT 0x0 > + > +/* Digital gain control */ > +#define IMX208_REG_GR_DIGITAL_GAIN 0x020e > +#define IMX208_REG_R_DIGITAL_GAIN0x0210 > +#define IMX208_REG_B_DIGITAL_GAIN0x0212 > +#define IMX208_REG_GB_DIGITAL_GAIN 0x0214 > +#define IMX208_DGTL_GAIN_MIN 0 > +#define IMX208_DGTL_GAIN_MA
Re: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung, On Tue, Jul 17, 2018 at 3:53 PM Chen, Ping-chung wrote: > > Hi Sakari, > > Some of suggestions below has been added in to [PATCH v2] or [PATCH v3]. > We will fix others in PATCH v4 soon. I don't see v2 or v3 on linux-media (or my inbox). Where were they sent? I'd like to review the driver, but if there is already v3, it would make sense to review that one. Also, please refrain from top-posting on mailing lists, it's considered bad manner (and makes threads difficult to read). Best regards, Tomasz
RE: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi PC,
Thanks for the patch.
Cc in Grant, and Intel Jim/Jason
> -Original Message-
> From: Chen, Ping-chung
> Sent: Tuesday, July 10, 2018 3:16 PM
> To: linux-media@vger.kernel.org
> Cc: sakari.ai...@linux.intel.com; Yeh, Andy ;
> tf...@chromium.org; Chen, Ping-chung
> Subject: [PATCH] media: imx208: Add imx208 camera sensor driver
> +};
> +
> +static int imx208_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code) {
> + /* Only one bayer order(GRBG) is supported */
> + if (code->index > 0)
> + return -EINVAL;
> +
There is no limitation on using GRBG bayer order now. You can refer to imx355
driver as well.
+static int imx355_enum_frame_size(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_size_enum
*fse)
+{
+ struct imx355 *imx355 = to_imx355(sd);
> + code->code = MEDIA_BUS_FMT_SRGGB10_1X10;
> +
> + return 0;
> +}
> +
> +static int imx208_enum_frame_size(struct v4l2_subdev *sd,
> +struct v4l2_subdev_pad_config *cfg,
> +struct v4l2_subdev_frame_size_enum *fse) {
> + if (fse->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + if (fse->code != MEDIA_BUS_FMT_SRGGB10_1X10)
> + return -EINVAL;
> +
> + 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 void imx208_update_pad_format(const struct imx208_mode *mode,
> + struct v4l2_subdev_format *fmt) {
> + fmt->format.width = mode->width;
> + fmt->format.height = mode->height;
> + fmt->format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> + fmt->format.field = V4L2_FIELD_NONE;
> +}
> +
> +
> +static int imx208_probe(struct i2c_client *client) {
> + struct imx208 *imx208;
> + int ret;
> + u32 val = 0;
> +
> + device_property_read_u32(&client->dev, "clock-frequency", &val);
> + if (val != 1920)
> + return -EINVAL;
> +
> + imx208 = devm_kzalloc(&client->dev, sizeof(*imx208), GFP_KERNEL);
> + if (!imx208)
> + return -ENOMEM;
> +
> + /* Initialize subdev */
> + v4l2_i2c_subdev_init(&imx208->sd, client, &imx208_subdev_ops);
> +
> + /* Check module identity */
> + ret = imx208_identify_module(imx208);
> + if (ret)
> + return ret;
> +
> + /* Set default mode to max resolution */
> + imx208->cur_mode = &supported_modes[0];
> +
> + ret = imx208_init_controls(imx208);
> + if (ret)
> + return ret;
> +
> + /* Initialize subdev */
> + imx208->sd.internal_ops = &imx208_internal_ops;
> + imx208->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +
> + /* Initialize source pad */
> + imx208->pad.flags = MEDIA_PAD_FL_SOURCE;
Please refer to IMX355 to change below code to new API on upstreamed kernel.
https://patchwork.linuxtv.org/patch/50028/
+ /* Initialize subdev */
+ imx355->sd.internal_ops = &imx355_internal_ops;
+ imx355->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
+ V4L2_SUBDEV_FL_HAS_EVENTS;
+ imx355->sd.entity.ops = &imx355_subdev_entity_ops;
+ imx355->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(&imx355->sd.entity, 1,
&imx355->pad);
> + ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
> + if (ret) {
> + dev_err(&client->dev, "%s failed:%d\n", __func__, ret);
> + goto error_handler_free;
> + }
> +
> + ret = v4l2_async_register_subdev_sensor_common(&imx208->sd);
> + if (ret < 0)
> + goto error_media_entity;
> +
> + pm_runtime_set_active(&client->dev);
> + pm_runtime_enable(&client->dev);
> + pm_runtime_idle(&client->dev);
> +
> + return 0;
> +
> +error_media_entity:
> + media_entity_cleanup(&imx208->sd.entity);
> +
> +error_handler_free:
> + imx208_free_controls(imx208);
> +
> + return ret;
> +}
> +
> +static int imx208_remove(struct i2c_client *client) {
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx208 *imx208 = to_imx208(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + imx208_free_controls(imx208);
> +
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops imx208_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS
Re: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.18-rc4 next-20180709]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Ping-chung-Chen/media-imx208-Add-imx208-camera-sensor-driver/20180710-153546
base: git://linuxtv.org/media_tree.git master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/media/i2c/imx208.c:881:26: sparse: no member 'type' in struct
media_entity
drivers/media/i2c/imx208.c:885:15: sparse: undefined identifier
'media_entity_init'
drivers/media/i2c/imx208.c:881:26: sparse: generating address of non-lvalue
(8)
drivers/media/i2c/imx208.c:885:32: sparse: call with no type!
drivers/media/i2c/imx208.c: In function 'imx208_probe':
>> drivers/media/i2c/imx208.c:881:20: error: 'struct media_entity' has no
>> member named 'type'; did you mean 'pipe'?
imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
^~~~
pipe
>> drivers/media/i2c/imx208.c:881:27: error: 'MEDIA_ENT_T_V4L2_SUBDEV_SENSOR'
>> undeclared (first use in this function); did you mean
>> 'MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN'?
imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
^~
MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN
drivers/media/i2c/imx208.c:881:27: note: each undeclared identifier is
reported only once for each function it appears in
>> drivers/media/i2c/imx208.c:885:8: error: implicit declaration of function
>> 'media_entity_init'; did you mean 'media_entity_put'?
>> [-Werror=implicit-function-declaration]
ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
^
media_entity_put
cc1: some warnings being treated as errors
sparse warnings: (new ones prefixed by >>)
drivers/media/i2c/imx208.c:881:26: sparse: no member 'type' in struct
media_entity
drivers/media/i2c/imx208.c:885:15: sparse: undefined identifier
'media_entity_init'
>> drivers/media/i2c/imx208.c:881:26: sparse: generating address of non-lvalue
>> (8)
>> drivers/media/i2c/imx208.c:885:32: sparse: call with no type!
drivers/media/i2c/imx208.c: In function 'imx208_probe':
drivers/media/i2c/imx208.c:881:20: error: 'struct media_entity' has no
member named 'type'; did you mean 'pipe'?
imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
^~~~
pipe
drivers/media/i2c/imx208.c:881:27: error: 'MEDIA_ENT_T_V4L2_SUBDEV_SENSOR'
undeclared (first use in this function); did you mean
'MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN'?
imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
^~
MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN
drivers/media/i2c/imx208.c:881:27: note: each undeclared identifier is
reported only once for each function it appears in
drivers/media/i2c/imx208.c:885:8: error: implicit declaration of function
'media_entity_init'; did you mean 'media_entity_put'?
[-Werror=implicit-function-declaration]
ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
^
media_entity_put
cc1: some warnings being treated as errors
vim +881 drivers/media/i2c/imx208.c
848
849 static int imx208_probe(struct i2c_client *client)
850 {
851 struct imx208 *imx208;
852 int ret;
853 u32 val = 0;
854
855 device_property_read_u32(&client->dev, "clock-frequency", &val);
856 if (val != 1920)
857 return -EINVAL;
858
859 imx208 = devm_kzalloc(&client->dev, sizeof(*imx208),
GFP_KERNEL);
860 if (!imx208)
861 return -ENOMEM;
862
863 /* Initialize subdev */
864 v4l2_i2c_subdev_init(&imx208->sd, client, &imx208_subdev_ops);
865
866 /* Check module identity */
867 ret = imx208_identify_module(imx208);
868 if (ret)
869 return ret;
870
871 /* Set default mode to max resolution */
872 imx208->cur_mode = &supported_modes[0];
873
874 ret = imx208_init_controls(imx208);
875 if (ret)
876 return ret;
877
878 /* Initialize subdev */
879 imx208->sd.internal_ops = &imx208_internal_ops;
880 imx208->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> 881 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
882
883 /* Initialize source pad */
Re: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.18-rc4 next-20180709]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Ping-chung-Chen/media-imx208-Add-imx208-camera-sensor-driver/20180710-153546
base: git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/media/i2c/imx208.c: In function 'imx208_probe':
drivers/media/i2c/imx208.c:881:20: error: 'struct media_entity' has no
member named 'type'; did you mean 'pipe'?
imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
^~~~
pipe
>> drivers/media/i2c/imx208.c:881:27: error: 'MEDIA_ENT_T_V4L2_SUBDEV_SENSOR'
>> undeclared (first use in this function); did you mean
>> 'MEDIA_ENTITY_TYPE_V4L2_SUBDEV'?
imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
^~
MEDIA_ENTITY_TYPE_V4L2_SUBDEV
drivers/media/i2c/imx208.c:881:27: note: each undeclared identifier is
reported only once for each function it appears in
drivers/media/i2c/imx208.c:885:8: error: implicit declaration of function
'media_entity_init'; did you mean 'media_entity_put'?
[-Werror=implicit-function-declaration]
ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
^
media_entity_put
cc1: some warnings being treated as errors
vim +881 drivers/media/i2c/imx208.c
848
849 static int imx208_probe(struct i2c_client *client)
850 {
851 struct imx208 *imx208;
852 int ret;
853 u32 val = 0;
854
855 device_property_read_u32(&client->dev, "clock-frequency", &val);
856 if (val != 1920)
857 return -EINVAL;
858
859 imx208 = devm_kzalloc(&client->dev, sizeof(*imx208),
GFP_KERNEL);
860 if (!imx208)
861 return -ENOMEM;
862
863 /* Initialize subdev */
864 v4l2_i2c_subdev_init(&imx208->sd, client, &imx208_subdev_ops);
865
866 /* Check module identity */
867 ret = imx208_identify_module(imx208);
868 if (ret)
869 return ret;
870
871 /* Set default mode to max resolution */
872 imx208->cur_mode = &supported_modes[0];
873
874 ret = imx208_init_controls(imx208);
875 if (ret)
876 return ret;
877
878 /* Initialize subdev */
879 imx208->sd.internal_ops = &imx208_internal_ops;
880 imx208->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> 881 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
882
883 /* Initialize source pad */
884 imx208->pad.flags = MEDIA_PAD_FL_SOURCE;
885 ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
886 if (ret) {
887 dev_err(&client->dev, "%s failed:%d\n", __func__, ret);
888 goto error_handler_free;
889 }
890
891 ret = v4l2_async_register_subdev_sensor_common(&imx208->sd);
892 if (ret < 0)
893 goto error_media_entity;
894
895 pm_runtime_set_active(&client->dev);
896 pm_runtime_enable(&client->dev);
897 pm_runtime_idle(&client->dev);
898
899 return 0;
900
901 error_media_entity:
902 media_entity_cleanup(&imx208->sd.entity);
903
904 error_handler_free:
905 imx208_free_controls(imx208);
906
907 return ret;
908 }
909
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
