[PATCH] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711
Matthias Brugger
mbrugger at suse.com
Wed Nov 12 20:06:15 CET 2025
On 11/11/2025 22:17, Cibil Pankiras wrote:
> 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` ?
>
I think I miss some basic understanding here. Please help me with that. From
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.yaml?h=v6.18-rc5#n88
<snip>
brcm,pull:
description: >
Legacy pull setting for the pins (0=none, 1=pull-down, 2=pull-up).
$ref: /schemas/types.yaml#/definitions/uint32-array
maximum: 2
</snip>
So if I understand correctly you are reading the number of bytes in brcm,pull,
which should be either one or two u32 values. But you error out only if the
pull_len is 0x3. How is that? My gut feeling tells me it should be either 0x2 or
0x4 everything else is an invalid DTB. But as I said I think I miss some
fundamental understanding here. Can you please elaborate.
By the way, I suppose we implement the deprecated binding because the RPi FW
still uses this, correct?
Regards,
Matthias
More information about the U-Boot
mailing list