[U-Boot] [PATCH 2/3] mmc: split fsl_esdhc driver for i.MX

Stefano Babic sbabic at denx.de
Wed Mar 13 11:53:01 UTC 2019


Hi Y.B lu,

On 21/02/19 08:55, Y.b. Lu wrote:
> The fsl_esdhc driver was for Freescale eSDHC on MPC83XX/MPC85XX
> initially. The later QoriQ series PowerPC processors (which were
> evolutions of MPC83XX/MPC85XX), QorIQ series ARM processors, and
> i.MX series processors were using this driver for their eSDHCs too.
> 
> For the two series processors, the eSDHCs are becoming more and
> more different. We should have split it into two drivers, like them
> (sdhci-of-esdhc.c/sdhci-esdhc-imx.c) in linux kernel.
> 
> This patch is just to create a fsl_esdhc_imx driver which is a copy
> of fsl_esdhc driver for i.MX processors. We will convert i.MX processors
> to use fsl_esdhc_imx, and clean up the two drivers separately in the
> future patches.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
> ---
>  drivers/mmc/Kconfig                          |  6 ++++++
>  drivers/mmc/Makefile                         |  1 +
>  drivers/mmc/{fsl_esdhc.c => fsl_esdhc_imx.c} |  5 +++--
>  include/{fsl_esdhc.h => fsl_esdhc_imx.h}     | 11 ++++++-----
>  4 files changed, 16 insertions(+), 7 deletions(-)
>  copy drivers/mmc/{fsl_esdhc.c => fsl_esdhc_imx.c} (99%)
>  copy include/{fsl_esdhc.h => fsl_esdhc_imx.h} (97%)
> 

IMHO we can do better - if we split the code, we should be able to
factorize common code (if any) into a separate file and move the
specific parts into processor specific files. And at the same time,
clean up what is not required (for example, CONFIG_MCF5441x should not
appear in imx code, and so on).

> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 04a4e7716f..09bc02fe9c 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -641,6 +641,12 @@ config FSL_ESDHC
>  	  This selects support for the eSDHC (enhanced secure digital host
>  	  controller) found on numerous Freescale/NXP SoCs.
>  
> +config FSL_ESDHC_IMX
> +	bool "Freescale/NXP i.MX eSDHC controller support"

We need a depend clause (ARCH_MX6, ARCH_MX5, ..)

> +	help
> +	  This selects support for the i.MX eSDHC (enhanced secure digital host
> +	  controller) found on numerous Freescale/NXP SoCs.
> +
>  endmenu
>  
>  config SYS_FSL_ERRATUM_ESDHC111
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 7892c468f0..1287ad4cc1 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_MMC_DW_K3)			+= hi6220_dw_mmc.o
>  obj-$(CONFIG_MMC_DW_ROCKCHIP)		+= rockchip_dw_mmc.o
>  obj-$(CONFIG_MMC_DW_SOCFPGA)		+= socfpga_dw_mmc.o
>  obj-$(CONFIG_FSL_ESDHC) += fsl_esdhc.o
> +obj-$(CONFIG_FSL_ESDHC_IMX) += fsl_esdhc_imx.o
>  obj-$(CONFIG_FTSDC010) += ftsdc010_mci.o
>  obj-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
>  obj-$(CONFIG_MMC_MESON_GX) += meson_gx_mmc.o
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc_imx.c
> similarity index 99%
> copy from drivers/mmc/fsl_esdhc.c
> copy to drivers/mmc/fsl_esdhc_imx.c
> index 21fa2ab1d4..9c823e86e2 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright 2007, 2010-2011 Freescale Semiconductor, Inc
>   * Andy Fleming
> + * Copyright 2019 NXP
>   *
>   * Based vaguely on the pxa mmc code:
>   * (C) Copyright 2003
> @@ -18,7 +19,7 @@
>  #include <part.h>
>  #include <power/regulator.h>
>  #include <malloc.h>
> -#include <fsl_esdhc.h>
> +#include <fsl_esdhc_imx.h>
>  #include <fdt_support.h>
>  #include <asm/io.h>
>  #include <dm.h>
> @@ -110,7 +111,7 @@ struct esdhc_soc_data {
>   * @non_removable: 0: removable; 1: non-removable
>   * @wp_enable: 1: enable checking wp; 0: no check
>   * @vs18_enable: 1: use 1.8V voltage; 0: use 3.3V
> - * @flags: ESDHC_FLAG_xx in include/fsl_esdhc.h
> + * @flags: ESDHC_FLAG_xx in include/fsl_esdhc_imx.h
>   * @caps: controller capabilities
>   * @tuning_step: tuning step setting in tuning_ctrl register
>   * @start_tuning_tap: the start point for tuning in tuning_ctrl register
> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc_imx.h
> similarity index 97%
> copy from include/fsl_esdhc.h
> copy to include/fsl_esdhc_imx.h
> index 8dbd5249a7..e05b24e7e8 100644
> --- a/include/fsl_esdhc.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -4,10 +4,11 @@
>   *-------------------------------------------------------------------
>   *
>   * Copyright 2007-2008,2010-2011 Freescale Semiconductor, Inc
> + * Copyright 2019 NXP
>   */
>  
> -#ifndef  __FSL_ESDHC_H__
> -#define	__FSL_ESDHC_H__
> +#ifndef __FSL_ESDHC_IMX_H__
> +#define __FSL_ESDHC_IMX_H__
>  
>  #include <linux/bitops.h>
>  #include <linux/errno.h>
> @@ -258,15 +259,15 @@ struct fsl_esdhc_cfg {
>  #error "Endianess is not defined: please fix to continue"
>  #endif
>  
> -#ifdef CONFIG_FSL_ESDHC
> +#ifdef CONFIG_FSL_ESDHC_IMX
>  int fsl_esdhc_mmc_init(bd_t *bis);
>  int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg);
>  void fdt_fixup_esdhc(void *blob, bd_t *bd);
>  #else
>  static inline int fsl_esdhc_mmc_init(bd_t *bis) { return -ENOSYS; }
>  static inline void fdt_fixup_esdhc(void *blob, bd_t *bd) {}
> -#endif /* CONFIG_FSL_ESDHC */
> +#endif /* CONFIG_FSL_ESDHC_IMX */
>  void __noreturn mmc_boot(void);
>  void mmc_spl_load_image(uint32_t offs, unsigned int size, void *vdst);
>  
> -#endif  /* __FSL_ESDHC_H__ */
> +#endif  /* __FSL_ESDHC_IMX_H__ */
> 

I tend to go further and to cleanup the new file removing what is not
required. Do it in a separate patch, but belonging to the same series,
so that is easier to review - thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================


More information about the U-Boot mailing list