[U-Boot] [PATCH 3/6] arm: socfpga: arria10: update dwmac reset function to support Arria10

[Marek Vasut <marex@denx.de>]

On Friday, January 08, 2016 at 04:51:20 PM, dinguyen at opensource.altera.com 
wrote:
> From: Dinh Nguyen <dinh.linux@gmail.com>
> 
> On the Arria10, the EMAC phy mode configuration for each EMACs is located
> in separate registers versus being in 1 register for the GEN5 devices. The
> Arria10 also has 3 EMACs compared to 2 for the GEN5 devices.
> 
> Update the dwmac_deassert_reset function to support both GEN5 and Arria10
> devices.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
>  arch/arm/mach-socfpga/include/mach/system_manager.h |  1 +
>  arch/arm/mach-socfpga/misc.c                        | 20
> ++++++++++++++++++++ 2 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h
> b/arch/arm/mach-socfpga/include/mach/system_manager.h index
> 9ca889a..bfabf00 100644
> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> @@ -218,6 +218,7 @@ struct socfpga_system_manager {
> 
>  #define SYSMGR_EMACGRP_CTRL_PHYSEL0_LSB			0
>  #define SYSMGR_EMACGRP_CTRL_PHYSEL1_LSB			2
> +#define SYSMGR_EMACGRP_CTRL_PHYSEL2_LSB			4
>  #define SYSMGR_EMACGRP_CTRL_PHYSEL_MASK			0x3
> 
>  /* For dedicated IO configuration */
> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> index 517f629..6ef49c3 100644
> --- a/arch/arm/mach-socfpga/misc.c
> +++ b/arch/arm/mach-socfpga/misc.c
> @@ -22,7 +22,12 @@
>  #include <asm/arch/scu.h>
>  #include <asm/pl310.h>
> 
> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
>  #include <dt-bindings/reset/altr,rst-mgr.h>
> +#else
> +#include <dt-bindings/reset/altr,rst-mgr-a10.h>
> +#endif
> +
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -95,11 +100,17 @@ static void dwmac_deassert_reset(const unsigned int
> of_reset_id) } else if (of_reset_id == EMAC1_RESET) {
>  		physhift = SYSMGR_EMACGRP_CTRL_PHYSEL1_LSB;
>  		reset = SOCFPGA_RESET(EMAC1);
> +#ifndef CONFIG_TARGET_SOCFPGA_GEN5
> +	} else if (of_reset_id == EMAC2_RESET) {
> +		physhift = SYSMGR_EMACGRP_CTRL_PHYSEL2_LSB;
> +		reset = SOCFPGA_RESET(EMAC2);
> +#endif
>  	} else {
>  		printf("GMAC: Invalid reset ID (%i)!\n", of_reset_id);
>  		return;
>  	}
> 
> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
>  	/* Clearing emac0 PHY interface select to 0 */
>  	clrbits_le32(&sysmgr_regs->emacgrp_ctrl,
>  		     SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << physhift);
> @@ -107,6 +118,15 @@ static void dwmac_deassert_reset(const unsigned int
> of_reset_id) /* configure to PHY interface select choosed */
>  	setbits_le32(&sysmgr_regs->emacgrp_ctrl,
>  		     SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII << physhift);
> +#else
> +	/* Clearing emac0 PHY interface select to 0 */
> +	clrbits_le32(&sysmgr_regs->emac0 + (0x2 * physhift),
> +		     SYSMGR_EMACGRP_CTRL_PHYSEL_MASK);
> +
> +	/* configure to PHY interface to RGMII */
> +	setbits_le32(&sysmgr_regs->emac0 + (0x2 * physhift),
> +		     SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII);

If you changed the structure definition for struct socfpga_system_manager such
that you'd replace the three u32 emac0...u32 emac2 there with u32 emac[3], you'd
be able to do something like

index = of_reset_id - EMAC0_RESET;
xxxbits_le32(&sysmgr_regs->emac[index], MASK);

Do you think it's worth it? I don't really like the 0x2 * physhift magic there,
it's pretty cryptic.

Also, I think it should be possible to convert the clrbits + setbits combo above 
into single clrsetbits_le32() call. Or is there a meaning behind first clearing
the bitfield and then setting it ? This could be done for Gen5 too.

> +#endif
> 
>  	/* Release the EMAC controller from reset */
>  	socfpga_per_reset(reset, 0);