Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 28 Jan 2018 14:09:27 -0800
From:      Oleksandr Tymoshenko <gonzo@bluezbox.com>
To:        "Klaus P. Ohrhallinger" <k@7he.at>
Cc:        freebsd-arm@freebsd.org
Subject:   Re: RPI3 / bcm2835 sdhci device driver
Message-ID:  <20180128220927.GA56771@bluezbox.com>
In-Reply-To: <6fc103a5-6634-8cc0-34d9-702b4484c2e0@7he.at>
References:  <6fc103a5-6634-8cc0-34d9-702b4484c2e0@7he.at>

next in thread | previous in thread | raw e-mail | index | archive | help
Klaus P. Ohrhallinger (k@7he.at) wrote:
> Hello,
> 
> This is a device driver for the second SDHCI on Raspberry Pi 3:
> 
> http://kweb.7he.at/tmp/rpi/sdhci0-01.diff
> 
> It allows the Arasan SDHCI to be used for other purposes, e.g. driving
> the radio (wlan) module.

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

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

- "+#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)

- 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.

-- 
gonzo



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180128220927.GA56771>