[U-Boot] [PATCH v2 2/2] mmc: Add bcm2835 sdhost controller

Tue Jan 23 16:03:16 UTC 2018

On 01/18/2018 05:41 AM, Jaehoon Chung wrote:
> On 01/18/2018 08:33 AM, Alexander Graf wrote:
>> The BCM2835 family of SoCs has 2 different SD controllers: One based on
>> the SDHCI spec and a custom, home-grown one.
>>
>> This patch implements a driver for the latter based on the Linux driver.
>> This is needed so that we can make use of device trees that assume driver
>> presence of both SD controllers.
>>
>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>
>> ---
>>
>> v1 -> v2:
>>
>>    - Remove hand written pinctrl support
>>    - Checkpatch fixes
>> ---
>>   MAINTAINERS                  |   1 +
>>   drivers/mmc/Kconfig          |  14 +
>>   drivers/mmc/Makefile         |   1 +
>>   drivers/mmc/bcm2835_sdhost.c | 994 +++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 1010 insertions(+)
>>   create mode 100644 drivers/mmc/bcm2835_sdhost.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1f2545191b..728d38aebf 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -97,6 +97,7 @@ S:	Orphaned (Since 2017-07)
>>   F:	arch/arm/mach-bcm283x/
>>   F:	drivers/gpio/bcm2835_gpio.c
>>   F:	drivers/mmc/bcm2835_sdhci.c
>> +F:	drivers/mmc/bcm2835_sdhost.c
>>   F:	drivers/serial/serial_bcm283x_mu.c
>>   F:	drivers/video/bcm2835.c
>>   F:	include/dm/platform_data/serial_bcm283x_mu.h
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index ab0627a8af..9b90db908b 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -256,6 +256,20 @@ config MMC_UNIPHIER
>>   	  This selects support for the Matsushita SD/MMC Host Controller on
>>   	  SocioNext UniPhier and Renesas RCar SoCs.
>>   
>> +config MMC_BCM2835
>> +	bool "BCM2835 family custom SD/MMC Host Controller support"
>> +	depends on ARCH_BCM283X
>> +	depends on BLK && DM_MMC
>> +	depends on OF_CONTROL
>> +	default y
>> +	help
>> +	  This selects support for the custom SD host controller in the BCM2835
>> +	  family of devices.
>> +
>> +	  If you have a BCM2835 platform with SD or MMC devices, say Y here.
>> +
>> +	  If unsure, say N.
>> +
>>   config MMC_SANDBOX
>>   	bool "Sandbox MMC support"
>>   	depends on SANDBOX
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 64b6f21c61..42113e2603 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -64,3 +64,4 @@ obj-$(CONFIG_MMC_SDHCI_ZYNQ)		+= zynq_sdhci.o
>>   
>>   obj-$(CONFIG_MMC_SUNXI)			+= sunxi_mmc.o
>>   obj-$(CONFIG_MMC_UNIPHIER)		+= uniphier-sd.o
>> +obj-$(CONFIG_MMC_BCM2835)		+= bcm2835_sdhost.o
>> diff --git a/drivers/mmc/bcm2835_sdhost.c b/drivers/mmc/bcm2835_sdhost.c
>> new file mode 100644
>> index 0000000000..ad9c7adb5a
>> --- /dev/null
>> +++ b/drivers/mmc/bcm2835_sdhost.c
>> @@ -0,0 +1,994 @@
>> +/*
>> + * bcm2835 sdhost driver.
>> + *
>> + * The 2835 has two SD controllers: The Arasan sdhci controller
>> + * (supported by the iproc driver) and a custom sdhost controller
>> + * (supported by this driver).
>> + *
>> + * The sdhci controller supports both sdcard and sdio.  The sdhost
>> + * controller supports the sdcard only, but has better performance.
>> + * Also note that the rpi3 has sdio wifi, so driving the sdcard with
>> + * the sdhost controller allows to use the sdhci controller for wifi
>> + * support.
>> + *
>> + * The configuration is done by devicetree via pin muxing.  Both
>> + * SD controller are available on the same pins (2 pin groups = pin 22
>> + * to 27 + pin 48 to 53).  So it's possible to use both SD controllers
>> + * at the same time with different pin groups.
>> + *
>> + * This code was ported to U-Boot by
>> + *  Alexander Graf <agraf at suse.de>
>> + * and is based on drivers/mmc/host/bcm2835.c in Linux which is written by
>> + *  Phil Elwell <phil at raspberrypi.org>
>> + *  Copyright (C) 2015-2016 Raspberry Pi (Trading) Ltd.
>> + * which is based on
>> + *  mmc-bcm2835.c by Gellert Weisz
>> + * which is, in turn, based on
>> + *  sdhci-bcm2708.c by Broadcom
>> + *  sdhci-bcm2835.c by Stephen Warren and Oleksandr Tymoshenko
>> + *  sdhci.c and sdhci-pci.c by Pierre Ossman
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> Can be replaced to SPDX-License-Identifier?
>
>

Thanks a lot for the very in-depth review. I think I've fixed most of 
your comments :).

[...]

>> +
>> +	if (!host->data)
>> +		return 0;
>> +	if (intmask & (SDHSTS_CRC16_ERROR | SDHSTS_FIFO_ERROR))
>> +		r = -EILSEQ;
>> +	if (intmask & SDHSTS_REW_TIME_OUT)
>> +		r = -ETIMEDOUT;
>> +
>> +	if (r)
>> +		printf("%s:%d %d\n", __func__, __LINE__, r);
>> +
>> +	return r;
>> +}
>> +
>> +static void bcm2835_busy_irq(struct bcm2835_host *host)
>> +{
>> +	if (WARN_ON(!host->cmd)) {
>> +		bcm2835_dumpregs(host);
>> +		return;
>> +	}
>> +
>> +	if (WARN_ON(!host->use_busy)) {
>> +		bcm2835_dumpregs(host);
>> +		return;
>> +	}
> Really need to use WARN_ON? for what?

We shouldn't get here without use_busy set. If we did, something went 
terribly wrong. I guess the alternative is an assert(), but that only 
runs when debug is defined.

Alex