[3/3] mmc: bcm2835_sdhci: Add pinmux support

Message ID 20180116134638.3879-4-agraf@suse.de
State New
Headers show
Series
  • Rpi: Add support for second sd host controller
Related show

Commit Message

Alexander Graf Jan. 16, 2018, 1:46 p.m.
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 <agraf@suse.de>
---
 drivers/mmc/bcm2835_sdhci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Simon Glass Jan. 17, 2018, 8:06 p.m. | #1
Hi Alex,

On 16 January 2018 at 05:46, Alexander Graf <agraf@suse.de> wrote:
> 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 <agraf@suse.de>
> ---
>  drivers/mmc/bcm2835_sdhci.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
> index 3157354d2a..62ad109361 100644
> --- a/drivers/mmc/bcm2835_sdhci.c
> +++ b/drivers/mmc/bcm2835_sdhci.c
> @@ -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);

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.

> +       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().
>          *
> --
> 2.12.3
>

Regards,
Simon
Alexander Graf Jan. 17, 2018, 10 p.m. | #2
On 17.01.18 21:06, Simon Glass wrote:
> Hi Alex,
> 
> On 16 January 2018 at 05:46, Alexander Graf <agraf@suse.de> wrote:
>> 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 <agraf@suse.de>
>> ---
>>  drivers/mmc/bcm2835_sdhci.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
>> index 3157354d2a..62ad109361 100644
>> --- a/drivers/mmc/bcm2835_sdhci.c
>> +++ b/drivers/mmc/bcm2835_sdhci.c
>> @@ -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);
> 
> 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

Patch

diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
index 3157354d2a..62ad109361 100644
--- a/drivers/mmc/bcm2835_sdhci.c
+++ b/drivers/mmc/bcm2835_sdhci.c
@@ -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().
 	 *