| Message ID | 20230918-imx8mp-dtsi-v1-2-1d008b3237c0@skidata.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | imx8mp: first clock propagation attempt (for LVDS) | expand |
On Sun, Sep 17, 2023 at 5:40 PM Benjamin Bara <bbara93@gmail.com> wrote: > > From: Benjamin Bara <benjamin.bara@skidata.com> > > Similar to commit 07bb2e368820 ("arm64: dts: imx8mp: Fix video clock > parents") the parent of IMX8MP_CLK_MEDIA_MIPI_PHY1_REF should be set in > the media_blk_ctrl. Currently, if mipi_dsi is not in use, its parent is > set to IMX8MP_VIDEO_PLL1_OUT, and might therefore clash with the > constraints coming from a panel. From what I can see, it looks like the IMX8MP_CLK_MEDIA_MIPI_PHY1_REF parent is being set to IMX8MP_CLK_24M. Isn't that the default? I also don't think we need to set a 24MHz clock to 24MHz if that's the default. If that is the case, I would suggest we try to remove the assignment altogether to make the device tree simpler and less to untangle if a board needs to manually manipulate the clocks for some specific reason. adam > > Cc: Adam Ford <aford173@gmail.com> > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > index c946749a3d73..9539d747e28e 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -1640,11 +1640,6 @@ mipi_dsi: dsi@32e60000 { > clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, > <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; > clock-names = "bus_clk", "sclk_mipi"; > - assigned-clocks = <&clk IMX8MP_CLK_MEDIA_APB>, > - <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>, > - <&clk IMX8MP_CLK_24M>; > - assigned-clock-rates = <200000000>, <24000000>; > samsung,pll-clock-frequency = <24000000>; > interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; > power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_DSI_1>; > @@ -1747,13 +1742,16 @@ media_blk_ctrl: blk-ctrl@32ec0000 { > <&clk IMX8MP_CLK_MEDIA_APB>, > <&clk IMX8MP_CLK_MEDIA_DISP1_PIX>, > <&clk IMX8MP_CLK_MEDIA_DISP2_PIX>, > - <&clk IMX8MP_VIDEO_PLL1>; > + <&clk IMX8MP_VIDEO_PLL1>, > + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; > assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>, > <&clk IMX8MP_SYS_PLL1_800M>, > <&clk IMX8MP_VIDEO_PLL1_OUT>, > - <&clk IMX8MP_VIDEO_PLL1_OUT>; > + <&clk IMX8MP_VIDEO_PLL1_OUT>, > + <&clk IMX8MP_CLK_24M>; > assigned-clock-rates = <500000000>, <200000000>, > - <0>, <0>, <1039500000>; > + <0>, <0>, <1039500000>, > + <24000000>; > #power-domain-cells = <1>; > > lvds_bridge: bridge@5c { > > -- > 2.34.1 >
Hi Adam, thanks for the feedback! On Tue, 3 Oct 2023 at 15:02, Adam Ford <aford173@gmail.com> wrote: > From what I can see, it looks like the IMX8MP_CLK_MEDIA_MIPI_PHY1_REF > parent is being set to IMX8MP_CLK_24M. Isn't that the default? I also > don't think we need to set a 24MHz clock to 24MHz if that's the > default. I can retry (have the patch applied since then), but as far as I remember, it was not. What was even funnier was that media_mipi_phy1_ref hat a divider != 1 set (it is a composite), so it wasn't sufficient to just re-parent it to OSC_24M - probably because set_rate() was called before it was re-parented to OSC_24M. But thanks for the catch, I will take a look again and adapt it if possible. Regards Benjamin > If that is the case, I would suggest we try to remove the assignment > altogether to make the device tree simpler and less to untangle if a > board needs to manually manipulate the clocks for some specific > reason. > > adam > > > > > Cc: Adam Ford <aford173@gmail.com> > > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > > --- > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > index c946749a3d73..9539d747e28e 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > @@ -1640,11 +1640,6 @@ mipi_dsi: dsi@32e60000 { > > clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, > > <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; > > clock-names = "bus_clk", "sclk_mipi"; > > - assigned-clocks = <&clk IMX8MP_CLK_MEDIA_APB>, > > - <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; > > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>, > > - <&clk IMX8MP_CLK_24M>; > > - assigned-clock-rates = <200000000>, <24000000>; > > samsung,pll-clock-frequency = <24000000>; > > interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; > > power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_DSI_1>; > > @@ -1747,13 +1742,16 @@ media_blk_ctrl: blk-ctrl@32ec0000 { > > <&clk IMX8MP_CLK_MEDIA_APB>, > > <&clk IMX8MP_CLK_MEDIA_DISP1_PIX>, > > <&clk IMX8MP_CLK_MEDIA_DISP2_PIX>, > > - <&clk IMX8MP_VIDEO_PLL1>; > > + <&clk IMX8MP_VIDEO_PLL1>, > > + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; > > assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>, > > <&clk IMX8MP_SYS_PLL1_800M>, > > <&clk IMX8MP_VIDEO_PLL1_OUT>, > > - <&clk IMX8MP_VIDEO_PLL1_OUT>; > > + <&clk IMX8MP_VIDEO_PLL1_OUT>, > > + <&clk IMX8MP_CLK_24M>; > > assigned-clock-rates = <500000000>, <200000000>, > > - <0>, <0>, <1039500000>; > > + <0>, <0>, <1039500000>, > > + <24000000>; > > #power-domain-cells = <1>; > > > > lvds_bridge: bridge@5c { > > > > -- > > 2.34.1 > >
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index c946749a3d73..9539d747e28e 100644 --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi @@ -1640,11 +1640,6 @@ mipi_dsi: dsi@32e60000 { clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>, <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; clock-names = "bus_clk", "sclk_mipi"; - assigned-clocks = <&clk IMX8MP_CLK_MEDIA_APB>, - <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; - assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>, - <&clk IMX8MP_CLK_24M>; - assigned-clock-rates = <200000000>, <24000000>; samsung,pll-clock-frequency = <24000000>; interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_DSI_1>; @@ -1747,13 +1742,16 @@ media_blk_ctrl: blk-ctrl@32ec0000 { <&clk IMX8MP_CLK_MEDIA_APB>, <&clk IMX8MP_CLK_MEDIA_DISP1_PIX>, <&clk IMX8MP_CLK_MEDIA_DISP2_PIX>, - <&clk IMX8MP_VIDEO_PLL1>; + <&clk IMX8MP_VIDEO_PLL1>, + <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>; assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>, <&clk IMX8MP_SYS_PLL1_800M>, <&clk IMX8MP_VIDEO_PLL1_OUT>, - <&clk IMX8MP_VIDEO_PLL1_OUT>; + <&clk IMX8MP_VIDEO_PLL1_OUT>, + <&clk IMX8MP_CLK_24M>; assigned-clock-rates = <500000000>, <200000000>, - <0>, <0>, <1039500000>; + <0>, <0>, <1039500000>, + <24000000>; #power-domain-cells = <1>; lvds_bridge: bridge@5c {