diff mbox

[U-Boot,2/3] wandboard: Add support for carrier board MicroSD card

Message ID CAP9ODKqrU0iGynYrDYPcWv1Mc5abhr15c=VgD672uuP3cHSo4A@mail.gmail.com
State Superseded
Delegated to: Stefano Babic
Headers show

Commit Message

Otavio Salvador April 15, 2013, 4:18 p.m. UTC
On Mon, Apr 15, 2013 at 12:00 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Apr 15, 2013 at 11:06 AM, Otavio Salvador
> <otavio@ossystems.com.br> wrote:
>
>> Should we fail if *any* fail?
>
> Yes, I think so.


--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

Comments

Fabio Estevam April 15, 2013, 4:40 p.m. UTC | #1
On Mon, Apr 15, 2013 at 1:18 PM, Otavio Salvador
<otavio@ossystems.com.br> wrote:

> --- a/board/wandboard/wandboard.c
> +++ b/board/wandboard/wandboard.c
> @@ -162,13 +162,15 @@ int board_mmc_init(bd_t *bis)
>                         gpio_direction_input(USDHC1_CD_GPIO);
>                         break;
>                 default:
> -                       printf("Warning: you configured more USDHC controllers"
> +                       printf("ERROR: you configured more USDHC controllers"
>                                    "(%d) than supported by the board\n", i + 1);
>                         return -EINVAL;
>                 }
>
> -               if (fsl_esdhc_initialize(bis, &usdhc_cfg[i]))
> -                       printf("Warning: failed to initialize mmc dev %d\n", i);
> +               if (fsl_esdhc_initialize(bis, &usdhc_cfg[i])) {
> +                       printf("ERROR: failed to initialize mmc dev %d\n", i);
> +                       return 1;

1 is not an appropriate return value for an error.

What about doing like sabrelite?
status |= fsl_esdhc_initialize(bis, &usdhc_cfg[i]);

and then you return status?
Otavio Salvador April 15, 2013, 5 p.m. UTC | #2
On Mon, Apr 15, 2013 at 1:40 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Apr 15, 2013 at 1:18 PM, Otavio Salvador
> <otavio@ossystems.com.br> wrote:
>
>> --- a/board/wandboard/wandboard.c
>> +++ b/board/wandboard/wandboard.c
>> @@ -162,13 +162,15 @@ int board_mmc_init(bd_t *bis)
>>                         gpio_direction_input(USDHC1_CD_GPIO);
>>                         break;
>>                 default:
>> -                       printf("Warning: you configured more USDHC controllers"
>> +                       printf("ERROR: you configured more USDHC controllers"
>>                                    "(%d) than supported by the board\n", i + 1);
>>                         return -EINVAL;
>>                 }
>>
>> -               if (fsl_esdhc_initialize(bis, &usdhc_cfg[i]))
>> -                       printf("Warning: failed to initialize mmc dev %d\n", i);
>> +               if (fsl_esdhc_initialize(bis, &usdhc_cfg[i])) {
>> +                       printf("ERROR: failed to initialize mmc dev %d\n", i);
>> +                       return 1;
>
> 1 is not an appropriate return value for an error.
>
> What about doing like sabrelite?
> status |= fsl_esdhc_initialize(bis, &usdhc_cfg[i]);
>
> and then you return status?

 int board_mmc_init(bd_t *bis)
 {
-       imx_iomux_v3_setup_multiple_pads(usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
+       int i;
+       int ret;

-       usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
-       usdhc_cfg[0].max_bus_width = 4;
-       gpio_direction_input(USDHC3_CD_GPIO);
+       /*
+        * According to the board_mmc_init() the following map is done:
+        * (U-boot device node)    (Physical Port)
+        * mmc0                    SOM MicroSD
+        * mmc1                    Carrier board MicroSD
+        */
+       for (i = 0; i < CONFIG_SYS_FSL_USDHC_NUM; i++) {
+               switch (i) {
+               case 0:
+                       imx_iomux_v3_setup_multiple_pads(
+                               usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
+                       usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
+                       usdhc_cfg[0].max_bus_width = 4;
+                       gpio_direction_input(USDHC3_CD_GPIO);
+                       break;
+               case 1:
+                       imx_iomux_v3_setup_multiple_pads(
+                               usdhc1_pads, ARRAY_SIZE(usdhc1_pads));
+                       usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK);
+                       usdhc_cfg[1].max_bus_width = 4;
+                       gpio_direction_input(USDHC1_CD_GPIO);
+                       break;
+               default:
+                       printf("ERROR: you configured more USDHC controllers"
+                                  "(%d) than supported by the board\n", i + 1);
+                       return -EINVAL;
+               }
+
+               ret |= fsl_esdhc_initialize(bis, &usdhc_cfg[i]);
+       }
+
+       if (ret)
+               printf("ERROR: failed to initialize mmc dev %d\n", i);

-       return fsl_esdhc_initialize(bis, &usdhc_cfg[0]);
+       return ret;
 }

This would be the final patch than...

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Wolfgang Denk April 16, 2013, 5:36 a.m. UTC | #3
Dear Otavio Salvador,

In message <CAP9ODKqO8V1-WYZQ4_GXRZ5DJbD2xiXw59xKOzGY0ZjsUJJjXQ@mail.gmail.com> you wrote:
>
> +               default:
> +                       printf("ERROR: you configured more USDHC controllers"
> +                                  "(%d) than supported by the board\n", i + 1);
> +                       return -EINVAL;
> +               }

Can you please make this a compile time test and error?

Thanks.

Wolfgang Denk
Otavio Salvador April 17, 2013, 10:21 p.m. UTC | #4
On Tue, Apr 16, 2013 at 2:36 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Otavio Salvador,
>
> In message <CAP9ODKqO8V1-WYZQ4_GXRZ5DJbD2xiXw59xKOzGY0ZjsUJJjXQ@mail.gmail.com> you wrote:
>>
>> +               default:
>> +                       printf("ERROR: you configured more USDHC controllers"
>> +                                  "(%d) than supported by the board\n", i + 1);
>> +                       return -EINVAL;
>> +               }
>
> Can you please make this a compile time test and error?

I will check how to do that.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Otavio Salvador April 19, 2013, 1:26 p.m. UTC | #5
On Wed, Apr 17, 2013 at 7:21 PM, Otavio Salvador
<otavio@ossystems.com.br> wrote:
> On Tue, Apr 16, 2013 at 2:36 AM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Otavio Salvador,
>>
>> In message <CAP9ODKqO8V1-WYZQ4_GXRZ5DJbD2xiXw59xKOzGY0ZjsUJJjXQ@mail.gmail.com> you wrote:
>>>
>>> +               default:
>>> +                       printf("ERROR: you configured more USDHC controllers"
>>> +                                  "(%d) than supported by the board\n", i + 1);
>>> +                       return -EINVAL;
>>> +               }
>>
>> Can you please make this a compile time test and error?
>
> I will check how to do that.

I checked and it will be a much bigger change as it should be done in
all boards using this so I will look at it but won't hold the patches
for now.

I will send a new revision without this change and work on this.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
diff mbox

Patch

--- a/board/wandboard/wandboard.c
+++ b/board/wandboard/wandboard.c
@@ -162,13 +162,15 @@  int board_mmc_init(bd_t *bis)
                        gpio_direction_input(USDHC1_CD_GPIO);
                        break;
                default:
-                       printf("Warning: you configured more USDHC controllers"
+                       printf("ERROR: you configured more USDHC controllers"
                                   "(%d) than supported by the board\n", i + 1);
                        return -EINVAL;
                }

-               if (fsl_esdhc_initialize(bis, &usdhc_cfg[i]))
-                       printf("Warning: failed to initialize mmc dev %d\n", i);
+               if (fsl_esdhc_initialize(bis, &usdhc_cfg[i])) {
+                       printf("ERROR: failed to initialize mmc dev %d\n", i);
+                       return 1;
+               }
        }

Like this?