Add in the RGB sensor on CSIPHY4.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts | 77 ++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
index 1611bf7302ddf..b09b437e0cd50 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
@@ -7,6 +7,7 @@
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/gpio-keys.h>
+#include <dt-bindings/phy/phy.h>
#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include "hamoa.dtsi"
@@ -863,6 +864,66 @@ &gpu_zap_shader {
firmware-name = "qcom/x1e80100/LENOVO/83ED/qcdxkmsuc8380.mbn";
};
+&camss {
+ status = "okay";
+
+ ports {
+ /*
+ * port0 => csiphy0
+ * port1 => csiphy1
+ * port2 => csiphy2
+ * port3 => csiphy4
+ */
+ port@3 {
+ camss_csiphy4_inep0: endpoint@0 {
+ clock-lanes = <7>;
+ data-lanes = <0 1>;
+ remote-endpoint = <&ov02c10_ep>;
+ };
+ };
+ };
+};
+
+&cci1 {
+ status = "okay";
+};
+
+&cci1_i2c1 {
+ camera@36 {
+ compatible = "ovti,ov02c10";
+ reg = <0x36>;
+
+ reset-gpios = <&tlmm 237 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&cam_rgb_default>;
+
+ clocks = <&camcc CAM_CC_MCLK4_CLK>;
+ assigned-clocks = <&camcc CAM_CC_MCLK4_CLK>;
+ assigned-clock-rates = <19200000>;
+
+ orientation = <0>; /* front facing */
+
+ avdd-supply = <&vreg_l7b_2p8>;
+ dvdd-supply = <&vreg_l1m_1p2>;
+ dovdd-supply = <&vreg_l3m_1p8>;
+
+ port {
+ ov02c10_ep: endpoint {
+ data-lanes = <1 2>;
+ link-frequencies = /bits/ 64 <400000000>;
+ remote-endpoint = <&camss_csiphy4_inep0>;
+ };
+ };
+ };
+};
+
+&csiphy4 {
+ vdda-0p8-supply = <&vreg_l2c_0p8>;
+ vdda-1p2-supply = <&vreg_l1c_1p2>;
+
+ status = "okay";
+};
+
&i2c0 {
clock-frequency = <400000>;
@@ -1410,6 +1471,22 @@ &tlmm {
<44 4>, /* SPI (TPM) */
<238 1>; /* UFS Reset */
+ cam_rgb_default: cam-rgb-default-state {
+ mclk-pins {
+ pins = "gpio100";
+ function = "cam_aon";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ reset-n-pins {
+ pins = "gpio237";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
edp_reg_en: edp-reg-en-state {
pins = "gpio70";
function = "gpio";
--
2.52.0
On 3/16/26 02:12, Bryan O'Donoghue wrote:
> Add in the RGB sensor on CSIPHY4.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> .../boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts | 77 ++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> index 1611bf7302ddf..b09b437e0cd50 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> @@ -7,6 +7,7 @@
>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/gpio-keys.h>
> +#include <dt-bindings/phy/phy.h>
> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>
> #include "hamoa.dtsi"
> @@ -863,6 +864,66 @@ &gpu_zap_shader {
> firmware-name = "qcom/x1e80100/LENOVO/83ED/qcdxkmsuc8380.mbn";
> };
>
> +&camss {
> + status = "okay";
> +
> + ports {
> + /*
> + * port0 => csiphy0
> + * port1 => csiphy1
> + * port2 => csiphy2
> + * port3 => csiphy4
> + */
> + port@3 {
> + camss_csiphy4_inep0: endpoint@0 {
> + clock-lanes = <7>;
> + data-lanes = <0 1>;
> + remote-endpoint = <&ov02c10_ep>;
This is quite wrong, with the PHY in a separate node, the lanes layout has nothing
to do in the "controller" ports since the sensor is connected to the the PHY which
configures the lanes functions.
The PHY should be a media element in a port/endpoint chain to properly describe the
data flow from the sensor to the controller.
PHY as a separate node is a first step, ideally all components of the CAMSS should be
in separate nodes with port/endpoints describing the whole data interconnection.
Neil
> + };
> + };
> + };
> +};
> +
> +&cci1 {
> + status = "okay";
> +};
> +
> +&cci1_i2c1 {
> + camera@36 {
> + compatible = "ovti,ov02c10";
> + reg = <0x36>;
> +
> + reset-gpios = <&tlmm 237 GPIO_ACTIVE_LOW>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&cam_rgb_default>;
> +
> + clocks = <&camcc CAM_CC_MCLK4_CLK>;
> + assigned-clocks = <&camcc CAM_CC_MCLK4_CLK>;
> + assigned-clock-rates = <19200000>;
> +
> + orientation = <0>; /* front facing */
> +
> + avdd-supply = <&vreg_l7b_2p8>;
> + dvdd-supply = <&vreg_l1m_1p2>;
> + dovdd-supply = <&vreg_l3m_1p8>;
> +
> + port {
> + ov02c10_ep: endpoint {
> + data-lanes = <1 2>;
> + link-frequencies = /bits/ 64 <400000000>;
> + remote-endpoint = <&camss_csiphy4_inep0>;
> + };
> + };
> + };
> +};
> +
> +&csiphy4 {
> + vdda-0p8-supply = <&vreg_l2c_0p8>;
> + vdda-1p2-supply = <&vreg_l1c_1p2>;
> +
> + status = "okay";
> +};
> +
> &i2c0 {
> clock-frequency = <400000>;
>
> @@ -1410,6 +1471,22 @@ &tlmm {
> <44 4>, /* SPI (TPM) */
> <238 1>; /* UFS Reset */
>
> + cam_rgb_default: cam-rgb-default-state {
> + mclk-pins {
> + pins = "gpio100";
> + function = "cam_aon";
> + drive-strength = <16>;
> + bias-disable;
> + };
> +
> + reset-n-pins {
> + pins = "gpio237";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> + };
> +
> edp_reg_en: edp-reg-en-state {
> pins = "gpio70";
> function = "gpio";
>
On 16/03/2026 08:28, Neil Armstrong wrote:
>> + ports {
>> + /*
>> + * port0 => csiphy0
>> + * port1 => csiphy1
>> + * port2 => csiphy2
>> + * port3 => csiphy4
>> + */
Hi.
Thanks for the review.
I think the above comment probably isn't making this very clear.
port0 => csiphy0 => msm_csiphy0 in the media-graph.
>> + port@3 {
>> + camss_csiphy4_inep0: endpoint@0 {
>> + clock-lanes = <7>;
>> + data-lanes = <0 1>;
>> + remote-endpoint = <&ov02c10_ep>;
>
> This is quite wrong, with the PHY in a separate node, the lanes layout
> has nothing
> to do in the "controller" ports since the sensor is connected to the the
> PHY which
> configures the lanes functions.
>
> The PHY should be a media element in a port/endpoint chain to properly
> describe the
> data flow from the sensor to the controller.
If I am reading your comment right, we are already defining the
data-lanes where you've said they should be msm_csiphyX below port@X
here maps to msm_csiphyX in the media graph.
So for example here is how we configure this before and after the
changes in this series
media-ctl -v -d /dev/media0 -V '"ov08x40
'2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGRBG10/3856x2416]'
media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -p
Its a NOP change from user-space's perspective. CAMSS has historically
used port@ to map to msm_csiphyX - taking logical elements and
configuring pieces of the CSIPHY and CSID based on that mapping.
Other examples of this upstream:
/* Cadence CSI2RX */
/* Sensor */
camera@10 {
port {
csi2_cam0: endpoint {
remote-endpoint = <&csi2rx0_in_sensor>;
data-lanes = <1 2>;
};
};
};
/* CSI bridge — NOT the PHY */
&cdns_csi2rx0 {
phys = <&dphy0>;
port@0 {
csi2rx0_in_sensor: endpoint {
remote-endpoint = <&csi2_cam0>;
data-lanes = <1 2>;
};
};
};
---
bod
On 3/16/26 12:46, Bryan O'Donoghue wrote:
> On 16/03/2026 08:28, Neil Armstrong wrote:
>>> + ports {
>>> + /*
>>> + * port0 => csiphy0
>>> + * port1 => csiphy1
>>> + * port2 => csiphy2
>>> + * port3 => csiphy4
>>> + */
>
> Hi.
>
> Thanks for the review.
>
> I think the above comment probably isn't making this very clear.
>
> port0 => csiphy0 => msm_csiphy0 in the media-graph.
>
>>> + port@3 {
>>> + camss_csiphy4_inep0: endpoint@0 {
>>> + clock-lanes = <7>;
>>> + data-lanes = <0 1>;
>>> + remote-endpoint = <&ov02c10_ep>;
>>
>> This is quite wrong, with the PHY in a separate node, the lanes layout has nothing
>> to do in the "controller" ports since the sensor is connected to the the PHY which
>> configures the lanes functions.
>>
>> The PHY should be a media element in a port/endpoint chain to properly describe the
>> data flow from the sensor to the controller.
>
> If I am reading your comment right, we are already defining the data-lanes where you've said they should be msm_csiphyX below port@X here maps to msm_csiphyX in the media graph.
>
> So for example here is how we configure this before and after the changes in this series
>
> media-ctl -v -d /dev/media0 -V '"ov08x40 '2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
> media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
> media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGRBG10/3856x2416]'
> media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
> media-ctl -d /dev/media0 -p
So a csiphy is a media element here, so why implement it as a PHY ? and there's a data link with CSID with should represented with a port/endpoint relationship...
>
> Its a NOP change from user-space's perspective. CAMSS has historically used port@ to map to msm_csiphyX - taking logical elements and configuring pieces of the CSIPHY and CSID based on that mapping.
>
> Other examples of this upstream:
>
> /* Cadence CSI2RX */
> /* Sensor */
> camera@10 {
> port {
> csi2_cam0: endpoint {
> remote-endpoint = <&csi2rx0_in_sensor>;
> data-lanes = <1 2>;
> };
> };
> };
>
> /* CSI bridge — NOT the PHY */
> &cdns_csi2rx0 {
> phys = <&dphy0>;
> port@0 {
> csi2rx0_in_sensor: endpoint {
> remote-endpoint = <&csi2_cam0>;
> data-lanes = <1 2>;
> };
> };
> };
Yeah this is totally unrelated...
Neil
>
> ---
> bod
On 3/18/26 10:26 AM, Neil Armstrong wrote:
> On 3/16/26 12:46, Bryan O'Donoghue wrote:
>> On 16/03/2026 08:28, Neil Armstrong wrote:
>>>> + ports {
>>>> + /*
>>>> + * port0 => csiphy0
>>>> + * port1 => csiphy1
>>>> + * port2 => csiphy2
>>>> + * port3 => csiphy4
>>>> + */
>>
>> Hi.
>>
>> Thanks for the review.
>>
>> I think the above comment probably isn't making this very clear.
>>
>> port0 => csiphy0 => msm_csiphy0 in the media-graph.
>>
>>>> + port@3 {
>>>> + camss_csiphy4_inep0: endpoint@0 {
>>>> + clock-lanes = <7>;
>>>> + data-lanes = <0 1>;
>>>> + remote-endpoint = <&ov02c10_ep>;
>>>
>>> This is quite wrong, with the PHY in a separate node, the lanes layout has nothing
>>> to do in the "controller" ports since the sensor is connected to the the PHY which
>>> configures the lanes functions.
>>>
>>> The PHY should be a media element in a port/endpoint chain to properly describe the
>>> data flow from the sensor to the controller.
>>
>> If I am reading your comment right, we are already defining the data-lanes where you've said they should be msm_csiphyX below port@X here maps to msm_csiphyX in the media graph.
>>
>> So for example here is how we configure this before and after the changes in this series
>>
>> media-ctl -v -d /dev/media0 -V '"ov08x40 '2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
>> media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
>> media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
>> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGRBG10/3856x2416]'
>> media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
>> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
>> media-ctl -d /dev/media0 -p
>
> So a csiphy is a media element here, so why implement it as a PHY ? and there's a data link with CSID with should represented with a port/endpoint relationship...
Does it need to be a media element in the first place?
If it's just about configuration, then we can (?) make an assumption
that the PHY is hardwired to the sensor, and if we want/need to mux
CSIDn to the sensor device, which would then trigger a change in the
PHY (if the PHY even needs to know it's being remuxed?)
i.e.
the above sequence would have:
media-ctl -l '"ov08x40 '2-0036'":1->"msm_csid0":0[1]'
Konrad
© 2016 - 2026 Red Hat, Inc.