On Mon, Sep 26, 2022 at 11:03:25PM -0700, Dmitry Torokhov wrote: > This switches PIKA Warp away from legacy gpio API and to newer gpiod > API, so that we can eventually deprecate the former. > > Because LEDs are normally driven by leds-gpio driver, but the > platform code also wants to access the LEDs during thermal shutdown, > and gpiod API does not allow locating GPIO without requesting it, > the platform code is now responsible for locating GPIOs through device > tree and requesting them. It then constructs platform data for > leds-gpio platform device and registers it. This allows platform > code to retain access to LED GPIO descriptors and use them when needed. > > Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
Gentle ping on this... Could I get a feedback if this is acceptable or if you want me to rework this somehow? Thanks! > --- > > Compiled only, no hardware to test this. > > arch/powerpc/boot/dts/warp.dts | 4 +- > arch/powerpc/platforms/44x/warp.c | 105 ++++++++++++++++++++++++++---- > 2 files changed, 94 insertions(+), 15 deletions(-) > > diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts > index b4f32740870e..aa62d08e97c2 100644 > --- a/arch/powerpc/boot/dts/warp.dts > +++ b/arch/powerpc/boot/dts/warp.dts > @@ -258,14 +258,12 @@ GPIO1: gpio@ef600c00 { > }; > > power-leds { > - compatible = "gpio-leds"; > + compatible = "warp-power-leds"; > green { > gpios = <&GPIO1 0 0>; > - default-state = "keep"; > }; > red { > gpios = <&GPIO1 1 0>; > - default-state = "keep"; > }; > }; > > diff --git a/arch/powerpc/platforms/44x/warp.c > b/arch/powerpc/platforms/44x/warp.c > index f03432ef010b..cefa313c09f0 100644 > --- a/arch/powerpc/platforms/44x/warp.c > +++ b/arch/powerpc/platforms/44x/warp.c > @@ -5,15 +5,17 @@ > * Copyright (c) 2008-2009 PIKA Technologies > * Sean MacLennan <smaclen...@pikatech.com> > */ > +#include <linux/err.h> > #include <linux/init.h> > #include <linux/of_platform.h> > #include <linux/kthread.h> > +#include <linux/leds.h> > #include <linux/i2c.h> > #include <linux/interrupt.h> > #include <linux/delay.h> > #include <linux/of_address.h> > #include <linux/of_irq.h> > -#include <linux/of_gpio.h> > +#include <linux/gpio/consumer.h> > #include <linux/slab.h> > #include <linux/export.h> > > @@ -92,8 +94,6 @@ static int __init warp_post_info(void) > > static LIST_HEAD(dtm_shutdown_list); > static void __iomem *dtm_fpga; > -static unsigned green_led, red_led; > - > > struct dtm_shutdown { > struct list_head list; > @@ -101,7 +101,6 @@ struct dtm_shutdown { > void *arg; > }; > > - > int pika_dtm_register_shutdown(void (*func)(void *arg), void *arg) > { > struct dtm_shutdown *shutdown; > @@ -132,6 +131,35 @@ int pika_dtm_unregister_shutdown(void (*func)(void > *arg), void *arg) > return -EINVAL; > } > > +#define WARP_GREEN_LED 0 > +#define WARP_RED_LED 1 > + > +static struct gpio_led warp_gpio_led_pins[] = { > + [WARP_GREEN_LED] = { > + .name = "green", > + .default_state = LEDS_DEFSTATE_KEEP, > + .gpiod = NULL, /* to be filled by pika_setup_leds() */ > + }, > + [WARP_RED_LED] = { > + .name = "red", > + .default_state = LEDS_DEFSTATE_KEEP, > + .gpiod = NULL, /* to be filled by pika_setup_leds() */ > + }, > +}; > + > +static struct gpio_led_platform_data warp_gpio_led_data = { > + .leds = warp_gpio_led_pins, > + .num_leds = ARRAY_SIZE(warp_gpio_led_pins), > +}; > + > +static struct platform_device warp_gpio_leds = { > + .name = "leds-gpio", > + .id = -1, > + .dev = { > + .platform_data = &warp_gpio_led_data, > + }, > +}; > + > static irqreturn_t temp_isr(int irq, void *context) > { > struct dtm_shutdown *shutdown; > @@ -139,7 +167,7 @@ static irqreturn_t temp_isr(int irq, void *context) > > local_irq_disable(); > > - gpio_set_value(green_led, 0); > + gpiod_set_value(warp_gpio_led_pins[WARP_GREEN_LED].gpiod, 0); > > /* Run through the shutdown list. */ > list_for_each_entry(shutdown, &dtm_shutdown_list, list) > @@ -153,7 +181,7 @@ static irqreturn_t temp_isr(int irq, void *context) > out_be32(dtm_fpga + 0x14, reset); > } > > - gpio_set_value(red_led, value); > + gpiod_set_value(warp_gpio_led_pins[WARP_RED_LED].gpiod, value); > value ^= 1; > mdelay(500); > } > @@ -162,25 +190,78 @@ static irqreturn_t temp_isr(int irq, void *context) > return IRQ_HANDLED; > } > > +/* > + * Because green and red power LEDs are normally driven by leds-gpio driver, > + * but in case of critical temperature shutdown we want to drive them > + * ourselves, we acquire both and then create leds-gpio platform device > + * ourselves, instead of doing it through device tree. This way we can still > + * keep access to the gpios and use them when needed. > + */ > static int pika_setup_leds(void) > { > struct device_node *np, *child; > + struct gpio_desc *gpio; > + struct gpio_led *led; > + int led_count = 0; > + int error; > + int i; > > - np = of_find_compatible_node(NULL, NULL, "gpio-leds"); > + np = of_find_compatible_node(NULL, NULL, "warp-power-leds"); > if (!np) { > printk(KERN_ERR __FILE__ ": Unable to find leds\n"); > return -ENOENT; > } > > - for_each_child_of_node(np, child) > - if (of_node_name_eq(child, "green")) > - green_led = of_get_gpio(child, 0); > - else if (of_node_name_eq(child, "red")) > - red_led = of_get_gpio(child, 0); > + for_each_child_of_node(np, child) { > + for (i = 0; i < ARRAY_SIZE(warp_gpio_led_pins); i++) { > + led = &warp_gpio_led_pins[i]; > + > + if (!of_node_name_eq(child, led->name)) > + continue; > + > + if (led->gpiod) { > + printk(KERN_ERR __FILE__ ": %s led has already > been defined\n", > + led->name); > + continue; > + } > + > + gpio = fwnode_gpiod_get_index(of_fwnode_handle(child), > + NULL, 0, GPIOD_ASIS, > + led->name); > + error = PTR_ERR_OR_ZERO(gpio); > + if (error) { > + printk(KERN_ERR __FILE__ ": Failed to get %s > led gpio: %d\n", > + led->name, error); > + of_node_put(child); > + goto err_cleanup_pins; > + } > + > + led->gpiod = gpio; > + led_count++; > + } > + } > > of_node_put(np); > > + /* Skip device registration if no leds have been defined */ > + if (led_count) { > + error = platform_device_register(&warp_gpio_leds); > + if (error) { > + printk(KERN_ERR __FILE__ ": Unable to add leds-gpio: > %d\n", > + error); > + goto err_cleanup_pins; > + } > + } > + > return 0; > + > +err_cleanup_pins: > + for (i = 0; i < ARRAY_SIZE(warp_gpio_led_pins); i++) { > + led = &warp_gpio_led_pins[i]; > + gpiod_put(led->gpiod); > + led->gpiod = NULL; > + } > + return error; > } > > static void pika_setup_critical_temp(struct device_node *np, > -- > 2.38.0.rc1.362.ged0d419d3c-goog > > > -- > Dmitry -- Dmitry