[U-Boot] [PATCH v3] socfpga: Adding Freeze Controller driver

[Chin Liang See]

On Mon, 2013-09-23 at 13:59 -0500, Dinh Nguyen wrote:
> On Fri, 2013-09-20 at 00:08 -0500, Chin Liang See wrote:

> > diff --git a/arch/arm/cpu/armv7/socfpga/freeze_controller.c b/arch/arm/cpu/armv7/socfpga/freeze_controller.c
> > new file mode 100644
> > index 0000000..93ad22a
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv7/socfpga/freeze_controller.c
> > @@ -0,0 +1,242 @@
> > +/*
> > + *  Copyright (C) 2013 Altera Corporation <www.altera.com>
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +
> 
> Remove extra line here...

Removed

> 
> > +#include <common.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/freeze_controller.h>
> > +#include <asm/arch/timer.h>
> > +#include <asm/errno.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +
> > +
> Remove extra lines here...

Removed

> 
> > +static const struct socfpga_freeze_controller *freeze_controller_base =
> > +		(void *)(SOCFPGA_SYSMGR_ADDRESS + SYSMGR_FRZCTRL_ADDRESS);
> > +
> > +/*
> > + * Default state from cold reset is FREEZE_ALL; the global
> > + * flag is set to TRUE to indicate the IO banks are frozen
> > + */
> > +static uint32_t frzctrl_channel_freeze[FREEZE_CHANNEL_NUM]
> > +	= { FREEZE_CTRL_FROZEN, FREEZE_CTRL_FROZEN,
> > +	FREEZE_CTRL_FROZEN, FREEZE_CTRL_FROZEN};
> > +
> > +
> 
> Ditto...

Removed

> 
> > +/* Freeze HPS IOs */
> > +u32 sys_mgr_frzctrl_freeze_req(u32 channel_id)
> > +{
> > +	u32 ioctrl_reg_offset;
> > +	u32 reg_value;
> > +	u32 reg_cfg_mask;
> > +
> > +	/* select software FSM */
> > +	writel(SYSMGR_FRZCTRL_SRC_VIO1_ENUM_SW,	&freeze_controller_base->src);
> > +
> > +	/* Freeze channel ID checking and base address */
> > +	switch (channel_id) {
> > +	case 0:
> > +	case 1:
> > +	case 2:
> > +		ioctrl_reg_offset = (u32)(
> > +			&freeze_controller_base->vioctrl +
> > +			(channel_id << SYSMGR_FRZCTRL_VIOCTRL_SHIFT));
> > +
> > +		/*
> > +		 * Assert active low enrnsl, plniotri
> > +		 * and niotri signals
> > +		 */
> > +		reg_cfg_mask =
> > +			SYSMGR_FRZCTRL_VIOCTRL_SLEW_MASK
> > +			| SYSMGR_FRZCTRL_VIOCTRL_WKPULLUP_MASK
> > +			| SYSMGR_FRZCTRL_VIOCTRL_TRISTATE_MASK;
> > +		clrbits_le32(ioctrl_reg_offset,	reg_cfg_mask);
> > +
> > +		/*
> > +		 * Note: Delay for 20ns at min
> > +		 * Assert active low bhniotri signal and de-assert
> > +		 * active high csrdone
> > +		 */
> 
> Where's the delay?

Actually single instruction would cause more than 20ns. With that, we
won't need additional delay here. Removed the delay command here for
avoiding confusion

> 
> > +		reg_cfg_mask
> > +			= SYSMGR_FRZCTRL_VIOCTRL_BUSHOLD_MASK
> > +			| SYSMGR_FRZCTRL_VIOCTRL_CFG_MASK;
> > +		clrbits_le32(ioctrl_reg_offset,	reg_cfg_mask);
> > +
> > +		/* Set global flag to indicate channel is frozen */
> > +		frzctrl_channel_freeze[channel_id] = FREEZE_CTRL_FROZEN;
> > +		break;
> > +
> > +	case 3:
> > +		/*
> > +		 * Assert active low enrnsl, plniotri and
> > +		 * niotri signals
> > +		 */
> > +		reg_cfg_mask
> > +			= SYSMGR_FRZCTRL_HIOCTRL_SLEW_MASK
> > +			| SYSMGR_FRZCTRL_HIOCTRL_WKPULLUP_MASK
> > +			| SYSMGR_FRZCTRL_HIOCTRL_TRISTATE_MASK;
> > +		clrbits_le32(&freeze_controller_base->hioctrl, reg_cfg_mask);
> > +
> > +		/*
> > +		 * Note: Delay for 40ns at min
> > +		 * assert active low bhniotri & nfrzdrv signals,
> > +		 * de-assert active high csrdone and assert
> > +		 * active high frzreg and nfrzdrv signals
> > +		 */
> 
> Where's the delay?

same comments as above

> 
> > +		reg_value = readl(&freeze_controller_base->hioctrl);
> > +		reg_cfg_mask
> > +			= SYSMGR_FRZCTRL_HIOCTRL_BUSHOLD_MASK
> > +			| SYSMGR_FRZCTRL_HIOCTRL_CFG_MASK;
> > +		reg_value
> > +			= (reg_value & ~reg_cfg_mask)
> > +			| SYSMGR_FRZCTRL_HIOCTRL_REGRST_MASK
> > +			| SYSMGR_FRZCTRL_HIOCTRL_OCTRST_MASK;
> > +		writel(reg_value, &freeze_controller_base->hioctrl);
> > +
> > +		/*
> > +		 * Note: Delay for 40ns at min
> > +		 * assert active high reinit signal and de-assert
> > +		 * active high pllbiasen signals
> > +		 */
> > +		reg_value = readl(&freeze_controller_base->hioctrl);
> > +		reg_value
> > +			= (reg_value &
> > +			~SYSMGR_FRZCTRL_HIOCTRL_OCT_CFGEN_CALSTART_MASK)
> > +			| SYSMGR_FRZCTRL_HIOCTRL_DLLRST_MASK;
> > +		writel(reg_value, &freeze_controller_base->hioctrl);
> > +
> > +		/* Set global flag to indicate channel is frozen */
> > +		frzctrl_channel_freeze[channel_id] = FREEZE_CTRL_FROZEN;
> > +		break;
> > +
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* Unfreeze/Thaw HPS IOs */
> > +u32 sys_mgr_frzctrl_thaw_req(u32 channel_id)
> > +{
> > +	u32 ioctrl_reg_offset;
> > +	u32 reg_cfg_mask;
> > +	u32 reg_value;
> > +
> > +
> 
> Extra line..

Removed

> 

> > diff --git a/arch/arm/cpu/armv7/socfpga/spl.c b/arch/arm/cpu/armv7/socfpga/spl.c
> > index 74bceab..e9cdfb87 100644
> > --- a/arch/arm/cpu/armv7/socfpga/spl.c
> > +++ b/arch/arm/cpu/armv7/socfpga/spl.c
> > @@ -13,6 +13,8 @@
> >  #include <asm/arch/reset_manager.h>
> >  #include <spl.h>
> >  #include <asm/arch/system_manager.h>
> > +#include <asm/arch/freeze_controller.h>
> > +
> 
> Extra line..

Removed

> >  
> >  DECLARE_GLOBAL_DATA_PTR;
> >  
> > @@ -27,6 +29,13 @@ u32 spl_boot_device(void)
> >  void spl_board_init(void)
> >  {
> >  #ifndef CONFIG_SOCFPGA_VIRTUAL_TARGET
> > +	debug("Freezing all I/O banks\n");
> > +	/* freeze all IO banks */
> > +	sys_mgr_frzctrl_freeze_req(0);
> > +	sys_mgr_frzctrl_freeze_req(1);
> > +	sys_mgr_frzctrl_freeze_req(2);
> > +	sys_mgr_frzctrl_freeze_req(3);
> 
> Are there any situation where you wouldn't freeze/unfreeze all 4 banks of IO?
> If not then, then you could simplify sys_mgr_frzctrl_freeze_req() to be only
> called once.

Actually customer might want to remain the same IOCSR setting for
particular banks which already setup for BootROM. But later seems not
much use case for this. With that, I already combine them as suggested.

Thanks

Chin Liang

> 
> Dinh