[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Stephen Warren]

This adds a simple driver for the BCM2835's SD controller.

Workarounds are implemented for:
* Register writes can't be too close to each-other in time, or they will
  be lost.
* Register accesses must all be 32-bit, so implement custom accessors.

This code was extracted from:
git://github.com/gonzoua/u-boot-pi.git master
which was created by Oleksandr Tymoshenko.

Portions of the code there were obviously based on the Linux kernel at:
git://github.com/raspberrypi/linux.git rpi-3.2.27

No s-o-b tags were present in either location.

swarren changed the following for upstream:
* Removed hack udelay()s in bcm2835_sdhci_raw_writel(); setting
  SDHCI_QUIRK_WAIT_SEND_CMD appears to solve the issues.
* Remove register logging from read*/write* functions.
* Sort out confusion with min/max_freq values passed to add_sdhci().
* Simplified and commented twoticks_delay calculation.
* checkpatch fixes.

Cc: Oleksandr Tymoshenko <gonzo@bluezbox.com>
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
This series is based on the previous bcm2835 patches I sent, to add the
mbox and video drivers. Patch 1 should be independant, but patch 2 depends
on those other patches.

 arch/arm/include/asm/arch-bcm2835/sdhci.h |   24 ++++
 drivers/mmc/Makefile                      |    1 +
 drivers/mmc/bcm2835_sdhci.c               |  172 +++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-bcm2835/sdhci.h
 create mode 100644 drivers/mmc/bcm2835_sdhci.c

diff --git a/arch/arm/include/asm/arch-bcm2835/sdhci.h b/arch/arm/include/asm/arch-bcm2835/sdhci.h
new file mode 100644
index 0000000..a4f867b
--- /dev/null
+++ b/arch/arm/include/asm/arch-bcm2835/sdhci.h
@@ -0,0 +1,24 @@
+/*
+ * (C) Copyright 2012 Stephen Warren
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#ifndef _BCM2835_SDHCI_H_
+#define _BCM2835_SDHCI_H_
+
+#define BCM2835_SDHCI_BASE 0x20300000
+
+int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq);
+
+#endif
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index a1dd730..6be36f2 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -43,6 +43,7 @@ COBJS-$(CONFIG_MXS_MMC) += mxsmmc.o
 COBJS-$(CONFIG_OMAP_HSMMC) += omap_hsmmc.o
 COBJS-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
 COBJS-$(CONFIG_SDHCI) += sdhci.o
+COBJS-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o
 COBJS-$(CONFIG_S5P_SDHCI) += s5p_sdhci.o
 COBJS-$(CONFIG_SH_MMCIF) += sh_mmcif.o
 COBJS-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
new file mode 100644
index 0000000..1d623c0
--- /dev/null
+++ b/drivers/mmc/bcm2835_sdhci.c
@@ -0,0 +1,172 @@
+/*
+ * This code was extracted from:
+ * git://github.com/gonzoua/u-boot-pi.git master
+ * and hence presumably (C) 2012 Oleksandr Tymoshenko
+ *
+ * Tweaks for U-Boot upstreaming
+ * (C) 2012 Stephen Warren
+ *
+ * Portions (e.g. read/write macros, concepts for back-to-back register write
+ * timing workarounds) obviously extracted from the Linux kernel at:
+ * https://github.com/raspberrypi/linux.git rpi-3.2.27
+ *
+ * The Linux kernel code has the following (c) and license, which is hence
+ * propagated to Oleksandr's tree and here:
+ *
+ * Support for SDHCI device on 2835
+ * Based on sdhci-bcm2708.c (c) 2010 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+/* Supports:
+ * SDHCI platform device - Arasan SD controller in BCM2708
+ *
+ * Inspired by sdhci-pci.c, by Pierre Ossman
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <sdhci.h>
+
+/* 400KHz is max freq for card ID etc. Use that as min */
+#define MIN_FREQ 400000
+
+static uint twoticks_delay;
+
+static inline void bcm2835_sdhci_raw_writel(struct sdhci_host *host, u32 val,
+						int reg)
+{
+	static ulong last_write;
+
+	/*
+	 * The Arasan has a bugette whereby it may lose the content of
+	 * successive writes to registers that are within two SD-card clock
+	 * cycles of each other (a clock domain crossing problem).
+	 * It seems, however, that the data register does not have this problem.
+	 * (Which is just as well - otherwise we'd have to nobble the DMA engine
+	 * too)
+	 */
+	while (get_timer(last_write) < twoticks_delay)
+		;
+
+	writel(val, host->ioaddr + reg);
+	last_write = get_timer(0);
+}
+
+static inline u32 bcm2835_sdhci_raw_readl(struct sdhci_host *host, int reg)
+{
+	return readl(host->ioaddr + reg);
+}
+
+static void bcm2835_sdhci_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	bcm2835_sdhci_raw_writel(host, val, reg);
+}
+
+static void bcm2835_sdhci_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	static u32 shadow;
+
+	u32 p = reg == SDHCI_COMMAND ? shadow :
+		bcm2835_sdhci_raw_readl(host, reg & ~3);
+	u32 s = reg << 3 & 0x18;
+	u32 l = val << s;
+	u32 m = 0xffff << s;
+
+	if (reg == SDHCI_TRANSFER_MODE)
+		shadow = (p & ~m) | l;
+	else
+		bcm2835_sdhci_raw_writel(host, (p & ~m) | l, reg & ~3);
+}
+
+static void bcm2835_sdhci_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	u32 p = bcm2835_sdhci_raw_readl(host, reg & ~3);
+	u32 s = reg << 3 & 0x18;
+	u32 l = val << s;
+	u32 m = 0xff << s;
+
+	bcm2835_sdhci_raw_writel(host, (p & ~m) | l, reg & ~3);
+}
+
+static u32 bcm2835_sdhci_readl(struct sdhci_host *host, int reg)
+{
+	u32 val = bcm2835_sdhci_raw_readl(host, reg);
+
+	return val;
+}
+
+static u16 bcm2835_sdhci_readw(struct sdhci_host *host, int reg)
+{
+	u32 val = bcm2835_sdhci_raw_readl(host, (reg & ~3));
+	val = val >> (reg << 3 & 0x18) & 0xffff;
+
+	return (u16)val;
+}
+
+static u8 bcm2835_sdhci_readb(struct sdhci_host *host, int reg)
+{
+	u32 val = bcm2835_sdhci_raw_readl(host, (reg & ~3));
+	val = val >> (reg << 3 & 0x18) & 0xff;
+
+	return (u8)val;
+}
+
+static const struct sdhci_ops bcm2835_ops = {
+	.write_l = bcm2835_sdhci_writel,
+	.write_w = bcm2835_sdhci_writew,
+	.write_b = bcm2835_sdhci_writeb,
+	.read_l = bcm2835_sdhci_readl,
+	.read_w = bcm2835_sdhci_readw,
+	.read_b = bcm2835_sdhci_readb,
+};
+
+int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq)
+{
+	struct sdhci_host *host = NULL;
+
+	/*
+	 * See the comments in bcm2835_sdhci_raw_writel().
+	 *
+	 * This should probably be dynamically calculated based on the actual
+	 * frequency. However, this is the longest we'll have to wait, and
+	 * doesn't seem to slow access down too much, so the added complexity
+	 * doesn't seem worth it for now.
+	 *
+	 * 1/MIN_FREQ is (max) time per tick of eMMC clock.
+	 * 2/MIN_FREQ is time for two ticks.
+	 * Multiply by 1000000 to get uS per two ticks.
+	 * +1 for hack rounding.
+	 */
+	twoticks_delay = ((2 * 1000000) / MIN_FREQ) + 1;
+
+	host = malloc(sizeof(struct sdhci_host));
+	if (!host) {
+		printf("sdhci_host malloc fail!\n");
+		return 1;
+	}
+
+	host->name = "bcm2835_sdhci";
+	host->ioaddr = (void *)regbase;
+	host->quirks = SDHCI_QUIRK_BROKEN_VOLTAGE | SDHCI_QUIRK_BROKEN_R1B |
+		SDHCI_QUIRK_WAIT_SEND_CMD;
+	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
+	host->ops = &bcm2835_ops;
+
+	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
+	add_sdhci(host, emmc_freq, MIN_FREQ);
+
+	return 0;
+}
-- 
1.7.9.5

[U-Boot] [PATCH 2/2] ARM: rpi_b: enable SD controller, add related env/cmds

[Stephen Warren]

Enable the SD controller driver for the Raspberry Pi. Enable a number
of useful MMC, partition, and filesystem-related commands. Set up the
environment to provide standard locations for loading a kernel, DTB,
etc. Provide a boot command that loads and executes boot.scr.uimg from
the SD card; this is written considering future extensibilty to USB
storage.

Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
 arch/arm/include/asm/arch-bcm2835/mbox.h |   26 ++++++++++++
 board/raspberrypi/rpi_b/rpi_b.c          |   26 ++++++++++++
 include/configs/rpi_b.h                  |   68 ++++++++++++++++++++++++++++--
 3 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/arch-bcm2835/mbox.h b/arch/arm/include/asm/arch-bcm2835/mbox.h
index 6e040c5..5a9bd87 100644
--- a/arch/arm/include/asm/arch-bcm2835/mbox.h
+++ b/arch/arm/include/asm/arch-bcm2835/mbox.h
@@ -143,6 +143,32 @@ struct bcm2835_mbox_tag_get_arm_mem {
 	} body;
 };
 
+#define BCM2835_MBOX_TAG_GET_CLOCK_RATE	0x00030002
+
+#define BCM2835_MBOX_CLOCK_ID_EMMC	1
+#define BCM2835_MBOX_CLOCK_ID_UART	2
+#define BCM2835_MBOX_CLOCK_ID_ARM	3
+#define BCM2835_MBOX_CLOCK_ID_CORE	4
+#define BCM2835_MBOX_CLOCK_ID_V3D	5
+#define BCM2835_MBOX_CLOCK_ID_H264	6
+#define BCM2835_MBOX_CLOCK_ID_ISP	7
+#define BCM2835_MBOX_CLOCK_ID_SDRAM	8
+#define BCM2835_MBOX_CLOCK_ID_PIXEL	9
+#define BCM2835_MBOX_CLOCK_ID_PWM	10
+
+struct bcm2835_mbox_tag_get_clock_rate {
+	struct bcm2835_mbox_tag_hdr tag_hdr;
+	union {
+		struct {
+			u32 clock_id;
+		} req;
+		struct {
+			u32 clock_id;
+			u32 rate_hz;
+		} resp;
+	} body;
+};
+
 #define BCM2835_MBOX_TAG_ALLOCATE_BUFFER	0x00040001
 
 struct bcm2835_mbox_tag_allocate_buffer {
diff --git a/board/raspberrypi/rpi_b/rpi_b.c b/board/raspberrypi/rpi_b/rpi_b.c
index 3c654a1..6b3e095 100644
--- a/board/raspberrypi/rpi_b/rpi_b.c
+++ b/board/raspberrypi/rpi_b/rpi_b.c
@@ -16,6 +16,7 @@
 
 #include <common.h>
 #include <asm/arch/mbox.h>
+#include <asm/arch/sdhci.h>
 #include <asm/global_data.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -26,6 +27,12 @@ struct msg_get_arm_mem {
 	u32 end_tag;
 };
 
+struct msg_get_clock_rate {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_get_clock_rate get_clock_rate;
+	u32 end_tag;
+};
+
 int dram_init(void)
 {
 	ALLOC_ALIGN_BUFFER(struct msg_get_arm_mem, msg, 1, 16);
@@ -51,3 +58,22 @@ int board_init(void)
 
 	return 0;
 }
+
+int board_mmc_init(void)
+{
+	ALLOC_ALIGN_BUFFER(struct msg_get_clock_rate, msg_clk, 1, 16);
+	int ret;
+
+	BCM2835_MBOX_INIT_HDR(msg_clk);
+	BCM2835_MBOX_INIT_TAG(&msg_clk->get_clock_rate, GET_CLOCK_RATE);
+	msg_clk->get_clock_rate.body.req.clock_id = BCM2835_MBOX_CLOCK_ID_EMMC;
+
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_clk->hdr);
+	if (ret) {
+		printf("bcm2835: Could not query eMMC clock rate\n");
+		return -1;
+	}
+
+	return bcm2835_sdhci_init(BCM2835_SDHCI_BASE,
+				  msg_clk->get_clock_rate.body.resp.rate_hz);
+}
diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h
index a709feb..4d834e1 100644
--- a/include/configs/rpi_b.h
+++ b/include/configs/rpi_b.h
@@ -50,6 +50,7 @@
 #define CONFIG_SYS_MALLOC_LEN		SZ_4M
 #define CONFIG_SYS_MEMTEST_START	0x00100000
 #define CONFIG_SYS_MEMTEST_END		0x00200000
+#define CONFIG_LOADADDR			0x00200000
 
 /* Flash */
 #define CONFIG_SYS_NO_FLASH
@@ -69,6 +70,13 @@
 #define CONFIG_VIDEO_BCM2835
 #define CONFIG_SYS_WHITE_ON_BLACK
 
+/* SD/MMC configuration */
+#define CONFIG_GENERIC_MMC
+#define CONFIG_MMC
+#define CONFIG_SDHCI
+#define CONFIG_MMC_SDHCI_IO_ACCESSORS
+#define CONFIG_BCM2835_SDHCI
+
 /* Console UART */
 #define CONFIG_PL011_SERIAL
 #define CONFIG_PL011_CLOCK		3000000
@@ -84,12 +92,59 @@
 /* Environment */
 #define CONFIG_ENV_SIZE			SZ_16K
 #define CONFIG_ENV_IS_NOWHERE
+#define CONFIG_ENV_VARS_UBOOT_CONFIG
 #define CONFIG_SYS_LOAD_ADDR		0x1000000
 #define CONFIG_CONSOLE_MUX
 #define CONFIG_SYS_CONSOLE_IS_IN_ENV
-#define CONFIG_EXTRA_ENV_SETTINGS	"stdin=serial\0" \
-					"stderr=serial,lcd\0" \
-					"stdout=serial,lcd\0"
+/*
+ * Memory layout for where various images get loaded by boot scripts:
+ *
+ * scriptaddr can be pretty much anywhere that doesn't conflict with something
+ *   else. Put it low in memory to avoid conflicts.
+ *
+ * kernel_addr_r must be within the first 128M of RAM in order for the
+ *   kernel's CONFIG_AUTO_ZRELADDR option to work. Since the kernel will
+ *   decompress itself to 0x8000 after the start of RAM, kernel_addr_r
+ *   should not overlap that area, or the kernel will have to copy itself
+ *   somewhere else before decompression. Similarly, the address of any other
+ *   data passed to the kernel shouldn't overlap the start of RAM. Pushing
+ *   this up to 16M allows for a sizable kernel to be decompressed below the
+ *   compressed load address.
+ *
+ * fdt_addr_r simply shouldn't overlap anything else. Choosing 32M allows for
+ *   the compressed kernel to be up to 16M too.
+ *
+ * ramdisk_addr_r simply shouldn't overlap anything else. Choosing 33M allows
+ *   for the FDT/DTB to be up to 1M, which is hopefully plenty.
+ */
+#define CONFIG_EXTRA_ENV_SETTINGS \
+	"stdin=serial\0" \
+	"stderr=serial,lcd\0" \
+	"stdout=serial,lcd\0" \
+	"scriptaddr=0x00000000\0" \
+	"kernel_addr_r=0x01000000\0" \
+	"fdt_addr_r=0x02000000\0" \
+	"ramdisk_addr_r=0x02100000\0" \
+	"boot_targets=mmc0\0" \
+	\
+	"script_boot=" \
+		"if fatload ${devtype} ${devnum}:1 " \
+					"${scriptaddr} boot.scr.uimg; then " \
+			"source ${scriptaddr}; " \
+		"fi;\0" \
+	\
+	"mmc_boot=" \
+		"setenv devtype mmc; " \
+		"if mmc dev ${devnum}; then " \
+			"run script_boot; " \
+		"fi\0" \
+	\
+	"bootcmd_mmc0=setenv devnum 0; run mmc_boot\0" \
+
+#define CONFIG_BOOTCOMMAND \
+	"for target in ${boot_targets}; do run bootcmd_${target}; done"
+
+#define CONFIG_BOOTDELAY		2
 
 /* Shell */
 #define CONFIG_SYS_HUSH_PARSER
@@ -104,6 +159,13 @@
 #include <config_cmd_default.h>
 #define CONFIG_CMD_BOOTZ
 #define CONFIG_CMD_GPIO
+#define CONFIG_CMD_MMC
+#define CONFIG_DOS_PARTITION
+#define CONFIG_PARTITION_UUIDS
+#define CONFIG_CMD_PART
+#define CONFIG_CMD_FS_GENERIC
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT
 /* Some things don't make sense on this HW or yet */
 #undef CONFIG_CMD_FPGA
 #undef CONFIG_CMD_NET
-- 
1.7.9.5

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Vikram Narayanan]

Some nitpicks.

On 10/24/2012 10:20 AM, Stephen Warren wrote:
> This adds a simple driver for the BCM2835's SD controller.
>
> Workarounds are implemented for:
> * Register writes can't be too close to each-other in time, or they will
>    be lost.
> * Register accesses must all be 32-bit, so implement custom accessors.
>
> This code was extracted from:
> git://github.com/gonzoua/u-boot-pi.git master
> which was created by Oleksandr Tymoshenko.
>
> Portions of the code there were obviously based on the Linux kernel at:
> git://github.com/raspberrypi/linux.git rpi-3.2.27
>
> No s-o-b tags were present in either location.
>
> swarren changed the following for upstream:
> * Removed hack udelay()s in bcm2835_sdhci_raw_writel(); setting
>    SDHCI_QUIRK_WAIT_SEND_CMD appears to solve the issues.
> * Remove register logging from read*/write* functions.
> * Sort out confusion with min/max_freq values passed to add_sdhci().
> * Simplified and commented twoticks_delay calculation.
> * checkpatch fixes.
>
> Cc: Oleksandr Tymoshenko<gonzo@bluezbox.com>
> Signed-off-by: Stephen Warren<swarren@wwwdotorg.org>
> ---
> This series is based on the previous bcm2835 patches I sent, to add the
> mbox and video drivers. Patch 1 should be independant, but patch 2 depends
> on those other patches.
>
>   arch/arm/include/asm/arch-bcm2835/sdhci.h |   24 ++++
>   drivers/mmc/Makefile                      |    1 +
>   drivers/mmc/bcm2835_sdhci.c               |  172 +++++++++++++++++++++++++++++
>   3 files changed, 197 insertions(+)
>   create mode 100644 arch/arm/include/asm/arch-bcm2835/sdhci.h
>   create mode 100644 drivers/mmc/bcm2835_sdhci.c
>
> diff --git a/arch/arm/include/asm/arch-bcm2835/sdhci.h b/arch/arm/include/asm/arch-bcm2835/sdhci.h
> new file mode 100644
> index 0000000..a4f867b
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-bcm2835/sdhci.h
> @@ -0,0 +1,24 @@
> +/*
> + * (C) Copyright 2012 Stephen Warren
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#ifndef _BCM2835_SDHCI_H_
> +#define _BCM2835_SDHCI_H_
> +
> +#define BCM2835_SDHCI_BASE 0x20300000
> +
> +int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq);
> +
> +#endif
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index a1dd730..6be36f2 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -43,6 +43,7 @@ COBJS-$(CONFIG_MXS_MMC) += mxsmmc.o
>   COBJS-$(CONFIG_OMAP_HSMMC) += omap_hsmmc.o
>   COBJS-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>   COBJS-$(CONFIG_SDHCI) += sdhci.o
> +COBJS-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o
>   COBJS-$(CONFIG_S5P_SDHCI) += s5p_sdhci.o
>   COBJS-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>   COBJS-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
> diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
> new file mode 100644
> index 0000000..1d623c0
> --- /dev/null
> +++ b/drivers/mmc/bcm2835_sdhci.c
> @@ -0,0 +1,172 @@
> +/*
> + * This code was extracted from:
> + * git://github.com/gonzoua/u-boot-pi.git master
> + * and hence presumably (C) 2012 Oleksandr Tymoshenko
> + *
> + * Tweaks for U-Boot upstreaming
> + * (C) 2012 Stephen Warren
> + *
> + * Portions (e.g. read/write macros, concepts for back-to-back register write
> + * timing workarounds) obviously extracted from the Linux kernel at:
> + * https://github.com/raspberrypi/linux.git rpi-3.2.27
> + *
> + * The Linux kernel code has the following (c) and license, which is hence
> + * propagated to Oleksandr's tree and here:
> + *
> + * Support for SDHCI device on 2835
> + * Based on sdhci-bcm2708.c (c) 2010 Broadcom
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/* Supports:
> + * SDHCI platform device - Arasan SD controller in BCM2708
> + *
> + * Inspired by sdhci-pci.c, by Pierre Ossman
> + */
> +
> +#include<common.h>
> +#include<malloc.h>
> +#include<sdhci.h>
> +
> +/* 400KHz is max freq for card ID etc. Use that as min */
> +#define MIN_FREQ 400000
> +
> +static uint twoticks_delay;
> +
> +static inline void bcm2835_sdhci_raw_writel(struct sdhci_host *host, u32 val,
> +						int reg)
> +{
> +	static ulong last_write;
> +
> +	/*
> +	 * The Arasan has a bugette whereby it may lose the content of
> +	 * successive writes to registers that are within two SD-card clock
> +	 * cycles of each other (a clock domain crossing problem).
> +	 * It seems, however, that the data register does not have this problem.
> +	 * (Which is just as well - otherwise we'd have to nobble the DMA engine
> +	 * too)
> +	 */
> +	while (get_timer(last_write)<  twoticks_delay)
> +		;
> +
> +	writel(val, host->ioaddr + reg);
> +	last_write = get_timer(0);
> +}
> +
> +static inline u32 bcm2835_sdhci_raw_readl(struct sdhci_host *host, int reg)
> +{
> +	return readl(host->ioaddr + reg);
> +}
> +
> +static void bcm2835_sdhci_writel(struct sdhci_host *host, u32 val, int reg)
> +{
> +	bcm2835_sdhci_raw_writel(host, val, reg);
> +}
> +
> +static void bcm2835_sdhci_writew(struct sdhci_host *host, u16 val, int reg)
> +{
> +	static u32 shadow;
> +
> +	u32 p = reg == SDHCI_COMMAND ? shadow :
> +		bcm2835_sdhci_raw_readl(host, reg&  ~3);
> +	u32 s = reg<<  3&  0x18;
> +	u32 l = val<<  s;
> +	u32 m = 0xffff<<  s;
> +
> +	if (reg == SDHCI_TRANSFER_MODE)
> +		shadow = (p&  ~m) | l;
> +	else
> +		bcm2835_sdhci_raw_writel(host, (p&  ~m) | l, reg&  ~3);
> +}
> +
> +static void bcm2835_sdhci_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> +	u32 p = bcm2835_sdhci_raw_readl(host, reg&  ~3);
> +	u32 s = reg<<  3&  0x18;
> +	u32 l = val<<  s;
> +	u32 m = 0xff<<  s;
> +
> +	bcm2835_sdhci_raw_writel(host, (p&  ~m) | l, reg&  ~3);
> +}
> +
> +static u32 bcm2835_sdhci_readl(struct sdhci_host *host, int reg)
> +{
> +	u32 val = bcm2835_sdhci_raw_readl(host, reg);
> +
> +	return val;
> +}
> +
> +static u16 bcm2835_sdhci_readw(struct sdhci_host *host, int reg)
> +{
> +	u32 val = bcm2835_sdhci_raw_readl(host, (reg&  ~3));
> +	val = val>>  (reg<<  3&  0x18)&  0xffff;
> +
> +	return (u16)val;
> +}
> +
> +static u8 bcm2835_sdhci_readb(struct sdhci_host *host, int reg)
> +{
> +	u32 val = bcm2835_sdhci_raw_readl(host, (reg&  ~3));
> +	val = val>>  (reg<<  3&  0x18)&  0xff;
> +
> +	return (u8)val;
> +}

Can the above used magics be made as macros?

~Vikram

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Tom Rini]

On Tue, Oct 23, 2012 at 10:50:47PM -0600, Stephen Warren wrote:
> This adds a simple driver for the BCM2835's SD controller.
> 
> Workarounds are implemented for:
> * Register writes can't be too close to each-other in time, or they will
>   be lost.
> * Register accesses must all be 32-bit, so implement custom accessors.
> 
> This code was extracted from:
> git://github.com/gonzoua/u-boot-pi.git master
> which was created by Oleksandr Tymoshenko.
> 
> Portions of the code there were obviously based on the Linux kernel at:
> git://github.com/raspberrypi/linux.git rpi-3.2.27
> 
> No s-o-b tags were present in either location.

Um, that's a bit worrying to me.  Wolfgang?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121026/7e1b5d2c/attachment.pgp>

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Oleksandr Tymoshenko]

On 10/26/2012 10:32 AM, Tom Rini wrote:
> On Tue, Oct 23, 2012 at 10:50:47PM -0600, Stephen Warren wrote:
>> This adds a simple driver for the BCM2835's SD controller.
>>
>> Workarounds are implemented for:
>> * Register writes can't be too close to each-other in time, or they will
>>    be lost.
>> * Register accesses must all be 32-bit, so implement custom accessors.
>>
>> This code was extracted from:
>> git://github.com/gonzoua/u-boot-pi.git master
>> which was created by Oleksandr Tymoshenko.
>>
>> Portions of the code there were obviously based on the Linux kernel at:
>> git://github.com/raspberrypi/linux.git rpi-3.2.27
>>
>> No s-o-b tags were present in either location.
> Um, that's a bit worrying to me.  Wolfgang?

I didn't have much experience with s-o-b stuff, so excuse my ignorance.
Is there official procedure, or is it enough to state that I sign off on
everything in  git://github.com/gonzoua/u-boot-pi.git master
authored by me?

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/26/12 10:48, Oleksandr Tymoshenko wrote:
> On 10/26/2012 10:32 AM, Tom Rini wrote:
>> On Tue, Oct 23, 2012 at 10:50:47PM -0600, Stephen Warren wrote:
>>> This adds a simple driver for the BCM2835's SD controller.
>>> 
>>> Workarounds are implemented for: * Register writes can't be too
>>> close to each-other in time, or they will be lost. * Register
>>> accesses must all be 32-bit, so implement custom accessors.
>>> 
>>> This code was extracted from: 
>>> git://github.com/gonzoua/u-boot-pi.git master which was created
>>> by Oleksandr Tymoshenko.
>>> 
>>> Portions of the code there were obviously based on the Linux
>>> kernel at: git://github.com/raspberrypi/linux.git rpi-3.2.27
>>> 
>>> No s-o-b tags were present in either location.
>> Um, that's a bit worrying to me.  Wolfgang?
> 
> I didn't have much experience with s-o-b stuff, so excuse my
> ignorance. Is there official procedure, or is it enough to state
> that I sign off on everything in
> git://github.com/gonzoua/u-boot-pi.git master authored by me?

Just about.  Please read
http://elinux.org/Developer_Certificate_Of_Origin and then assuming
that is all true, reply to this email with a properly formatted
Signed-off-by line (and then hey, you end up in the contributors for
the next U-Boot release, once the code is merged).  And then you can
get in the habit of doing 'git commit -s' for projects where the above
applies.  There's still a question about the portions taken from the
kernel tree.  Thanks!

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQitDzAAoJENk4IS6UOR1W/K4P/1V5mFKlragudKsi4tUug2TN
ihjpQjESpEbwHCcdgK831PIpf/bd8igY+ba50aSJRSO0BrRtwiWe7whnJv5FEg01
dGBhlYlE0E+B6RcyJmORsEIftDphnY1Wrc4DmTpo18CxDJTZ4jdXBiEPygLo+L/v
IIC5xVArVXvwvhtkeMB0gO6fg9NbeC13+6TdSOm9Jn1qNyJ3U05DjIjC/h7/3+rn
GCScP3qxrnbrTF0xkoRi7LCaBZA2Czfu7n510S7SHFufNXq9Px19GOImonvanrVG
z5p9pZJolIE8Jfu6bwXrPfdu8NCo6dv1/apDigDka0jO/KGxIG6l0DPJZTjLf6G9
FJ5G5n/UE+MRheps2AAtHA/ID5nxF7t7F1pjrWw1rkY1BkGzrOf7e13SlBa3QR6j
ciLa0lWVdfb4fGU+C3On/ZksP+mMVqHZFwjzYaE7d25jOBYSc60E8O5Q0WzDS8mp
0sar4+/aRQgb3NhjIz1ADxSVrEmuwCTjtgZAwBcCLCK8OfdlYgpx40Dy8OJ7JKtP
H+6pz812kqk5fHYAmNDstdbVSX67QNV4US/euCgSspDAsBdqweDCjUosuidVLXjH
iq2XNG5HObnUY7zLlmTb2QPxBgarmakAf37jWSaIqNo9QLvzBitt1gDQUZZfjAT0
jeAfVX7FvI35pPbpEoCT
=5cJl
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Greg KH]

On Fri, Oct 26, 2012 at 10:48:06AM -0700, Oleksandr Tymoshenko wrote:
> On 10/26/2012 10:32 AM, Tom Rini wrote:
> >On Tue, Oct 23, 2012 at 10:50:47PM -0600, Stephen Warren wrote:
> >>This adds a simple driver for the BCM2835's SD controller.
> >>
> >>Workarounds are implemented for:
> >>* Register writes can't be too close to each-other in time, or they will
> >>   be lost.
> >>* Register accesses must all be 32-bit, so implement custom accessors.
> >>
> >>This code was extracted from:
> >>git://github.com/gonzoua/u-boot-pi.git master
> >>which was created by Oleksandr Tymoshenko.
> >>
> >>Portions of the code there were obviously based on the Linux kernel at:
> >>git://github.com/raspberrypi/linux.git rpi-3.2.27
> >>
> >>No s-o-b tags were present in either location.
> >Um, that's a bit worrying to me.  Wolfgang?
> 
> I didn't have much experience with s-o-b stuff, so excuse my ignorance.

Please read the file, Documentation/SubmittingPatches, in the Linux
kernel source directory, for what it means, and what it is about, before
doing anything.

thanks,

greg k-h

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Stephen Warren]

On 10/26/2012 12:05 PM, Tom Rini wrote:
> On 10/26/12 10:48, Oleksandr Tymoshenko wrote:
>> On 10/26/2012 10:32 AM, Tom Rini wrote:
>>> On Tue, Oct 23, 2012 at 10:50:47PM -0600, Stephen Warren
>>> wrote:
>>>> This adds a simple driver for the BCM2835's SD controller.
>>>> 
>>>> Workarounds are implemented for: * Register writes can't be
>>>> too close to each-other in time, or they will be lost. *
>>>> Register accesses must all be 32-bit, so implement custom
>>>> accessors.
>>>> 
>>>> This code was extracted from: 
>>>> git://github.com/gonzoua/u-boot-pi.git master which was
>>>> created by Oleksandr Tymoshenko.
>>>> 
>>>> Portions of the code there were obviously based on the Linux 
>>>> kernel at: git://github.com/raspberrypi/linux.git rpi-3.2.27
>>>> 
>>>> No s-o-b tags were present in either location.
>>> Um, that's a bit worrying to me.  Wolfgang?
> 
>> I didn't have much experience with s-o-b stuff, so excuse my 
>> ignorance. Is there official procedure, or is it enough to state 
>> that I sign off on everything in 
>> git://github.com/gonzoua/u-boot-pi.git master authored by me?
> 
> Just about.  Please read 
> http://elinux.org/Developer_Certificate_Of_Origin and then
> assuming that is all true, reply to this email with a properly
> formatted Signed-off-by line (and then hey, you end up in the
> contributors for the next U-Boot release, once the code is merged).
> And then you can get in the habit of doing 'git commit -s' for
> projects where the above applies.  There's still a question about
> the portions taken from the kernel tree.  Thanks!

Although do remember that U-Boot uses S-o-b differently to how the Linux
kernel uses it, based on our recent discussions. For U-Boot, it's
apparently only about original authorship and/or changes to the
patches, whereas for the kernel, people who handle the patch without
changes (i.e. apply it to a git tree) also sign the patch off.

Equally, since this code was derived from Linux kernel source, we should
track down who wrote that and get them to sign it off too.

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Stephen Warren]

On 10/26/2012 05:33 AM, Vikram Narayanan wrote:
> Some nitpicks.
> 
> On 10/24/2012 10:20 AM, Stephen Warren wrote:
>> This adds a simple driver for the BCM2835's SD controller.
>>
>> Workarounds are implemented for:
>> * Register writes can't be too close to each-other in time, or they will
>>    be lost.
>> * Register accesses must all be 32-bit, so implement custom accessors.
...

>> +static u8 bcm2835_sdhci_readb(struct sdhci_host *host, int reg)
>> +{
>> +    u32 val = bcm2835_sdhci_raw_readl(host, (reg&  ~3));
>> +    val = val>>  (reg<<  3&  0x18)&  0xff;
>> +
>> +    return (u8)val;
>> +}
> 
> Can the above used magics be made as macros?

This code was taken directly from the downstream Linux kernel, so I
changed it as little as possible, to make comparisons easier. Still, if
people want I can certainly make it easier to understand the expression
a bit.

I don't think the issue is the magic numbers so much as understanding
what the expression does; the magic are obvious then. It's simply
extracting byte n from from a u32. Would the following be more obvious:

byte_num = reg & 3;
byte_shift = bytenum * 8;
byte = (val >> byte_shift) & 0xff;

... and similar for the other functions?

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Vikram Narayanan]

On 10/28/2012 8:58 AM, Stephen Warren wrote:
> On 10/26/2012 05:33 AM, Vikram Narayanan wrote:
>> Some nitpicks.
>>
>> On 10/24/2012 10:20 AM, Stephen Warren wrote:
<snip>
>>> +static u8 bcm2835_sdhci_readb(struct sdhci_host *host, int reg)
>>> +{
>>> +    u32 val = bcm2835_sdhci_raw_readl(host, (reg&   ~3));
>>> +    val = val>>   (reg<<   3&   0x18)&   0xff;
>>> +
>>> +    return (u8)val;
>>> +}
>>
>> Can the above used magics be made as macros?
>
> This code was taken directly from the downstream Linux kernel, so I
> changed it as little as possible, to make comparisons easier. Still, if
> people want I can certainly make it easier to understand the expression
> a bit.

Seems reasonable.

> I don't think the issue is the magic numbers so much as understanding
> what the expression does; the magic are obvious then. It's simply
> extracting byte n from from a u32. Would the following be more obvious:
>
> byte_num = reg&  3;
> byte_shift = bytenum * 8;
> byte = (val>>  byte_shift)&  0xff;
>
> ... and similar for the other functions?

This looks better to me than the former. Thanks.

~Vikram

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Albert ARIBAUD]

Hi Stephen,

On Sun, 28 Oct 2012 22:36:25 +0530, Vikram Narayanan
<vikram186@gmail.com> wrote:

> On 10/28/2012 8:58 AM, Stephen Warren wrote:
> > On 10/26/2012 05:33 AM, Vikram Narayanan wrote:
> >> Some nitpicks.
> >>
> >> On 10/24/2012 10:20 AM, Stephen Warren wrote:
> <snip>
> >>> +static u8 bcm2835_sdhci_readb(struct sdhci_host *host, int reg)
> >>> +{
> >>> +    u32 val = bcm2835_sdhci_raw_readl(host, (reg&   ~3));
> >>> +    val = val>>   (reg<<   3&   0x18)&   0xff;
> >>> +
> >>> +    return (u8)val;
> >>> +}
> >>
> >> Can the above used magics be made as macros?
> >
> > This code was taken directly from the downstream Linux kernel, so I
> > changed it as little as possible, to make comparisons easier. Still, if
> > people want I can certainly make it easier to understand the expression
> > a bit.
> 
> Seems reasonable.
> 
> > I don't think the issue is the magic numbers so much as understanding
> > what the expression does; the magic are obvious then. It's simply
> > extracting byte n from from a u32. Would the following be more obvious:
> >
> > byte_num = reg&  3;
> > byte_shift = bytenum * 8;
> > byte = (val>>  byte_shift)&  0xff;
> >
> > ... and similar for the other functions?
> 
> This looks better to me than the former. Thanks.

Stephen,

Does this mean there'll be a new version of this patch?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/2] mmc: add bcm2835 driver

[Stephen Warren]

On 11/04/2012 08:32 AM, Albert ARIBAUD wrote:
> Hi Stephen,
> 
> On Sun, 28 Oct 2012 22:36:25 +0530, Vikram Narayanan
> <vikram186@gmail.com> wrote:
> 
>> On 10/28/2012 8:58 AM, Stephen Warren wrote:
>>> On 10/26/2012 05:33 AM, Vikram Narayanan wrote:
>>>> Some nitpicks.
>>>>
>>>> On 10/24/2012 10:20 AM, Stephen Warren wrote:
>> <snip>
>>>>> +static u8 bcm2835_sdhci_readb(struct sdhci_host *host, int reg)
>>>>> +{
>>>>> +    u32 val = bcm2835_sdhci_raw_readl(host, (reg&   ~3));
>>>>> +    val = val>>   (reg<<   3&   0x18)&   0xff;
>>>>> +
>>>>> +    return (u8)val;
>>>>> +}
>>>>
>>>> Can the above used magics be made as macros?
>>>
>>> This code was taken directly from the downstream Linux kernel, so I
>>> changed it as little as possible, to make comparisons easier. Still, if
>>> people want I can certainly make it easier to understand the expression
>>> a bit.
>>
>> Seems reasonable.
>>
>>> I don't think the issue is the magic numbers so much as understanding
>>> what the expression does; the magic are obvious then. It's simply
>>> extracting byte n from from a u32. Would the following be more obvious:
>>>
>>> byte_num = reg&  3;
>>> byte_shift = bytenum * 8;
>>> byte = (val>>  byte_shift)&  0xff;
>>>
>>> ... and similar for the other functions?
>>
>> This looks better to me than the former. Thanks.
> 
> Stephen,
> 
> Does this mean there'll be a new version of this patch?
> 
> Amicalement,

Hopefully. I'm waiting on Signed-off-by lines from Oleksandr Tymoshenko
and Dom Cobley (assuming they can provide them...) before I make this
change and repost.