[U-Boot] [v3 25/30] arm: socfpga: arria10: Added drivers for Arria10 Reset Manager

[Chee, Tien Fong <tien.fong.chee@intel.com>]

On Jum, 2017-01-06 at 17:03 -0600, Dinh Nguyen wrote:
> 
> On 01/06/2017 05:19 AM, Chee Tien Fong wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > Drivers for reset manager is restructured such that common
> > functions,
> > gen5 drivers and Arria10 drivers are moved to reset_manager.c,
> > reset_manager_gen5.c and reset_manager_arria10.c respectively.
> > 
> In Linux, I was able to use the same reset manager driver to support?
> both gen5 and Arria10 devices, are you sure you can't do the same
> here?
> 
> Take a look at this commit:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commi
> /drivers/rese/resetsocfpga.c?id=27e44646dc0083c931b71bbb8e179aeb38010
> d31
> 
> My guess is that you can probably use the same driver but with
> different?
> macro defines.
> 
We have some prepocessing required before triggering the simple reset
assert/deassert on the right peripheral. Some new functions are created
for specific group reset purpose like all bridges reset based on
handoff. As per agreement between marek and CL, those functions merging
which require #ifdef arria10/gen5, then for the sake of clean code, it
is better putting them in different files based on device type.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > Cc: Chin Liang See <chin.liang.see@intel.com>
> > Cc: Tien Fong <skywindctf@gmail.com>
> > ---
> > Changes for V3
> > - no changes
> > Changes for V2
> > - Reverted license changes, removing extern and volatile
> > declaration
> > ---
> > ?arch/arm/mach-socfpga/Makefile?????????????????????|???16 +-
> > ?arch/arm/mach-socfpga/include/mach/reset_manager.h |??155 ++++++--
> > ?arch/arm/mach-socfpga/reset_manager.c??????????????|??112 +------
> > ?arch/arm/mach-socfpga/reset_manager_arria10.c??????|??407
> > ++++++++++++++++++++
> > ?.../{reset_manager.c => reset_manager_gen5.c}??????|???94 ++---
> > ?5 files changed, 570 insertions(+), 214 deletions(-)
> > ?create mode 100644 arch/arm/mach-socfpga/reset_manager_arria10.c
> > ?copy arch/arm/mach-socfpga/{reset_manager.c =>
> > reset_manager_gen5.c} (58%)
> > 
> > diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-
> > socfpga/Makefile
> > index 809cd47..b8fcf6e 100644
> > --- a/arch/arm/mach-socfpga/Makefile
> > +++ b/arch/arm/mach-socfpga/Makefile
> > @@ -2,21 +2,27 @@
> > ?# (C) Copyright 2000-2003
> > ?# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> > ?#
> > -# Copyright (C) 2012 Altera Corporation <www.altera.com>
> > +# Copyright (C) 2012-2016 Altera Corporation <www.altera.com>
> > ?#
> > ?# SPDX-License-Identifier:	GPL-2.0+
> > ?#
> > 
> > ?obj-y	+= misc.o timer.o reset_manager.o system_manager.o
> > clock_manager.o \
> > ?	???fpga_manager.o board.o
> > +obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += reset_manager_arria10.o
> > +obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += scan_manager.o
> > wrap_pll_config.o \
> > +				reset_manager_gen5.o
> > 
> > -obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o
> > +ifdef CONFIG_SPL_BUILD
> > +obj-y += spl.o
> > +obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += freeze_controller.o
> > wrap_iocsr_config.o \
> > +				wrap_pinmux_config.o
> > wrap_sdram_config.o
> > +endif
> > 
> > +ifdef CONFIG_TARGET_SOCFPGA_GEN5
> > ?# QTS-generated config file wrappers
> > -obj-$(CONFIG_TARGET_SOCFPGA_GEN5)	+= scan_manager.o
> > wrap_pll_config.o
> > -obj-$(CONFIG_SPL_BUILD) += wrap_iocsr_config.o
> > wrap_pinmux_config.o	\
> > -			???wrap_sdram_config.o
> > ?CFLAGS_wrap_iocsr_config.o	+=
> > -I$(srctree)/board/$(BOARDDIR)
> > ?CFLAGS_wrap_pinmux_config.o	+=
> > -I$(srctree)/board/$(BOARDDIR)
> > ?CFLAGS_wrap_pll_config.o	+= -I$(srctree)/board/$(BOARDDIR)
> > ?CFLAGS_wrap_sdram_config.o	+=
> > -I$(srctree)/board/$(BOARDDIR)
> > +endif
> > diff --git a/arch/arm/mach-socfpga/include/mach/reset_manager.h
> > b/arch/arm/mach-socfpga/include/mach/reset_manager.h
> > index 6225118..13f9731 100644
> > --- a/arch/arm/mach-socfpga/include/mach/reset_manager.h
> > +++ b/arch/arm/mach-socfpga/include/mach/reset_manager.h
> > @@ -1,5 +1,5 @@
> > ?/*
> > - *??Copyright (C) 2012 Altera Corporation <www.altera.com>
> > + *??Copyright (C) 2012-2016 Altera Corporation <www.altera.com>
> > ? *
> > ? * SPDX-License-Identifier:	GPL-2.0+
> > ? */
> > @@ -7,15 +7,27 @@
> > ?#ifndef	_RESET_MANAGER_H_
> > ?#define	_RESET_MANAGER_H_
> > 
> > +/* Common function prototypes */
> > ?void reset_cpu(ulong addr);
> > -void reset_deassert_peripherals_handoff(void);
> > -
> > ?void socfpga_bridges_reset(int enable);
> > -
> > ?void socfpga_per_reset(u32 reset, int set);
> > ?void socfpga_per_reset_all(void);
> > 
> > ?#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > +void reset_deassert_peripherals_handoff(void);
> > +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > +void watchdog_disable(void);
> > +void reset_deassert_noc_ddr_scheduler(void);
> > +int is_wdt_in_reset(void);
> > +void emac_manage_reset(ulong emacbase, uint state);
> > +int reset_deassert_bridges_handoff(void);
> > +void reset_assert_fpga_connected_peripherals(void);
> > +void reset_deassert_osc1wd0(void);
> > +void reset_assert_uart(void);
> > +void reset_deassert_uart(void);
> Not sure why you need these new functions instead of re-using?
> socfpga_per_reset()?
> 
Those reset functions are not just simple reset, there are some
preprocessing or populating based on handoff required before triggering
reset on right peripheral. For example, U-boot can reset deassert
correct UART port based on the handoff. So, we tidy up those
preprocessing ?and reset into one function. But for the
socfpga_per_reset, we have different register name in both
gen5/arria10, so i prefer to put them in separate file based on device
type. i believe it helps to avoid confusion when mapping the name to
our registers datasheet.
> > 
> > +#endif
> > +
> > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
> > ?struct socfpga_reset_manager {
> > ?	u32	status;
> > ?	u32	ctrl;
> > @@ -29,40 +41,40 @@ struct socfpga_reset_manager {
> > ?	u32	padding2[12];
> > ?	u32	tstscratch;
> > ?};
> > -#else
> > +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > ?struct socfpga_reset_manager {
> > -	u32	stat;
> > -	u32	ramstat;
> > -	u32	miscstat;
> > -	u32	ctrl;
> > -	u32	hdsken;
> > -	u32	hdskreq;
> > -	u32	hdskack;
> > -	u32	counts;
> > -	u32	mpu_mod_reset;
> > -	u32	per_mod_reset;	/* stated as
> > per0_mod_reset in A10 datasheet */
> > -	u32	per2_mod_reset;	/* stated as
> > per1_mod_reset in A10 datasheet */
> > -	u32	brg_mod_reset;
> > -	u32	misc_mod_reset;	/* stated as
> > sys_mod_reset in A10 datasheet */
> > -	u32	coldmodrst;
> > -	u32	nrstmodrst;
> > -	u32	dbgmodrst;
> > -	u32	mpuwarmmask;
> > -	u32	per0warmmask;
> > -	u32	per1warmmask;
> > -	u32	brgwarmmask;
> > -	u32	syswarmmask;
> > -	u32	nrstwarmmask;
> > -	u32	l3warmmask;
> > -	u32	tststa;
> > -	u32	tstscratch;
> > -	u32	hdsktimeout;
> > -	u32	hmcintr;
> > -	u32	hmcintren;
> > -	u32	hmcintrens;
> > -	u32	hmcintrenr;
> > -	u32	hmcgpout;
> > -	u32	hmcgpin;
> > +	uint32_t??stat;
> > +	uint32_t??ramstat;
> > +	uint32_t??miscstat;
> > +	uint32_t??ctrl;
> > +	uint32_t??hdsken;
> > +	uint32_t??hdskreq;
> > +	uint32_t??hdskack;
> > +	uint32_t??counts;
> > +	uint32_t??mpumodrst;
> > +	uint32_t??per0modrst;
> > +	uint32_t??per1modrst;
> > +	uint32_t??brgmodrst;
> > +	uint32_t??sysmodrst;
> > +	uint32_t??coldmodrst;
> > +	uint32_t??nrstmodrst;
> > +	uint32_t??dbgmodrst;
> > +	uint32_t??mpuwarmmask;
> > +	uint32_t??per0warmmask;
> > +	uint32_t??per1warmmask;
> > +	uint32_t??brgwarmmask;
> > +	uint32_t??syswarmmask;
> > +	uint32_t??nrstwarmmask;
> > +	uint32_t??l3warmmask;
> > +	uint32_t??tststa;
> > +	uint32_t??tstscratch;
> > +	uint32_t??hdsktimeout;
> > +	uint32_t??hmcintr;
> > +	uint32_t??hmcintren;
> > +	uint32_t??hmcintrens;
> > +	uint32_t??hmcintrenr;
> > +	uint32_t??hmcgpout;
> > +	uint32_t??hmcgpin;
> > ?};
> > ?#endif
> > 
> Why did you have to change the type for these reset manager
> variables?
> 
> Dinh

I recalled i did some checking on these two types, for u32 type, the
build support on this type can be turned off, and some comment for the
limitation of using this type, like "avoid exported outside the kernel
to avoid name space clashes". However, uint32_t is widely accepted and
using in U-boot. For details, i think you need to dig more from the
codes, then i believe you will think uint32_t is better.