Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
PHY devices.
The hardware can support both CPHY, DPHY and a special split-mode DPHY. We
capture those modes as:
- PHY_QCOM_CSI2_MODE_DPHY
- PHY_QCOM_CSI2_MODE_CPHY
- PHY_QCOM_CSI2_MODE_SPLIT_DPHY
The CSIPHY devices have their own pinouts on the SoC as well as their own
individual voltage rails.
The need to model voltage rails on a per-PHY basis leads us to define
CSIPHY devices as individual nodes.
Two nice outcomes in terms of schema and DT arise from this change.
1. The ability to define on a per-PHY basis voltage rails.
2. The ability to require those voltage.
We have had a complete bodge upstream for this where a single set of
voltage rail for all CSIPHYs has been buried inside of CAMSS.
Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in
CAMSS parlance, the CSIPHY devices should be individually modelled.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++++++++++++
include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++
2 files changed, 145 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
new file mode 100644
index 0000000000000..63114151104b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CSI2 PHY
+
+maintainers:
+ - Bryan O'Donoghue <bod@kernel.org>
+
+description:
+ Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
+ to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
+ modes.
+
+properties:
+ compatible:
+ const: qcom,x1e80100-csi2-phy
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 1
+ description:
+ The single cell specifies the PHY operating mode.
+ See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: core
+ - const: timer
+
+ interrupts:
+ maxItems: 1
+
+ operating-points-v2:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description: MXC or MXA voltage rail
+ - description: MMCX voltage rail
+
+ power-domain-names:
+ items:
+ - const: mx
+ - const: mmcx
+
+ vdda-0p9-supply:
+ description: Phandle to a 0.9V regulator supply to a PHY.
+
+ vdda-1p2-supply:
+ description: Phandle to 1.2V regulator supply to a PHY.
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+ - clocks
+ - clock-names
+ - interrupts
+ - operating-points-v2
+ - power-domains
+ - power-domain-names
+ - vdda-0p9-supply
+ - vdda-1p2-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
+ #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
+ #include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ csiphy4: csiphy@ace4000 {
+ compatible = "qcom,x1e80100-csi2-phy";
+ reg = <0x0ace4000 0x2000>;
+ #phy-cells = <1>;
+
+ clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
+ clock-names = "core",
+ "timer";
+
+ operating-points-v2 = <&csiphy_opp_table>;
+
+ interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+ power-domains = <&rpmhpd RPMHPD_MX>,
+ <&rpmhpd RPMHPD_MMCX>;
+ power-domain-names = "mx",
+ "mmcx";
+
+ vdda-0p9-supply = <&vreg_l2c_0p8>;
+ vdda-1p2-supply = <&vreg_l1c_1p2>;
+ };
+
+ csiphy_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ required-opps = <&rpmhpd_opp_low_svs_d1>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ required-opps = <&rpmhpd_opp_low_svs>,
+ <&rpmhpd_opp_low_svs>;
+ };
+
+ opp-480000000 {
+ opp-hz = /bits/ 64 <480000000>;
+ required-opps = <&rpmhpd_opp_low_svs>,
+ <&rpmhpd_opp_low_svs>;
+ };
+ };
+
+ isp@acb7000 {
+ phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
+ };
diff --git a/include/dt-bindings/phy/phy-qcom-mipi-csi2.h b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
new file mode 100644
index 0000000000000..fa48fd75c58d8
--- /dev/null
+++ b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Qualcomm MIPI CSI2 PHY constants
+ *
+ * Copyright (C) 2026 Linaro Limited
+ */
+
+#ifndef __DT_BINDINGS_PHY_MIPI_CSI2__
+#define __DT_BINDINGS_PHY_MIPI_CSI2__
+
+#define PHY_QCOM_CSI2_MODE_DPHY 0
+#define PHY_QCOM_CSI2_MODE_CPHY 1
+#define PHY_QCOM_CSI2_MODE_SPLIT_DPHY 2
+
+#endif /* __DT_BINDINGS_PHY_MIPI_CSI2__ */
--
2.52.0On 3/26/26 2:04 AM, Bryan O'Donoghue wrote: > Add a base schema initially compatible with x1e80100 to describe MIPI CSI2 > PHY devices. > > The hardware can support both CPHY, DPHY and a special split-mode DPHY. We > capture those modes as: > > - PHY_QCOM_CSI2_MODE_DPHY > - PHY_QCOM_CSI2_MODE_CPHY > - PHY_QCOM_CSI2_MODE_SPLIT_DPHY Does the _PHY_ DT node need to be aware about this upfront? If we have some sideband signal (e.g. the sensor driver specifically requesting C-PHY mode), we can simply throw all this complexity into phy_mode + phy_configure_opts, all at runtime Further, the combo/split mode may possibly be selected through aggregation of requests. The question remains whether the sensor should have a direct connection to the PHY itself (i.e. phys = <&csiphyN> or of_graph straight into the PHY) or whether it's going to be translated by the camss node (which would be the one holding a PHY reference) - there's probably surface for adding such negotiation logic in both places Note this is a question and I'm not aware of all the possible combinations Konrad
On 27/03/2026 10:07, Konrad Dybcio wrote: > On 3/26/26 2:04 AM, Bryan O'Donoghue wrote: >> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2 >> PHY devices. >> >> The hardware can support both CPHY, DPHY and a special split-mode DPHY. We >> capture those modes as: >> >> - PHY_QCOM_CSI2_MODE_DPHY >> - PHY_QCOM_CSI2_MODE_CPHY >> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY > Does the_PHY_ DT node need to be aware about this upfront? Yeah that's a fair question. The standard model is to pass the mode via binding right now. You _could_ configure it @ run time in principle. And you could even conceive of a sensor hardware that might find value in that - but IMO it's a 100% hypothetical use-case - you'd basically need an FPGA that could output CPHY, DPHY or for some reason SPLIT_MODE DPHY. But that's a pretty off the wall use-case to hypothesize. Split-mode OTOH is a board-level physical reality which => a DT description not a runtime choice. > If we have some sideband signal (e.g. the sensor driver specifically > requesting C-PHY mode), we can simply throw all this complexity into > phy_mode + phy_configure_opts, all at runtime Like I say its conceivable but IMO not a real thing and unless your sensor is an FPGA not possible to support in real hardware. > Further, the combo/split mode may possibly be selected through > aggregation of requests. > > The question remains whether the sensor should have a direct connection to > the PHY itself (i.e. phys = <&csiphyN> or of_graph straight into the PHY) > or whether it's going to be translated by the camss node (which would be > the one holding a PHY reference) - there's probably surface for adding such > negotiation logic in both places To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ? The model we have right now, right or wrong is sensor endpoint to controller. Similarly the <&phy MODE_GOES_HERE> is a pattern Rob Herring suggested and IMO is a consistent pattern we should aim for upstream. We see it in latest Rockchip, Cadence. > Note this is a question and I'm not aware of all the possible combinations > > Konrad
Le 27/03/2026 à 15:38, Bryan O'Donoghue a écrit : > On 27/03/2026 10:07, Konrad Dybcio wrote: >> On 3/26/26 2:04 AM, Bryan O'Donoghue wrote: >>> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2 >>> PHY devices. >>> >>> The hardware can support both CPHY, DPHY and a special split-mode DPHY. We >>> capture those modes as: >>> >>> - PHY_QCOM_CSI2_MODE_DPHY >>> - PHY_QCOM_CSI2_MODE_CPHY >>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY >> Does the_PHY_ DT node need to be aware about this upfront? > > Yeah that's a fair question. > > The standard model is to pass the mode via binding right now. You _could_ configure it @ run time in principle. > > And you could even conceive of a sensor hardware that might find value in that - but IMO it's a 100% hypothetical use-case - you'd basically need an FPGA that could output CPHY, DPHY or for some reason SPLIT_MODE DPHY. > > But that's a pretty off the wall use-case to hypothesize. Split-mode OTOH is a board-level physical reality which => a DT description not a runtime choice. > >> If we have some sideband signal (e.g. the sensor driver specifically >> requesting C-PHY mode), we can simply throw all this complexity into >> phy_mode + phy_configure_opts, all at runtime > > Like I say its conceivable but IMO not a real thing and unless your sensor is an FPGA not possible to support in real hardware. > >> Further, the combo/split mode may possibly be selected through >> aggregation of requests. >> >> The question remains whether the sensor should have a direct connection to >> the PHY itself (i.e. phys = <&csiphyN> or of_graph straight into the PHY) >> or whether it's going to be translated by the camss node (which would be >> the one holding a PHY reference) - there's probably surface for adding such >> negotiation logic in both places > > To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ? This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately. The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline. > > The model we have right now, right or wrong is sensor endpoint to controller. Similarly the <&phy MODE_GOES_HERE> is a pattern Rob Herring suggested and IMO is a consistent pattern we should aim for upstream. We see it in latest Rockchip, Cadence. This doesn’t properly describe the data path, we got out of this classic scheme when implementing the whole USB-C complex on qualcomm SoCs where we had to describe the data flow within all the USB/DSP/DP/PHYs elements, and we have a link between the DP controller and the Combo PHY acting as a DRM bridge. And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here. Looking at other vendors and sticking to this is wrong, we need to solve how to describe the Qualcomm CAMSS complex with accurate representation of the data flow and hw components interconnections. The choice of kernel API to access an HW element is irrelevant in the discussion. Starting refactoring the DT representation of CAMSS is good, starting with PHY is weird since I don't see what it does solve we can't solve already, but you need to take in account how the CSIPHYs are connected to other HW blocks. Simply adding a "phys =" reference from CAMSS node doesn't reflect at all which HW element will consume the PHY, and what are the link parameters between the consumer and the PHY and between the PHY and the Sensor. Those are the whole meaning of the port/endpoint links, where we can define multiple endpoints for a same connection to, for example, define multiple sensors on a same PHY for the split-mode. Neil > > >> Note this is a question and I'm not aware of all the possible combinations >> >> Konrad >
On 27/03/2026 15:28, Neil Armstrong wrote: >> To be frankly honest you can make an argument for it either way. >> However my honestly held position is analysing other upstream >> implementations connecting to the PHY means we can't make the PHY >> device a drivers/phy device - it would have to be a V4L2 device and >> then for me the question is why is that even required ? > > This is plain wrong, DT definition is different from software > implementation, you can do whatever you want if you describe HW accurately. I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so. I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing. However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow. We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints. Is that what you want ? > The CSIPHYs are not tied to a single "consumer" block, they can be > connected to different consumers at runtime, which is not something > classic PHY devices are designed for. So they are de facto a media > element in the dynamic camera pipeline. The existing CAMSS binding and media graph are not changed by this series. > And actually Rob Herring asked use to define the complete data flow, it > was a strong requirement. I don't see why we wouldn't here. I'm implementing feedback from Rob. https://lore.kernel.org/linux-media/20250710230846.GA44483-robh@kernel.org/ To me, here is where we stand: - Individual nodes - we all agree that - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry I'm fine with it too. - drivers/phy - I think we are accepting this is also fine ? - endpoints should flow into the PHY and then back to the controller I get that argument. In fact I _like_ that argument at least I like my conception of that argument. I'll stipulate to that argument meaning then that, new CSI2 PHYs shall include endpoints for this purpose globally. As I've said before, there's nothing Qualcomm specific about this discussion, really. --- bod
On 3/27/26 18:42, Bryan O'Donoghue wrote: > On 27/03/2026 15:28, Neil Armstrong wrote: >>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ? >> >> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately. > > I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so. I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle. The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/poweroff()/exit() directly from the CSIPHY media callbacks. > > I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing. > > However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow. It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it... In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware. > > We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints. We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard. I mean this is common sense, why would we continue to stick to the current CAMSS bindings ??? > > Is that what you want ? > >> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline. > > The existing CAMSS binding and media graph are not changed by this series. This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately. In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software. Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux. > >> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here. > > I'm implementing feedback from Rob. > > https://lore.kernel.org/linux-media/20250710230846.GA44483-robh@kernel.org/ Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ? > > To me, here is where we stand: > > - Individual nodes - we all agree that > - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry > I'm fine with it too. > - drivers/phy - I think we are accepting this is also fine ? Like I said this adds a supplementary API layer for no reason and will make life harder, but I don't care personally. > - endpoints should flow into the PHY and then back to the controller > > I get that argument. In fact I _like_ that argument at least I like my conception of that argument. > > I'll stipulate to that argument meaning then that, new CSI2 PHYs shall include endpoints for this purpose globally. > > As I've said before, there's nothing Qualcomm specific about this discussion, really. There is, because the current Qualcomm CAMSS bindings are insufficient and should be entirely redesigned from the ground up to properly describe the HW. Neil > > --- > bod
On 30/03/2026 08:49, Neil Armstrong wrote: > On 3/27/26 18:42, Bryan O'Donoghue wrote: >> On 27/03/2026 15:28, Neil Armstrong wrote: >>>> To be frankly honest you can make an argument for it either way. >>>> However my honestly held position is analysing other upstream >>>> implementations connecting to the PHY means we can't make the PHY >>>> device a drivers/phy device - it would have to be a V4L2 device and >>>> then for me the question is why is that even required ? >>> >>> This is plain wrong, DT definition is different from software >>> implementation, you can do whatever you want if you describe HW >>> accurately. >> >> I'm not sure what point it is you are trying to make here. Are you >> trying to say drivers/phy is OK with you but you want an endpoint ? If >> so, please just say so. > > I'm against using the "phys = <>" property in the CAMSS to reference the > PHYs, a "PHY" in the classic terminology is tied to a single consumer, > and if it can be shared to multiple consumer you must model a mux or > whatever in the middle. The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework. DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern. > The PHY API as an internal software implementation is probably fine, > even if it makes implementation of split mode much much harder and > doesn't really solve anything, you can just call init()/poweron()/ > poweroff()/exit() directly from the CSIPHY media callbacks. Great. >> I can see an argument for that hence my response to Konrad, I just >> don't see why its a Qualcomm specific argument and of course >> understood stuff bubbles up in review, we have a public debate and >> come to a consensus - that's a good thing. >> >> However, I'd want wider buy-in and understanding that endpoints in the >> PHYs is a more accurate description of the data-flow. > > It is, and it was designed for that, and extensively used in the media > DT representation, so I wonder here you would not use it... > In an ideal world, you would add nodes for each CAMSS hw elements and > adds port/endpoints links between all nodes to describe the data graph, > this would be used to construct the media controller graph, and make it > much easier supporting new hardware. Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY. Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way. > >> >> We've been applying DT bindings aplenty without that so far. So we >> would establish new CSI2 PHY bindings should represent the sensor >> endpoints. > > We've been using a dummy representation of CAMM in a single node with > only endpoints connecting to the sensors and hiding all the hardware > layout in code, it doesn't scale and makes supporting new HW hard. > I mean this is common sense, why would we continue to stick to the > current CAMSS bindings ??? We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until. I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months... Anyway one conclusion of that is we want to transition to individual nodes for everything. PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches. drivers/phy notwithstanding. >> >> Is that what you want ? >> >>> The CSIPHYs are not tied to a single "consumer" block, they can be >>> connected to different consumers at runtime, which is not something >>> classic PHY devices are designed for. So they are de facto a media >>> element in the dynamic camera pipeline. >> >> The existing CAMSS binding and media graph are not changed by this >> series. > > This is not my point, I don't care about the software implementation at > all, I care about accurate hardware representation. Using the "phys = > <>" property does not describe hardware accurately. > > In other words: The CSIPHY are not connected to CAMSS. This is _not_ > true, tying the CSIPHYs to the CAMSS block hides the real data muxing in > software. > > Please remind DT is used by multiple operating systems, and properly > describing hardware in DT will help have good software support over all > OSes, not just Linux. > >> >>> And actually Rob Herring asked use to define the complete data flow, >>> it was a strong requirement. I don't see why we wouldn't here. >> >> I'm implementing feedback from Rob. >> >> https://lore.kernel.org/linux-media/20250710230846.GA44483- >> robh@kernel.org/ > > Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs > are muxed to multiple consumers which are burried in the CAMSS code ? I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID. You seriously want a mux device in the kernel to model one bit in a register ? No. >> >> To me, here is where we stand: >> >> - Individual nodes - we all agree that >> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry >> I'm fine with it too. >> - drivers/phy - I think we are accepting this is also fine ? > > Like I said this adds a supplementary API layer for no reason and will > make life harder, but I don't care personally. > >> - endpoints should flow into the PHY and then back to the controller >> >> I get that argument. In fact I _like_ that argument at least I like my >> conception of that argument. >> >> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall >> include endpoints for this purpose globally. >> >> As I've said before, there's nothing Qualcomm specific about this >> discussion, really. > > There is, because the current Qualcomm CAMSS bindings are insufficient > and should be entirely redesigned from the ground up to properly > describe the HW. The long term plan is to do that. Discussed extensively with the Qcom teams delivering CAMSS, even invited community members along. Here are the meeting notes - its all in the public domain https://docs.google.com/document/d/1yUwWHaKLuovKVgGTzvKm68K1zS6xBONVzjOsRjDl03U/edit?tab=t.0#heading=h.mtbm7qfohwfs --- bod
On 3/30/26 12:02, Bryan O'Donoghue wrote: > On 30/03/2026 08:49, Neil Armstrong wrote: >> On 3/27/26 18:42, Bryan O'Donoghue wrote: >>> On 27/03/2026 15:28, Neil Armstrong wrote: >>>>> To be frankly honest you can make an argument for it either way. >>>>> However my honestly held position is analysing other upstream >>>>> implementations connecting to the PHY means we can't make the PHY >>>>> device a drivers/phy device - it would have to be a V4L2 device and >>>>> then for me the question is why is that even required ? >>>> >>>> This is plain wrong, DT definition is different from software >>>> implementation, you can do whatever you want if you describe HW >>>> accurately. >>> >>> I'm not sure what point it is you are trying to make here. Are you >>> trying to say drivers/phy is OK with you but you want an endpoint ? If >>> so, please just say so. >> >> I'm against using the "phys = <>" property in the CAMSS to reference the >> PHYs, a "PHY" in the classic terminology is tied to a single consumer, >> and if it can be shared to multiple consumer you must model a mux or >> whatever in the middle. > > The CSIPHY-to-CSID routing is runtime-configurable and is already > managed by the media controller framework. > > DT describes static hardware connections. The dynamic mux is a software > concern, not a hardware description concern. > > >> The PHY API as an internal software implementation is probably fine, >> even if it makes implementation of split mode much much harder and >> doesn't really solve anything, you can just call init()/poweron()/ >> poweroff()/exit() directly from the CSIPHY media callbacks. > > Great. > >>> I can see an argument for that hence my response to Konrad, I just >>> don't see why its a Qualcomm specific argument and of course >>> understood stuff bubbles up in review, we have a public debate and >>> come to a consensus - that's a good thing. >>> >>> However, I'd want wider buy-in and understanding that endpoints in the >>> PHYs is a more accurate description of the data-flow. >> >> It is, and it was designed for that, and extensively used in the media >> DT representation, so I wonder here you would not use it... >> In an ideal world, you would add nodes for each CAMSS hw elements and >> adds port/endpoints links between all nodes to describe the data graph, >> this would be used to construct the media controller graph, and make it >> much easier supporting new hardware. > > Yes but be pragmatic Neil. The first step in making the monolith into > sub-nodes is the CSIPHY. > > Once the CSIPHY is in, we can follow on with adding new nodes that way > IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff > in that new way. > > >> >>> >>> We've been applying DT bindings aplenty without that so far. So we >>> would establish new CSI2 PHY bindings should represent the sensor >>> endpoints. >> >> We've been using a dummy representation of CAMM in a single node with >> only endpoints connecting to the sensors and hiding all the hardware >> layout in code, it doesn't scale and makes supporting new HW hard. >> I mean this is common sense, why would we continue to stick to the >> current CAMSS bindings ??? > > We _won't_ I just don't support a big bang integration. Progressive > changes over a longer timeline make the transition manageable, without > accepting endless sub-standard stuff in the meantime or holding up all > new SoC submission unless/until. > > I mean there is a CAMSS meeting which I've been running for nearly a > year now that both you and Vlad are invited to where we have been > discussing this for months... The established process of Linux kernel development is based on email discussions, the sent changes including CSIPHY dt bindings were plainly ignored by the maintainer, because it's a NIH material or whatever. There was a chance to discuss CSIPHY dt bindings changes one year ago, now it might be not a coincidence that eventually after the series of updates the new CSIPHY dt bindings will be very close to the once displayed ones, it took a year, but still this is a good progress IMO. -- Best wishes, Vladimir
On 3/30/26 11:02, Bryan O'Donoghue wrote: > On 30/03/2026 08:49, Neil Armstrong wrote: >> On 3/27/26 18:42, Bryan O'Donoghue wrote: >>> On 27/03/2026 15:28, Neil Armstrong wrote: >>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ? >>>> >>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately. >>> >>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so. >> >> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle. > > The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework. This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node. > > DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern. DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described. > > >> The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/ poweroff()/exit() directly from the CSIPHY media callbacks. > > Great. > >>> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing. >>> >>> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow. >> >> It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it... >> In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware. > > Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY. I am, and I agree it's fine to do it step by step. > > Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way. I agree on the approach, I never said otherwise, but you need to have the big picture in mind. When you'll have the CSID subnodes, where will you add the phys properties ? you'll keep them in the CAMSS top node ? add all CSIPHY to all CSID nodes ? this is wrong and this needs to be fixed now. > > >> >>> >>> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints. >> >> We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard. >> I mean this is common sense, why would we continue to stick to the current CAMSS bindings ??? > > We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until. > > I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months... > > Anyway one conclusion of that is we want to transition to individual nodes for everything. > > PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches. > > drivers/phy notwithstanding. As I said I agree on the progressive approach, not the PHY DT bindins approach. > >>> >>> Is that what you want ? >>> >>>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline. >>> >>> The existing CAMSS binding and media graph are not changed by this series. >> >> This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately. >> >> In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software. >> >> Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux. >> >>> >>>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here. >>> >>> I'm implementing feedback from Rob. >>> >>> https://lore.kernel.org/linux-media/20250710230846.GA44483- robh@kernel.org/ >> >> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ? > > I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID. Honestly I don't care about the userspace media graph, this is a software problem and we can totally make the transition seamless if we want. Don't limit the DT hardware description because of a software userspace ABI breakage, this approach is not the right one. > > You seriously want a mux device in the kernel to model one bit in a register ? Why not ? We have drivers that even toggles even single bit to solve those kind of situations. Physically they're a mux to route the CSIPHY to the consumers, it's a fact... even if it's a register or a single bit. > > No. > >>> >>> To me, here is where we stand: >>> >>> - Individual nodes - we all agree that >>> - As sub-nodes - I think the majority agrees this Krzsztof, Dmitry >>> I'm fine with it too. >>> - drivers/phy - I think we are accepting this is also fine ? >> >> Like I said this adds a supplementary API layer for no reason and will make life harder, but I don't care personally. >> >>> - endpoints should flow into the PHY and then back to the controller >>> >>> I get that argument. In fact I _like_ that argument at least I like my conception of that argument. >>> >>> I'll stipulate to that argument meaning then that, new CSI2 PHYs shall include endpoints for this purpose globally. >>> >>> As I've said before, there's nothing Qualcomm specific about this discussion, really. >> >> There is, because the current Qualcomm CAMSS bindings are insufficient and should be entirely redesigned from the ground up to properly describe the HW. > > The long term plan is to do that. Discussed extensively with the Qcom teams delivering CAMSS, even invited community members along. > > Here are the meeting notes - its all in the public domain > > https://docs.google.com/document/d/1yUwWHaKLuovKVgGTzvKm68K1zS6xBONVzjOsRjDl03U/edit?tab=t.0#heading=h.mtbm7qfohwfs > > --- > bod
On 30/03/2026 10:17, Neil Armstrong wrote: > On 3/30/26 11:02, Bryan O'Donoghue wrote: >> On 30/03/2026 08:49, Neil Armstrong wrote: >>> On 3/27/26 18:42, Bryan O'Donoghue wrote: >>>> On 27/03/2026 15:28, Neil Armstrong wrote: >>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ? >>>>> >>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately. >>>> >>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so. >>> >>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle. >> >> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework. > > This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node. > >> >> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern. > > DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described. But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_. When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to. > >> >> >>> The PHY API as an internal software implementation is probably fine, even if it makes implementation of split mode much much harder and doesn't really solve anything, you can just call init()/poweron()/ poweroff()/exit() directly from the CSIPHY media callbacks. >> >> Great. >> >>>> I can see an argument for that hence my response to Konrad, I just don't see why its a Qualcomm specific argument and of course understood stuff bubbles up in review, we have a public debate and come to a consensus - that's a good thing. >>>> >>>> However, I'd want wider buy-in and understanding that endpoints in the PHYs is a more accurate description of the data-flow. >>> >>> It is, and it was designed for that, and extensively used in the media DT representation, so I wonder here you would not use it... >>> In an ideal world, you would add nodes for each CAMSS hw elements and adds port/endpoints links between all nodes to describe the data graph, this would be used to construct the media controller graph, and make it much easier supporting new hardware. >> >> Yes but be pragmatic Neil. The first step in making the monolith into sub-nodes is the CSIPHY. > > I am, and I agree it's fine to do it step by step. Cool. So let's talk about muxing to CSID devices, when we have CSID devices in the DT. >> >> Once the CSIPHY is in, we can follow on with adding new nodes that way IPE, BPS, ICP, JPEG whatever and also work on implementing the old stuff in that new way. > > I agree on the approach, I never said otherwise, but you need to have the big picture in mind. > > When you'll have the CSID subnodes, where will you add the phys properties ? you'll keep them in the CAMSS top node ? add all CSIPHY to all CSID nodes ? this is wrong and this needs to be fixed now. > >> >> >>> >>>> >>>> We've been applying DT bindings aplenty without that so far. So we would establish new CSI2 PHY bindings should represent the sensor endpoints. >>> >>> We've been using a dummy representation of CAMM in a single node with only endpoints connecting to the sensors and hiding all the hardware layout in code, it doesn't scale and makes supporting new HW hard. >>> I mean this is common sense, why would we continue to stick to the current CAMSS bindings ??? >> >> We _won't_ I just don't support a big bang integration. Progressive changes over a longer timeline make the transition manageable, without accepting endless sub-standard stuff in the meantime or holding up all new SoC submission unless/until. >> >> I mean there is a CAMSS meeting which I've been running for nearly a year now that both you and Vlad are invited to where we have been discussing this for months... >> >> Anyway one conclusion of that is we want to transition to individual nodes for everything. >> >> PHY is the first step which I'm taking because its easier for me as CAMSS maintainer to convince the CAMSS maintainer to take the relevant patches. >> >> drivers/phy notwithstanding. > > As I said I agree on the progressive approach, not the PHY DT bindins approach. > >> >>>> >>>> Is that what you want ? >>>> >>>>> The CSIPHYs are not tied to a single "consumer" block, they can be connected to different consumers at runtime, which is not something classic PHY devices are designed for. So they are de facto a media element in the dynamic camera pipeline. >>>> >>>> The existing CAMSS binding and media graph are not changed by this series. >>> >>> This is not my point, I don't care about the software implementation at all, I care about accurate hardware representation. Using the "phys = <>" property does not describe hardware accurately. >>> >>> In other words: The CSIPHY are not connected to CAMSS. This is _not_ true, tying the CSIPHYs to the CAMSS block hides the real data muxing in software. >>> >>> Please remind DT is used by multiple operating systems, and properly describing hardware in DT will help have good software support over all OSes, not just Linux. >>> >>>> >>>>> And actually Rob Herring asked use to define the complete data flow, it was a strong requirement. I don't see why we wouldn't here. >>>> >>>> I'm implementing feedback from Rob. >>>> >>>> https://lore.kernel.org/linux-media/20250710230846.GA44483- robh@kernel.org/ >>> >>> Where did he ask using the PHY DT bindings ? Is he aware those CSIPHYs are muxed to multiple consumers which are burried in the CAMSS code ? >> >> I freely admit to taking the initiative of phys = <> but Neil there is _no_change_ to the media graph and the "mux" is a runtime configuration that is one register in the CSID. > > Honestly I don't care about the userspace media graph, this is a software problem and we can totally make the transition seamless if we want. > > Don't limit the DT hardware description because of a software userspace ABI breakage, this approach is not the right one. > >> >> You seriously want a mux device in the kernel to model one bit in a register ? > > Why not ? We have drivers that even toggles even single bit to solve those kind of situations. > > Physically they're a mux to route the CSIPHY to the consumers, it's a fact... even if it's a register or a single bit. That's fine. I can understanding making that case if/when CSID becomes its own node but, I don't think it makes sense when connecting the PHY back to the monolith. --- bod
On 3/30/26 11:25 AM, Bryan O'Donoghue wrote:
> On 30/03/2026 10:17, Neil Armstrong wrote:
>> On 3/30/26 11:02, Bryan O'Donoghue wrote:
>>> On 30/03/2026 08:49, Neil Armstrong wrote:
>>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
>>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
>>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
>>>>>>
>>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
>>>>>
>>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
>>>>
>>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
>>>
>>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
>>
>> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
>>
>>>
>>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
>>
>> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
>
> But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_.
>
> When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to.
Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
the resulting nodes would either look like:
// hardcoded, m may != n
csid_n: csid@1000000 {
phys = <&csiphy_m>;
};
or
// determined at runtime
csid_n: csid@1000000 {
phys = <&csiphy_0>,
[...]
<&csiphy_n-1>;
};
or we could store them once, centrally, in the "CAMSS_TOP" node and
pass handles around as necessary:
// camss "catalog/manager" driver/library provides CSIDn with PHYm
camss: camss@10000000 {
phys = <&csiphy_0>,
[...]
<&csiphy_n-1>;
csid_n: csid@1000 {
// no PHY references
};
};
Konrad
On Mon, Mar 30, 2026 at 01:34:57PM +0200, Konrad Dybcio wrote:
> On 3/30/26 11:25 AM, Bryan O'Donoghue wrote:
> > On 30/03/2026 10:17, Neil Armstrong wrote:
> >> On 3/30/26 11:02, Bryan O'Donoghue wrote:
> >>> On 30/03/2026 08:49, Neil Armstrong wrote:
> >>>> On 3/27/26 18:42, Bryan O'Donoghue wrote:
> >>>>> On 27/03/2026 15:28, Neil Armstrong wrote:
> >>>>>>> To be frankly honest you can make an argument for it either way. However my honestly held position is analysing other upstream implementations connecting to the PHY means we can't make the PHY device a drivers/phy device - it would have to be a V4L2 device and then for me the question is why is that even required ?
> >>>>>>
> >>>>>> This is plain wrong, DT definition is different from software implementation, you can do whatever you want if you describe HW accurately.
> >>>>>
> >>>>> I'm not sure what point it is you are trying to make here. Are you trying to say drivers/phy is OK with you but you want an endpoint ? If so, please just say so.
> >>>>
> >>>> I'm against using the "phys = <>" property in the CAMSS to reference the PHYs, a "PHY" in the classic terminology is tied to a single consumer, and if it can be shared to multiple consumer you must model a mux or whatever in the middle.
> >>>
> >>> The CSIPHY-to-CSID routing is runtime-configurable and is already managed by the media controller framework.
> >>
> >> This is not compatible with the PHY bindings if you don't have a defined MUX device in the middle, it's wrong. You're hiding the muxing details in the CAMSS blob node.
> >>
> >>>
> >>> DT describes static hardware connections. The dynamic mux is a software concern, not a hardware description concern.
> >>
> >> DT must describe the possible interconnections between the nodes, if a PHY can be used by multiple hardware components, it must be described.
> >
> > But right now the CAMSS block is described as a single block. There is no CSID device in the kernel _yet_.
> >
> > When we break CSID into its own block then fine, lets have a debate about a mux then but right now the "nodes" are CAMSS[MONOLITH] <=> CSIPHY there is no DT CSID device to model this to.
>
> Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
> the resulting nodes would either look like:
>
> // hardcoded, m may != n
> csid_n: csid@1000000 {
> phys = <&csiphy_m>;
> };
>
> or
>
> // determined at runtime
> csid_n: csid@1000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
> };
I think the bigger problem is:
&csid_L: {
phys = <&csiphy_M>;
};
&csid_N: {
phys = <&csiphy_M>;
};
aka split mode.
>
> or we could store them once, centrally, in the "CAMSS_TOP" node and
> pass handles around as necessary:
>
> // camss "catalog/manager" driver/library provides CSIDn with PHYm
> camss: camss@10000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
>
> csid_n: csid@1000 {
> // no PHY references
> };
> };
>
> Konrad
--
With best wishes
Dmitry
On 30/03/2026 12:49, Dmitry Baryshkov wrote:
>> // determined at runtime
>> csid_n: csid@1000000 {
>> phys = <&csiphy_0>,
>> [...]
>> <&csiphy_n-1>;
>> };
> I think the bigger problem is:
>
> &csid_L: {
> phys = <&csiphy_M>;
> };
>
> &csid_N: {
> phys = <&csiphy_M>;
> };
>
> aka split mode.
Depends on how you model it.
It feels like a philosophical as opposed to an engineering debate in a way.
The CSIPHY block is defined as one thing - is split mode one PHY or two ?
You could argue it either way BUT one strong argument for it being one
PHY is - voltage rails, input clocks etc power the block.
Sure there is an esoteric mode called split or combo mode but the
hardware block itself the thing we usually call the PHY lives as a block
diagram as a discreet entity.
So I think split-mode really is more like
&csid_L: {
phys = <&csiphy_0 DPHY>, <&csiphy_1 CPHY>;
};
&csid_M: {
phys = <&csiphy_0 DPHY>, <&csiphy_1 CPHY>;
};
Recall the debate about a mux is because CSID may connect to any CSIPHY.
csiphy_0: {
clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
<&camcc CAM_CC_CSI0PHYTIMER_CLK>;
clock-names = "core",
"timer";
operating-points-v2 = <&csiphy_opp_table>;
interrupts = <GIC_SPI 1 IRQ_TYPE_EDGE_RISING>;
power-domains = <&rpmhpd RPMHPD_MX>,
<&rpmhpd RPMHPD_MMCX>;
power-domain-names = "mx",
"mmcx";
vdda-0p9-supply = <&vreg_xyz_0p8>;
vdda-1p2-supply = <&vreg_qrs_1p2>;
ports {
port@0{};
port@1{};
};
};
csiphy_N: {
clocks = <&camcc CAM_CC_CSIPHY1_CLK>,
<&camcc CAM_CC_CSI1PHYTIMER_CLK>;
clock-names = "core",
"timer";
operating-points-v2 = <&csiphy_opp_table>;
interrupts = <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>;
power-domains = <&rpmhpd RPMHPD_MX>,
<&rpmhpd RPMHPD_MMCX>;
power-domain-names = "mx",
"mmcx";
vdda-0p9-supply = <&vreg_abc_0p8>;
vdda-1p2-supply = <&vreg_def_1p2>;
ports {
port@0{};
};
};
IMO split mode is a special mode of that hardware block, not two
individual PHYs.
---
bod
On 30/03/2026 12:34, Konrad Dybcio wrote:
> Let's take a step back - since any CSIPHY can feed into any CSID (at runtime),
> the resulting nodes would either look like:
>
> // hardcoded, m may != n
> csid_n: csid@1000000 {
> phys = <&csiphy_m>;
> };
>
Well that would be wrong they can connect to any CSID. We'd be churning
the user-space ABI and imposing an artificial constraint on what the hw
can do.
>
> // determined at runtime
> csid_n: csid@1000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
> };
This I think works well and actually maps to what the hardware can do.
This would be where to talk more about Neil's mux.
>
> or we could store them once, centrally, in the "CAMSS_TOP" node and
> pass handles around as necessary:
>
> // camss "catalog/manager" driver/library provides CSIDn with PHYm
> camss: camss@10000000 {
> phys = <&csiphy_0>,
> [...]
> <&csiphy_n-1>;
>
> csid_n: csid@1000 {
> // no PHY references
> };
> };
That could work too.
Either way I think this is to be hashed out when doing CSID.
---
bod
On 3/27/26 11:07 AM, Konrad Dybcio wrote: > On 3/26/26 2:04 AM, Bryan O'Donoghue wrote: >> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2 >> PHY devices. >> >> The hardware can support both CPHY, DPHY and a special split-mode DPHY. We >> capture those modes as: >> >> - PHY_QCOM_CSI2_MODE_DPHY >> - PHY_QCOM_CSI2_MODE_CPHY >> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY > > Does the _PHY_ DT node need to be aware about this upfront? > > If we have some sideband signal (e.g. the sensor driver specifically > requesting C-PHY mode), we can simply throw all this complexity into > phy_mode + phy_configure_opts, all at runtime Actually perhaps just phy_configure_opts, because phy_mode could be left as "MIPI_CSI" to reduce complexity (while still carrying info about D/C mode in the latter) Konrad
On Thu, 26 Mar 2026 01:04:43 +0000, Bryan O'Donoghue wrote: > Add a base schema initially compatible with x1e80100 to describe MIPI CSI2 > PHY devices. > > The hardware can support both CPHY, DPHY and a special split-mode DPHY. We > capture those modes as: > > - PHY_QCOM_CSI2_MODE_DPHY > - PHY_QCOM_CSI2_MODE_CPHY > - PHY_QCOM_CSI2_MODE_SPLIT_DPHY > > The CSIPHY devices have their own pinouts on the SoC as well as their own > individual voltage rails. > > The need to model voltage rails on a per-PHY basis leads us to define > CSIPHY devices as individual nodes. > > Two nice outcomes in terms of schema and DT arise from this change. > > 1. The ability to define on a per-PHY basis voltage rails. > 2. The ability to require those voltage. > > We have had a complete bodge upstream for this where a single set of > voltage rail for all CSIPHYs has been buried inside of CAMSS. > > Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in > CAMSS parlance, the CSIPHY devices should be individually modelled. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++++++++++++ > include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++ > 2 files changed, 145 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.example.dts:75.21-77.11: Warning (unit_address_vs_reg): /example-0/isp@acb7000: node has a unit name, but no reg or ranges property doc reference errors (make refcheckdocs): See https://patchwork.kernel.org/project/devicetree/patch/20260326-x1e-csi2-phy-v5-1-0c0fc7f5c01b@linaro.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 3/26/26 03:04, Bryan O'Donoghue wrote:
> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
> PHY devices.
>
> The hardware can support both CPHY, DPHY and a special split-mode DPHY. We
> capture those modes as:
>
> - PHY_QCOM_CSI2_MODE_DPHY
> - PHY_QCOM_CSI2_MODE_CPHY
> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
Distinction between PHY_QCOM_CSI2_MODE_DPHY and PHY_QCOM_CSI2_MODE_SPLIT_DPHY
is
1) insufficient in just this simplistic form, because the assignment of
particular lanes is also needed,
2) and under the assumption that the lane mapping is set somewhere else, then
there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
it's just DPHY, and the subtype is deductible from data-lanes property on
the consumer side.
So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
and PHY_TYPE_CPHY is needed here, those two are sufficient.
>
> The CSIPHY devices have their own pinouts on the SoC as well as their own
> individual voltage rails.
>
> The need to model voltage rails on a per-PHY basis leads us to define
> CSIPHY devices as individual nodes.
>
> Two nice outcomes in terms of schema and DT arise from this change.
>
> 1. The ability to define on a per-PHY basis voltage rails.
> 2. The ability to require those voltage.
>
> We have had a complete bodge upstream for this where a single set of
> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>
> Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in
> CAMSS parlance, the CSIPHY devices should be individually modelled.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++++++++++++
> include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++
> 2 files changed, 145 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> new file mode 100644
> index 0000000000000..63114151104b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> @@ -0,0 +1,130 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CSI2 PHY
> +
> +maintainers:
> + - Bryan O'Donoghue <bod@kernel.org>
> +
> +description:
> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
> + modes.
> +
> +properties:
> + compatible:
> + const: qcom,x1e80100-csi2-phy
> +
> + reg:
> + maxItems: 1
> +
> + "#phy-cells":
> + const: 1
> + description:
> + The single cell specifies the PHY operating mode.
> + See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
include/dt-bindings/phy/phy.h should be good enough as it's stated above.
> +
> + clocks:
> + maxItems: 2
> +
> + clock-names:
> + items:
> + - const: core
> + - const: timer
> +
> + interrupts:
> + maxItems: 1
> +
> + operating-points-v2:
> + maxItems: 1
> +
> + power-domains:
> + items:
> + - description: MXC or MXA voltage rail
> + - description: MMCX voltage rail
> +
> + power-domain-names:
> + items:
> + - const: mx
> + - const: mmcx
> +
> + vdda-0p9-supply:
> + description: Phandle to a 0.9V regulator supply to a PHY.
> +
> + vdda-1p2-supply:
> + description: Phandle to 1.2V regulator supply to a PHY.
> +
> +required:
> + - compatible
> + - reg
> + - "#phy-cells"
> + - clocks
> + - clock-names
> + - interrupts
> + - operating-points-v2
> + - power-domains
> + - power-domain-names
> + - vdda-0p9-supply
> + - vdda-1p2-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
> + #include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
> + #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> + csiphy4: csiphy@ace4000 {
> + compatible = "qcom,x1e80100-csi2-phy";
> + reg = <0x0ace4000 0x2000>;
> + #phy-cells = <1>;
> +
> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
> + clock-names = "core",
> + "timer";
> +
> + operating-points-v2 = <&csiphy_opp_table>;
> +
> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> + power-domains = <&rpmhpd RPMHPD_MX>,
> + <&rpmhpd RPMHPD_MMCX>;
> + power-domain-names = "mx",
> + "mmcx";
> +
> + vdda-0p9-supply = <&vreg_l2c_0p8>;
> + vdda-1p2-supply = <&vreg_l1c_1p2>;
> + };
> +
> + csiphy_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + required-opps = <&rpmhpd_opp_low_svs_d1>,
> + <&rpmhpd_opp_low_svs_d1>;
> + };
> +
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + required-opps = <&rpmhpd_opp_low_svs>,
> + <&rpmhpd_opp_low_svs>;
> + };
> +
> + opp-480000000 {
> + opp-hz = /bits/ 64 <480000000>;
> + required-opps = <&rpmhpd_opp_low_svs>,
> + <&rpmhpd_opp_low_svs>;
> + };
> + };
> +
> + isp@acb7000 {
> + phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
> + };
This example is incomplete in sense that it does not include CAMSS
CSIPHY IP hardware configuration in whole.
> diff --git a/include/dt-bindings/phy/phy-qcom-mipi-csi2.h b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
> new file mode 100644
> index 0000000000000..fa48fd75c58d8
> --- /dev/null
> +++ b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
> +/*
> + * Qualcomm MIPI CSI2 PHY constants
> + *
> + * Copyright (C) 2026 Linaro Limited
> + */
> +
> +#ifndef __DT_BINDINGS_PHY_MIPI_CSI2__
> +#define __DT_BINDINGS_PHY_MIPI_CSI2__
> +
> +#define PHY_QCOM_CSI2_MODE_DPHY 0
> +#define PHY_QCOM_CSI2_MODE_CPHY 1
> +#define PHY_QCOM_CSI2_MODE_SPLIT_DPHY 2
> +
> +#endif /* __DT_BINDINGS_PHY_MIPI_CSI2__ */
>
--
Best wishes,
Vladimir
On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
> On 3/26/26 03:04, Bryan O'Donoghue wrote:
>> Add a base schema initially compatible with x1e80100 to describe MIPI
>> CSI2
>> PHY devices.
>>
>> The hardware can support both CPHY, DPHY and a special split-mode
>> DPHY. We
>> capture those modes as:
>>
>> - PHY_QCOM_CSI2_MODE_DPHY
>> - PHY_QCOM_CSI2_MODE_CPHY
>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>
> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> is
> 1) insufficient in just this simplistic form, because the assignment of
> particular lanes is also needed,
> 2) and under the assumption that the lane mapping is set somewhere else,
> then
> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
> it's just DPHY, and the subtype is deductible from data-lanes property on
> the consumer side.
>
> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
> and PHY_TYPE_CPHY is needed here, those two are sufficient.
Because knowing the split-mode exists and that you have asked about how
such a thing would be supported, I thought about how to represent that
mode right from the start, even if we don't support it.
To support split phy we will need to pass the parameter.
So we define those parameters upfront.
>
>>
>> The CSIPHY devices have their own pinouts on the SoC as well as their own
>> individual voltage rails.
>>
>> The need to model voltage rails on a per-PHY basis leads us to define
>> CSIPHY devices as individual nodes.
>>
>> Two nice outcomes in terms of schema and DT arise from this change.
>>
>> 1. The ability to define on a per-PHY basis voltage rails.
>> 2. The ability to require those voltage.
>>
>> We have had a complete bodge upstream for this where a single set of
>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>
>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>> bus in
>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++
>> ++++++++++
>> include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++
>> 2 files changed, 145 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>> phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>> phy.yaml
>> new file mode 100644
>> index 0000000000000..63114151104b4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>> @@ -0,0 +1,130 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm CSI2 PHY
>> +
>> +maintainers:
>> + - Bryan O'Donoghue <bod@kernel.org>
>> +
>> +description:
>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2
>> sensors
>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>> D-PHY
>> + modes.
>> +
>> +properties:
>> + compatible:
>> + const: qcom,x1e80100-csi2-phy
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + "#phy-cells":
>> + const: 1
>> + description:
>> + The single cell specifies the PHY operating mode.
>> + See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>
> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
While include/dt-bindings/phy/phy.h provides generic definitions for
D-PHY and C-PHY, it does not contain a definition for Qualcomm's
proprietary Split D-PHY mode. Because this hardware supports a
vendor-specific operating mode, introducing a vendor-specific header to
define that state is necessary.
This is exactly what we do with the QMP to support a similar use-case -
the PHYs do vendor specific things, so we use vendor specific defines.
If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
say adding split-mode to an upstream SoC as the DT ABI will not then
facilitate the mode.
>
>> +
>> + clocks:
>> + maxItems: 2
>> +
>> + clock-names:
>> + items:
>> + - const: core
>> + - const: timer
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + operating-points-v2:
>> + maxItems: 1
>> +
>> + power-domains:
>> + items:
>> + - description: MXC or MXA voltage rail
>> + - description: MMCX voltage rail
>> +
>> + power-domain-names:
>> + items:
>> + - const: mx
>> + - const: mmcx
>> +
>> + vdda-0p9-supply:
>> + description: Phandle to a 0.9V regulator supply to a PHY.
>> +
>> + vdda-1p2-supply:
>> + description: Phandle to 1.2V regulator supply to a PHY.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - "#phy-cells"
>> + - clocks
>> + - clock-names
>> + - interrupts
>> + - operating-points-v2
>> + - power-domains
>> + - power-domain-names
>> + - vdda-0p9-supply
>> + - vdda-1p2-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>> + #include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>> +
>> + csiphy4: csiphy@ace4000 {
>> + compatible = "qcom,x1e80100-csi2-phy";
>> + reg = <0x0ace4000 0x2000>;
>> + #phy-cells = <1>;
>> +
>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>> + clock-names = "core",
>> + "timer";
>> +
>> + operating-points-v2 = <&csiphy_opp_table>;
>> +
>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>> +
>> + power-domains = <&rpmhpd RPMHPD_MX>,
>> + <&rpmhpd RPMHPD_MMCX>;
>> + power-domain-names = "mx",
>> + "mmcx";
>> +
>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>> + };
>> +
>> + csiphy_opp_table: opp-table {
>> + compatible = "operating-points-v2";
>> +
>> + opp-300000000 {
>> + opp-hz = /bits/ 64 <300000000>;
>> + required-opps = <&rpmhpd_opp_low_svs_d1>,
>> + <&rpmhpd_opp_low_svs_d1>;
>> + };
>> +
>> + opp-400000000 {
>> + opp-hz = /bits/ 64 <400000000>;
>> + required-opps = <&rpmhpd_opp_low_svs>,
>> + <&rpmhpd_opp_low_svs>;
>> + };
>> +
>> + opp-480000000 {
>> + opp-hz = /bits/ 64 <480000000>;
>> + required-opps = <&rpmhpd_opp_low_svs>,
>> + <&rpmhpd_opp_low_svs>;
>> + };
>> + };
>> +
>> + isp@acb7000 {
>> + phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>> + };
>
> This example is incomplete in sense that it does not include CAMSS
> CSIPHY IP hardware configuration in whole.
No that's not the way examples work. You don't replicate entire nodes
from other schemas you just give a terse reference.
>
>> diff --git a/include/dt-bindings/phy/phy-qcom-mipi-csi2.h b/include/
>> dt-bindings/phy/phy-qcom-mipi-csi2.h
>> new file mode 100644
>> index 0000000000000..fa48fd75c58d8
>> --- /dev/null
>> +++ b/include/dt-bindings/phy/phy-qcom-mipi-csi2.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
>> +/*
>> + * Qualcomm MIPI CSI2 PHY constants
>> + *
>> + * Copyright (C) 2026 Linaro Limited
>> + */
>> +
>> +#ifndef __DT_BINDINGS_PHY_MIPI_CSI2__
>> +#define __DT_BINDINGS_PHY_MIPI_CSI2__
>> +
>> +#define PHY_QCOM_CSI2_MODE_DPHY 0
>> +#define PHY_QCOM_CSI2_MODE_CPHY 1
>> +#define PHY_QCOM_CSI2_MODE_SPLIT_DPHY 2
>> +
>> +#endif /* __DT_BINDINGS_PHY_MIPI_CSI2__ */
>>
>
On 3/26/26 04:03, Bryan O'Donoghue wrote:
> On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
>> On 3/26/26 03:04, Bryan O'Donoghue wrote:
>>> Add a base schema initially compatible with x1e80100 to describe MIPI
>>> CSI2
>>> PHY devices.
>>>
>>> The hardware can support both CPHY, DPHY and a special split-mode
>>> DPHY. We
>>> capture those modes as:
>>>
>>> - PHY_QCOM_CSI2_MODE_DPHY
>>> - PHY_QCOM_CSI2_MODE_CPHY
>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>
>> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>> is
>> 1) insufficient in just this simplistic form, because the assignment of
>> particular lanes is also needed,
>> 2) and under the assumption that the lane mapping is set somewhere else,
>> then
>> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
>> it's just DPHY, and the subtype is deductible from data-lanes property on
>> the consumer side.
>>
>> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
>> and PHY_TYPE_CPHY is needed here, those two are sufficient.
>
> Because knowing the split-mode exists and that you have asked about how
> such a thing would be supported, I thought about how to represent that
> mode right from the start, even if we don't support it.
It is good to think about this hardware confguration in advance, however
the process of describing such hardware setup is incomplete.
>
> To support split phy we will need to pass the parameter.
What you call "split phy" is a DPHY, and "split phy" can not be supported
by adding this parameter, because it does not provide information about
lanes, and after removing this information it is just DPHY.
> So we define those parameters upfront.
This new header file has to be removed, it does not bring anything valuable.
>>
>>>
>>> The CSIPHY devices have their own pinouts on the SoC as well as their own
>>> individual voltage rails.
>>>
>>> The need to model voltage rails on a per-PHY basis leads us to define
>>> CSIPHY devices as individual nodes.
>>>
>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>
>>> 1. The ability to define on a per-PHY basis voltage rails.
>>> 2. The ability to require those voltage.
>>>
>>> We have had a complete bodge upstream for this where a single set of
>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>
>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>> bus in
>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++
>>> ++++++++++
>>> include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++
>>> 2 files changed, 145 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>> phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>> phy.yaml
>>> new file mode 100644
>>> index 0000000000000..63114151104b4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>> @@ -0,0 +1,130 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm CSI2 PHY
>>> +
>>> +maintainers:
>>> + - Bryan O'Donoghue <bod@kernel.org>
>>> +
>>> +description:
>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2
>>> sensors
>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>> D-PHY
>>> + modes.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: qcom,x1e80100-csi2-phy
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + "#phy-cells":
>>> + const: 1
>>> + description:
>>> + The single cell specifies the PHY operating mode.
>>> + See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>>
>> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
>
> While include/dt-bindings/phy/phy.h provides generic definitions for
> D-PHY and C-PHY, it does not contain a definition for Qualcomm's
> proprietary Split D-PHY mode. Because this hardware supports a
What Qualcomm's proprietary Split D-PHY mode is manifested by lane mapping,
there is no need to introduce another PHY mode, it is DPHY.
> vendor-specific operating mode, introducing a vendor-specific header to
> define that state is necessary.
>
> This is exactly what we do with the QMP to support a similar use-case -
> the PHYs do vendor specific things, so we use vendor specific defines.
>
> If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
> say adding split-mode to an upstream SoC as the DT ABI will not then
> facilitate the mode.
>
>>
>>> +
>>> + clocks:
>>> + maxItems: 2
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: core
>>> + - const: timer
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + operating-points-v2:
>>> + maxItems: 1
>>> +
>>> + power-domains:
>>> + items:
>>> + - description: MXC or MXA voltage rail
>>> + - description: MMCX voltage rail
>>> +
>>> + power-domain-names:
>>> + items:
>>> + - const: mx
>>> + - const: mmcx
>>> +
>>> + vdda-0p9-supply:
>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>> +
>>> + vdda-1p2-supply:
>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - "#phy-cells"
>>> + - clocks
>>> + - clock-names
>>> + - interrupts
>>> + - operating-points-v2
>>> + - power-domains
>>> + - power-domain-names
>>> + - vdda-0p9-supply
>>> + - vdda-1p2-supply
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>> + #include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>> +
>>> + csiphy4: csiphy@ace4000 {
>>> + compatible = "qcom,x1e80100-csi2-phy";
>>> + reg = <0x0ace4000 0x2000>;
>>> + #phy-cells = <1>;
>>> +
>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>> + clock-names = "core",
>>> + "timer";
>>> +
>>> + operating-points-v2 = <&csiphy_opp_table>;
>>> +
>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>> +
>>> + power-domains = <&rpmhpd RPMHPD_MX>,
>>> + <&rpmhpd RPMHPD_MMCX>;
>>> + power-domain-names = "mx",
>>> + "mmcx";
>>> +
>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>> + };
>>> +
>>> + csiphy_opp_table: opp-table {
>>> + compatible = "operating-points-v2";
>>> +
>>> + opp-300000000 {
>>> + opp-hz = /bits/ 64 <300000000>;
>>> + required-opps = <&rpmhpd_opp_low_svs_d1>,
>>> + <&rpmhpd_opp_low_svs_d1>;
>>> + };
>>> +
>>> + opp-400000000 {
>>> + opp-hz = /bits/ 64 <400000000>;
>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>> + <&rpmhpd_opp_low_svs>;
>>> + };
>>> +
>>> + opp-480000000 {
>>> + opp-hz = /bits/ 64 <480000000>;
>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>> + <&rpmhpd_opp_low_svs>;
>>> + };
>>> + };
>>> +
>>> + isp@acb7000 {
>>> + phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>>> + };
>>
>> This example is incomplete in sense that it does not include CAMSS
>> CSIPHY IP hardware configuration in whole.
>
>
> No that's not the way examples work. You don't replicate entire nodes
> from other schemas you just give a terse reference.
>
If so, then this example makes no sense and it'd be better to remove it.
--
Best wishes,
Vladimir
On 26/03/2026 10:28, Vladimir Zapolskiy wrote:
> On 3/26/26 04:03, Bryan O'Donoghue wrote:
>> On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
>>> On 3/26/26 03:04, Bryan O'Donoghue wrote:
>>>> Add a base schema initially compatible with x1e80100 to describe MIPI
>>>> CSI2
>>>> PHY devices.
>>>>
>>>> The hardware can support both CPHY, DPHY and a special split-mode
>>>> DPHY. We
>>>> capture those modes as:
>>>>
>>>> - PHY_QCOM_CSI2_MODE_DPHY
>>>> - PHY_QCOM_CSI2_MODE_CPHY
>>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>>
>>> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
>>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>> is
>>> 1) insufficient in just this simplistic form, because the assignment of
>>> particular lanes is also needed,
>>> 2) and under the assumption that the lane mapping is set somewhere else,
>>> then
>>> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
>>> it's just DPHY, and the subtype is deductible from data-lanes property on
>>> the consumer side.
>>>
>>> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
>>> and PHY_TYPE_CPHY is needed here, those two are sufficient.
>>
>> Because knowing the split-mode exists and that you have asked about how
>> such a thing would be supported, I thought about how to represent that
>> mode right from the start, even if we don't support it.
>
> It is good to think about this hardware confguration in advance, however
> the process of describing such hardware setup is incomplete.
>
>>
>> To support split phy we will need to pass the parameter.
>
> What you call "split phy" is a DPHY, and "split phy" can not be supported
> by adding this parameter, because it does not provide information about
> lanes, and after removing this information it is just DPHY.
That's just not true. If you read the camx source code you can see
split/combo mode 2+1 1+1 data/clock mode requires special programming of
the PHY to support.
https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b285
There is disjunction all over this file depending on the mode.
https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b767
And besides, think about it - you need different init sequences if one
of the lanes is clock instead of data...
If we use phy.h::PHY_TYPE_DPHY then that means to support split-mode in
the future we need to get that mode represented in phy.h - but really
this fixed split mode isn't a generic CSI2 PHY mode, its a Qualcommism.
Nothing wrong with that - but then the mode should reflect the fact it
is vendor specific and we absolutely 100% have to do different things in
the PHY driver whether we are in regular DPHY mode or in split DPHY mode.
If we use PHY_TYPE_DPHY as I did in the previous patch then we can't
convert to a vendor type later on as its an ABI break.
So we have both a sound technical reason hardware will require it to
differentiate between DPHY and split-mode DPHY - and we also don't want
to be bound to phy.h and then try to upstream a new DPHY_SPLIT_MODE here
which a reviewer might reasonably say "why is this special mode from a
specific vendor driving new defines in a shared file"
>
>> So we define those parameters upfront.
>
> This new header file has to be removed, it does not bring anything valuable.
>
>>>
>>>>
>>>> The CSIPHY devices have their own pinouts on the SoC as well as their own
>>>> individual voltage rails.
>>>>
>>>> The need to model voltage rails on a per-PHY basis leads us to define
>>>> CSIPHY devices as individual nodes.
>>>>
>>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>>
>>>> 1. The ability to define on a per-PHY basis voltage rails.
>>>> 2. The ability to require those voltage.
>>>>
>>>> We have had a complete bodge upstream for this where a single set of
>>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>>
>>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>>> bus in
>>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>>
>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>> ---
>>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++
>>>> ++++++++++
>>>> include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++
>>>> 2 files changed, 145 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>>> phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>>> phy.yaml
>>>> new file mode 100644
>>>> index 0000000000000..63114151104b4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>> @@ -0,0 +1,130 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Qualcomm CSI2 PHY
>>>> +
>>>> +maintainers:
>>>> + - Bryan O'Donoghue <bod@kernel.org>
>>>> +
>>>> +description:
>>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2
>>>> sensors
>>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>>> D-PHY
>>>> + modes.
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + const: qcom,x1e80100-csi2-phy
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + "#phy-cells":
>>>> + const: 1
>>>> + description:
>>>> + The single cell specifies the PHY operating mode.
>>>> + See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>>>
>>> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
>>
>> While include/dt-bindings/phy/phy.h provides generic definitions for
>> D-PHY and C-PHY, it does not contain a definition for Qualcomm's
>> proprietary Split D-PHY mode. Because this hardware supports a
>
> What Qualcomm's proprietary Split D-PHY mode is manifested by lane mapping,
> there is no need to introduce another PHY mode, it is DPHY.
>
>> vendor-specific operating mode, introducing a vendor-specific header to
>> define that state is necessary.
>>
>> This is exactly what we do with the QMP to support a similar use-case -
>> the PHYs do vendor specific things, so we use vendor specific defines.
>>
>> If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
>> say adding split-mode to an upstream SoC as the DT ABI will not then
>> facilitate the mode.
>>
>>>
>>>> +
>>>> + clocks:
>>>> + maxItems: 2
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: core
>>>> + - const: timer
>>>> +
>>>> + interrupts:
>>>> + maxItems: 1
>>>> +
>>>> + operating-points-v2:
>>>> + maxItems: 1
>>>> +
>>>> + power-domains:
>>>> + items:
>>>> + - description: MXC or MXA voltage rail
>>>> + - description: MMCX voltage rail
>>>> +
>>>> + power-domain-names:
>>>> + items:
>>>> + - const: mx
>>>> + - const: mmcx
>>>> +
>>>> + vdda-0p9-supply:
>>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>>> +
>>>> + vdda-1p2-supply:
>>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - reg
>>>> + - "#phy-cells"
>>>> + - clocks
>>>> + - clock-names
>>>> + - interrupts
>>>> + - operating-points-v2
>>>> + - power-domains
>>>> + - power-domain-names
>>>> + - vdda-0p9-supply
>>>> + - vdda-1p2-supply
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + - |
>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>>> + #include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>> +
>>>> + csiphy4: csiphy@ace4000 {
>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>> + reg = <0x0ace4000 0x2000>;
>>>> + #phy-cells = <1>;
>>>> +
>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>> + clock-names = "core",
>>>> + "timer";
>>>> +
>>>> + operating-points-v2 = <&csiphy_opp_table>;
>>>> +
>>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>> +
>>>> + power-domains = <&rpmhpd RPMHPD_MX>,
>>>> + <&rpmhpd RPMHPD_MMCX>;
>>>> + power-domain-names = "mx",
>>>> + "mmcx";
>>>> +
>>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>> + };
>>>> +
>>>> + csiphy_opp_table: opp-table {
>>>> + compatible = "operating-points-v2";
>>>> +
>>>> + opp-300000000 {
>>>> + opp-hz = /bits/ 64 <300000000>;
>>>> + required-opps = <&rpmhpd_opp_low_svs_d1>,
>>>> + <&rpmhpd_opp_low_svs_d1>;
>>>> + };
>>>> +
>>>> + opp-400000000 {
>>>> + opp-hz = /bits/ 64 <400000000>;
>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>> + <&rpmhpd_opp_low_svs>;
>>>> + };
>>>> +
>>>> + opp-480000000 {
>>>> + opp-hz = /bits/ 64 <480000000>;
>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>> + <&rpmhpd_opp_low_svs>;
>>>> + };
>>>> + };
>>>> +
>>>> + isp@acb7000 {
>>>> + phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>>>> + };
>>>
>>> This example is incomplete in sense that it does not include CAMSS
>>> CSIPHY IP hardware configuration in whole.
>>
>>
>> No that's not the way examples work. You don't replicate entire nodes
>> from other schemas you just give a terse reference.
>>
>
> If so, then this example makes no sense and it'd be better to remove it.
You know you're right its not strictly necessary - its just there to be
helpful.
"Be less helpful" is not usually review feedback I give or take but,
I'll drop this anyway.
---
bod
On Thu, Mar 26, 2026 at 02:42:10PM +0000, Bryan O'Donoghue wrote:
> On 26/03/2026 10:28, Vladimir Zapolskiy wrote:
> > On 3/26/26 04:03, Bryan O'Donoghue wrote:
> > > On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
> > > > On 3/26/26 03:04, Bryan O'Donoghue wrote:
> > > > > Add a base schema initially compatible with x1e80100 to describe MIPI
> > > > > CSI2
> > > > > PHY devices.
> > > > >
> > > > > The hardware can support both CPHY, DPHY and a special split-mode
> > > > > DPHY. We
> > > > > capture those modes as:
> > > > >
> > > > > - PHY_QCOM_CSI2_MODE_DPHY
> > > > > - PHY_QCOM_CSI2_MODE_CPHY
> > > > > - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> > > >
> > > > Distinction between PHY_QCOM_CSI2_MODE_DPHY and
> > > > PHY_QCOM_CSI2_MODE_SPLIT_DPHY
> > > > is
> > > > 1) insufficient in just this simplistic form, because the assignment of
> > > > particular lanes is also needed,
> > > > 2) and under the assumption that the lane mapping is set somewhere else,
> > > > then
> > > > there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
> > > > it's just DPHY, and the subtype is deductible from data-lanes property on
> > > > the consumer side.
> > > >
> > > > So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
> > > > and PHY_TYPE_CPHY is needed here, those two are sufficient.
> > >
> > > Because knowing the split-mode exists and that you have asked about how
> > > such a thing would be supported, I thought about how to represent that
> > > mode right from the start, even if we don't support it.
> >
> > It is good to think about this hardware confguration in advance, however
> > the process of describing such hardware setup is incomplete.
> >
> > >
> > > To support split phy we will need to pass the parameter.
> >
> > What you call "split phy" is a DPHY, and "split phy" can not be supported
> > by adding this parameter, because it does not provide information about
> > lanes, and after removing this information it is just DPHY.
>
> That's just not true. If you read the camx source code you can see
> split/combo mode 2+1 1+1 data/clock mode requires special programming of the
> PHY to support.
This needs to be identified from the data-lanes / clock-lanes topology.
And once you do that, there would be (probably) no difference in the
hardware definition.
In other words, I'd also ask to drop this mode from the DT. This
infromation can and should be deduced from other, already-defined
properties.
--
With best wishes
Dmitry
On 27/03/2026 20:51, Dmitry Baryshkov wrote: >> That's just not true. If you read the camx source code you can see >> split/combo mode 2+1 1+1 data/clock mode requires special programming of the >> PHY to support. > This needs to be identified from the data-lanes / clock-lanes topology. > And once you do that, there would be (probably) no difference in the > hardware definition. > > > In other words, I'd also ask to drop this mode from the DT. This > infromation can and should be deduced from other, already-defined > properties. It still needs to be communicated to the PHY from the controller, however that is not a problem I am trying to solve now. If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. I'll aim for DPHY only and we can come back to this topic when someone actually tries to enable it. --- bod
On 3/28/26 00:29, Bryan O'Donoghue wrote: > On 27/03/2026 20:51, Dmitry Baryshkov wrote: >>> That's just not true. If you read the camx source code you can see >>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the >>> PHY to support. >> This needs to be identified from the data-lanes / clock-lanes topology. >> And once you do that, there would be (probably) no difference in the >> hardware definition. >> >> >> In other words, I'd also ask to drop this mode from the DT. This >> infromation can and should be deduced from other, already-defined >> properties. > > It still needs to be communicated to the PHY from the controller, > however that is not a problem I am trying to solve now. > > If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. > > I'll aim for DPHY only and we can come back to this topic when someone > actually tries to enable it. > DPHY may be the only supported phy type in the driver, it does not matter at this point, however it's totally essential to cover the called by you 'split mode' right from the beginning in the renewed device tree binding descriptions of CAMSS IPs to progress further. -- Best wishes, Vladimir
On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote: > On 3/28/26 00:29, Bryan O'Donoghue wrote: > > On 27/03/2026 20:51, Dmitry Baryshkov wrote: > > > > That's just not true. If you read the camx source code you can see > > > > split/combo mode 2+1 1+1 data/clock mode requires special programming of the > > > > PHY to support. > > > This needs to be identified from the data-lanes / clock-lanes topology. > > > And once you do that, there would be (probably) no difference in the > > > hardware definition. > > > > > > > > > In other words, I'd also ask to drop this mode from the DT. This > > > infromation can and should be deduced from other, already-defined > > > properties. > > > > It still needs to be communicated to the PHY from the controller, > > however that is not a problem I am trying to solve now. > > > > If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. > > > > I'll aim for DPHY only and we can come back to this topic when someone > > actually tries to enable it. > > > > DPHY may be the only supported phy type in the driver, it does not matter > at this point, however it's totally essential to cover the called by you > 'split mode' right from the beginning in the renewed device tree binding > descriptions of CAMSS IPs to progress further. Okay. How would we describe that there are two sensors connected to the single PHY anyway? How would it be described with the current bindings? -- With best wishes Dmitry
On 3/28/26 01:23, Dmitry Baryshkov wrote: > On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote: >> On 3/28/26 00:29, Bryan O'Donoghue wrote: >>> On 27/03/2026 20:51, Dmitry Baryshkov wrote: >>>>> That's just not true. If you read the camx source code you can see >>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the >>>>> PHY to support. >>>> This needs to be identified from the data-lanes / clock-lanes topology. >>>> And once you do that, there would be (probably) no difference in the >>>> hardware definition. >>>> >>>> >>>> In other words, I'd also ask to drop this mode from the DT. This >>>> infromation can and should be deduced from other, already-defined >>>> properties. >>> >>> It still needs to be communicated to the PHY from the controller, >>> however that is not a problem I am trying to solve now. >>> >>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. >>> >>> I'll aim for DPHY only and we can come back to this topic when someone >>> actually tries to enable it. >>> >> >> DPHY may be the only supported phy type in the driver, it does not matter >> at this point, however it's totally essential to cover the called by you >> 'split mode' right from the beginning in the renewed device tree binding >> descriptions of CAMSS IPs to progress further. > > Okay. How would we describe that there are two sensors connected to the > single PHY anyway? How would it be described with the current bindings? > An RFC example was sent about one year ago, it allows to specify one or two endpoints under a single phy port: https://lore.kernel.org/linux-arm-msm/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org/ -- Best wishes, Vladimir
On 27/03/2026 23:23, Dmitry Baryshkov wrote: > On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote: >> On 3/28/26 00:29, Bryan O'Donoghue wrote: >>> On 27/03/2026 20:51, Dmitry Baryshkov wrote: >>>>> That's just not true. If you read the camx source code you can see >>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the >>>>> PHY to support. >>>> This needs to be identified from the data-lanes / clock-lanes topology. >>>> And once you do that, there would be (probably) no difference in the >>>> hardware definition. >>>> >>>> >>>> In other words, I'd also ask to drop this mode from the DT. This >>>> infromation can and should be deduced from other, already-defined >>>> properties. >>> >>> It still needs to be communicated to the PHY from the controller, >>> however that is not a problem I am trying to solve now. >>> >>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. >>> >>> I'll aim for DPHY only and we can come back to this topic when someone >>> actually tries to enable it. >>> >> >> DPHY may be the only supported phy type in the driver, it does not matter >> at this point, however it's totally essential to cover the called by you >> 'split mode' right from the beginning in the renewed device tree binding >> descriptions of CAMSS IPs to progress further. > > Okay. How would we describe that there are two sensors connected to the > single PHY anyway? How would it be described with the current bindings? > > -- > With best wishes > Dmitry Assuming you add endpoints to the PHY i.e. that is what Neil appears to be asking for and I personally am _fine_ with that, then it should just be port@0 port@1 if port@1 exists, you know you are in split-phy mode. Its actually straight forward enough, really. To be clear though I can write that yaml - the _most_ support I'm willing to put into the PHY code is to detect the port@1 and say "nope not supported yet", since like CPHY its not. --- bod
On Fri, Mar 27, 2026 at 11:40:51PM +0000, Bryan O'Donoghue wrote: > On 27/03/2026 23:23, Dmitry Baryshkov wrote: > > On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote: > > > On 3/28/26 00:29, Bryan O'Donoghue wrote: > > > > On 27/03/2026 20:51, Dmitry Baryshkov wrote: > > > > > > That's just not true. If you read the camx source code you can see > > > > > > split/combo mode 2+1 1+1 data/clock mode requires special programming of the > > > > > > PHY to support. > > > > > This needs to be identified from the data-lanes / clock-lanes topology. > > > > > And once you do that, there would be (probably) no difference in the > > > > > hardware definition. > > > > > > > > > > > > > > > In other words, I'd also ask to drop this mode from the DT. This > > > > > infromation can and should be deduced from other, already-defined > > > > > properties. > > > > > > > > It still needs to be communicated to the PHY from the controller, > > > > however that is not a problem I am trying to solve now. > > > > > > > > If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. > > > > > > > > I'll aim for DPHY only and we can come back to this topic when someone > > > > actually tries to enable it. > > > > > > > > > > DPHY may be the only supported phy type in the driver, it does not matter > > > at this point, however it's totally essential to cover the called by you > > > 'split mode' right from the beginning in the renewed device tree binding > > > descriptions of CAMSS IPs to progress further. > > > > Okay. How would we describe that there are two sensors connected to the > > single PHY anyway? How would it be described with the current bindings? > > > > -- > > With best wishes > > Dmitry > > Assuming you add endpoints to the PHY i.e. that is what Neil appears to be > asking for and I personally am _fine_ with that, then it should just be > > port@0 > port@1 > > if port@1 exists, you know you are in split-phy mode. > > Its actually straight forward enough, really. To be clear though I can write > that yaml - the _most_ support I'm willing to put into the PHY code is to > detect the port@1 and say "nope not supported yet", since like CPHY its not. SGTM. But let's define the schema for those usecases. > > --- > bod -- With best wishes Dmitry
On 3/29/26 12:54 PM, Dmitry Baryshkov wrote: > On Fri, Mar 27, 2026 at 11:40:51PM +0000, Bryan O'Donoghue wrote: >> On 27/03/2026 23:23, Dmitry Baryshkov wrote: >>> On Sat, Mar 28, 2026 at 01:12:22AM +0200, Vladimir Zapolskiy wrote: >>>> On 3/28/26 00:29, Bryan O'Donoghue wrote: >>>>> On 27/03/2026 20:51, Dmitry Baryshkov wrote: >>>>>>> That's just not true. If you read the camx source code you can see >>>>>>> split/combo mode 2+1 1+1 data/clock mode requires special programming of the >>>>>>> PHY to support. >>>>>> This needs to be identified from the data-lanes / clock-lanes topology. >>>>>> And once you do that, there would be (probably) no difference in the >>>>>> hardware definition. >>>>>> >>>>>> >>>>>> In other words, I'd also ask to drop this mode from the DT. This >>>>>> infromation can and should be deduced from other, already-defined >>>>>> properties. >>>>> >>>>> It still needs to be communicated to the PHY from the controller, >>>>> however that is not a problem I am trying to solve now. >>>>> >>>>> If I can't get consensus for PHY_QCOM_CSI2_MODE_SPLIT_DPHY then so be it. >>>>> >>>>> I'll aim for DPHY only and we can come back to this topic when someone >>>>> actually tries to enable it. >>>>> >>>> >>>> DPHY may be the only supported phy type in the driver, it does not matter >>>> at this point, however it's totally essential to cover the called by you >>>> 'split mode' right from the beginning in the renewed device tree binding >>>> descriptions of CAMSS IPs to progress further. >>> >>> Okay. How would we describe that there are two sensors connected to the >>> single PHY anyway? How would it be described with the current bindings? >>> >>> -- >>> With best wishes >>> Dmitry >> >> Assuming you add endpoints to the PHY i.e. that is what Neil appears to be >> asking for and I personally am _fine_ with that, then it should just be >> >> port@0 >> port@1 >> >> if port@1 exists, you know you are in split-phy mode. >> >> Its actually straight forward enough, really. To be clear though I can write >> that yaml - the _most_ support I'm willing to put into the PHY code is to >> detect the port@1 and say "nope not supported yet", since like CPHY its not. > > SGTM. But let's define the schema for those usecases. Let's perhaps also add a short example for both a single- and dual-sensor cases in the YAML, even if there's no plans to support the latter configuration now Konrad
On 3/26/26 16:42, Bryan O'Donoghue wrote:
> On 26/03/2026 10:28, Vladimir Zapolskiy wrote:
>> On 3/26/26 04:03, Bryan O'Donoghue wrote:
>>> On 26/03/2026 01:46, Vladimir Zapolskiy wrote:
>>>> On 3/26/26 03:04, Bryan O'Donoghue wrote:
>>>>> Add a base schema initially compatible with x1e80100 to describe MIPI
>>>>> CSI2
>>>>> PHY devices.
>>>>>
>>>>> The hardware can support both CPHY, DPHY and a special split-mode
>>>>> DPHY. We
>>>>> capture those modes as:
>>>>>
>>>>> - PHY_QCOM_CSI2_MODE_DPHY
>>>>> - PHY_QCOM_CSI2_MODE_CPHY
>>>>> - PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>>>
>>>> Distinction between PHY_QCOM_CSI2_MODE_DPHY and
>>>> PHY_QCOM_CSI2_MODE_SPLIT_DPHY
>>>> is
>>>> 1) insufficient in just this simplistic form, because the assignment of
>>>> particular lanes is also needed,
>>>> 2) and under the assumption that the lane mapping is set somewhere else,
>>>> then
>>>> there should be no difference between PHY_QCOM_CSI2_MODE_{DPHY,SPLIT_DPHY},
>>>> it's just DPHY, and the subtype is deductible from data-lanes property on
>>>> the consumer side.
>>>>
>>>> So far the rationale is unclear, why anything above regular PHY_TYPE_DPHY
>>>> and PHY_TYPE_CPHY is needed here, those two are sufficient.
>>>
>>> Because knowing the split-mode exists and that you have asked about how
>>> such a thing would be supported, I thought about how to represent that
>>> mode right from the start, even if we don't support it.
>>
>> It is good to think about this hardware confguration in advance, however
>> the process of describing such hardware setup is incomplete.
>>
>>>
>>> To support split phy we will need to pass the parameter.
>>
>> What you call "split phy" is a DPHY, and "split phy" can not be supported
>> by adding this parameter, because it does not provide information about
>> lanes, and after removing this information it is just DPHY.
>
> That's just not true. If you read the camx source code you can see
> split/combo mode 2+1 1+1 data/clock mode requires special programming of
> the PHY to support.
Please do not reduce the upraised problem of proper hardware description
to some particular realisation in camx, this is irrelevant.
Here the description of hardware is done, and my point is that the new
PHY_QCOM_CSI2_MODE_SPLIT_DPHY phy type is simply not needed, since it's
possible to give a proper description of hardware without this invention.
> https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b285
>
> There is disjunction all over this file depending on the mode.
>
> https://review.lineageos.org/c/LineageOS/android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/cam_csiphy/cam_csiphy_core.c#b767
>
> And besides, think about it - you need different init sequences if one
> of the lanes is clock instead of data...
>
> If we use phy.h::PHY_TYPE_DPHY then that means to support split-mode in
> the future we need to get that mode represented in phy.h - but really
> this fixed split mode isn't a generic CSI2 PHY mode, its a Qualcommism.
>
> Nothing wrong with that - but then the mode should reflect the fact it
> is vendor specific and we absolutely 100% have to do different things in
> the PHY driver whether we are in regular DPHY mode or in split DPHY mode.
>
> If we use PHY_TYPE_DPHY as I did in the previous patch then we can't
> convert to a vendor type later on as its an ABI break.
>
> So we have both a sound technical reason hardware will require it to
> differentiate between DPHY and split-mode DPHY - and we also don't want
> to be bound to phy.h and then try to upstream a new DPHY_SPLIT_MODE here
> which a reviewer might reasonably say "why is this special mode from a
> specific vendor driving new defines in a shared file"
>
>>
>>> So we define those parameters upfront.
>>
>> This new header file has to be removed, it does not bring anything valuable.
>>
>>>>
>>>>>
>>>>> The CSIPHY devices have their own pinouts on the SoC as well as their own
>>>>> individual voltage rails.
>>>>>
>>>>> The need to model voltage rails on a per-PHY basis leads us to define
>>>>> CSIPHY devices as individual nodes.
>>>>>
>>>>> Two nice outcomes in terms of schema and DT arise from this change.
>>>>>
>>>>> 1. The ability to define on a per-PHY basis voltage rails.
>>>>> 2. The ability to require those voltage.
>>>>>
>>>>> We have had a complete bodge upstream for this where a single set of
>>>>> voltage rail for all CSIPHYs has been buried inside of CAMSS.
>>>>>
>>>>> Much like the I2C bus which is dedicated to Camera sensors - the CCI
>>>>> bus in
>>>>> CAMSS parlance, the CSIPHY devices should be individually modelled.
>>>>>
>>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>> ---
>>>>> .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 130 +++++++++++
>>>>> ++++++++++
>>>>> include/dt-bindings/phy/phy-qcom-mipi-csi2.h | 15 +++
>>>>> 2 files changed, 145 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>>>> phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-
>>>>> phy.yaml
>>>>> new file mode 100644
>>>>> index 0000000000000..63114151104b4
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>>>>> @@ -0,0 +1,130 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm CSI2 PHY
>>>>> +
>>>>> +maintainers:
>>>>> + - Bryan O'Donoghue <bod@kernel.org>
>>>>> +
>>>>> +description:
>>>>> + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2
>>>>> sensors
>>>>> + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and
>>>>> D-PHY
>>>>> + modes.
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + const: qcom,x1e80100-csi2-phy
>>>>> +
>>>>> + reg:
>>>>> + maxItems: 1
>>>>> +
>>>>> + "#phy-cells":
>>>>> + const: 1
>>>>> + description:
>>>>> + The single cell specifies the PHY operating mode.
>>>>> + See include/dt-bindings/phy/phy-qcom-mipi-csi2.h for valid values.
>>>>
>>>> include/dt-bindings/phy/phy.h should be good enough as it's stated above.
>>>
>>> While include/dt-bindings/phy/phy.h provides generic definitions for
>>> D-PHY and C-PHY, it does not contain a definition for Qualcomm's
>>> proprietary Split D-PHY mode. Because this hardware supports a
>>
>> What Qualcomm's proprietary Split D-PHY mode is manifested by lane mapping,
>> there is no need to introduce another PHY mode, it is DPHY.
>>
>>> vendor-specific operating mode, introducing a vendor-specific header to
>>> define that state is necessary.
>>>
>>> This is exactly what we do with the QMP to support a similar use-case -
>>> the PHYs do vendor specific things, so we use vendor specific defines.
>>>
>>> If we lock to phy.h CPHY/DPHY only then we exclude the possibility of
>>> say adding split-mode to an upstream SoC as the DT ABI will not then
>>> facilitate the mode.
>>>
>>>>
>>>>> +
>>>>> + clocks:
>>>>> + maxItems: 2
>>>>> +
>>>>> + clock-names:
>>>>> + items:
>>>>> + - const: core
>>>>> + - const: timer
>>>>> +
>>>>> + interrupts:
>>>>> + maxItems: 1
>>>>> +
>>>>> + operating-points-v2:
>>>>> + maxItems: 1
>>>>> +
>>>>> + power-domains:
>>>>> + items:
>>>>> + - description: MXC or MXA voltage rail
>>>>> + - description: MMCX voltage rail
>>>>> +
>>>>> + power-domain-names:
>>>>> + items:
>>>>> + - const: mx
>>>>> + - const: mmcx
>>>>> +
>>>>> + vdda-0p9-supply:
>>>>> + description: Phandle to a 0.9V regulator supply to a PHY.
>>>>> +
>>>>> + vdda-1p2-supply:
>>>>> + description: Phandle to 1.2V regulator supply to a PHY.
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>> + - reg
>>>>> + - "#phy-cells"
>>>>> + - clocks
>>>>> + - clock-names
>>>>> + - interrupts
>>>>> + - operating-points-v2
>>>>> + - power-domains
>>>>> + - power-domain-names
>>>>> + - vdda-0p9-supply
>>>>> + - vdda-1p2-supply
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> + #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
>>>>> + #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
>>>>> + #include <dt-bindings/phy/phy-qcom-mipi-csi2.h>
>>>>> + #include <dt-bindings/power/qcom,rpmhpd.h>
>>>>> +
>>>>> + csiphy4: csiphy@ace4000 {
>>>>> + compatible = "qcom,x1e80100-csi2-phy";
>>>>> + reg = <0x0ace4000 0x2000>;
>>>>> + #phy-cells = <1>;
>>>>> +
>>>>> + clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
>>>>> + <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
>>>>> + clock-names = "core",
>>>>> + "timer";
>>>>> +
>>>>> + operating-points-v2 = <&csiphy_opp_table>;
>>>>> +
>>>>> + interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
>>>>> +
>>>>> + power-domains = <&rpmhpd RPMHPD_MX>,
>>>>> + <&rpmhpd RPMHPD_MMCX>;
>>>>> + power-domain-names = "mx",
>>>>> + "mmcx";
>>>>> +
>>>>> + vdda-0p9-supply = <&vreg_l2c_0p8>;
>>>>> + vdda-1p2-supply = <&vreg_l1c_1p2>;
>>>>> + };
>>>>> +
>>>>> + csiphy_opp_table: opp-table {
>>>>> + compatible = "operating-points-v2";
>>>>> +
>>>>> + opp-300000000 {
>>>>> + opp-hz = /bits/ 64 <300000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs_d1>,
>>>>> + <&rpmhpd_opp_low_svs_d1>;
>>>>> + };
>>>>> +
>>>>> + opp-400000000 {
>>>>> + opp-hz = /bits/ 64 <400000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>> + <&rpmhpd_opp_low_svs>;
>>>>> + };
>>>>> +
>>>>> + opp-480000000 {
>>>>> + opp-hz = /bits/ 64 <480000000>;
>>>>> + required-opps = <&rpmhpd_opp_low_svs>,
>>>>> + <&rpmhpd_opp_low_svs>;
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + isp@acb7000 {
>>>>> + phys = <&csiphy4 PHY_QCOM_CSI2_MODE_DPHY>;
>>>>> + };
>>>>
>>>> This example is incomplete in sense that it does not include CAMSS
>>>> CSIPHY IP hardware configuration in whole.
>>>
>>>
>>> No that's not the way examples work. You don't replicate entire nodes
>>> from other schemas you just give a terse reference.
>>>
>>
>> If so, then this example makes no sense and it'd be better to remove it.
> You know you're right its not strictly necessary - its just there to be
> helpful.
>
> "Be less helpful" is not usually review feedback I give or take but,
> I'll drop this anyway.
>
Thank you.
--
Best wishes,
Vladimir
On 26/03/2026 14:49, Vladimir Zapolskiy wrote: > Here the description of hardware is done, and my point is that the new > PHY_QCOM_CSI2_MODE_SPLIT_DPHY phy type is simply not needed, since it's > possible to give a proper description of hardware without this invention. Perhaps I'm not understanding you. If we use PHY_TYPE_DPHY include/dt-bindings/phy/phy.h:#define PHY_TYPE_DPHY 10 We _must_ then add SPLIT_MODE to phy.h if/when we implement that support. Which means successfully arguing the toss of weather SPLIT_MODE is a Qualcommism - a vendor specific mode or not. <&phy PHY_TYPE_DPHY> committed to an upstream dts will then need to be supported perpetually. So for example qrb5615 - kona/rb5 support split mode. Pretend go with <&phy PHY_TYPE_DPHY>; and retrofit individual PHY support to this platform. Grand so far. The pretend we want to switch from one sensor to a split-mode sensor on the existing mezzanine. Then we need a representation of split mode in phy.h to represent that in DT. <&phy PHY_TYPE_DPHY_SPLIT_MODE>; Except split-mode is not an appropriate mode to define in phy.h since it is vendor specific - even if a few vendors support it, its not a generic PHY mode. Hence we would have an enormously difficult time justifying adding that mode to phy.h and rightly so. >> https://review.lineageos.org/c/LineageOS/ >> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/ >> cam_csiphy/cam_csiphy_core.c#b285 >> >> There is disjunction all over this file depending on the mode. >> >> https://review.lineageos.org/c/LineageOS/ >> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/ >> cam_csiphy/cam_csiphy_core.c#b767 OTOH - SPLIT_MODE will certainly require _both_ separate init sequences and specific logical disjunction for additional configuration steps lane-assignment and masking, etc. - That phy.h isn't the right location for SPLIT_MODE as its vendor specific. Just look at the modes we have for the USB PHYs same logic => include/dt-bindings/phy/phy-qcom-qmp.h same raison d'être - And that specifying PHY_TYPE_DPHY now binds us into an ABI that we cannot subsequently change - it will not be possible to introduce include/dt-bindings/phy/phy-qcom-mipi-csi2.h later on with our mode So therefore include/dt-bindings/phy/phy-qcom-mipi-csi2.h + PHY modes is the logical outcome. --- bod
On 3/27/26 03:03, Bryan O'Donoghue wrote: > On 26/03/2026 14:49, Vladimir Zapolskiy wrote: >> Here the description of hardware is done, and my point is that the new >> PHY_QCOM_CSI2_MODE_SPLIT_DPHY phy type is simply not needed, since it's >> possible to give a proper description of hardware without this invention. > > Perhaps I'm not understanding you. You are welcome to ask questions, it may save time. > If we use PHY_TYPE_DPHY > > include/dt-bindings/phy/phy.h:#define PHY_TYPE_DPHY 10 > > We _must_ then add SPLIT_MODE to phy.h if/when we implement that > support. I don't think it is the must. > Which means successfully arguing the toss of weather SPLIT_MODE > is a Qualcommism - a vendor specific mode or not. > > <&phy PHY_TYPE_DPHY> committed to an upstream dts will then need to be > supported perpetually. > First of all, nobody says/defines that the phy type is mandatory to be present in the cell at all, for instance it could be provided over bus-type property of media endpoints, and a connected sensor unavoidably postulates the value of this property. > So for example qrb5615 - kona/rb5 support split mode. > > Pretend go with <&phy PHY_TYPE_DPHY>; and retrofit individual PHY > support to this platform. > > Grand so far. > > The pretend we want to switch from one sensor to a split-mode sensor on > the existing mezzanine. You may think how it should be done, it's been asked for a while to provide a complete valid example, it may help you to get a better understanding of the whole picture. > > Then we need a representation of split mode in phy.h to represent that > in DT. Some kind of split mode representation should be in DT, it does not mean that it sticks to phy.h or whatever else. Lanes (and bus-type) are described under endpoint device tree nodes, this is totally sufficient to separate what you call "a split mode". So, it excludes phy.h. > > <&phy PHY_TYPE_DPHY_SPLIT_MODE>; > > Except split-mode is not an appropriate mode to define in phy.h since it > is vendor specific - even if a few vendors support it, its not a generic > PHY mode. > > Hence we would have an enormously difficult time justifying adding that > mode to phy.h and rightly so. We still discuss a hardware description, it should not be problematic to provide descriptions of regular DPHY and what you call 'split mode' DPHY without any new extentions of the existing dt bindings. >>> https://review.lineageos.org/c/LineageOS/ >>> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/ >>> cam_csiphy/cam_csiphy_core.c#b285 >>> >>> There is disjunction all over this file depending on the mode. >>> >>> https://review.lineageos.org/c/LineageOS/ >>> android_kernel_motorola_sm6375/+/423960/1/drivers/cam_sensor_module/ >>> cam_csiphy/cam_csiphy_core.c#b767 > > > OTOH > > - SPLIT_MODE will certainly require _both_ separate init sequences > and specific logical disjunction for additional configuration steps > lane-assignment and masking, etc. > > - That phy.h isn't the right location for SPLIT_MODE as its vendor > specific. Just look at the modes we have for the USB PHYs > same logic => include/dt-bindings/phy/phy-qcom-qmp.h same > raison d'être > > - And that specifying PHY_TYPE_DPHY now binds us into an ABI that we > cannot subsequently change - it will not be possible to introduce > include/dt-bindings/phy/phy-qcom-mipi-csi2.h later on with our mode > > So therefore include/dt-bindings/phy/phy-qcom-mipi-csi2.h + PHY modes is > the logical outcome. > Unnecessary extention of the dt bindings ABI is not needed to complete the task. -- Best wishes, Vladimir
© 2016 - 2026 Red Hat, Inc.