[U-Boot] [PATCH] video: ipuv3_fb: skip IPU shutdown if IPU was not enabled before

[U-Boot] [PATCH] video: ipuv3_fb: skip IPU shutdown if IPU was not enabled before

[Anatolij Gustschin]

Boards can skip display interface init using board_video_skip().
If display interface was not initialized (e.g. no ipuv3 framebuffer
registered or IPU clock disabled), booting Linux stops due to the
crash in IPU shutdown function, when accessing IPU registers.
Check IPU clock and skip shutdown if clock is not enabled.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/video/mxc_ipuv3_fb.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/video/mxc_ipuv3_fb.c b/drivers/video/mxc_ipuv3_fb.c
index 0d0a0a9..8836229 100644
--- a/drivers/video/mxc_ipuv3_fb.c
+++ b/drivers/video/mxc_ipuv3_fb.c
@@ -13,6 +13,7 @@
 
 #include <common.h>
 #include <linux/errno.h>
+#include <asm/arch/crm_regs.h>
 #include <asm/global_data.h>
 #include <linux/string.h>
 #include <linux/list.h>
@@ -568,8 +569,18 @@ err0:
 
 void ipuv3_fb_shutdown(void)
 {
-	int i;
+	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
 	struct ipu_stat *stat = (struct ipu_stat *)IPU_STAT;
+	u32 reg;
+	int i;
+
+	/*
+	 * Check if IPU clock was enabled before. Won't access
+	 * IPU registers if clock is not enabled.
+	 */
+	reg = readl(&mxc_ccm->CCGR3);
+	if ((reg & MXC_CCM_CCGR3_IPU1_IPU_MASK) == 0)
+		return;
 
 	for (i = 0; i < ARRAY_SIZE(mxcfb_info); i++) {
 		struct fb_info *fbi = mxcfb_info[i];
-- 
2.7.4

[U-Boot] [PATCH] video: ipuv3_fb: skip IPU shutdown if IPU was not enabled before

[Fabio Estevam]

Hi Anatolij,

On Fri, Aug 25, 2017 at 10:10 AM, Anatolij Gustschin <agust@denx.de> wrote:

>  void ipuv3_fb_shutdown(void)
>  {
> -       int i;
> +       struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
>         struct ipu_stat *stat = (struct ipu_stat *)IPU_STAT;
> +       u32 reg;
> +       int i;
> +
> +       /*
> +        * Check if IPU clock was enabled before. Won't access
> +        * IPU registers if clock is not enabled.
> +        */
> +       reg = readl(&mxc_ccm->CCGR3);
> +       if ((reg & MXC_CCM_CCGR3_IPU1_IPU_MASK) == 0)
> +               return;

Maybe you could also check whether IPU2 has been enabled?

[U-Boot] [PATCH] video: ipuv3_fb: skip IPU shutdown if IPU was not enabled before

[Stefano Babic]

Hi Anatolji,

On 25/08/2017 15:10, Anatolij Gustschin wrote:
> Boards can skip display interface init using board_video_skip().
> If display interface was not initialized (e.g. no ipuv3 framebuffer
> registered or IPU clock disabled), booting Linux stops due to the
> crash in IPU shutdown function, when accessing IPU registers.
> Check IPU clock and skip shutdown if clock is not enabled.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  drivers/video/mxc_ipuv3_fb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/mxc_ipuv3_fb.c b/drivers/video/mxc_ipuv3_fb.c
> index 0d0a0a9..8836229 100644
> --- a/drivers/video/mxc_ipuv3_fb.c
> +++ b/drivers/video/mxc_ipuv3_fb.c
> @@ -13,6 +13,7 @@
>  
>  #include <common.h>
>  #include <linux/errno.h>
> +#include <asm/arch/crm_regs.h>
>  #include <asm/global_data.h>
>  #include <linux/string.h>
>  #include <linux/list.h>
> @@ -568,8 +569,18 @@ err0:
>  
>  void ipuv3_fb_shutdown(void)
>  {
> -	int i;
> +	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
>  	struct ipu_stat *stat = (struct ipu_stat *)IPU_STAT;
> +	u32 reg;
> +	int i;
> +
> +	/*
> +	 * Check if IPU clock was enabled before. Won't access
> +	 * IPU registers if clock is not enabled.
> +	 */
> +	reg = readl(&mxc_ccm->CCGR3);
> +	if ((reg & MXC_CCM_CCGR3_IPU1_IPU_MASK) == 0)
> +		return;
>  

It looks to me quite weak to simply check if clock is gated. On some
board clock is gated even if IPU is not active. Do you have a way
reading inside IPU itself ? Then we can better decide if it was really
acrive. For example, checking if ipu_enable_channel() was called (and
DMA registers are set).

Best regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] video: ipuv3_fb: skip IPU shutdown if IPU was not enabled before

[Anatolij Gustschin]

Hi Fabio,

On Mon, 28 Aug 2017 13:22:19 -0300
Fabio Estevam festevam at gmail.com wrote:
...
> > +       /*
> > +        * Check if IPU clock was enabled before. Won't access
> > +        * IPU registers if clock is not enabled.
> > +        */
> > +       reg = readl(&mxc_ccm->CCGR3);
> > +       if ((reg & MXC_CCM_CCGR3_IPU1_IPU_MASK) == 0)
> > +               return;  
> 
> Maybe you could also check whether IPU2 has been enabled?

currently IPU2 is not used, the driver never enables it. I checked
it when preparing this patch.

Thanks,
Anatolij

[U-Boot] [PATCH] video: ipuv3_fb: skip IPU shutdown if IPU was not enabled before

[Anatolij Gustschin]

Hi Stefano,

On Mon, 28 Aug 2017 18:54:39 +0200
Stefano Babic sbabic at denx.de wrote:
...
> > +	/*
> > +	 * Check if IPU clock was enabled before. Won't access
> > +	 * IPU registers if clock is not enabled.
> > +	 */
> > +	reg = readl(&mxc_ccm->CCGR3);
> > +	if ((reg & MXC_CCM_CCGR3_IPU1_IPU_MASK) == 0)
> > +		return;
> >    
> 
> It looks to me quite weak to simply check if clock is gated. On some
> board clock is gated even if IPU is not active. Do you have a way
> reading inside IPU itself ? Then we can better decide if it was really
> acrive. For example, checking if ipu_enable_channel() was called (and
> DMA registers are set).

we cannot access IPU registers when IPU clock is disabled (register
access will hang the CPU).

Thanks,
Anatolij