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

Reply via email to