diff mbox

[9/9] hwmon: gpio-fan: Convert to use GPIO descriptors

Message ID 20170925230911.20824-10-linus.walleij@linaro.org (mailing list archive)
State Superseded
Headers show

Commit Message

Linus Walleij Sept. 25, 2017, 11:09 p.m. UTC
This converts the GPIO fan driver to use GPIO descriptors. This way
we avoid indirection since the gpiolib anyway just use descriptors
inside, and we also get rid of explicit polarity handling: the
descriptors internally knows if the line is active high or active
low.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/hwmon/gpio-fan.c | 83 ++++++++++++++++++------------------------------
 1 file changed, 31 insertions(+), 52 deletions(-)

Comments

Guenter Roeck Oct. 8, 2017, 2:39 p.m. UTC | #1
On Tue, Sep 26, 2017 at 01:09:11AM +0200, Linus Walleij wrote:
> This converts the GPIO fan driver to use GPIO descriptors. This way
> we avoid indirection since the gpiolib anyway just use descriptors
> inside, and we also get rid of explicit polarity handling: the
> descriptors internally knows if the line is active high or active
> low.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/hwmon/gpio-fan.c | 83 ++++++++++++++++++------------------------------
>  1 file changed, 31 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 18b3c7c27d36..bd289e2e7359 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -29,10 +29,9 @@
>  #include <linux/err.h>
>  #include <linux/mutex.h>
>  #include <linux/hwmon.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_gpio.h>
>  #include <linux/thermal.h>
>  
>  struct gpio_fan_speed {
> @@ -47,7 +46,7 @@ struct gpio_fan_data {
>  	struct thermal_cooling_device *cdev;
>  	struct mutex		lock; /* lock GPIOs operations. */
>  	int			num_gpios;
> -	unsigned int		*gpios;
> +	struct gpio_desc	**gpios;
>  	int			num_speed;
>  	struct gpio_fan_speed	*speed;
>  	int			speed_index;
> @@ -55,8 +54,7 @@ struct gpio_fan_data {
>  	int			resume_speed;
>  #endif
>  	bool			pwm_enable;
> -	unsigned int		alarm_gpio;
> -	unsigned int		alarm_gpio_active_low;
> +	struct gpio_desc	*alarm_gpio;
>  	struct work_struct	alarm_work;
>  };
>  
> @@ -86,9 +84,9 @@ static ssize_t fan1_alarm_show(struct device *dev,
>  			       struct device_attribute *attr, char *buf)
>  {
>  	struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> -	int value = gpio_get_value_cansleep(fan_data->alarm_gpio);
> +	int value = gpiod_get_value_cansleep(fan_data->alarm_gpio);
>  
> -	if (fan_data->alarm_gpio_active_low)
> +	if (gpiod_is_active_low(fan_data->alarm_gpio))

Is this still needed ? Just wondering - I would have thought that
gpiod_get_value_cansleep() does the conversion.

Thanks,
Guenter

>  		value = !value;
>  
>  	return sprintf(buf, "%d\n", value);
> @@ -98,31 +96,21 @@ static DEVICE_ATTR_RO(fan1_alarm);
>  
>  static int fan_alarm_init(struct gpio_fan_data *fan_data)
>  {
> -	int err;
>  	int alarm_irq;
>  	struct device *dev = fan_data->dev;
>  
> -	err = devm_gpio_request(dev, fan_data->alarm_gpio, "GPIO fan alarm");
> -	if (err)
> -		return err;
> -
> -	err = gpio_direction_input(fan_data->alarm_gpio);
> -	if (err)
> -		return err;
> -
>  	/*
>  	 * If the alarm GPIO don't support interrupts, just leave
>  	 * without initializing the fail notification support.
>  	 */
> -	alarm_irq = gpio_to_irq(fan_data->alarm_gpio);
> +	alarm_irq = gpiod_to_irq(fan_data->alarm_gpio);
>  	if (alarm_irq < 0)
>  		return 0;
>  
>  	INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
>  	irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
> -	err = devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
> -			       IRQF_SHARED, "GPIO fan alarm", fan_data);
> -	return err;
> +	return devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
> +				IRQF_SHARED, "GPIO fan alarm", fan_data);
>  }
>  
>  /*
> @@ -135,8 +123,8 @@ static void __set_fan_ctrl(struct gpio_fan_data *fan_data, int ctrl_val)
>  	int i;
>  
>  	for (i = 0; i < fan_data->num_gpios; i++)
> -		gpio_set_value_cansleep(fan_data->gpios[i],
> -					(ctrl_val >> i) & 1);
> +		gpiod_set_value_cansleep(fan_data->gpios[i],
> +					 (ctrl_val >> i) & 1);
>  }
>  
>  static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
> @@ -147,7 +135,7 @@ static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
>  	for (i = 0; i < fan_data->num_gpios; i++) {
>  		int value;
>  
> -		value = gpio_get_value_cansleep(fan_data->gpios[i]);
> +		value = gpiod_get_value_cansleep(fan_data->gpios[i]);
>  		ctrl_val |= (value << i);
>  	}
>  	return ctrl_val;
> @@ -362,19 +350,19 @@ static const struct attribute_group *gpio_fan_groups[] = {
>  
>  static int fan_ctrl_init(struct gpio_fan_data *fan_data)
>  {
> -	struct device *dev = fan_data->dev;
>  	int num_gpios = fan_data->num_gpios;
> -	unsigned int *gpios = fan_data->gpios;
> +	struct gpio_desc **gpios = fan_data->gpios;
>  	int i, err;
>  
>  	for (i = 0; i < num_gpios; i++) {
> -		err = devm_gpio_request(dev, gpios[i],
> -					"GPIO fan control");
> -		if (err)
> -			return err;
> -
> -		err = gpio_direction_output(gpios[i],
> -					    gpio_get_value_cansleep(gpios[i]));
> +		/*
> +		 * The GPIO descriptors were retrieved with GPIOD_ASIS so here
> +		 * we set the GPIO into output mode, carefully preserving the
> +		 * current value by setting it to whatever it is already set
> +		 * (no surprise changes in default fan speed).
> +		 */
> +		err = gpiod_direction_output(gpios[i],
> +					gpiod_get_value_cansleep(gpios[i]));
>  		if (err)
>  			return err;
>  	}
> @@ -437,43 +425,34 @@ static int gpio_fan_get_of_data(struct gpio_fan_data *fan_data)
>  	struct gpio_fan_speed *speed;
>  	struct device *dev = fan_data->dev;
>  	struct device_node *np = dev->of_node;
> -	unsigned int *gpios;
> +	struct gpio_desc **gpios;
>  	unsigned i;
>  	u32 u;
>  	struct property *prop;
>  	const __be32 *p;
>  
>  	/* Alarm GPIO if one exists */
> -	if (of_gpio_named_count(np, "alarm-gpios") > 0) {
> -		int val;
> -		enum of_gpio_flags flags;
> -
> -		val = of_get_named_gpio_flags(np, "alarm-gpios", 0, &flags);
> -		if (val < 0)
> -			return val;
> -		fan_data->alarm_gpio = val;
> -		fan_data->alarm_gpio_active_low = flags & OF_GPIO_ACTIVE_LOW;
> -	}
> +	fan_data->alarm_gpio = devm_gpiod_get_optional(dev, "alarm", GPIOD_IN);
> +	if (IS_ERR(fan_data->alarm_gpio))
> +		return PTR_ERR(fan_data->alarm_gpio);
>  
>  	/* Fill GPIO pin array */
> -	fan_data->num_gpios = of_gpio_count(np);
> +	fan_data->num_gpios = gpiod_count(dev, NULL);
>  	if (fan_data->num_gpios <= 0) {
>  		if (fan_data->alarm_gpio)
>  			return 0;
>  		dev_err(dev, "DT properties empty / missing");
>  		return -ENODEV;
>  	}
> -	gpios = devm_kzalloc(dev, fan_data->num_gpios * sizeof(unsigned int),
> -			    GFP_KERNEL);
> +	gpios = devm_kzalloc(dev,
> +			     fan_data->num_gpios * sizeof(struct gpio_desc *),
> +			     GFP_KERNEL);
>  	if (!gpios)
>  		return -ENOMEM;
>  	for (i = 0; i < fan_data->num_gpios; i++) {
> -		int val;
> -
> -		val = of_get_gpio(np, i);
> -		if (val < 0)
> -			return val;
> -		gpios[i] = val;
> +		gpios[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
> +		if (IS_ERR(gpios[i]))
> +			return PTR_ERR(gpios[i]);
>  	}
>  	fan_data->gpios = gpios;
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck Oct. 8, 2017, 4:20 p.m. UTC | #2
On Tue, Sep 26, 2017 at 01:09:11AM +0200, Linus Walleij wrote:
> This converts the GPIO fan driver to use GPIO descriptors. This way
> we avoid indirection since the gpiolib anyway just use descriptors
> inside, and we also get rid of explicit polarity handling: the
> descriptors internally knows if the line is active high or active
> low.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/hwmon/gpio-fan.c | 83 ++++++++++++++++++------------------------------
>  1 file changed, 31 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 18b3c7c27d36..bd289e2e7359 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -29,10 +29,9 @@
>  #include <linux/err.h>
>  #include <linux/mutex.h>
>  #include <linux/hwmon.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_gpio.h>
>  #include <linux/thermal.h>
>  
>  struct gpio_fan_speed {
> @@ -47,7 +46,7 @@ struct gpio_fan_data {
>  	struct thermal_cooling_device *cdev;
>  	struct mutex		lock; /* lock GPIOs operations. */
>  	int			num_gpios;
> -	unsigned int		*gpios;
> +	struct gpio_desc	**gpios;
>  	int			num_speed;
>  	struct gpio_fan_speed	*speed;
>  	int			speed_index;
> @@ -55,8 +54,7 @@ struct gpio_fan_data {
>  	int			resume_speed;
>  #endif
>  	bool			pwm_enable;
> -	unsigned int		alarm_gpio;
> -	unsigned int		alarm_gpio_active_low;
> +	struct gpio_desc	*alarm_gpio;
>  	struct work_struct	alarm_work;
>  };
>  
> @@ -86,9 +84,9 @@ static ssize_t fan1_alarm_show(struct device *dev,
>  			       struct device_attribute *attr, char *buf)
>  {
>  	struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> -	int value = gpio_get_value_cansleep(fan_data->alarm_gpio);
> +	int value = gpiod_get_value_cansleep(fan_data->alarm_gpio);
>  
> -	if (fan_data->alarm_gpio_active_low)
> +	if (gpiod_is_active_low(fan_data->alarm_gpio))
>  		value = !value;
>  
>  	return sprintf(buf, "%d\n", value);
> @@ -98,31 +96,21 @@ static DEVICE_ATTR_RO(fan1_alarm);
>  
>  static int fan_alarm_init(struct gpio_fan_data *fan_data)
>  {
> -	int err;
>  	int alarm_irq;
>  	struct device *dev = fan_data->dev;
>  
> -	err = devm_gpio_request(dev, fan_data->alarm_gpio, "GPIO fan alarm");
> -	if (err)
> -		return err;
> -
> -	err = gpio_direction_input(fan_data->alarm_gpio);
> -	if (err)
> -		return err;
> -
Followup question: I assume this is no longer necessary because the pin
is used as interrupt pin ?
If so, what if it can not be used as interrupt pin (ie gpiod_to_irq()
fails) ?

Thanks,
Guenter

>  	/*
>  	 * If the alarm GPIO don't support interrupts, just leave
>  	 * without initializing the fail notification support.
>  	 */
> -	alarm_irq = gpio_to_irq(fan_data->alarm_gpio);
> +	alarm_irq = gpiod_to_irq(fan_data->alarm_gpio);
>  	if (alarm_irq < 0)
>  		return 0;
>  
>  	INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
>  	irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
> -	err = devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
> -			       IRQF_SHARED, "GPIO fan alarm", fan_data);
> -	return err;
> +	return devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
> +				IRQF_SHARED, "GPIO fan alarm", fan_data);
>  }
>  
>  /*
> @@ -135,8 +123,8 @@ static void __set_fan_ctrl(struct gpio_fan_data *fan_data, int ctrl_val)
>  	int i;
>  
>  	for (i = 0; i < fan_data->num_gpios; i++)
> -		gpio_set_value_cansleep(fan_data->gpios[i],
> -					(ctrl_val >> i) & 1);
> +		gpiod_set_value_cansleep(fan_data->gpios[i],
> +					 (ctrl_val >> i) & 1);
>  }
>  
>  static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
> @@ -147,7 +135,7 @@ static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
>  	for (i = 0; i < fan_data->num_gpios; i++) {
>  		int value;
>  
> -		value = gpio_get_value_cansleep(fan_data->gpios[i]);
> +		value = gpiod_get_value_cansleep(fan_data->gpios[i]);
>  		ctrl_val |= (value << i);
>  	}
>  	return ctrl_val;
> @@ -362,19 +350,19 @@ static const struct attribute_group *gpio_fan_groups[] = {
>  
>  static int fan_ctrl_init(struct gpio_fan_data *fan_data)
>  {
> -	struct device *dev = fan_data->dev;
>  	int num_gpios = fan_data->num_gpios;
> -	unsigned int *gpios = fan_data->gpios;
> +	struct gpio_desc **gpios = fan_data->gpios;
>  	int i, err;
>  
>  	for (i = 0; i < num_gpios; i++) {
> -		err = devm_gpio_request(dev, gpios[i],
> -					"GPIO fan control");
> -		if (err)
> -			return err;
> -
> -		err = gpio_direction_output(gpios[i],
> -					    gpio_get_value_cansleep(gpios[i]));
> +		/*
> +		 * The GPIO descriptors were retrieved with GPIOD_ASIS so here
> +		 * we set the GPIO into output mode, carefully preserving the
> +		 * current value by setting it to whatever it is already set
> +		 * (no surprise changes in default fan speed).
> +		 */
> +		err = gpiod_direction_output(gpios[i],
> +					gpiod_get_value_cansleep(gpios[i]));
>  		if (err)
>  			return err;
>  	}
> @@ -437,43 +425,34 @@ static int gpio_fan_get_of_data(struct gpio_fan_data *fan_data)
>  	struct gpio_fan_speed *speed;
>  	struct device *dev = fan_data->dev;
>  	struct device_node *np = dev->of_node;
> -	unsigned int *gpios;
> +	struct gpio_desc **gpios;
>  	unsigned i;
>  	u32 u;
>  	struct property *prop;
>  	const __be32 *p;
>  
>  	/* Alarm GPIO if one exists */
> -	if (of_gpio_named_count(np, "alarm-gpios") > 0) {
> -		int val;
> -		enum of_gpio_flags flags;
> -
> -		val = of_get_named_gpio_flags(np, "alarm-gpios", 0, &flags);
> -		if (val < 0)
> -			return val;
> -		fan_data->alarm_gpio = val;
> -		fan_data->alarm_gpio_active_low = flags & OF_GPIO_ACTIVE_LOW;
> -	}
> +	fan_data->alarm_gpio = devm_gpiod_get_optional(dev, "alarm", GPIOD_IN);
> +	if (IS_ERR(fan_data->alarm_gpio))
> +		return PTR_ERR(fan_data->alarm_gpio);
>  
>  	/* Fill GPIO pin array */
> -	fan_data->num_gpios = of_gpio_count(np);
> +	fan_data->num_gpios = gpiod_count(dev, NULL);
>  	if (fan_data->num_gpios <= 0) {
>  		if (fan_data->alarm_gpio)
>  			return 0;
>  		dev_err(dev, "DT properties empty / missing");
>  		return -ENODEV;
>  	}
> -	gpios = devm_kzalloc(dev, fan_data->num_gpios * sizeof(unsigned int),
> -			    GFP_KERNEL);
> +	gpios = devm_kzalloc(dev,
> +			     fan_data->num_gpios * sizeof(struct gpio_desc *),
> +			     GFP_KERNEL);
>  	if (!gpios)
>  		return -ENOMEM;
>  	for (i = 0; i < fan_data->num_gpios; i++) {
> -		int val;
> -
> -		val = of_get_gpio(np, i);
> -		if (val < 0)
> -			return val;
> -		gpios[i] = val;
> +		gpios[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
> +		if (IS_ERR(gpios[i]))
> +			return PTR_ERR(gpios[i]);
>  	}
>  	fan_data->gpios = gpios;
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Oct. 8, 2017, 11:05 p.m. UTC | #3
On Sun, Oct 8, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:

>> @@ -86,9 +84,9 @@ static ssize_t fan1_alarm_show(struct device *dev,
>>                              struct device_attribute *attr, char *buf)
>>  {
>>       struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
>> -     int value = gpio_get_value_cansleep(fan_data->alarm_gpio);
>> +     int value = gpiod_get_value_cansleep(fan_data->alarm_gpio);
>>
>> -     if (fan_data->alarm_gpio_active_low)
>> +     if (gpiod_is_active_low(fan_data->alarm_gpio))
>
> Is this still needed ? Just wondering - I would have thought that
> gpiod_get_value_cansleep() does the conversion.

My bad. Fixing in V2, sending it soon.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Oct. 8, 2017, 11:12 p.m. UTC | #4
On Sun, Oct 8, 2017 at 6:20 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Tue, Sep 26, 2017 at 01:09:11AM +0200, Linus Walleij wrote:

>>  static int fan_alarm_init(struct gpio_fan_data *fan_data)
>>  {
>> -     int err;
>>       int alarm_irq;
>>       struct device *dev = fan_data->dev;
>>
>> -     err = devm_gpio_request(dev, fan_data->alarm_gpio, "GPIO fan alarm");
>> -     if (err)
>> -             return err;
>> -
>> -     err = gpio_direction_input(fan_data->alarm_gpio);
>> -     if (err)
>> -             return err;
>> -
> Followup question: I assume this is no longer necessary because the pin
> is used as interrupt pin ?

It is not longer necessary because it is requested as input in probe():

    /* Alarm GPIO if one exists */
    fan_data->alarm_gpio = devm_gpiod_get_optional(dev, "alarm", GPIOD_IN);
    if (IS_ERR(fan_data->alarm_gpio))
        return PTR_ERR(fan_data->alarm_gpio);

GPIOD_IN takes care of setting the line as input.

> If so, what if it can not be used as interrupt pin (ie gpiod_to_irq()
> fails) ?

It now looks like this:

    /*
     * If the alarm GPIO don't support interrupts, just leave
     * without initializing the fail notification support.
     */
    alarm_irq = gpiod_to_irq(fan_data->alarm_gpio);
    if (alarm_irq < 0)
        return 0;

That should be <= 0 since 0 is not a legal IRQ number though so
fixing that too. (And hoping noone is using the illegal 0 irq anymore.)
It was essentially the same before though.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 18b3c7c27d36..bd289e2e7359 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -29,10 +29,9 @@ 
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/hwmon.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
-#include <linux/of_gpio.h>
 #include <linux/thermal.h>
 
 struct gpio_fan_speed {
@@ -47,7 +46,7 @@  struct gpio_fan_data {
 	struct thermal_cooling_device *cdev;
 	struct mutex		lock; /* lock GPIOs operations. */
 	int			num_gpios;
-	unsigned int		*gpios;
+	struct gpio_desc	**gpios;
 	int			num_speed;
 	struct gpio_fan_speed	*speed;
 	int			speed_index;
@@ -55,8 +54,7 @@  struct gpio_fan_data {
 	int			resume_speed;
 #endif
 	bool			pwm_enable;
-	unsigned int		alarm_gpio;
-	unsigned int		alarm_gpio_active_low;
+	struct gpio_desc	*alarm_gpio;
 	struct work_struct	alarm_work;
 };
 
@@ -86,9 +84,9 @@  static ssize_t fan1_alarm_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
 	struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
-	int value = gpio_get_value_cansleep(fan_data->alarm_gpio);
+	int value = gpiod_get_value_cansleep(fan_data->alarm_gpio);
 
-	if (fan_data->alarm_gpio_active_low)
+	if (gpiod_is_active_low(fan_data->alarm_gpio))
 		value = !value;
 
 	return sprintf(buf, "%d\n", value);
@@ -98,31 +96,21 @@  static DEVICE_ATTR_RO(fan1_alarm);
 
 static int fan_alarm_init(struct gpio_fan_data *fan_data)
 {
-	int err;
 	int alarm_irq;
 	struct device *dev = fan_data->dev;
 
-	err = devm_gpio_request(dev, fan_data->alarm_gpio, "GPIO fan alarm");
-	if (err)
-		return err;
-
-	err = gpio_direction_input(fan_data->alarm_gpio);
-	if (err)
-		return err;
-
 	/*
 	 * If the alarm GPIO don't support interrupts, just leave
 	 * without initializing the fail notification support.
 	 */
-	alarm_irq = gpio_to_irq(fan_data->alarm_gpio);
+	alarm_irq = gpiod_to_irq(fan_data->alarm_gpio);
 	if (alarm_irq < 0)
 		return 0;
 
 	INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
 	irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
-	err = devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
-			       IRQF_SHARED, "GPIO fan alarm", fan_data);
-	return err;
+	return devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
+				IRQF_SHARED, "GPIO fan alarm", fan_data);
 }
 
 /*
@@ -135,8 +123,8 @@  static void __set_fan_ctrl(struct gpio_fan_data *fan_data, int ctrl_val)
 	int i;
 
 	for (i = 0; i < fan_data->num_gpios; i++)
-		gpio_set_value_cansleep(fan_data->gpios[i],
-					(ctrl_val >> i) & 1);
+		gpiod_set_value_cansleep(fan_data->gpios[i],
+					 (ctrl_val >> i) & 1);
 }
 
 static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
@@ -147,7 +135,7 @@  static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
 	for (i = 0; i < fan_data->num_gpios; i++) {
 		int value;
 
-		value = gpio_get_value_cansleep(fan_data->gpios[i]);
+		value = gpiod_get_value_cansleep(fan_data->gpios[i]);
 		ctrl_val |= (value << i);
 	}
 	return ctrl_val;
@@ -362,19 +350,19 @@  static const struct attribute_group *gpio_fan_groups[] = {
 
 static int fan_ctrl_init(struct gpio_fan_data *fan_data)
 {
-	struct device *dev = fan_data->dev;
 	int num_gpios = fan_data->num_gpios;
-	unsigned int *gpios = fan_data->gpios;
+	struct gpio_desc **gpios = fan_data->gpios;
 	int i, err;
 
 	for (i = 0; i < num_gpios; i++) {
-		err = devm_gpio_request(dev, gpios[i],
-					"GPIO fan control");
-		if (err)
-			return err;
-
-		err = gpio_direction_output(gpios[i],
-					    gpio_get_value_cansleep(gpios[i]));
+		/*
+		 * The GPIO descriptors were retrieved with GPIOD_ASIS so here
+		 * we set the GPIO into output mode, carefully preserving the
+		 * current value by setting it to whatever it is already set
+		 * (no surprise changes in default fan speed).
+		 */
+		err = gpiod_direction_output(gpios[i],
+					gpiod_get_value_cansleep(gpios[i]));
 		if (err)
 			return err;
 	}
@@ -437,43 +425,34 @@  static int gpio_fan_get_of_data(struct gpio_fan_data *fan_data)
 	struct gpio_fan_speed *speed;
 	struct device *dev = fan_data->dev;
 	struct device_node *np = dev->of_node;
-	unsigned int *gpios;
+	struct gpio_desc **gpios;
 	unsigned i;
 	u32 u;
 	struct property *prop;
 	const __be32 *p;
 
 	/* Alarm GPIO if one exists */
-	if (of_gpio_named_count(np, "alarm-gpios") > 0) {
-		int val;
-		enum of_gpio_flags flags;
-
-		val = of_get_named_gpio_flags(np, "alarm-gpios", 0, &flags);
-		if (val < 0)
-			return val;
-		fan_data->alarm_gpio = val;
-		fan_data->alarm_gpio_active_low = flags & OF_GPIO_ACTIVE_LOW;
-	}
+	fan_data->alarm_gpio = devm_gpiod_get_optional(dev, "alarm", GPIOD_IN);
+	if (IS_ERR(fan_data->alarm_gpio))
+		return PTR_ERR(fan_data->alarm_gpio);
 
 	/* Fill GPIO pin array */
-	fan_data->num_gpios = of_gpio_count(np);
+	fan_data->num_gpios = gpiod_count(dev, NULL);
 	if (fan_data->num_gpios <= 0) {
 		if (fan_data->alarm_gpio)
 			return 0;
 		dev_err(dev, "DT properties empty / missing");
 		return -ENODEV;
 	}
-	gpios = devm_kzalloc(dev, fan_data->num_gpios * sizeof(unsigned int),
-			    GFP_KERNEL);
+	gpios = devm_kzalloc(dev,
+			     fan_data->num_gpios * sizeof(struct gpio_desc *),
+			     GFP_KERNEL);
 	if (!gpios)
 		return -ENOMEM;
 	for (i = 0; i < fan_data->num_gpios; i++) {
-		int val;
-
-		val = of_get_gpio(np, i);
-		if (val < 0)
-			return val;
-		gpios[i] = val;
+		gpios[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
+		if (IS_ERR(gpios[i]))
+			return PTR_ERR(gpios[i]);
 	}
 	fan_data->gpios = gpios;