From: Sakari Ailus <sakari.ailus@iki.fi>
To: Hermes.wu@ite.com.tw
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 2/2] media: i2c: add driver for ITE IT6625/IT6626
Date: Wed, 16 Sep 2026 14:02:09 +0300 [thread overview]
Message-ID: <aqp3MYVynMUMVvcy@valkosipuli.retiisi.eu> (raw)
In-Reply-To: <20260811-upstream-it6625-it6626-v10-2-9b6bb75262c9@ite.com.tw>
Hi Hermes,
I accidentally started reviewing the first version, then noticed there were
more. Could you address my comments in follow-up patches, please?
On Tue, Aug 11, 2026 at 06:31:26PM +0800, 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 v10:
> - Advertise the true single-logical-address CEC capability instead of
> CEC_MAX_LOG_ADDRS (4): confirmed against the hardware that
> CMD_SET_CEC_LA overwrites rather than adds an address, so claiming 4
> simultaneous addresses let the CEC core silently evict an
> already-configured one from hardware every time it claimed another.
> Also fix it6625_cec_adap_log_addr() to actually release the address
> in hardware on CEC_LOG_ADDR_INVALID (previously a no-op) by toggling
> CMD_SET_CEC_ENABLE off then on -- confirmed this fully resets both
> the logical address and monitor-all mode -- conditioned on
> adap->is_enabled so a call landing while the adapter should stay
> disabled doesn't silently re-enable it.
> - Add .init_state so a freshly-opened subdev's V4L2_SUBDEV_FORMAT_TRY
> pad format isn't left zero-initialized, matching the pattern used by
> lt6911uxe.c, the closest comparable upstream HDMI-to-MIPI-CSI2
> bridge driver.
> - Stop hardcoding IRQF_TRIGGER_LOW in the IRQ request: it unconditionally
> overrode whatever polarity the board's devicetree "interrupts"
> property configured. The reference DTS already specifies
> IRQ_TYPE_LEVEL_LOW so behavior there is unchanged.
> - Add a dedicated edid_lock spanning the complete it6625_s_edid()
> sequence: previously each sub-step (disable_hpd/write_edid/
> cec_s_phys_addr/enable_hpd) was individually locked but the whole
> sequence wasn't, so two concurrent VIDIOC_SUBDEV_S_EDID calls could
> leave hardware programmed with one call's EDID while CEC recorded
> the other's physical address. Also fix a related probe() unwind bug
> found while adding this lock: an IRQ request failure returned
> directly instead of going through the existing cleanup path, leaking
> it6625_lock (pre-existing) and now edid_lock too.
>
> All four found by the sashiko.dev automated review of v9. One other v9
> review finding was checked and confirmed real, but is not being acted
> on this round: a UAF where it6625_remove() can free devm-allocated
> state while an ioctl on /dev/v4l-subdevX is still executing, since
> subdev devnodes have no supported pre-registration lock/lifetime hook
> a driver can use to close it -- doing so properly needs a
> v4l2_subdev_release() core reorder (drivers/media/v4l2-core/
> v4l2-device.c) plus a kref'd/gated driver restructuring, both out of
> scope for this series.
>
> - Fix it6625_cec_reset_la() to actually write CMD_SET_CEC_LA with
> CEC_LOG_ADDR_UNREGISTERED (0xf) when the adapter should stay
> enabled, instead of toggling CEC enable off and back on, which
> never cleared the logical address at all on that path.
> - Drop the redundant CEC message dump (the CEC core already logs the
> full message hex on both TX/RX), replace two hardcoded message-size
> bounds with CEC_MAX_MSG_SIZE, and use dev_info instead of dev_err
> for an unrecognized CEC TX status byte, since it isn't necessarily
> an error.
> - Use HZ/7 instead of msecs_to_jiffies(100) for the HPD delay -- more
> reliable across sources with inaccurate HPD-low timing.
> - Raise V4L2_EVENT_SOURCE_CHANGE on signal loss too, not just on
> resolution change.
> - Stop calling it6625_enable_stream(false) from it6625_s_dv_timings():
> video output should only be controlled by s_stream, not by
> S_DV_TIMINGS.
> - Inline the three pad-ops DV-timings wrappers
> (s/g/query_dv_timings) directly into their bodies and drop the
> separate non-pad functions, which existed only to do a pad check
> before calling straight through.
> - Track the actual number of loaded EDID blocks (new edid_blocks
> field, defaulting to 2 since firmware ships with a verified
> 2-block default EDID already in EDID RAM at power-on) instead of
> always assuming a fixed EDID_NUM_BLOCKS_MAX, and rewrite
> it6625_g_edid()/it6625_s_edid() to match tc358743_g_edid()'s
> capacity-query/-ENODATA/-EINVAL semantics (using a subtraction-based
> clamp rather than tc358743's addition-based one, which can integer-
> overflow on attacker-controlled __u32 block counts).
> it6625_s_edid() now validates the CEC physical address with
> v4l2_get_edid_phys_addr()/v4l2_phys_addr_validate() before taking
> edid_lock or touching any hardware/CEC state, so a validation
> failure leaves everything untouched; a hardware write failure
> returns immediately, before the HPD-reenable block, leaving HPD
> disabled and edid_blocks at 0 rather than assuming the previous
> EDID is still valid.
> - Drop the two custom IT6625_CID_HDMI_INFO/IT6625_CID_MIPI_CONFIG
> controls entirely. HDMI_INFO's format/fps string moves to
> it6625_log_status() instead (using a single locked snapshot of
> csi_format and the configured timings so the reported pair was
> actually configured together); MIPI_CONFIG exposed a raw settable
> MIPI data-type register with no real use case and is simply
> removed. it6625_g_volatile_ctrl()/it6625_s_ctrl()/it6625_ctrl_ops
> and two now-dead helpers (hdl_to_6625(), the unlocked
> it6625_set_mipi_config() wrapper) go with them.
> - Add the required InfoFrame debugfs export (AVI/Audio/SPD/HDMI via
> v4l2_debugfs_if_alloc(); DRM is not wired up this round, not yet
> confirmed by hardware testing) and drop the edid_ram debugfs dump
> it replaces, which was redundant with G_EDID. Synchronizing the
> export against the interrupt path uses two new locks
> (if_read_lock/if_state_lock) and a driver-private
> if_active/if_snapshot/if_snapshot_done request-tracking state
> instead of pausing every interrupt producer; the interrupt-path
> drain is factored into it6625_drain_interrupts()/
> it6625_drain_rx_int_status() so both the interrupt handler and the
> debugfs callback share the same error-aware drain logic.
> it6625_remove() now frees the InfoFrame export before stopping the
> IRQ/timer/polling-work producers and before unregistering the
> subdevice, so no in-flight read or ordinary interrupt can be
> dispatched against already-torn-down state. Two hardware-side
> timing properties (whether disarming REG_IF_LATCH_HB synchronously
> prevents an already-latched packet from surfacing, and whether the
> one-shot capture is guaranteed to see the first triggering frame
> rather than a later one) can't be verified from source and are
> accepted as documented residual risk pending hardware confirmation.
> - Reserve a proper V4L2_CID_USER_IT6625_BASE in v4l2-controls.h and
> move the two remaining audio controls
> (AUDIO_SAMPLING_RATE/AUDIO_PRESENT) into a new public
> include/uapi/linux/it6625.h, renamed to the public
> V4L2_CID_IT6625_* form -- the old locally-defined base collided
> with V4L2_CID_USER_NPCM_BASE. While touching this control's
> definition: raise the sampling-rate control's .max from 768000 to
> 1536000 to match what get_audio_sampling_rate()'s s_fsid_map can
> actually report, check the REG_HDMI_AUDIO_INFO1 read for I2C
> failure instead of letting a negative error code truncate into the
> u8 FSID comparison, and store exact Hz values in s_fsid_map instead
> of a lossy *1000 shorthand that rounded five 44.1kHz-family rates
> (including the previously-missing AUD1411K entry) away from their
> real CEA/IEC values, and title-case both controls' .name strings
> ("Audio Sampling Rate"/"Audio Present") to match what Hans's
> review quoted them as -- missed in the reservation fix above,
> applied now.
>
> All of the above found by Hans Verkuil's on-list review of v9.
>
> - Raise the DV-timings cap and correct V4L2_CID_LINK_FREQ reporting
> for IT6626's three-trio C-PHY link (it6626_cphy_3trio_timings_cap/
> it6625_get_timings_cap(), and a new C-PHY entry in
> it6625_link_freq[]). The previous shared 300 MHz DV-timings cap
> and single fixed 445.5 MHz link-frequency value meant this driver
> could never accept or correctly describe a 4K60 4:4:4/RGB timing
> on this topology, even though the chip supports it there. This
> was deferred pending two things, both now confirmed:
> 2.5 Gsym/s confirmed as the hardware's max C-PHY capability for
> the tested single-port, three-trio configuration, and 4K60 RGB444
> capture on that configuration confirmed working -- tested on an
> MTK G720 platform, which supports C-PHY but is a different SoC
> than this driver's existing Qualcomm CAMSS reference deployment
> documented in Testing below. Scoped to csi_lanes == 3 regardless
> of port_num, since the CSI-2 pixel-rate formula this cap is
> checked against has no port_num term; one- and two-trio C-PHY
> stay on the original cap/link frequency -- unvalidated at the
> higher rate, not proven incapable of it.
>
> 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 | 8 +
> drivers/media/i2c/Kconfig | 18 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/it6625.c | 2394 ++++++++++++++++++++++++++++++++++++
> include/uapi/linux/it6625.h | 25 +
> include/uapi/linux/v4l2-controls.h | 6 +
> 6 files changed, 2452 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3785b8c1de0a61b6e0be2f8b0bc506b6f23ec25b..0036fc35227ca5937bc1b894b29c4336507bcb9d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13850,6 +13850,14 @@ 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
> +F: include/uapi/linux/it6625.h
> +
> 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 5c52007f9cbeb77fa3adf3008428f4d1dc529203..c1b7a53fbe43a2669f9a562bd657a4fc09f6708d 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1301,6 +1301,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 d04bd5724552e88fc2ad39f9bbaf9b4ce1763939..b54145f685d65e28aaaaf2be296766c9f913eea9 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_VIDEO_IMX678) += imx678.o
> obj-$(CONFIG_VIDEO_IMX471) += imx471.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..80610cce196266ced10917f88fe4310f0f2a84fa
> --- /dev/null
> +++ b/drivers/media/i2c/it6625.c
> @@ -0,0 +1,2394 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * it6625 - ite HDMI to MIPI bridge
> + */
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/completion.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>
> +#include <uapi/linux/it6625.h>
> +
> +static int debug = 3;
Why is the default to enable printing all debug information? Shouldn't this
be 0?
> +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;
You have a single sub-device. Why not to use the sub-device state lock for
this? This driver should also fully switch to sub-device state (related
comments below).
> + /* serializes the complete VIDIOC_S_EDID sequence against itself */
> + struct mutex edid_lock;
> + /* serializes a full InfoFrame debugfs read transaction against itself */
> + struct mutex if_read_lock;
> + /*
> + * protects if_active/if_type/if_snapshot/if_snapshot_err/
> + * if_snapshot_done and the REG_MCU_INTERRUPT/REG_RX_INT_STATUS1/2/
> + * REG_IF_LATCH_HB/REG_IF_DATA register group between the interrupt
> + * path and the InfoFrame debugfs callback. Never held across
> + * wait_for_completion_timeout(). Nests outside it6625_lock.
> + */
> + struct mutex if_state_lock;
> +
> + struct v4l2_subdev sd;
> + struct v4l2_mbus_config_mipi_csi2 bus;
This seems to be unused.
> + 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 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;
Avoid storing state information outside sub-devie state.
> + u32 mbus_fmt_code;
> + /* number of EDID blocks currently loaded, protected by edid_lock */
> + u8 edid_blocks;
> +
> + struct gpio_desc *reset_gpio;
> +
> + struct cec_adapter *cec_adap;
> +
> + struct dentry *debugfs_dir;
> +
> + struct v4l2_debugfs_if *infoframes;
> + struct completion if_latched;
> + /* true while an InfoFrame debugfs request is outstanding */
> + bool if_active;
> + /* HDMI packet-type byte currently armed in REG_IF_LATCH_HB */
> + u32 if_type;
> + /* driver-private copy of REG_IF_DATA, captured at the genuine latch */
> + u8 if_snapshot[31];
> + /* it6625_read_bytes() result for the if_snapshot capture */
> + int if_snapshot_err;
> + /* set just before complete(), to disambiguate timeout vs. capture */
> + bool if_snapshot_done;
> +};
> +
> +/*
> + * Index 0: D-PHY (4-lane). Index 1: C-PHY (3-trio) -- the confirmed
> + * hardware max C-PHY capability, tested single-port/three-trio.
> + */
You won't be switching PHYs at runtime, will you? You should have two
arrays in this case, selected based on PHY type.
> +static const s64 it6625_link_freq[] = {
> + 445500000,
> + 2500000000LL,
> +};
> +
> +/*
> + * Shared cap for every topology except the reference exception below:
> + * IT6625 (no C-PHY support at all), IT6626 running D-PHY, and IT6626
> + * running C-PHY with fewer than three trios. This is conservative
> + * scoping, not a claim that one-/two-trio C-PHY can't also support a
> + * higher rate -- they're simply unvalidated.
> + */
> +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 */
Please drop, minimum is 8.1 nowadays.
> + .reserved = { 0 },
Ditto.
> +
> + 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)
> +};
> +
> +/*
> + * IT6626 C-PHY, three trios: raised pixel-clock ceiling (594 MHz vs
> + * the shared 300 MHz cap) for this topology's higher C-PHY capability.
> + */
> +static const struct v4l2_dv_timings_cap it6626_cphy_3trio_timings_cap = {
> + .type = V4L2_DV_BT_656_1120,
> + .reserved = { 0 },
Ditto.
> +
> + V4L2_INIT_BT_TIMINGS(640, 3840, 480, 2160, 27000000, 594000000,
> + 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 v4l2_dv_timings_cap *
> +it6625_get_timings_cap(struct it6625 *it6625)
> +{
> + if (it6625->chip_type == IT6626_CHIP &&
> + it6625->bus_type == V4L2_MBUS_CSI2_CPHY &&
> + it6625->csi_lanes == 3)
> + return &it6626_cphy_3trio_timings_cap;
> +
> + return &it6625_timings_cap;
> +}
> +
> +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++) {
unsigned int and you can declare it here.
> + 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++) {
Ditto.
> + 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 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;
Please make variable declarations look like a reverse Christmas tree.
Applies to the rest of the driver, too.
> +
> + 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,
s/1000/USEC_PER_MSEC/
> + timeout_round_ms * 1000,
Ditto.
> + false, it6625, reg);
> +
> + dev_info(dev, "%s status = %d %d", __func__, status, (int)rval);
No need to cast rval.
Is this useful? Shouldn't this be dev_dbg()?
> + 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);
Redundant parentheses.
> +}
> +
> +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);
Ditto.
> +}
> +
> +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);
Ditto.
> +}
> +
> +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);
Ditto.
> +}
> +
> +static int get_audio_sampling_rate(struct it6625 *it6625)
> +{
> + int fs_id;
> + int i, freq = 0;
> + const struct fs_id_map {
static const?
> + u8 fs_id;
> + u32 freq;
> + } s_fsid_map[] = {
> + { AUD32K, 32000 },
> + { AUD44K, 44100 },
> + { AUD48K, 48000 },
> + { AUD64K, 64000 },
> + { AUD88K, 88200 },
> + { AUD96K, 96000 },
> +
> + { AUD128K, 128000 },
> + { AUD176K, 176400 },
> + { AUD192K, 192000 },
> + { AUD256K, 256000 },
> + { AUD352K, 352800 },
> + { AUD384K, 384000 },
> +
> + { AUD512K, 512000 },
> + { AUD705K, 705600 },
> + { AUD768K, 768000 },
> + { AUD1024K, 1024000 },
> + { AUD1411K, 1411200 },
> + { AUD1536K, 1536000 },
> + };
> +
> + if (no_signal(it6625) || !audio_present(it6625))
> + return 0;
> +
> + guard(mutex)(&it6625->it6625_lock);
> +
> + fs_id = it6625_read_byte(it6625, REG_HDMI_AUDIO_INFO1);
> + if (fs_id < 0)
> + return 0;
> +
> + 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;
> + 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);
s/4/sizeof(ck)/
> + 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];
pclk = get_unaligned_be32(ck);
And include linux/unaligned.h. I guess it'd be safe to assume this would be
aligned though.
> +
> + v4l2_dbg(1, debug, &it6625->sd, "%s: pclk=%u (%08x)",
> + __func__, pclk, pclk);
> +
> + return (u64)pclk * 1000;
s/1000/KHZ_PER_MHZ/
And include linux/units.h.
> +}
> +
> +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++) {
unsigned int 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;
unsigned int except for err?
> + 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, HZ / 7);
> +}
> +
> +static void it6625_hpd_delayed_work(struct work_struct *work)
> +{
> + struct it6625 *it6625 = container_of(work,
> + struct it6625, hpd_delayed_work.work);
Either align the latter line to start after the opening parenthesis or wrap
after '=' instead.
> + 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];
width = get_unaligned_be16(buffer);
> + height = ((buffer[2] & 0xff) << 8) + buffer[3];
height = get_unaligned_be16(buffer + 2);
It'd be nicer to define a struct for this actually.
> +
> + 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];
Same for this one.
> +
> + 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);
Don't do bitwise or on error codes!
> +
> + 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 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 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;
You can assign these in declaration as you do below.
> + guard(mutex)(&it6625->it6625_lock);
> + it6625_write_command(it6625, cmds, sizeof(cmds));
> +
> + return 0;
> +}
> +
> +static void it6625_cec_reset_la(struct it6625 *it6625, bool keep_enabled)
> +{
> + u8 cmds[2];
> +
> + if (keep_enabled) {
> + cmds[0] = CMD_SET_CEC_LA;
> + cmds[1] = CEC_LOG_ADDR_UNREGISTERED;
> + } else {
> + cmds[0] = CMD_SET_CEC_ENABLE;
> + cmds[1] = 0;
> + }
> +
> + guard(mutex)(&it6625->it6625_lock);
> + it6625_write_command(it6625, cmds, sizeof(cmds));
> +}
> +
> +static int it6625_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
> +{
> + struct it6625 *it6625 = adap->priv;
> + u8 cmds[2] = {CMD_SET_CEC_LA, log_addr};
{ Spaces inside braces, please. }
> +
> + dev_dbg(it6625->dev, "%s: la=%d", __func__, log_addr);
> +
> + if (log_addr == CEC_LOG_ADDR_INVALID) {
> + it6625_cec_reset_la(it6625, adap->is_enabled);
> + return 0;
> + }
> +
> + guard(mutex)(&it6625->it6625_lock);
> + it6625_write_command(it6625, cmds, sizeof(cmds));
> +
> + 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;
> +
> + 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 <= CEC_MAX_MSG_SIZE)
> + 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 <= CEC_MAX_MSG_SIZE) {
> + rxmsg.len = val;
> + cec_received_msg(it6625->cec_adap, &rxmsg);
> + } 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_info(it6625->dev, "unknown CEC status %02X", val);
> + 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 (sd->devnode)
> + v4l2_subdev_notify_event(sd, &it6625_ev_fmt);
> + 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)
> +{
> + if (!it6625->if_active)
> + return;
> +
> + it6625->if_active = false;
> +
> + scoped_guard(mutex, &it6625->it6625_lock) {
> + it6625->if_snapshot_err =
> + it6625_read_bytes(it6625, REG_IF_DATA,
> + it6625->if_snapshot, 31);
> + }
> + it6625->if_snapshot_done = true;
> + complete(&it6625->if_latched);
> +}
> +
> +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 void it6625_dispatch_int_status1(struct it6625 *it6625, int int_sts1)
> +{
> + 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);
> +}
> +
> +static void it6625_dispatch_int_status2(struct it6625 *it6625, int int_sts2)
> +{
> + struct v4l2_subdev *sd = &it6625->sd;
> +
> + 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);
> +}
> +
> +/* caller must already hold if_state_lock */
> +static int it6625_drain_rx_int_status(struct it6625 *it6625)
> +{
> + int val1, val2, err;
> +
> + val1 = it6625_read_byte(it6625, REG_RX_INT_STATUS1);
> + val2 = it6625_read_byte(it6625, REG_RX_INT_STATUS2);
> +
> + err = 0;
> + if (val1 < 0)
> + err = val1;
> + else if (val2 < 0)
> + err = val2;
> +
> + if (val1 >= 0) {
> + int werr = it6625_write_byte(it6625, REG_RX_INT_STATUS1, 0x00);
> +
> + if (werr && !err)
> + err = werr;
> + it6625_dispatch_int_status1(it6625, val1);
> + }
> +
> + if (val2 >= 0) {
> + int werr = it6625_write_byte(it6625, REG_RX_INT_STATUS2, 0x00);
> +
> + if (werr && !err)
> + err = werr;
> + it6625_dispatch_int_status2(it6625, val2);
> + }
> +
> + return err;
> +}
> +
> +/* caller must already hold if_state_lock */
> +static int it6625_drain_interrupts(struct it6625 *it6625, bool *had_event)
> +{
> + struct v4l2_subdev *sd = &it6625->sd;
> + int val, err;
> +
> + val = it6625_read_byte(it6625, REG_MCU_INTERRUPT);
> + if (val < 0) {
> + if (had_event)
> + *had_event = false;
> + return val;
> + }
> + if (had_event)
> + *had_event = val > 0;
> + if (val == 0)
> + return 0;
> +
> + err = 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 child_err = it6625_drain_rx_int_status(it6625);
> +
> + if (child_err && !err)
> + err = child_err;
> + }
> +
> + return err;
> +}
> +
> +static bool it6625_interrupt_handler(struct it6625 *it6625)
> +{
> + bool had_event = false;
> +
> + guard(mutex)(&it6625->if_state_lock);
> + it6625_drain_interrupts(it6625, &had_event);
> + return had_event;
> +}
> +
> +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 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";
Do you need this? V4L2 already has names for formats.
> + }
> +}
> +
> +static int it6625_log_status(struct v4l2_subdev *sd)
> +{
> + struct it6625 *it6625 = sd_to_6625(sd);
> + struct v4l2_dv_timings timings, configured_timings;
> + struct v4l2_bt_timings bt;
> + u8 csi_format;
> +
> + 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);
> +
> + /* snapshot together so the reported pair was actually configured together */
> + scoped_guard(mutex, &it6625->it6625_lock) {
> + csi_format = it6625->csi_format;
> + bt = it6625->timings.bt;
> + }
> +
> + v4l2_info(sd, "CSI format: %s @ %uHz",
> + it6625_csi_format_name(csi_format),
> + fps_from_bt_timings(&bt));
> +
> + 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;
return 0;
> + } else if (!v4l2_valid_dv_timings(timings, it6625_get_timings_cap(it6625),
> + NULL, NULL)) {
> + ret = -ERANGE;
return -ERANGE;
> + } else {
> + it6625->timings = *timings;
> + ret = 1;
return 1;
And drop the else's.
> + }
> +
> + return ret;
> +}
> +
> +static int it6625_enum_dv_timings(struct v4l2_subdev *sd,
> + struct v4l2_enum_dv_timings *timings)
> +{
> + struct it6625 *it6625 = sd_to_6625(sd);
> +
> + if (timings->pad != 0)
> + return -EINVAL;
> +
> + return v4l2_enum_dv_timings_cap(timings,
> + it6625_get_timings_cap(it6625), NULL, NULL);
> +}
> +
> +static int it6625_dv_timings_cap(struct v4l2_subdev *sd,
> + struct v4l2_dv_timings_cap *cap)
> +{
> + struct it6625 *it6625 = sd_to_6625(sd);
> +
> + if (cap->pad != 0)
> + return -EINVAL;
> +
> + *cap = *it6625_get_timings_cap(it6625);
> +
> + 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);
Error checks are missing.
The s_stream() video op is deprecated, please switch to enable_streams and
disable_streams pad ops.
> + 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)
> +{
> + struct it6625 *it6625 = sd_to_6625(sd);
> + int ret;
> +
> + if (pad != 0)
> + return -EINVAL;
> +
> + 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;
> +}
> +
> +static int it6625_pad_g_dv_timings(struct v4l2_subdev *sd, unsigned int pad,
> + struct v4l2_dv_timings *timings)
> +{
> + struct it6625 *it6625 = sd_to_6625(sd);
> +
> + if (pad != 0)
> + return -EINVAL;
> +
> + it6625_get_timings(it6625, timings);
> +
> + return 0;
> +}
> +
> +static int it6625_pad_query_dv_timings(struct v4l2_subdev *sd,
> + unsigned int pad,
> + struct v4l2_dv_timings *timings)
> +{
> + struct it6625 *it6625 = sd_to_6625(sd);
> + int ret;
> +
> + if (pad != 0)
> + return -EINVAL;
> +
> + 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_get_timings_cap(it6625), NULL, NULL)) {
> + v4l2_dbg(1, debug, sd, "%s: timings out of range", __func__);
> + return -ERANGE;
> + }
> +
> + return 0;
> +}
> +
> +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 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));
> +
> + guard(mutex)(&it6625->edid_lock);
> +
> + if (edid->start_block == 0 && edid->blocks == 0) {
> + edid->blocks = it6625->edid_blocks;
> + return 0;
> + }
> +
> + if (it6625->edid_blocks == 0)
> + return -ENODATA;
> +
> + if (edid->start_block >= it6625->edid_blocks || edid->blocks == 0)
> + return -EINVAL;
> +
> + if (edid->blocks > it6625->edid_blocks - edid->start_block)
> + edid->blocks = it6625->edid_blocks - edid->start_block;
> +
> + err = it6625_read_edid(it6625, edid->edid, edid->start_block,
> + edid->blocks);
> +
> + 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;
> + u16 parent_pa = CEC_PHYS_ADDR_INVALID;
> +
> + 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;
> + }
> +
> + if (edid->blocks != 0) {
> + u16 pa = v4l2_get_edid_phys_addr(edid->edid,
> + edid->blocks * 128, NULL);
> + err = v4l2_phys_addr_validate(pa, &parent_pa, NULL);
> + if (err) {
> + v4l2_err(sd, "invalid CEC physical address in EDID");
> + return err;
> + }
> + }
> +
> + guard(mutex)(&it6625->edid_lock);
> +
> + it6625_disable_hpd(it6625);
> + cec_phys_addr_invalidate(it6625->cec_adap);
> + it6625->edid_blocks = 0;
> +
> + if (edid->blocks == 0)
> + return 0;
> +
> + err = it6625_write_edid(it6625, edid->edid,
> + edid->start_block, edid->blocks);
> + if (err < 0) {
> + v4l2_err(sd, "write edid failed");
> + return err;
> + }
> +
> + it6625->edid_blocks = edid->blocks;
> + cec_s_phys_addr(it6625->cec_adap, parent_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 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 int it6625_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state)
> +{
> + struct v4l2_mbus_framefmt *fmt = v4l2_subdev_state_get_format(sd_state, 0);
> +
> + fmt->code = it6625_formats[0].mbus_fmt_code;
> + fmt->colorspace = format_to_colorspace(it6625_formats[0].csi_format);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_internal_ops it6625_internal_ops = {
> + .init_state = it6625_init_state,
> +};
> +
> +static const struct v4l2_ctrl_config it6625_ctrl_audio_sampling_rate = {
> + .id = V4L2_CID_IT6625_AUDIO_SAMPLING_RATE,
> + .name = "Audio Sampling Rate",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = 1536000,
> + .step = 1,
> + .def = 0,
> + .flags = V4L2_CTRL_FLAG_READ_ONLY,
> +};
> +
> +static const struct v4l2_ctrl_config it6625_ctrl_audio_present = {
> + .id = V4L2_CID_IT6625_AUDIO_PRESENT,
> + .name = "Audio Present",
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .min = 0,
> + .max = 1,
> + .step = 1,
> + .def = 0,
> + .flags = V4L2_CTRL_FLAG_READ_ONLY,
> +};
> +
> +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,
> + it6625->bus_type == V4L2_MBUS_CSI2_CPHY ? 1 : 0,
> + it6625_link_freq);
> + 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++) {
unsigned int 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_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,
> +};
> +
> +/*
> + * Maps a V4L2_DEBUGFS_IF_* framework flag to the actual HDMI packet-type
> + * header byte REG_IF_LATCH_HB expects to arm that InfoFrame type.
> + */
> +static int it6625_if_packet_type(u32 type)
> +{
> + switch (type) {
> + case V4L2_DEBUGFS_IF_AVI:
> + return 0x82;
> + case V4L2_DEBUGFS_IF_AUDIO:
> + return 0x84;
> + case V4L2_DEBUGFS_IF_SPD:
> + return 0x83;
> + case V4L2_DEBUGFS_IF_HDMI:
> + return 0x81;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static ssize_t it6625_debugfs_if_read(u32 type, void *priv, struct file *filp,
> + char __user *ubuf, size_t count,
> + loff_t *ppos)
> +{
> + struct v4l2_subdev *sd = priv;
> + struct it6625 *it6625 = sd_to_6625(sd);
> + u8 buf[32] = {};
> + int packet_type;
> + int err, err_reset;
> + bool captured;
> + int len;
> +
> + packet_type = it6625_if_packet_type(type);
> + if (packet_type < 0)
> + return 0;
> +
> + guard(mutex)(&it6625->if_read_lock);
> +
> + scoped_guard(mutex, &it6625->if_state_lock) {
> + scoped_guard(mutex, &it6625->it6625_lock)
> + err = it6625_write_byte(it6625, REG_IF_LATCH_HB, 0);
> + if (err)
> + return err;
> +
> + err = it6625_drain_interrupts(it6625, NULL);
> + if (err)
> + return err;
> +
> + reinit_completion(&it6625->if_latched);
> +
> + it6625->if_active = true;
> + it6625->if_type = packet_type;
> + it6625->if_snapshot_done = false;
> + it6625->if_snapshot_err = 0;
> +
> + scoped_guard(mutex, &it6625->it6625_lock)
> + err = it6625_write_byte(it6625, REG_IF_LATCH_HB,
> + packet_type);
> + if (err) {
> + scoped_guard(mutex, &it6625->it6625_lock)
> + it6625_write_byte(it6625, REG_IF_LATCH_HB, 0);
> + it6625->if_active = false;
> + return err;
> + }
> + }
> +
> + wait_for_completion_timeout(&it6625->if_latched, msecs_to_jiffies(100));
> +
> + scoped_guard(mutex, &it6625->if_state_lock) {
> + captured = it6625->if_snapshot_done;
> + it6625->if_active = false;
> +
> + scoped_guard(mutex, &it6625->it6625_lock)
> + err_reset = it6625_write_byte(it6625, REG_IF_LATCH_HB, 0);
> +
> + if (!captured) {
> + if (err_reset)
> + return err_reset;
> + return 0;
> + }
> +
> + if (!it6625->if_snapshot_err) {
> + buf[0] = packet_type;
> + memcpy(&buf[1], it6625->if_snapshot, 31);
> + }
> +
> + err = it6625->if_snapshot_err ? it6625->if_snapshot_err : err_reset;
> + if (err)
> + return err;
> + }
> +
> + len = buf[2] ? buf[2] + 4 : -ENOENT;
> + if (len > (int)sizeof(buf))
> + len = -ENOENT;
> + if (len < 0)
> + return 0;
> + return simple_read_from_buffer(ubuf, count, ppos, buf, len);
> +}
> +
> +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);
> +
> + it6625->infoframes = v4l2_debugfs_if_alloc(it6625->debugfs_dir,
> + V4L2_DEBUGFS_IF_AVI | V4L2_DEBUGFS_IF_AUDIO |
> + V4L2_DEBUGFS_IF_SPD | V4L2_DEBUGFS_IF_HDMI,
> + &it6625->sd, it6625_debugfs_if_read);
> +}
> +
> +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;
These values come from DT -- see my comments on the 1st patch, too.
> + it6625->csi_format = it6625_formats[0].csi_format;
> + it6625->mbus_fmt_code = it6625_formats[0].mbus_fmt_code;
> + it6625->timings = default_timing;
> + /* firmware ships with a verified 2-block default EDID in EDID RAM */
> + it6625->edid_blocks = 2;
> +}
> +
> +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);
Please don't add this, the driver should assume the configuration *is*
available in DT. Also the NULL check is redundant;
v4l2_fwnode_endpoint_alloc_parse() does it.
> + 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;
Either combine the error checks or add a label for error handling, use goto
here and below.
> + }
> +
> + 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->internal_ops = &it6625_internal_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;
> + }
I wonder if it'd be cleaner to have this in probe -- error handling becomes
difficult with these intermediate initialisation helpers that do apprently
unrelated things.
> +
> + 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);
> + mutex_init(&it6625->edid_lock);
> + mutex_init(&it6625->if_read_lock);
> + mutex_init(&it6625->if_state_lock);
> + init_completion(&it6625->if_latched);
> + 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_ONESHOT |
> + IRQF_NO_AUTOEN,
> + "it6625", it6625);
> + if (err)
> + goto err_clean_work_queues;
> + } 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,
> + 1);
> + 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);
You'll need to call v4l2_subdev_init_finalize() here to fully initialise
the sub-device state.
Similarly, call v4l2_subdev_cleanup() on error path after finalize() as
well as in remove.
> +
> + 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:
> + v4l2_debugfs_if_free(it6625->infoframes);
> + 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);
> + mutex_destroy(&it6625->edid_lock);
> + mutex_destroy(&it6625->if_read_lock);
> + mutex_destroy(&it6625->if_state_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_debugfs_if_free(it6625->infoframes);
> +
> + 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);
> +
> + v4l2_async_unregister_subdev(sd);
> + v4l2_device_unregister_subdev(sd);
> +
> + debugfs_remove_recursive(it6625->debugfs_dir);
> + cec_unregister_adapter(it6625->cec_adap);
> + mutex_destroy(&it6625->it6625_lock);
> + mutex_destroy(&it6625->edid_lock);
> + mutex_destroy(&it6625->if_read_lock);
> + mutex_destroy(&it6625->if_state_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");
> diff --git a/include/uapi/linux/it6625.h b/include/uapi/linux/it6625.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..a895658aaf6a80d3fb624cdece33c5135161c72d
> --- /dev/null
> +++ b/include/uapi/linux/it6625.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * Controls header for IT6625/IT6626 driver
> + */
> +
> +#ifndef _UAPI_LINUX_IT6625_H
> +#define _UAPI_LINUX_IT6625_H
> +
> +#include <linux/v4l2-controls.h>
> +
> +/*
> + * Currently detected HDMI audio sampling rate, in Hz. Read-only.
> + * 0 means the rate is unavailable/unknown: no audio is currently
> + * present on the input, the hardware reported a sample-rate id this
> + * driver doesn't recognize, or the status read itself failed. Never a
> + * literal 0 Hz sample rate.
> + */
> +#define V4L2_CID_IT6625_AUDIO_SAMPLING_RATE (V4L2_CID_USER_IT6625_BASE + 0)
> +
> +/*
> + * Whether HDMI audio is currently present on the input. Read-only.
> + */
> +#define V4L2_CID_IT6625_AUDIO_PRESENT (V4L2_CID_USER_IT6625_BASE + 1)
> +
> +#endif /* _UAPI_LINUX_IT6625_H */
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index affec0ab4781507be761cb791ff737460acf9cb7..6aa2a57753ae5aacb433201d68d51bb345ca0229 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -234,6 +234,12 @@ enum v4l2_colorfx {
> */
> #define V4L2_CID_USER_MALI_C55_BASE (V4L2_CID_USER_BASE + 0x1230)
>
> +/*
> + * The base for IT6625/IT6626 driver controls.
> + * We reserve 16 controls for this driver.
> + */
> +#define V4L2_CID_USER_IT6625_BASE (V4L2_CID_USER_BASE + 0x1240)
> +
> /* MPEG-class control IDs */
> /* The MPEG controls are applicable to all codec controls
> * and the 'MPEG' part of the define is historical */
>
--
Kind regards,
Sakari Ailus
next prev parent reply other threads:[~2026-09-16 11:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-11 10:31 [PATCH v10 0/2] media: i2c: add support for ITE IT6625/IT6626 HDMI to MIPI CSI-2 bridge Hermes Wu via B4 Relay
2026-08-11 10:31 ` [PATCH v10 1/2] dt-bindings: media: add ITE IT6625/IT6626 HDMI bridge Hermes Wu via B4 Relay
2026-08-11 10:31 ` [PATCH v10 2/2] media: i2c: add driver for ITE IT6625/IT6626 Hermes Wu via B4 Relay
2026-09-16 11:02 ` Sakari Ailus [this message]
2026-09-17 3:30 ` Hermes.Wu
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=aqp3MYVynMUMVvcy@valkosipuli.retiisi.eu \
--to=sakari.ailus@iki.fi \
--cc=Hermes.wu@ite.com.tw \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.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
all inboxes | Powered by JetHome®