1. Patchew
  2. linux
  3. [PATCH 00/21] gpiolib: fence off legacy interfaces
  4. View patch

[PATCH 18/21] nfc: s3fwrn5: convert to gpio descriptors

Arnd Bergmann posted 21 patches 1 month, 3 weeks ago
[PATCH 18/21] nfc: s3fwrn5: convert to gpio descriptors
Posted by Arnd Bergmann 1 month, 3 weeks ago 
From: Arnd Bergmann <arnd@arndb.de>

There is no need for this driver to still use the legacy interfaces,
so convert all the legacy calls into their modern equivalents.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/nfc/s3fwrn5/i2c.c        | 42 +++++++++-----------------------
 drivers/nfc/s3fwrn5/phy_common.c | 12 ++++-----
 drivers/nfc/s3fwrn5/phy_common.h |  4 +--
 drivers/nfc/s3fwrn5/uart.c       | 30 ++++++-----------------
 4 files changed, 28 insertions(+), 60 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index 110d086cfe5b..411be709b397 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -8,7 +8,7 @@
 
 #include <linux/clk.h>
 #include <linux/i2c.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/delay.h>
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
@@ -149,29 +149,22 @@ static irqreturn_t s3fwrn5_i2c_irq_thread_fn(int irq, void *phy_id)
 static int s3fwrn5_i2c_parse_dt(struct i2c_client *client)
 {
 	struct s3fwrn5_i2c_phy *phy = i2c_get_clientdata(client);
-	struct device_node *np = client->dev.of_node;
+	struct device *dev = &client->dev;
 
-	if (!np)
-		return -ENODEV;
-
-	phy->common.gpio_en = of_get_named_gpio(np, "en-gpios", 0);
-	if (!gpio_is_valid(phy->common.gpio_en)) {
+	phy->common.gpio_en = devm_gpiod_get(dev, "en", GPIOD_OUT_HIGH);
+	if (IS_ERR(phy->common.gpio_en)) {
 		/* Support also deprecated property */
-		phy->common.gpio_en = of_get_named_gpio(np,
-							"s3fwrn5,en-gpios",
-							0);
-		if (!gpio_is_valid(phy->common.gpio_en))
-			return -ENODEV;
+		phy->common.gpio_en = devm_gpiod_get(dev, "s3fwrn5,en", GPIOD_OUT_HIGH);
+		if (IS_ERR(phy->common.gpio_en))
+			return PTR_ERR(phy->common.gpio_en);
 	}
 
-	phy->common.gpio_fw_wake = of_get_named_gpio(np, "wake-gpios", 0);
-	if (!gpio_is_valid(phy->common.gpio_fw_wake)) {
+	phy->common.gpio_fw_wake = devm_gpiod_get(dev, "wake", GPIOD_OUT_LOW);
+	if (IS_ERR(phy->common.gpio_fw_wake)) {
 		/* Support also deprecated property */
-		phy->common.gpio_fw_wake = of_get_named_gpio(np,
-							     "s3fwrn5,fw-gpios",
-							     0);
-		if (!gpio_is_valid(phy->common.gpio_fw_wake))
-			return -ENODEV;
+		phy->common.gpio_fw_wake = devm_gpiod_get(dev, "s3fwrn5,fw", GPIOD_OUT_LOW);
+		if (IS_ERR(phy->common.gpio_fw_wake))
+			return PTR_ERR(phy->common.gpio_en);
 	}
 
 	return 0;
@@ -197,17 +190,6 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	ret = devm_gpio_request_one(&phy->i2c_dev->dev, phy->common.gpio_en,
-				    GPIOF_OUT_INIT_HIGH, "s3fwrn5_en");
-	if (ret < 0)
-		return ret;
-
-	ret = devm_gpio_request_one(&phy->i2c_dev->dev,
-				    phy->common.gpio_fw_wake,
-				    GPIOF_OUT_INIT_LOW, "s3fwrn5_fw_wake");
-	if (ret < 0)
-		return ret;
-
 	/*
 	 * S3FWRN5 depends on a clock input ("XI" pin) to function properly.
 	 * Depending on the hardware configuration this could be an always-on
diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
index deb2c039f0fd..e802b4e609c8 100644
--- a/drivers/nfc/s3fwrn5/phy_common.c
+++ b/drivers/nfc/s3fwrn5/phy_common.c
@@ -8,7 +8,7 @@
  * Bongsu Jeon <bongsu.jeon@samsung.com>
  */
 
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 
@@ -19,7 +19,7 @@ void s3fwrn5_phy_set_wake(void *phy_id, bool wake)
 	struct phy_common *phy = phy_id;
 
 	mutex_lock(&phy->mutex);
-	gpio_set_value(phy->gpio_fw_wake, wake);
+	gpiod_set_value(phy->gpio_fw_wake, wake);
 	if (wake)
 		msleep(S3FWRN5_EN_WAIT_TIME);
 	mutex_unlock(&phy->mutex);
@@ -33,14 +33,14 @@ bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum s3fwrn5_mode mode)
 
 	phy->mode = mode;
 
-	gpio_set_value(phy->gpio_en, 1);
-	gpio_set_value(phy->gpio_fw_wake, 0);
+	gpiod_set_value(phy->gpio_en, 1);
+	gpiod_set_value(phy->gpio_fw_wake, 0);
 	if (mode == S3FWRN5_MODE_FW)
-		gpio_set_value(phy->gpio_fw_wake, 1);
+		gpiod_set_value(phy->gpio_fw_wake, 1);
 
 	if (mode != S3FWRN5_MODE_COLD) {
 		msleep(S3FWRN5_EN_WAIT_TIME);
-		gpio_set_value(phy->gpio_en, 0);
+		gpiod_set_value(phy->gpio_en, 0);
 		msleep(S3FWRN5_EN_WAIT_TIME);
 	}
 
diff --git a/drivers/nfc/s3fwrn5/phy_common.h b/drivers/nfc/s3fwrn5/phy_common.h
index 9cef25436bf9..5451f46f7e27 100644
--- a/drivers/nfc/s3fwrn5/phy_common.h
+++ b/drivers/nfc/s3fwrn5/phy_common.h
@@ -21,8 +21,8 @@
 struct phy_common {
 	struct nci_dev *ndev;
 
-	int gpio_en;
-	int gpio_fw_wake;
+	struct gpio_desc *gpio_en;
+	struct gpio_desc *gpio_fw_wake;
 
 	struct mutex mutex;
 
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
index 9c09c10c2a46..39e3a64c4f4c 100644
--- a/drivers/nfc/s3fwrn5/uart.c
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -15,7 +15,7 @@
 #include <linux/netdevice.h>
 #include <linux/of.h>
 #include <linux/serdev.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of_gpio.h>
 
 #include "phy_common.h"
@@ -91,18 +91,15 @@ MODULE_DEVICE_TABLE(of, s3fwrn82_uart_of_match);
 static int s3fwrn82_uart_parse_dt(struct serdev_device *serdev)
 {
 	struct s3fwrn82_uart_phy *phy = serdev_device_get_drvdata(serdev);
-	struct device_node *np = serdev->dev.of_node;
+	struct device *dev = &serdev->dev;
 
-	if (!np)
-		return -ENODEV;
+	phy->common.gpio_en = devm_gpiod_get(dev, "en", GPIOD_OUT_HIGH);
+	if (IS_ERR(phy->common.gpio_en))
+		return PTR_ERR(phy->common.gpio_en);
 
-	phy->common.gpio_en = of_get_named_gpio(np, "en-gpios", 0);
-	if (!gpio_is_valid(phy->common.gpio_en))
-		return -ENODEV;
-
-	phy->common.gpio_fw_wake = of_get_named_gpio(np, "wake-gpios", 0);
-	if (!gpio_is_valid(phy->common.gpio_fw_wake))
-		return -ENODEV;
+	phy->common.gpio_fw_wake = devm_gpiod_get(dev, "wake", GPIOD_OUT_LOW);
+	if (IS_ERR(phy->common.gpio_fw_wake))
+		return PTR_ERR(phy->common.gpio_fw_wake);
 
 	return 0;
 }
@@ -144,17 +141,6 @@ static int s3fwrn82_uart_probe(struct serdev_device *serdev)
 	if (ret < 0)
 		goto err_serdev;
 
-	ret = devm_gpio_request_one(&phy->ser_dev->dev, phy->common.gpio_en,
-				    GPIOF_OUT_INIT_HIGH, "s3fwrn82_en");
-	if (ret < 0)
-		goto err_serdev;
-
-	ret = devm_gpio_request_one(&phy->ser_dev->dev,
-				    phy->common.gpio_fw_wake,
-				    GPIOF_OUT_INIT_LOW, "s3fwrn82_fw_wake");
-	if (ret < 0)
-		goto err_serdev;
-
 	ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->ser_dev->dev,
 			    &uart_phy_ops);
 	if (ret < 0)
-- 
2.39.5
Re: [PATCH 18/21] nfc: s3fwrn5: convert to gpio descriptors
Posted by Dmitry Torokhov 1 month, 3 weeks ago 
Hi Arnd,

On Fri, Aug 08, 2025 at 05:18:02PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> There is no need for this driver to still use the legacy interfaces,
> so convert all the legacy calls into their modern equivalents.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/nfc/s3fwrn5/i2c.c        | 42 +++++++++-----------------------
>  drivers/nfc/s3fwrn5/phy_common.c | 12 ++++-----
>  drivers/nfc/s3fwrn5/phy_common.h |  4 +--
>  drivers/nfc/s3fwrn5/uart.c       | 30 ++++++-----------------
>  4 files changed, 28 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> index 110d086cfe5b..411be709b397 100644
> --- a/drivers/nfc/s3fwrn5/i2c.c
> +++ b/drivers/nfc/s3fwrn5/i2c.c
> @@ -8,7 +8,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/i2c.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/delay.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_irq.h>
> @@ -149,29 +149,22 @@ static irqreturn_t s3fwrn5_i2c_irq_thread_fn(int irq, void *phy_id)
>  static int s3fwrn5_i2c_parse_dt(struct i2c_client *client)
>  {
>  	struct s3fwrn5_i2c_phy *phy = i2c_get_clientdata(client);
> -	struct device_node *np = client->dev.of_node;
> +	struct device *dev = &client->dev;
>  
> -	if (!np)
> -		return -ENODEV;
> -
> -	phy->common.gpio_en = of_get_named_gpio(np, "en-gpios", 0);
> -	if (!gpio_is_valid(phy->common.gpio_en)) {
> +	phy->common.gpio_en = devm_gpiod_get(dev, "en", GPIOD_OUT_HIGH);
> +	if (IS_ERR(phy->common.gpio_en)) {
>  		/* Support also deprecated property */
> -		phy->common.gpio_en = of_get_named_gpio(np,
> -							"s3fwrn5,en-gpios",
> -							0);
> -		if (!gpio_is_valid(phy->common.gpio_en))
> -			return -ENODEV;
> +		phy->common.gpio_en = devm_gpiod_get(dev, "s3fwrn5,en", GPIOD_OUT_HIGH);
> +		if (IS_ERR(phy->common.gpio_en))
> +			return PTR_ERR(phy->common.gpio_en);
>  	}

Should be GPIOD_OUT_LOW or ASIS.

>  
> -	phy->common.gpio_fw_wake = of_get_named_gpio(np, "wake-gpios", 0);
> -	if (!gpio_is_valid(phy->common.gpio_fw_wake)) {
> +	phy->common.gpio_fw_wake = devm_gpiod_get(dev, "wake", GPIOD_OUT_LOW);
> +	if (IS_ERR(phy->common.gpio_fw_wake)) {
>  		/* Support also deprecated property */
> -		phy->common.gpio_fw_wake = of_get_named_gpio(np,
> -							     "s3fwrn5,fw-gpios",
> -							     0);
> -		if (!gpio_is_valid(phy->common.gpio_fw_wake))
> -			return -ENODEV;
> +		phy->common.gpio_fw_wake = devm_gpiod_get(dev, "s3fwrn5,fw", GPIOD_OUT_LOW);
> +		if (IS_ERR(phy->common.gpio_fw_wake))
> +			return PTR_ERR(phy->common.gpio_en);
>  	}
>  
>  	return 0;
> @@ -197,17 +190,6 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = devm_gpio_request_one(&phy->i2c_dev->dev, phy->common.gpio_en,
> -				    GPIOF_OUT_INIT_HIGH, "s3fwrn5_en");
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = devm_gpio_request_one(&phy->i2c_dev->dev,
> -				    phy->common.gpio_fw_wake,
> -				    GPIOF_OUT_INIT_LOW, "s3fwrn5_fw_wake");
> -	if (ret < 0)
> -		return ret;
> -
>  	/*
>  	 * S3FWRN5 depends on a clock input ("XI" pin) to function properly.
>  	 * Depending on the hardware configuration this could be an always-on
> diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
> index deb2c039f0fd..e802b4e609c8 100644
> --- a/drivers/nfc/s3fwrn5/phy_common.c
> +++ b/drivers/nfc/s3fwrn5/phy_common.c
> @@ -8,7 +8,7 @@
>   * Bongsu Jeon <bongsu.jeon@samsung.com>
>   */
>  
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
>  
> @@ -19,7 +19,7 @@ void s3fwrn5_phy_set_wake(void *phy_id, bool wake)
>  	struct phy_common *phy = phy_id;
>  
>  	mutex_lock(&phy->mutex);
> -	gpio_set_value(phy->gpio_fw_wake, wake);
> +	gpiod_set_value(phy->gpio_fw_wake, wake);
>  	if (wake)
>  		msleep(S3FWRN5_EN_WAIT_TIME);
>  	mutex_unlock(&phy->mutex);
> @@ -33,14 +33,14 @@ bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum s3fwrn5_mode mode)
>  
>  	phy->mode = mode;
>  
> -	gpio_set_value(phy->gpio_en, 1);
> -	gpio_set_value(phy->gpio_fw_wake, 0);
> +	gpiod_set_value(phy->gpio_en, 1);
> +	gpiod_set_value(phy->gpio_fw_wake, 0);
>  	if (mode == S3FWRN5_MODE_FW)
> -		gpio_set_value(phy->gpio_fw_wake, 1);
> +		gpiod_set_value(phy->gpio_fw_wake, 1);
>  
>  	if (mode != S3FWRN5_MODE_COLD) {
>  		msleep(S3FWRN5_EN_WAIT_TIME);
> -		gpio_set_value(phy->gpio_en, 0);
> +		gpiod_set_value(phy->gpio_en, 0);
>  		msleep(S3FWRN5_EN_WAIT_TIME);

The GPIO is describe as "active low" in DTS:

arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi

So here you are leaving the chip disabled. You need to use logical
polarity.

Thanks.

-- 
Dmitry

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.