* [PATCH v2] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711
@ 2025-11-13 23:45 Cibil Pankiras
2025-11-14 8:51 ` Matthias Brugger
0 siblings, 1 reply; 4+ messages in thread
From: Cibil Pankiras @ 2025-11-13 23:45 UTC (permalink / raw)
To: u-boot; +Cc: Matthias Brugger, Peter Robinson, Tom Rini
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@egym.com>
---
Changes in v2:
- If brcm,pull contains a single value, apply it to all pins listed in brcm,pins
---
arch/arm/mach-bcm283x/include/mach/gpio.h | 10 ++
drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 105 ++++++++++++++++++++-
2 files changed, 112 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..4ecc8bac645 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,10 @@ 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;
+ int pull_value;
if (!dev_read_prop(config, "brcm,pins", &len) || !len ||
len & 0x3 || dev_read_u32_array(config, "brcm,pins", pin_arr,
@@ -82,8 +145,44 @@ 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) {
+ debug("Invalid pull array length for pinconfig %s (%d)\n",
+ config->name, pull_len);
+ return -EINVAL;
+ }
+
+ pull_count = pull_len / sizeof(u32);
+
+ if (pull_count != 1 && pull_count != pin_count) {
+ debug("Pull array count (%d) must be 1 or match pin count (%d) for pinconfig %s\n",
+ pull_count, pin_count, config->name);
+ return -EINVAL;
+ }
+
+ if (dev_read_u32_array(config, "brcm,pull", pull_arr, pull_count)) {
+ debug("Failed reading pull array for pinconfig %s\n", config->name);
+ return -EINVAL;
+ }
+
+ /* Validate pull values */
+ 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;
+ }
+ }
+ }
+
+ for (i = 0; i < pin_count; i++) {
bcm2835_gpio_set_func_id(dev, pin_arr[i], function);
+ if (pull_count > 0) {
+ pull_value = (pull_count == 1) ? pull_arr[0] : pull_arr[i];
+ bcm283x_gpio_set_pull(dev, pin_arr[i], pull_value);
+ }
+ }
return 0;
}
--
2.43.0
--
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711
2025-11-13 23:45 [PATCH v2] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711 Cibil Pankiras
@ 2025-11-14 8:51 ` Matthias Brugger
2025-11-17 9:26 ` Cibil Pankiras
0 siblings, 1 reply; 4+ messages in thread
From: Matthias Brugger @ 2025-11-14 8:51 UTC (permalink / raw)
To: Cibil Pankiras, u-boot; +Cc: Peter Robinson, Tom Rini
On 14/11/25 00:45, 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@egym.com>
We check the brcm,pull byte lenght to be on par with the DT binding.
That's not something I would expect normally to be done in a driver. But
as this is also done for brcm,pin in this driver and also done in the
kernel: let's not bike-shed.
Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> ---
> Changes in v2:
> - If brcm,pull contains a single value, apply it to all pins listed in brcm,pins
> ---
> arch/arm/mach-bcm283x/include/mach/gpio.h | 10 ++
> drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 105 ++++++++++++++++++++-
> 2 files changed, 112 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..4ecc8bac645 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,10 @@ 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;
> + int pull_value;
>
> if (!dev_read_prop(config, "brcm,pins", &len) || !len ||
> len & 0x3 || dev_read_u32_array(config, "brcm,pins", pin_arr,
> @@ -82,8 +145,44 @@ 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) {
> + debug("Invalid pull array length for pinconfig %s (%d)\n",
> + config->name, pull_len);
> + return -EINVAL;
> + }
> +
> + pull_count = pull_len / sizeof(u32);
> +
> + if (pull_count != 1 && pull_count != pin_count) {
> + debug("Pull array count (%d) must be 1 or match pin count (%d) for pinconfig %s\n",
> + pull_count, pin_count, config->name);
> + return -EINVAL;
> + }
> +
> + if (dev_read_u32_array(config, "brcm,pull", pull_arr, pull_count)) {
> + debug("Failed reading pull array for pinconfig %s\n", config->name);
> + return -EINVAL;
> + }
> +
> + /* Validate pull values */
> + 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;
> + }
> + }
> + }
> +
> + for (i = 0; i < pin_count; i++) {
> bcm2835_gpio_set_func_id(dev, pin_arr[i], function);
> + if (pull_count > 0) {
> + pull_value = (pull_count == 1) ? pull_arr[0] : pull_arr[i];
> + bcm283x_gpio_set_pull(dev, pin_arr[i], pull_value);
> + }
> + }
>
> return 0;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711
2025-11-14 8:51 ` Matthias Brugger
@ 2025-11-17 9:26 ` Cibil Pankiras
2025-11-17 10:16 ` Peter Robinson
0 siblings, 1 reply; 4+ messages in thread
From: Cibil Pankiras @ 2025-11-17 9:26 UTC (permalink / raw)
To: Matthias Brugger; +Cc: u-boot, Peter Robinson, Tom Rini
Hi Matthias,
On Fri, Nov 14, 2025 at 9:51 AM Matthias Brugger <mbrugger@suse.com> wrote:
>
>
>
> On 14/11/25 00:45, 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@egym.com>
>
> We check the brcm,pull byte lenght to be on par with the DT binding.
> That's not something I would expect normally to be done in a driver. But
> as this is also done for brcm,pin in this driver and also done in the
> kernel: let's not bike-shed.
>
> Reviewed-by: Matthias Brugger <mbrugger@suse.com>
Thanks for reviewing my patch
Should I resend the patch with the Reviewed-by tag, or will it be
added when the patch is applied to the tree?
Is there anything else needed from my side?
Regards,
Cibil
>
> > ---
> > Changes in v2:
> > - If brcm,pull contains a single value, apply it to all pins listed in brcm,pins
> > ---
> > arch/arm/mach-bcm283x/include/mach/gpio.h | 10 ++
> > drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 105 ++++++++++++++++++++-
> > 2 files changed, 112 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..4ecc8bac645 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,10 @@ 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;
> > + int pull_value;
> >
> > if (!dev_read_prop(config, "brcm,pins", &len) || !len ||
> > len & 0x3 || dev_read_u32_array(config, "brcm,pins", pin_arr,
> > @@ -82,8 +145,44 @@ 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) {
> > + debug("Invalid pull array length for pinconfig %s (%d)\n",
> > + config->name, pull_len);
> > + return -EINVAL;
> > + }
> > +
> > + pull_count = pull_len / sizeof(u32);
> > +
> > + if (pull_count != 1 && pull_count != pin_count) {
> > + debug("Pull array count (%d) must be 1 or match pin count (%d) for pinconfig %s\n",
> > + pull_count, pin_count, config->name);
> > + return -EINVAL;
> > + }
> > +
> > + if (dev_read_u32_array(config, "brcm,pull", pull_arr, pull_count)) {
> > + debug("Failed reading pull array for pinconfig %s\n", config->name);
> > + return -EINVAL;
> > + }
> > +
> > + /* Validate pull values */
> > + 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;
> > + }
> > + }
> > + }
> > +
> > + for (i = 0; i < pin_count; i++) {
> > bcm2835_gpio_set_func_id(dev, pin_arr[i], function);
> > + if (pull_count > 0) {
> > + pull_value = (pull_count == 1) ? pull_arr[0] : pull_arr[i];
> > + bcm283x_gpio_set_pull(dev, pin_arr[i], pull_value);
> > + }
> > + }
> >
> > return 0;
> > }
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711
2025-11-17 9:26 ` Cibil Pankiras
@ 2025-11-17 10:16 ` Peter Robinson
0 siblings, 0 replies; 4+ messages in thread
From: Peter Robinson @ 2025-11-17 10:16 UTC (permalink / raw)
To: Cibil Pankiras; +Cc: Matthias Brugger, u-boot, Tom Rini
On Mon, 17 Nov 2025 at 09:26, Cibil Pankiras <cibil.pankiras@egym.com> wrote:
>
> Hi Matthias,
>
> On Fri, Nov 14, 2025 at 9:51 AM Matthias Brugger <mbrugger@suse.com> wrote:
> >
> >
> >
> > On 14/11/25 00:45, 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@egym.com>
> >
> > We check the brcm,pull byte lenght to be on par with the DT binding.
> > That's not something I would expect normally to be done in a driver. But
> > as this is also done for brcm,pin in this driver and also done in the
> > kernel: let's not bike-shed.
> >
> > Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> Thanks for reviewing my patch
> Should I resend the patch with the Reviewed-by tag, or will it be
> added when the patch is applied to the tree?
> Is there anything else needed from my side?
Nope, will be added automatically by patchwork, I'm just testing all
the rpi bits now.
> Regards,
> Cibil
> >
> > > ---
> > > Changes in v2:
> > > - If brcm,pull contains a single value, apply it to all pins listed in brcm,pins
> > > ---
> > > arch/arm/mach-bcm283x/include/mach/gpio.h | 10 ++
> > > drivers/pinctrl/broadcom/pinctrl-bcm283x.c | 105 ++++++++++++++++++++-
> > > 2 files changed, 112 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..4ecc8bac645 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,10 @@ 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;
> > > + int pull_value;
> > >
> > > if (!dev_read_prop(config, "brcm,pins", &len) || !len ||
> > > len & 0x3 || dev_read_u32_array(config, "brcm,pins", pin_arr,
> > > @@ -82,8 +145,44 @@ 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) {
> > > + debug("Invalid pull array length for pinconfig %s (%d)\n",
> > > + config->name, pull_len);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + pull_count = pull_len / sizeof(u32);
> > > +
> > > + if (pull_count != 1 && pull_count != pin_count) {
> > > + debug("Pull array count (%d) must be 1 or match pin count (%d) for pinconfig %s\n",
> > > + pull_count, pin_count, config->name);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (dev_read_u32_array(config, "brcm,pull", pull_arr, pull_count)) {
> > > + debug("Failed reading pull array for pinconfig %s\n", config->name);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Validate pull values */
> > > + 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;
> > > + }
> > > + }
> > > + }
> > > +
> > > + for (i = 0; i < pin_count; i++) {
> > > bcm2835_gpio_set_func_id(dev, pin_arr[i], function);
> > > + if (pull_count > 0) {
> > > + pull_value = (pull_count == 1) ? pull_arr[0] : pull_arr[i];
> > > + bcm283x_gpio_set_pull(dev, pin_arr[i], pull_value);
> > > + }
> > > + }
> > >
> > > return 0;
> > > }
> >
>
> --
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-17 10:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-13 23:45 [PATCH v2] pinctrl: bcm283x: Add GPIO pull-up/down control for BCM2835 and BCM2711 Cibil Pankiras
2025-11-14 8:51 ` Matthias Brugger
2025-11-17 9:26 ` Cibil Pankiras
2025-11-17 10:16 ` Peter Robinson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox