Re: [PATCH v13 3/5] media: qcom: camss: Add support for PHY API devices
From: Bryan O'Donoghue
Date: Wed Jul 29 2026 - 16:34:12 EST
On 29/07/2026 14:45, Nihal Kumar Gupta wrote:
On 28-07-2026 15:05, Bryan O'Donoghue wrote:
+ ret = PTR_ERR(csd);Hi Bryan,
+ 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;
}
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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxx>
---
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