Re: [PATCH] pinctrl: bcm283x: Fix GPIO pull state register values for BCM2711

[Matthias Brugger]

On 04/02/2026 21:16, Cibil Pankiras wrote:

On Wed, Feb 4, 2026 at 6:34 PM Matthias Brugger <mbrug...@suse.com> wrote:

On 29/01/2026 16:15, Cibil Pankiras wrote:

BCM2711 has different pull-up/down register values compared to BCM2835

- BCM2835: NONE=0, DOWN=1, UP=2
- BCM2711: NONE=0, UP=1, DOWN=2

This patch fixes the pull state register values for BCM2711.

Fixes: 2c39d975f87c ("pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 
and BCM2711")
Signed-off-by: Cibil Pankiras <cibil.panki...@egym.com>
---
   drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 20 +++++++++++++++++---
   1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c 
b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
index 4ecc8bac645a..33638515b5db 100644
--- a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
+++ b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
@@ -30,6 +30,11 @@ struct bcm283x_pinctrl_priv {

   #define MAX_PINS_PER_BANK 16

+/* pull states for BCM2711 */
+#define BCM2711_PULL_NONE 0
+#define BCM2711_PULL_UP   1
+#define BCM2711_PULL_DOWN 2
+
   static void bcm2835_gpio_set_func_id(struct udevice *dev, unsigned int gpio,
                                    int func)
   {
@@ -84,7 +89,7 @@ static void bcm2835_gpio_set_pull(struct udevice *dev, 
unsigned int gpio, int pu
    * bcm2711_gpio_set_pull: Set GPIO pull-up/down resistor for BCM2711
    * @dev: the pinctrl device
    * @gpio: the GPIO pin number
- * @pull: pull setting (BCM2835_PUD_OFF, BCM2835_PUD_DOWN, BCM2835_PUD_UP)
+ * @pull: pull setting (BCM2711_PULL_NONE, BCM2711_PULL_DOWN, BCM2711_PULL_UP)

Why call it BCM2711_PULL_x instead of BCM2711_PUD_x?

I referenced the kernel, but it would make more sense to name it
BCM2711_PUD_x, just like BCM2835_PUD_x. I will rename it and send v2.

Yes, sorry didn't saw that. Then I would say let's stay with kernel convention.

Thanks,
Matthias

    */
   static void bcm2711_gpio_set_pull(struct udevice *dev, unsigned int gpio, 
int pull)
   {
@@ -108,10 +113,19 @@ static void bcm2711_gpio_set_pull(struct udevice *dev, 
unsigned int gpio, int pu

   static void bcm283x_gpio_set_pull(struct udevice *dev, unsigned int gpio, 
int pull)
   {
-     if (device_is_compatible(dev, "brcm,bcm2835-gpio"))
+     if (device_is_compatible(dev, "brcm,bcm2835-gpio")) {
               bcm2835_gpio_set_pull(dev, gpio, pull);
-     else
+     } else {
+             /* BCM2711's pull values differ from BCM2835 */
+             if (pull == BCM2835_PUD_UP)
+                     pull = BCM2711_PULL_UP;
+             else if (pull == BCM2835_PUD_DOWN)
+                     pull = BCM2711_PULL_DOWN;
+             else
+                     pull = BCM2711_PULL_NONE;
+

That code should go into bcm2711_gpio_set_pull().

I will make the change and send v2.

Warm Regards,
Cibil