From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Adam Ford <aford173@gmail.com>,
Conor Dooley <conor+dt@kernel.org>,
Fabio Estevam <festevam@gmail.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Marek Vasut <marex@denx.de>,
Paul Elder <paul.elder@ideasonboard.com>,
Peng Fan <peng.fan@nxp.com>, Rob Herring <robh@kernel.org>,
Sascha Hauer <s.hauer@pengutronix.de>,
Shawn Guo <shawnguo@kernel.org>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
devicetree@vger.kernel.org, linux-media@vger.kernel.org
Subject: Re: [PATCH] arm64: dts: imx8mp: Clarify csis clock frequency
Date: Thu, 15 Aug 2024 14:12:09 +0200 [thread overview]
Message-ID: <3314370.44csPzL39Z@steina-w> (raw)
In-Reply-To: <20240813234010.17852-1-laurent.pinchart@ideasonboard.com>
Hi Laurent,
thanks for the patch.
Am Mittwoch, 14. August 2024, 01:40:10 CEST schrieb Laurent Pinchart:
> The DT nodes for the MIPI CSI-2 receivers (MIPI_CSI) configure the
> CAM1_PIX and CAM2_PIX clocks to 266 MHz through the assigned-clock-rates
> property, and report that frequency in the clock-frequency property. The
> i.MX8MP reference manual and datasheet list 266 MHz as a nominal
> frequency when using both CSI-2 receivers, so all looks normal.
>
> In reality, the clock is actually set to 250 MHz, as the selected
> parent, IMX8MP_SYS_PLL2_1000M, has a 1/4 output that is selected as the
> closest frequency to 266 MHz. This doesn't break operation of the
> device, but is clearly misleading.
>
> Clarify the clock configuration by selecting the IMX8MP_SYS_PLL2_250M
> parent, dropping the redundant assigned-clock-rates, and setting
> clock-frequency to 250 MHz. This doesn't cause any functional change.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 603dfe80216f..d9b5c40f6460 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1687,7 +1687,7 @@ mipi_csi_0: csi@32e40000 {
> compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
> reg = <0x32e40000 0x10000>;
> interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> - clock-frequency = <266000000>;
> + clock-frequency = <250000000>;
This doesn't seem to be necessary, no? I can't be read from
IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT. But oh well, that's what we have right now.
Despite that:
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
> <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> @@ -1695,9 +1695,8 @@ mipi_csi_0: csi@32e40000 {
> clock-names = "pclk", "wrap", "phy", "axi";
> assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>,
> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
> - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_250M>,
> <&clk IMX8MP_CLK_24M>;
> - assigned-clock-rates = <266000000>;
> power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
> status = "disabled";
>
> @@ -1723,7 +1722,7 @@ mipi_csi_1: csi@32e50000 {
> compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
> reg = <0x32e50000 0x10000>;
> interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
> - clock-frequency = <266000000>;
> + clock-frequency = <250000000>;
> clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
> <&clk IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>,
> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> @@ -1731,9 +1730,8 @@ mipi_csi_1: csi@32e50000 {
> clock-names = "pclk", "wrap", "phy", "axi";
> assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM2_PIX>,
> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
> - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_250M>,
> <&clk IMX8MP_CLK_24M>;
> - assigned-clock-rates = <266000000>;
> power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>;
> status = "disabled";
>
>
> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
next prev parent reply other threads:[~2024-08-15 12:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-13 23:40 [PATCH] arm64: dts: imx8mp: Clarify csis clock frequency Laurent Pinchart
2024-08-15 12:12 ` Alexander Stein [this message]
2024-08-15 22:22 ` Laurent Pinchart
2024-08-31 13:14 ` Shawn Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3314370.44csPzL39Z@steina-w \
--to=alexander.stein@ew.tq-group.com \
--cc=aford173@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=festevam@gmail.com \
--cc=imx@lists.linux.dev \
--cc=kernel@pengutronix.de \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-media@vger.kernel.org \
--cc=marex@denx.de \
--cc=paul.elder@ideasonboard.com \
--cc=peng.fan@nxp.com \
--cc=robh@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.