[v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Changhuang Liang]

Add camera subsystem nodes for the StarFive JH7110 SoC. They contain the
imx219, dphy-rx, csi2rx, camss nodes.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 103 ++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  67 ++++++++++++
 2 files changed, 170 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index b89e9791efa7..e0027bb379ef 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -30,6 +30,37 @@ cpus {
 		timebase-frequency = <4000000>;
 	};
 
+	imx219_clk: imx219-clock {
+		compatible = "fixed-clock";
+		clock-output-names = "imx219_clk";
+		clock-frequency = <24000000>;
+		#clock-cells = <0>;
+	};
+
+	imx219_vana_2v8: 2p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "camera_vana";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		regulator-always-on;
+	};
+
+	imx219_vdig_1v8: 1p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "camera_vdig";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <1500000>;
+		regulator-always-on;
+	};
+
+	imx219_vddl_1v2: 1p2v {
+		compatible = "regulator-fixed";
+		regulator-name = "camera_vddl";
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		regulator-always-on;
+	};
+
 	memory@40000000 {
 		device_type = "memory";
 		reg = <0x0 0x40000000 0x1 0x0>;
@@ -125,6 +156,59 @@ &tdm_ext {
 	clock-frequency = <49152000>;
 };
 
+&camss {
+	assigned-clocks = <&ispcrg JH7110_ISPCLK_DOM4_APB_FUNC>,
+			  <&ispcrg JH7110_ISPCLK_MIPI_RX0_PXL>;
+	assigned-clock-rates = <49500000>, <198000000>;
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+		};
+
+		port@1 {
+			reg = <1>;
+
+			camss_from_csi2rx: endpoint {
+				remote-endpoint = <&csi2rx_to_camss>;
+			};
+		};
+	};
+};
+
+&csi2rx {
+	assigned-clocks = <&ispcrg JH7110_ISPCLK_VIN_SYS>;
+	assigned-clock-rates = <297000000>;
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			csi2rx_from_imx219: endpoint {
+				remote-endpoint = <&imx219_to_csi2rx>;
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			csi2rx_to_camss: endpoint {
+				remote-endpoint = <&camss_from_csi2rx>;
+			};
+		};
+	};
+};
+
 &gmac0 {
 	phy-handle = <&phy0>;
 	phy-mode = "rgmii-id";
@@ -228,6 +312,25 @@ &i2c6 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c6_pins>;
 	status = "okay";
+
+	imx219: sensor@10 {
+		compatible = "sony,imx219";
+		reg = <0x10>;
+		clocks = <&imx219_clk>;
+		reset-gpios = <&sysgpio 18 GPIO_ACTIVE_HIGH>;
+		VANA-supply = <&imx219_vana_2v8>;
+		VDIG-supply = <&imx219_vdig_1v8>;
+		VDDL-supply = <&imx219_vddl_1v2>;
+
+		port {
+			imx219_to_csi2rx: endpoint {
+				remote-endpoint = <&csi2rx_from_imx219>;
+				data-lanes = <1 2>;
+				clock-noncontinuous;
+				link-frequencies = /bits/ 64 <456000000>;
+			};
+		};
+	};
 };
 
 &i2srx {
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 45213cdf50dc..6ea1c3f5dded 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -1104,6 +1104,32 @@ pwrc: power-controller@17030000 {
 			#power-domain-cells = <1>;
 		};
 
+		csi2rx: csi@19800000 {
+			compatible = "starfive,jh7110-csi2rx", "cdns,csi2rx";
+			reg = <0x0 0x19800000 0x0 0x10000>;
+			clocks = <&ispcrg JH7110_ISPCLK_VIN_SYS>,
+				 <&ispcrg JH7110_ISPCLK_VIN_APB>,
+				 <&ispcrg JH7110_ISPCLK_VIN_PIXEL_IF0>,
+				 <&ispcrg JH7110_ISPCLK_VIN_PIXEL_IF1>,
+				 <&ispcrg JH7110_ISPCLK_VIN_PIXEL_IF2>,
+				 <&ispcrg JH7110_ISPCLK_VIN_PIXEL_IF3>;
+			clock-names = "sys_clk", "p_clk",
+				      "pixel_if0_clk", "pixel_if1_clk",
+				      "pixel_if2_clk", "pixel_if3_clk";
+			resets = <&ispcrg JH7110_ISPRST_VIN_SYS>,
+				 <&ispcrg JH7110_ISPRST_VIN_APB>,
+				 <&ispcrg JH7110_ISPRST_VIN_PIXEL_IF0>,
+				 <&ispcrg JH7110_ISPRST_VIN_PIXEL_IF1>,
+				 <&ispcrg JH7110_ISPRST_VIN_PIXEL_IF2>,
+				 <&ispcrg JH7110_ISPRST_VIN_PIXEL_IF3>;
+			reset-names = "sys", "reg_bank",
+				      "pixel_if0", "pixel_if1",
+				      "pixel_if2", "pixel_if3";
+			phys = <&csi_phy>;
+			phy-names = "dphy";
+			status = "disabled";
+		};
+
 		ispcrg: clock-controller@19810000 {
 			compatible = "starfive,jh7110-ispcrg";
 			reg = <0x0 0x19810000 0x0 0x10000>;
@@ -1121,6 +1147,47 @@ ispcrg: clock-controller@19810000 {
 			power-domains = <&pwrc JH7110_PD_ISP>;
 		};
 
+		csi_phy: phy@19820000 {
+			compatible = "starfive,jh7110-dphy-rx";
+			reg = <0x0 0x19820000 0x0 0x10000>;
+			clocks = <&ispcrg JH7110_ISPCLK_M31DPHY_CFG_IN>,
+				 <&ispcrg JH7110_ISPCLK_M31DPHY_REF_IN>,
+				 <&ispcrg JH7110_ISPCLK_M31DPHY_TX_ESC_LAN0>;
+			clock-names = "cfg", "ref", "tx";
+			resets = <&ispcrg JH7110_ISPRST_M31DPHY_HW>,
+				 <&ispcrg JH7110_ISPRST_M31DPHY_B09_AON>;
+			power-domains = <&aon_syscon JH7110_AON_PD_DPHY_RX>;
+			#phy-cells = <0>;
+		};
+
+		camss: isp@19840000 {
+			compatible = "starfive,jh7110-camss";
+			reg = <0x0 0x19840000 0x0 0x10000>,
+			      <0x0 0x19870000 0x0 0x30000>;
+			reg-names = "syscon", "isp";
+			clocks = <&ispcrg JH7110_ISPCLK_DOM4_APB_FUNC>,
+				 <&ispcrg JH7110_ISPCLK_ISPV2_TOP_WRAPPER_C>,
+				 <&ispcrg JH7110_ISPCLK_DVP_INV>,
+				 <&ispcrg JH7110_ISPCLK_VIN_P_AXI_WR>,
+				 <&ispcrg JH7110_ISPCLK_MIPI_RX0_PXL>,
+				 <&syscrg JH7110_SYSCLK_ISP_TOP_CORE>,
+				 <&syscrg JH7110_SYSCLK_ISP_TOP_AXI>;
+			clock-names = "apb_func", "wrapper_clk_c", "dvp_inv",
+				      "axiwr", "mipi_rx0_pxl", "ispcore_2x",
+				      "isp_axi";
+			resets = <&ispcrg JH7110_ISPRST_ISPV2_TOP_WRAPPER_P>,
+				 <&ispcrg JH7110_ISPRST_ISPV2_TOP_WRAPPER_C>,
+				 <&ispcrg JH7110_ISPRST_VIN_P_AXI_RD>,
+				 <&ispcrg JH7110_ISPRST_VIN_P_AXI_WR>,
+				 <&syscrg JH7110_SYSRST_ISP_TOP>,
+				 <&syscrg JH7110_SYSRST_ISP_TOP_AXI>;
+			reset-names = "wrapper_p", "wrapper_c", "axird",
+				      "axiwr", "isp_top_n", "isp_top_axi";
+			power-domains = <&pwrc JH7110_PD_ISP>;
+			interrupts = <92>, <87>, <90>, <88>;
+			status = "disabled";
+		};
+
 		voutcrg: clock-controller@295c0000 {
 			compatible = "starfive,jh7110-voutcrg";
 			reg = <0x0 0x295c0000 0x0 0x10000>;
-- 
2.25.1

Re: [v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

On Tue, Jan 30, 2024 at 12:25:09AM -0800, Changhuang Liang wrote:
> Add camera subsystem nodes for the StarFive JH7110 SoC. They contain the
> imx219, dphy-rx, csi2rx, camss nodes.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-2.dtsi         | 103 ++++++++++++++++++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |  67 ++++++++++++
>  2 files changed, 170 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index b89e9791efa7..e0027bb379ef 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -30,6 +30,37 @@ cpus {
>  		timebase-frequency = <4000000>;
>  	};
>  
> +	imx219_clk: imx219-clock {
> +		compatible = "fixed-clock";
> +		clock-output-names = "imx219_clk";
> +		clock-frequency = <24000000>;
> +		#clock-cells = <0>;
> +	};

Why do you need an output name here?
Also, where does this clock come from? Is it an oscillator on the board?

> +	imx219_vana_2v8: 2p8v {

For all of these regulators, please put "regular-" as a prefix for the
node names.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

回复: [v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Changhuang Liang]

Hi, Conor

> On Tue, Jan 30, 2024 at 12:25:09AM -0800, Changhuang Liang wrote:
> > Add camera subsystem nodes for the StarFive JH7110 SoC. They contain
> > the imx219, dphy-rx, csi2rx, camss nodes.
> >
> > Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> > ---
> >  .../jh7110-starfive-visionfive-2.dtsi         | 103
> ++++++++++++++++++
> >  arch/riscv/boot/dts/starfive/jh7110.dtsi      |  67 ++++++++++++
> >  2 files changed, 170 insertions(+)
> >
> > diff --git
> > a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > index b89e9791efa7..e0027bb379ef 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > @@ -30,6 +30,37 @@ cpus {
> >  		timebase-frequency = <4000000>;
> >  	};
> >
> > +	imx219_clk: imx219-clock {
> > +		compatible = "fixed-clock";
> > +		clock-output-names = "imx219_clk";
> > +		clock-frequency = <24000000>;
> > +		#clock-cells = <0>;
> > +	};
> 
> Why do you need an output name here?

The output name can be unnecessary. 

> Also, where does this clock come from? Is it an oscillator on the board?
> 

This clock come from imx219, not the VisionFive 2 board.

regards
Changhuang

Re: 回复: [v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]

On Thu, Feb 01, 2024 at 01:42:13AM +0000, Changhuang Liang wrote:
> Hi, Conor
> 
> > On Tue, Jan 30, 2024 at 12:25:09AM -0800, Changhuang Liang wrote:
> > > Add camera subsystem nodes for the StarFive JH7110 SoC. They contain
> > > the imx219, dphy-rx, csi2rx, camss nodes.
> > >
> > > Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> > > ---
> > >  .../jh7110-starfive-visionfive-2.dtsi         | 103
> > ++++++++++++++++++
> > >  arch/riscv/boot/dts/starfive/jh7110.dtsi      |  67 ++++++++++++
> > >  2 files changed, 170 insertions(+)
> > >
> > > diff --git
> > > a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > > b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > > index b89e9791efa7..e0027bb379ef 100644
> > > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> > > @@ -30,6 +30,37 @@ cpus {
> > >  		timebase-frequency = <4000000>;
> > >  	};
> > >
> > > +	imx219_clk: imx219-clock {
> > > +		compatible = "fixed-clock";
> > > +		clock-output-names = "imx219_clk";
> > > +		clock-frequency = <24000000>;
> > > +		#clock-cells = <0>;
> > > +	};
> > 
> > Why do you need an output name here?
> 
> The output name can be unnecessary. 
> 
> > Also, where does this clock come from? Is it an oscillator on the board?
> > 
> 
> This clock come from imx219, not the VisionFive 2 board.

If the camera is not on the visionfive 2 board, why are you adding this
to visionfive-2.dtsi?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

回复: 回复: [v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Changhuang Liang]

Hi, Conor,

[...]
> > > > +	imx219_clk: imx219-clock {
> > > > +		compatible = "fixed-clock";
> > > > +		clock-output-names = "imx219_clk";
> > > > +		clock-frequency = <24000000>;
> > > > +		#clock-cells = <0>;
> > > > +	};
> > >
> > > Why do you need an output name here?
> >
> > The output name can be unnecessary.
> >
> > > Also, where does this clock come from? Is it an oscillator on the board?
> > >
> >
> > This clock come from imx219, not the VisionFive 2 board.
> 
> If the camera is not on the visionfive 2 board, why are you adding this to
> visionfive-2.dtsi?

It seems that I need to use dtso. Right?

regards
Changhuang

Re: 回复: 回复: [v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On Fri, Feb 02, 2024 at 02:29:16AM +0000, Changhuang Liang wrote:
> 
> Hi, Conor,
> 
> [...]
> > > > > +	imx219_clk: imx219-clock {
> > > > > +		compatible = "fixed-clock";
> > > > > +		clock-output-names = "imx219_clk";
> > > > > +		clock-frequency = <24000000>;
> > > > > +		#clock-cells = <0>;
> > > > > +	};
> > > >
> > > > Why do you need an output name here?
> > >
> > > The output name can be unnecessary.
> > >
> > > > Also, where does this clock come from? Is it an oscillator on the board?
> > > >
> > >
> > > This clock come from imx219, not the VisionFive 2 board.
> > 
> > If the camera is not on the visionfive 2 board, why are you adding this to
> > visionfive-2.dtsi?
> 
> It seems that I need to use dtso. Right?

Yes. FWIW, the same comments as applied for the sound card Walker tried
to add applies here too, unless this ships with the boards:
| I'm not against allowing in-tree overlays for hats/capes/daughter-boards
| that come bundled with a board, but accepting ones for a hat that
| someone decided to use theoretically has no limit! The "someone" in this
| case might be a StarFive developer, but it could be any random one of
| your customers next!
https://lore.kernel.org/linux-riscv/20230510-riveter-ridden-3f056251e623@spud/

I figure this is a similar case as that?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

回复: 回复: 回复: [v2] riscv: dts: starfive: jh7110: Add camera subsystem nodes

[Changhuang Liang]

Hi, Conor,

[...]
> > > > > > +	imx219_clk: imx219-clock {
> > > > > > +		compatible = "fixed-clock";
> > > > > > +		clock-output-names = "imx219_clk";
> > > > > > +		clock-frequency = <24000000>;
> > > > > > +		#clock-cells = <0>;
> > > > > > +	};
> > > > >
> > > > > Why do you need an output name here?
> > > >
> > > > The output name can be unnecessary.
> > > >
> > > > > Also, where does this clock come from? Is it an oscillator on the board?
> > > > >
> > > >
> > > > This clock come from imx219, not the VisionFive 2 board.
> > >
> > > If the camera is not on the visionfive 2 board, why are you adding
> > > this to visionfive-2.dtsi?
> >
> > It seems that I need to use dtso. Right?
> 
> Yes. FWIW, the same comments as applied for the sound card Walker tried to
> add applies here too, unless this ships with the boards:
> | I'm not against allowing in-tree overlays for
> | hats/capes/daughter-boards that come bundled with a board, but
> | accepting ones for a hat that someone decided to use theoretically has
> | no limit! The "someone" in this case might be a StarFive developer,
> | but it could be any random one of your customers next!
> https://lore.kernel.org/linux-riscv/20230510-riveter-ridden-3f056251e623@s
> pud/
> 
> I figure this is a similar case as that?

Yes, I will drop the IMX219 node in next version.