Re: [PATCH v13 3/5] media: qcom: camss: Add support for PHY API devices

[Oleg Keri]

Hi Bryan,

On Tue, Jul 28, 2026 at 10:35:34AM +0100, Bryan O'Donoghue wrote:
> Add support for PHY API devices

One thing I ran into while bringing this up on a Lenovo Yoga Slim 7x
Gen 11 (Glymur, ov08x40 on CSIPHY4, two lanes), with Nihal's Glymur
CAMSS series on top: the two ends of the CAMSS <-> PHY link count lanes
differently, and the CAMSS side silently ends up on the wrong lanes.

The csi2-phy binding numbers data-lanes from 1, and the PHY driver
converts:

	/* Convert data-lanes = <1 2 3 4> to bit positions */
	csi2phy->stream_cfg.lane_cfg.data[i].pos = data_lanes[i] - 1;

The CAMSS endpoint still takes its data-lanes as 0-based positions;
camss_parse_endpoint_node() stores them verbatim and
csid_get_lane_assign() packs them straight into CSID_CSI2_RX_CFG0's
DLn_INPUT_SEL fields.

With the reference boards' style on both endpoints, i.e.

	&camss_csiphy4_inep { data-lanes = <1 2>; };
	&csiphy4_in_ep      { data-lanes = <1 2>; ... };

the PHY enables physical lanes 0 and 2 as before, but the CSID is told
DL0 <- 1, DL1 <- 2. Everything probes, the pipeline configures, VFE never
sees a frame and nothing is logged. With <0 1> on the CAMSS endpoint
alone, frames flow.

The x1e80100-crd and glymur-crd sensor patches use <1 2 3 4> on the
CAMSS endpoint; with four lanes that yields a lane assign of 0x4321
rather than 0x3210, so either the CSID tolerates it in the all-lanes
case or those boards only work by accident. Two lanes are not
tolerated.

Would it make sense to make the CAMSS endpoint follow the PHY
convention when its remote is a PHY, so the same numbers can be written
on both ends? Something like this on top of 3/5, against the parse
function:

	if (!legacy) {
		if (mipi_csi2->data_lanes[i] < 1)
			return -EINVAL;
		lncfg->data[i].pos = mipi_csi2->data_lanes[i] - 1;
	} else {
		lncfg->data[i].pos = mipi_csi2->data_lanes[i];
	}

where "legacy" is what camss_detect_legacy_phy() already computes, and
the binding example for the PHY-attached case says so. The alternative
is to document that the CAMSS endpoint stays 0-based and fix the two
board files, but having <1 2 3 4> mean two different things on the two
ends of one link seems worse.

Happy to send either as a patch if you prefer; this is your series, so I
did not want to do that uninvited.

Thanks,
Oleg