[PATCH v5 0/2] Support Nvidia BlueField-3 GPIO driver and pin controller

[PATCH v5 0/2] Support Nvidia BlueField-3 GPIO driver and pin controller

[Asmaa Mnebhi]

Support the BlueField-3 SoC GPIO driver for handling interrupts and
providing the option to change the direction and value of a GPIO.
Support the BlueField-3 SoC pin controller driver for allowing a
select number of GPIO pins to be manipulated from userspace or
the kernel.

The gpio-mlxbf3.c driver handles hardware registers and logic
that are different from gpio-mlxbf.c and gpio-mlxbf2.c.
For that reason, we have separate drivers for each generation.

Changes from v4->v5:

gpio-mlxbf3.c:
- Update Kconfig dependency in gpio
- remove version.h header
- use irq_hw_number_t
- release lock in mlxbf3_gpio_irq_set_type
- add IRQCHIP_IMMUTABLE and GPIOCHIP_IRQ_RESOURCE_HELPERS
- remove npins property to use ngpios instead
- Assign handle_bad_irq() in probe
- Use handle_edge_irq instead  

pinctrl-mlxbf3.c:
- change the driver name to pinctrl-mlxbf3
- alignment cleanup
- use PINCTRL_PINFUNCTION
- use pinctrl_add_gpio_ranges
- use devm_platform_ioremap_resource


Asmaa Mnebhi (2):
  gpio: gpio-mlxbf3: Add gpio driver support
  pinctrl: pinctrl-mlxbf: Add pinctrl driver support

 drivers/gpio/Kconfig             |  13 ++
 drivers/gpio/Makefile            |   1 +
 drivers/gpio/gpio-mlxbf3.c       | 245 ++++++++++++++++++++++++
 drivers/pinctrl/Kconfig          |  13 ++
 drivers/pinctrl/Makefile         |   1 +
 drivers/pinctrl/pinctrl-mlxbf3.c | 318 +++++++++++++++++++++++++++++++
 6 files changed, 591 insertions(+)
 create mode 100644 drivers/gpio/gpio-mlxbf3.c
 create mode 100644 drivers/pinctrl/pinctrl-mlxbf3.c

-- 
2.30.1

[PATCH v5 1/2] gpio: gpio-mlxbf3: Add gpio driver support

[Asmaa Mnebhi]

Add support for the BlueField-3 SoC GPIO driver.
This driver configures and handles GPIO interrupts. It also enables a user
to manipulate certain GPIO pins via libgpiod tools or other kernel drivers.
The usables pins are defined via the "gpio-reserved-ranges" property.

Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/gpio/Kconfig       |  13 ++
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-mlxbf3.c | 245 +++++++++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+)
 create mode 100644 drivers/gpio/gpio-mlxbf3.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ec7cfd4f52b1..3b67e1591519 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1534,6 +1534,19 @@ config GPIO_MLXBF2
 	help
 	  Say Y here if you want GPIO support on Mellanox BlueField 2 SoC.
 
+config GPIO_MLXBF3
+	tristate "Mellanox BlueField 3 SoC GPIO"
+	depends on (MELLANOX_PLATFORM && ARM64) || COMPILE_TEST
+	select GPIO_GENERIC
+	select GPIOLIB_IRQCHIP
+	help
+	  Say Y if you want GPIO support on Mellanox BlueField 3 SoC.
+	  This GPIO controller supports interrupt handling and enables the
+	  manipulation of certain GPIO pins.
+	  This controller should be used in parallel with pinctrl-mlxbf to
+	  control the desired gpios.
+	  This driver can also be built as a module called mlxbf3-gpio.
+
 config GPIO_ML_IOH
 	tristate "OKI SEMICONDUCTOR ML7213 IOH GPIO support"
 	depends on X86 || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 010587025fc8..76545ca31457 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_GPIO_MERRIFIELD)		+= gpio-merrifield.o
 obj-$(CONFIG_GPIO_ML_IOH)		+= gpio-ml-ioh.o
 obj-$(CONFIG_GPIO_MLXBF)		+= gpio-mlxbf.o
 obj-$(CONFIG_GPIO_MLXBF2)		+= gpio-mlxbf2.o
+obj-$(CONFIG_GPIO_MLXBF3)		+= gpio-mlxbf3.o
 obj-$(CONFIG_GPIO_MM_LANTIQ)		+= gpio-mm-lantiq.o
 obj-$(CONFIG_GPIO_MOCKUP)		+= gpio-mockup.o
 obj-$(CONFIG_GPIO_MOXTET)		+= gpio-moxtet.o
diff --git a/drivers/gpio/gpio-mlxbf3.c b/drivers/gpio/gpio-mlxbf3.c
new file mode 100644
index 000000000000..4225f6d38db3
--- /dev/null
+++ b/drivers/gpio/gpio-mlxbf3.c
@@ -0,0 +1,245 @@
+// 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/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+/*
+ * There are 2 YU GPIO blocks:
+ * gpio[0]: HOST_GPIO0->HOST_GPIO31
+ * gpio[1]: HOST_GPIO32->HOST_GPIO55
+ */
+#define MLXBF3_GPIO_MAX_PINS_PER_BLOCK 32
+
+/*
+ * fw_gpio[x] block registers and their offset
+ */
+#define MLXBF_GPIO_FW_OUTPUT_ENABLE_SET	  0x00
+#define MLXBF_GPIO_FW_DATA_OUT_SET        0x04
+
+#define MLXBF_GPIO_FW_OUTPUT_ENABLE_CLEAR 0x00
+#define MLXBF_GPIO_FW_DATA_OUT_CLEAR      0x04
+
+#define MLXBF_GPIO_CAUSE_RISE_EN          0x00
+#define MLXBF_GPIO_CAUSE_FALL_EN          0x04
+#define MLXBF_GPIO_READ_DATA_IN           0x08
+
+#define MLXBF_GPIO_CAUSE_OR_CAUSE_EVTEN0  0x00
+#define MLXBF_GPIO_CAUSE_OR_EVTEN0        0x14
+#define MLXBF_GPIO_CAUSE_OR_CLRCAUSE      0x18
+
+struct mlxbf3_gpio_context {
+	struct gpio_chip gc;
+
+	/* YU GPIO block address */
+	void __iomem *gpio_set_io;
+	void __iomem *gpio_clr_io;
+	void __iomem *gpio_io;
+
+	/* YU GPIO cause block address */
+	void __iomem *gpio_cause_io;
+};
+
+static void mlxbf3_gpio_irq_enable(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc);
+	irq_hw_number_t offset = irqd_to_hwirq(irqd);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
+	gpiochip_enable_irq(gc, offset);
+	writel(BIT(offset), gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CLRCAUSE);
+
+	val = readl(gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
+	val |= BIT(offset);
+	writel(val, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
+	raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+}
+
+static void mlxbf3_gpio_irq_disable(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc);
+	irq_hw_number_t offset = irqd_to_hwirq(irqd);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
+	val = readl(gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
+	val &= ~BIT(offset);
+	writel(val, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
+	gpiochip_disable_irq(gc, offset);
+	raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+}
+
+static irqreturn_t mlxbf3_gpio_irq_handler(int irq, void *ptr)
+{
+	struct mlxbf3_gpio_context *gs = ptr;
+	struct gpio_chip *gc = &gs->gc;
+	unsigned long pending;
+	u32 level;
+
+	pending = readl(gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CAUSE_EVTEN0);
+	writel(pending, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CLRCAUSE);
+
+	for_each_set_bit(level, &pending, gc->ngpio)
+		generic_handle_domain_irq(gc->irq.domain, level);
+
+	return IRQ_RETVAL(pending);
+}
+
+static int
+mlxbf3_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc);
+	irq_hw_number_t offset = irqd_to_hwirq(irqd);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_BOTH:
+		val = readl(gs->gpio_io + MLXBF_GPIO_CAUSE_FALL_EN);
+		val |= BIT(offset);
+		writel(val, gs->gpio_io + MLXBF_GPIO_CAUSE_FALL_EN);
+		val = readl(gs->gpio_io + MLXBF_GPIO_CAUSE_RISE_EN);
+		val |= BIT(offset);
+		writel(val, gs->gpio_io + MLXBF_GPIO_CAUSE_RISE_EN);
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		val = readl(gs->gpio_io + MLXBF_GPIO_CAUSE_RISE_EN);
+		val |= BIT(offset);
+		writel(val, gs->gpio_io + MLXBF_GPIO_CAUSE_RISE_EN);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		val = readl(gs->gpio_io + MLXBF_GPIO_CAUSE_FALL_EN);
+		val |= BIT(offset);
+		writel(val, gs->gpio_io + MLXBF_GPIO_CAUSE_FALL_EN);
+		break;
+	default:
+		raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+		return -EINVAL;
+	}
+
+	raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+
+	irq_set_handler_locked(irqd, handle_edge_irq);
+
+	return 0;
+}
+
+/* This function needs to be defined for handle_edge_irq */
+static void mlxbf3_gpio_irq_ack(struct irq_data *data)
+{
+}
+
+static const struct irq_chip gpio_mlxbf3_irqchip = {
+	.name = "MLNXBF33",
+	.irq_ack = mlxbf3_gpio_irq_ack,
+	.irq_set_type = mlxbf3_gpio_irq_set_type,
+	.irq_enable = mlxbf3_gpio_irq_enable,
+	.irq_disable = mlxbf3_gpio_irq_disable,
+	.flags = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int mlxbf3_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mlxbf3_gpio_context *gs;
+	struct gpio_irq_chip *girq;
+	struct gpio_chip *gc;
+	int ret, irq;
+
+	gs = devm_kzalloc(dev, sizeof(*gs), GFP_KERNEL);
+	if (!gs)
+		return -ENOMEM;
+
+	gs->gpio_io = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(gs->gpio_io))
+		return PTR_ERR(gs->gpio_io);
+
+	gs->gpio_cause_io = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(gs->gpio_cause_io))
+		return PTR_ERR(gs->gpio_cause_io);
+
+	gs->gpio_set_io = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(gs->gpio_set_io))
+		return PTR_ERR(gs->gpio_set_io);
+
+	gs->gpio_clr_io = devm_platform_ioremap_resource(pdev, 3);
+	if (IS_ERR(gs->gpio_clr_io))
+		return PTR_ERR(gs->gpio_clr_io);
+	gc = &gs->gc;
+
+	ret = bgpio_init(gc, dev, 4,
+			gs->gpio_io + MLXBF_GPIO_READ_DATA_IN,
+			gs->gpio_set_io + MLXBF_GPIO_FW_DATA_OUT_SET,
+			gs->gpio_clr_io + MLXBF_GPIO_FW_DATA_OUT_CLEAR,
+			gs->gpio_set_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_SET,
+			gs->gpio_clr_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_CLEAR, 0);
+
+	gc->request = gpiochip_generic_request;
+	gc->free = gpiochip_generic_free;
+	gc->owner = THIS_MODULE;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq >= 0) {
+		girq = &gs->gc.irq;
+		gpio_irq_chip_set_chip(girq, &gpio_mlxbf3_irqchip);
+		girq->default_type = IRQ_TYPE_NONE;
+		/* This will let us handle the parent IRQ in the driver */
+		girq->num_parents = 0;
+		girq->parents = NULL;
+		girq->parent_handler = NULL;
+		girq->handler = handle_bad_irq;
+
+		/*
+		 * Directly request the irq here instead of passing
+		 * a flow-handler because the irq is shared.
+		 */
+		ret = devm_request_irq(dev, irq, mlxbf3_gpio_irq_handler,
+				       IRQF_SHARED, dev_name(dev), gs);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to request IRQ");
+	}
+
+	platform_set_drvdata(pdev, gs);
+
+	ret = devm_gpiochip_add_data(dev, &gs->gc, gs);
+	if (ret)
+		dev_err_probe(dev, ret, "Failed adding memory mapped gpiochip\n");
+
+	return 0;
+}
+
+static const struct acpi_device_id __maybe_unused mlxbf3_gpio_acpi_match[] = {
+	{ "MLNXBF33", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, mlxbf3_gpio_acpi_match);
+
+static struct platform_driver mlxbf3_gpio_driver = {
+	.driver = {
+		.name = "mlxbf3_gpio",
+		.acpi_match_table = mlxbf3_gpio_acpi_match,
+	},
+	.probe    = mlxbf3_gpio_probe,
+};
+module_platform_driver(mlxbf3_gpio_driver);
+
+MODULE_DESCRIPTION("NVIDIA BlueField-3 GPIO Driver");
+MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.30.1

[PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support

[Asmaa Mnebhi]

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 pin muxing.

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

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 7d5f5458c72e..9009bc6adbea 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -523,6 +523,19 @@ config PINCTRL_ZYNQMP
 	  This driver can also be built as a module. If so, the module
 	  will be called pinctrl-zynqmp.
 
+config PINCTRL_MLXBF3
+	tristate "NVIDIA BlueField-3 SoC Pinctrl driver"
+	depends on (MELLANOX_PLATFORM && ARM64) || 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 allows selecting the mux function for
+	  each pin. This driver can also be built as a module called
+	  pinctrl-mlxbf3.
+
 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..10dd072e8423 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_MLXBF3)	+= pinctrl-mlxbf3.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-mlxbf3.c b/drivers/pinctrl/pinctrl-mlxbf3.c
new file mode 100644
index 000000000000..19293eddc2e8
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mlxbf3.c
@@ -0,0 +1,318 @@
+// 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 MLXBF3_NGPIOS_GPIO0    32
+#define MLXBF3_MAX_GPIO_PINS   56
+
+enum {
+	MLXBF3_GPIO_HW_MODE,
+	MLXBF3_GPIO_SW_MODE,
+};
+
+struct mlxbf3_pinctrl {
+	void __iomem *fw_ctrl_set0;
+	void __iomem *fw_ctrl_clr0;
+	void __iomem *fw_ctrl_set1;
+	void __iomem *fw_ctrl_clr1;
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	struct pinctrl_gpio_range gpio_range;
+};
+
+#define MLXBF3_GPIO_RANGE(_id, _pinbase, _gpiobase, _npins)	\
+	{							\
+		.name = "mlxbf3_gpio_range",			\
+		.id = _id,					\
+		.base = _gpiobase,				\
+		.pin_base = _pinbase,				\
+		.npins = _npins,				\
+	}
+
+static struct pinctrl_gpio_range mlxbf3_pinctrl_gpio_ranges[] = {
+	MLXBF3_GPIO_RANGE(0, 0,  480, 32),
+	MLXBF3_GPIO_RANGE(1,  32, 456, 24),
+};
+
+static const struct pinctrl_pin_desc mlxbf3_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 mlxbf3_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"
+};
+
+static int mlxbf3_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	/* Number single-pin groups */
+	return MLXBF3_MAX_GPIO_PINS;
+}
+
+static const char *mlxbf3_get_group_name(struct pinctrl_dev *pctldev,
+					 unsigned int selector)
+{
+	return mlxbf3_pinctrl_single_group_names[selector];
+}
+
+static int mlxbf3_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 = &selector;
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const struct pinctrl_ops mlxbf3_pinctrl_group_ops = {
+	.get_groups_count = mlxbf3_get_groups_count,
+	.get_group_name = mlxbf3_get_group_name,
+	.get_group_pins = mlxbf3_get_group_pins,
+};
+
+/*
+ * Only 2 functions are supported and they apply to all pins:
+ * 1) Default hardware functionality
+ * 2) Software controlled GPIO
+ */
+static const char * const mlxbf3_gpiofunc_group_names[] = { "swctrl" };
+static const char * const mlxbf3_hwfunc_group_names[]   = { "hwctrl" };
+
+struct pinfunction mlxbf3_pmx_funcs[] = {
+	PINCTRL_PINFUNCTION("hwfunc", mlxbf3_hwfunc_group_names, 1),
+	PINCTRL_PINFUNCTION("gpiofunc", mlxbf3_gpiofunc_group_names, 1),
+};
+
+static int mlxbf3_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(mlxbf3_pmx_funcs);
+}
+
+static const char *mlxbf3_pmx_get_func_name(struct pinctrl_dev *pctldev,
+					   unsigned int selector)
+{
+	return mlxbf3_pmx_funcs[selector].name;
+}
+
+static int mlxbf3_pmx_get_groups(struct pinctrl_dev *pctldev,
+				 unsigned int selector,
+				 const char * const **groups,
+				 unsigned int * const num_groups)
+{
+	*groups = mlxbf3_pmx_funcs[selector].groups;
+	*num_groups = MLXBF3_MAX_GPIO_PINS;
+
+	return 0;
+}
+
+static int mlxbf3_pmx_set(struct pinctrl_dev *pctldev,
+			      unsigned int selector,
+			      unsigned int group)
+{
+	struct mlxbf3_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+
+	if (selector == MLXBF3_GPIO_HW_MODE) {
+		if (group < MLXBF3_NGPIOS_GPIO0)
+			writel(BIT(group), priv->fw_ctrl_clr0);
+		else
+			writel(BIT(group % MLXBF3_NGPIOS_GPIO0), priv->fw_ctrl_clr1);
+	}
+
+	if (selector == MLXBF3_GPIO_SW_MODE) {
+		if (group < MLXBF3_NGPIOS_GPIO0)
+			writel(BIT(group), priv->fw_ctrl_set0);
+		else
+			writel(BIT(group % MLXBF3_NGPIOS_GPIO0), priv->fw_ctrl_set1);
+	}
+
+	return 0;
+}
+
+static int mlxbf3_gpio_request_enable(struct pinctrl_dev *pctldev,
+				     struct pinctrl_gpio_range *range,
+				     unsigned int offset)
+{
+	struct mlxbf3_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+
+	if (offset < MLXBF3_NGPIOS_GPIO0)
+		writel(BIT(offset), priv->fw_ctrl_set0);
+	else
+		writel(BIT(offset % MLXBF3_NGPIOS_GPIO0), priv->fw_ctrl_set1);
+
+	return 0;
+}
+
+static void mlxbf3_gpio_disable_free(struct pinctrl_dev *pctldev,
+				    struct pinctrl_gpio_range *range,
+				    unsigned int offset)
+{
+	struct mlxbf3_pinctrl *priv = pinctrl_dev_get_drvdata(pctldev);
+
+	/* disable GPIO functionality by giving control back to hardware */
+	if (offset < MLXBF3_NGPIOS_GPIO0)
+		writel(BIT(offset), priv->fw_ctrl_clr0);
+	else
+		writel(BIT(offset % MLXBF3_NGPIOS_GPIO0), priv->fw_ctrl_clr1);
+}
+
+static const struct pinmux_ops mlxbf3_pmx_ops = {
+	.get_functions_count = mlxbf3_pmx_get_funcs_count,
+	.get_function_name = mlxbf3_pmx_get_func_name,
+	.get_function_groups = mlxbf3_pmx_get_groups,
+	.set_mux = mlxbf3_pmx_set,
+	.gpio_request_enable = mlxbf3_gpio_request_enable,
+	.gpio_disable_free = mlxbf3_gpio_disable_free,
+};
+
+static struct pinctrl_desc mlxbf3_pin_desc = {
+	.name = "pinctrl-mlxbf3",
+	.pins = mlxbf3_pins,
+	.npins = ARRAY_SIZE(mlxbf3_pins),
+	.pctlops = &mlxbf3_pinctrl_group_ops,
+	.pmxops = &mlxbf3_pmx_ops,
+	.owner = THIS_MODULE,
+};
+
+static_assert(ARRAY_SIZE(mlxbf3_pinctrl_single_group_names) == MLXBF3_MAX_GPIO_PINS);
+
+static int mlxbf3_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mlxbf3_pinctrl *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+
+	priv->fw_ctrl_set0 = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->fw_ctrl_set0))
+		return PTR_ERR(priv->fw_ctrl_set0);
+
+	priv->fw_ctrl_clr0 = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(priv->fw_ctrl_set0))
+		return PTR_ERR(priv->fw_ctrl_set0);
+
+	priv->fw_ctrl_set1 = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(priv->fw_ctrl_set0))
+		return PTR_ERR(priv->fw_ctrl_set0);
+
+	priv->fw_ctrl_clr1 = devm_platform_ioremap_resource(pdev, 3);
+	if (IS_ERR(priv->fw_ctrl_set0))
+		return PTR_ERR(priv->fw_ctrl_set0);
+
+	ret = devm_pinctrl_register_and_init(dev,
+					     &mlxbf3_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_ranges(priv->pctl, mlxbf3_pinctrl_gpio_ranges, 2);
+
+	return 0;
+}
+
+static const struct acpi_device_id mlxbf3_pinctrl_acpi_ids[] = {
+	{ "MLNXBF34", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, mlxbf3_pinctrl_acpi_ids);
+
+static struct platform_driver mlxbf3_pinctrl_driver = {
+	.driver = {
+		.name = "pinctrl-mlxbf3",
+		.acpi_match_table = mlxbf3_pinctrl_acpi_ids,
+	},
+	.probe = mlxbf3_pinctrl_probe,
+};
+module_platform_driver(mlxbf3_pinctrl_driver);
+
+MODULE_DESCRIPTION("NVIDIA pinctrl driver");
+MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.30.1

Re: [PATCH v5 1/2] gpio: gpio-mlxbf3: Add gpio driver support

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 12:16 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> Add support for the BlueField-3 SoC GPIO driver.
> This driver configures and handles GPIO interrupts. It also enables a user
> to manipulate certain GPIO pins via libgpiod tools or other kernel drivers.
> The usables pins are defined via the "gpio-reserved-ranges" property.

Thank you for the update, my comments below.

...

> +         This controller should be used in parallel with pinctrl-mlxbf to
> +         control the desired gpios.

GPIOs

Btw, have you considered renaming that driver to pinctrl-mlxbf3?

...

> +#include <linux/resource.h>

I dunno why you added this one.

The missing ones are:
err.h
io.h

...

> +       raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +       gpiochip_enable_irq(gc, offset);

No need to call this under the spin lock, or must be explained why.

> +       writel(BIT(offset), gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CLRCAUSE);
> +
> +       val = readl(gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
> +       val |= BIT(offset);
> +       writel(val, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
> +       raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);

...

> +       raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +       val = readl(gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
> +       val &= ~BIT(offset);
> +       writel(val, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0);
> +       gpiochip_disable_irq(gc, offset);

Ditto.

> +       raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
> +}

...

> +/* This function needs to be defined for handle_edge_irq */

handle_edge_irq()

...

> +static const struct acpi_device_id __maybe_unused mlxbf3_gpio_acpi_match[] = {

Why __maybe_unused?

> +       { "MLNXBF33", 0 },
> +       {}
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 12:16 AM 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 pin muxing.

...

>  drivers/pinctrl/pinctrl-mlxbf3.c | 318 +++++++++++++++++++++++++++++++

Ah, cool, so it's a typo in the GPIO driver then.

...

> +#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>

+ err.h
+ types.h

...

> +struct mlxbf3_pinctrl {
> +       void __iomem *fw_ctrl_set0;
> +       void __iomem *fw_ctrl_clr0;
> +       void __iomem *fw_ctrl_set1;
> +       void __iomem *fw_ctrl_clr1;

> +       struct device *dev;
> +       struct pinctrl_dev *pctl;

Depending on what is used more often in the code you can shuffle the
order and save a few bytes in the generated code. You may play with
bloat-o-meter to check.

> +       struct pinctrl_gpio_range gpio_range;
> +};

...

> +static const char * const mlxbf3_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"

You can leave the trailing comma.

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/2] Support Nvidia BlueField-3 GPIO driver and pin controller

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 12:16 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> Support the BlueField-3 SoC GPIO driver for handling interrupts and
> providing the option to change the direction and value of a GPIO.
> Support the BlueField-3 SoC pin controller driver for allowing a
> select number of GPIO pins to be manipulated from userspace or
> the kernel.
>
> The gpio-mlxbf3.c driver handles hardware registers and logic
> that are different from gpio-mlxbf.c and gpio-mlxbf2.c.
> For that reason, we have separate drivers for each generation.

This one is in pretty good shape, a few minor things to be addressed
and the v6 I believe will be ready to go.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 11:21 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Mar 15, 2023 at 12:16 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:

...

> >  drivers/pinctrl/pinctrl-mlxbf3.c | 318 +++++++++++++++++++++++++++++++
>
> Ah, cool, so it's a typo in the GPIO driver then.

And Subject here should be "pinctrl: mlxbf3: ...".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/2] gpio: gpio-mlxbf3: Add gpio driver support

[Andy Shevchenko]

On Wed, Mar 15, 2023 at 11:15 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Mar 15, 2023 at 12:16 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:

...

> Btw, have you considered renaming that driver to pinctrl-mlxbf3?

And Subject here should be "gpio: mlxbf3: ...".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/2] Support Nvidia BlueField-3 GPIO driver and pin controller

[Linus Walleij]

Hi Asamaa,

thanks for your patches!

Once you have addressed the final review comments from Andy,
do these drivers have compile time depenencies so we need to merge
them both at the same time or can the two patches be applied
individually to GPIO and pin control?

BTW: big thanks to Andy for the big investment in reviewing this
driver!

Yours,
Linus Walleij

RE: [PATCH v5 0/2] Support Nvidia BlueField-3 GPIO driver and pin controller

[Asmaa Mnebhi]

> Hi Asamaa,
> 
> thanks for your patches!
> 
> Once you have addressed the final review comments from Andy, do these
> drivers have compile time depenencies so we need to merge them both at
> the same time or can the two patches be applied individually to GPIO and
> pin control?
> 
> BTW: big thanks to Andy for the big investment in reviewing this driver!

Hi Linus,

These drivers don’t have any compile dependency on each other.

And yes! Thank you Andy for your review, I learned a lot!

Thanks.
Asmaa

RE: [PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support

[Asmaa Mnebhi]

> > +struct mlxbf3_pinctrl {
> > +       void __iomem *fw_ctrl_set0;
> > +       void __iomem *fw_ctrl_clr0;
> > +       void __iomem *fw_ctrl_set1;
> > +       void __iomem *fw_ctrl_clr1;
> 
> > +       struct device *dev;
> > +       struct pinctrl_dev *pctl;
> 
> Depending on what is used more often in the code you can shuffle the order
> and save a few bytes in the generated code. You may play with bloat-o-
> meter to check.

I moved around some of the structs above and it hasn’t changed the size of the struct. I also played around
With bloat-o-meter to compare the generated .o files and I don’t see a difference.