Fix Nvidia BlueField-3 GPIO access
Subsystems (2)
cover Fix Nvidia BlueField-3 GPIO access
Fix Nvidia BlueField-3 GPIO access via libgpiod gpioset tool. gpioset tool fails to modify the GPIO value due to the following:
1) the pinctrl-mlxbf3 driver defines mlxbf3_gpio_request_enable() to enable software to take control over a gpio. Only then can the gpio-mlxbf3 driver modify the direction and value of the gpio. mlxbf3_gpio_disable_free() gives control back to hardware and is called when the "gpioset" command is invoked. This cancels out the effort to change the GPIO value and direction. So mlxbf3_gpio_disable_free() needs to be removed.
2) the gpio-mlxbf3 driver calls gpiochip_generic_request() which calls mlxbf3_gpio_request_enable(). "pin_ranges" needs not to be empty for mlxbf3_gpio_request_enable() to be invoked. So gpio-mlxbf3 needs to populate "pin_ranges".
Asmaa Mnebhi (2):
pinctrl: mlxbf3: Remove gpio_disable_free()
gpio: mlxbf3: Support add_pin_ranges()
drivers/gpio/gpio-mlxbf3.c | 20 ++++++++++++++++++++ drivers/pinctrl/pinctrl-mlxbf3.c | 14 -------------- 2 files changed, 20 insertions(+), 14 deletions(-)
Signature · 1 line
2.30.1
PATCH 1/2 pinctrl: mlxbf3: Remove gpio_disable_free()
Remove support for gpio_disable_free() because it is called when the libgpiod command "gpiset" is invoked. This gives the GPIO control back to hardware which cancels out the effort to set the GPIO value.
Reminder of the code flow to change a GPIO value from software:
1) All GPIOs are controlled by hardware by default
2) To change the GPIO value, enable software control via a mux.
3) Once software has control over the GPIO pin, the gpio-mlxbf3 driver will be able to change the direction and value of the GPIO.
When the user runs "gpioset gpiochip0 0=0" for example, the gpio pin value should change from 1 to 0. In this case, mlxbf3_gpio_request_enable() is called via gpiochip_generic_request(). The latter switches GPIO control from hardware to software. Then the GPIO value is changed from 1 to 0. However, gpio_disable_free() is also called which changes control back to hardware which changes the GPIO value back to 1.
--- drivers/pinctrl/pinctrl-mlxbf3.c | 14 -------------- 1 file changed, 14 deletions(-)
drivers/pinctrl/pinctrl-mlxbf3.c +0 -14
@@ -223,26 +223,12 @@ static int mlxbf3_gpio_request_enable(struct pinctrl_dev *pctldev,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 = {
Signature · 1 line
2.30.1
PATCH 2/2 gpio: mlxbf3: Support add_pin_ranges()
In reply to Asmaa MnebhiSupport add_pin_ranges() so that pinctrl_gpio_request() can be called. The GPIO value is not modified when the user runs the "gpioset" tool. This is because when gpiochip_generic_request is invoked by the gpio-mlxbf3 driver, "pin_ranges" is empty so it skips "pinctrl_gpio_request()". pinctrl_gpio_request() is essential in the code flow because it changes the mux value so that software has control over modifying the GPIO value. Adding add_pin_ranges() creates a dependency on the pinctrl-mlxbf3.c driver.
--- drivers/gpio/gpio-mlxbf3.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
drivers/gpio/gpio-mlxbf3.c +20 -0
@@ -158,6 +158,24 @@ static const struct irq_chip gpio_mlxbf3_irqchip = { GPIOCHIP_IRQ_RESOURCE_HELPERS, }; +static int mlxbf3_gpio_add_pin_ranges (struct gpio_chip *chip) +{ +int ret; +int id =0 ; + + id = !!(chip->ngpio %MLXBF3_GPIO_MAX_PINS_PER_BLOCK ); + + ret =gpiochip_add_pin_range (chip, +"MLNXBF34:00" , + chip->base, + id *MLXBF3_GPIO_MAX_PINS_PER_BLOCK , + chip->ngpio); +if (ret) +return ret; + +return 0 ; +} +static int mlxbf3_gpio_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev;
@@ -197,6 +215,7 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev) gc->request = gpiochip_generic_request; gc->free = gpiochip_generic_free; gc->owner =THIS_MODULE ; + gc->add_pin_ranges = mlxbf3_gpio_add_pin_ranges; irq =platform_get_irq (pdev,0 );if (irq >=0 ) {
@@ -243,6 +262,7 @@ static struct platform_driver mlxbf3_gpio_driver = { };module_platform_driver (mlxbf3_gpio_driver); +MODULE_SOFTDEP ("pre: pinctrl-mlxbf3" );MODULE_DESCRIPTION ("NVIDIA BlueField-3 GPIO Driver" );MODULE_AUTHOR ("Asmaa Mnebhi <[email protected]>" );MODULE_LICENSE ("Dual BSD/GPL" );
Signature · 1 line
2.30.1
Re: [PATCH v1 2/2] gpio: mlxbf3: Support add_pin_ranges()
Quoted 8 lines · Asmaa Mnebhi
Support
add_pin_ranges()so thatpinctrl_gpio_request()can be called. The GPIO value is not modified when the user runs the "gpioset" tool. This is because whengpiochip_generic_requestis invoked by the gpio-mlxbf3 driver, "pin_ranges" is empty so it skips "pinctrl_gpio_request()".pinctrl_gpio_request()is essential in the code flow because it changes the mux value so that software has control over modifying the GPIO value. Addingadd_pin_ranges()creates a dependency on thepinctrl-mlxbf3.cdriver.
...
drivers/gpio/gpio-mlxbf3.c static const struct irq_chip gpio_mlxbf3_irqchip = { · line 164
Quoted 4 lines
drivers/gpio/gpio-mlxbf3.c +4
-0+static int mlxbf3_gpio_add_pin_ranges (struct gpio_chip *chip) +{ +int ret; +int id =0 ;
Redundant assignment.
Quoted 1 line
+ id = !!(chip->ngpio %
MLXBF3_GPIO_MAX_PINS_PER_BLOCK);
Using int as boolean. Seems to me you wanted something different here.
drivers/gpio/gpio-mlxbf3.c static const struct irq_chip gpio_mlxbf3_irqchip = { · line 176
Quoted 9 lines
drivers/gpio/gpio-mlxbf3.c +9
-0+ ret =gpiochip_add_pin_range (chip, +"MLNXBF34:00" , + chip->base, + id *MLXBF3_GPIO_MAX_PINS_PER_BLOCK , + chip->ngpio); +if (ret) +return ret; + +return 0 ;
You can return directly:
return gpiochip_add_pin_range(...);
Quoted 1 line
+}
Signature · 2 lines
With Best Regards, Andy Shevchenko