[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240111083255.GC30988@pendragon.ideasonboard.com>
Date: Thu, 11 Jan 2024 10:32:55 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Julien Stephan <jstephan@...libre.com>
Cc: Andy Hsieh <andy.hsieh@...iatek.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
Florian Sylvestre <fsylvestre@...libre.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-media@...r.kernel.org,
Louis Kuo <louis.kuo@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Paul Elder <paul.elder@...asonboard.com>,
Phi-bang Nguyen <pnguyen@...libre.com>,
Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v4 5/5] arm64: dts: mediatek: mt8365: Add support for
camera
Hi Julien,
Thank you for the patch.
On Wed, Jan 10, 2024 at 03:14:42PM +0100, Julien Stephan wrote:
> Add base support for cameras for mt8365 platforms. This requires nodes
> for the sensor interface, camsv, and CSI receivers.
>
> Signed-off-by: Julien Stephan <jstephan@...libre.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8365.dtsi | 128 +++++++++++++++++++++++
> 1 file changed, 128 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8365.dtsi b/arch/arm64/boot/dts/mediatek/mt8365.dtsi
> index 24581f7410aa..9059b2f83b83 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,133 @@ ethernet: ethernet@...a0000 {
> status = "disabled";
> };
>
> + camsv1: camsv@...50000 {
> + compatible = "mediatek,mt8365-camsv";
> + reg = <0 0x15050000 0 0x0040>,
> + <0 0x15050208 0 0x0020>,
> + <0 0x15050400 0 0x0100>;
> + interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&camsys CLK_CAM>,
> + <&camsys CLK_CAMTG>,
> + <&camsys CLK_CAMSV0>;
> + clock-names = "cam", "camtg", "camsv";
> + iommus = <&iommu M4U_PORT_CAM_IMGO>;
> + mediatek,larb = <&larb2>;
> + power-domains = <&spm MT8365_POWER_DOMAIN_CAM>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + camsv1_endpoint: endpoint {
> + remote-endpoint =
> + <&seninf_camsv1_endpoint>;
I think you can keep this on a single line. Same below.
> + };
> + };
> + };
> + };
> +
> + camsv2: camsv@...50800 {
> + compatible = "mediatek,mt8365-camsv";
> + reg = <0 0x15050800 0 0x0040>,
> + <0 0x15050228 0 0x0020>,
> + <0 0x15050C00 0 0x0100>;
Lower-case hex constants.
> + interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&camsys CLK_CAM>,
> + <&camsys CLK_CAMTG>,
> + <&camsys CLK_CAMSV1>;
> + clock-names = "cam", "camtg", "camsv";
> + iommus = <&iommu M4U_PORT_CAM_IMGO>;
> +
Nitpicking, camsv doesn't have a blank line here. I'm fine either way,
but please do the same for both nodes.
> + mediatek,larb = <&larb2>;
> + power-domains = <&spm MT8365_POWER_DOMAIN_CAM>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + camsv2_endpoint: endpoint {
> + remote-endpoint =
> + <&seninf_camsv2_endpoint>;
> + };
> + };
> + };
> + };
> +
> + seninf: seninf@...40000 {
This should go before the two camsv instances to keep nodes sorted by
address. The camsv1, camsv2 and seninf nodes need to be moved further
down for the same reason.
> + 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>;
> + };
> +
> + port@4 {
> + reg = <4>;
> + seninf_camsv1_endpoint: endpoint {
> + remote-endpoint =
> + <&camsv1_endpoint>;
> + };
> + };
> +
> + port@5 {
> + reg = <5>;
> + seninf_camsv2_endpoint: endpoint {
> + remote-endpoint =
> + <&camsv2_endpoint>;
> + };
> + };
> + };
> + };
> +
> + mipi_csi0: mipi-csi0@...10000 {
> + compatible = "mediatek,mt8365-csi-rx";
> + reg = <0 0x11C10000 0 0x2000>;
Lower-case hex constants.
> + status = "disabled";
> + num-lanes = <4>;
> + #phy-cells = <1>;
> + };
> +
> + mipi_csi1: mipi-csi1@...12000 {
> + compatible = "mediatek,mt8365-csi-rx";
> + reg = <0 0x11C12000 0 0x2000>;
Lower-case hex constants.
Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> + phy-type = <PHY_TYPE_DPHY>;
> + status = "disabled";
> + num-lanes = <4>;
> + #phy-cells = <0>;
> + };
> +
> u3phy: t-phy@...c0000 {
> compatible = "mediatek,mt8365-tphy", "mediatek,generic-tphy-v2";
> #address-cells = <1>;
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists