linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/warp: switch to using gpiod API
@ 2022-09-27  6:03 Dmitry Torokhov
  2022-10-28  5:05 ` Dmitry Torokhov
  2022-11-30  9:23 ` Michael Ellerman
  0 siblings, 2 replies; 4+ messages in thread
From: Dmitry Torokhov @ 2022-09-27  6:03 UTC (permalink / raw)
  To: Michael Ellerman, Nicholas Piggin, Christophe Leroy
  Cc: linuxppc-dev, linux-kernel

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@gmail.com>
---

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 <smaclennan@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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/warp: switch to using gpiod API
  2022-09-27  6:03 [PATCH] powerpc/warp: switch to using gpiod API Dmitry Torokhov
@ 2022-10-28  5:05 ` Dmitry Torokhov
  2022-11-14 12:10   ` Christophe Leroy
  2022-11-30  9:23 ` Michael Ellerman
  1 sibling, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2022-10-28  5:05 UTC (permalink / raw)
  To: Michael Ellerman, Nicholas Piggin, Christophe Leroy
  Cc: linuxppc-dev, linux-kernel

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@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 <smaclennan@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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/warp: switch to using gpiod API
  2022-10-28  5:05 ` Dmitry Torokhov
@ 2022-11-14 12:10   ` Christophe Leroy
  0 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2022-11-14 12:10 UTC (permalink / raw)
  To: Dmitry Torokhov, Michael Ellerman, Nicholas Piggin
  Cc: linuxppc-dev, linux-kernel



Le 28/10/2022 à 07:05, Dmitry Torokhov a écrit :
> 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@gmail.com>
> 
> Gentle ping on this... Could I get a feedback if this is acceptable or
> if you want me to rework this somehow?

Not much to say about it. It adds several lines of code, but if that's 
the only way to allow you to switch to gpiod and abandon gpio, then I 
guess it is ok.

Otherwise, as mentionned by Michael during initial discussion, having 
the LED lightning when the machine dies is probably not vital. So if you 
have something simpler, just go for it. Then we can make something more 
eloborated if somebody screams.

Regardless,

Acked-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> 
> 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 <smaclennan@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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/warp: switch to using gpiod API
  2022-09-27  6:03 [PATCH] powerpc/warp: switch to using gpiod API Dmitry Torokhov
  2022-10-28  5:05 ` Dmitry Torokhov
@ 2022-11-30  9:23 ` Michael Ellerman
  1 sibling, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2022-11-30  9:23 UTC (permalink / raw)
  To: Nicholas Piggin, Christophe Leroy, Michael Ellerman, Dmitry Torokhov
  Cc: linuxppc-dev, linux-kernel

On Mon, 26 Sep 2022 23:03:25 -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.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/warp: switch to using gpiod API
      https://git.kernel.org/powerpc/c/1892e87a3e9170146549779622cb844582f1e2bb

cheers

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-30  9:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27  6:03 [PATCH] powerpc/warp: switch to using gpiod API Dmitry Torokhov
2022-10-28  5:05 ` Dmitry Torokhov
2022-11-14 12:10   ` Christophe Leroy
2022-11-30  9:23 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).