2023-10-01 14:16:09

by Duje Mihanović

[permalink] [raw]
Subject: [PATCH RFC v4 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors

Gumstix still uses the legacy GPIO interface for resetting the Bluetooth
device.

Convert it to use the GPIO descriptor interface.

Reviewed-by: Linus Walleij <[email protected]>
Signed-off-by: Duje Mihanović <[email protected]>
---
arch/arm/mach-pxa/gumstix.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-pxa/gumstix.c b/arch/arm/mach-pxa/gumstix.c
index c9f0f62187bd..14e1b9274d7a 100644
--- a/arch/arm/mach-pxa/gumstix.c
+++ b/arch/arm/mach-pxa/gumstix.c
@@ -20,8 +20,8 @@
#include <linux/delay.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/gpio/consumer.h>
#include <linux/gpio/machine.h>
-#include <linux/gpio.h>
#include <linux/err.h>
#include <linux/clk.h>

@@ -129,6 +129,9 @@ static void gumstix_udc_init(void)
#endif

#ifdef CONFIG_BT
+GPIO_LOOKUP_SINGLE(gumstix_bt_gpio_table, "pxa2xx-uart.1", "pxa-gpio",
+ GPIO_GUMSTIX_BTRESET, "BTRST", GPIO_ACTIVE_LOW);
+
/* Normally, the bootloader would have enabled this 32kHz clock but many
** boards still have u-boot 1.1.4 so we check if it has been turned on and
** if not, we turn it on with a warning message. */
@@ -153,24 +156,23 @@ static void gumstix_setup_bt_clock(void)

static void __init gumstix_bluetooth_init(void)
{
- int err;
+ struct gpio_desc *desc;
+
+ gpiod_add_lookup_table(&gumstix_bt_gpio_table);

gumstix_setup_bt_clock();

- err = gpio_request(GPIO_GUMSTIX_BTRESET, "BTRST");
- if (err) {
+ desc = gpiod_get(&pxa_device_btuart.dev, "BTRST", GPIOD_OUT_HIGH);
+ if (IS_ERR(desc)) {
pr_err("gumstix: failed request gpio for bluetooth reset\n");
return;
}

- err = gpio_direction_output(GPIO_GUMSTIX_BTRESET, 1);
- if (err) {
- pr_err("gumstix: can't reset bluetooth\n");
- return;
- }
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 0);
+ gpiod_set_value(desc, 0);
udelay(100);
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 1);
+ gpiod_set_value(desc, 1);
+
+ gpiod_put(desc);
}
#else
static void gumstix_bluetooth_init(void)

--
2.42.0



2023-10-02 13:17:48

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH RFC v4 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors

On Sun, Oct 1, 2023 at 4:13 PM Duje Mihanović <[email protected]> wrote:
>
> Gumstix still uses the legacy GPIO interface for resetting the Bluetooth
> device.
>
> Convert it to use the GPIO descriptor interface.
>
> Reviewed-by: Linus Walleij <[email protected]>
> Signed-off-by: Duje Mihanović <[email protected]>
> ---
> arch/arm/mach-pxa/gumstix.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/gumstix.c b/arch/arm/mach-pxa/gumstix.c
> index c9f0f62187bd..14e1b9274d7a 100644
> --- a/arch/arm/mach-pxa/gumstix.c
> +++ b/arch/arm/mach-pxa/gumstix.c
> @@ -20,8 +20,8 @@
> #include <linux/delay.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/gpio/machine.h>
> -#include <linux/gpio.h>
> #include <linux/err.h>
> #include <linux/clk.h>
>
> @@ -129,6 +129,9 @@ static void gumstix_udc_init(void)
> #endif
>
> #ifdef CONFIG_BT
> +GPIO_LOOKUP_SINGLE(gumstix_bt_gpio_table, "pxa2xx-uart.1", "pxa-gpio",
> + GPIO_GUMSTIX_BTRESET, "BTRST", GPIO_ACTIVE_LOW);
> +
> /* Normally, the bootloader would have enabled this 32kHz clock but many
> ** boards still have u-boot 1.1.4 so we check if it has been turned on and
> ** if not, we turn it on with a warning message. */
> @@ -153,24 +156,23 @@ static void gumstix_setup_bt_clock(void)
>
> static void __init gumstix_bluetooth_init(void)
> {
> - int err;
> + struct gpio_desc *desc;
> +
> + gpiod_add_lookup_table(&gumstix_bt_gpio_table);
>
> gumstix_setup_bt_clock();
>
> - err = gpio_request(GPIO_GUMSTIX_BTRESET, "BTRST");
> - if (err) {
> + desc = gpiod_get(&pxa_device_btuart.dev, "BTRST", GPIOD_OUT_HIGH);
> + if (IS_ERR(desc)) {
> pr_err("gumstix: failed request gpio for bluetooth reset\n");
> return;
> }
>
> - err = gpio_direction_output(GPIO_GUMSTIX_BTRESET, 1);
> - if (err) {
> - pr_err("gumstix: can't reset bluetooth\n");
> - return;
> - }
> - gpio_set_value(GPIO_GUMSTIX_BTRESET, 0);
> + gpiod_set_value(desc, 0);
> udelay(100);
> - gpio_set_value(GPIO_GUMSTIX_BTRESET, 1);
> + gpiod_set_value(desc, 1);
> +
> + gpiod_put(desc);

This changes the way this code works. You release the descriptor here,
it returns to the driver and can be re-requested by someone else. Its
value is also not guaranteed to remain as "active". Is this what you
want?

Bart

> }
> #else
> static void gumstix_bluetooth_init(void)
>
> --
> 2.42.0
>
>

2023-10-02 15:03:53

by Duje Mihanović

[permalink] [raw]
Subject: Re: [PATCH RFC v4 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors

On Monday, October 2, 2023 9:42:52 AM CEST Bartosz Golaszewski wrote:
> This changes the way this code works. You release the descriptor here,
> it returns to the driver and can be re-requested by someone else. Its
> value is also not guaranteed to remain as "active". Is this what you
> want?

Good point. Is it enough to not call gpiod_put() at the end or is it necessary
to use a static gpio_desc instead of a local one?

Regards,
Duje



2023-10-02 23:05:37

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH RFC v4 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors

On Mon, Oct 2, 2023 at 4:53 PM Duje Mihanović <[email protected]> wrote:
>
> On Monday, October 2, 2023 9:42:52 AM CEST Bartosz Golaszewski wrote:
> > This changes the way this code works. You release the descriptor here,
> > it returns to the driver and can be re-requested by someone else. Its
> > value is also not guaranteed to remain as "active". Is this what you
> > want?
>
> Good point. Is it enough to not call gpiod_put() at the end or is it necessary
> to use a static gpio_desc instead of a local one?
>

Technically it's enough to not put it. It will live on but the
reference will be leaked and most likely this will be reported by
kmemleak. So static desc would make more sense.

Bart