Re: [PATCH v15 15/22] media: i2c: add Maxim GMSL2/3 deserializer framework

["Artur Andrzejczak" <andrzejczak.artur@gmail.com>]

On Fri Aug 7, 2026 at 3:02 PM CEST, Dumitru Ceclan wrote:
> [...]

hweight_long(des->info->versions) >= 1 is true for any driver that
declares a version, so the check makes .set_link_version mandatory for
every chip driver. The "Multiple version" message suggests "> 1" was
intended. Both error messages also name .select_link_version(), which
does not exist.

An automated review already flagged the unguarded .select_links() call
in max_des_init_link_ser_xlate(). The same unguarded call is in
max_des_ser_attach_addr() (out_select_links) and max_des_i2c_atr_init().
Only max_des_i2c_mux_select() checks the op before calling it. The
">= 1" check is what currently requires every driver to provide the op,
so changing it to "> 1" alone would expose those call sites to a
single-version driver without .select_links.

In max_des_parse_src_dt_endpoint(): "Unsupported bus-type %u on port %u"
is printed with (pad, bus_type). The arguments seem swapped.

Doubled 10/12-bpp is handled at the PHY level via alt_mem_map10/12 in
max_des_get_phy_mode(), but the pipe-level dbl10/dbl12 (the
BPP10DBL/BPP12DBL bits the chip drivers write) are never set by the
core. Only dbl8 is. Is this on purpose, or should
max_des_get_pipe_mode() set dbl10/dbl12 as well? If it is intentional,
isn't the dbl10/dbl12 handling in the chip drivers unused?

The version loop in max_des_ser_attach_addr() sets RX_RATE using
set_link_version() and seems to rely on the select_links() call inside
max_des_init_link_ser_xlate() doing RESET_ONESHOT to latch it, as the
multi-link paths do. But max9296a_select_links() returns early for
num_links == 1, before the reset. max96714_des_info declares both 3G and
6G speeds, so on MAX96714 the fallback iteration writes a new rate with
no oneshot reset after it. The fallback rate would then not take effect,
and a 3G-only serializer may fail to attach, or attach with the wrong
version stored. How is the new rate meant to take effect in that case?

Also in the same loop, if no version bit is inside [min, max], the loop
ends with ret == 0, and the code after it marks ser_xlate.en = true with
version = min - 1 and no serializer attached. Not reachable with the
current chip infos, but an info with an empty versions mask might hit it
(an empty mask also passes the probe check above, since
hweight_long(0) == 0). Initializing ret to -ENOENT could solve it.

Kind Regards,
Artur Andrzejczak