| Message ID | 20231001-pxa-gpio-v4-5-0f3b975e6ed5@skole.hr (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ARM: pxa: GPIO descriptor conversions | expand |
On Sun, Oct 1, 2023 at 4:13 PM Duje Mihanović <duje.mihanovic@skole.hr> 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 <linus.walleij@linaro.org> > Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr> > --- > 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 > >
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
On Mon, Oct 2, 2023 at 4:53 PM Duje Mihanović <duje.mihanovic@skole.hr> 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
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)