Re: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor
Hi Vishal, In probe(), it calls imx274_load_default(imx274), which has I2C register read/write to the IMX274. If it fails, it will exit probe(). So it works as a sensor detection function as you suggested. Regards, Leon Luo 1130 Cadillac CT Milpitas, CA 95035 Phone: (510)371-1169 Fax: (408) 217-1960 Email: le...@leopardimaging.com www.leopardimaging.com On Thu, Nov 2, 2017 at 9:36 PM, Vishal Sagar wrote: > Hi Leon, > > I understand this fixes correctly freeing the v4l control handlers in probe(). > > But if there is a scenario where the sensor is mounted on a removable > daughter card, > shouldn't the probe fail if the daughter card is not connected? > A sample read/write to an IMX274 register should be sufficient to confirm > this in the probe() and fail. > > Does it make sense to add this in the probe()? > > Regards > Vishal Sagar > >> -Original Message- >> From: linux-media-ow...@vger.kernel.org [mailto:linux-media- >> ow...@vger.kernel.org] On Behalf Of Leon Luo >> Sent: Thursday, October 26, 2017 12:21 PM >> To: mche...@kernel.org; robh...@kernel.org; mark.rutl...@arm.com; >> hans.verk...@cisco.com; sakari.ai...@linux.intel.com >> Cc: linux-media@vger.kernel.org; devicet...@vger.kernel.org; linux- >> ker...@vger.kernel.org; le...@leopardimaging.com; Soren Brinkmann >> >> Subject: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor >> >> The imx274 is a Sony CMOS image sensor that has 1/2.5 image size. >> It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface >> is 4-lane MIPI CSI-2 running at 1.44Gbps each. >> >> This driver has been tested on Xilinx ZCU102 platform with a Leopard >> LI-IMX274MIPI-FMC camera board. >> >> Support for the following features: >> -Resolutions: 3840x2160, 1920x1080, 1280x720 >> -Frame rate: 3840x2160 : 5 – 60fps >> 1920x1080 : 5 – 120fps >> 1280x720 : 5 – 120fps >> -Exposure time: 16 – (frame interval) micro-seconds >> -Gain: 1x - 180x >> -VFLIP: enable/disabledrivers/media/i2c/imx274.c >> -Test pattern: 12 test patterns >> >> Signed-off-by: Leon Luo >> Tested-by: Sören Brinkmann >> Acked-by: Sakari Ailus >> Acked-by: Chris Kohn >> --- >> v9: >> - remove v4l2_async_unregister_subdev from probe error >> v8: >> - check division by zero error >> v7: >> - use __v4l2_ctrl_s_ctrl instead of v4l2_ctrl_s_ctrl to have >>clean mutex_lock/mutex_unlock in imx274_s_stream() >> - define imx274_tp_regs[] as static, move the test pattern reg >>out of imx274_tp_regs[], and define it as a macro >>IMX274_TEST_PATTERN_REG >> v6: >> - remove media/v4l2-image-sizes.h from include header >> - make the header file alphabetical order >> - remove fmt->pad check in imx274_get_fmt, >>the V4L2 subdev framework does it already >> - change 'struct reg_8 *regs' to 'struct reg_8 regs[n]', >>where n is the exact numbers needed for this function >> - move MODULE_DEVICE_TABLE(of, imx274_of_id_table); closer >>to imx274_of_id_table definition >> - remove return check of imx274_write_table in imx274_remove, >>because it should remove all resources even i2c fails here >> - move imx274_load_default before v4l2_async_register_subdev >> v5: >> - no changes >> v4: >> - use 32-bit data type to avoid __divdi3 compile error for i386 >> - clean up OR together error codesdrivers/media/i2c/imx274.c >> v3: >> - clean up header files >> - use struct directly instead of alias #define >> - use v4l2_ctrl_s_ctrl instead of v4l2_s_ctrl >> - revise debug output >> - move static helpers closer to their call site >> - don't OR toegether error codes >> - use closest valid gain value instead of erroring out >> - assigne lock to the control handler and omit explicit locking >> - acquire mutex lock for imx274_get_fmt >> - remove format->pad check in imx274_set_fmt since the pad is always 0 >> - pass struct v4l2_ctrl pointer in gain/exposure/vlip/test pattern controls >> - remove priv->ctrls.vflip->val = val in imx274_set_vflip() >> - remove priv->ctrls.test_pattern->val = val in imx274_set_test_pattern() >> - remove empty open/close callbacks >> - remove empty core ops >> - add more error labels in probe >> - use v4l2_async_unregister_subdev instead of v4l2_device_unregister_subdev >> - use dynamic debug >> - split start_stream to two steps: imx274_mode_regs() and >> imx274_start_stream() >>frame rate & exposure can be updated >>between imx274_mode_regs() & imx274_start_stream() >> >> v2: >> - Fix Kconfig to not remove existing options >> --- >> drivers/media/i2c/Kconfig |7 + >> drivers/media/i2c/Makefile |1 + >> drivers/media/i2c/imx274.c | 1810 >> >> 3 files changed, 1818 insertions(+) >> create mode 100644 drivers/media/i2c/imx274.c >> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index 47113774a297..9659849e33a0 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -555,6 +555,1
RE: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor
Hi Leon, I understand this fixes correctly freeing the v4l control handlers in probe(). But if there is a scenario where the sensor is mounted on a removable daughter card, shouldn't the probe fail if the daughter card is not connected? A sample read/write to an IMX274 register should be sufficient to confirm this in the probe() and fail. Does it make sense to add this in the probe()? Regards Vishal Sagar > -Original Message- > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Leon Luo > Sent: Thursday, October 26, 2017 12:21 PM > To: mche...@kernel.org; robh...@kernel.org; mark.rutl...@arm.com; > hans.verk...@cisco.com; sakari.ai...@linux.intel.com > Cc: linux-media@vger.kernel.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org; le...@leopardimaging.com; Soren Brinkmann > > Subject: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor > > The imx274 is a Sony CMOS image sensor that has 1/2.5 image size. > It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface > is 4-lane MIPI CSI-2 running at 1.44Gbps each. > > This driver has been tested on Xilinx ZCU102 platform with a Leopard > LI-IMX274MIPI-FMC camera board. > > Support for the following features: > -Resolutions: 3840x2160, 1920x1080, 1280x720 > -Frame rate: 3840x2160 : 5 – 60fps > 1920x1080 : 5 – 120fps > 1280x720 : 5 – 120fps > -Exposure time: 16 – (frame interval) micro-seconds > -Gain: 1x - 180x > -VFLIP: enable/disabledrivers/media/i2c/imx274.c > -Test pattern: 12 test patterns > > Signed-off-by: Leon Luo > Tested-by: Sören Brinkmann > Acked-by: Sakari Ailus > Acked-by: Chris Kohn > --- > v9: > - remove v4l2_async_unregister_subdev from probe error > v8: > - check division by zero error > v7: > - use __v4l2_ctrl_s_ctrl instead of v4l2_ctrl_s_ctrl to have >clean mutex_lock/mutex_unlock in imx274_s_stream() > - define imx274_tp_regs[] as static, move the test pattern reg >out of imx274_tp_regs[], and define it as a macro >IMX274_TEST_PATTERN_REG > v6: > - remove media/v4l2-image-sizes.h from include header > - make the header file alphabetical order > - remove fmt->pad check in imx274_get_fmt, >the V4L2 subdev framework does it already > - change 'struct reg_8 *regs' to 'struct reg_8 regs[n]', >where n is the exact numbers needed for this function > - move MODULE_DEVICE_TABLE(of, imx274_of_id_table); closer >to imx274_of_id_table definition > - remove return check of imx274_write_table in imx274_remove, >because it should remove all resources even i2c fails here > - move imx274_load_default before v4l2_async_register_subdev > v5: > - no changes > v4: > - use 32-bit data type to avoid __divdi3 compile error for i386 > - clean up OR together error codesdrivers/media/i2c/imx274.c > v3: > - clean up header files > - use struct directly instead of alias #define > - use v4l2_ctrl_s_ctrl instead of v4l2_s_ctrl > - revise debug output > - move static helpers closer to their call site > - don't OR toegether error codes > - use closest valid gain value instead of erroring out > - assigne lock to the control handler and omit explicit locking > - acquire mutex lock for imx274_get_fmt > - remove format->pad check in imx274_set_fmt since the pad is always 0 > - pass struct v4l2_ctrl pointer in gain/exposure/vlip/test pattern controls > - remove priv->ctrls.vflip->val = val in imx274_set_vflip() > - remove priv->ctrls.test_pattern->val = val in imx274_set_test_pattern() > - remove empty open/close callbacks > - remove empty core ops > - add more error labels in probe > - use v4l2_async_unregister_subdev instead of v4l2_device_unregister_subdev > - use dynamic debug > - split start_stream to two steps: imx274_mode_regs() and > imx274_start_stream() >frame rate & exposure can be updated >between imx274_mode_regs() & imx274_start_stream() > > v2: > - Fix Kconfig to not remove existing options > --- > drivers/media/i2c/Kconfig |7 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/imx274.c | 1810 > > 3 files changed, 1818 insertions(+) > create mode 100644 drivers/media/i2c/imx274.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 47113774a297..9659849e33a0 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -555,6 +555,13 @@ config VIDEO_APTINA_PLL > config VIDEO_SMIAPP_PLL > tristate > > +config VIDEO_IMX274 > + tristate "Sony IMX274 sensor support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + ---help--- > + This is a V4L2 sensor-level driver for the Sony IMX274 > + CMOS image sensor. > + > config VIDEO_OV2640 > tristate "OmniVision OV2640 sensor support" > depends on VIDEO_V4L2 && I2C > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index c843c181dfb9.
