Driver complaint to SD Host Controller Specification 3.0

[André Marques]

Hello mudit,

Às 09:13 de 30-05-2016, Mudit Jain escreveu:
> Hi all,
>
> I have ported the code from the present FreeBSD.
>
> Link : https://github.com/spark1729/rtems-libbsd/commits/rpi_sd_card
>
> Kindly review the same and suggest changes.
>
> The commits are a bit unstructured, the second last one being a huge 
> one. I will be breaking it down into smaller and logical patches while 
> submitting.
>
> Files changed/added in the three commits are as follows :
>
> freebsd/sys/dev/ofw/ofw_bus.h
> freebsd/sys/dev/ofw/ofw_bus_if.c
> freebsd/sys/dev/ofw/ofw_bus_if.h
> freebsd/sys/dev/ofw/ofw_bus_subr.c
> freebsd/sys/dev/ofw/ofw_bus_subr.h
>
> freebsd/sys/sys/queue.h
>
> freebsd/sys/contrib/libfdt/libfdt_env.h
> freebsd/sys/dev/fdt/fdt_common.c
> freebsd/sys/dev/fdt/fdt_common.h
> freebsd/sys/dev/ofw/ofw_bus_subr.c
> freebsd/sys/sys/slicer.h
>
> freebsd/sys/dev/mbox/mbox_if.c
> freebsd/sys/dev/mbox/mbox_if.h
> freebsd/sys/dev/ofw/ofw_if.c
> freebsd/sys/dev/ofw/ofw_if.h
> freebsd/sys/dev/ofw/ofwvar.h
> freebsd/sys/dev/ofw/openfirm.c
> freebsd/sys/dev/ofw/openfirm.h
> freebsd/sys/dev/sdhci/bcm2835_dma.c
> freebsd/sys/dev/sdhci/bcm2835_dma.h
> freebsd/sys/dev/sdhci/bcm2835_mbox.c
> freebsd/sys/dev/sdhci/bcm2835_mbox.h
> freebsd/sys/dev/sdhci/bcm2835_mbox_prop.h
> freebsd/sys/dev/sdhci/bcm2835_sdhci.c
> freebsd/sys/dev/sdhci/bcm2835_vcbus.h
> freebsd/sys/dev/sdhci/sdhci.c
> freebsd/sys/dev/sdhci/sdhci.h
> freebsd/sys/dev/sdhci/sdhci_if.c
> freebsd/sys/dev/sdhci/sdhci_if.h
>
> Files added to the build process [ libbsd_waf.py ]
>
> freebsd/sys/dev/ofw/ofw_bus_if.c
> freebsd/sys/dev/ofw/ofw_bus_subr.c
> freebsd/sys/dev/fdt/fdt_common.c
> freebsd/sys/dev/ofw/ofw_bus_subr.c
> freebsd/sys/dev/mbox/mbox_if.c
> freebsd/sys/dev/ofw/ofw_if.c
> freebsd/sys/dev/ofw/openfirm.c
> freebsd/sys/dev/sdhci/bcm2835_dma.c
> freebsd/sys/dev/sdhci/bcm2835_mbox.c
> freebsd/sys/dev/sdhci/bcm2835_sdhci.c
> freebsd/sys/dev/sdhci/sdhci.c
> freebsd/sys/dev/sdhci/sdhci_if.c
>
> Errors and how they were resolved :
> 1.  The *_if.m files were converted into *_if.c and *_if.h using the 
> following script : Link 
> <http://opensource.apple.com//source/smb/smb-66/kernel/sys5/kern/makeobjops.pl?txt>
> 2.  fdt and ofw support was added as the driver had dependencies on that.
> 3. /taskqueue_swi_giant/ global is not provided by RTEMS. That was 
> solved by adding a new field to the /sdhci slot/ structure - /sdhci_tq/.
> 4.  BUS_PHYSADDR used in bcm_sdhci.c is a macro defined in 
> arm/include/bus.h. This macro is copied bcm_sdhci.c. /
>
> /
> Presently the SD driver module is built from bcm_sdhci.c using the 
> generic and other functions defined in sdhci.c. However for modularity 
> a different
>
> Question :
> 1. Presently I have just kept the bcm2835 specific code in /dev/sdhci/ 
> directory. I don't think that would be ideal, I wanted to discuss the 
> positioning of the bcm2835 specific files.
> 2. Presently the SD driver module is built from bcm_sdhci.c using the 
> generic and other functions defined in sdhci.c. Is this fine ?
> 3. I have used the /.start_actual /in nexus-devices.h for /rpi_dma_res 
> /as/0x7e007000. /Is this the correct value ?Link 
> <https://www.kernel.org/doc/Documentation/devicetree/bindings/dma/brcm,bcm2835-dma.txt>
> Note : I have not added the macro LIBBSP_ARM_RASPBERRYPI_BSP_H to 
> bcm2835 specific code. I will be doing that.
>
> waf builds everything that was added successfully.
> /
> /
> Thanks
> Mudit
>

Hello Mudit,

My feedback from a quick skim through your code:

There is already some mailbox support from Yang Quiao's last year GSOC, 
so if anything is missing from his work please improve it.

The ofw bus is not necessary for the sdhci driver in RTEMS, so that can 
be left out.

As Sebastian suggested, you need to follow the FreeBSD port guidelines, 
and also do not replace entire files (e.g..: sdhci.c and sdhci.h).

--André Marques

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20160601/39864648/attachment-0002.html>