diff mbox series

[v4,2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support

Message ID 4cda8cfc37fb15a0c3b180ab4c34a6f6f859fe3c.1676668853.git.asmaa@nvidia.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series Support Nvidia BlueField-3 GPIO driver and pin controller | expand

Commit Message

Asmaa Mnebhi Feb. 17, 2023, 9:26 p.m. UTC
NVIDIA BlueField-3 SoC has a few pins that can be used as GPIOs
or take the default hardware functionality. Add a driver for
the pinmuxing.

Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/pinctrl/Kconfig         |  14 ++
 drivers/pinctrl/Makefile        |   1 +
 drivers/pinctrl/pinctrl-mlxbf.c | 338 ++++++++++++++++++++++++++++++++
 3 files changed, 353 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-mlxbf.c

Comments

Andy Shevchenko Feb. 17, 2023, 11:24 p.m. UTC | #1
On Fri, Feb 17, 2023 at 11:27 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> NVIDIA BlueField-3 SoC has a few pins that can be used as GPIOs
> or take the default hardware functionality. Add a driver for
> the pinmuxing.

pin muxing

...

> +++ b/drivers/pinctrl/pinctrl-mlxbf.c

Wondering if it would be better to match the GPIO driver naming
schema, i.e. by adding 3. In this case the additional explanation in
Kconfig help won't be necessary.

...

> +#define MLXBF_GPIO0_FW_CONTROL_SET   0
> +#define MLXBF_GPIO0_FW_CONTROL_CLEAR 0x14
> +#define MLXBF_GPIO1_FW_CONTROL_SET   0x80
> +#define MLXBF_GPIO1_FW_CONTROL_CLEAR 0x94

Unclear if these are commands or register offsets. If they are of the
same type (semantically), make them fixed width, e.g., 0x00.

...

> +enum {
> +       MLXBF_GPIO_HW_MODE,
> +       MLXBF_GPIO_SW_MODE

I would leave a comma here as it might be extended in the future.

> +};

...

> +static const char * const mlxbf_pinctrl_single_group_names[] = {
> +       "gpio0", "gpio1",  "gpio2",  "gpio3",  "gpio4",  "gpio5",  "gpio6",
> +       "gpio7", "gpio8",  "gpio9",  "gpio10", "gpio11", "gpio12", "gpio13",
> +       "gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19", "gpio20",
> +       "gpio21", "gpio22", "gpio23", "gpio24", "gpio25", "gpio26", "gpio27",
> +       "gpio28", "gpio29", "gpio30", "gpio31", "gpio32", "gpio33", "gpio34",
> +       "gpio35", "gpio36", "gpio37", "gpio38", "gpio39", "gpio40", "gpio41",
> +       "gpio42", "gpio43", "gpio44", "gpio45", "gpio46", "gpio47", "gpio48",
> +       "gpio49", "gpio50", "gpio51", "gpio52", "gpio53", "gpio54", "gpio55"

Ditto.
Can you group by 8?

> +};
> +
> +/* Set of pin numbers for single-pin groups */
> +static const unsigned int mlxbf_pinctrl_single_group_pins[] = {
> +       0,  1,  2,  3,  4,  5,  6,
> +       7,  8,  9, 10, 11, 12, 13,
> +       14, 15, 16, 17, 18, 19, 20,
> +       21, 22, 23, 24, 25, 26, 27,
> +       28, 29, 30, 31, 32, 33, 34,
> +       35, 36, 37, 38, 39, 40, 41,
> +       42, 43, 44, 45, 46, 47, 48,
> +       49, 50, 51, 52, 53, 54, 55,

Group by 8 which is the more natural length of subarray per line.

Is it just 1:1 to the index? If so, why do you need this table at all?

> +};

...

> +static const struct {
> +       const char *name;
> +       const char * const *group_names;

Use this instead
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/tree/include/linux/pinctrl/pinctrl.h?h=devel#n215
and this
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/tree/include/linux/pinctrl/pinctrl.h?h=devel#n222

> +} mlxbf_pmx_funcs[] = {

> +};

...

> +{
> +       struct mlxbf_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
> +
> +       /* disable GPIO functionality by giving control back to hardware */
> +       if (offset < MLXBF_NGPIOS_GPIO0)
> +               writel(BIT(offset), priv->base + MLXBF_GPIO0_FW_CONTROL_CLEAR);
> +       else
> +               writel(BIT(offset % MLXBF_NGPIOS_GPIO0), priv->base + MLXBF_GPIO1_FW_CONTROL_CLEAR);

> +

Redundant blank line.

> +}

...

> +static_assert(ARRAY_SIZE(mlxbf_pinctrl_single_group_names) ==
> +             ARRAY_SIZE(mlxbf_pinctrl_single_group_pins));

I would put it on a single line, but it's up to you.

...

> +       struct resource *res;

Useless?

...

> +       /* This resource is shared so use devm_ioremap */

Can you elaborate on who actually requests the region? And why is it
not _this_ driver?

> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -ENODEV;

...

> +       ret = devm_pinctrl_register_and_init(priv->dev,

Is the priv->dev different from dev?

> +                                            &mlxbf_pin_desc,
> +                                            priv,
> +                                            &priv->pctl);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "Failed to register pinctrl\n");

...

> +       pinctrl_add_gpio_range(priv->pctl, &mlxbf_pinctrl_gpio_ranges[0]);
> +       pinctrl_add_gpio_range(priv->pctl, &mlxbf_pinctrl_gpio_ranges[1]);

pinctrl_add_gpio_ranges() ?

--
With Best Regards,
Andy Shevchenko
Asmaa Mnebhi Feb. 23, 2023, 9:07 p.m. UTC | #2
> > +static const struct {
> > +       const char *name;
> > +       const char * const *group_names;
> 
> Use this instead
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-
> pinctrl.git/tree/include/linux/pinctrl/pinctrl.h?h=devel#n215
> and this
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-
> pinctrl.git/tree/include/linux/pinctrl/pinctrl.h?h=devel#n222
> 
> > +} mlxbf_pmx_funcs[] = {
> 
> > +};

so copy that struct definition and macro to my driver? (I don’t see these code changes in master)  

> 
> > +       /* This resource is shared so use devm_ioremap */
> 
> Can you elaborate on who actually requests the region? And why is it not
> _this_ driver?

This resource is shared with the gpio-mlxbf3.c driver. The gpio-mlxbf3.c driver does not access the same offsets as the pinctrl-mlxbf3.c driver, but it accesses several other registers offsets in between.
Andy Shevchenko Feb. 24, 2023, 9:44 a.m. UTC | #3
On Thu, Feb 23, 2023 at 11:07 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> > > +static const struct {
> > > +       const char *name;
> > > +       const char * const *group_names;
> >
> > Use this instead
> > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-
> > pinctrl.git/tree/include/linux/pinctrl/pinctrl.h?h=devel#n215
> > and this
> > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-
> > pinctrl.git/tree/include/linux/pinctrl/pinctrl.h?h=devel#n222
> >
> > > +} mlxbf_pmx_funcs[] = {
> >
> > > +};
>
> so copy that struct definition and macro to my driver? (I don’t see these code changes in master)

Which master?

First of all, you should do your development based on the "for-next"
of the respective subsystem (okay, for pin control Linus Walleij
called his published branch "devel"). So, the above mentioned
functionality was there a while ago.

Second, a couple of days ago Linus Torvalds pulled PR, so it's part of
upstream now.

TL;DR: just use those types and macros in your code.

> > > +       /* This resource is shared so use devm_ioremap */
> >
> > Can you elaborate on who actually requests the region? And why is it not
> > _this_ driver?
>
> This resource is shared with the gpio-mlxbf3.c driver. The gpio-mlxbf3.c driver does not access the same offsets as the pinctrl-mlxbf3.c driver, but it accesses several other registers offsets in between.

Okay, so in such a case you need a common denominator that actually
does this all for you via, for example, regmap. If the region is not
requested, bad things may happen.
Asmaa Mnebhi March 14, 2023, 8:30 p.m. UTC | #4
> > > > +       /* This resource is shared so use devm_ioremap */
> > >
> > > Can you elaborate on who actually requests the region? And why is it
> > > not _this_ driver?
> >
> > This resource is shared with the gpio-mlxbf3.c driver. The gpio-mlxbf3.c
> driver does not access the same offsets as the pinctrl-mlxbf3.c driver, but it
> accesses several other registers offsets in between.
> 
> Okay, so in such a case you need a common denominator that actually does
> this all for you via, for example, regmap. If the region is not requested, bad
> things may happen.

Hi Andy,

I have taken a look at the regmap driver and decided we will maintain the
Current implementation with devm_platform_ioremap_resource.
The main reason is for backward compatibility with older kernels. Although we
Do support the latest kernel, we have some customers who are still using old
Kernels which don’t have regmap support. But I have found a solution which
Involves using  devm_platform_ioremap_resource here as you initially
suggested.
patch v5 coming soon!

Thanks.
Asmaa
diff mbox series

Patch

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 7d5f5458c72e..900fa17635c9 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -523,6 +523,20 @@  config PINCTRL_ZYNQMP
 	  This driver can also be built as a module. If so, the module
 	  will be called pinctrl-zynqmp.
 
+config PINCTRL_MLXBF
+	tristate "NVIDIA BlueField-3 SoC Pinctrl driver"
+	depends on (MELLANOX_PLATFORM && ARM64) || (64BIT && COMPILE_TEST)
+	select PINMUX
+	select GPIOLIB
+	select GPIOLIB_IRQCHIP
+	select GPIO_MLXBF3
+	help
+	  Say Y to select the pinctrl driver for BlueField-3 SoCs.
+	  This pin controller works hand in hand with the gpio-mlxbf3
+	  driver and allows selecting the mux function for each
+	  pin. This driver can also be built as a module called
+	  pinctrl-mlxbf.
+
 source "drivers/pinctrl/actions/Kconfig"
 source "drivers/pinctrl/aspeed/Kconfig"
 source "drivers/pinctrl/bcm/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index d5939840bb2a..67252469e76b 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -36,6 +36,7 @@  obj-$(CONFIG_PINCTRL_MCP23S08_I2C)	+= pinctrl-mcp23s08_i2c.o
 obj-$(CONFIG_PINCTRL_MCP23S08_SPI)	+= pinctrl-mcp23s08_spi.o
 obj-$(CONFIG_PINCTRL_MCP23S08)	+= pinctrl-mcp23s08.o
 obj-$(CONFIG_PINCTRL_MICROCHIP_SGPIO)	+= pinctrl-microchip-sgpio.o
+obj-$(CONFIG_PINCTRL_MLXBF)	+= pinctrl-mlxbf.o
 obj-$(CONFIG_PINCTRL_OCELOT)	+= pinctrl-ocelot.o
 obj-$(CONFIG_PINCTRL_OXNAS)	+= pinctrl-oxnas.o
 obj-$(CONFIG_PINCTRL_PALMAS)	+= pinctrl-palmas.o
diff --git a/drivers/pinctrl/pinctrl-mlxbf.c b/drivers/pinctrl/pinctrl-mlxbf.c
new file mode 100644
index 000000000000..95fc5b8f2dc7
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mlxbf.c
@@ -0,0 +1,338 @@ 
+// SPDX-License-Identifier: GPL-2.0-only or BSD-3-Clause
+/* Copyright (C) 2022 NVIDIA CORPORATION & AFFILIATES */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#define MLXBF_GPIO0_FW_CONTROL_SET   0
+#define MLXBF_GPIO0_FW_CONTROL_CLEAR 0x14
+#define MLXBF_GPIO1_FW_CONTROL_SET   0x80
+#define MLXBF_GPIO1_FW_CONTROL_CLEAR 0x94
+
+#define MLXBF_NGPIOS_GPIO0    32
+
+enum {
+	MLXBF_GPIO_HW_MODE,
+	MLXBF_GPIO_SW_MODE
+};
+
+struct mlxbf_pinctrl {
+	void __iomem *base;
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	struct pinctrl_gpio_range gpio_range;
+};
+
+#define MLXBF_GPIO_RANGE(_id, _pinbase, _gpiobase, _npins)	\
+	{							\
+		.name = "mlxbf_gpio_range",			\
+		.id = _id,					\
+		.base = _gpiobase,				\
+		.pin_base = _pinbase,				\
+		.npins = _npins,				\
+	}
+
+static struct pinctrl_gpio_range mlxbf_pinctrl_gpio_ranges[] = {
+	MLXBF_GPIO_RANGE(0, 0,  480, 32),
+	MLXBF_GPIO_RANGE(1,  32, 456, 24),
+};
+
+static const struct pinctrl_pin_desc mlxbf_pins[] = {
+	PINCTRL_PIN(0, "gpio0"),
+	PINCTRL_PIN(1, "gpio1"),
+	PINCTRL_PIN(2, "gpio2"),
+	PINCTRL_PIN(3, "gpio3"),
+	PINCTRL_PIN(4, "gpio4"),
+	PINCTRL_PIN(5, "gpio5"),
+	PINCTRL_PIN(6, "gpio6"),
+	PINCTRL_PIN(7, "gpio7"),
+	PINCTRL_PIN(8, "gpio8"),
+	PINCTRL_PIN(9, "gpio9"),
+	PINCTRL_PIN(10, "gpio10"),
+	PINCTRL_PIN(11, "gpio11"),
+	PINCTRL_PIN(12, "gpio12"),
+	PINCTRL_PIN(13, "gpio13"),
+	PINCTRL_PIN(14, "gpio14"),
+	PINCTRL_PIN(15, "gpio15"),
+	PINCTRL_PIN(16, "gpio16"),
+	PINCTRL_PIN(17, "gpio17"),
+	PINCTRL_PIN(18, "gpio18"),
+	PINCTRL_PIN(19, "gpio19"),
+	PINCTRL_PIN(20, "gpio20"),
+	PINCTRL_PIN(21, "gpio21"),
+	PINCTRL_PIN(22, "gpio22"),
+	PINCTRL_PIN(23, "gpio23"),
+	PINCTRL_PIN(24, "gpio24"),
+	PINCTRL_PIN(25, "gpio25"),
+	PINCTRL_PIN(26, "gpio26"),
+	PINCTRL_PIN(27, "gpio27"),
+	PINCTRL_PIN(28, "gpio28"),
+	PINCTRL_PIN(29, "gpio29"),
+	PINCTRL_PIN(30, "gpio30"),
+	PINCTRL_PIN(31, "gpio31"),
+	PINCTRL_PIN(32, "gpio32"),
+	PINCTRL_PIN(33, "gpio33"),
+	PINCTRL_PIN(34, "gpio34"),
+	PINCTRL_PIN(35, "gpio35"),
+	PINCTRL_PIN(36, "gpio36"),
+	PINCTRL_PIN(37, "gpio37"),
+	PINCTRL_PIN(38, "gpio38"),
+	PINCTRL_PIN(39, "gpio39"),
+	PINCTRL_PIN(40, "gpio40"),
+	PINCTRL_PIN(41, "gpio41"),
+	PINCTRL_PIN(42, "gpio42"),
+	PINCTRL_PIN(43, "gpio43"),
+	PINCTRL_PIN(44, "gpio44"),
+	PINCTRL_PIN(45, "gpio45"),
+	PINCTRL_PIN(46, "gpio46"),
+	PINCTRL_PIN(47, "gpio47"),
+	PINCTRL_PIN(48, "gpio48"),
+	PINCTRL_PIN(49, "gpio49"),
+	PINCTRL_PIN(50, "gpio50"),
+	PINCTRL_PIN(51, "gpio51"),
+	PINCTRL_PIN(52, "gpio52"),
+	PINCTRL_PIN(53, "gpio53"),
+	PINCTRL_PIN(54, "gpio54"),
+	PINCTRL_PIN(55, "gpio55"),
+};
+
+/*
+ * All single-pin functions can be mapped to any GPIO, however pinmux applies
+ * functions to pin groups and only those groups declared as supporting that
+ * function. To make this work we must put each pin in its own dummy group so
+ * that the functions can be described as applying to all pins.
+ * We use the same name as in the datasheet.
+ */
+static const char * const mlxbf_pinctrl_single_group_names[] = {
+	"gpio0", "gpio1",  "gpio2",  "gpio3",  "gpio4",  "gpio5",  "gpio6",
+	"gpio7", "gpio8",  "gpio9",  "gpio10", "gpio11", "gpio12", "gpio13",
+	"gpio14", "gpio15", "gpio16", "gpio17", "gpio18", "gpio19", "gpio20",
+	"gpio21", "gpio22", "gpio23", "gpio24", "gpio25", "gpio26", "gpio27",
+	"gpio28", "gpio29", "gpio30", "gpio31", "gpio32", "gpio33", "gpio34",
+	"gpio35", "gpio36", "gpio37", "gpio38", "gpio39", "gpio40", "gpio41",
+	"gpio42", "gpio43", "gpio44", "gpio45", "gpio46", "gpio47", "gpio48",
+	"gpio49", "gpio50", "gpio51", "gpio52", "gpio53", "gpio54", "gpio55"
+};
+
+/* Set of pin numbers for single-pin groups */
+static const unsigned int mlxbf_pinctrl_single_group_pins[] = {
+	0,  1,  2,  3,  4,  5,  6,
+	7,  8,  9, 10, 11, 12, 13,
+	14, 15, 16, 17, 18, 19, 20,
+	21, 22, 23, 24, 25, 26, 27,
+	28, 29, 30, 31, 32, 33, 34,
+	35, 36, 37, 38, 39, 40, 41,
+	42, 43, 44, 45, 46, 47, 48,
+	49, 50, 51, 52, 53, 54, 55,
+};
+
+static int mlxbf_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	/* Number single-pin groups */
+	return ARRAY_SIZE(mlxbf_pinctrl_single_group_pins);
+}
+
+static const char *mlxbf_get_group_name(struct pinctrl_dev *pctldev,
+					 unsigned int selector)
+{
+	return mlxbf_pinctrl_single_group_names[selector];
+}
+
+static int mlxbf_get_group_pins(struct pinctrl_dev *pctldev,
+				 unsigned int selector,
+				 const unsigned int **pins,
+				 unsigned int *num_pins)
+{
+	/* return the dummy group for a single pin */
+	*pins = &mlxbf_pinctrl_single_group_pins[selector];
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const struct pinctrl_ops mlxbf_pinctrl_group_ops = {
+	.get_groups_count = mlxbf_get_groups_count,
+	.get_group_name = mlxbf_get_group_name,
+	.get_group_pins = mlxbf_get_group_pins,
+};
+
+static const char * const mlxbf_gpiofunc_group_names[] = { "swctrl" };
+static const char * const mlxbf_hwfunc_group_names[]   = { "hwctrl" };
+
+/*
+ * Only 2 functions are supported and they apply to all pins:
+ * 1) Default hardware functionality
+ * 2) Software controlled GPIO
+ */
+static const struct {
+	const char *name;
+	const char * const *group_names;
+} mlxbf_pmx_funcs[] = {
+	{
+		.name = "hwfunc",
+		.group_names = mlxbf_hwfunc_group_names
+	},
+	{
+		.name = "gpiofunc",
+		.group_names = mlxbf_gpiofunc_group_names
+	},
+};
+
+static int mlxbf_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(mlxbf_pmx_funcs);
+}
+
+static const char *mlxbf_pmx_get_func_name(struct pinctrl_dev *pctldev,
+					   unsigned int selector)
+{
+	return mlxbf_pmx_funcs[selector].name;
+}
+
+static int mlxbf_pmx_get_groups(struct pinctrl_dev *pctldev,
+				 unsigned int selector,
+				 const char * const **groups,
+				 unsigned int * const num_groups)
+{
+	*groups = mlxbf_pmx_funcs[selector].group_names;
+	*num_groups = ARRAY_SIZE(mlxbf_pinctrl_single_group_pins);
+
+	return 0;
+}
+
+static int mlxbf_pmx_set(struct pinctrl_dev *pctldev,
+			      unsigned int selector,
+			      unsigned int group)
+{
+	struct mlxbf_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+
+	if (selector == MLXBF_GPIO_HW_MODE) {
+		if (group < MLXBF_NGPIOS_GPIO0)
+			writel(BIT(group), priv->base + MLXBF_GPIO0_FW_CONTROL_CLEAR);
+		else
+			writel(BIT(group % MLXBF_NGPIOS_GPIO0), priv->base + MLXBF_GPIO1_FW_CONTROL_CLEAR);
+	}
+
+	if (selector == MLXBF_GPIO_SW_MODE) {
+		if (group < MLXBF_NGPIOS_GPIO0)
+			writel(BIT(group), priv->base + MLXBF_GPIO0_FW_CONTROL_SET);
+		else
+			writel(BIT(group % MLXBF_NGPIOS_GPIO0), priv->base + MLXBF_GPIO1_FW_CONTROL_SET);
+	}
+
+	return 0;
+}
+
+static int mlxbf_gpio_request_enable(struct pinctrl_dev *pctldev,
+				     struct pinctrl_gpio_range *range,
+				     unsigned int offset)
+{
+	struct mlxbf_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+
+	if (offset < MLXBF_NGPIOS_GPIO0)
+		writel(BIT(offset), priv->base + MLXBF_GPIO0_FW_CONTROL_SET);
+	else
+		writel(BIT(offset % MLXBF_NGPIOS_GPIO0), priv->base + MLXBF_GPIO1_FW_CONTROL_SET);
+
+	return 0;
+}
+
+static void mlxbf_gpio_disable_free(struct pinctrl_dev *pctldev,
+				    struct pinctrl_gpio_range *range,
+				    unsigned int offset)
+{
+	struct mlxbf_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+
+	/* disable GPIO functionality by giving control back to hardware */
+	if (offset < MLXBF_NGPIOS_GPIO0)
+		writel(BIT(offset), priv->base + MLXBF_GPIO0_FW_CONTROL_CLEAR);
+	else
+		writel(BIT(offset % MLXBF_NGPIOS_GPIO0), priv->base + MLXBF_GPIO1_FW_CONTROL_CLEAR);
+
+}
+
+static const struct pinmux_ops mlxbf_pmx_ops = {
+	.get_functions_count = mlxbf_pmx_get_funcs_count,
+	.get_function_name = mlxbf_pmx_get_func_name,
+	.get_function_groups = mlxbf_pmx_get_groups,
+	.set_mux = mlxbf_pmx_set,
+	.gpio_request_enable = mlxbf_gpio_request_enable,
+	.gpio_disable_free = mlxbf_gpio_disable_free,
+};
+
+static struct pinctrl_desc mlxbf_pin_desc = {
+	.name = "pinctrl-mlxbf",
+	.pins = mlxbf_pins,
+	.npins = ARRAY_SIZE(mlxbf_pins),
+	.pctlops = &mlxbf_pinctrl_group_ops,
+	.pmxops = &mlxbf_pmx_ops,
+	.owner = THIS_MODULE,
+};
+
+static_assert(ARRAY_SIZE(mlxbf_pinctrl_single_group_names) ==
+	      ARRAY_SIZE(mlxbf_pinctrl_single_group_pins));
+
+static int mlxbf_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mlxbf_pinctrl *priv;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+
+	/* This resource is shared so use devm_ioremap */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	priv->base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!priv->base)
+		return -ENOMEM;
+
+	ret = devm_pinctrl_register_and_init(priv->dev,
+					     &mlxbf_pin_desc,
+					     priv,
+					     &priv->pctl);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
+
+	ret = pinctrl_enable(priv->pctl);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable pinctrl\n");
+
+	pinctrl_add_gpio_range(priv->pctl, &mlxbf_pinctrl_gpio_ranges[0]);
+	pinctrl_add_gpio_range(priv->pctl, &mlxbf_pinctrl_gpio_ranges[1]);
+
+	return 0;
+}
+
+static const struct acpi_device_id mlxbf_pinctrl_acpi_ids[] = {
+	{ "MLNXBF34", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, mlxbf_pinctrl_acpi_ids);
+
+static struct platform_driver mlxbf_pinctrl_driver = {
+	.driver = {
+		.name = "pinctrl-mlxbf",
+		.acpi_match_table = mlxbf_pinctrl_acpi_ids,
+	},
+	.probe = mlxbf_pinctrl_probe,
+};
+module_platform_driver(mlxbf_pinctrl_driver);
+
+MODULE_DESCRIPTION("NVIDIA pinctrl driver");
+MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>");
+MODULE_LICENSE("Dual BSD/GPL");