From: <Hermes.Wu@ite.com.tw>
To: <hverkuil+cisco@kernel.org>, <mchehab@kernel.org>,
<robh@kernel.org>, <krzk+dt@kernel.org>, <conor+dt@kernel.org>
Cc: <linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v9 2/2] media: i2c: add driver for ITE IT6625/IT6626
Date: Mon, 3 Aug 2026 11:25:31 +0000 [thread overview]
Message-ID: <3bffbc016a6d4f968dbc0594bfef14c0@ite.com.tw> (raw)
In-Reply-To: <149e0c8d-9aef-4a0a-8cae-0c6f0ee64d23@kernel.org>
Hi Hans,
>-----Original Message-----
>From: Hans Verkuil <hverkuil+cisco@kernel.org>
>Sent: Friday, July 31, 2026 7:57 PM
>To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>; Mauro Carvalho Chehab <mchehab@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH v9 2/2] media: i2c: add driver for ITE IT6625/IT6626
>
>Hi Hermes,
>
>Thank you for posting this driver!
>
>See below for my code review.
>
>One high-level question: how is this driver tested, specifically with what CSI/capture
>driver?
>
>Can you provide the 'v4l2-compliance output -s' with that driver?
>
>I suspect that the IT6626 is currently limited to HDMI 2.0, is that correct? If so,
>then that should be documented somewhere in the code (it might be there already,
>in that case I missed it).
>
>On 30/07/2026 08:55, Hermes Wu via B4 Relay wrote:
>> From: Hermes Wu <Hermes.wu@ite.com.tw>
>>
>> Add a V4L2 subdevice driver for the ITE IT6625/IT6626 HDMI-to-MIPI
>> CSI-2 bridge chips. IT6625 accepts an HDMI 2.0 input and IT6626 an
>> HDMI 2.1 input, converting it to a D-PHY (or C/D-PHY on IT6626)
>> MIPI CSI-2 output. The bridge is configured over I2C, exposes an
>> HDMI CEC adapter, and supports EDID read/write, DV timings
>> detection/configuration, and HPD control via the standard V4L2
>> subdevice pad and video ops.
>>
>> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
>> ---
>> Changes in v9:
>> - Change it6625_interrupt_handler() to return bool and
>> it6625_irq_handler() to translate that into IRQ_HANDLED/IRQ_NONE:
>> the handler returned early without clearing REG_MCU_INTERRUPT
>> whenever the read was spurious (val == 0) or failed (val < 0), yet
>> the IRQ handler unconditionally returned IRQ_HANDLED. On that path
>> no IT6625 interrupt source was observed for this invocation, so
>> always reporting IRQ_HANDLED regardless defeats the kernel's
>> spurious-IRQ storm protection, which relies on IRQ_NONE to identify
>> and disable misbehaving lines. Matches the pattern already used by
>> tc358743.c and adv7604.c. Found by the sashiko.dev automated review
>> of v8.
>> - Take it6625_lock in it6625_initial_setup(): it wrote
>> B_CONFIG_UPDATE to REG_HOST_CTRL_INT and polled for it to clear
>> without holding the lock, but it runs from probe() after
>> cec_register_adapter() has already exposed /dev/cecX to userspace,
>> and it6625_cec_adap_transmit() (reachable immediately via a CEC
>> ioctl) sets B_CEC_SEND_DATA in the same register under the lock.
>> Every other accessor of REG_HOST_CTRL_INT already took the lock;
>> this was the sole exception. Single call site, lock already
>> initialized before it, so no deadlock is introduced. Found by the
>> sashiko.dev automated review of v8.
>>
>> One other v8 review finding was checked and is not being acted on:
>> - fps_from_bt_timings() computing
>> V4L2_DV_BT_FRAME_HEIGHT(t) * V4L2_DV_BT_FRAME_WIDTH(t) as a 32-bit
>> product, which the review claims can overflow to 0 given
>> V4L2_DV_BT_CAP_CUSTOM's relaxed matching. Checked: it6625->timings,
>> the only value this function is ever called on, is populated only
>> by it6625_update_timings_if_changed(), which rejects anything
>> failing v4l2_valid_dv_timings() against it6625_timings_cap (width
>> 640-3840, height 480-2160), or by a static default.
>> v4l2_valid_dv_timings() additionally bounds every porch field
>> (hfrontporch/hsync/hbackporch <= 3x width;
>> vfrontporch/vsync/vbackporch/il_* <= 10240). Given those caps the
>> maximum possible FRAME_WIDTH * FRAME_HEIGHT is
>> 38400 x 63600 ~= 2.44x10^9, safely under 2^32 ~= 4.29x10^9 -- the
>> claimed overflow input is mathematically unreachable through this
>> driver's validated timings path.
>>
>> Changes in v8:
>> - Drop of_match_ptr() around it6625_of_match in the i2c_driver: the
>> table is unconditionally compiled in, so wrapping it is unnecessary
>> and risks an unused-variable warning on configs without CONFIG_OF.
>> Found by Krzysztof Kozlowski's on-list review of v7.
>> - Stop halving bt->pixelclock for interlaced signals in
>> it6625_get_detected_timings(). Checked against the IT6625 spec: for
>> an interlaced source (e.g. 1080i) the chip's built-in MCU already
>> reports REG_VID_PCLK as the correct, real pixel clock (74.25 MHz for
>> 1080i, not a half-rate/field clock), while the height it reports is
>> the per-field active height (540), which the existing
>> "bt->height *= 2" correctly turns into the true full-frame height.
>> Dividing the already-correct pixel clock by 2 on top of that mangled
>> it to 37.125 MHz, understating the reported frame rate by half.
>> Found by the sashiko.dev automated review of v7. il_vfrontporch/
>> il_vbackporch are still left unset for interlaced signals -- the
>> IT6625 MCU has no separate second-field porch registers, so there's
>> nothing to read for them; not fixed, since there's nothing to fix in
>> software.
>>
>> One other v7 review finding was checked and is not being acted on:
>> - it6625_get_fmt() unconditionally writes the live detected timings
>> into format->format.width/height/field regardless of TRY vs ACTIVE,
>> and it6625_set_fmt()'s TRY branch only persists code/colorspace into
>> sd_state. This exactly matches tc358743_get_fmt()/tc358743_set_fmt()
>> in tc358743.c, which never consult sd_state for width/height/field
>> in either TRY or ACTIVE either. For this class of HDMI-receiver
>> bridge, frame dimensions are dictated by the detected external
>> signal, not independently negotiable via TRY -- only the pixel
>> format (code/colorspace) is meaningfully TRY-isolatable, and that's
>> exactly what both drivers isolate. Found by the sashiko.dev
>> automated review of v7.
>>
>> Also, per Krzysztof Kozlowski's on-list review: the it6625/it6626
>> of_device_id entries do encode a real behavioral difference --
>> it6625_parse_endpoint() rejects C-PHY endpoints unless
>> chip_type == IT6626_CHIP, so "ite,it6625" boards are correctly
>> restricted to D-PHY. Replying on-list to point this out, not a code
>> change.
>>
>> Changes in v7:
>> - Fix three correctness issues found by the automated review of v6:
>> it6625_parse_endpoint() left the fwnode endpoint's bus_type as
>> V4L2_MBUS_UNKNOWN so v4l2_fwnode_endpoint_alloc_parse() could
>> autodetect C-PHY vs D-PHY, but if the endpoint node has none of the
>> CSI-2-specific properties, the parser silently falls back to
>> V4L2_MBUS_PARALLEL/BT656 instead -- the driver then read
>> endpoint.bus.mipi_csi2.num_data_lanes unconditionally, treating
>> memory populated as a different union member as a lane count.
>> it6625_set_fmt()'s V4L2_SUBDEV_FORMAT_ACTIVE branch applied the new
>> format to hardware but never updated format->format.colorspace like
>> the TRY branch does, so VIDIOC_SUBDEV_S_FMT callers got back the
>> stale colorspace from before the call. it6625_s_edid() never
>> checked edid->start_block, contrary to the VIDIOC_S_EDID spec
>> (Documentation/userspace-api/media/v4l/vidioc-g-edid.rst), which
>> requires -EINVAL for any start_block other than 0 since setting an
>> EDID is all-or-nothing; added the same check already present in
>> adv7604_s_edid(), tc358743_s_edid() and adv7842_s_edid() for the
>> same requirement.
>>
>> Changes in v6:
>> - Fix two correctness issues found by the automated review of v5:
>> it6625_s_edid() called v4l2_phys_addr_validate() but discarded its
>> return value, so a CEC physical address that failed topology
>> validation (e.g. a gap like 1.0.1.0) still reached
>> cec_s_phys_addr() and got broadcast on the CEC bus instead of being
>> rejected; and it6625_parse_endpoint() pre-set the fwnode endpoint's
>> bus_type to D-PHY before parsing, which (per
>> include/media/v4l2-fwnode.h) makes the parser treat it as a hard
>> requirement and reject any endpoint whose DT bus-type disagrees --
>> every IT6626 C-PHY device tree failed to probe as a result.
>>
>> One other v5 review finding was checked and is not being acted on:
>> - A concurrent VIDIOC_SUBDEV_S_EDID ioctl racing it6625_remove()'s
>> cancel_delayed_work_sync() could in principle schedule
>> hpd_delayed_work after teardown frees the driver state. Real in the
>> abstract -- no lock serializes an in-flight ioctl against
>> video_unregister_device() -- but the identical unlocked pattern
>> (delayed work scheduled from the s_edid path, remove() doing only
>> cancel_delayed_work_sync()) exists unfixed in tc358743.c and
>> adv7604.c, and this is the same residual risk already dismissed in
>> the v2/v3/v4 rounds of this series for the same reason: no peer
>> driver adds extra synchronization for it either.
>>
>> Changes in v5:
>> - Fix a kernel-stack-memory leak in it6625_edid_ram_show() and
>> it6625_mipi_reg_show(): both printed an uninitialized on-stack
>> register buffer to userspace via debugfs without checking whether
>> the preceding I2C bulk read actually succeeded.
>> - Fix two related format-negotiation bugs: it6625_set_fmt()'s
>> V4L2_SUBDEV_FORMAT_TRY branch never updated fmt->colorspace to
>> match the new fmt->code, and it6625_get_fmt()'s TRY branch computed
>> the returned colorspace from the active hardware state instead of
>> the TRY state's own format, leaking active state into what should
>> be an isolated TRY buffer -- store and read code and colorspace
>> together in sd_state instead. Separately, it6625_set_fmt()'s
>> active-format path released it6625_lock before programming the
>> hardware, letting two concurrent S_FMT calls interleave and leave
>> the hardware programmed for the wrong thread's format; add _locked
>> variants of it6625_enable_stream()/it6625_set_mipi_config() and call
>> them from inside set_fmt()'s existing lock scope so the whole
>> update is now atomic.
>> - Fix it6625_s_edid(): per the VIDIOC_S_EDID spec, blocks == 0 must
>> leave the EDID disabled ("no longer available"), but the driver
>> disabled HPD and then fell through to the normal completion path,
>> which unconditionally re-enabled it -- return immediately instead.
>> Also stop a failed it6625_write_edid() from skipping the HPD
>> re-enable entirely, which left HPD disabled permanently after any
>> single failed S_EDID call.
>> - Take it6625_lock around it6625_mipi_reg_write()'s I2C write,
>> matching every other register access in the driver.
>> - Derive IT6625_CID_HDMI_INFO's string from real state
>> (it6625->csi_format and fps_from_bt_timings()) instead of returning
>> a hardcoded "RGB444 10bit @ 60Hz" regardless of actual signal.
>> - Wire up the previously-unused reset-gpios line in probe(): acquire
>> it via devm_gpiod_get_optional() and toggle it (assert briefly, then
>> deassert and let it settle) before the first I2C access.
>>
>> All found by the sashiko.dev automated review of v4. Three other v4
>> review findings were checked and are not being acted on:
>> - it6625_mipi_reg_show()'s raw register dump could destructively
>> consume a pending CEC RX message via REG_CEC_RX_DATA (0x20) --
>> checked with the hardware author: reading REG_CEC_RX_DATA is not
>> destructive, only the explicit write-0 to REG_CEC_RX_DATA_LEN
>> clears state. The v4 changelog's "likely clear-on-read" assumption
>> for this same register was wrong.
>> - devm_kzalloc'd state + subdev devnode UAF-in-the-abstract on unbind
>> -- same reasoning as the v2/v3 rounds: matches tc358743.c/adv7604.c/
>> adv7842.c exactly, an accepted V4L2-subsystem-wide pattern, not a
>> defect unique to this driver.
>> - Concurrent VIDIOC_SUBDEV_S_EDID racing it6625_remove()'s
>> cancel_delayed_work_sync()/mutex_destroy() -- the same residual risk
>> dismissed in the v3 round: real in the abstract, but no peer driver
>> adds extra synchronization for it either, and the v2 round's fix
>> (unregister the subdev first in remove()) already narrowed this to
>> the same residual risk level every peer driver carries.
>>
>> Changes in v4:
>> - Always acknowledge the CEC RX length register once the RX interrupt
>> branch is entered, even when the reported length is invalid --
>> previously the ack write only happened on valid lengths, so an
>> invalid length left the interrupt asserted, which could storm the
>> IRQ line.
>> - Fall back to a default format instead of returning -EINVAL from
>> it6625_set_fmt() when the requested mbus code isn't recognized,
>> matching the V4L2 subdev set_fmt convention (cf. adv7604.c). Take
>> it6625_lock around it6625_get_fmt()'s ACTIVE-path reads of
>> csi_format/mbus_fmt_code, which could otherwise observe a torn
>> update from a concurrent set_fmt().
>> - Fix EDID ioctl spec conformance: it6625_g_edid() now clamps the
>> requested block count and returns success instead of propagating an
>> error when the request exceeds capacity; it6625_s_edid() now returns
>> -E2BIG (not -EINVAL) with blocks set to the maximum, without
>> attempting the write, when the request exceeds capacity; and
>> it6625_read_edid()/it6625_write_edid()'s low-level bounds check now
>> tests start_block and num_blocks individually before summing them,
>> since the sum could otherwise overflow given the unsigned __u32
>> fields of struct v4l2_edid.
>> - Harden the mipi_reg debugfs handlers: it6625_mipi_reg_write() now
>> always parses from a local offset that starts at 0 instead of the
>> real file offset, since this is a stateless one-write-one-command
>> interface, not a seekable stream; it6625_mipi_reg_show() now takes
>> it6625_lock around its raw register-bank read, matching every other
>> banked-register access in the driver (the dumped range also includes
>> REG_CEC_RX_DATA, which the CEC handler treats as clear-on-read, so an
>> unlocked read could silently steal a pending CEC RX message).
>> - Mark the HDMI info control (IT6625_CID_HDMI_INFO) volatile:
>> it6625_g_volatile_ctrl() already had a live case for it, but the
>> control's v4l2_ctrl_config was missing V4L2_CTRL_FLAG_VOLATILE, so
>> that case was never actually reached and the control's value was
>> never refreshed.
>>
>> All found by the sashiko.dev automated review of v3.
>>
>> Separately, not a review finding: remove a redundant FW-start register
>> trigger from it6625_initial_setup() -- the chip's firmware starts
>> automatically on power-on, so the explicit software trigger was dead
>> weight.
>>
>> Changes in v3:
>> - Finish the banked-register locking from v2: it6625_irq_infoframe_latch()
>> and get_audio_sampling_rate() also read banked registers without
>> it6625_lock; take the lock inside the CEC RX branch across the
>> length read, data read and length-clear together (closing a TOCTOU
>> where the length could go stale under lock contention), and stop the
>> RX branch from using an early return that skipped a simultaneous CEC
>> TX interrupt.
>> - Fix it6625_s_edid() incorrectly treating a successful EDID write as
>> a failure: it6625_write_edid() returns a positive block count on
>> success, not 0, so `if (err)` skipped the CEC phys-addr update and
>> HPD re-enable on every successful write. Pre-existing since v1, not
>> introduced by the v2 blocks==0 handling.
>> - Add real V4L2_SUBDEV_FORMAT_TRY support in get_fmt/set_fmt via
>> sd_state, matching the pattern used by adv7604.c, and lock the
>> csi_format/mbus_fmt_code commit in the ACTIVE path against
>> concurrent S_FMT calls.
>> - Read the chip's real porch/sync-width registers
>> (REG_H_FP_1..REG_V_BP_0) in it6625_get_detected_timings() instead of
>> lumping the whole blanking interval into hsync/vsync, so detected
>> timings can actually match the standard CEA/VESA tables.
>>
>> All found by the sashiko.dev automated review of v2. Four other v2
>> review findings were checked against the code and are not being acted
>> on:
>> - "timer_container_of() doesn't exist" -- it does
>> (include/linux/timer.h), the driver already builds clean with it.
>> - "devm_kzalloc'd state + subdev devnode risks a UAF on unbind" --
>> true in the abstract, but it's the exact pattern used by every
>> comparable in-tree driver (tc358743.c, adv7604.c, adv7842.c all
>> devm_kzalloc their main state struct despite setting
>> V4L2_SUBDEV_FL_HAS_DEVNODE too).
>> - "unregistering the subdev before disabling IRQ/work in remove()
>> leaves sd->devnode dangling" -- checked the core:
>> v4l2_device_unregister_subdev() only marks sd->devnode unregistered
>> via video_unregister_device(), it does not free it; the struct is
>> freed later via kref on last close, so the pointer stays valid.
>> - the residual "ioctl reschedules hpd_delayed_work concurrently with
>> remove()" race -- real in the abstract (requires an ioctl already
>> in flight at the exact moment of unbind), but no peer driver adds
>> extra synchronization for this either (none set sd->devnode->lock),
>> and the v2 fix (unregister first in remove()) already narrowed this
>> down to the same residual risk level every peer driver carries.
>>
>> Changes in v2:
>> - Fix premature v4l2_async_register_subdev(): move it to the end of
>> probe() (after ctrl handler, CEC and initial hardware setup), and
>> mirror the unregister order at the top of remove(), closing a
>> use-after-free window where a concurrent ioctl could reschedule
>> hpd_delayed_work during teardown.
>> - Take it6625_lock around the banked register reads in
>> it6625_get_detected_timings() and the CEC RX path in
>> it6625_cec_handler(), which could otherwise race with an in-progress
>> EDID read/write and observe the wrong bank.
>> - Fix it6625_wait_for_status()'s poll interval exceeding the timeout
>> budget, split the CEC RX/TX interrupt handling out of an else-if so
>> simultaneous events aren't dropped, and wait for hardware to latch
>> the CEC transmit trigger bit before releasing the lock.
>> - Stop it6625_set_fmt() from mutating active state on
>> V4L2_SUBDEV_FORMAT_TRY, and handle edid->blocks == 0 in g_edid/s_edid
>> per the V4L2 spec (capacity query / EDID clear).
>> - Zero-initialize the CEC rxmsg and debugfs mipi_reg write buffer to
>> avoid leaking uninitialized stack bytes, and use designated
>> initializers in the i2c_device_id table.
>>
>> All found by the sashiko.dev automated review of v1.
>> ---
>> MAINTAINERS | 7 +
>> drivers/media/i2c/Kconfig | 18 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/it6625.c | 2231 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 2257 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 15011f5752a994cf1b354f490d6c4e411588df88..2d5b0ee107f9e3255a5365ec0a25167a716b1e6e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13847,6 +13847,13 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
>> F: Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
>> F: drivers/gpu/drm/bridge/ite-it66121.c
>>
>> +ITE IT6625 HDMI to MIPI MEDIA DRIVER
>> +M: Hermes Wu <Hermes.Wu@ite.com.tw>
>> +S: Maintained
>> +T: git git://linuxtv.org/media.git
>> +F: Documentation/devicetree/bindings/media/i2c/ite,it6625.yaml
>> +F: drivers/media/i2c/it6625.c
>> +
>> IVTV VIDEO4LINUX DRIVER
>> M: Andy Walls <awalls@md.metrocast.net>
>> L: linux-media@vger.kernel.org
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 5d173e0ecf424f2f204f8d426be818e44357f8e4..fb4b24684f7710f90eb399976d11f57701488d26 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -1278,6 +1278,24 @@ config VIDEO_ISL7998X
>> Support for Intersil ISL7998x analog to MIPI-CSI2 or
>> BT.656 decoder.
>>
>> +config VIDEO_IT6625
>> + tristate "IT6625 HDMI to MIPI CSI bridge"
>> + depends on VIDEO_DEV && I2C
>> + depends on OF
>> + select CEC_CORE
>> + select MEDIA_CONTROLLER
>> + select REGMAP_I2C
>> + select V4L2_FWNODE
>> + select VIDEO_V4L2_SUBDEV_API
>> + help
>> + V4L2 subdevice driver for the ITE IT6625/IT6626 HDMI to MIPI
>> + CSI-2 bridge chips. IT6625 accepts an HDMI 2.0 input and
>> + IT6626 an HDMI 2.1 input, converting it to a MIPI CSI-2
>> + output. The driver also exposes an HDMI CEC adapter.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called it6625.
>> +
>> config VIDEO_LT6911UXE
>> tristate "Lontium LT6911UXE decoder"
>> depends on ACPI && VIDEO_DEV && I2C
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index e45359efe0e41e13e3c0869e5ead7d6cf4aca3a7..9bbceaa3d07573958edb8a36bc7f0942f1410a39 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_IMX412) += imx412.o
>> obj-$(CONFIG_VIDEO_IMX415) += imx415.o
>> obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
>> obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
>> +obj-$(CONFIG_VIDEO_IT6625) += it6625.o
>> obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
>> obj-$(CONFIG_VIDEO_LM3560) += lm3560.o
>> obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
>> diff --git a/drivers/media/i2c/it6625.c b/drivers/media/i2c/it6625.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..c47422edeba7969ec3c94594a95a97c6170f49f2
>> --- /dev/null
>> +++ b/drivers/media/i2c/it6625.c
>> @@ -0,0 +1,2231 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * it6625 - ite HDMI to MIPI bridge
>> + */
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/hdmi.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/timer.h>
>> +#include <linux/v4l2-dv-timings.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include <media/cec.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-dv-timings.h>
>> +#include <media/v4l2-event.h>
>> +#include <media/v4l2-fwnode.h>
>> +
>> +static int debug = 3;
>> +module_param(debug, int, 0644);
>> +MODULE_PARM_DESC(debug, "debug level (0-3)");
>> +
>> +#define REG_CHIP_ID_0 0x00
>> +#define REG_CHIP_ID_1 0x01
>> +#define REG_FW_VER_MAJOR 0x03
>> +#define REG_FW_VER_MINOR 0x04
>> +#define REG_PROTOCOL_VERSION 0x05
>> +#define B_PVER_MINOR BIT(0)
>> +#define B_PVER_MAJOR BIT(4)
>> +
>> +#define REG_CMD_SET 0x10
>> +#define CMD_SET_CEC_LA 0xC0
>> +#define CMD_SET_CEC_ENABLE 0xC1
>> +
>> +#define REG_EDID_START 0x20
>> +#define REG_CEC_RX_DATA 0x20
>> +#define REG_CEC_TX_DATA 0x30
>> +
>> +#define REG_H_ACTIVE_1 0x50
>> +#define REG_H_ACTIVE_0 0x51
>> +#define REG_V_ACTIVE_1 0x52
>> +#define REG_V_ACTIVE_0 0x53
>> +#define REG_H_TOTAL_1 0x54
>> +#define REG_H_TOTAL_0 0x55
>> +#define REG_V_TOTAL_1 0x56
>> +#define REG_V_TOTAL_0 0x57
>> +#define REG_VID_PCLK 0x58
>> +#define REG_H_FP_1 0x5C
>> +#define REG_H_FP_0 0x5D
>> +#define REG_H_SW_1 0x5E
>> +#define REG_H_SW_0 0x5F
>> +#define REG_H_BP_1 0x60
>> +#define REG_H_BP_0 0x61
>> +#define REG_V_FP_1 0x62
>> +#define REG_V_FP_0 0x63
>> +#define REG_V_SW_1 0x64
>> +#define REG_V_SW_0 0x65
>> +#define REG_V_BP_1 0x66
>> +#define REG_V_BP_0 0x67
>> +#define REG_VID_INFO 0x68
>> +#define B_INTERLACE BIT(0)
>> +#define B_HSWPOL BIT(1)
>> +#define B_VSWPOL BIT(2)
>> +#define B_PIXEL_REP BIT(4)
>> +
>> +#define REG_VIC 0x69
>> +#define REG_HDMI_VIDEO_INFO 0x6A
>> +#define B_COLOR_MODE BIT(0)
>> +#define B_COLOR_DEPTH BIT(4)
>> +#define REG_HDMI_AUDIO_INFO1 0x6B
>> +#define B_AUD_FS BIT(0)
>> +
>> +#define REG_HDMI_AUDIO_INFO2 0x6C
>> +#define B_AUD_CH BIT(0)
>> +#define B_AUD_WL BIT(4)
>> +
>> +#define REG_HDMI_AUDIO_INFO3 0x6D
>> +#define B_AUD_TYPE BIT(0)
>> +#define B_AUD_MS BIT(2)
>> +#define B_AUD_3D BIT(3)
>> +
>> +#define REG_AUDIO_FMT 0x6E
>> +#define B_I2S_WL BIT(0)
>> +#define B_I2S_ALN BIT(2)
>> +#define B_I2S_DLY BIT(3)
>> +#define B_I2S_LR BIT(4)
>> +#define B_I2S_SFT BIT(5)
>> +#define B_AUD_OUT_IF BIT(6)
>> +
>> +#define REG_IF_LATCH_HB 0x6F
>> +#define REG_IF_DATA 0x70
>> +#define REG_EMP_DATA 0xA0
>> +#define REG_AVI_DATA 0xAE
>> +
>> +#define REG_TX_STATUS 0xE0
>> +
>> +#define REG_RX_STATUS 0xE2
>> +#define B_RX_5V BIT(0)
>> +#define B_RX_HPD BIT(1)
>> +#define B_RX_STABLE BIT(2)
>> +#define B_RX_HDMI BIT(3)
>> +#define B_RX_AVMUTE BIT(4)
>> +#define B_RX_AUD_ON BIT(5)
>> +
>> +#define REG_RX_HDCP_STS 0xE3
>> +#define B_HDCP1_AUTH_START BIT(0)
>> +#define B_HDCP2_AUTH_START BIT(1)
>> +#define B_HDCP_AUTH_DONE BIT(2)
>> +#define B_HDCP_ENC BIT(3)
>> +
>> +#define REG_CEC_STATUS 0xE4
>> +#define B_CEC_TX_DONE BIT(0)
>> +#define B_CEC_TX_NACK BIT(1)
>> +
>> +#define REG_CEC_RX_DATA_LEN 0xE5
>> +#define REG_CEC_TX_DATA_LEN 0xE6
>> +
>> +#define REG_SYS_MIPI_INT 0xEB
>> +#define B_MIPI_OUTPUT_ENABLE BIT(0)
>> +#define B_MIPI_VIDEO_UNSTABLE BIT(1)
>> +
>> +#define REG_RX_INT_STATUS1 0xEC
>> +#define B_HDMI_5V_CHG BIT(0)
>> +#define B_HDMI_VID_CHG BIT(1)
>> +#define B_HDMI_AUD_CHG BIT(2)
>> +#define B_HDMI_CP_CHG BIT(3)
>> +#define B_HDMI_IF_LATCH BIT(4)
>> +#define B_HDMI_NO_IF_LATCH BIT(5)
>> +#define B_HDMI_EMP BIT(6)
>> +#define B_HDMI_NO_EMP BIT(7)
>> +
>> +#define REG_RX_INT_STATUS2 0xED
>> +#define B_HDMI_AVI BIT(0)
>> +#define B_HDMI_NO_AVI BIT(1)
>> +#define B_HDMI_VSIF BIT(2)
>> +#define B_HDMI_NO_VSIF BIT(3)
>> +#define B_HDMI_AVMUTE_CHG BIT(4)
>> +
>> +#define REG_CHIP_CONTROL 0xF0
>> +#define B_HDMI_RESET BIT(5)
>> +#define B_FW_START BIT(6)
>> +
>> +#define REG_MIPI_CFG 0xF1
>> +#define M_MIPI_LANE 0x03
>> +#define B_MIPI_SPLIT BIT(2)
>> +#define B_MIPI_SPLIT_CFG BIT(3)
>> +#define B_MIPI_DPHY BIT(4)
>> +#define B_MIPI_USE_DSI BIT(5)
>> +#define B_MIPI_CONTINU_CLK BIT(6)
>> +
>> +#define REG_MIPI_DATA_TYPE 0xF2
>> +#define CSI_RGB444 0x20
>> +#define CSI_RGB555 0x21
>> +#define CSI_RGB565 0x22
>> +#define CSI_RGB666 0x23
>> +#define CSI_RGB888 0x24
>> +#define CSI_YUV420_8b_L 0x1A
>> +#define CSI_YUV420_8b 0x1C
>> +#define CSI_YUV420_10b 0x1D
>> +#define CSI_YUV422_8b 0x1E
>> +#define CSI_YUV422_10b 0x1F
>> +#define CSI_RGB_10b 0x30
>> +#define CSI_RGB_12b 0x31
>> +#define CSI_YUV422_12b 0x32
>> +#define CSI_YUV420_10b_L 0x33
>> +#define CSI_YUV420_12b 0x34
>> +#define CSI_YUV444_8b 0x35
>> +#define CSI_YUV444_10b 0x36
>> +#define CSI_YUV444_12b 0x37
>> +
>> +#define REG_MIPI_CONTROL 0xF3
>> +#define B_MIPI_OUTPUT BIT(0)
>> +
>> +#define REG_RX_CFG 0xF4
>> +#define B_MANUAL_HPD BIT(0)
>> +#define B_HPD_HIGH BIT(1)
>> +#define B_HPD_TOGGLE BIT(3)
>> +
>> +#define REG_CSC_CFG 0xF5
>> +#define B_DYNAMIC_RANGE BIT(0)
>> +
>> +#define REG_MISC_CFG 0xF6
>> +#define B_BAUD_RATE BIT(0)
>> +#define B_DEBUG_MSG BIT(1)
>> +
>> +#define REG_HPD_DELAY 0xF7
>> +#define B_DELAY_COUNT BIT(0)
>> +#define B_DELAY_UNIT BIT(7)
>> +
>> +#define REG_INFO_BANK_SEL 0xFD
>> +#define CTL_BANK_EDID_READ 1
>> +#define CTL_BANK_EDID_WRITE 5
>> +
>> +#define REG_HOST_CTRL_INT 0xFE
>> +#define B_CMD_SET BIT(4)
>> +#define B_CEC_SEND_DATA BIT(5)
>> +#define B_CONFIG_UPDATE BIT(6)
>> +#define B_IF_BANK BIT(7)
>> +
>> +#define REG_MCU_INTERRUPT 0xFF
>> +#define B_SYS_INT_ACTIVE BIT(0)
>> +#define B_CEC_RX_RECEIVED BIT(1)
>> +#define B_CEC_TX_UPDATE BIT(2)
>> +
>> +#define EDID_NUM_BLOCKS_MAX 4
>> +#define EDID_BLOCK_SIZE 128
>> +
>> +#define I2C_MAX_XFER_SIZE 8
>> +#define POLL_INTERVAL_CEC_MS 10
>> +#define POLL_INTERVAL_MS 40
>> +
>> +#define AUD32K 0x03
>> +#define AUD44K 0x00
>> +#define AUD48K 0x02
>> +#define AUD64K 0x0B
>> +#define AUD88K 0x08
>> +#define AUD96K 0x0A
>> +#define AUD128K 0x2B
>> +#define AUD176K 0x0C
>> +#define AUD192K 0x0E
>> +#define AUD256K 0x1B
>> +#define AUD352K 0x0D
>> +#define AUD384K 0x05
>> +#define AUD512K 0x3B
>> +#define AUD705K 0x2D
>> +#define AUD768K 0x09
>> +#define AUD1024K 0x35
>> +#define AUD1411K 0x1D
>> +#define AUD1536K 0x15
>> +
>> +enum it6625_chip_type {
>> + IT6625_CHIP = 0,
>> + IT6626_CHIP = 1,
>> +};
>> +
>> +struct it6625 {
>> + struct device *dev;
>> + struct i2c_client *i2c_client;
>> + struct regmap *it6625_regmap;
>> + enum it6625_chip_type chip_type;
>> +
>> + /* protects concurrent access to the chip's registers and state */
>> + struct mutex it6625_lock;
>> +
>> + struct v4l2_subdev sd;
>> + struct v4l2_mbus_config_mipi_csi2 bus;
>> + struct video_device *vdev;
>> + struct media_pad pad;
>> + struct v4l2_ctrl_handler hdl;
>> +
>> + /* controls */
>> + struct v4l2_ctrl *ctrl_5v_detect;
>> + struct v4l2_ctrl *ctrl_audio_sampling_rate;
>> + struct v4l2_ctrl *ctrl_audio_present;
>> + struct v4l2_ctrl *ctrl_link_freq;
>> + struct v4l2_ctrl *ctrl_get_hdmi_info;
>> + struct v4l2_ctrl *ctrl_mipi_config;
>> +
>> + struct delayed_work hpd_delayed_work;
>> +
>> + struct timer_list timer;
>> + struct work_struct polling_work;
>> +
>> + struct v4l2_dv_timings timings;
>> +
>> + u8 csi_lanes;
>> + u8 port_num;
>> + enum v4l2_mbus_type bus_type;
>> + u8 csi_format;
>> + u32 mbus_fmt_code;
>> +
>> + struct gpio_desc *reset_gpio;
>> +
>> + struct cec_adapter *cec_adap;
>> +
>> + struct dentry *debugfs_dir;
>> +};
>> +
>> +static const s64 it6625_link_freq[] = { 445500000 };
>> +
>> +static const struct v4l2_dv_timings_cap it6625_timings_cap = {
>> + .type = V4L2_DV_BT_656_1120,
>> + /* keep this initialization for compatibility with GCC < 4.4.6 */
>> + .reserved = { 0 },
>> +
>> + V4L2_INIT_BT_TIMINGS(640, 3840, 480, 2160, 27000000, 300000000,
>> + V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
>> + V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
>> + V4L2_DV_BT_CAP_PROGRESSIVE | V4L2_DV_BT_CAP_INTERLACED |
>> + V4L2_DV_BT_CAP_REDUCED_BLANKING | V4L2_DV_BT_CAP_CUSTOM)
>> +};
>> +
>> +static const struct it6625_format_info {
>> + u8 csi_format;
>> + u32 mbus_fmt_code;
>> +} it6625_formats[] = {
>> + { CSI_YUV422_8b, MEDIA_BUS_FMT_UYVY8_1X16 },
>> + { CSI_RGB888, MEDIA_BUS_FMT_RGB888_1X24 },
>> + { CSI_YUV444_8b, MEDIA_BUS_FMT_YUV8_1X24 },
>> +};
>> +
>> +static inline int it6625_csi_format_idx(u8 csi_format)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(it6625_formats); i++) {
>> + if (it6625_formats[i].csi_format == csi_format)
>> + return i;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static inline int it6625_csi_mbus_code_idx(u32 mbus_fmt_code)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(it6625_formats); i++) {
>> + if (it6625_formats[i].mbus_fmt_code == mbus_fmt_code)
>> + return i;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static inline struct it6625 *sd_to_6625(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct it6625, sd);
>> +}
>> +
>> +static inline struct it6625 *hdl_to_6625(struct v4l2_ctrl_handler *hdl)
>> +{
>> + return container_of(hdl, struct it6625, hdl);
>> +}
>> +
>> +static const struct regmap_config it6625_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .max_register = 0xff,
>> + .cache_type = REGCACHE_NONE,
>> + .max_raw_read = I2C_MAX_XFER_SIZE,
>> + .max_raw_write = I2C_MAX_XFER_SIZE,
>> +};
>> +
>> +static int it6625_regmap_i2c_init(struct i2c_client *client,
>> + struct it6625 *it6625)
>> +{
>> + it6625->i2c_client = client;
>> + it6625->dev = &client->dev;
>> +
>> + it6625->it6625_regmap = devm_regmap_init_i2c(it6625->i2c_client,
>> + &it6625_regmap_config);
>> + if (IS_ERR(it6625->it6625_regmap))
>> + return PTR_ERR(it6625->it6625_regmap);
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_read_byte(struct it6625 *it6625, u8 reg)
>> +{
>> + unsigned int val;
>> + int err;
>> + struct device *dev = it6625->dev;
>> +
>> + err = regmap_read(it6625->it6625_regmap, reg, &val);
>> + if (err < 0) {
>> + dev_err(dev, "reg[0x%x] read failed err: %d", reg, err);
>> + return err;
>> + }
>> +
>> + return val;
>> +}
>> +
>> +static int it6625_write_byte(struct it6625 *it6625, u8 reg, u8 val)
>> +{
>> + int err;
>> + struct device *dev = it6625->dev;
>> +
>> + err = regmap_write(it6625->it6625_regmap, reg, val);
>> + if (err < 0) {
>> + dev_err(dev, "reg[0x%x] write failed err: %d", reg, err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_set_bits(struct it6625 *it6625, u8 reg, u8 mask, u8 val)
>> +{
>> + int err;
>> + struct device *dev = it6625->dev;
>> +
>> + err = regmap_update_bits(it6625->it6625_regmap, reg, mask, val);
>> + if (err < 0) {
>> + dev_err(dev, "reg[0x%x] set bits failed err: %d", reg, err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_read_bytes(struct it6625 *it6625, u8 reg, u8 *buf, int len)
>> +{
>> + int err;
>> + struct device *dev = it6625->dev;
>> +
>> + err = regmap_bulk_read(it6625->it6625_regmap, reg, buf, len);
>> + if (err < 0) {
>> + dev_err(dev, "reg[0x%x] read failed err: %d", reg, err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_write_bytes(struct it6625 *it6625, u8 reg, u8 *buf, int len)
>> +{
>> + int err;
>> + struct device *dev = it6625->dev;
>> +
>> + err = regmap_bulk_write(it6625->it6625_regmap, reg, buf, len);
>> + if (err < 0) {
>> + dev_err(dev, "reg[0x%x] write failed err: %d", reg, err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_wait_for_status(struct it6625 *it6625, u8 reg, u8 val,
>> + int timeout_ms)
>> +{
>> + struct device *dev = it6625->dev;
>> + int status;
>> + int rval;
>> + int sleep_ms = 10;
>> + int timeout_round_ms = DIV_ROUND_UP(timeout_ms, sleep_ms) * sleep_ms;
>> +
>> + status = read_poll_timeout(it6625_read_byte, rval, rval == val,
>> + sleep_ms * 1000,
>> + timeout_round_ms * 1000,
>> + false, it6625, reg);
>> +
>> + dev_info(dev, "%s status = %d %d", __func__, status, (int)rval);
>> + if (status < 0) {
>> + dev_err(dev, "%s err status = %d", __func__, status);
>> + return -ETIMEDOUT;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void it6625_write_command(struct it6625 *it6625, u8 *cmds, int cmd_len)
>> +{
>> + it6625_write_bytes(it6625, REG_CMD_SET, cmds, cmd_len);
>> + it6625_write_byte(it6625, REG_HOST_CTRL_INT, B_CMD_SET);
>> + it6625_wait_for_status(it6625, REG_HOST_CTRL_INT, 0x00, 25);
>> +}
>> +
>> +static int it6625_update_config(struct it6625 *it6625)
>> +{
>> + int err;
>> +
>> + err = it6625_set_bits(it6625, REG_HOST_CTRL_INT,
>> + B_CONFIG_UPDATE, B_CONFIG_UPDATE);
>> + if (err < 0)
>> + return err;
>> +
>> + return it6625_wait_for_status(it6625, REG_HOST_CTRL_INT, 0x00, 25);
>> +}
>> +
>> +static int it6625_set_bank(struct it6625 *it6625, u8 bank)
>> +{
>> + int err;
>> +
>> + err = it6625_write_byte(it6625, REG_INFO_BANK_SEL, bank);
>> + if (err < 0)
>> + return err;
>> +
>> + err = it6625_write_byte(it6625, REG_HOST_CTRL_INT, B_IF_BANK);
>> + if (err < 0)
>> + return err;
>> +
>> + return it6625_wait_for_status(it6625, REG_HOST_CTRL_INT, 0x00, 25);
>> +}
>> +
>> +static inline bool is_hdmi(struct it6625 *it6625)
>> +{
>> + int val;
>> +
>> + val = it6625_read_byte(it6625, REG_RX_STATUS);
>> + return (val < 0) ? false : (val & B_RX_HDMI);
>> +}
>> +
>> +static inline bool hdmi_5v_power_present(struct it6625 *it6625)
>> +{
>> + int val;
>> +
>> + val = it6625_read_byte(it6625, REG_RX_STATUS);
>> + return (val < 0) ? false : (val & B_RX_5V);
>> +}
>> +
>> +static inline bool no_signal(struct it6625 *it6625)
>> +{
>> + int val;
>> +
>> + val = it6625_read_byte(it6625, REG_RX_STATUS);
>> + return (val < 0) ? true : !(val & B_RX_STABLE);
>> +}
>> +
>> +static inline bool audio_present(struct it6625 *it6625)
>> +{
>> + int val;
>> +
>> + val = it6625_read_byte(it6625, REG_RX_STATUS);
>> + return (val < 0) ? false : (val & B_RX_AUD_ON);
>> +}
>> +
>> +static int get_audio_sampling_rate(struct it6625 *it6625)
>> +{
>> + u8 fs_id;
>> + int i, freq = 0;
>> + const struct fs_id_map {
>> + u8 fs_id;
>> + u16 freq;
>> + } s_fsid_map[] = {
>> + { AUD32K, 32 },
>> + { AUD44K, 44 },
>> + { AUD48K, 48 },
>> + { AUD64K, 64 },
>> + { AUD88K, 88 },
>> + { AUD96K, 96 },
>> +
>> + { AUD128K, 128 },
>> + { AUD176K, 176 },
>> + { AUD192K, 192 },
>> + { AUD256K, 256 },
>> + { AUD352K, 352 },
>> + { AUD384K, 384 },
>> +
>> + { AUD512K, 512 },
>> + { AUD705K, 705 },
>> + { AUD768K, 768 },
>> + { AUD1024K, 1024 },
>> + { AUD1411K, 1411 },
>> + { AUD1536K, 1536 },
>> + };
>> +
>> + if (no_signal(it6625) || !audio_present(it6625))
>> + return 0;
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> +
>> + fs_id = it6625_read_byte(it6625, REG_HDMI_AUDIO_INFO1);
>> +
>> + for (i = 0; i < ARRAY_SIZE(s_fsid_map); i++) {
>> + if (s_fsid_map[i].fs_id == fs_id) {
>> + freq = s_fsid_map[i].freq * 1000;
>> + break;
>> + }
>> + }
>> +
>> + return freq;
>> +}
>> +
>> +static u64 it6625_get_pclk(struct it6625 *it6625)
>> +{
>> + u32 pclk;
>> + u8 ck[4];
>> + int ret;
>> +
>> + ret = it6625_read_bytes(it6625, REG_VID_PCLK, ck, 4);
>> + if (ret < 0) {
>> + dev_err(it6625->dev, "failed to read pixel clock");
>> + return 0;
>> + }
>> +
>> + pclk = ck[0];
>> + pclk <<= 8;
>> + pclk |= ck[1];
>> + pclk <<= 8;
>> + pclk |= ck[2];
>> + pclk <<= 8;
>> + pclk |= ck[3];
>> +
>> + v4l2_dbg(1, debug, &it6625->sd, "%s: pclk=%u (%08x)",
>> + __func__, pclk, pclk);
>> +
>> + return (u64)pclk * 1000;
>> +}
>> +
>> +static int it6625_read_edid(struct it6625 *it6625, u8 *edid, int start_block,
>> + int num_blocks)
>> +{
>> + int i, bank_ctrl, err = 0;
>> + struct device *dev = it6625->dev;
>> +
>> + if (!edid) {
>> + dev_err(dev, "edid buffer is NULL");
>> + return -EINVAL;
>> + }
>> +
>> + if (start_block < 0 || num_blocks <= 0 ||
>> + start_block > EDID_NUM_BLOCKS_MAX ||
>> + num_blocks > EDID_NUM_BLOCKS_MAX ||
>> + start_block + num_blocks > EDID_NUM_BLOCKS_MAX) {
>> + dev_err(dev,
>> + "invalid block range: start_block=%d, num_blocks=%d",
>> + start_block, num_blocks);
>> + return -EINVAL;
>> + }
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + for (i = 0; i < num_blocks; i++) {
>> + bank_ctrl = CTL_BANK_EDID_READ + start_block + i;
>> + err = it6625_set_bank(it6625, bank_ctrl);
>> + if (err < 0)
>> + break;
>> +
>> + err = it6625_read_bytes(it6625, REG_EDID_START,
>> + edid + (i * 128), 128);
>> + if (err < 0)
>> + break;
>> + }
>> +
>> + it6625_set_bank(it6625, 0);
>> +
>> + return err < 0 ? err : num_blocks;
>> +}
>> +
>> +static int it6625_write_edid(struct it6625 *it6625, u8 *edid, int start_block,
>> + int num_blocks)
>> +{
>> + int i, bank_ctrl, err = 0;
>> + struct device *dev = it6625->dev;
>> +
>> + if (start_block < 0 || num_blocks <= 0 ||
>> + start_block > EDID_NUM_BLOCKS_MAX ||
>> + num_blocks > EDID_NUM_BLOCKS_MAX ||
>> + start_block + num_blocks > EDID_NUM_BLOCKS_MAX) {
>> + dev_err(dev,
>> + "invalid block range: start_block=%d, num_blocks=%d",
>> + start_block, num_blocks);
>> + return -EINVAL;
>> + }
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + for (i = 0; i < num_blocks; i++) {
>> + bank_ctrl = CTL_BANK_EDID_WRITE + start_block + i;
>> + err = it6625_set_bank(it6625, bank_ctrl);
>> + if (err < 0)
>> + break;
>> +
>> + err = it6625_write_bytes(it6625, REG_EDID_START,
>> + edid + (i * 128), 128);
>> + if (err < 0)
>> + break;
>> +
>> + err = it6625_update_config(it6625);
>> + if (err < 0)
>> + break;
>> + }
>> +
>> + it6625_set_bank(it6625, 0);
>> +
>> + return err < 0 ? err : num_blocks;
>> +}
>> +
>> +static void it6625_enable_auto_hpd(struct it6625 *it6625)
>> +{
>> + dev_dbg(it6625->dev, "%s: auto HPD", __func__);
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_set_bits(it6625, REG_RX_CFG, 0x03, 0x00);
>> + it6625_update_config(it6625);
>> +}
>> +
>> +static void it6625_disable_hpd(struct it6625 *it6625)
>> +{
>> + cancel_delayed_work_sync(&it6625->hpd_delayed_work);
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_set_bits(it6625, REG_RX_CFG, 0x03, 0x01);
>> + it6625_update_config(it6625);
>> +}
>> +
>> +static void it6625_enable_hpd(struct it6625 *it6625)
>> +{
>> + schedule_delayed_work(&it6625->hpd_delayed_work,
>> + msecs_to_jiffies(100));
>
>I recommend to use 'HZ / 7' here. Often video sources are not very accurate
>in measuring how long the HPD is low. 'HZ / 7' has proven to be a reliable value.
>
>> +}
>> +
>> +static void it6625_hpd_delayed_work(struct work_struct *work)
>> +{
>> + struct it6625 *it6625 = container_of(work,
>> + struct it6625, hpd_delayed_work.work);
>> + int val = 0;
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_set_bits(it6625, REG_RX_CFG, 0x03, val);
>> + it6625_update_config(it6625);
>> +}
>> +
>> +static int it6625_get_detected_timings(struct it6625 *it6625,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + struct v4l2_bt_timings *bt = &timings->bt;
>> + int val;
>> + unsigned int width, height;
>> + u8 buffer[4];
>> + u8 buffer2[12];
>> +
>> + if (no_signal(it6625)) {
>> + dev_err(it6625->dev, "no signal detected");
>> + return -ENOLINK;
>> + }
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> +
>> + memset(timings, 0, sizeof(struct v4l2_dv_timings));
>> + timings->type = V4L2_DV_BT_656_1120;
>> + val = it6625_read_byte(it6625, REG_VID_INFO);
>> + if (val < 0) {
>> + dev_err(it6625->dev, "failed to read video info");
>> + return -EIO;
>> + }
>> +
>> + bt->interlaced = val & B_INTERLACE ?
>> + V4L2_DV_INTERLACED : V4L2_DV_PROGRESSIVE;
>> +
>> + if (it6625_read_bytes(it6625, REG_H_ACTIVE_1, buffer, 4) < 0)
>> + return -EIO;
>> +
>> + width = ((buffer[0] & 0xff) << 8) + buffer[1];
>> + height = ((buffer[2] & 0xff) << 8) + buffer[3];
>> +
>> + bt->width = width;
>> + bt->height = height;
>> +
>> + if (it6625_read_bytes(it6625, REG_H_FP_1, buffer2, 12) < 0)
>> + return -EIO;
>> +
>> + bt->hfrontporch = ((buffer2[0] & 0xff) << 8) + buffer2[1];
>> + bt->hsync = ((buffer2[2] & 0xff) << 8) + buffer2[3];
>> + bt->hbackporch = ((buffer2[4] & 0xff) << 8) + buffer2[5];
>> + bt->vfrontporch = ((buffer2[6] & 0xff) << 8) + buffer2[7];
>> + bt->vsync = ((buffer2[8] & 0xff) << 8) + buffer2[9];
>> + bt->vbackporch = ((buffer2[10] & 0xff) << 8) + buffer2[11];
>> +
>> + bt->pixelclock = it6625_get_pclk(it6625);
>> + if (bt->interlaced == V4L2_DV_INTERLACED) {
>> + bt->height *= 2;
>> + bt->il_vsync = bt->vsync + 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void it6625_show_avi_infoframe(struct it6625 *it6625)
>> +{
>> + struct device *dev = it6625->dev;
>> + union hdmi_infoframe frame;
>> + u8 buffer[HDMI_INFOFRAME_SIZE(AVI)];
>> + u8 ver, len;
>> + int ret;
>> +
>> + if (!is_hdmi(it6625)) {
>> + dev_err(dev, "not HDMI signal, skip AVI infoframe log");
>> + return;
>> + }
>> +
>> + ret = it6625_read_bytes(it6625, REG_AVI_DATA, buffer + 1,
>> + HDMI_INFOFRAME_SIZE(AVI) - 1);
>> + if (ret < 0) {
>> + dev_err(dev, "failed to read AVI infoframe data");
>> + return;
>> + }
>> +
>> + len = buffer[1];
>> + ver = buffer[2];
>> +
>> + buffer[0] = HDMI_INFOFRAME_TYPE_AVI;
>> + buffer[1] = ver;
>> + buffer[2] = len;
>> +
>> + ret = hdmi_infoframe_unpack(&frame, buffer, sizeof(buffer));
>> + if (ret < 0) {
>> + dev_err(dev, "unpack of AVI infoframe failed");
>> + return;
>> + }
>> +
>> + hdmi_infoframe_log(KERN_INFO, dev, &frame);
>> +}
>> +
>> +static int it6625_s_ctrl_detect_hdmi_5v(struct v4l2_subdev *sd)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + return v4l2_ctrl_s_ctrl(it6625->ctrl_5v_detect,
>> + hdmi_5v_power_present(it6625));
>> +}
>> +
>> +static int it6625_s_ctrl_audio_sampling_rate(struct v4l2_subdev *sd)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + return v4l2_ctrl_s_ctrl(it6625->ctrl_audio_sampling_rate,
>> + get_audio_sampling_rate(it6625));
>> +}
>> +
>> +static int it6625_s_ctrl_audio_present(struct v4l2_subdev *sd)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + return v4l2_ctrl_s_ctrl(it6625->ctrl_audio_present,
>> + audio_present(it6625));
>> +}
>> +
>> +static int it6625_v4l2_sd_ctrl_update(struct v4l2_subdev *sd)
>> +{
>> + int ret = 0;
>> +
>> + ret |= it6625_s_ctrl_detect_hdmi_5v(sd);
>> + ret |= it6625_s_ctrl_audio_sampling_rate(sd);
>> + ret |= it6625_s_ctrl_audio_present(sd);
>> +
>> + return 0;
>> +}
>> +
>> +static void it6625_enable_stream_locked(struct it6625 *it6625, bool enable)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> + int val;
>> +
>> + lockdep_assert_held(&it6625->it6625_lock);
>> +
>> + v4l2_dbg(3, debug, sd, "%s: %sable",
>> + __func__, enable ? "en" : "dis");
>> +
>> + val = enable ? B_MIPI_OUTPUT : 0;
>> + it6625_set_bits(it6625, REG_MIPI_CONTROL, B_MIPI_OUTPUT, val);
>> + it6625_update_config(it6625);
>> +}
>> +
>> +static void it6625_enable_stream(struct it6625 *it6625, bool enable)
>> +{
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_enable_stream_locked(it6625, enable);
>> +}
>> +
>> +static void it6625_set_mipi_config_locked(struct it6625 *it6625, u32 cfg_val)
>> +{
>> + u8 mipi_data_type;
>> +
>> + lockdep_assert_held(&it6625->it6625_lock);
>> +
>> + dev_dbg(it6625->dev, "mipi_data_type = 0x%x", cfg_val);
>> +
>> + mipi_data_type = cfg_val & 0xFF;
>> + it6625_write_byte(it6625, REG_MIPI_DATA_TYPE, mipi_data_type);
>> + it6625_update_config(it6625);
>> +}
>> +
>> +static void it6625_set_mipi_config(struct it6625 *it6625, u32 cfg_val)
>> +{
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_set_mipi_config_locked(it6625, cfg_val);
>> +}
>> +
>> +static inline unsigned int fps_from_bt_timings(const struct v4l2_bt_timings *t)
>> +{
>> + if (!V4L2_DV_BT_FRAME_HEIGHT(t) || !V4L2_DV_BT_FRAME_WIDTH(t))
>> + return 0;
>> +
>> + return DIV_ROUND_CLOSEST((unsigned int)t->pixelclock,
>> + V4L2_DV_BT_FRAME_HEIGHT(t) *
>> + V4L2_DV_BT_FRAME_WIDTH(t));
>> +}
>> +
>> +static void it6625_initial_setup(struct it6625 *it6625)
>> +{
>> + int val = 0;
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> +
>> + /*
>> + * REG_MIPI_CFG[0:2] lane count field: 1 lane -> 0, 2 lanes -> 1,
>> + * 3 lanes (C-PHY only) -> 3, 4 lanes (D-PHY only) -> 3.
>> + */
>> + switch (it6625->csi_lanes) {
>> + case 1:
>> + val = FIELD_PREP(M_MIPI_LANE, 0);
>> + break;
>> + case 2:
>> + val = FIELD_PREP(M_MIPI_LANE, 1);
>> + break;
>> + default:
>> + val = FIELD_PREP(M_MIPI_LANE, 3);
>> + break;
>> + }
>> +
>> + if (it6625->bus_type == V4L2_MBUS_CSI2_DPHY)
>> + val |= FIELD_PREP(B_MIPI_DPHY, 1);
>> +
>> + if (it6625->port_num == 2)
>> + val |= FIELD_PREP(B_MIPI_SPLIT, 1);
>> +
>> + it6625_write_byte(it6625, REG_MIPI_CFG, val);
>> + it6625_write_byte(it6625, REG_MIPI_DATA_TYPE, it6625->csi_format);
>> + it6625_write_byte(it6625, REG_MIPI_CONTROL, 0x00);
>> + it6625_write_byte(it6625, REG_RX_CFG, 0x00);
>> +
>> + it6625_set_bits(it6625, REG_HOST_CTRL_INT, B_CONFIG_UPDATE, B_CONFIG_UPDATE);
>> + it6625_wait_for_status(it6625, REG_HOST_CTRL_INT, 0x00, 25);
>> +}
>> +
>> +static void dump_cec_msg(struct it6625 *it6625, struct cec_msg *msg, char *str)
>> +{
>> + int i;
>> +
>> + dev_dbg(it6625->dev, "%s: from %d to %d len=%d", str,
>> + msg->msg[0] >> 4, msg->msg[0] & 0x0f, msg->len);
>> + for (i = 1; i < msg->len; i++)
>> + dev_dbg(it6625->dev, " %02x", msg->msg[i]);
>> +}
>
>This shouldn't be necessary: the cec code already has CEC debugging in place.
>
>> +
>> +static int it6625_cec_adap_enable(struct cec_adapter *adap, bool enable)
>> +{
>> + struct it6625 *it6625 = adap->priv;
>> + u8 cmds[2];
>> +
>> + cmds[0] = CMD_SET_CEC_ENABLE;
>> + cmds[1] = enable ? 1 : 0;
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_write_command(it6625, cmds, sizeof(cmds));
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
>> +{
>> + struct it6625 *it6625 = adap->priv;
>> +
>> + dev_dbg(it6625->dev, "%s: la=%d", __func__, log_addr);
>> +
>> + if (log_addr != CEC_LOG_ADDR_INVALID) {
>> + u8 cmds[2] = {CMD_SET_CEC_LA, log_addr};
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_write_command(it6625, cmds, sizeof(cmds));
>
>How do you clear the logical address? If log_addr is called with CEC_LOG_ADDR_INVALID,
>then that should clear the current LA.
>
>I suspect that for this HW the LA should be set to 0xf, but I can't be certain.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
>> + u32 signal_free_time, struct cec_msg *msg)
>> +{
>> + struct it6625 *it6625 = adap->priv;
>> +
>> + dump_cec_msg(it6625, msg, "CEC_TX");
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + it6625_write_bytes(it6625, REG_CEC_TX_DATA, msg->msg, msg->len);
>> + it6625_write_byte(it6625, REG_CEC_TX_DATA_LEN, msg->len);
>> + it6625_set_bits(it6625, REG_HOST_CTRL_INT, B_CEC_SEND_DATA, B_CEC_SEND_DATA);
>> + it6625_wait_for_status(it6625, REG_HOST_CTRL_INT, 0x00, 25);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct cec_adap_ops it6625_cec_adap_ops = {
>> + .adap_enable = it6625_cec_adap_enable,
>> + .adap_log_addr = it6625_cec_adap_log_addr,
>> + .adap_transmit = it6625_cec_adap_transmit,
>> +};
>> +
>> +static void it6625_cec_handler(struct it6625 *it6625, u8 intstatus)
>> +{
>> + struct cec_msg rxmsg = {};
>> + int val = 0;
>> +
>> + if (intstatus & B_CEC_RX_RECEIVED) {
>> + scoped_guard(mutex, &it6625->it6625_lock) {
>> + val = it6625_read_byte(it6625, REG_CEC_RX_DATA_LEN);
>> + if (val > 0 && val <= 16)
>
>Use CEC_MAX_MSG_SIZE instead of hardcoding '16'. Same elsewhere in this function.
>
>> + it6625_read_bytes(it6625, REG_CEC_RX_DATA, &rxmsg.msg[0], val);
>> + it6625_write_byte(it6625, REG_CEC_RX_DATA_LEN, 0);
>> + }
>> +
>> + if (val > 0 && val <= 16) {
>> + rxmsg.len = val;
>> + cec_received_msg(it6625->cec_adap, &rxmsg);
>> + dump_cec_msg(it6625, &rxmsg, "CEC_RX");
>> + } else {
>> + dev_err(it6625->dev, "invalid CEC RX length %d", val);
>> + }
>> + }
>> +
>> + if (intstatus & B_CEC_TX_UPDATE) {
>> + val = it6625_read_byte(it6625, REG_CEC_STATUS);
>> + if (val < 0) {
>> + dev_err(it6625->dev, "read CEC status failed");
>> + return;
>> + }
>> +
>> + if (val & BIT(0)) {
>> + cec_transmit_attempt_done(it6625->cec_adap,
>> + CEC_TX_STATUS_OK);
>> + } else if (val & BIT(1)) {
>> + cec_transmit_attempt_done(it6625->cec_adap,
>> + CEC_TX_STATUS_NACK);
>> + } else {
>> + dev_err(it6625->dev, "unknown CEC status %02X", val);
>
>This probably should be dev_info. If the hardware returns a known bad value,
>then this would be an error. But if it is a valid value, but you just do not
>know what it means, then dev_info is more appropriate.
>
>> + cec_transmit_attempt_done(it6625->cec_adap,
>> + CEC_TX_STATUS_NACK);
>> + }
>> + }
>> +}
>> +
>> +static void it6625_irq_format_change(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> + struct v4l2_dv_timings timings;
>> + const struct v4l2_event it6625_ev_fmt = {
>> + .type = V4L2_EVENT_SOURCE_CHANGE,
>> + .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
>> + };
>> + int ret;
>> +
>> + if (no_signal(it6625))
>
>If the signal disappears, then also raise the SOURCE_CHANGE event.
>I think that is missing here.
>
>> + return;
>> +
>> + ret = it6625_get_detected_timings(it6625, &timings);
>> + if (ret < 0) {
>> + v4l2_dbg(1, debug, sd, "Failed to get detected timings");
>> + return;
>> + }
>> +
>> + if (sd->devnode)
>> + v4l2_subdev_notify_event(sd, &it6625_ev_fmt);
>> +}
>> +
>> +static void it6625_irq_hdmi_audio_change(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> +
>> + it6625_s_ctrl_audio_sampling_rate(sd);
>> + it6625_s_ctrl_audio_present(sd);
>> +}
>> +
>> +static void it6625_get_timings(struct it6625 *it6625,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + guard(mutex)(&it6625->it6625_lock);
>> + *timings = it6625->timings;
>> +}
>> +
>> +static void it6625_clear_timings(struct it6625 *it6625)
>> +{
>> + guard(mutex)(&it6625->it6625_lock);
>> + memset(&it6625->timings, 0, sizeof(it6625->timings));
>> +}
>> +
>> +static void it6625_irq_hdmi_5v_change(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> +
>> + it6625_clear_timings(it6625);
>> + it6625_v4l2_sd_ctrl_update(sd);
>> +}
>> +
>> +static void it6625_irq_hdcp_change(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> + u8 cp_sts;
>> +
>> + cp_sts = it6625_read_byte(it6625, REG_RX_HDCP_STS);
>> + v4l2_info(sd, "HDCP change to %02X", cp_sts);
>> +}
>> +
>> +static void it6625_irq_infoframe_latch(struct it6625 *it6625)
>> +{
>> + u8 info_frame[32];
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> +
>> + it6625_read_bytes(it6625, REG_IF_DATA, info_frame, 31);
>> +}
>> +
>> +static void it6625_irq_emata_packet_latch(struct it6625 *it6625)
>> +{
>> + u8 extend_packet[14];
>> +
>> + it6625_read_bytes(it6625, REG_EMP_DATA, extend_packet, 13);
>> +}
>> +
>> +static void it6625_irq_avmute_change(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> + int val;
>> + u8 avmute;
>> +
>> + val = it6625_read_byte(it6625, REG_RX_STATUS);
>> + if (val < 0)
>> + return;
>> +
>> + avmute = (val & B_RX_AVMUTE) ? 1 : 0;
>> + v4l2_info(sd, "AVMute change to %d", avmute);
>> +}
>> +
>> +static bool it6625_interrupt_handler(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> + int val, int_sts1 = 0, int_sts2 = 0;
>> +
>> + val = it6625_read_byte(it6625, REG_MCU_INTERRUPT);
>> + if (val <= 0)
>> + return false;
>> +
>> + it6625_write_byte(it6625, REG_MCU_INTERRUPT, val);
>> +
>> + v4l2_dbg(1, debug, sd, "%s: INT = 0x%02X", __func__, val);
>> +
>> + if (val & (B_CEC_RX_RECEIVED | B_CEC_TX_UPDATE) && it6625->cec_adap)
>> + it6625_cec_handler(it6625, val & (B_CEC_RX_RECEIVED | B_CEC_TX_UPDATE));
>> +
>> + if (val & B_SYS_INT_ACTIVE) {
>> + int_sts1 = it6625_read_byte(it6625, REG_RX_INT_STATUS1);
>> + int_sts2 = it6625_read_byte(it6625, REG_RX_INT_STATUS2);
>> + v4l2_dbg(1, debug, sd, "%s: hdmi_int = 0x%02X 0x%02X",
>> + __func__, int_sts1, int_sts2);
>> +
>> + if (int_sts1 >= 0) {
>> + it6625_write_byte(it6625, REG_RX_INT_STATUS1, 0x00);
>> + if (int_sts1 & B_HDMI_5V_CHG)
>> + it6625_irq_hdmi_5v_change(it6625);
>> +
>> + if (int_sts1 & B_HDMI_VID_CHG)
>> + it6625_irq_format_change(it6625);
>> +
>> + if (int_sts1 & B_HDMI_AUD_CHG)
>> + it6625_irq_hdmi_audio_change(it6625);
>> +
>> + if (int_sts1 & B_HDMI_CP_CHG)
>> + it6625_irq_hdcp_change(it6625);
>> +
>> + if (int_sts1 & B_HDMI_IF_LATCH)
>> + it6625_irq_infoframe_latch(it6625);
>> +
>> + if (int_sts1 & B_HDMI_EMP)
>> + it6625_irq_emata_packet_latch(it6625);
>> + }
>> +
>> + if (int_sts2 >= 0) {
>> + it6625_write_byte(it6625, REG_RX_INT_STATUS2, 0x00);
>> +
>> + if (int_sts2 & B_HDMI_AVI)
>> + it6625_show_avi_infoframe(it6625);
>> +
>> + if (int_sts2 & B_HDMI_NO_AVI)
>> + v4l2_info(sd, "AVI-info stopped");
>> +
>> + if (int_sts2 & B_HDMI_AVMUTE_CHG)
>> + it6625_irq_avmute_change(it6625);
>> + }
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static irqreturn_t it6625_irq_handler(int unused, void *data)
>> +{
>> + struct it6625 *it6625 = data;
>> +
>> + return it6625_interrupt_handler(it6625) ? IRQ_HANDLED : IRQ_NONE;
>> +}
>> +
>> +static void it6625_irq_poll_timer(struct timer_list *t)
>> +{
>> + struct it6625 *it6625 = timer_container_of(it6625, t, timer);
>> + unsigned int msecs;
>> +
>> + schedule_work(&it6625->polling_work);
>> + /*
>> + * If CEC is present, then we need to poll more frequently,
>> + * otherwise we will miss CEC messages.
>> + */
>> + msecs = it6625->cec_adap ? POLL_INTERVAL_CEC_MS : POLL_INTERVAL_MS;
>> + mod_timer(&it6625->timer, jiffies + msecs_to_jiffies(msecs));
>> +}
>> +
>> +static void it6625_polling_work(struct work_struct *work)
>> +{
>> + struct it6625 *it6625 = container_of(work, struct it6625,
>> + polling_work);
>> +
>> + it6625_interrupt_handler(it6625);
>> +}
>> +
>> +static int it6625_log_status(struct v4l2_subdev *sd)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + struct v4l2_dv_timings timings, configured_timings;
>> +
>> + if (it6625_get_detected_timings(it6625, &timings))
>> + v4l2_info(sd, "No video detected");
>> + else
>> + v4l2_print_dv_timings(sd->name, "Detected format: ", &timings,
>> + true);
>> +
>> + it6625_get_timings(it6625, &configured_timings);
>> + v4l2_print_dv_timings(sd->name, "Configured format: ",
>> + &configured_timings, true);
>> +
>> + it6625_show_avi_infoframe(it6625);
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + schedule_work(&it6625->polling_work);
>> + *handled = true;
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
>> + struct v4l2_event_subscription *sub)
>> +{
>> + switch (sub->type) {
>> + case V4L2_EVENT_SOURCE_CHANGE:
>> + return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
>> + case V4L2_EVENT_CTRL:
>> + return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int it6625_g_input_status(struct v4l2_subdev *sd, u32 *status)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + bool val;
>> +
>> + val = no_signal(it6625);
>> + *status = 0;
>> + *status |= val ? V4L2_IN_ST_NO_SIGNAL : 0;
>> + *status |= val ? V4L2_IN_ST_NO_SYNC : 0;
>> +
>> + v4l2_dbg(1, debug, sd, "%s: status = 0x%x", __func__, *status);
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +it6625_update_timings_if_changed(struct it6625 *it6625,
>> + const struct v4l2_dv_timings *timings)
>> +{
>> + int ret;
>> +
>> + guard(mutex)(&it6625->it6625_lock);
>> + if (v4l2_match_dv_timings(&it6625->timings, timings, 0, false)) {
>> + ret = 0;
>> + } else if (!v4l2_valid_dv_timings(timings, &it6625_timings_cap,
>> + NULL, NULL)) {
>> + ret = -ERANGE;
>> + } else {
>> + it6625->timings = *timings;
>> + ret = 1;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int it6625_s_dv_timings(struct v4l2_subdev *sd,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + int ret;
>> +
>> + if (!timings)
>> + return -EINVAL;
>> +
>> + if (debug)
>> + v4l2_print_dv_timings(sd->name, __func__, timings, false);
>> +
>> + ret = it6625_update_timings_if_changed(it6625, timings);
>> + if (ret == -ERANGE) {
>> + v4l2_dbg(1, debug, sd, "%s: timings out of range", __func__);
>> + return ret;
>> + }
>> +
>> + if (ret == 0) {
>> + v4l2_dbg(1, debug, sd, "%s: no change", __func__);
>> + return 0;
>> + }
>> +
>> + it6625_enable_stream(it6625, false);
>
>This doesn't look right. Video output should only be enabled when the bridge driver
>calls the s_stream op. Doing it here can cause problems if the remainder of the
>video pipeline isn't properly configured.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_g_dv_timings(struct v4l2_subdev *sd,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + it6625_get_timings(it6625, timings);
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_enum_dv_timings(struct v4l2_subdev *sd,
>> + struct v4l2_enum_dv_timings *timings)
>> +{
>> + if (timings->pad != 0)
>> + return -EINVAL;
>> +
>> + return v4l2_enum_dv_timings_cap(timings,
>> + &it6625_timings_cap, NULL, NULL);
>> +}
>> +
>> +static int it6625_query_dv_timings(struct v4l2_subdev *sd,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + int ret;
>> +
>> + ret = it6625_get_detected_timings(it6625, timings);
>> + if (ret)
>> + return ret;
>> +
>> + if (debug)
>> + v4l2_print_dv_timings(sd->name, __func__, timings, false);
>> +
>> + if (!v4l2_valid_dv_timings(timings, &it6625_timings_cap, NULL, NULL)) {
>> + v4l2_dbg(1, debug, sd, "%s: timings out of range", __func__);
>> + return -ERANGE;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_dv_timings_cap(struct v4l2_subdev *sd,
>> + struct v4l2_dv_timings_cap *cap)
>> +{
>> + if (cap->pad != 0)
>> + return -EINVAL;
>> +
>> + *cap = it6625_timings_cap;
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + it6625_enable_stream(it6625, enable);
>> + return 0;
>> +}
>> +
>> +static int it6625_enum_mbus_code(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *sd_state,
>> + struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> + dev_dbg(sd->dev, "%s: index=%d", __func__, code->index);
>> +
>> + if (code->index >= ARRAY_SIZE(it6625_formats))
>> + return -EINVAL;
>> +
>> + dev_dbg(sd->dev, "%s: code=0x%08x", __func__,
>> + it6625_formats[code->index].mbus_fmt_code);
>> + code->code = it6625_formats[code->index].mbus_fmt_code;
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_get_mbus_config(struct v4l2_subdev *sd,
>> + unsigned int pad,
>> + struct v4l2_mbus_config *cfg)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + if (pad != 0)
>> + return -EINVAL;
>> +
>> + cfg->type = it6625->bus_type;
>> + cfg->bus.mipi_csi2.flags = 0;
>> + cfg->bus.mipi_csi2.num_data_lanes = it6625->csi_lanes;
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_pad_s_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + if (pad != 0)
>> + return -EINVAL;
>> +
>> + return it6625_s_dv_timings(sd, timings);
>
>Why not just move the it6625_s_dv_timings code here? Ditto for the next two functions.
>
>> +}
>> +
>> +static int it6625_pad_g_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + if (pad != 0)
>> + return -EINVAL;
>> +
>> + return it6625_g_dv_timings(sd, timings);
>> +}
>> +
>> +static int it6625_pad_query_dv_timings(struct v4l2_subdev *sd,
>> + unsigned int pad,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + if (pad != 0)
>> + return -EINVAL;
>> +
>> + return it6625_query_dv_timings(sd, timings);
>> +}
>> +
>> +static inline u32 format_to_colorspace(u8 csi_format)
>> +{
>> + switch (csi_format) {
>> + case CSI_RGB444:
>> + case CSI_RGB555:
>> + case CSI_RGB565:
>> + case CSI_RGB666:
>> + case CSI_RGB888:
>> + case CSI_RGB_10b:
>> + case CSI_RGB_12b:
>> + return V4L2_COLORSPACE_SRGB;
>> + case CSI_YUV420_8b_L:
>> + case CSI_YUV420_8b:
>> + case CSI_YUV420_10b:
>> + case CSI_YUV422_8b:
>> + case CSI_YUV422_10b:
>> + case CSI_YUV422_12b:
>> + case CSI_YUV420_10b_L:
>> + case CSI_YUV420_12b:
>> + case CSI_YUV444_8b:
>> + case CSI_YUV444_10b:
>> + case CSI_YUV444_12b:
>> + return V4L2_COLORSPACE_REC709;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static const char *it6625_csi_format_name(u8 csi_format)
>> +{
>> + switch (csi_format) {
>> + case CSI_YUV422_8b:
>> + return "YUV422 8bit";
>> + case CSI_RGB888:
>> + return "RGB888 8bit";
>> + case CSI_YUV444_8b:
>> + return "YUV444 8bit";
>> + default:
>> + return "unknown";
>> + }
>> +}
>> +
>> +static int it6625_get_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *sd_state,
>> + struct v4l2_subdev_format *format)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + struct v4l2_dv_timings timings;
>> +
>> + if (format->pad != 0)
>> + return -EINVAL;
>> +
>> + it6625_get_timings(it6625, &timings);
>> + format->format.width = timings.bt.width;
>> + format->format.height = timings.bt.height;
>> + format->format.field = timings.bt.interlaced == V4L2_DV_INTERLACED ?
>> + V4L2_FIELD_INTERLACED : V4L2_FIELD_NONE;
>> +
>> + if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> + struct v4l2_mbus_framefmt *fmt;
>> +
>> + fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
>> + format->format.code = fmt->code;
>> + format->format.colorspace = fmt->colorspace;
>> + } else {
>> + scoped_guard(mutex, &it6625->it6625_lock) {
>> + format->format.colorspace =
>> + format_to_colorspace(it6625->csi_format);
>> + format->format.code = it6625->mbus_fmt_code;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_set_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *sd_state,
>> + struct v4l2_subdev_format *format)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + int ret;
>> + u32 mbus_fmt_code = format->format.code;
>> +
>> + ret = it6625_get_fmt(sd, sd_state, format);
>> + format->format.code = mbus_fmt_code;
>> +
>> + if (ret)
>> + return ret;
>> +
>> + ret = it6625_csi_mbus_code_idx(mbus_fmt_code);
>> +
>> + if (ret < 0) {
>> + v4l2_dbg(1, debug, sd,
>> + "%s: unsupported format code 0x%x, falling back to default",
>> + __func__, mbus_fmt_code);
>> + ret = 0;
>> + mbus_fmt_code = it6625_formats[ret].mbus_fmt_code;
>> + format->format.code = mbus_fmt_code;
>> + }
>> +
>> + if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>> + struct v4l2_mbus_framefmt *fmt;
>> +
>> + fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
>> + fmt->code = format->format.code;
>> + fmt->colorspace = format_to_colorspace(it6625_formats[ret].csi_format);
>> + format->format.colorspace = fmt->colorspace;
>> + v4l2_dbg(1, debug, sd, "%s: try format code = 0x%x",
>> + __func__, format->format.code);
>> + return 0;
>> + }
>> +
>> + scoped_guard(mutex, &it6625->it6625_lock) {
>> + it6625->csi_format = it6625_formats[ret].csi_format;
>> + it6625->mbus_fmt_code = format->format.code;
>> + it6625_enable_stream_locked(it6625, false);
>> + it6625_set_mipi_config_locked(it6625, it6625->csi_format);
>> + }
>> +
>> + format->format.colorspace = format_to_colorspace(it6625_formats[ret].csi_format);
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_g_edid(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_edid *edid)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + int err;
>> +
>> + if (edid->pad != 0)
>> + return -EINVAL;
>> +
>> + memset(edid->reserved, 0, sizeof(edid->reserved));
>> +
>> + if (edid->blocks == 0) {
>> + edid->blocks = EDID_NUM_BLOCKS_MAX;
>> + return 0;
>> + }
>> +
>> + if (edid->start_block >= EDID_NUM_BLOCKS_MAX) {
>> + edid->blocks = 0;
>> + return 0;
>> + }
>> +
>> + if (edid->blocks > EDID_NUM_BLOCKS_MAX - edid->start_block)
>> + edid->blocks = EDID_NUM_BLOCKS_MAX - edid->start_block;
>> +
>> + err = it6625_read_edid(it6625, edid->edid, edid->start_block,
>> + edid->blocks);
>
>This always reads 4 EDID blocks, even if the EDID has fewer blocks.
>
>You should determine the actual number of blocks. Easiest is just to
>store that when s_edid is called so you know how many blocks the EDID
>has.
>
>> +
>> + return (err < 0) ? err : 0;
>> +}
>> +
>> +static int it6625_s_edid(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_edid *edid)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + int err = 0;
>> + u16 pa;
>> + int pa_err;
>> +
>> + if (edid->pad != 0) {
>> + v4l2_err(sd, "invalid pad %d", edid->pad);
>> + return -EINVAL;
>> + }
>> +
>> + memset(edid->reserved, 0, sizeof(edid->reserved));
>> +
>> + if (edid->start_block != 0) {
>> + v4l2_err(sd, "start_block must be 0 for set edid");
>> + return -EINVAL;
>> + }
>> +
>> + if (edid->blocks > EDID_NUM_BLOCKS_MAX) {
>> + v4l2_err(sd, "too many edid blocks: %d", edid->blocks);
>> + edid->blocks = EDID_NUM_BLOCKS_MAX;
>> + return -E2BIG;
>> + }
>> +
>> + it6625_disable_hpd(it6625);
>> +
>> + if (edid->blocks == 0) {
>> + cec_phys_addr_invalidate(it6625->cec_adap);
>> + return 0;
>> + }
>> +
>> + err = it6625_write_edid(it6625, edid->edid,
>> + edid->start_block, edid->blocks);
>> + if (err < 0) {
>> + v4l2_err(sd, "write edid failed");
>> + } else {
>> + pa = cec_get_edid_phys_addr(edid->edid, edid->blocks * 128, NULL);
>
>This code is wrong. Use this:
>
> pa = v4l2_get_edid_phys_addr(edid->edid, edid->blocks * 128, NULL);
> err = v4l2_phys_addr_validate(pa, &parent_pa, NULL);
> if (err)
> return err;
>
>This validate check must be done before you write the new EDID.
>
> cec_s_phys_addr(it6625->cec_adap, parent_pa, false);
>
>And this cec_s_phys_addr call should be done after the EDID was written.
>
>> + pa_err = pa == CEC_PHYS_ADDR_INVALID ? -EINVAL :
>> + v4l2_phys_addr_validate(pa, NULL, NULL);
>> + if (pa_err) {
>> + v4l2_warn(sd, "invalid CEC physical address in EDID");
>> + cec_phys_addr_invalidate(it6625->cec_adap);
>> + } else {
>> + cec_s_phys_addr(it6625->cec_adap, pa, false);
>> + }
>> + }
>> +
>> + if (hdmi_5v_power_present(it6625)) {
>> + it6625_enable_hpd(it6625);
>> + it6625_s_ctrl_detect_hdmi_5v(sd);
>> + } else {
>> + it6625_enable_auto_hpd(it6625);
>> + }
>> +
>> + return err < 0 ? err : 0;
>> +}
>> +
>> +#define V4L2_CID_USER_IT6625_BASE (V4L2_CID_USER_BASE + 0x11b0)
>
>Custom driver controls must be reserved in include/uapi/linux/v4l2-controls.h.
>
>> +#define IT6625_CID_AUDIO_SAMPLING_RATE (V4L2_CID_USER_IT6625_BASE + 0)
>> +#define IT6625_CID_AUDIO_PRESENT (V4L2_CID_USER_IT6625_BASE + 1)
>> +#define IT6625_CID_HDMI_INFO (V4L2_CID_USER_IT6625_BASE + 2)
>> +#define IT6625_CID_MIPI_CONFIG (V4L2_CID_USER_IT6625_BASE + 3)
>
>Typically a new public header is created for the driver controls. See e.g.
>include/uapi/linux/npcm-video.h. These controls should be documented in
>that header.
>
>> +static int it6625_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> + struct it6625 *it6625 = hdl_to_6625(ctrl->handler);
>> + struct v4l2_subdev *sd = &it6625->sd;
>> +
>> + v4l2_dbg(1, debug, sd, "%s, id=0x%x, val=%d",
>> + __func__, ctrl->id, ctrl->val);
>> +
>> + switch (ctrl->id) {
>> + case IT6625_CID_HDMI_INFO:
>> + scoped_guard(mutex, &it6625->it6625_lock) {
>> + snprintf(ctrl->p_new.p_char, ctrl->maximum, "%s @ %uHz",
>> + it6625_csi_format_name(it6625->csi_format),
>> + fps_from_bt_timings(&it6625->timings.bt));
>
>This isn't a volatile control: instead, when csi_format or timings.bt change,
>just update this control with the new value.
>
>But I think this control should be dropped as don't see any value in it.
>
>If you like, you can report this in it6625_log_status, that's a more appropriate
>place.
>
>> + }
>> + break;
>> + case IT6625_CID_MIPI_CONFIG:
>> + ctrl->val = it6625_read_byte(it6625, REG_MIPI_DATA_TYPE);
>
>I wonder about this control as well: volatile controls are meant for controls where
>the value has to be read from the hardware and that value can change at any moment.
>
>A typical example is if AUTOGAIN is enabled, and you read the current GAIN value from
>the hardware.
>
>The MIPI_DATA_TYPE is, I think, controlled by the driver and does not randomly change.
>
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_s_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> + struct it6625 *it6625 = hdl_to_6625(ctrl->handler);
>> + struct v4l2_subdev *sd = &it6625->sd;
>> +
>> + v4l2_dbg(1, debug, sd, "%s, id=0x%x, val=%d",
>> + __func__, ctrl->id, ctrl->val);
>> +
>> + switch (ctrl->id) {
>> + case IT6625_CID_MIPI_CONFIG:
>> + it6625_set_mipi_config(it6625, ctrl->val);
>
>Why is this settable? Isn't this derived from the mbus_code? This doesn't
>look right.
>
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_core_ops it6625_core_ops = {
>> + .log_status = it6625_log_status,
>> + .interrupt_service_routine = it6625_isr,
>> + .subscribe_event = it6625_subscribe_event,
>> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
>> +};
>> +
>> +static const struct v4l2_subdev_video_ops it6625_video_ops = {
>> + .g_input_status = it6625_g_input_status,
>> + .s_stream = it6625_s_stream,
>> +};
>> +
>> +static const struct v4l2_subdev_pad_ops it6625_pad_ops = {
>> + .enum_mbus_code = it6625_enum_mbus_code,
>> + .set_fmt = it6625_set_fmt,
>> + .get_fmt = it6625_get_fmt,
>> + .get_edid = it6625_g_edid,
>> + .set_edid = it6625_s_edid,
>> + .enum_dv_timings = it6625_enum_dv_timings,
>> + .dv_timings_cap = it6625_dv_timings_cap,
>> + .get_mbus_config = it6625_get_mbus_config,
>> + .s_dv_timings = it6625_pad_s_dv_timings,
>> + .g_dv_timings = it6625_pad_g_dv_timings,
>> + .query_dv_timings = it6625_pad_query_dv_timings,
>> +};
>> +
>> +static const struct v4l2_subdev_ops it6625_ops = {
>> + .core = &it6625_core_ops,
>> + .video = &it6625_video_ops,
>> + .pad = &it6625_pad_ops,
>> +};
>> +
>> +static const struct v4l2_ctrl_ops it6625_ctrl_ops = {
>> + .g_volatile_ctrl = it6625_g_volatile_ctrl,
>> + .s_ctrl = it6625_s_ctrl,
>> +};
>> +
>> +static const struct v4l2_ctrl_config it6625_ctrl_audio_sampling_rate = {
>> + .id = IT6625_CID_AUDIO_SAMPLING_RATE,
>> + .name = "Audio sampling rate",
>
>"Audio Sampling Rate"
>
>> + .type = V4L2_CTRL_TYPE_INTEGER,
>> + .min = 0,
>> + .max = 768000,
>> + .step = 1,
>> + .def = 0,
>> + .flags = V4L2_CTRL_FLAG_READ_ONLY,
>> +};
>> +
>> +static const struct v4l2_ctrl_config it6625_ctrl_audio_present = {
>> + .id = IT6625_CID_AUDIO_PRESENT,
>> + .name = "Audio present",
>
>"Audio Present"
>
>But rather than making these two audio controls specific for this driver, they should
>be made standard controls. The tc358743.c driver has the same controls. And they are
>typical for HDMI drivers.
>
>They can be part of the Digital Video Controls.
For the two read-only audio controls, Audio Sampling Rate and Audio
Present, would it be acceptable to keep them as properly reserved
driver-specific controls, following the tc358743 model?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/tc358743.c#n1973
I will reserve V4L2_CID_USER_IT6625_BASE in
include/uapi/linux/v4l2-controls.h and add a documented public
include/uapi/linux/it6625.
HDMI Info and MIPI Config will be removed.
>> + .type = V4L2_CTRL_TYPE_BOOLEAN,
>> + .min = 0,
>> + .max = 1,
>> + .step = 1,
>> + .def = 0,
>> + .flags = V4L2_CTRL_FLAG_READ_ONLY,
>> +};
>> +
>> +static const struct v4l2_ctrl_config it6625_ctrl_get_hdmi_info = {
>> + .ops = &it6625_ctrl_ops,
>> + .id = IT6625_CID_HDMI_INFO,
>> + .name = "HDMI Info",
>> + .type = V4L2_CTRL_TYPE_STRING,
>> + .max = 128,
>> + .step = 1,
>> + .flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE,
>> +};
>> +
>> +static const struct v4l2_ctrl_config it6625_ctrl_mipi_config = {
>> + .ops = &it6625_ctrl_ops,
>> + .id = IT6625_CID_MIPI_CONFIG,
>> + .name = "MIPI config",
>
>"MIPI Config"
>
>> + .type = V4L2_CTRL_TYPE_INTEGER,
>> + .min = 0,
>> + .max = 0x7FFFFFFF,
>> + .step = 1,
>> + .def = 0x3E,
>> + .flags = V4L2_CTRL_FLAG_VOLATILE,
>> +};
>> +
>> +static int it6625_v4l2_init_controls(struct v4l2_subdev *sd)
>> +{
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> + struct v4l2_ctrl_handler *hdl = &it6625->hdl;
>> +
>> + v4l2_ctrl_handler_init(hdl, 4);
>> + it6625->ctrl_5v_detect =
>> + v4l2_ctrl_new_std(hdl, NULL, V4L2_CID_DV_RX_POWER_PRESENT,
>> + 0, 1, 0, 0);
>> +
>> + it6625->ctrl_audio_sampling_rate =
>> + v4l2_ctrl_new_custom(hdl,
>> + &it6625_ctrl_audio_sampling_rate,
>> + NULL);
>> + it6625->ctrl_audio_present =
>> + v4l2_ctrl_new_custom(hdl, &it6625_ctrl_audio_present, NULL);
>> + it6625->ctrl_link_freq =
>> + v4l2_ctrl_new_int_menu(hdl, NULL, V4L2_CID_LINK_FREQ,
>> + ARRAY_SIZE(it6625_link_freq) - 1,
>> + 0, it6625_link_freq);
>> + it6625->ctrl_get_hdmi_info =
>> + v4l2_ctrl_new_custom(hdl, &it6625_ctrl_get_hdmi_info, NULL);
>> + it6625->ctrl_mipi_config =
>> + v4l2_ctrl_new_custom(hdl, &it6625_ctrl_mipi_config, NULL);
>> + if (hdl->error) {
>> + v4l2_err(sd, "Failed to initialize controls");
>> + v4l2_ctrl_handler_free(hdl);
>> + return hdl->error;
>> + }
>> +
>> + sd->ctrl_handler = hdl;
>> +
>> + return 0;
>> +}
>> +
>> +static void it6625_regdump_print(struct seq_file *s, const u8 *reg_buf)
>> +{
>> + int i;
>> +
>> + seq_puts(s, " 0x00 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0A 0x0B 0x0C 0x0D 0x0E 0x0F\n");
>> +
>> + for (i = 0; i < 256; i++) {
>> + if (i % 16 == 0)
>> + seq_printf(s, "[%02X] ", i & 0xF0);
>> + seq_printf(s, "0x%02X ", reg_buf[i]);
>> + if (i % 16 == 15)
>> + seq_putc(s, '\n');
>> + }
>> +}
>> +
>> +static int it6625_edid_ram_show(struct seq_file *s, void *data)
>> +{
>> + struct it6625 *it6625 = s->private;
>> + u8 reg_buf[256];
>> + int ret;
>> +
>> + ret = it6625_read_edid(it6625, reg_buf, 0, 2);
>> + if (ret < 0)
>> + return ret;
>> + it6625_regdump_print(s, reg_buf);
>> +
>> + return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(it6625_edid_ram);
>> +
>> +static int it6625_mipi_reg_show(struct seq_file *s, void *data)
>> +{
>> + struct it6625 *it6625 = s->private;
>> + u8 reg_buf[256];
>> + int ret;
>> +
>> + scoped_guard(mutex, &it6625->it6625_lock)
>> + ret = it6625_read_bytes(it6625, 0x00, reg_buf, sizeof(reg_buf));
>> + if (ret < 0)
>> + return ret;
>> + it6625_regdump_print(s, reg_buf);
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_mipi_reg_open(struct inode *inode, struct file *file)
>> +{
>> + return single_open(file, it6625_mipi_reg_show, inode->i_private);
>> +}
>> +
>> +static ssize_t it6625_mipi_reg_write(struct file *file,
>> + const char __user *user_buf,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct it6625 *it6625 = file_inode(file)->i_private;
>> + char buf[32] = {};
>> + unsigned int addr, val;
>> + ssize_t len;
>> + loff_t pos = 0;
>> +
>> + len = simple_write_to_buffer(buf, sizeof(buf) - 1, &pos, user_buf, count);
>> + if (len < 0)
>> + return len;
>> + buf[len] = '\0';
>> +
>> + if (sscanf(buf, "%X %X", &addr, &val) != 2)
>> + return -EINVAL;
>> +
>> + scoped_guard(mutex, &it6625->it6625_lock)
>> + it6625_write_byte(it6625, addr, val);
>> +
>> + return count;
>> +}
>> +
>> +static const struct file_operations it6625_mipi_reg_fops = {
>> + .owner = THIS_MODULE,
>> + .open = it6625_mipi_reg_open,
>> + .read = seq_read,
>> + .write = it6625_mipi_reg_write,
>> + .llseek = seq_lseek,
>> + .release = single_release,
>> +};
>> +
>> +static void it6625_debugfs_init(struct it6625 *it6625, struct i2c_client *client)
>> +{
>> + it6625->debugfs_dir = debugfs_create_dir(dev_name(&client->dev), NULL);
>> +
>> + debugfs_create_file("mipi_reg", 0600, it6625->debugfs_dir, it6625,
>> + &it6625_mipi_reg_fops);
>> + debugfs_create_file("edid_ram", 0400, it6625->debugfs_dir, it6625,
>> + &it6625_edid_ram_fops);
>
>Dumping the EDID RAM seems pointless to me, given that you can get the EDID data
>with an ioctl call at any time. Is there a particular reason for adding this?
>
>What should be added here is code to export InfoFrames to debugfs. Search for
>v4l2_debugfs_if_alloc in drivers/media.
>
>All new HDMI receivers must support that.
>
>> +}
>> +
>> +static void it6625_init_data(struct it6625 *it6625)
>> +{
>> + static struct v4l2_dv_timings default_timing =
>> + V4L2_DV_BT_CEA_1920X1080P60;
>> +
>> + it6625->csi_lanes = 4;
>> + it6625->port_num = 1;
>> + it6625->bus_type = V4L2_MBUS_CSI2_DPHY;
>> + it6625->csi_format = it6625_formats[0].csi_format;
>> + it6625->mbus_fmt_code = it6625_formats[0].mbus_fmt_code;
>> + it6625->timings = default_timing;
>> +}
>> +
>> +static int it6625_parse_endpoint(struct it6625 *it6625)
>> +{
>> + struct device *dev = it6625->dev;
>> + /*
>> + * Pre-setting bus_type here makes v4l2_fwnode_endpoint_alloc_parse()
>> + * treat it as a hard requirement and reject any endpoint whose DT
>> + * bus-type disagrees, so this must stay V4L2_MBUS_UNKNOWN to let it
>> + * autodetect C-PHY vs D-PHY from the endpoint itself.
>> + */
>> + struct v4l2_fwnode_endpoint endpoint = { .bus_type = V4L2_MBUS_UNKNOWN };
>> + struct device_node *ep = NULL;
>> + unsigned int max_lanes;
>> + unsigned int port;
>> + int ret;
>> +
>> + /*
>> + * port@0 and port@1 are the two CSI-2 output ports MIPI0/MIPI1
>> + * (port@2 is the HDMI input). This chip series can drive both
>> + * simultaneously in split or mirror mode, so port_num counts how
>> + * many of MIPI0/MIPI1 have an endpoint wired up. This driver only
>> + * wires up a single source pad, so lane/bus-type configuration is
>> + * parsed from whichever of the two is found first.
>> + */
>> + it6625->port_num = 0;
>> + for (port = 0; port < 2; port++) {
>> + struct device_node *port_ep =
>> + of_graph_get_endpoint_by_regs(dev->of_node, port, -1);
>> +
>> + if (!port_ep)
>> + continue;
>> +
>> + it6625->port_num++;
>> + if (!ep)
>> + ep = port_ep;
>> + else
>> + of_node_put(port_ep);
>> + }
>> +
>> + if (!ep) {
>> + it6625->port_num = 1;
>> + dev_dbg(dev, "no CSI-2 endpoint node found, using default %u CSI lanes",
>> + it6625->csi_lanes);
>> + return 0;
>> + }
>> +
>> + ret = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep), &endpoint);
>> + of_node_put(ep);
>> + if (ret) {
>> + dev_err(dev, "failed to parse endpoint: %d", ret);
>> + return ret;
>> + }
>> +
>> + if (endpoint.bus_type != V4L2_MBUS_CSI2_DPHY &&
>> + endpoint.bus_type != V4L2_MBUS_CSI2_CPHY) {
>> + dev_err(dev, "unsupported bus type %d, expected CSI-2 D-PHY or C-PHY",
>> + endpoint.bus_type);
>> + v4l2_fwnode_endpoint_free(&endpoint);
>> + return -EINVAL;
>> + }
>> +
>> + if (endpoint.bus_type == V4L2_MBUS_CSI2_CPHY &&
>> + it6625->chip_type != IT6626_CHIP) {
>> + dev_err(dev, "IT6625 does not support C-PHY, only IT6626 does");
>> + v4l2_fwnode_endpoint_free(&endpoint);
>> + return -EINVAL;
>> + }
>> +
>> + max_lanes = (endpoint.bus_type == V4L2_MBUS_CSI2_CPHY) ? 3 : 4;
>> +
>> + if (endpoint.bus.mipi_csi2.num_data_lanes == 0 ||
>> + endpoint.bus.mipi_csi2.num_data_lanes > max_lanes) {
>> + dev_err(dev,
>> + "invalid number of CSI data lanes: %u (max %u for this bus type)",
>> + endpoint.bus.mipi_csi2.num_data_lanes, max_lanes);
>> + v4l2_fwnode_endpoint_free(&endpoint);
>> + return -EINVAL;
>> + }
>> +
>> + it6625->csi_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
>> + it6625->bus_type = endpoint.bus_type;
>> + v4l2_fwnode_endpoint_free(&endpoint);
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_parse_dt(struct it6625 *it6625)
>> +{
>> + return it6625_parse_endpoint(it6625);
>> +}
>> +
>> +static int it6625_init_v4l2_subdev(struct it6625 *it6625)
>> +{
>> + struct v4l2_subdev *sd = &it6625->sd;
>> + int err;
>> +
>> + sd->dev = it6625->dev;
>> +
>> + v4l2_i2c_subdev_init(sd, it6625->i2c_client, &it6625_ops);
>> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
>> + if (it6625_v4l2_init_controls(sd)) {
>> + dev_err(it6625->dev, "Failed to initialize v4l2 controls");
>> + return -ENOMEM;
>> + }
>> +
>> + it6625->pad.flags = MEDIA_PAD_FL_SOURCE;
>> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
>> + err = media_entity_pads_init(&sd->entity, 1, &it6625->pad);
>> + if (err < 0) {
>> + dev_err(it6625->dev, "%s %d err=%d", __func__, __LINE__, err);
>> + v4l2_ctrl_handler_free(sd->ctrl_handler);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_check_device(struct it6625 *it6625)
>> +{
>> + static const u8 chip_ids[][2] = {
>> + { 0x66, 0x25 },
>> + { 0x66, 0x26 },
>> + };
>> + int chip_id0, chip_id1;
>> +
>> + chip_id0 = it6625_read_byte(it6625, REG_CHIP_ID_0);
>> + chip_id1 = it6625_read_byte(it6625, REG_CHIP_ID_1);
>> + if (chip_id0 != chip_ids[it6625->chip_type][0] ||
>> + chip_id1 != chip_ids[it6625->chip_type][1]) {
>> + dev_err(it6625->dev,
>> + "chip ID mismatch: got 0x%02x%02x, expected 0x%02x%02x",
>> + chip_id0, chip_id1,
>> + chip_ids[it6625->chip_type][0],
>> + chip_ids[it6625->chip_type][1]);
>> + return -ENODEV;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int it6625_probe(struct i2c_client *client)
>> +{
>> + struct it6625 *it6625;
>> + struct v4l2_subdev *sd;
>> + int err;
>> +
>> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>> + return -EIO;
>> +
>> + it6625 = devm_kzalloc(&client->dev, sizeof(struct it6625), GFP_KERNEL);
>> + if (!it6625)
>> + return -ENOMEM;
>> +
>> + it6625->chip_type = (uintptr_t)i2c_get_match_data(client);
>> +
>> + it6625->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(it6625->reset_gpio))
>> + return PTR_ERR(it6625->reset_gpio);
>> +
>> + if (it6625->reset_gpio) {
>> + usleep_range(1000, 2000);
>> + gpiod_set_value_cansleep(it6625->reset_gpio, 0);
>> + usleep_range(10000, 11000);
>> + }
>> +
>> + err = it6625_regmap_i2c_init(client, it6625);
>> + if (err)
>> + return err;
>> +
>> + err = it6625_check_device(it6625);
>> + if (err)
>> + return err;
>> +
>> + it6625_init_data(it6625);
>> +
>> + err = it6625_parse_dt(it6625);
>> + if (err)
>> + return err;
>> +
>> + mutex_init(&it6625->it6625_lock);
>> + INIT_DELAYED_WORK(&it6625->hpd_delayed_work, it6625_hpd_delayed_work);
>> + INIT_WORK(&it6625->polling_work, it6625_polling_work);
>> +
>> + if (client->irq) {
>> + err = devm_request_threaded_irq(&client->dev, client->irq,
>> + NULL, it6625_irq_handler,
>> + IRQF_TRIGGER_LOW |
>> + IRQF_ONESHOT |
>> + IRQF_NO_AUTOEN,
>> + "it6625", it6625);
>> + if (err)
>> + return err;
>> + } else {
>> + dev_info(it6625->dev, "no IRQ, falling back to polling");
>> + timer_setup(&it6625->timer, it6625_irq_poll_timer, 0);
>> + }
>> +
>> + sd = &it6625->sd;
>> + err = it6625_init_v4l2_subdev(it6625);
>> + if (err)
>> + goto err_clean_work_queues;
>> +
>> + err = v4l2_ctrl_handler_setup(sd->ctrl_handler);
>> + if (err)
>> + goto err_clean_hdl;
>> +
>> + it6625->cec_adap = cec_allocate_adapter(&it6625_cec_adap_ops,
>> + it6625, dev_name(it6625->dev),
>> + CEC_CAP_DEFAULTS |
>> + CEC_CAP_MONITOR_ALL |
>> + CEC_CAP_PHYS_ADDR,
>> + CEC_MAX_LOG_ADDRS);
>> + if (IS_ERR(it6625->cec_adap)) {
>> + err = PTR_ERR(it6625->cec_adap);
>> + dev_err(it6625->dev, "%s %d", __func__, __LINE__);
>> + goto err_clean_hdl;
>> + }
>> +
>> + err = cec_register_adapter(it6625->cec_adap, &client->dev);
>> + if (err < 0) {
>> + dev_err(it6625->dev, "%s: failed to register the cec device", __func__);
>> + cec_delete_adapter(it6625->cec_adap);
>> + it6625->cec_adap = NULL;
>> + goto err_clean_hdl;
>> + }
>> +
>> + it6625_debugfs_init(it6625, client);
>> +
>> + it6625_initial_setup(it6625);
>> + it6625_v4l2_sd_ctrl_update(sd);
>> +
>> + err = v4l2_async_register_subdev(sd);
>> + if (err < 0) {
>> + dev_err(it6625->dev, "%s %d err=%d", __func__, __LINE__, err);
>> + goto err_clean_debugfs;
>> + }
>> +
>> + if (client->irq)
>> + enable_irq(client->irq);
>> + else
>> + mod_timer(&it6625->timer, jiffies + msecs_to_jiffies(POLL_INTERVAL_MS));
>> +
>> + return 0;
>> +
>> +err_clean_debugfs:
>> + debugfs_remove_recursive(it6625->debugfs_dir);
>> + cec_unregister_adapter(it6625->cec_adap);
>> +err_clean_hdl:
>> + media_entity_cleanup(&sd->entity);
>> + v4l2_ctrl_handler_free(&it6625->hdl);
>> +
>> +err_clean_work_queues:
>> + if (!client->irq)
>> + timer_shutdown_sync(&it6625->timer);
>> + cancel_work_sync(&it6625->polling_work);
>> + cancel_delayed_work_sync(&it6625->hpd_delayed_work);
>> + mutex_destroy(&it6625->it6625_lock);
>> + return err;
>> +}
>> +
>> +static void it6625_remove(struct i2c_client *client)
>> +{
>> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
>> + struct it6625 *it6625 = sd_to_6625(sd);
>> +
>> + v4l2_async_unregister_subdev(sd);
>> + v4l2_device_unregister_subdev(sd);
>> +
>> + if (client->irq)
>> + disable_irq(client->irq);
>> + else
>> + timer_shutdown_sync(&it6625->timer);
>> +
>> + cancel_work_sync(&it6625->polling_work);
>> + cancel_delayed_work_sync(&it6625->hpd_delayed_work);
>> + debugfs_remove_recursive(it6625->debugfs_dir);
>> + cec_unregister_adapter(it6625->cec_adap);
>> + mutex_destroy(&it6625->it6625_lock);
>> + media_entity_cleanup(&sd->entity);
>> + v4l2_ctrl_handler_free(&it6625->hdl);
>> +}
>> +
>> +static const struct i2c_device_id it6625_id[] = {
>> + { .name = "it6625", .driver_data = IT6625_CHIP },
>> + { .name = "it6626", .driver_data = IT6626_CHIP },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, it6625_id);
>> +
>> +static const struct of_device_id it6625_of_match[] = {
>> + { .compatible = "ite,it6625", .data = (void *)IT6625_CHIP },
>> + { .compatible = "ite,it6626", .data = (void *)IT6626_CHIP },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, it6625_of_match);
>> +
>> +static struct i2c_driver it6625_driver = {
>> + .driver = {
>> + .name = "it6625",
>> + .of_match_table = it6625_of_match,
>> + },
>> + .probe = it6625_probe,
>> + .remove = it6625_remove,
>> + .id_table = it6625_id,
>> +};
>> +module_i2c_driver(it6625_driver);
>> +
>> +MODULE_DESCRIPTION("iTE it6625/it6626 HDMI to MIPI CSI bridge driver");
>> +MODULE_AUTHOR("Hermes Wu <Hermes.wu@ite.com.tw>");
>> +MODULE_LICENSE("GPL");
>>
>
>Regards,
>
> Hans
>
Thanks,
Hermes
prev parent reply other threads:[~2026-08-03 11:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-30 6:55 [PATCH v9 0/2] media: i2c: add support for ITE IT6625/IT6626 HDMI to MIPI CSI-2 bridge Hermes Wu via B4 Relay
2026-07-30 6:55 ` [PATCH v9 1/2] dt-bindings: media: add ITE IT6625/IT6626 HDMI bridge Hermes Wu via B4 Relay
2026-07-30 6:55 ` [PATCH v9 2/2] media: i2c: add driver for ITE IT6625/IT6626 Hermes Wu via B4 Relay
2026-07-31 11:56 ` Hans Verkuil
2026-08-03 3:43 ` Hermes.Wu
2026-08-03 11:25 ` Hermes.Wu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3bffbc016a6d4f968dbc0594bfef14c0@ite.com.tw \
--to=hermes.wu@ite.com.tw \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=hverkuil+cisco@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox