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

Chin Liang See clsee at altera.com
Thu Sep 19 17:13:52 CEST 2013


Hi Pavel,

On Thu, 2013-09-19 at 14:11 +0200, ZY - pavel wrote:
> Hi!
> 
> > Adding Freeze Controller driver. All HPS IOs need to be
> > in freeze state during pin mux or IO buffer configuration.
> > It is to avoid any glitch which might happen
> > during the configuration from propagating to external devices.
> 
> Thanks for the patch.

Thanks to you too for the speedy feedback.

> 
> (What version is it against? Aha, it depends on previous patches,
> right?)

I patched against latest release plus the last 2 patches for System
Manager & pin mux (other earlier patches already in latest release).
These patches already went through few round of reviews. I believe we
just need to wait Albert to apply them into master git :) 

> 
> AFAICT: 
> 
> FREEZE_CONTROLLER_FSM_HW is unused and can be removed.

Yup, they can removed. It was put initially as customer can use either
SW or HW state machine. But rethinking of this, most likely customer
will not change this area. So its good to remove the dead code.

> 
> frzctrl_channel_freeze[] is write-only and can be removed.

Actually they will be used in other function within this driver. This
function is yet to be upstreamed. I plan to do it together with new
driver "Scan Manager" in later days. The Scan Manager will need this
function to work together. With that, is it ok that we maintain this
first?

> 
> Local variables do not need frzctrl_ prefix.

Oops, seems overdone here... haha. I already fix it

> 
> [I have fixed those, suggested incremental diff is attached. Please
> review & apply to your tree.]

Thanks and applied

> 
> Is it really neccessary to introduce get_timer_count_masked? Replace
> the delays with udelay(1) and it should still work?

Good suggestion here. I converted them to use the delay now to avoid
creating the new timer functions. End of day, not much impact to
performance when converting clock cycles to microseconds.


I will send up the v2 in short time. Thanks again and have a fun day.

Chin Liang

> 
> Thanks,
> 								Pavel
> 
> diff --git a/arch/arm/cpu/armv7/socfpga/freeze_controller.c b/arch/arm/cpu/armv7/socfpga/freeze_controller.c
> index af612c9..058d9b5 100644
> --- a/arch/arm/cpu/armv7/socfpga/freeze_controller.c
> +++ b/arch/arm/cpu/armv7/socfpga/freeze_controller.c
> @@ -9,226 +9,183 @@
>  #include <asm/io.h>
>  #include <asm/arch/freeze_controller.h>
>  #include <asm/arch/timer.h>
> +#include <asm/errno.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +
> +
>  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};
> -
> -/* Get current state of freeze channel 1 (VIO) */
> -static inline u32 sys_mgr_frzctrl_frzchn1_state_get(void)
> -{
> -	u32 frzchn1_state;
> -	frzchn1_state = readl(&freeze_controller_base->hwctrl);
> -	frzchn1_state = SYSMGR_FRZCTRL_HWCTRL_VIO1STATE_GET(frzchn1_state);
> -	return frzchn1_state;
> -}
>  
>  /* Freeze HPS IOs */
>  u32 sys_mgr_frzctrl_freeze_req(FreezeChannelSelect channel_id,
> -	FreezeControllerFSMSelect fsm_select)
> +			       FreezeControllerFSMSelect fsm_select)
>  {
> -	u32 frzctrl_ioctrl_reg_offset;
> -	u32 frzctrl_reg_value;
> -	u32 frzctrl_reg_cfg_mask;
> +	u32 ioctrl_reg_offset;
> +	u32 reg_value;
> +	u32 reg_cfg_mask;
>  	u32 i;
>  
> -	if (FREEZE_CONTROLLER_FSM_SW == fsm_select) {
> -
> -		/* 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 FREEZE_CHANNEL_0:
> -		case FREEZE_CHANNEL_1:
> -		case FREEZE_CHANNEL_2:
> -			frzctrl_ioctrl_reg_offset = (u32)(
> -				&freeze_controller_base->vioctrl +
> -				(channel_id << SYSMGR_FRZCTRL_VIOCTRL_SHIFT));
> -
> -			/*
> -			 * Assert active low enrnsl, plniotri
> -			 * and niotri signals
> -			 */
> -			frzctrl_reg_cfg_mask =
> -				SYSMGR_FRZCTRL_VIOCTRL_SLEW_MASK
> -				| SYSMGR_FRZCTRL_VIOCTRL_WKPULLUP_MASK
> -				| SYSMGR_FRZCTRL_VIOCTRL_TRISTATE_MASK;
> -			clrbits_le32(frzctrl_ioctrl_reg_offset,
> -				frzctrl_reg_cfg_mask);
> -
> -			/*
> -			 * Note: Delay for 20ns at min
> -			 * Assert active low bhniotri signal and de-assert
> -			 * active high csrdone
> -			 */
> -			frzctrl_reg_cfg_mask
> -				= SYSMGR_FRZCTRL_VIOCTRL_BUSHOLD_MASK
> -				| SYSMGR_FRZCTRL_VIOCTRL_CFG_MASK;
> -			clrbits_le32(frzctrl_ioctrl_reg_offset,
> -				frzctrl_reg_cfg_mask);
> -
> -			/* Set global flag to indicate channel is frozen */
> -			frzctrl_channel_freeze[channel_id] =
> -				FREEZE_CTRL_FROZEN;
> -			break;
> -
> -		case FREEZE_CHANNEL_3:
> -			/*
> -			 * Assert active low enrnsl, plniotri and
> -			 * niotri signals
> -			 */
> -			frzctrl_reg_cfg_mask
> -				= SYSMGR_FRZCTRL_HIOCTRL_SLEW_MASK
> -				| SYSMGR_FRZCTRL_HIOCTRL_WKPULLUP_MASK
> -				| SYSMGR_FRZCTRL_HIOCTRL_TRISTATE_MASK;
> -			clrbits_le32(&freeze_controller_base->hioctrl,
> -				frzctrl_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
> -			 */
> -			frzctrl_reg_value =
> -				readl(&freeze_controller_base->hioctrl);
> -			frzctrl_reg_cfg_mask
> -				= SYSMGR_FRZCTRL_HIOCTRL_BUSHOLD_MASK
> -				| SYSMGR_FRZCTRL_HIOCTRL_CFG_MASK;
> -			frzctrl_reg_value
> -				= (frzctrl_reg_value & ~frzctrl_reg_cfg_mask)
> -				| SYSMGR_FRZCTRL_HIOCTRL_REGRST_MASK
> -				| SYSMGR_FRZCTRL_HIOCTRL_OCTRST_MASK;
> -			writel(frzctrl_reg_value,
> -				&freeze_controller_base->hioctrl);
> -
> -			/*
> -			 * Note: Delay for 40ns at min
> -			 * assert active high reinit signal and de-assert
> -			 * active high pllbiasen signals
> -			 */
> -			frzctrl_reg_value =
> -				readl(&freeze_controller_base->hioctrl);
> -			frzctrl_reg_value
> -				= (frzctrl_reg_value &
> -				~SYSMGR_FRZCTRL_HIOCTRL_OCT_CFGEN_CALSTART_MASK)
> -				| SYSMGR_FRZCTRL_HIOCTRL_DLLRST_MASK;
> -			writel(frzctrl_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 1;
> -		}
> -	} else if (FREEZE_CONTROLLER_FSM_HW == fsm_select) {
> -
> -		/* select hardware FSM */
> -		writel(SYSMGR_FRZCTRL_SRC_VIO1_ENUM_HW,
> -			&freeze_controller_base->src);
> -
> -		/* write to hwctrl reg to enable freeze req */
> -		setbits_le32(&freeze_controller_base->hwctrl,
> -			SYSMGR_FRZCTRL_HWCTRL_VIO1REQ_MASK);
> -
> -		i = 0;
> -		while (SYSMGR_FRZCTRL_HWCTRL_VIO1STATE_ENUM_FROZEN
> -			!= sys_mgr_frzctrl_frzchn1_state_get()) {
> -			i++;
> -			if (SYSMGR_FRZCTRL_LOOP_PARAM < i)
> -				return 1;
> -		} /* while ( not frozen) */
> -
> -		/* Set global flag to indicate channel is frozen */
> -		frzctrl_channel_freeze[channel_id] = FREEZE_CTRL_FROZEN;
> -	} else {
> -		return 1;
> -	} /* if-else (fsm_select) */
> +	if (FREEZE_CONTROLLER_FSM_SW != fsm_select)
> +		return -EINVAL;
> +
> +	/* 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 FREEZE_CHANNEL_0:
> +	case FREEZE_CHANNEL_1:
> +	case FREEZE_CHANNEL_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
> +		 */
> +		reg_cfg_mask
> +			= SYSMGR_FRZCTRL_VIOCTRL_BUSHOLD_MASK
> +			| SYSMGR_FRZCTRL_VIOCTRL_CFG_MASK;
> +		clrbits_le32(ioctrl_reg_offset,
> +			     reg_cfg_mask);
> +		break;
> +
> +	case FREEZE_CHANNEL_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
> +		 */
> +		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);
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	return 0;
>  }
>  
>  /* Unfreeze/Thaw HPS IOs */
>  u32 sys_mgr_frzctrl_thaw_req(FreezeChannelSelect channel_id,
> -	FreezeControllerFSMSelect fsm_select)
> +			     FreezeControllerFSMSelect fsm_select)
>  {
> -	u32 frzctrl_ioctrl_reg_offset;
> -	u32 frzctrl_reg_cfg_mask;
> -	u32 frzctrl_reg_value;
> +	u32 ioctrl_reg_offset;
> +	u32 reg_cfg_mask;
> +	u32 reg_value;
>  	u32 i;
>  	u32 start_count, clk_cycles_count;
>  
> -	if (FREEZE_CONTROLLER_FSM_SW == fsm_select) {
> -		/* select software FSM */
> -		writel(SYSMGR_FRZCTRL_SRC_VIO1_ENUM_SW,
> -			&freeze_controller_base->src);
> +	if (FREEZE_CONTROLLER_FSM_SW != fsm_select) {
> +		return -EINVAL;
> +	}
> +
> +
> +	/* 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 FREEZE_CHANNEL_0:
>  	case FREEZE_CHANNEL_1:
>  	case FREEZE_CHANNEL_2:
> -		frzctrl_ioctrl_reg_offset
> +		ioctrl_reg_offset
>  			= (u32)(&freeze_controller_base->vioctrl
> -			+ (channel_id << SYSMGR_FRZCTRL_VIOCTRL_SHIFT));
> +				+ (channel_id << SYSMGR_FRZCTRL_VIOCTRL_SHIFT));
>  
>  		/*
>  		 * Assert active low bhniotri signal and
>  		 * de-assert active high csrdone
>  		 */
> -		frzctrl_reg_cfg_mask
> +		reg_cfg_mask
>  			= SYSMGR_FRZCTRL_VIOCTRL_BUSHOLD_MASK
>  			| SYSMGR_FRZCTRL_VIOCTRL_CFG_MASK;
> -		setbits_le32(frzctrl_ioctrl_reg_offset,
> -			frzctrl_reg_cfg_mask);
> +		setbits_le32(ioctrl_reg_offset,
> +			     reg_cfg_mask);
>  
>  		/*
>  		 * Note: Delay for 20ns at min
>  		 * de-assert active low plniotri and niotri signals
>  		 */
> -		frzctrl_reg_cfg_mask
> +		reg_cfg_mask
>  			= SYSMGR_FRZCTRL_VIOCTRL_WKPULLUP_MASK
>  			| SYSMGR_FRZCTRL_VIOCTRL_TRISTATE_MASK;
> -		setbits_le32(frzctrl_ioctrl_reg_offset,
> -			frzctrl_reg_cfg_mask);
> +		setbits_le32(ioctrl_reg_offset,
> +			     reg_cfg_mask);
>  
>  		/*
>  		 * Note: Delay for 20ns at min
>  		 * de-assert active low enrnsl signal
>  		 */
> -		setbits_le32(frzctrl_ioctrl_reg_offset,
> -			SYSMGR_FRZCTRL_VIOCTRL_SLEW_MASK);
> -
> -		/* Set global flag to indicate channel is thawed */
> -		frzctrl_channel_freeze[channel_id] = FREEZE_CTRL_THAWED;
> -
> +		setbits_le32(ioctrl_reg_offset,
> +			     SYSMGR_FRZCTRL_VIOCTRL_SLEW_MASK);
>  		break;
>  
>  	case FREEZE_CHANNEL_3:
>  		/* de-assert active high reinit signal */
>  		clrbits_le32(&freeze_controller_base->hioctrl,
> -			SYSMGR_FRZCTRL_HIOCTRL_DLLRST_MASK);
> +			     SYSMGR_FRZCTRL_HIOCTRL_DLLRST_MASK);
>  
>  		/*
>  		 * Note: Delay for 40ns at min
>  		 * assert active high pllbiasen signals
>  		 */
>  		setbits_le32(&freeze_controller_base->hioctrl,
> -			SYSMGR_FRZCTRL_HIOCTRL_OCT_CFGEN_CALSTART_MASK);
> +			     SYSMGR_FRZCTRL_HIOCTRL_OCT_CFGEN_CALSTART_MASK);
>  
>  		/* Delay 1000 intosc. intosc is based on eosc1 */
>  		reset_timer_count();
> @@ -238,18 +195,18 @@ u32 sys_mgr_frzctrl_thaw_req(FreezeChannelSelect channel_id,
>  			clk_cycles_count =
>  				get_timer_count(start_count);
>  		} while (clk_cycles_count <
> -			SYSMGR_FRZCTRL_INTOSC_1000);
> +			 SYSMGR_FRZCTRL_INTOSC_1000);
>  
>  		/*
>  		 * de-assert active low bhniotri signals,
>  		 * assert active high csrdone and nfrzdrv signal
>  		 */
> -		frzctrl_reg_value = readl(&freeze_controller_base->hioctrl);
> -		frzctrl_reg_value = (frzctrl_reg_value
> -			| SYSMGR_FRZCTRL_HIOCTRL_BUSHOLD_MASK
> -			| SYSMGR_FRZCTRL_HIOCTRL_CFG_MASK)
> +		reg_value = readl(&freeze_controller_base->hioctrl);
> +		reg_value = (reg_value
> +				     | SYSMGR_FRZCTRL_HIOCTRL_BUSHOLD_MASK
> +				     | SYSMGR_FRZCTRL_HIOCTRL_CFG_MASK)
>  			& ~SYSMGR_FRZCTRL_HIOCTRL_OCTRST_MASK;
> -		writel(frzctrl_reg_value, &freeze_controller_base->hioctrl);
> +		writel(reg_value, &freeze_controller_base->hioctrl);
>  
>  		/* Delay 33 intosc */
>  		reset_timer_count();
> @@ -259,63 +216,35 @@ u32 sys_mgr_frzctrl_thaw_req(FreezeChannelSelect channel_id,
>  			clk_cycles_count =
>  				get_timer_count(start_count);
>  		} while (clk_cycles_count <
> -			SYSMGR_FRZCTRL_INTOSC_33);
> +			 SYSMGR_FRZCTRL_INTOSC_33);
>  
>  		/* de-assert active low plniotri and niotri signals */
> -		frzctrl_reg_cfg_mask
> +		reg_cfg_mask
>  			= SYSMGR_FRZCTRL_HIOCTRL_WKPULLUP_MASK
>  			| SYSMGR_FRZCTRL_HIOCTRL_TRISTATE_MASK;
>  
>  		setbits_le32(&freeze_controller_base->hioctrl,
> -			frzctrl_reg_cfg_mask);
> +			     reg_cfg_mask);
>  
>  		/*
>  		 * Note: Delay for 40ns at min
>  		 * de-assert active high frzreg signal
>  		 */
>  		clrbits_le32(&freeze_controller_base->hioctrl,
> -			SYSMGR_FRZCTRL_HIOCTRL_REGRST_MASK);
> +			     SYSMGR_FRZCTRL_HIOCTRL_REGRST_MASK);
>  
>  		/*
>  		 * Note: Delay for 40ns at min
>  		 * de-assert active low enrnsl signal
>  		 */
>  		setbits_le32(&freeze_controller_base->hioctrl,
> -			SYSMGR_FRZCTRL_HIOCTRL_SLEW_MASK);
> -
> -		/* Set global flag to indicate channel is thawed */
> -		frzctrl_channel_freeze[channel_id] = FREEZE_CTRL_THAWED;
> -
> +			     SYSMGR_FRZCTRL_HIOCTRL_SLEW_MASK);
>  		break;
>  
>  	default:
> -		return 1;
> +		return -EINVAL;
>  	}
>  
> -	} else if (FREEZE_CONTROLLER_FSM_HW == fsm_select) {
> -
> -		/* select hardware FSM */
> -		writel(SYSMGR_FRZCTRL_SRC_VIO1_ENUM_HW,
> -			&freeze_controller_base->src);
> -
> -		/* write to hwctrl reg to enable thaw req; 0: thaw */
> -		clrbits_le32(&freeze_controller_base->hwctrl,
> -			SYSMGR_FRZCTRL_HWCTRL_VIO1REQ_MASK);
> -
> -		i = 0;
> -		while (SYSMGR_FRZCTRL_HWCTRL_VIO1STATE_ENUM_THAWED
> -			!= sys_mgr_frzctrl_frzchn1_state_get()) {
> -			i++;
> -			if (SYSMGR_FRZCTRL_LOOP_PARAM < i)
> -				return 1;
> -		} /* while (not thaw) */
> -
> -		/* Set global flag to indicate channel is thawed */
> -		frzctrl_channel_freeze[channel_id] = FREEZE_CTRL_THAWED;
> -	} else {
> -		return 1;
> -	} /* if-else (fsm_select) */
> -
>  	return 0;
>  }
>  
> 





More information about the U-Boot mailing list