[PATCH 10/18] mmc: sdhci-iproc: Add support for emmc2 of the BCM2711

[PATCH 10/18] mmc: sdhci-iproc: Add support for emmc2 of the BCM2711

[Stefan Wahren]

The additional emmc2 interface of the BCM2711 is an improved version
of the old emmc controller, which is able to provide DDR50 mode on the
Raspberry Pi 4. Except 32 bit only register access no other quirks are
known yet.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/mmc/host/sdhci-iproc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 2feb4ef..2b9cdcd 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -261,8 +261,17 @@ static const struct sdhci_iproc_data bcm2835_data = {
 	.mmc_caps = 0x00000000,
 };

+static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
+	.ops = &sdhci_iproc_32only_ops,
+};
+
+static const struct sdhci_iproc_data bcm2711_data = {
+	.pdata = &sdhci_bcm2711_pltfm_data,
+};
+
 static const struct of_device_id sdhci_iproc_of_match[] = {
 	{ .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data },
+	{ .compatible = "brcm,bcm2711-emmc2", .data = &bcm2711_data },
 	{ .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data},
 	{ .compatible = "brcm,sdhci-iproc", .data = &iproc_data },
 	{ }
--
2.7.4

[PATCH 11/18] dt-bindings: pinctrl: bcm2835: Add brcm,bcm2711 compatible

[Stefan Wahren]

Add a new compatible for the BCM2711.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index ac6d614..3cab733 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible: should be one of:
   "brcm,bcm2835-gpio" - BCM2835 compatible pinctrl
   "brcm,bcm7211-gpio" - BCM7211 compatible pinctrl
+  "brcm,bcm2711-gpio" - BCM2711 compatible pinctrl
 - reg: Should contain the physical address of the GPIO module's registers.
 - gpio-controller: Marks the device node as a GPIO controller.
 - #gpio-cells : Should be two. The first cell is the pin number and the
--
2.7.4

Re: [PATCH 11/18] dt-bindings: pinctrl: bcm2835: Add brcm,bcm2711 compatible

[Linus Walleij]

On Mon, Jul 22, 2019 at 8:23 AM Stefan Wahren <wahrenst@gmx.net> wrote:

> Add a new compatible for the BCM2711.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
(...)
> +  "brcm,bcm2711-gpio" - BCM2711 compatible pinctrl

Completely uncontroversial so patch applied.

Yours,
Linus Walleij

[PATCH 12/18] pinctrl: bcm2835: Add support for BCM2711 pull-up functionality

[Stefan Wahren]

The BCM2711 has a new way of selecting the pull-up/pull-down setting
for a GPIO pin. The registers used for the BCM2835, GP_PUD and
GP_PUDCLKn0, are no longer connected. A new set of registers,
GP_GPIO_PUP_PDN_CNTRL_REGx must be used. This commit will add
a new compatible string "brcm,bcm2711-gpio" and the kernel
driver will use it to select which method is used to select
pull-up/pull-down.

This patch based on a patch by Al Cooper which was intended for the
BCM7211. This is a bugfixed and improved version.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 105 ++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 183d1ff..a493205 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -57,15 +57,24 @@
 #define GPAFEN0		0x88	/* Pin Async Falling Edge Detect */
 #define GPPUD		0x94	/* Pin Pull-up/down Enable */
 #define GPPUDCLK0	0x98	/* Pin Pull-up/down Enable Clock */
+#define GP_GPIO_PUP_PDN_CNTRL_REG0 0xe4 /* 2711 Pin Pull-up/down select */

 #define FSEL_REG(p)		(GPFSEL0 + (((p) / 10) * 4))
 #define FSEL_SHIFT(p)		(((p) % 10) * 3)
 #define GPIO_REG_OFFSET(p)	((p) / 32)
 #define GPIO_REG_SHIFT(p)	((p) % 32)

+#define PUD_2711_MASK		0x3
+#define PUD_2711_REG_OFFSET(p)	((p) / 16)
+#define PUD_2711_REG_SHIFT(p)	(((p) % 16) * 2)
+
 /* argument: bcm2835_pinconf_pull */
 #define BCM2835_PINCONF_PARAM_PULL	(PIN_CONFIG_END + 1)

+#define BCM2711_PULL_NONE	0x0
+#define BCM2711_PULL_UP		0x1
+#define BCM2711_PULL_DOWN	0x2
+
 struct bcm2835_pinctrl {
 	struct device *dev;
 	void __iomem *base;
@@ -975,6 +984,77 @@ static const struct pinconf_ops bcm2835_pinconf_ops = {
 	.pin_config_set = bcm2835_pinconf_set,
 };

+static void bcm2711_pull_config_set(struct bcm2835_pinctrl *pc,
+				    unsigned int pin, unsigned int arg)
+{
+	u32 shifter;
+	u32 value;
+	u32 off;
+
+	off = PUD_2711_REG_OFFSET(pin);
+	shifter = PUD_2711_REG_SHIFT(pin);
+
+	value = bcm2835_gpio_rd(pc, GP_GPIO_PUP_PDN_CNTRL_REG0 + (off * 4));
+	value &= ~(PUD_2711_MASK << shifter);
+	value |= (arg << shifter);
+	bcm2835_gpio_wr(pc, GP_GPIO_PUP_PDN_CNTRL_REG0 + (off * 4), value);
+}
+
+static int bcm2711_pinconf_set(struct pinctrl_dev *pctldev,
+			       unsigned int pin, unsigned long *configs,
+			       unsigned int num_configs)
+{
+	struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	u32 param, arg;
+	int i;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		/* convert legacy brcm,pull */
+		case BCM2835_PINCONF_PARAM_PULL:
+			if (arg == BCM2835_PUD_UP)
+				arg = BCM2711_PULL_UP;
+			else if (arg == BCM2835_PUD_DOWN)
+				arg = BCM2711_PULL_DOWN;
+			else
+				arg = BCM2711_PULL_NONE;
+
+			bcm2711_pull_config_set(pc, pin, arg);
+			break;
+
+		/* Set pull generic bindings */
+		case PIN_CONFIG_BIAS_DISABLE:
+			bcm2711_pull_config_set(pc, pin, BCM2711_PULL_NONE);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			bcm2711_pull_config_set(pc, pin, BCM2711_PULL_DOWN);
+			break;
+		case PIN_CONFIG_BIAS_PULL_UP:
+			bcm2711_pull_config_set(pc, pin, BCM2711_PULL_UP);
+			break;
+
+		/* Set output-high or output-low */
+		case PIN_CONFIG_OUTPUT:
+			bcm2835_gpio_set_bit(pc, arg ? GPSET0 : GPCLR0, pin);
+			break;
+
+		default:
+			return -ENOTSUPP;
+		}
+	} /* for each config */
+
+	return 0;
+}
+
+static const struct pinconf_ops bcm2711_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = bcm2835_pinconf_get,
+	.pin_config_set = bcm2711_pinconf_set,
+};
+
 static struct pinctrl_desc bcm2835_pinctrl_desc = {
 	.name = MODULE_NAME,
 	.pins = bcm2835_gpio_pins,
@@ -990,6 +1070,18 @@ static struct pinctrl_gpio_range bcm2835_pinctrl_gpio_range = {
 	.npins = BCM2835_NUM_GPIOS,
 };

+static const struct of_device_id bcm2835_pinctrl_match[] = {
+	{
+		.compatible = "brcm,bcm2835-gpio",
+		.data = &bcm2835_pinconf_ops,
+	},
+	{
+		.compatible = "brcm,bcm2711-gpio",
+		.data = &bcm2711_pinconf_ops,
+	},
+	{}
+};
+
 static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -997,6 +1089,8 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	struct bcm2835_pinctrl *pc;
 	struct resource iomem;
 	int err, i;
+	const struct of_device_id *match;
+
 	BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_pins) != BCM2835_NUM_GPIOS);
 	BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_groups) != BCM2835_NUM_GPIOS);

@@ -1073,6 +1167,12 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 					     bcm2835_gpio_irq_handler);
 	}

+	match = of_match_node(bcm2835_pinctrl_match, pdev->dev.of_node);
+	if (match) {
+		bcm2835_pinctrl_desc.confops =
+			(const struct pinconf_ops *)match->data;
+	}
+
 	pc->pctl_dev = devm_pinctrl_register(dev, &bcm2835_pinctrl_desc, pc);
 	if (IS_ERR(pc->pctl_dev)) {
 		gpiochip_remove(&pc->gpio_chip);
@@ -1087,11 +1187,6 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	return 0;
 }

-static const struct of_device_id bcm2835_pinctrl_match[] = {
-	{ .compatible = "brcm,bcm2835-gpio" },
-	{}
-};
-
 static struct platform_driver bcm2835_pinctrl_driver = {
 	.probe = bcm2835_pinctrl_probe,
 	.driver = {
--
2.7.4

Re: [PATCH 12/18] pinctrl: bcm2835: Add support for BCM2711 pull-up functionality

[Linus Walleij]

On Mon, Jul 22, 2019 at 8:24 AM Stefan Wahren <wahrenst@gmx.net> wrote:

> The BCM2711 has a new way of selecting the pull-up/pull-down setting
> for a GPIO pin. The registers used for the BCM2835, GP_PUD and
> GP_PUDCLKn0, are no longer connected. A new set of registers,
> GP_GPIO_PUP_PDN_CNTRL_REGx must be used. This commit will add
> a new compatible string "brcm,bcm2711-gpio" and the kernel
> driver will use it to select which method is used to select
> pull-up/pull-down.
>
> This patch based on a patch by Al Cooper which was intended for the
> BCM7211. This is a bugfixed and improved version.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Patch applied.

I think I complained about some other version of this patch, this one
looks entirely acceptable.

Can we get rid of custom pull settings etc from the upstream device
trees so we don't set bad examples? I have a strong urge to
throw in a pr_warn() about any use of it.

Yours,
Linus Walleij

Re: [PATCH 12/18] pinctrl: bcm2835: Add support for BCM2711 pull-up functionality

[Stefan Wahren]

Am 05.08.19 um 11:38 schrieb Linus Walleij:
> On Mon, Jul 22, 2019 at 8:24 AM Stefan Wahren <wahrenst@gmx.net> wrote:
>
>> The BCM2711 has a new way of selecting the pull-up/pull-down setting
>> for a GPIO pin. The registers used for the BCM2835, GP_PUD and
>> GP_PUDCLKn0, are no longer connected. A new set of registers,
>> GP_GPIO_PUP_PDN_CNTRL_REGx must be used. This commit will add
>> a new compatible string "brcm,bcm2711-gpio" and the kernel
>> driver will use it to select which method is used to select
>> pull-up/pull-down.
>>
>> This patch based on a patch by Al Cooper which was intended for the
>> BCM7211. This is a bugfixed and improved version.
>>
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Patch applied.
Thanks
>
> I think I complained about some other version of this patch, this one
> looks entirely acceptable.
>
> Can we get rid of custom pull settings etc from the upstream device
> trees so we don't set bad examples? I have a strong urge to
> throw in a pr_warn() about any use of it.

Ironically, my pre-RFC version tried to convert all BCM2835 pinmux
settings to generic ones. Unfortunately it seems that i made a mistake,
because it didn't work as expected. Since we stumpled above more and
more other issues (not relevant to pinctrl) during upstream review, i
decided to start with legacy pull-up support, so we can fix this later
in the devicetree for both platforms (currently BCM2711 uses most of the
old BCM2835 pinmuxes including the legacy stuff). So yes my plan is to
fix this soon.

Stefan

>
> Yours,
> Linus Walleij

Re: [PATCH 12/18] pinctrl: bcm2835: Add support for BCM2711 pull-up functionality

[Stefan Wahren]

Hi Linus,

Am 11.08.19 um 22:15 schrieb Stefan Wahren:
> Am 05.08.19 um 11:38 schrieb Linus Walleij:
>> On Mon, Jul 22, 2019 at 8:24 AM Stefan Wahren <wahrenst@gmx.net> wrote:
>>
>>> The BCM2711 has a new way of selecting the pull-up/pull-down setting
>>> for a GPIO pin. The registers used for the BCM2835, GP_PUD and
>>> GP_PUDCLKn0, are no longer connected. A new set of registers,
>>> GP_GPIO_PUP_PDN_CNTRL_REGx must be used. This commit will add
>>> a new compatible string "brcm,bcm2711-gpio" and the kernel
>>> driver will use it to select which method is used to select
>>> pull-up/pull-down.
>>>
>>> This patch based on a patch by Al Cooper which was intended for the
>>> BCM7211. This is a bugfixed and improved version.
>>>
>>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> Patch applied.
> Thanks
>> I think I complained about some other version of this patch, this one
>> looks entirely acceptable.
>>
>> Can we get rid of custom pull settings etc from the upstream device
>> trees so we don't set bad examples? I have a strong urge to
>> throw in a pr_warn() about any use of it.
> Ironically, my pre-RFC version tried to convert all BCM2835 pinmux
> settings to generic ones. Unfortunately it seems that i made a mistake,
> because it didn't work as expected. Since we stumpled above more and
> more other issues (not relevant to pinctrl) during upstream review, i
> decided to start with legacy pull-up support, so we can fix this later
> in the devicetree for both platforms (currently BCM2711 uses most of the
> old BCM2835 pinmuxes including the legacy stuff). So yes my plan is to
> fix this soon.

today i had the time to try this out. Instead of the following:

    i2c0_gpio0: i2c0_gpio0 {
        brcm,pins = <0 1>;
        brcm,function = <BCM2835_FSEL_ALT0>;
        brcm,pull = <BCM2835_PUD_UP
                 BCM2835_PUD_OFF>;
    }

you want this?

    i2c0_gpio0: i2c0_gpio0 {
        pin-sda {
            function = "alt0";
            pins = "gpio0";
            bias-pull-up;
        };
        pin-scl {
            function = "alt0";
            pins = "gpio1";
            bias-disable;
        };
    };

Unfortunately i don't know U-Boot is handle the BCM2835 specific pinctrl
functions.

>
> Stefan
>
>> Yours,
>> Linus Walleij

Re: [PATCH 12/18] pinctrl: bcm2835: Add support for BCM2711 pull-up functionality

[Linus Walleij]

On Fri, Sep 27, 2019 at 9:29 PM Stefan Wahren <wahrenst@gmx.net> wrote:

> today i had the time to try this out. Instead of the following:
>
>     i2c0_gpio0: i2c0_gpio0 {
>         brcm,pins = <0 1>;
>         brcm,function = <BCM2835_FSEL_ALT0>;
>         brcm,pull = <BCM2835_PUD_UP
>                  BCM2835_PUD_OFF>;
>     }
>
> you want this?
>
>     i2c0_gpio0: i2c0_gpio0 {
>         pin-sda {
>             function = "alt0";
>             pins = "gpio0";
>             bias-pull-up;
>         };
>         pin-scl {
>             function = "alt0";
>             pins = "gpio1";
>             bias-disable;
>         };
>     };

Yes that looks much better. In my opinion.

I understand that it puts some developers off because of being
more lines or excessively verbose, so to be on the clear,
verboseness in itself is not the goal.

The goal is universal
portability: i.e. it should not matter one bit whether I work
on an 2001 Intel StrongARM SoC, a 2019 Broadcom SoC
or a 2011 ST-Ericsson SoC: I will understand what bias-disable;
or bias-pull-up; means, which lowers the threshold to
maintenance.

Opaque macros, however helpfully named, still creates
a higher cognitive resistance and stresses developers.

> Unfortunately i don't know U-Boot is handle the BCM2835 specific pinctrl
> functions.

I think it would be nice if boot loaders avoid to forking the
standards, but I suppose it will invariably happen.
Just keep in mind the IETF motto "rough consensus and
running code".

Yours,
Linus Walleij

Re: [PATCH 10/18] mmc: sdhci-iproc: Add support for emmc2 of the BCM2711

[Ulf Hansson]

On Mon, 22 Jul 2019 at 08:24, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> The additional emmc2 interface of the BCM2711 is an improved version
> of the old emmc controller, which is able to provide DDR50 mode on the
> Raspberry Pi 4. Except 32 bit only register access no other quirks are
> known yet.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-iproc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 2feb4ef..2b9cdcd 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -261,8 +261,17 @@ static const struct sdhci_iproc_data bcm2835_data = {
>         .mmc_caps = 0x00000000,
>  };
>
> +static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
> +       .ops = &sdhci_iproc_32only_ops,
> +};
> +
> +static const struct sdhci_iproc_data bcm2711_data = {
> +       .pdata = &sdhci_bcm2711_pltfm_data,
> +};
> +
>  static const struct of_device_id sdhci_iproc_of_match[] = {
>         { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data },
> +       { .compatible = "brcm,bcm2711-emmc2", .data = &bcm2711_data },
>         { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data},
>         { .compatible = "brcm,sdhci-iproc", .data = &iproc_data },
>         { }
> --
> 2.7.4
>