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