RPI3 / bcm2835 sdhci device driver
Klaus P. Ohrhallinger
k at 7he.at
Fri Feb 2 11:11:38 UTC 2018
On 28.01.2018 23:09, Oleksandr Tymoshenko wrote:
Hello,
>
> Hi Klaus,
>
> Thanks a lot for working on this. You might consider using
> https://reviews.freebsd.org for patch submissions. Its UI is not
> exactly the paramaunt of usability but still beats posting comments
> in emails. Also there is command-line cient for patch management
> that is pretty usable: https://wiki.freebsd.org/Phabricator
There it is:
https://reviews.freebsd.org/D14168
>
> I have several comments on current version of the driver:
>
> - Looks like Linux and official name for the driver is sdhost,
> as compatible string suggest, so I suggest renaming it to
> bcm2835_sdhost. It's more distinctive from sdhci comparing to
> sdhci0
Renaming done.
>
> - "+#define DEBUG 0". DEBUG flag is set using "options DEBUG" in
> kernel config, so overriding it introduces side-effects. If you
> need internal flag for development use prefixed version like
> SDHOST_DEBUG. Also I'd suggest against using numeric values to
> indicate boolean conditions in preprocessor directives. Common
> expectation is to signal such conditions using #define FOO and
> to check them using #ifdef FOO or #if defined(FOO)
Renamed to SDHOST_DEBUG and not using numeric values anymore.
>
> - bcm2835_gpio_dev + bcm_gpio_set_alternate hack is bad. I
> know that there are hacks like this in other drivers but I am
> working on pinmux support for Pi and once it's done whole
> bcm_sdhci0_swap_pins can be just deleted. Pins are going to be
> muxed using information from device tree. I hope to finish it
> "real soon". I will let you know once it's done.
I know it's a bad hack and I will be happy to remove it.
The new version also contains a fix for properly waiting on erase commands.
After some more testing, the panic()s should be replaced/deleted.
It would be good if some people could test the driver with different
types and makes of sd-cards.
Also, DMA support would be nice to have, but I don't have time for that now.
Regards,
Klaus
More information about the freebsd-arm
mailing list