linux-kernel - Re: [PATCH v6 5/5] arm64: dts: mediatek: mt8365: Add support for camera

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2e4d8a1-f683-415d-853f-b55abdb8c27c@collabora.com>
Date: Mon, 29 Jul 2024 17:08:48 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Julien Stephan <jstephan@...libre.com>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Andy Hsieh <andy.hsieh@...iatek.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>
Cc: linux-media@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v6 5/5] arm64: dts: mediatek: mt8365: Add support for
 camera

Il 29/07/24 16:48, Julien Stephan ha scritto:
> Add base support for cameras for mt8365 platforms. This requires nodes
> for the sensor interface, camsv, and CSI receivers.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Signed-off-by: Julien Stephan <jstephan@...libre.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt8365.dtsi | 125 +++++++++++++++++++++++++++++++
>   1 file changed, 125 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8365.dtsi b/arch/arm64/boot/dts/mediatek/mt8365.dtsi
> index 24581f7410aa..cabdb51f4041 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8365.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8365.dtsi
> @@ -10,6 +10,7 @@
>   #include <dt-bindings/interrupt-controller/irq.h>
>   #include <dt-bindings/phy/phy.h>
>   #include <dt-bindings/power/mediatek,mt8365-power.h>
> +#include <dt-bindings/memory/mediatek,mt8365-larb-port.h>
>   
>   / {
>   	compatible = "mediatek,mt8365";
> @@ -703,6 +704,23 @@ ethernet: ethernet@...a0000 {
>   			status = "disabled";
>   		};
>   
> +		mipi_csi0: mipi-csi0@...10000 {

Names must be generic. This is mipi-csi-phy@xyz or csi-phy@xyz as it looks
like it's not the entire CSI interface but just its PHY. Am I wrong?

> +			compatible = "mediatek,mt8365-csi-rx";
> +			reg = <0 0x11c10000 0 0x2000>;
> +			status = "disabled";
> +			num-lanes = <4>;
> +			#phy-cells = <1>;
> +		};
> +
> +		mipi_csi1: mipi-csi1@...12000 {

Same for this one.

> +			compatible = "mediatek,mt8365-csi-rx";
> +			reg = <0 0x11c12000 0 0x2000>;
> +			phy-type = <PHY_TYPE_DPHY>;

...so the secondary CSIPHY only supports DPHY, while the first one supports either
C or D?

> +			status = "disabled";
> +			num-lanes = <4>;
> +			#phy-cells = <0>;

...and then, it's confusing, one CSI has got one PHY cell, one has got two?

> +		};
> +
>   		u3phy: t-phy@...c0000 {
>   			compatible = "mediatek,mt8365-tphy", "mediatek,generic-tphy-v2";
>   			#address-cells = <1>;
> @@ -773,6 +791,113 @@ larb2: larb@...01000 {
>   			mediatek,larb-id = <2>;
>   		};
>   
> +		seninf: seninf@...40000 {
> +			compatible = "mediatek,mt8365-seninf";
> +			reg = <0 0x15040000 0 0x6000>;
> +			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_LOW>;
> +			clocks = <&camsys CLK_CAM_SENIF>,
> +				 <&topckgen CLK_TOP_SENIF_SEL>;
> +			clock-names = "camsys", "top_mux";
> +
> +			power-domains = <&spm MT8365_POWER_DOMAIN_CAM>;
> +
> +			phys = <&mipi_csi0 PHY_TYPE_DPHY>, <&mipi_csi1>;
> +			phy-names = "csi0", "csi1";
> +
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +				};
> +
> +				port@3 {
> +					reg = <3>;
> +				};
> +

Empty ports, why?

Regards,
Angelo