[PATCH] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711

Cibil Pankiras cibil.pankiras at egym.com
Tue Nov 11 22:17:30 CET 2025


Hi Matthias,

On Mon, Nov 10, 2025 at 4:42 PM Matthias Brugger <mbrugger at suse.com> wrote:
>
>
>
> On 10/11/2025 12:24, Cibil Pankiras wrote:
> > This patch adds support for configuring GPIO pull-up and pull-down
> > resistors in the BCM283x pinctrl driver. It implements the brcm,pull
> > device tree property to control pin bias settings.
> >
> > The implementation follows the hardware-specific pull control
> > mechanisms:
> > - BCM2835: two-step GPPUD register sequence
> > - BCM2711: direct per-pin control registers
> >
> > This enables device tree configurations to specify pull-up, pull-down,
> > or no bias for individual GPIO pins.
> >
> > Tested on Raspberry Pi boards with both BCM2835 and BCM2711 SoCs.
> >
> > Signed-off-by: Cibil Pankiras <cibil.pankiras at egym.com>
> > ---
> >   arch/arm/mach-bcm283x/include/mach/gpio.h  |  10 ++
> >   drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 102 ++++++++++++++++++++-
> >   2 files changed, 109 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-bcm283x/include/mach/gpio.h b/arch/arm/mach-bcm283x/include/mach/gpio.h
> > index 4aeb48eeb20..c54414a012c 100644
> > --- a/arch/arm/mach-bcm283x/include/mach/gpio.h
> > +++ b/arch/arm/mach-bcm283x/include/mach/gpio.h
> > @@ -26,6 +26,16 @@
> >   #define BCM2835_GPIO_FSEL_BANK(gpio)        (gpio / 10)
> >   #define BCM2835_GPIO_FSEL_SHIFT(gpio)       ((gpio % 10) * 3)
> >
> > +/* BCM2835 GPIO Pull-up/down register offsets */
> > +#define BCM2835_GPPUD                37
> > +#define BCM2835_GPPUDCLK0    38
> > +
> > +/* BCM2711 GPIO Pull-up/down control */
> > +#define BCM2711_GPPUD_CNTRL_REG0     57
> > +#define BCM2711_PUD_REG_OFFSET(gpio) ((gpio) / 16)
> > +#define BCM2711_PUD_REG_SHIFT(gpio)  (((gpio) % 16) * 2)
> > +#define BCM2711_PUD_2711_MASK                0x3
> > +
> >   struct bcm2835_gpio_regs {
> >       u32 gpfsel[6];
> >       u32 reserved1;
> > diff --git a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
> > index cf9350c151e..c588015905d 100644
> > --- a/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
> > +++ b/drivers/pinctrl/broadcom/pinctrl-bcm283x.c
> > @@ -21,6 +21,8 @@
> >   #include <asm/system.h>
> >   #include <asm/io.h>
> >   #include <asm/gpio.h>
> > +#include <dt-bindings/pinctrl/bcm2835.h>
> > +#include <linux/delay.h>
> >
> >   struct bcm283x_pinctrl_priv {
> >       u32 *base_reg;
> > @@ -54,7 +56,66 @@ static int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned int gpio)
> >   }
> >
> >   /*
> > - * bcm283x_pinctrl_set_state: configure pin functions.
> > + * bcm2835_gpio_set_pull: Set GPIO pull-up/down resistor for BCM2835
> > + * @dev: the pinctrl device
> > + * @gpio: the GPIO pin number
> > + * @pull: pull setting (BCM2835_PUD_OFF, BCM2835_PUD_DOWN, BCM2835_PUD_UP)
> > + */
> > +static void bcm2835_gpio_set_pull(struct udevice *dev, unsigned int gpio, int pull)
> > +{
> > +     struct bcm283x_pinctrl_priv *priv = dev_get_priv(dev);
> > +     u32 bank = BCM2835_GPPUDCLK0 + BCM2835_GPIO_COMMON_BANK(gpio);
> > +     u32 bit = BCM2835_GPIO_COMMON_SHIFT(gpio);
> > +
> > +     /* Set required control signal */
> > +     writel(pull & 0x3, &priv->base_reg[BCM2835_GPPUD]);
> > +     udelay(1);
> > +
> > +     /* Clock the control signal into the GPIO pads */
> > +     writel(1 << bit, &priv->base_reg[bank]);
> > +     udelay(1);
> > +
> > +     /* Remove the control signal and clock */
> > +     writel(0, &priv->base_reg[BCM2835_GPPUD]);
> > +     writel(0, &priv->base_reg[bank]);
> > +}
> > +
> > +/*
> > + * 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)
> > + */
> > +static void bcm2711_gpio_set_pull(struct udevice *dev, unsigned int gpio, int pull)
> > +{
> > +     struct bcm283x_pinctrl_priv *priv = dev_get_priv(dev);
> > +     u32 reg_offset;
> > +     u32 bit_shift;
> > +     u32 pull_bits;
> > +
> > +     /* Findout which GPIO_PUP_PDN_CNTRL register to use */
> > +     reg_offset = BCM2711_GPPUD_CNTRL_REG0 + BCM2711_PUD_REG_OFFSET(gpio);
> > +
> > +     /* Findout the bit position */
> > +     bit_shift = BCM2711_PUD_REG_SHIFT(gpio);
> > +
> > +     /* Update the 2-bit field for this GPIO */
> > +     pull_bits = pull & BCM2711_PUD_2711_MASK;
> > +     clrsetbits_le32(&priv->base_reg[reg_offset],
> > +                     BCM2711_PUD_2711_MASK << bit_shift,
> > +                     pull_bits << bit_shift);
> > +}
> > +
> > +static void bcm283x_gpio_set_pull(struct udevice *dev, unsigned int gpio, int pull)
> > +{
> > +     if (device_is_compatible(dev, "brcm,bcm2835-gpio"))
> > +             bcm2835_gpio_set_pull(dev, gpio, pull);
> > +     else
> > +             bcm2711_gpio_set_pull(dev, gpio, pull);
> > +}
> > +
> > +/*
> > + * bcm283x_pinctrl_set_state: configure pin functions and pull states.
> >    * @dev: the pinctrl device to be configured.
> >    * @config: the state to be configured.
> >    * @return: 0 in success
> > @@ -62,8 +123,9 @@ static int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned int gpio)
> >   int bcm283x_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> >   {
> >       u32 pin_arr[MAX_PINS_PER_BANK];
> > +     u32 pull_arr[MAX_PINS_PER_BANK];
> >       int function;
> > -     int i, len, pin_count = 0;
> > +     int i, len, pin_count = 0, pull_len = 0, pull_count = 0;
> >
> >       if (!dev_read_prop(config, "brcm,pins", &len) || !len ||
> >           len & 0x3 || dev_read_u32_array(config, "brcm,pins", pin_arr,
> > @@ -82,8 +144,42 @@ int bcm283x_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> >               return -EINVAL;
> >       }
> >
> > -     for (i = 0; i < pin_count; i++)
> > +     /* Check if brcm,pull property exists */
> > +     if (dev_read_prop(config, "brcm,pull", &pull_len) && pull_len > 0) {
> > +             if (pull_len & 0x3) {
>
> We should check for arbitrary values, so check should be, pull_len > 0x2

In the current version of the patch, I validate each element in
pull_arr[] as follows:
    for (i = 0; i < pull_count; i++) {
        if (pull_arr[i] > 2) {
            debug("Invalid pull value %d for pin %d in pinconfig %s\n",
                  pull_arr[i], pin_arr[i], config->name);
            return -EINVAL;

Could you please clarify? Did you mean to replace the alignment check
`if (pull_len & 0x3)` with `pull_len > 0x2` ?

Regards,
Cibil



        }
    }

>
> With that change:
> Reviewed-by: Matthias Brugger <mbrugger at suse.com>
>

-- 
EGYM SE, Einsteinstraße 172, 81677 München
Geschäftsführende Direktoren: 
Patrick Meininger, Philipp Roesch-Schlanderer, Florian Sauter
Gerichtsstand 
München | Amtsgericht München HRB 303509 | USt.-Id. DE275313632


More information about the U-Boot mailing list