ARM: imx6: Mask mmdc_ch1 handshake for periph2_sel and mmdc_ch1_axi_podf
Subsystems (3)
PATCH 1/3 ARM: imx6: Mask mmdc_ch1 handshake for periph2_sel and mmdc_ch1_axi_podf
MMDC CH1 is not used on i.MX6Q, so the handshake needed to change the parent of periph2_sel or the divider of mmdc_ch1_axi_podf will never succeed. Disable the handshake mechanism to allow changing the frequency of mmdc_ch1_axi, allowing to use it as a possible source for the LDB DI clock.
--- arch/arm/mach-imx/clk-imx6q.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
arch/arm/mach-imx/clk-imx6q.c +14 -0
@@ -120,6 +120,19 @@ static unsigned int share_count_ssi1;static unsigned int share_count_ssi2;static unsigned int share_count_ssi3; +#define CCM_CCDR 0x04 + +#define CCDR_MMDC_CH1_MASK BIT(16) + +static void __init imx6q_mmdc_ch1_mask_handshake (void __iomem *ccm_base) +{ +unsigned int reg; + + reg =readl_relaxed (ccm_base +CCM_CCDR ); + reg |=CCDR_MMDC_CH1_MASK ; +writel_relaxed (reg, ccm_base +CCM_CCDR ); +} +static void __init imx6q_clocks_init (struct device_node *ccm_node) {struct device_node *np;
@@ -261,6 +274,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)WARN_ON (!base);imx6q_pm_set_ccm_base (base); +imx6q_mmdc_ch1_mask_handshake (base);/* name reg shift width parent_names num_parents */ clk[IMX6QDL_CLK_STEP ] =imx_clk_mux ("step" , base +0xc ,8 ,1 , step_sels,ARRAY_SIZE (step_sels));
Signature · 1 line
2.1.4
PATCH 2/3 ARM: imx6: Make the LDB_DI0 and LDB_DI1 clocks read-only.
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.
To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. As this can not be guaranteed by the clock framework during runtime, make the ldb_di[x]_sel muxes read-only. A workaround to set the muxes once during boot could be added to the kernel or bootloader.
--- arch/arm/mach-imx/clk-imx6q.c | 10 ++-------- arch/arm/mach-imx/clk.h | 8 ++++++++ 2 files changed, 10 insertions(+), 8 deletions(-)
arch/arm/mach-imx/clk-imx6q.c +2 -8
@@ -296,8 +296,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk[IMX6QDL_CLK_GPU3D_SHADER_SEL ] =imx_clk_mux ("gpu3d_shader_sel" , base +0x18 ,8 ,2 , gpu3d_shader_sels,ARRAY_SIZE (gpu3d_shader_sels)); clk[IMX6QDL_CLK_IPU1_SEL ] =imx_clk_mux ("ipu1_sel" , base +0x3c ,9 ,2 , ipu_sels,ARRAY_SIZE (ipu_sels)); clk[IMX6QDL_CLK_IPU2_SEL ] =imx_clk_mux ("ipu2_sel" , base +0x3c ,14 ,2 , ipu_sels,ARRAY_SIZE (ipu_sels)); - clk[IMX6QDL_CLK_LDB_DI0_SEL ] =imx_clk_mux_flags ("ldb_di0_sel" , base +0x2c ,9 ,3 , ldb_di_sels,ARRAY_SIZE (ldb_di_sels),CLK_SET_RATE_PARENT ); - clk[IMX6QDL_CLK_LDB_DI1_SEL ] =imx_clk_mux_flags ("ldb_di1_sel" , base +0x2c ,12 ,3 , ldb_di_sels,ARRAY_SIZE (ldb_di_sels),CLK_SET_RATE_PARENT ); + clk[IMX6QDL_CLK_LDB_DI0_SEL ] =imx_clk_mux_ldb ("ldb_di0_sel" , base +0x2c ,9 ,3 , ldb_di_sels,ARRAY_SIZE (ldb_di_sels)); + clk[IMX6QDL_CLK_LDB_DI1_SEL ] =imx_clk_mux_ldb ("ldb_di1_sel" , base +0x2c ,12 ,3 , ldb_di_sels,ARRAY_SIZE (ldb_di_sels)); clk[IMX6QDL_CLK_IPU1_DI0_PRE_SEL ] =imx_clk_mux_flags ("ipu1_di0_pre_sel" , base +0x34 ,6 ,3 , ipu_di_pre_sels,ARRAY_SIZE (ipu_di_pre_sels),CLK_SET_RATE_PARENT ); clk[IMX6QDL_CLK_IPU1_DI1_PRE_SEL ] =imx_clk_mux_flags ("ipu1_di1_pre_sel" , base +0x34 ,15 ,3 , ipu_di_pre_sels,ARRAY_SIZE (ipu_di_pre_sels),CLK_SET_RATE_PARENT ); clk[IMX6QDL_CLK_IPU2_DI0_PRE_SEL ] =imx_clk_mux_flags ("ipu2_di0_pre_sel" , base +0x38 ,6 ,3 , ipu_di_pre_sels,ARRAY_SIZE (ipu_di_pre_sels),CLK_SET_RATE_PARENT );
@@ -491,12 +491,6 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)clk_register_clkdev (clk[IMX6QDL_CLK_ENET_REF ],"enet_ref" ,NULL ); -if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || -cpu_is_imx6dl ()) { -clk_set_parent (clk[IMX6QDL_CLK_LDB_DI0_SEL ], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV ]); -clk_set_parent (clk[IMX6QDL_CLK_LDB_DI1_SEL ], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV ]); - } -clk_set_parent (clk[IMX6QDL_CLK_IPU1_DI0_PRE_SEL ], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV ]);clk_set_parent (clk[IMX6QDL_CLK_IPU1_DI1_PRE_SEL ], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV ]);clk_set_parent (clk[IMX6QDL_CLK_IPU2_DI0_PRE_SEL ], clk[IMX6QDL_CLK_PLL5_VIDEO_DIV ]);
arch/arm/mach-imx/clk.h +8 -0
@@ -124,6 +124,14 @@ static inline struct clk *imx_clk_mux_flags(const char *name, &imx_ccm_lock); } +static inline struct clk *imx_clk_mux_ldb (const char *name,void __iomem *reg, +u8 shift,u8 width,const char **parents,int num_parents) +{ +return clk_register_mux (NULL , name, parents, num_parents, +CLK_SET_RATE_NO_REPARENT |CLK_SET_RATE_PARENT , reg, + shift, width,CLK_MUX_READ_ONLY , &imx_ccm_lock); +} +static inline struct clk *imx_clk_fixed_factor (const char *name,const char *parent,unsigned int mult,unsigned int div) {
Signature · 1 line
2.1.4
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
In reply to Philipp ZabelFrom: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the ldb_di_ipu_div divider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when the ipu_di_clk is sourced from ldb_di_clk.
To fix the problem, both the new and current parent of the ldb_di_clk should be disabled before the switch. This patch ensures that correct steps are followed when ldb_di_clk parent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected using the assigned-clocks and assigned-clock-parents properties of the ccm device tree node:
&clks {
assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
<&clks IMX6QDL_CLK_LDB_DI1_SEL>;
assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>,
<&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>;
};--- arch/arm/mach-imx/clk-imx6q.c | 201 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+)
arch/arm/mach-imx/clk-imx6q.c +201 -0
@@ -120,9 +120,80 @@ static unsigned int share_count_ssi1;static unsigned int share_count_ssi2;static unsigned int share_count_ssi3; +static int ldb_di_sel_by_clock_id (int clock_id) +{ +switch (clock_id) { +case IMX6QDL_CLK_PLL5_VIDEO_DIV : +return 0 ; +case IMX6QDL_CLK_PLL2_PFD0_352M : +return 1 ; +case IMX6QDL_CLK_PLL2_PFD2_396M : +return 2 ; +case IMX6QDL_CLK_MMDC_CH1_AXI : +return 3 ; +case IMX6QDL_CLK_PLL3_USB_OTG : +return 4 ; +default : +return -ENOENT ; + } +} + +static void of_assigned_ldb_sels (struct device_node *node, +int *ldb_di0_sel,int *ldb_di1_sel) +{ +struct of_phandle_args clkspec; +int index, rc, num_parents; +int parent, child, sel; + + num_parents =of_count_phandle_with_args (node,"assigned-clock-parents" , +"#clock-cells" ); +for (index =0 ; index < num_parents; index++) { + rc =of_parse_phandle_with_args (node,"assigned-clock-parents" , +"#clock-cells" , index, &clkspec); +if (rc <0 ) { +/* skip empty (null) phandles */ +if (rc == -ENOENT ) +continue ; +else +return ; + } +if (clkspec.np != node || clkspec.args[0 ] >=IMX6QDL_CLK_END ) { +pr_err ("ccm: parent clock %d not in ccm\n" , index); +return ; + } + parent = clkspec.args[0 ]; + + rc =of_parse_phandle_with_args (node,"assigned-clocks" , +"#clock-cells" , index, &clkspec); +if (rc <0 ) +return ; +if (clkspec.np != node || clkspec.args[0 ] >=IMX6QDL_CLK_END ) { +pr_err ("ccm: child clock %d not in ccm\n" , index); +return ; + } + child = clkspec.args[0 ]; + +if (child !=IMX6QDL_CLK_LDB_DI0_SEL && + child !=IMX6QDL_CLK_LDB_DI1_SEL ) +continue ; + + sel =ldb_di_sel_by_clock_id (parent); +if (sel <0 ) +pr_err ("ccm: invalid ldb_di parent clock\n" ); + +if (child ==IMX6QDL_CLK_LDB_DI0_SEL ) + *ldb_di0_sel = sel; +if (child ==IMX6QDL_CLK_LDB_DI1_SEL ) + *ldb_di1_sel = sel; + } +} +#define CCM_CCDR 0x04 +#define CCM_CCSR 0x0c +#define CCM_CS2CDR 0x2c#define CCDR_MMDC_CH1_MASK BIT(16) +#define CCSR_PLL3_SW_CLK_SEL BIT(0)static void __init imx6q_mmdc_ch1_mask_handshake (void __iomem *ccm_base) {
@@ -133,6 +204,124 @@ static void __init imx6q_mmdc_ch1_mask_handshake(void __iomem *ccm_base)writel_relaxed (reg, ccm_base +CCM_CCDR ); } +/* +* The only way to disable the MMDC_CH1 clock is to move it to pll3_sw_clk +* via periph2_clk2_sel and then to disable pll3_sw_clk by selecting the +* bypass clock source, since there is no CG bit for mmdc_ch1. +*/ +static void mmdc_ch1_disable (void __iomem *ccm_base) +{ +unsigned int reg; + +clk_set_parent (clk[IMX6QDL_CLK_PERIPH2_CLK2_SEL ], + clk[IMX6QDL_CLK_PLL3_USB_OTG ]); + +/* +* Handshake with mmdc_ch1 module must be masked when changing +* periph2_clk_sel. +*/ +clk_set_parent (clk[IMX6QDL_CLK_PERIPH2 ], clk[IMX6QDL_CLK_PERIPH2_CLK2 ]); + +/* Disable pll3_sw_clk by selecting the bypass clock source */ + reg =readl_relaxed (ccm_base +CCM_CCSR ); + reg |=CCSR_PLL3_SW_CLK_SEL ; +writel_relaxed (reg, ccm_base +CCM_CCSR ); +} + +static void mmdc_ch1_reenable (void __iomem *ccm_base) +{ +unsigned int reg; + +/* Enable pll3_sw_clk by disabling the bypass */ + reg =readl_relaxed (ccm_base +CCM_CCSR ); + reg &= ~CCSR_PLL3_SW_CLK_SEL ; +writel_relaxed (reg, ccm_base +CCM_CCSR ); + +clk_set_parent (clk[IMX6QDL_CLK_PERIPH2 ], clk[IMX6QDL_CLK_PERIPH2_PRE ]); +} + +/* +* Need to follow a strict procedure when changing the LDB +* clock, else we can introduce a glitch. Things to keep in +* mind: +* 1. The current and new parent clocks must be disabled. +* 2. The default clock for ldb_dio_clk is mmdc_ch1 which has +* no CG bit. +* 3. In the RTL implementation of the LDB_DI_CLK_SEL mux +* the top four options are in one mux and the PLL3 option along +* with another option is in the second mux. There is third mux +* used to decide between the first and second mux. +* The code below switches the parent to the bottom mux first +* and then manipulates the top mux. This ensures that no glitch +* will enter the divider. +*/ +static void init_ldb_clks (struct device_node *np,void __iomem *ccm_base) +{ +unsigned int reg; +int ldb_di0_sel[4 ] = {0 }; +int ldb_di1_sel[4 ] = {0 }; +int i; + + reg =readl_relaxed (ccm_base +CCM_CS2CDR ); + ldb_di0_sel[0 ] = (reg >>9 ) &7 ; + ldb_di1_sel[0 ] = (reg >>12 ) &7 ; + +of_assigned_ldb_sels (np, &ldb_di0_sel[3 ], &ldb_di1_sel[3 ]); + +if (ldb_di0_sel[0 ] == ldb_di0_sel[3 ] && + ldb_di1_sel[0 ] == ldb_di1_sel[3 ]) +return ; + +if (ldb_di0_sel[0 ] !=3 || ldb_di1_sel[0 ] !=3 ) +pr_warn ("ccm: ldb_di_sel already changed from reset value\n" ); + +if (ldb_di0_sel[0 ] >3 || ldb_di1_sel[0 ] >3 || + ldb_di0_sel[3 ] >3 || ldb_di1_sel[3 ] >3 ) { +pr_err ("ccm: ldb_di_sel workaround only for top mux\n" ); +return ; + } + + ldb_di0_sel[1 ] = ldb_di0_sel[0 ] |4 ; + ldb_di0_sel[2 ] = ldb_di0_sel[3 ] |4 ; + ldb_di1_sel[1 ] = ldb_di1_sel[0 ] |4 ; + ldb_di1_sel[2 ] = ldb_di1_sel[3 ] |4 ; + +mmdc_ch1_disable (ccm_base); + +for (i =1 ; i <4 ; i++) { + reg =readl_relaxed (ccm_base +CCM_CS2CDR ); + reg &= ~((7 <<9 ) | (7 <<12 )); + reg |= ((ldb_di0_sel[i] <<9 ) | (ldb_di1_sel[i] <<12 )); +writel_relaxed (reg, ccm_base +CCM_CS2CDR ); + } + +mmdc_ch1_reenable (ccm_base); +} + +static void disable_anatop_clocks (void __iomem *anatop_base) +{ +unsigned int reg; + +/* Make sure PFDs are disabled@boot. */ + reg =readl_relaxed (anatop_base +0x100 ); +/* Cannot gate PFD2 if pll2_pfd2_396m is the parent of MMDC clock */ +if (clk_get_parent (clk[IMX6QDL_CLK_PERIPH_PRE ]) == + clk[IMX6QDL_CLK_PLL2_PFD2_396M ]) + reg |=0x00008080 ; +else + reg |=0x00808080 ; +writel_relaxed (reg, anatop_base +0x100 ); + + reg =readl_relaxed (anatop_base +0xf0 ); + reg |=0x80808080 ; +writel_relaxed (reg, anatop_base +0xf0 ); + +/* Make sure PLLs is disabled */ + reg =readl_relaxed (anatop_base +0xa0 ); + reg &= ~(1 <<13 ); +writel_relaxed (reg, anatop_base +0xa0 ); +} +static void __init imx6q_clocks_init (struct device_node *ccm_node) {struct device_node *np;
@@ -269,6 +458,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk[IMX6QDL_CLK_PLL5_POST_DIV ] =clk_register_divider_table (NULL ,"pll5_post_div" ,"pll5_video" ,CLK_SET_RATE_PARENT , base +0xa0 ,19 ,2 ,0 , post_div_table, &imx_ccm_lock); clk[IMX6QDL_CLK_PLL5_VIDEO_DIV ] =clk_register_divider_table (NULL ,"pll5_video_div" ,"pll5_post_div" ,CLK_SET_RATE_PARENT , base +0x170 ,30 ,2 ,0 , video_div_table, &imx_ccm_lock); +disable_anatop_clocks (base); + np = ccm_node; base =of_iomap (np,0 );WARN_ON (!base);
@@ -296,6 +487,16 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node) clk[IMX6QDL_CLK_GPU3D_SHADER_SEL ] =imx_clk_mux ("gpu3d_shader_sel" , base +0x18 ,8 ,2 , gpu3d_shader_sels,ARRAY_SIZE (gpu3d_shader_sels)); clk[IMX6QDL_CLK_IPU1_SEL ] =imx_clk_mux ("ipu1_sel" , base +0x3c ,9 ,2 , ipu_sels,ARRAY_SIZE (ipu_sels)); clk[IMX6QDL_CLK_IPU2_SEL ] =imx_clk_mux ("ipu2_sel" , base +0x3c ,14 ,2 , ipu_sels,ARRAY_SIZE (ipu_sels)); + +/* +* The LDB_DI0/1_SEL muxes are registered read-only due to a hardware +* bug. Set the muxes to the requested values before registering the +* ldb_di_sel clocks. +*/ +if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || +cpu_is_imx6dl ()) +init_ldb_clks (np, base); + clk[IMX6QDL_CLK_LDB_DI0_SEL ] =imx_clk_mux_ldb ("ldb_di0_sel" , base +0x2c ,9 ,3 , ldb_di_sels,ARRAY_SIZE (ldb_di_sels)); clk[IMX6QDL_CLK_LDB_DI1_SEL ] =imx_clk_mux_ldb ("ldb_di1_sel" , base +0x2c ,12 ,3 , ldb_di_sels,ARRAY_SIZE (ldb_di_sels)); clk[IMX6QDL_CLK_IPU1_DI0_PRE_SEL ] =imx_clk_mux_flags ("ipu1_di0_pre_sel" , base +0x34 ,6 ,3 , ipu_di_pre_sels,ARRAY_SIZE (ipu_di_pre_sels),CLK_SET_RATE_PARENT );
Signature · 1 line
2.1.4
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Quoted 22 lines · Philipp Zabel
From: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the
ldb_di_ipu_divdivider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when theipu_di_clkis sourced fromldb_di_clk.To fix the problem, both the new and current parent of the
ldb_di_clkshould be disabled before the switch. This patch ensures that correct steps are followed whenldb_di_clkparent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected using the assigned-clocks and assigned-clock-parents properties of the ccm device tree node:&clks { assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>; assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>, <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>; };
Quoted 3 lines
Signed-off-by Ranjani Vaidyanathan <[email protected]>Signed-off-by Fabio Estevam <[email protected]>Signed-off-by Philipp Zabel <[email protected]>
Before I start reviewing the patch, I would like to know if there is a formal errata for this issue, including the bug description, recommended workaround, and affected i.MX6 variants etc.
Shawn
Fabio Estevam
reviewerPATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Hi Shawn,
Quoted 3 lines · Shawn Guo
Before I start reviewing the patch, I would like to know if there is a formal errata for this issue, including the bug description, recommended workaround, and affected i.MX6 variants etc.
The latest mx6 reference manual available on the web shows the LDB clock gate in the correct position now.
It also adds the following note:
"NOTE: This clock path is not controlled by a clock gate (CG). For proper clock switching procedures refer to the Clock Switching Multiplexers section."
There is no formal errata available.
Please consider reviewing this series, as it addresses a real problem that affect many users.
Thanks,
Fabio Estevam
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Quoted 13 lines · Fabio Estevam
Hi Shawn,
Quoted 3 lines · Shawn Guo
Before I start reviewing the patch, I would like to know if there is a formal errata for this issue, including the bug description, recommended workaround, and affected i.MX6 variants etc.
The latest mx6 reference manual available on the web shows the LDB clock gate in the correct position now.
It also adds the following note:
Quoted 8 lines
"NOTE: This clock path is not controlled by a clock gate (CG). For proper clock switching procedures refer to the Clock Switching Multiplexers section."
There is no formal errata available.
Please consider reviewing this series, as it addresses a real problem that affect many users.
Just to get a better understanding for the background of this series:
Are the patches in this series somehow related to Shawn's ENGR00318063-x patches in the FSL 3.10.53_1.1.0_ga kernel:
?
Or are this completely different topics?
Best regards
Dirk
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Quoted 24 lines · Dirk Behme
Quoted 13 lines · Fabio Estevam
Hi Shawn,
Quoted 3 lines · Shawn Guo
Before I start reviewing the patch, I would like to know if there is a formal errata for this issue, including the bug description, recommended workaround, and affected i.MX6 variants etc.
The latest mx6 reference manual available on the web shows the LDB clock gate in the correct position now.
It also adds the following note:
Quoted 8 lines
"NOTE: This clock path is not controlled by a clock gate (CG). For proper clock switching procedures refer to the Clock Switching Multiplexers section."
There is no formal errata available.
Please consider reviewing this series, as it addresses a real problem that affect many users.
Quoted 7 lines
Just to get a better understanding for the background of this
series: Are the patches in this series somehow related to Shawn's ENGR00318063-x patches in the FSL 3.10.53_1.1.0_ga kernel:?
Fabio,
Look, without the errata for this problem, your customer do not even know what this patch is for.
Dirk,
Yes, it tries to fix the buggy ldb_di_ipu clock output which doesn't have a gate between ldb_di_clk_sel and div_3.5_7. That's also part of the issues that ENGR00318063-x patches try to address.
Shawn
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
In reply to Fabio EstevamQuoted 8 lines · Fabio Estevam
Hi Shawn,
Quoted 3 lines · Shawn Guo
Before I start reviewing the patch, I would like to know if there is a formal errata for this issue, including the bug description, recommended workaround, and affected i.MX6 variants etc.
Quoted 8 lines
The latest mx6 reference manual available on the web shows the LDB clock gate in the correct position now.
It also adds the following note:
"NOTE: This clock path is not controlled by a clock gate (CG). For proper clock switching procedures refer to the Clock Switching Multiplexers section."
No. The Clock Switching Multiplexers section documents the procedure for the glitchy multiplexer with a clock gate in the right place, not this buggy LDB multiplexer.
Quoted 5 lines
There is no formal errata available.
Please consider reviewing this series, as it addresses a real problem that affect many users.
I'm not going to review/take the patch until I see a formal errata documenting the software workaround procedure for the issue.
Shawn
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
In reply to Philipp ZabelQuoted 33 lines · Philipp Zabel
From: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the
ldb_di_ipu_divdivider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when theipu_di_clkis sourced fromldb_di_clk.To fix the problem, both the new and current parent of the
ldb_di_clkshould be disabled before the switch. This patch ensures that correct steps are followed whenldb_di_clkparent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected using the assigned-clocks and assigned-clock-parents properties of the ccm device tree node:&clks { assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>; assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>, <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>; };Signed-off-by Ranjani Vaidyanathan <[email protected]>Signed-off-by Fabio Estevam <[email protected]>Signed-off-by Philipp Zabel <[email protected]>--- arch/arm/mach-imx/clk-imx6q.c | 201 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+)arch/arm/mach-imx/clk-imx6q.c +0
-0
...
arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 498
Quoted 8 lines
arch/arm/mach-imx/clk-imx6q.c +8
-0+/* +* The LDB_DI0/1_SEL muxes are registered read-only due to a hardware +* bug. Set the muxes to the requested values before registering the +* ldb_di_sel clocks. +*/ +if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || +cpu_is_imx6dl ()) +init_ldb_clks (np, base);
I've a hopefully simple understanding question regarding this if():
What does happen (or: has to be done) in the (non-existing) else case of this if() statement?
E.g. how to configure the clocks for the imx_get_soc_revision() == IMX_CHIP_REVISION_1_0 case?
For the if() case, we can configure the clocks via the assigned-clocks and assigned-clock-parents as described in the commit message. But how to configure the clocks/muxes in the else case? To my understanding, with the patch 2/3 [1] the clocks/muxes are configured to read only.
Sorry if I missed anything ;)
Many thanks,
Dirk
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313619.html
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Quoted 77 lines · Dirk Behme
Quoted 40 lines · Philipp Zabel
From: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the
ldb_di_ipu_divdivider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when theipu_di_clkis sourced fromldb_di_clk.To fix the problem, both the new and current parent of the
ldb_di_clkshould be disabled before the switch. This patch ensures that correct steps are followed whenldb_di_clkparent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected using the assigned-clocks and assigned-clock-parents properties of the ccm>
device tree node:&clks { assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>; assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>, <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>; };Signed-off-by Ranjani Vaidyanathan <[email protected]>Signed-off-by Fabio Estevam <[email protected]>Signed-off-by Philipp Zabel <[email protected]>---
arch/arm/mach-imx/clk-imx6q.c | 201++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+)
arch/arm/mach-imx/clk-imx6q.c +0
-0...
arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 498
Quoted 8 lines
arch/arm/mach-imx/clk-imx6q.c +8
-0+/* +* The LDB_DI0/1_SEL muxes are registered read-only due to a hardware +* bug. Set the muxes to the requested values before registering the +* ldb_di_sel clocks. +*/ +if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || +cpu_is_imx6dl ()) +init_ldb_clks (np, base);I've a hopefully simple understanding question regarding this
if():What does happen (or: has to be done) in the (non-existing) else case of this
if()statement?E.g. how to configure the clocks for the
imx_get_soc_revision()==IMX_CHIP_REVISION_1_0case?For the
if()case, we can configure the clocks via the assigned-clocks and assigned-clock-parents as described in the commit message. But how to configure the clocks/muxes in the else case? To my understanding, with the patch 2/3 [1] the clocks/muxes are configured to read only.Sorry if I missed anything ;)
Many thanks,
Dirk
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313619.ht
ml
Quoted 4 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I have at least one imx6q SoM with IMX_CHIP_REVISION_1_0, and can confirm that with the current mainline kernel, it gets no lvds video, or rather very little.
I see penguins, and then it goes black.
with the 3.0.35 vendor kernel it was working.
My only clue so far has been that there are a couple of clocks and regulators that come up different on the newer chip.
vddarm and vddpu are 1100 mV on 1.0, 1150 mV on 1.5
and there is a message that looks like so on 1.0 chip:
Switching to timer-based delay loop, resolution 15ns and like so on 1.5 (aka 1.3d):
Switching to timer-based delay loop, resolution 333ns
Hope that helps.
Signature · 7 lines
Joshua Clayton Software Engineer UniWest 122 S. 4th Avenue Pasco, WA 99301 Ph: (509) 544-0720 Fx: (509) 544-0868
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Quoted 76 lines · Joshua Clayton
Quoted 75 lines · Dirk Behme
Quoted 40 lines · Philipp Zabel
From: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter the
ldb_di_ipu_divdivider. If the divider gets locked up, no ldb_di[x]_clk is generated, and the LVDS display will hang when theipu_di_clkis sourced fromldb_di_clk.To fix the problem, both the new and current parent of the
ldb_di_clkshould be disabled before the switch. This patch ensures that correct steps are followed whenldb_di_clkparent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected using the assigned-clocks and assigned-clock-parents properties of the ccm>
device tree node:&clks { assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>; assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>, <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>; };Signed-off-by Ranjani Vaidyanathan <[email protected]>Signed-off-by Fabio Estevam <[email protected]>Signed-off-by Philipp Zabel <[email protected]>---
arch/arm/mach-imx/clk-imx6q.c | 201++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+)
arch/arm/mach-imx/clk-imx6q.c +0
-0...
arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 498
Quoted 8 lines
arch/arm/mach-imx/clk-imx6q.c +8
-0+/* +* The LDB_DI0/1_SEL muxes are registered read-only due to a hardware +* bug. Set the muxes to the requested values before registering the +* ldb_di_sel clocks. +*/ +if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || +cpu_is_imx6dl ()) +init_ldb_clks (np, base);I've a hopefully simple understanding question regarding this
if():What does happen (or: has to be done) in the (non-existing) else case of this
if()statement?E.g. how to configure the clocks for the
imx_get_soc_revision()==IMX_CHIP_REVISION_1_0case?For the
if()case, we can configure the clocks via the assigned-clocks and assigned-clock-parents as described in the commit message. But how to configure the clocks/muxes in the else case? To my understanding, with the patch 2/3 [1] the clocks/muxes are configured to read only.Sorry if I missed anything ;)
Many thanks,
Dirk
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313619.ht
ml
....
Quoted 2 lines
I have at least one imx6q SoM with
IMX_CHIP_REVISION_1_0, and can confirm that with the current mainline kernel, it gets no lvds video,
Just to avoid confusion: I don't think the patch we are talking about here is already in current mainline:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/arch/arm/mach-imx/clk-imx6q.c
I.e. the issue you describe is an other topic than the one I asked for.
Best regards
Dirk
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
In reply to Dirk BehmeQuoted 62 lines · Dirk Behme
Quoted 41 lines · Philipp Zabel
From: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree,
the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter theldb_di_ipu_divdivider. If the divider gets locked up, no ldb_di[x]_clk is
generated, and the LVDS display will hang when theipu_di_clkis sourced fromldb_di_clk.To fix the problem, both the new and current parent of the
ldb_di_clkshould
be disabled before the switch. This patch ensures that correct steps are followed whenldb_di_clkparent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected
using the assigned-clocks and assigned-clock-parents properties of the ccm
device tree node:&clks { assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>; assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>, <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>; };Signed-off-by Ranjani Vaidyanathan <[email protected]>Signed-off-by Fabio Estevam <[email protected]>Signed-off-by Philipp Zabel <[email protected]>--- arch/arm/mach-imx/clk-imx6q.c | 201 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+)arch/arm/mach-imx/clk-imx6q.c +0
-0...
arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 498
Quoted 9 lines
arch/arm/mach-imx/clk-imx6q.c +2
-0+/* +* The LDB_DI0/1_SEL muxes are registered read-only due to a hardware
arch/arm/mach-imx/clk-imx6q.c +6
-0+* bug. Set the muxes to the requested values before registering the +* ldb_di_sel clocks. +*/ +if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || +cpu_is_imx6dl ()) +init_ldb_clks (np, base);I've a hopefully simple understanding question regarding this
if():What does happen (or: has to be done) in the (non-existing) else case of this
if()statement?E.g. how to configure the clocks for the
imx_get_soc_revision()==IMX_CHIP_REVISION_1_0case?
Quoted 6 lines
For the
if()case, we can configure the clocks via the assigned-clocks and assigned-clock-parents as described in the commit message. But how to configure the clocks/muxes in the else case? To my understanding, with the patch 2/3 [1] the clocks/muxes are configured to read only.Sorry if I missed anything ;)
Any ideas regarding this?
Many thanks
Dirk
PATCH 3/3 ARM: imx6: Fix procedure to switch the parent of LDB_DI_CLK
Hi Dirk,
Am Donnerstag, den 26.02.2015, 07:32 +0100 schrieb Dirk Behme:
Quoted 63 lines · Dirk Behme
Quoted 62 lines · Philipp Zabel
Quoted 41 lines
From: Fabio Estevam <[email protected]>
Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk tree,
the glitchy parent mux of ldb_di[x]_clk can cause a glitch to enter theldb_di_ipu_divdivider. If the divider gets locked up, no ldb_di[x]_clk is
generated, and the LVDS display will hang when theipu_di_clkis sourced fromldb_di_clk.To fix the problem, both the new and current parent of the
ldb_di_clkshould
be disabled before the switch. This patch ensures that correct steps are followed whenldb_di_clkparent is switched in the beginning of boot. The glitchy muxes are then registered as read-only. The clock parent can be selected
using the assigned-clocks and assigned-clock-parents properties of the ccm
device tree node:&clks { assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>, <&clks IMX6QDL_CLK_LDB_DI1_SEL>; assigned-clock-parents = <&clks IMX6QDL_CLK_MMDC_CH1_AXI>, <&clks IMX6QDL_CLK_PLL5_VIDEO_DIV>; };Signed-off-by Ranjani Vaidyanathan <[email protected]>Signed-off-by Fabio Estevam <[email protected]>Signed-off-by Philipp Zabel <[email protected]>--- arch/arm/mach-imx/clk-imx6q.c | 201 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+)arch/arm/mach-imx/clk-imx6q.c +0
-0...
arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 498
Quoted 9 lines
arch/arm/mach-imx/clk-imx6q.c +2
-0+/* +* The LDB_DI0/1_SEL muxes are registered read-only due to a hardware
arch/arm/mach-imx/clk-imx6q.c +6
-0+* bug. Set the muxes to the requested values before registering the +* ldb_di_sel clocks. +*/ +if ((imx_get_soc_revision () !=IMX_CHIP_REVISION_1_0 ) || +cpu_is_imx6dl ()) +init_ldb_clks (np, base);I've a hopefully simple understanding question regarding this
if():What does happen (or: has to be done) in the (non-existing) else case of this
if()statement?E.g. how to configure the clocks for the
imx_get_soc_revision()==IMX_CHIP_REVISION_1_0case?
Quoted 8 lines
Quoted 6 lines
For the
if()case, we can configure the clocks via the assigned-clocks and assigned-clock-parents as described in the commit message. But how to configure the clocks/muxes in the else case? To my understanding, with the patch 2/3 [1] the clocks/muxes are configured to read only.Sorry if I missed anything ;)
Any ideas regarding this?
I guess for the ((imx_get_soc_revision() == IMX_CHIP_REVISION_1_0) && ! cpu_is_imx6dl()) case we could still call init_ldb_clks and change ldb_di_sel_clock_id to return -ENOENT for IMX6QDL_CLK_PLL5_VIDEO_DEV. And the default should be changed.
regards
Philipp
Fabio Estevam
reviewerPATCH 1/3 ARM: imx6: Mask mmdc_ch1 handshake for periph2_sel and mmdc_ch1_axi_podf
In reply to Philipp ZabelHi Philipp,
Quoted 6 lines · Philipp Zabel
MMDC CH1 is not used on i.MX6Q, so the handshake needed to change the parent of
periph2_selor the divider ofmmdc_ch1_axi_podfwill never succeed. Disable the handshake mechanism to allow changing the frequency ofmmdc_ch1_axi, allowing to use it as a possible source for the LDB DI clock.Signed-off-by Philipp Zabel <[email protected]>
Thanks for working on this series. It looks great.
I am adding Christian on Cc as he could reproduce the bug quickly on his setup. Maybe he could give his Tested-by?
On my board it takes a long time to reproduce the ldb clock lock-up issue.
Thanks
PATCH 1/3 ARM: imx6: Mask mmdc_ch1 handshake for periph2_sel and mmdc_ch1_axi_podf
Hi all
Quoted 6 lines · Philipp Zabel
Quoted 5 lines
MMDC CH1 is not used on i.MX6Q, so the handshake needed to change the parent of
periph2_selor the divider ofmmdc_ch1_axi_podfwill never succeed. Disable the handshake mechanism to allow changing the frequency ofmmdc_ch1_axi, allowing to use it as a possible source for the LDB DI clock.
Quoted 7 lines
Quoted 1 line
Signed-off-by Philipp Zabel <[email protected]>Thanks for working on this series. It looks great.
I am adding Christian on Cc as he could reproduce the bug quickly on his setup. Maybe he could give his Tested-by?
I will have a look at it as I am able to reproduce this problem in less then 10 minutes. I am not
sure if I find time during this week, but I try my best.
greets
--
Christian Gmeiner, MSc