| Message ID | 20230818164314.8505-3-asmaa@nvidia.com |
|---|---|
| State | Accepted |
| Commit | 38a700efc51080c7184f71edbf5e49561da9754f |
| Headers | show |
| Series | Fix Nvidia BlueField-3 GPIO access | expand |
On Fri, Aug 18, 2023 at 6:43 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > > Support 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. > > Fixes: cd33f216d24 ("gpio: mlxbf3: Add gpio driver support") > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > v4->v5: > - Add "Reviewed-By" Tag > v3->v4: > - Drop the common define for MLXBF3_GPIO_MAX_PINS_BLOCK0 > v2->v3: > - Replace boolean logic by clear switch statement logic in > mlxbf3_gpio_add_pin_ranges() > v1->v2: > - Cleanup mlxbf3_gpio_add_pin_ranges() > > drivers/gpio/gpio-mlxbf3.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpio/gpio-mlxbf3.c b/drivers/gpio/gpio-mlxbf3.c > index e30cee108986..0a5f241a8352 100644 > --- a/drivers/gpio/gpio-mlxbf3.c > +++ b/drivers/gpio/gpio-mlxbf3.c > @@ -19,6 +19,8 @@ > * gpio[1]: HOST_GPIO32->HOST_GPIO55 > */ > #define MLXBF3_GPIO_MAX_PINS_PER_BLOCK 32 > +#define MLXBF3_GPIO_MAX_PINS_BLOCK0 32 > +#define MLXBF3_GPIO_MAX_PINS_BLOCK1 24 > > /* > * fw_gpio[x] block registers and their offset > @@ -158,6 +160,26 @@ static const struct irq_chip gpio_mlxbf3_irqchip = { > GPIOCHIP_IRQ_RESOURCE_HELPERS, > }; > > +static int mlxbf3_gpio_add_pin_ranges(struct gpio_chip *chip) > +{ > + unsigned int id; > + > + switch(chip->ngpio) { > + case MLXBF3_GPIO_MAX_PINS_BLOCK0: > + id = 0; > + break; > + case MLXBF3_GPIO_MAX_PINS_BLOCK1: > + id = 1; > + break; > + default: > + return -EINVAL; > + } > + > + return gpiochip_add_pin_range(chip, "MLNXBF34:00", > + chip->base, id * MLXBF3_GPIO_MAX_PINS_PER_BLOCK, > + chip->ngpio); > +} > + > static int mlxbf3_gpio_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -197,6 +219,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 +266,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 <asmaa@nvidia.com>"); > MODULE_LICENSE("Dual BSD/GPL"); > -- > 2.30.1 > It's not clear to me whether this depends on patch 1? If only at run-time then I guess Linus and I can take the two patches through ours respective trees? Bart
> > +MODULE_SOFTDEP("pre: pinctrl-mlxbf3"); > > MODULE_DESCRIPTION("NVIDIA BlueField-3 GPIO Driver"); > > MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>"); > > MODULE_LICENSE("Dual BSD/GPL"); > > -- > > 2.30.1 > > > > It's not clear to me whether this depends on patch 1? If only at run-time then I > guess Linus and I can take the two patches through ours respective trees? Indeed from a build point of view, there is no dependency so you could take the 2 patches through your respective tree. However, at run-time, the gpio-mlxbf3.c driver fails to load without the pinctrl-mlxbf3.c driver. Should I add a "depends on" in the Kconfig? Then you will have to include both patches in your tree. Thanks. Asmaa
On Mon, Aug 21, 2023 at 2:55 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > > > > +MODULE_SOFTDEP("pre: pinctrl-mlxbf3"); > > > MODULE_DESCRIPTION("NVIDIA BlueField-3 GPIO Driver"); > > > MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>"); > > > MODULE_LICENSE("Dual BSD/GPL"); > > > -- > > > 2.30.1 > > > > > > > It's not clear to me whether this depends on patch 1? If only at run-time then I > > guess Linus and I can take the two patches through ours respective trees? > > Indeed from a build point of view, there is no dependency so you could take the 2 patches through your respective tree. However, at run-time, the gpio-mlxbf3.c driver fails to load without the pinctrl-mlxbf3.c driver. Should I add a "depends on" in the Kconfig? Then you will have to include both patches in your tree. > Linus, are you fine with me taking this patch? It will not break the build and with you taking the other one, next will be fine too. Bart
On Mon, Aug 21, 2023 at 5:04 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > On Mon, Aug 21, 2023 at 2:55 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > > > > > > +MODULE_SOFTDEP("pre: pinctrl-mlxbf3"); > > > > MODULE_DESCRIPTION("NVIDIA BlueField-3 GPIO Driver"); > > > > MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>"); > > > > MODULE_LICENSE("Dual BSD/GPL"); > > > > -- > > > > 2.30.1 > > > > > > > > > > It's not clear to me whether this depends on patch 1? If only at run-time then I > > > guess Linus and I can take the two patches through ours respective trees? > > > > Indeed from a build point of view, there is no dependency so you could take the 2 patches through your respective tree. However, at run-time, the gpio-mlxbf3.c driver fails to load without the pinctrl-mlxbf3.c driver. Should I add a "depends on" in the Kconfig? Then you will have to include both patches in your tree. > > > > Linus, are you fine with me taking this patch? It will not break the > build and with you taking the other one, next will be fine too. Yep pick this one, I applied 1/2 to the pinctrl tree now. Yours, Linus Walleij
diff --git a/drivers/gpio/gpio-mlxbf3.c b/drivers/gpio/gpio-mlxbf3.c index e30cee108986..0a5f241a8352 100644 --- a/drivers/gpio/gpio-mlxbf3.c +++ b/drivers/gpio/gpio-mlxbf3.c @@ -19,6 +19,8 @@ * gpio[1]: HOST_GPIO32->HOST_GPIO55 */ #define MLXBF3_GPIO_MAX_PINS_PER_BLOCK 32 +#define MLXBF3_GPIO_MAX_PINS_BLOCK0 32 +#define MLXBF3_GPIO_MAX_PINS_BLOCK1 24 /* * fw_gpio[x] block registers and their offset @@ -158,6 +160,26 @@ static const struct irq_chip gpio_mlxbf3_irqchip = { GPIOCHIP_IRQ_RESOURCE_HELPERS, }; +static int mlxbf3_gpio_add_pin_ranges(struct gpio_chip *chip) +{ + unsigned int id; + + switch(chip->ngpio) { + case MLXBF3_GPIO_MAX_PINS_BLOCK0: + id = 0; + break; + case MLXBF3_GPIO_MAX_PINS_BLOCK1: + id = 1; + break; + default: + return -EINVAL; + } + + return gpiochip_add_pin_range(chip, "MLNXBF34:00", + chip->base, id * MLXBF3_GPIO_MAX_PINS_PER_BLOCK, + chip->ngpio); +} + static int mlxbf3_gpio_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -197,6 +219,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 +266,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 <asmaa@nvidia.com>"); MODULE_LICENSE("Dual BSD/GPL");
Support 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. Fixes: cd33f216d24 ("gpio: mlxbf3: Add gpio driver support") Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- v4->v5: - Add "Reviewed-By" Tag v3->v4: - Drop the common define for MLXBF3_GPIO_MAX_PINS_BLOCK0 v2->v3: - Replace boolean logic by clear switch statement logic in mlxbf3_gpio_add_pin_ranges() v1->v2: - Cleanup mlxbf3_gpio_add_pin_ranges() drivers/gpio/gpio-mlxbf3.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)