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

[Bryan O'Donoghue <bod@kernel.org>]

On 29/07/2026 14:45, Nihal Kumar Gupta wrote:
> 
> 
> On 28-07-2026 15:05, Bryan O'Donoghue wrote:
>> +				ret = PTR_ERR(csd);
>> +				goto err_cleanup;
>> +			}
>> +		}
>> +
>>   		if (IS_ERR(csd)) {
>>   			ret = PTR_ERR(csd);
>>   			goto err_cleanup;
>> @@ -4819,6 +4854,29 @@ static int camss_parse_ports(struct camss *camss)
>>   	return ret;
>>   }
> Hi Bryan,
> 
> While rebasing Glymur on the following series I found an issue —
> Could you please review the changes below.
> 
> Tested on
> 1. Glymur CRD with OV08X40, working as expected.
> 2. Monaco EVK with IMX577(legacy path) — working as expected.
> 
> Picked up following for testing:
> - PHY driver:     Add a CSI2 MIPI DPHY driver [v14]
> - CAMSS PHY API:  Add dt-bindings and PHY updates for CAMSS on x1e80100 silicon [v13]
> - Below patchset
> - Glymur DT binding/DTS: locally
> 
> Holding off formal submission until the above stabilise.
> 
> ----------------------------------------------------------------------
>  From b86af6812bb655e885dbcd23ffd28c92177017cd Mon Sep 17 00:00:00 2001
> From: Nihal Kumar Gupta <nihal.gupta@oss.qualcomm.com>
> Date: Wed, 29 Jul 2026 18:18:54 +0530
> Subject: [PATCH] media: qcom: camss: Read lane config from PHY endpoint for
>   PHY API path
> 
> In the new PHY API topology the CAMSS endpoint carries only port/ID
> information. The bus-type and data-lane configuration are described on
> the PHY node's sensor-side endpoint (port@0), not on the CAMSS endpoint.

So I don't think this model is in-keep with established DT practices - 
if a driver needs to parse a property it needs to have that property in 
the device node it handles.

So with data-lanes per existing camss bindings in the IFE/CSID node 
everything though should work as-is for you.

https://gitlab.com/Linaro/arm64-laptops/linux/-/tree/arm64-laptops-v7.2-rc2-camss-v11?ref_type=heads

https://gitlab.com/Linaro/arm64-laptops/linux/-/blob/arm64-laptops-v7.2-rc2-camss-v11/arch/arm64/boot/dts/qcom/x1-crd.dtsi?ref_type=heads#L947

&camss {
	status = "okay";

	ports {
		/*
		 * port0 => csiphy0
		 * port1 => csiphy1
		 * port2 => csiphy2
		 * port3 => csiphy4
		 */
		port@3 {
			camss_csiphy4_inep: endpoint {
				data-lanes = <0 1 2 3>;
				remote-endpoint = <&csiphy4_out_ep>;
			};
		};
	};
};

&cci1 {
	status = "okay";
};

&cci1_i2c1 {
	camera@36 {
		compatible = "ovti,ov08x40";
		reg = <0x36>;

		reset-gpios = <&tlmm 237 GPIO_ACTIVE_LOW>;
		pinctrl-0 = <&cam_rgb_default>;
		pinctrl-names = "default";

		clocks = <&camcc CAM_CC_MCLK4_CLK>;
		assigned-clocks = <&camcc CAM_CC_MCLK4_CLK>;
		assigned-clock-rates = <19200000>;

		orientation = <0>; /* front facing */

		avdd-supply = <&vreg_l7b_2p8>;
		dovdd-supply = <&vreg_l3m_1p8>;

		port {
			ov08x40_out_ep: endpoint {
				data-lanes = <1 2 3 4>;
				link-frequencies = /bits/ 64 <400000000>;
				remote-endpoint = <&csiphy4_in_ep>;
			};
		};
	};
};

&csiphy4 {
	vdda-0p9-supply = <&vreg_l2c_0p8>;
	vdda-1p2-supply = <&vreg_l1c_1p2>;

	status = "okay";

	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		port@0 {
			reg = <0>;
			csiphy4_in_ep: endpoint {
				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
				data-lanes = <0 1 2 3>;
				remote-endpoint = <&ov08x40_out_ep>;
			};
		};

		port@1 {
			reg = <1>;
			csiphy4_out_ep: endpoint {
				remote-endpoint = <&camss_csiphy4_inep>;
			};
		};
	};
};


> Add a phy_in parameter to camss_parse_endpoint_node(). Extract csiphy_id
> from the CAMSS endpoint port number and read bus-type and lane properties
> from phy_in. Check fwnode_device_is_available() on the PHY node and skip
> disabled PHYs. Fix missing fwnode_handle_put() on the disabled-PHY early
> exit path. For legacy DTS phy_in is the CAMSS endpoint itself so existing
> behaviour is preserved.
> 
> Signed-off-by: Nihal Kumar Gupta <nihal.gupta@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/camss/camss.c | 49 +++++++++++++++++------
>   1 file changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 84097d82d99c..7254f2b28eb0 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4742,11 +4742,13 @@ static const struct parent_dev_ops vfe_parent_dev_ops = {
>    */
>   static int camss_parse_endpoint_node(struct device *dev,
>   				     struct fwnode_handle *ep,
> +				     struct fwnode_handle *phy_in,
>   				     struct camss_async_subdev *csd)
>   {
>   	struct csiphy_lanes_cfg *lncfg = &csd->interface.csi2.lane_cfg;
>   	struct v4l2_mbus_config_mipi_csi2 *mipi_csi2;
>   	struct v4l2_fwnode_endpoint vep = { { 0 } };
> +	struct v4l2_fwnode_endpoint phy_vep = { { 0 } };
>   	unsigned int i;
>   	int ret;
> 
> @@ -4754,18 +4756,26 @@ static int camss_parse_endpoint_node(struct device *dev,
>   	if (ret)
>   		return ret;
> 
> +	csd->interface.csiphy_id = vep.base.port;
> +
> +	/* bus-type and lane config from csiphy endpoint (phy_in) */
> +	ret = v4l2_fwnode_endpoint_parse(phy_in, &phy_vep);
> +	if (ret) {
> +		dev_err(dev, "parse_endpoint: failed to parse phy_in ep (%pfw): %d\n",
> +			phy_in, ret);
> +		return ret;
> +	}
> +
>   	/*
>   	 * Most SoCs support both D-PHY and C-PHY standards, but currently only
>   	 * D-PHY is supported in the driver.
>   	 */
> -	if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> -		dev_err(dev, "Unsupported bus type %d\n", vep.bus_type);
> +	if (phy_vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> +		dev_err(dev, "Unsupported bus type %d\n", phy_vep.bus_type);
>   		return -EINVAL;
>   	}
> 
> -	csd->interface.csiphy_id = vep.base.port;
> -
> -	mipi_csi2 = &vep.bus.mipi_csi2;
> +	mipi_csi2 = &phy_vep.bus.mipi_csi2;
>   	lncfg->clk.pos = mipi_csi2->clock_lane;
>   	lncfg->clk.pol = mipi_csi2->lane_polarities[0];
>   	lncfg->num_data = mipi_csi2->num_data_lanes;
> @@ -4798,15 +4808,15 @@ static int camss_parse_ports(struct camss *camss)
> 
>   	fwnode_graph_for_each_endpoint(fwnode, ep) {
>   		struct camss_async_subdev *csd;
> -
> -		if (!fwnode_device_is_available(ep))
> -			continue;
> +		struct fwnode_handle *phy_in;
> 
>   		if (camss->legacy_phy) {
>   			csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
>   							      typeof(*csd));
> +			/* legacy DTS has bus-type/data-lanes on the camss endpoint */
> +			phy_in = fwnode_handle_get(ep);
>   		} else {
> -			struct fwnode_handle *phy_out, *phy_node, *phy_in, *sensor_ep;
> +			struct fwnode_handle *phy_out, *phy_node, *sensor_ep;
> 
>   			phy_out = fwnode_graph_get_remote_endpoint(ep);
>   			if (!phy_out)
> @@ -4817,31 +4827,44 @@ static int camss_parse_ports(struct camss *camss)
>   			if (!phy_node)
>   				continue;
> 
> +			if (!fwnode_device_is_available(phy_node)) {
> +				dev_info(dev, "parse_ports: phy_node=%s disabled\n",
> +				 of_node_full_name(to_of_node(phy_node)));
> +				continue;
> +			};

This is a correct change I can take into my patch - I'll add you down as 
Co-developed by.

> +
> +			/* port@0 of the phy node is the sensor-side input port */
>   			phy_in = fwnode_graph_get_endpoint_by_id(phy_node, 0, 0, 0);
>   			fwnode_handle_put(phy_node);
> -			if (!phy_in)
> +			if (!phy_in) {
> +				dev_info(dev, "parse_ports: csiphy port@0 has no endpoint, skip\n");
>   				continue;
> +			};
> 
>   			sensor_ep = fwnode_graph_get_remote_endpoint(phy_in);
> -			fwnode_handle_put(phy_in);
> -			if (!sensor_ep)
> +			if (!sensor_ep) {
> +				fwnode_handle_put(phy_in);
>   				continue;
> +			};
> 
>   			csd = v4l2_async_nf_add_fwnode(&camss->notifier, sensor_ep,
>   						struct camss_async_subdev);
>   			fwnode_handle_put(sensor_ep);
>   			if (IS_ERR(csd)) {
> +				fwnode_handle_put(phy_in);
>   				ret = PTR_ERR(csd);
>   				goto err_cleanup;
>   			}
>   		}
> 
>   		if (IS_ERR(csd)) {
> +			fwnode_handle_put(phy_in);
>   			ret = PTR_ERR(csd);
>   			goto err_cleanup;
>   		}
> 
> -		ret = camss_parse_endpoint_node(dev, ep, csd);
> +		ret = camss_parse_endpoint_node(dev, ep, phy_in, csd);
> +		fwnode_handle_put(phy_in);
>   		if (ret < 0)
>   			goto err_cleanup;
>   	}
> --
> 2.34.1