From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E879BC77B76 for ; Mon, 17 Apr 2023 08:15:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229912AbjDQIPE (ORCPT ); Mon, 17 Apr 2023 04:15:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33946 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229803AbjDQIPD (ORCPT ); Mon, 17 Apr 2023 04:15:03 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D0961FDB; Mon, 17 Apr 2023 01:15:00 -0700 (PDT) Received: from pendragon.ideasonboard.com (133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F249DDE6; Mon, 17 Apr 2023 10:14:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1681719293; bh=fB6GDrDT9giAq0/ETq0gBYFcxiahDr5O0kDTfl1LlZo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CtYkEImMB844Wi5MM3czBnQCBQ8wK8QsNIxmSpIcjTR48F6R/V2Ttb6v/XckA0zxb 28xcLndxd4GUsh5wLA5XkfID8Hlhz12PeUu+hiA6Lx7nmUeT5ZlcVeierl2ABretFB brv6wS++dOrEGWjqv4v50APmWF1Z6TiRvpP1NiLY= Date: Mon, 17 Apr 2023 11:15:10 +0300 From: Laurent Pinchart To: Marco Felsch Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, kernel@pengutronix.de, Xavier Roumegue , Rob Herring , linux-imx@nxp.com, Krzysztof Kozlowski , Jacopo Mondi , Shawn Guo , linux-media@vger.kernel.org Subject: Re: [PATCH v1 1/2] arm64: dts: imx8mp: Add CSIS DT nodes Message-ID: <20230417081510.GA19964@pendragon.ideasonboard.com> References: <20230417055627.16482-1-laurent.pinchart@ideasonboard.com> <20230417055627.16482-2-laurent.pinchart@ideasonboard.com> <20230417065059.fgmdfwk7pnj62amm@pengutronix.de> <20230417074148.GF28551@pendragon.ideasonboard.com> <20230417080117.jiqpynebq2we2hh4@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20230417080117.jiqpynebq2we2hh4@pengutronix.de> Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi Marco, On Mon, Apr 17, 2023 at 10:01:17AM +0200, Marco Felsch wrote: > On 23-04-17, Laurent Pinchart wrote: > > On Mon, Apr 17, 2023 at 08:50:59AM +0200, Marco Felsch wrote: > > > Hi Laurent, > > > > > > your patch LGTM just one nit/idea, please see below. > > > > > > On 23-04-17, Laurent Pinchart wrote: > > > > Add DT nodes for the two CSI-2 receivers of the i.MX8MP. > > > > > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 60 +++++++++++++++++++++++ > > > > 1 file changed, 60 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > > > index 2dd60e3252f3..2a374a4c14a2 100644 > > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > > > @@ -1239,6 +1239,66 @@ ldb_lvds_ch1: endpoint { > > > > }; > > > > }; > > > > > > > > + mipi_csi_0: csi@32e40000 { > > > > + compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2"; > > > > + reg = <0x32e40000 0x10000>; > > > > + interrupts = ; > > > > + clock-frequency = <500000000>; > > > > + clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>; > > > > + clock-names = "pclk", "wrap", "phy", "axi"; > > > > + assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>; > > > > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>; > > > > + assigned-clock-rates = <500000000>; > > > > + power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>; > > > > + status = "disabled"; > > > > + > > > > + ports { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + port@0 { > > > > + reg = <0>; > > > > > > If we would add: > > > mipi_csi_0_in: endpoint {}; > > > > > > here we could refernce it from overlays/board dts files more easily. > > > > Isn't there an unwritten rule (or consensus) that an endpoint should > > always have a remote-endpoint property ? > > I don't know if there is one. > > > While ports describe hardware properties of a device and should always > > be there regardless of connections, endpoints describe connections and > > I don't think they should be instantiated with a valid > > remote-endpoint. > > I know, therefore I mentioned it as idea to make it 'easier' to add > camera nodes. As a middleground, would it be useful to have a label for the port ? Something like mipi_csi_0: csi@32e40000 { ports { mipi_csi_0_port_0: port@0 { }; }; }; An overlay could then reference that and create the endpoint. I'm not entirely sure how useful that would be though, as the overlay would need to enable the CSI node anyway. Compare -------- &mipi_csi_0 { status = "okay"; }; &mipi_csi_0_port_0 { mipi_csi_0_in: endpoint { remote-endpoint = <&imx327_out>; }; }; -------- with -------- &mipi_csi_0 { status = "okay"; ports { port@0 { mipi_csi_0_in: endpoint { remote-endpoint = <&imx327_out>; }; }; }; }; -------- I have a slight preference for the latter as it groups all the CSI0 data in a single overlay target, but if the former is generally preferred, I'm fine with that too. > > > > + }; > > > > + > > > > + port@1 { > > > > + reg = <1>; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > + mipi_csi_1: csi@32e50000 { > > > > + compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2"; > > > > + reg = <0x32e50000 0x10000>; > > > > + interrupts = ; > > > > + clock-frequency = <266000000>; > > > > + clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>; > > > > + clock-names = "pclk", "wrap", "phy", "axi"; > > > > + assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM2_PIX>; > > > > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>; > > > > + assigned-clock-rates = <266000000>; > > > > + power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>; > > > > + status = "disabled"; > > > > + > > > > + ports { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + port@0 { > > > > + reg = <0>; > > > > + }; > > > > + > > > > + port@1 { > > > > + reg = <1>; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > pcie_phy: pcie-phy@32f00000 { > > > > compatible = "fsl,imx8mp-pcie-phy"; > > > > reg = <0x32f00000 0x10000>; -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 45821C77B72 for ; Mon, 17 Apr 2023 08:15:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=LSYYM4Wrv6Yx97fE5Or8ehwlWCfWneinJFe4psctA4c=; b=MtZVMX7ZjNZTEC UtD2r/N0FopFxDvMJ5zKMjw+9Xbv3+1x4I+sZARFrht/ncHrBy2OQDDYH88tQxG6lQwSaw6BMUpgF FYK3wboSz05LBs2RS002KvkCKfkcwPGVyNTjUmZK1Mskzktdjk+Ko+A3WIvDHi6yGauCyVl0WN+5w v0MQH0NMTY8uV6LAhbQbz3vY3TasGzjsNRXdkJ3HogKHJqT3wnYSbv0xLJ4YO4MA2IgO1TcA9xK91 fGVhuNOacOaT/CEYaZ5zMOXBz2+U0ilfH9OWvTV4suDdV4SI/Z6UdgvPw6CQ5L7i5YPgaGWodQL4C GRKZlbCMMaUw/MwAKFQQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1poK0s-00FLQc-21; Mon, 17 Apr 2023 08:15:02 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1poK0p-00FLQ0-26 for linux-arm-kernel@lists.infradead.org; Mon, 17 Apr 2023 08:15:01 +0000 Received: from pendragon.ideasonboard.com (133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F249DDE6; Mon, 17 Apr 2023 10:14:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1681719293; bh=fB6GDrDT9giAq0/ETq0gBYFcxiahDr5O0kDTfl1LlZo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CtYkEImMB844Wi5MM3czBnQCBQ8wK8QsNIxmSpIcjTR48F6R/V2Ttb6v/XckA0zxb 28xcLndxd4GUsh5wLA5XkfID8Hlhz12PeUu+hiA6Lx7nmUeT5ZlcVeierl2ABretFB brv6wS++dOrEGWjqv4v50APmWF1Z6TiRvpP1NiLY= Date: Mon, 17 Apr 2023 11:15:10 +0300 From: Laurent Pinchart To: Marco Felsch Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, kernel@pengutronix.de, Xavier Roumegue , Rob Herring , linux-imx@nxp.com, Krzysztof Kozlowski , Jacopo Mondi , Shawn Guo , linux-media@vger.kernel.org Subject: Re: [PATCH v1 1/2] arm64: dts: imx8mp: Add CSIS DT nodes Message-ID: <20230417081510.GA19964@pendragon.ideasonboard.com> References: <20230417055627.16482-1-laurent.pinchart@ideasonboard.com> <20230417055627.16482-2-laurent.pinchart@ideasonboard.com> <20230417065059.fgmdfwk7pnj62amm@pengutronix.de> <20230417074148.GF28551@pendragon.ideasonboard.com> <20230417080117.jiqpynebq2we2hh4@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230417080117.jiqpynebq2we2hh4@pengutronix.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230417_011459_854359_20DBEBEB X-CRM114-Status: GOOD ( 28.67 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Marco, On Mon, Apr 17, 2023 at 10:01:17AM +0200, Marco Felsch wrote: > On 23-04-17, Laurent Pinchart wrote: > > On Mon, Apr 17, 2023 at 08:50:59AM +0200, Marco Felsch wrote: > > > Hi Laurent, > > > > > > your patch LGTM just one nit/idea, please see below. > > > > > > On 23-04-17, Laurent Pinchart wrote: > > > > Add DT nodes for the two CSI-2 receivers of the i.MX8MP. > > > > > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 60 +++++++++++++++++++++++ > > > > 1 file changed, 60 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > > > index 2dd60e3252f3..2a374a4c14a2 100644 > > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > > > @@ -1239,6 +1239,66 @@ ldb_lvds_ch1: endpoint { > > > > }; > > > > }; > > > > > > > > + mipi_csi_0: csi@32e40000 { > > > > + compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2"; > > > > + reg = <0x32e40000 0x10000>; > > > > + interrupts = ; > > > > + clock-frequency = <500000000>; > > > > + clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>; > > > > + clock-names = "pclk", "wrap", "phy", "axi"; > > > > + assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>; > > > > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>; > > > > + assigned-clock-rates = <500000000>; > > > > + power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>; > > > > + status = "disabled"; > > > > + > > > > + ports { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + port@0 { > > > > + reg = <0>; > > > > > > If we would add: > > > mipi_csi_0_in: endpoint {}; > > > > > > here we could refernce it from overlays/board dts files more easily. > > > > Isn't there an unwritten rule (or consensus) that an endpoint should > > always have a remote-endpoint property ? > > I don't know if there is one. > > > While ports describe hardware properties of a device and should always > > be there regardless of connections, endpoints describe connections and > > I don't think they should be instantiated with a valid > > remote-endpoint. > > I know, therefore I mentioned it as idea to make it 'easier' to add > camera nodes. As a middleground, would it be useful to have a label for the port ? Something like mipi_csi_0: csi@32e40000 { ports { mipi_csi_0_port_0: port@0 { }; }; }; An overlay could then reference that and create the endpoint. I'm not entirely sure how useful that would be though, as the overlay would need to enable the CSI node anyway. Compare -------- &mipi_csi_0 { status = "okay"; }; &mipi_csi_0_port_0 { mipi_csi_0_in: endpoint { remote-endpoint = <&imx327_out>; }; }; -------- with -------- &mipi_csi_0 { status = "okay"; ports { port@0 { mipi_csi_0_in: endpoint { remote-endpoint = <&imx327_out>; }; }; }; }; -------- I have a slight preference for the latter as it groups all the CSI0 data in a single overlay target, but if the former is generally preferred, I'm fine with that too. > > > > + }; > > > > + > > > > + port@1 { > > > > + reg = <1>; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > + mipi_csi_1: csi@32e50000 { > > > > + compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2"; > > > > + reg = <0x32e50000 0x10000>; > > > > + interrupts = ; > > > > + clock-frequency = <266000000>; > > > > + clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>, > > > > + <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>; > > > > + clock-names = "pclk", "wrap", "phy", "axi"; > > > > + assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM2_PIX>; > > > > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>; > > > > + assigned-clock-rates = <266000000>; > > > > + power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>; > > > > + status = "disabled"; > > > > + > > > > + ports { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + port@0 { > > > > + reg = <0>; > > > > + }; > > > > + > > > > + port@1 { > > > > + reg = <1>; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > pcie_phy: pcie-phy@32f00000 { > > > > compatible = "fsl,imx8mp-pcie-phy"; > > > > reg = <0x32f00000 0x10000>; -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel