Re: [PATCH 2/9] media: camss: csiphy: add CSIPHY 2.4.0 lane settings for SM8750

[Shuai Liu]

Hi Qianhui,

Reviewing the series (built for x86_64 with COMPILE_TEST, checkpatch
clean, no functional testing - no hardware here).

> The register map is the one the CSIPHY 2.4.0 vendor driver uses for its
> two-phase variant (csiphy_2ph_v2_4_0_reg, 103 entries): address, mask
> and parameter type are identical for all 103 entries, and the data byte
> is identical for 99.  The four remaining entries are the SKEW_CAL data
> bytes of the four lanes (0x005c and its per-lane copies), which are
> 0x00 here, i.e. the vendor value.

The commit message and the table contradict each other.  The four
SKEW_CAL entries added by this patch program 0x54:

  {0x0094, 0xD7, 0x00, CSIPHY_SKEW_CAL},
  {0x005C, 0x54, 0x00, CSIPHY_SKEW_CAL},
  {0x0060, 0xFD, 0x00, CSIPHY_SKEW_CAL},
  {0x0064, 0x7F, 0x00, CSIPHY_SKEW_CAL},

while the commit message states twice that the 0x005c data byte is
"0x00 here, i.e. the vendor value" and that "the vendor value is kept".

As posted the correction does exist, but it is buried in patch 4/9,
which flips exactly these four entries back to 0x00 - so the final tree
matches the prose, but patch 2 as applied does not, and readers of
patch 2 alone cannot tell which value is intended.

Please make patch 2 self-consistent: if the vendor value (0x00) is what
ships, program it directly here; if 0x54 is intended, fix the commit
message instead.  Either way the csiphy hunk in patch 4/9 should go
away (see my reply there).

Thanks,
Shuai Liu

(This review was prepared with the assistance of an AI coding tool;
all findings were verified against the patched source.)