Thread (11 messages) 11 messages, 3 authors, 2022-02-25

Re: [PATCH v10 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x

From: Hans Verkuil <hidden>
Date: 2022-02-23 11:52:11
Also in: linux-media

On 2/17/22 16:44, Michael Tretter wrote:
From: Marek Vasut <marex@denx.de>

Add driver for the Intersil ISL7998x Analog to MIPI CSI-2/BT656 decoder.
This chip supports 1/2/4 analog video inputs and converts them into
1/2/4 VCs in MIPI CSI2 stream.

This driver currently supports ISL79987 and both 720x480 and 720x576
resolutions, however as per specification, all inputs must use the
same resolution and standard. The only supported pixel format is now
YUYV/YUV422. The chip should support RGB565 on the CSI2 as well, but
this is currently unsupported.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
To: linux-media@vger.kernel.org
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Changelog:

v10:

- add a lock for subdev calls
- remove unnecessary pm_runtime_enabled
- fix indentation and format
- free controls on error
- fix set_standard call
- remove camel case in macro definitions
- add macros for video formats
- rework lookup of video standards
- add support for PAL Nc
- add explicit trigger for format detection

v9: none

v8:

- fix warning "type qualifiers ignored on function return type"

v7:

- reserve driver specific controls
- add documentation for driver specific controls
- implement g_input_status
- track device enabled state in driver
- store norm instead of mode in driver
- select test pattern based on video norm
- improve debug message for enabled test pattern
- fix off by one with 4 inputs
- implement querystd and friends
- fix polling condition for standard detection
- use v4l2_norm_to_name instead of custom implementation

v6:

- remove unused log2.h
- add select MEDIA_CONTROLLER
- use poll_read_timeout to wait for power on
- add timeout to polling for video standard
- use fwnode_graph_get_endpoint_by_id
- fix invalid bus type error message

v5: none

v4:

- fix lines over 80 chars where applicable
- fix possible NULL pointer access in link_freq
- initialize bus type with CSI2_DPHY
- iterate over pads instead of hard coded 4
- merge power_{on,off} functions into resume,suspend
- switch to v4l2_subdev_state
- report field order based on video standard
- add error message for timeout
- simplify dev_dbg statement in update_std
- call v4l2_ctrl_handler_setup
- don't set control if pm_runtime is not enabled
- fix YUV422 byte order
- switch to pre_streamon callback for LP11 mode

v3:

- follow dt binding change: pd-gpios -> powerdown-gpios

v2:

- general cleanup
- remove isl7998x_g_mbus_config function
- implement enum_frame_size function
- replace msleep with usleep_range
- rework set_fmt/get_fmt functions
- calculate number of inputs using number of input ports
- switch to runtime_pm
- add reset gpio
- add adv_debug support
- add MAINTAINERS entry
---
 MAINTAINERS                        |    8 +
 drivers/media/i2c/Kconfig          |   10 +
 drivers/media/i2c/Makefile         |    1 +
 drivers/media/i2c/isl7998x.c       | 1633 ++++++++++++++++++++++++++++
 include/uapi/linux/v4l2-controls.h |    6 +
 5 files changed, 1658 insertions(+)
 create mode 100644 drivers/media/i2c/isl7998x.c
<snip>
+static int isl7998x_pre_streamon(struct v4l2_subdev *sd, u32 flags)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct device *dev = &client->dev;
+
+	if (flags & V4L2_SUBDEV_PRE_STREAMON_FL_MANUAL_LP)
+		return pm_runtime_resume_and_get(dev);
+
+	return 0;
This feels a bit scary: if V4L2_SUBDEV_PRE_STREAMON_FL_MANUAL_LP is NOT
set, then pm_runtime_resume_and_get() isn't called, but this function
still returns success...
+}
+
+static int isl7998x_post_streamoff(struct v4l2_subdev *sd)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct device *dev = &client->dev;
+
+	pm_runtime_put(dev);
...and pm_runtime_put() is called without the corresponding get.

The documentation in v4l2-subdev.h isn't very clear about what
pre_streamon should return. I'm inclined to say that it should
return -EACCES.

Sakari, what do you think?
+
+	return 0;
+}
Regards,

	Hans
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help