Support Nvidia BlueField-3 GPIO driver and pin controller
Subsystems (3)
v5 cover Support Nvidia BlueField-3 GPIO driver and pin controller
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
Signature · 1 line
2.30.1
PATCH v5 1/2 gpio: gpio-mlxbf3: Add gpio driver support
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.
--- 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
drivers/gpio/Kconfig +13 -0
@@ -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
drivers/gpio/Makefile +1 -0
@@ -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
drivers/gpio/gpio-mlxbf3.c new file +245 -0
245 lines not on this page · raw message
Signature · 1 line
2.30.1
Re: [PATCH v5 1/2] gpio: gpio-mlxbf3: Add gpio driver support
Quoted 5 lines · 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.
Thank you for the update, my comments below.
...
drivers/gpio/Kconfig config GPIO_MLXBF2 · line 1547
Quoted 2 lines
drivers/gpio/Kconfig +2
-0+ 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?
...
Quoted 1 line
+#include <
linux/resource.h>
I dunno why you added this one.
The missing ones are:err.hio.h
...
drivers/gpio/gpio-mlxbf3.c line 60
Quoted 2 lines
drivers/gpio/gpio-mlxbf3.c new file +2
-0+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.
Quoted 6 lines
drivers/gpio/gpio-mlxbf3.c new file +6
-0+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);
...
drivers/gpio/gpio-mlxbf3.c line 81
Quoted 5 lines
drivers/gpio/gpio-mlxbf3.c new file +5
-0+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.
Quoted 2 lines
(quoted diff) +2
-0+ raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags); +}
...
Quoted 1 line
+/* This function needs to be defined for
handle_edge_irq*/
handle_edge_irq()
...
Quoted 1 line
+static const struct
acpi_device_id__maybe_unusedmlxbf3_gpio_acpi_match[] = {
Why __maybe_unused?
Quoted 3 lines
drivers/gpio/gpio-mlxbf3.c new file +3
-0+ { "MLNXBF33", 0 }, + {} +};
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v5 1/2] gpio: gpio-mlxbf3: Add gpio driver support
Quoted 1 line · Andy Shevchenko
On Wed, Mar 15, 2023 at 12:16 AM Asmaa Mnebhi <[email protected]> wrote:
...
Quoted 1 line
Btw, have you considered renaming that driver to pinctrl-mlxbf3?
And Subject here should be "gpio: mlxbf3: ...".
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v5 2/2 pinctrl: pinctrl-mlxbf: Add pinctrl driver support
In reply to Asmaa MnebhiNVIDIA 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/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
drivers/pinctrl/Kconfig +13 -0
@@ -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"
drivers/pinctrl/Makefile +1 -0
@@ -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
drivers/pinctrl/pinctrl-mlxbf3.c new file +318 -0
318 lines not on this page · raw message
Signature · 1 line
2.30.1
Re: [PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support
Quoted 4 lines · 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.
...
Quoted 1 line
drivers/pinctrl/pinctrl-mlxbf3.c | 318 +++++++++++++++++++++++++++++++
Ah, cool, so it's a typo in the GPIO driver then.
...
drivers/pinctrl/pinctrl-mlxbf3.c line 9
Quoted 6 lines
drivers/pinctrl/pinctrl-mlxbf3.c new file +6
-0+#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
...
drivers/pinctrl/pinctrl-mlxbf3.c line 28
Quoted 8 lines
drivers/pinctrl/pinctrl-mlxbf3.c new file +7
-0+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.
Quoted 2 lines
(quoted diff) +2
-0+ struct pinctrl_gpio_range gpio_range; +};
...
drivers/pinctrl/pinctrl-mlxbf3.c line 119
Quoted 8 lines
drivers/pinctrl/pinctrl-mlxbf3.c new file +8
-0+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.
Quoted 1 line
+};
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support
Quoted 1 line · Andy Shevchenko
On Wed, Mar 15, 2023 at 12:16 AM Asmaa Mnebhi <[email protected]> wrote:
...
Quoted 3 lines
Quoted 1 line
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: ...".
Signature · 2 lines
With Best Regards, Andy Shevchenko
RE: [PATCH v5 2/2] pinctrl: pinctrl-mlxbf: Add pinctrl driver support
In reply to Andy ShevchenkoQuoted 6 lines
Quoted 5 lines
drivers/pinctrl/pinctrl-mlxbf3.c new file +5
-0+struct mlxbf3_pinctrl { +void __iomem *fw_ctrl_set0; +void __iomem *fw_ctrl_clr0; +void __iomem *fw_ctrl_set1; +void __iomem *fw_ctrl_clr1;
Quoted 6 lines
drivers/pinctrl/pinctrl-mlxbf3.c line 28
Quoted 2 lines
drivers/pinctrl/pinctrl-mlxbf3.c new file +2
-0+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.
Quoted 10 lines · 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.cdriver handles hardware registers and logic that are different fromgpio-mlxbf.candgpio-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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
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
Quoted 10 lines
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