diff mbox series

[2/4] mx6cuboxi: enable MMC iomux on board_early_init_f

Message ID 20200304144125.8564-3-walter.lozano@collabora.com
State New
Headers show
Series mx6cuboxi: enable support for OF_CONTROL and DM in SPL | expand

Commit Message

Walter Lozano March 4, 2020, 2:41 p.m. UTC
MMC iomux is done on board_mmc_init which is valid when DM_MMC is not
enabled. After enabling it, the iomux setup needs to be moved to a
valid place.

This patch moves the MMC iomux to board_early_init_f where other iomux
is done.

Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
---
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Baruch Siach March 8, 2020, 4:38 a.m. UTC | #1
Hi Walter,

Thanks for your SPL_DM support work on this platform.

One comment inline below.

On Wed, Mar 04 2020, Walter Lozano wrote:
> MMC iomux is done on board_mmc_init which is valid when DM_MMC is not
> enabled. After enabling it, the iomux setup needs to be moved to a
> valid place.
>
> This patch moves the MMC iomux to board_early_init_f where other iomux
> is done.
>
> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
> ---
>  board/solidrun/mx6cuboxi/mx6cuboxi.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> index 6a96f9ecdb..71c77ad2a2 100644
> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -179,6 +179,28 @@ int board_mmc_get_env_dev(int devno)
>  
>  #define USDHC2_CD_GPIO  IMX_GPIO_NR(1, 4)
>  
> +static int setup_iomux_mmc(void)
> +{
> +	struct src *psrc = (struct src *)SRC_BASE_ADDR;
> +	unsigned reg = readl(&psrc->sbmr1) >> 11;

This mostly duplicates the existing mmc_init_spl() routine. As I understand,
mmc_init_spl() becomes dead code once you enable CONFIG_SPL_DM in patch #4 of
this series. Can you remove mmc_init_spl() in a followup patch? Both
struct fsl_esdhc_cfg can also be removed, I believe.

baruch

> +
> +	/*
> +	 * Upon reading BOOT_CFG register the following map is done:
> +	 * Bit 11 and 12 of BOOT_CFG register can determine the current
> +	 * mmc port
> +	 * 0x1                  SD2
> +	 * 0x2                  SD3
> +	 */
> +	switch (reg & 0x3) {
> +	case 0x1:
> +		SETUP_IOMUX_PADS(usdhc2_pads);
> +	case 0x2:
> +		SETUP_IOMUX_PADS(usdhc3_pads);
> +	}
> +
> +	return 0;
> +}
> +
>  int board_mmc_getcd(struct mmc *mmc)
>  {
>  	struct fsl_esdhc_cfg *cfg = mmc->priv;
> @@ -432,9 +454,12 @@ int board_early_init_f(void)
>  {
>  	setup_iomux_uart();
>  
> +	setup_iomux_mmc();
> +
>  #ifdef CONFIG_CMD_SATA
>  	setup_sata();
>  #endif
> +
>  	return 0;
>  }
Walter Lozano March 9, 2020, 7:55 p.m. UTC | #2
Hi Baruch,

Thanks for you comments.

On 8/3/20 01:38, Baruch Siach wrote:
> Hi Walter,
>
> Thanks for your SPL_DM support work on this platform.
>
> One comment inline below.
>
> On Wed, Mar 04 2020, Walter Lozano wrote:
>> MMC iomux is done on board_mmc_init which is valid when DM_MMC is not
>> enabled. After enabling it, the iomux setup needs to be moved to a
>> valid place.
>>
>> This patch moves the MMC iomux to board_early_init_f where other iomux
>> is done.
>>
>> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
>> ---
>>   board/solidrun/mx6cuboxi/mx6cuboxi.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>> index 6a96f9ecdb..71c77ad2a2 100644
>> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
>> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>> @@ -179,6 +179,28 @@ int board_mmc_get_env_dev(int devno)
>>   
>>   #define USDHC2_CD_GPIO  IMX_GPIO_NR(1, 4)
>>   
>> +static int setup_iomux_mmc(void)
>> +{
>> +	struct src *psrc = (struct src *)SRC_BASE_ADDR;
>> +	unsigned reg = readl(&psrc->sbmr1) >> 11;
> This mostly duplicates the existing mmc_init_spl() routine. As I understand,
> mmc_init_spl() becomes dead code once you enable CONFIG_SPL_DM in patch #4 of
> this series. Can you remove mmc_init_spl() in a followup patch? Both
> struct fsl_esdhc_cfg can also be removed, I believe.

You are right, if we enable both CONFIG_SPL_DM and CONFIG_SPL_DM_MMC the 
function mmc_init_spl is useless, so I understand we can safety remove it.

> baruch
>
Regards,

Walter
diff mbox series

Patch

diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 6a96f9ecdb..71c77ad2a2 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -179,6 +179,28 @@  int board_mmc_get_env_dev(int devno)
 
 #define USDHC2_CD_GPIO  IMX_GPIO_NR(1, 4)
 
+static int setup_iomux_mmc(void)
+{
+	struct src *psrc = (struct src *)SRC_BASE_ADDR;
+	unsigned reg = readl(&psrc->sbmr1) >> 11;
+
+	/*
+	 * Upon reading BOOT_CFG register the following map is done:
+	 * Bit 11 and 12 of BOOT_CFG register can determine the current
+	 * mmc port
+	 * 0x1                  SD2
+	 * 0x2                  SD3
+	 */
+	switch (reg & 0x3) {
+	case 0x1:
+		SETUP_IOMUX_PADS(usdhc2_pads);
+	case 0x2:
+		SETUP_IOMUX_PADS(usdhc3_pads);
+	}
+
+	return 0;
+}
+
 int board_mmc_getcd(struct mmc *mmc)
 {
 	struct fsl_esdhc_cfg *cfg = mmc->priv;
@@ -432,9 +454,12 @@  int board_early_init_f(void)
 {
 	setup_iomux_uart();
 
+	setup_iomux_mmc();
+
 #ifdef CONFIG_CMD_SATA
 	setup_sata();
 #endif
+
 	return 0;
 }