SD card driver for Freescale i.MX6 in Genode

Martin Stein martin.stein at ...1...
Tue Mar 24 14:50:05 CET 2015


Hi Praveen,

Thank you for taking the time to implement the suggestions :)

On 24.03.2015 08:33, Praveen B wrote:
> 
> The reason we created another sd_card.h is, USDHC returns CSD register
> shifted by 8 bits. That could also have been handled in the read_csd()
> function of the controller instead of sd_card.h. We have made that
> change and added a new commit. 

We've had a similar problem at the eSDHC of the i.MX53 and found that it
didn't apply only to the CSD but to 136-bit responses in general. Thus
we went a step further and implemented the decoding directly in the
declaration of the response registers. If you're interested, you can
find this approach on our staging branch [1] in the eSDHC driver [2]
(Cmdrsp0..3 provide the raw response data while Rsp136_0..3 combine them
in a way that decoding is done automatically by the register framework).

> 
> We have also added Set_blocklen() command to sd_card.h. We currently see
> that the change is present in the staging branch of Genode (for FS
> iMX53) and does not collide with any other sd_card driver implementation.

It's cool to see the similarities: We had to implement the Set_blocklen
command too for the eSDHC. It is now provided the same way as in your
commit on our staging branch [1] and thus wouldn't have to be added for
the uSDHC any more.

>  
> 
> 
>     * For sources you've added around the driver like
>         repos/ports-foc/run/linux_imx6.run
>         repos/os/run/part_blk_sd.run
>         repos/dde_rump/run/rump_ext2_sd.run
>     I would create dedicated commits so the driver commit really
>     concentrates on the driver and its dependencies. This way, one can add
>     and test this feature without the need for fetching such sattelite code.
> 
> 
> Thanks for the suggestions.
> I've created two different commits, please take a look and let us know
> if they look fine.

Despite some minor style fixes, the commit looks fine to me. However,
when contributing to our public branch, two rather bureaucratic issues
regarding the file and commit headers will come up:

* First, as the contributors agreement behind the code refers to the IIT
Madras, it would be good for us to have an according connection in the
commit messages. Hence, most appreciated would be if you use an
IIT-Madras mail address for authoring instead of the current Gmail
address. Alternatively, an additional line in the commit messages like
"Contributed by the Department of Computer Sience & Engg., IIT Madras."
would be fine too.

* Second, the date in the copyright headers of the new files should be
changed to 2015. I hope this is OK for you.

Last but not least, as I tried to re-base your code I stumbled over a
problem not related to the uSDHC but the i.MX6 support it relies on. The
according patch is distributed over several commits with unrelated
commits in between. This complicates handling of the patch in general.
The commits should be combined into a single one before being
contributed. If you agree, I would do this as I aim to merge our local
CuBox-i and WandBoard support with your i.MX6 support anyway.

Best regards,
Martin

[1] https://github.com/genodelabs/genode/tree/staging
[2] os/src/drivers/sd_card/imx53/esdhcv2.h




More information about the users mailing list