Re: [PATCH] media: qcom: camss: configure C-PHY in the new csiphy_stream_on()

[Bryan O'Donoghue]

On 17/10/2026 05:05, Anusha Arun Nandi wrote:
> +	const bool cphy = (lncfg->phy_cfg == V4L2_MBUS_CSI2_CPHY);

suggest is_cphy

>   	s64 link_freq;
> +	int i;
>   	int ret;
>   
> -	dphy_cfg = &dphy_opts.mipi_dphy;
> -
> -	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes, false);
> +	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes, cphy);
>   
>   	if (link_freq <= 0) {
>   		dev_err(dev,
> @@ -346,23 +345,49 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>   		return -EINVAL;
>   	}
>   
> -	ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
> -							 dphy_cfg);
> -	if (ret < 0) {
> -		dev_err(dev, "Invalid config for link frequency %lld\n",
> -			link_freq);
> -		return ret;
> -	}
> +	if (cphy) {
> +		struct phy_configure_opts_mipi_cphy *cphy_cfg = &phy_opts.mipi_cphy;
>   
> -	ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
> -	if (ret) {
> -		dev_err(dev, "Failed to set phy mode\n");
> -		return ret;
> +		ret = phy_mipi_cphy_get_default_config_for_hsclk(link_freq, num_lanes,
> +								 cphy_cfg);
> +		if (ret < 0) {
> +			dev_err(dev, "Invalid config for link frequency %lld\n",
> +				link_freq);
> +			return ret;
> +		}
> +
> +		/* Set data trio lane positions and polarities (no clock lane) */
> +		for (i = 0; i < num_lanes; i++) {
> +			cphy_cfg->lane_positions[i] = lncfg->data[i].pos;
> +			cphy_cfg->lane_polarities[i] = lncfg->data[i].pol;
> +		}
> +
> +		ret = phy_set_mode(csiphy->phy, PHY_MODE_MIPI_CPHY);
> +		if (ret) {
> +			dev_err(dev, "Failed to set phy mode\n");
> +			return ret;

We already derive the lane positions and polarities in the PHY driver. 
There's no reason for the CSID block to know or care what the polarities 
or positions are.

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c::phy_qcom_mipi_csi2_parse_routing()

I guess the PHY already knows the polarities and positions - or at least 
knows them for the DPHY case.

        /* lane-polarities: optional, up to num_data_lanes + 1 entries */
        memset(lane_polarities, 0x00, sizeof(lane_polarities));
        num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
        if (num_polarities > 0) {
                if (num_polarities != stream_cfg->num_data_lanes + 1) {
                        ret = -EINVAL;
                        dev_err(dev, "clock+data-lane %d/polarities %d 
mismatch\n",
                                stream_cfg->num_data_lanes + 1, 
num_polarities);
                        goto out_put;
                }

                ret = fwnode_property_read_u32_array(ep, 
"lane-polarities", lane_polarities,
                                                     num_polarities);
                if (ret) {
                        dev_err(dev, "Failed to read lane-polarities: 
%d\n", ret);
                        goto out_put;
                }
        }

        csi2phy->stream_cfg.lane_cfg.clk.pos = CSI2_DEFAULT_CLK_LANE;
        csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];

        for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
                if (data_lanes[i] < 1 || data_lanes[i] > 
CSI2_MAX_DATA_LANES) {
                        dev_err(dev, "Invalid lane %d\n", data_lanes[i]);
                        ret = -EINVAL;
                        goto out_put;
                }

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

---
bod