[U-Boot] [PATCHv3 9/9] arm: socfpga: fix up a questionable macro for SDMMC

Marek Vasut
2015-12-02T14:23:03Z

On Wednesday, December 02, 2015 at 07:12:56 AM, dinguyen at opensource.altera.com
wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> Move the macro into the socfpga_dwmci_clksel().
>
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
> arch/arm/mach-socfpga/include/mach/system_manager.h | 8 +++++---
> drivers/mmc/socfpga_dw_mmc.c | 5 +++--
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h
> b/arch/arm/mach-socfpga/include/mach/system_manager.h index
> cc070dc..f8a9b69 100644
> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> @@ -201,9 +201,11 @@ struct socfpga_system_manager {
> #define SYSMGR_FPGAINTF_NAND (1 << 4)
> #define SYSMGR_FPGAINTF_SDMMC (1 << 5)
>
> -/* FIXME: This is questionable macro. */
> -#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> - ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38))
> +#if defined(CONFIG_SOCFPGA_GEN5)
> +#define SYSMGR_SDMMC_SMPSEL_SHIFT 3

Should this be SMPSEL or SMPLSEL (the original variable contained 'l') ?

> +#else
> +#define SYSMGR_SDMMC_SMPSEL_SHIFT 4
> +#endif
>
> /* EMAC Group Bit definitions */
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
> diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
> index 5b0c3a8..6a55a6c 100644
> --- a/drivers/mmc/socfpga_dw_mmc.c
> +++ b/drivers/mmc/socfpga_dw_mmc.c
> @@ -33,6 +33,8 @@ struct dwmci_socfpga_priv_data {
> static void socfpga_dwmci_clksel(struct dwmci_host *host)
> {
> struct dwmci_socfpga_priv_data *priv = host->priv;
> + u32 sdmmc_mask = ((((priv->smplsel) & 0x7) << SYSMGR_SDMMC_SMPSEL_SHIFT)
> | + ((priv->drvsel) & 0x7) << 0);

I am just waiting for someone smart in the hardware design dept. to move the
drvsel register a bit. You should really define SYSMGR_SDMMC_DRVSEL_SHIFT and
use it here.

> /* Disable SDMMC clock. */
> clrbits_le32(&clock_manager_base->per_pll.en,
> @@ -40,8 +42,7 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
>
> debug("%s: drvsel %d smplsel %d\n", __func__,
> priv->drvsel, priv->smplsel);
> - writel(SYSMGR_SDMMC_CTRL_SET(priv->smplsel, priv->drvsel),
> - &system_manager_base->sdmmcgrp_ctrl);
> + writel(sdmmc_mask, &system_manager_base->sdmmcgrp_ctrl);
>
> debug("%s: SYSMGR_SDMMCGRP_CTRL_REG = 0x%x\n", __func__,
> readl(&system_manager_base->sdmmcgrp_ctrl));

Best regards,
Marek Vasut