Rpi: Add support for second sd host controller

PATCH Superseded Alexander Graf 4 participants 10 messages Jan 16, 2018 – Jan 17, 2018

Subsystems (6)

Arm (32-bit) Arm sub-architectures GPIO MMC, SD and SDIO +2 more subsystems

lore

Share & tools

lore

Alexander Graf

author

2018-01-16 13:46 UTC #0 lore raw

cover Rpi: Add support for second sd host controller

The Raspberry Pi (bcm283x SoC) family contains 2 IP blocks to drive SD
peripherals: A pretty standard SDHCI one called iProc and a home grown one called "sdhost".

When driving U-Boot by the same device tree that we use for Linux, we end up in situations where only the "sdhost" device is available for use. In those situations, current U-Boot can simply not drive the SD card, because the device tree disables the SDHCI device.

This patch set also adds support for pinmuxing the SD devices properly to their respective pins. This is necessary because the RPi firmware doesn't mux the SD pins according to the device tree but instead expects the payload (usually Linux, U-Boot for us) to do that.

With this patch set, I can successfully use U-Boot on a RPi3 Compute Module with CONFIG_OF_BOARD=y and a device tree generated by the RPi firmware (plus a few overlays in config.txt for vc4, serial, etc).

Alexander Graf (3):
bcm2835_gpio: Add support for pinmux
mmc: Add bcm2835 sdhost controller
mmc: bcm2835_sdhci: Add pinmux support

 arch/arm/mach-bcm283x/include/mach/gpio.h |    1 +
 drivers/gpio/bcm2835_gpio.c               |   40 ++
 drivers/mmc/Kconfig                       |   14 +
 drivers/mmc/Makefile                      |    1 +
 drivers/mmc/bcm2835_sdhci.c               |   11 +
 drivers/mmc/bcm2835_sdhost.c              | 1000 +++++++++++++++++++++++++++++
 6 files changed, 1067 insertions(+)
 create mode 100644 drivers/mmc/bcm2835_sdhost.c

Signature · 1 line

2.12.3

Alexander Graf

author

2018-01-16 13:46 UTC #1 lore raw

PATCH 1/3 bcm2835_gpio: Add support for pinmux

On the bcm2835 the GPIO IP block is responsible to control pin muxing of the configurable pins on the chip.

This adds a simple helper function that allows a device driver to set pin muxing according to device tree configuration on those devices.

Signed-off-by Alexander Graf <[email protected]>

---
 arch/arm/mach-bcm283x/include/mach/gpio.h |  1 +
 drivers/gpio/bcm2835_gpio.c               | 40 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

arch/arm/mach-bcm283x/include/mach/gpio.h +1 -0

index daaee52f81..751594d09f 100644
@@ -62,5 +62,6 @@ struct bcm2835_gpio_platdata {
 };
 
 int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio);
+void bcm2835_gpio_set_pinmux(struct udevice *dev, int handle);
 
 #endif /* _BCM2835_GPIO_H_ */

drivers/gpio/bcm2835_gpio.c +40 -0

index beaa21853a..209cbed9e6 100644
@@ -83,6 +83,46 @@ int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio)
 	return (val >> BCM2835_GPIO_FSEL_SHIFT(gpio) & BCM2835_GPIO_FSEL_MASK);
 }
 
+static void bcm2835_gpio_set_func_id(struct udevice *dev, unsigned gpio, int func)
+{
+	struct bcm2835_gpios *gpios = dev_get_priv(dev);
+	u32 val;
+
+	val = readl(&gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+	val &= ~(BCM2835_GPIO_FSEL_MASK << BCM2835_GPIO_FSEL_SHIFT(gpio));
+	val |= (func << BCM2835_GPIO_FSEL_SHIFT(gpio));
+	writel(val, &gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+}
+
+void bcm2835_gpio_set_pinmux(struct udevice *dev, int handle)
+{
+	int node = fdt_node_offset_by_phandle(gd->fdt_blob, handle);
+	u32 pins[16];
+	int len;
+	u32 func;
+	int i;
+
+	if (!node)
+		goto err;
+
+	func = fdtdec_get_int(gd->fdt_blob, node, "brcm,function", -1);
+	if (func <= 0)
+		goto err;
+
+	len = fdtdec_get_int_array_count(gd->fdt_blob, node, "brcm,pins", pins,
+					 ARRAY_SIZE(pins));
+	if (len <= 0)
+		goto err;
+
+	for (i = 0; i < len; i++)
+		bcm2835_gpio_set_func_id(dev, pins[i], func);
+
+	return;
+err:
+	printf("Failed to pinmux phandle 0x%x\n", handle);
+	return;
+}
+
 static int bcm2835_gpio_get_function(struct udevice *dev, unsigned offset)
 {
 	int funcid = bcm2835_gpio_get_func_id(dev, offset);

Signature · 1 line

2.12.3

Amit Tomer

2018-01-17 09:26 UTC #2 lore raw

PATCH 1/3 bcm2835_gpio: Add support for pinmux

Hi,

drivers/gpio/bcm2835_gpio.c int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio) · line 94

Quoted 4 lines

drivers/gpio/bcm2835_gpio.c +4 -0

+       val = readl(&gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+       val &= ~(BCM2835_GPIO_FSEL_MASK << BCM2835_GPIO_FSEL_SHIFT(gpio));
+       val |= (func << BCM2835_GPIO_FSEL_SHIFT(gpio));
+       writel(val, &gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);

Can clrsetbits_le32 be used here ?

Thanks
-Amit

Alexander Graf

author

2018-01-17 22:00 UTC #3 lore raw

PATCH 1/3 bcm2835_gpio: Add support for pinmux

Quoted 8 lines · Amit Tomer

Hi,
drivers/gpio/bcm2835_gpio.c int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio) · line 94
Quoted 4 lines
drivers/gpio/bcm2835_gpio.c +4 -0
+       val = readl(&gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+       val &= ~(BCM2835_GPIO_FSEL_MASK << BCM2835_GPIO_FSEL_SHIFT(gpio));
+       val |= (func << BCM2835_GPIO_FSEL_SHIFT(gpio));
+       writel(val, &gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
Can clrsetbits_le32 be used here ?

Yeah, thanks for the suggestion!

Alex

Simon Glass

2018-01-17 20:06 UTC #4 lore raw

PATCH 1/3 bcm2835_gpio: Add support for pinmux

In reply to Alexander Graf

Hi Alex,

Quoted 60 lines · Alexander Graf

On the bcm2835 the GPIO IP block is responsible to control pin muxing of the configurable pins on the chip.

This adds a simple helper function that allows a device driver to set pin muxing according to device tree configuration on those devices.

Signed-off-by Alexander Graf <[email protected]>

---
 arch/arm/mach-bcm283x/include/mach/gpio.h |  1 +
 drivers/gpio/bcm2835_gpio.c               | 40 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

arch/arm/mach-bcm283x/include/mach/gpio.h +1 -0

index daaee52f81..751594d09f 100644
@@ -62,5 +62,6 @@ struct bcm2835_gpio_platdata {
 };
 
 int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio);
+void bcm2835_gpio_set_pinmux(struct udevice *dev, int handle);
 
 #endif /* _BCM2835_GPIO_H_ */

drivers/gpio/bcm2835_gpio.c +29 -0

index beaa21853a..209cbed9e6 100644
@@ -83,6 +83,46 @@ int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio)
        return (val >> BCM2835_GPIO_FSEL_SHIFT(gpio) & BCM2835_GPIO_FSEL_MASK);
 }
 
+static void bcm2835_gpio_set_func_id(struct udevice *dev, unsigned gpio, int func)
+{
+       struct bcm2835_gpios *gpios = dev_get_priv(dev);
+       u32 val;
+
+       val = readl(&gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+       val &= ~(BCM2835_GPIO_FSEL_MASK << BCM2835_GPIO_FSEL_SHIFT(gpio));
+       val |= (func << BCM2835_GPIO_FSEL_SHIFT(gpio));
+       writel(val, &gpios->reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+}
+
+void bcm2835_gpio_set_pinmux(struct udevice *dev, int handle)
+{
+       int node = fdt_node_offset_by_phandle(gd->fdt_blob, handle);
+       u32 pins[16];
+       int len;
+       u32 func;
+       int i;
+
+       if (!node)
+               goto err;
+
+       func = fdtdec_get_int(gd->fdt_blob, node, "brcm,function", -1);
+       if (func <= 0)
+               goto err;
+
+       len = fdtdec_get_int_array_count(gd->fdt_blob, node, "brcm,pins", pins,
+                                        ARRAY_SIZE(pins));
+       if (len <= 0)

drivers/gpio/bcm2835_gpio.c int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio) · line 124

Quoted 10 lines

drivers/gpio/bcm2835_gpio.c +10 -0

+               goto err;
+
+       for (i = 0; i < len; i++)
+               bcm2835_gpio_set_func_id(dev, pins[i], func);
+
+       return;
+err:
+       printf("Failed to pinmux phandle 0x%x\n", handle);
+       return;
+}

This should use livetree functions and I'm pretty sure it should go in a pinctrl driver, not GPIO.

Quoted 7 lines

drivers/gpio/bcm2835_gpio.c +1 -1

+
 static int bcm2835_gpio_get_function(struct udevice *dev, unsigned offset)
 {
        int funcid = bcm2835_gpio_get_func_id(dev, offset);
--

2.12.3

Regards,
Simon

Alexander Graf

author

2018-01-16 13:46 UTC #5 lore raw

PATCH 2/3 mmc: Add bcm2835 sdhost controller

In reply to Alexander Graf

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 <[email protected]>

---
 drivers/mmc/Kconfig          |   14 +
 drivers/mmc/Makefile         |    1 +
 drivers/mmc/bcm2835_sdhost.c | 1000 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1015 insertions(+)
 create mode 100644 drivers/mmc/bcm2835_sdhost.c

drivers/mmc/Kconfig +14 -0

index ab0627a8af..9b90db908b 100644
@@ -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

drivers/mmc/Makefile +1 -0

index 64b6f21c61..42113e2603 100644
@@ -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

drivers/mmc/bcm2835_sdhost.c new file +1000 -0

1,000 lines not on this page · raw message

Signature · 1 line

2.12.3

Alexander Graf

author

2018-01-16 13:46 UTC #6 lore raw

PATCH 3/3 mmc: bcm2835_sdhci: Add pinmux support

In reply to Alexander Graf

The bcm2835 firmware provided device trees expect device tree users to support pin muxing for the SD devices to work properly.

This patch adds pin muxing support to the sdhci based SD controller on said family of SoCs, so that its pins are getting configured correctly on boot.

Signed-off-by Alexander Graf <[email protected]>

---
 drivers/mmc/bcm2835_sdhci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/mmc/bcm2835_sdhci.c +11 -0

index 3157354d2a..62ad109361 100644
@@ -45,6 +45,7 @@
 #include <asm/arch/mbox.h>
 #include <mach/sdhci.h>
 #include <mach/timer.h>
+#include <mach/gpio.h>
 
 /* 400KHz is max freq for card ID etc. Use that as min */
 #define MIN_FREQ 400000

@@ -178,6 +179,7 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
 	fdt_addr_t base;
 	int emmc_freq;
 	int ret;
+	int pinctrl_handle;
 
 	base = devfdt_get_addr(dev);
 	if (base == FDT_ADDR_T_NONE)

@@ -190,6 +192,15 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
 	}
 	emmc_freq = ret;
 
+	pinctrl_handle = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "pinctrl-0", -1);
+	if (pinctrl_handle != -1) {
+		struct udevice *dev;
+
+		/* Need to set up pinmuxing */
+		if (!uclass_first_device(UCLASS_GPIO, &dev) && dev)
+			bcm2835_gpio_set_pinmux(dev, pinctrl_handle);
+	}
+
 	/*
 	 * See the comments in bcm2835_sdhci_raw_writel().
 	 *

Signature · 1 line

2.12.3

Simon Glass

2018-01-17 20:06 UTC #7 lore raw

PATCH 3/3 mmc: bcm2835_sdhci: Add pinmux support

Hi Alex,

Quoted 33 lines · Alexander Graf

The bcm2835 firmware provided device trees expect device tree users to support pin muxing for the SD devices to work properly.
This patch adds pin muxing support to the sdhci based SD controller on said family of SoCs, so that its pins are getting configured correctly on boot.
Signed-off-by Alexander Graf <[email protected]>
---
 drivers/mmc/bcm2835_sdhci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/mmc/bcm2835_sdhci.c +2 -0
index 3157354d2a..62ad109361 100644
@@ -45,6 +45,7 @@
 #include <asm/arch/mbox.h>
 #include <mach/sdhci.h>
 #include <mach/timer.h>
+#include <mach/gpio.h>
 
 /* 400KHz is max freq for card ID etc. Use that as min */
 #define MIN_FREQ 400000
@@ -178,6 +179,7 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
        fdt_addr_t base;
        int emmc_freq;
        int ret;
+       int pinctrl_handle;
 
        base = devfdt_get_addr(dev);
        if (base == FDT_ADDR_T_NONE)

drivers/mmc/bcm2835_sdhci.c static int bcm2835_sdhci_probe(struct udevice *dev) · line 195

Quoted 4 lines

drivers/mmc/bcm2835_sdhci.c +1 -0

@@ -190,6 +192,15 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
        }
        emmc_freq = ret;
 
+       pinctrl_handle = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "pinctrl-0", -1);

You can't do this sort of thing :-)

This should go in a pinctrl driver. The clue is that you are having to look up things manually here. You can create a basic driver very easily.

Quoted 12 lines

drivers/mmc/bcm2835_sdhci.c +8 -1

+       if (pinctrl_handle != -1) {
+               struct udevice *dev;
+
+               /* Need to set up pinmuxing */
+               if (!uclass_first_device(UCLASS_GPIO, &dev) && dev)
+                       bcm2835_gpio_set_pinmux(dev, pinctrl_handle);
+       }
+
        /*
         * See the comments in bcm2835_sdhci_raw_writel().
         *
--

Quoted 2 lines

2.12.3

Regards,
Simon

Alexander Graf

author

2018-01-17 22:00 UTC #8 lore raw

PATCH 3/3 mmc: bcm2835_sdhci: Add pinmux support

Quoted 39 lines · Simon Glass

Hi Alex,
Quoted 36 lines · Alexander Graf
The bcm2835 firmware provided device trees expect device tree users to support pin muxing for the SD devices to work properly.
This patch adds pin muxing support to the sdhci based SD controller on said family of SoCs, so that its pins are getting configured correctly on boot.
Signed-off-by Alexander Graf <[email protected]>
---
 drivers/mmc/bcm2835_sdhci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/mmc/bcm2835_sdhci.c +2 -0
index 3157354d2a..62ad109361 100644
@@ -45,6 +45,7 @@
 #include <asm/arch/mbox.h>
 #include <mach/sdhci.h>
 #include <mach/timer.h>
+#include <mach/gpio.h>
 
 /* 400KHz is max freq for card ID etc. Use that as min */
 #define MIN_FREQ 400000
@@ -178,6 +179,7 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
        fdt_addr_t base;
        int emmc_freq;
        int ret;
+       int pinctrl_handle;
 
        base = devfdt_get_addr(dev);
        if (base == FDT_ADDR_T_NONE)
@@ -190,6 +192,15 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
        }
        emmc_freq = ret;
 

Quoted 7 lines

drivers/mmc/bcm2835_sdhci.c static int bcm2835_sdhci_probe(struct udevice *dev) · line 195
Quoted 1 line
drivers/mmc/bcm2835_sdhci.c +1 -0
+       pinctrl_handle = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "pinctrl-0", -1);
You can't do this sort of thing :-)
This should go in a pinctrl driver. The clue is that you are having to look up things manually here. You can create a basic driver very easily.

Yup, that's basically what I've spent today on :). Unfortunately my RPi seems to corrupt its U-Boot binary on boot every so often, so some times I end up with an unbootable system and I've been unable to actually trace it down for real yet. For example some times I get weird corruptions in the .dyn.rel section.

Either way, I've switched to network boot now, let's hope that makes life easier. If things work out I'll send a new version tomorrow with an actual pinmux driver that then instantiates the existing GPIO one as a child (so that the pinmux one gets autoloaded).

Alex

Jaehoon Chung

2018-01-17 11:06 UTC #9 lore raw

cover Rpi: Add support for second sd host controller

In reply to Alexander Graf

Quoted 9 lines · Alexander Graf

The Raspberry Pi (bcm283x SoC) family contains 2 IP blocks to drive SD
peripherals: A pretty standard SDHCI one called iProc and a home grown one called "sdhost".
When driving U-Boot by the same device tree that we use for Linux, we end up in situations where only the "sdhost" device is available for use. In those situations, current U-Boot can simply not drive the SD card, because the device tree disables the SDHCI device.

Quoted 8 lines

This patch set also adds support for pinmuxing the SD devices properly to their respective pins. This is necessary because the RPi firmware doesn't mux the SD pins according to the device tree but instead expects the payload (usually Linux, U-Boot for us) to do that.
With this patch set, I can successfully use U-Boot on a RPi3 Compute Module with CONFIG_OF_BOARD=y and a device tree generated by the RPi firmware (plus a few overlays in config.txt for vc4, serial, etc).

Before reviewing, Cold you fix the checkpatch error and warning? There are too many error and warning..

Best Regards,
Jaehoon Chung

Quoted 15 lines

Alexander Graf (3):
bcm2835_gpio: Add support for pinmux
mmc: Add bcm2835 sdhost controller
mmc: bcm2835_sdhci: Add pinmux support

 arch/arm/mach-bcm283x/include/mach/gpio.h |    1 +
 drivers/gpio/bcm2835_gpio.c               |   40 ++
 drivers/mmc/Kconfig                       |   14 +
 drivers/mmc/Makefile                      |    1 +
 drivers/mmc/bcm2835_sdhci.c               |   11 +
 drivers/mmc/bcm2835_sdhost.c              | 1000 +++++++++++++++++++++++++++++
 6 files changed, 1067 insertions(+)
 create mode 100644 drivers/mmc/bcm2835_sdhost.c