| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y1tjIOCohHF3faQb@google.com>
Date: Thu, 27 Oct 2022 22:05:36 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>
Cc: linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc/warp: switch to using gpiod API
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.torokhov@...il.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@...00c00 {
> };
>
> 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 <smaclennan@...atech.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
Powered by blists - more mailing lists