All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code
@ 2017-03-07 10:25 Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 2/5] NFC: st21nfca: Get rid of platform data Andy Shevchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-07 10:25 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard
  Cc: Andy Shevchenko

We return -ENODEV if ACPI provides a GPIO resource. Looks really wrong.
If it has even been tested?

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nfc/st21nfca/i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
index 5a82f553906c..737384d287aa 100644
--- a/drivers/nfc/st21nfca/i2c.c
+++ b/drivers/nfc/st21nfca/i2c.c
@@ -514,9 +514,9 @@ static int st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
 	/* Get EN GPIO from ACPI */
 	gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN, 1,
 					 GPIOD_OUT_LOW);
-	if (!IS_ERR(gpiod_ena)) {
+	if (IS_ERR(gpiod_ena)) {
 		nfc_err(dev, "Unable to get ENABLE GPIO\n");
-		return -ENODEV;
+		return PTR_ERR(gpiod_ena);
 	}
 
 	phy->gpio_ena = desc_to_gpio(gpiod_ena);
-- 
2.11.0

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

* [PATCH v2 2/5] NFC: st21nfca: Get rid of platform data
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
@ 2017-03-07 10:25 ` Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 3/5] NFC: st21nfca: Get rid of "interesting" use of interrupt polarity Andy Shevchenko
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-07 10:25 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard
  Cc: Andy Shevchenko

Legacy platform data must go away. We are on the safe side here since
there are no users of it in the kernel.

If anyone by any odd reason needs it the GPIO lookup tables and
built-in device properties at your service.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nfc/st21nfca/i2c.c             | 46 +++-------------------------------
 include/linux/platform_data/st21nfca.h | 33 ------------------------
 2 files changed, 3 insertions(+), 76 deletions(-)
 delete mode 100644 include/linux/platform_data/st21nfca.h

diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
index 737384d287aa..1eb558f00290 100644
--- a/drivers/nfc/st21nfca/i2c.c
+++ b/drivers/nfc/st21nfca/i2c.c
@@ -20,7 +20,6 @@
 #include <linux/crc-ccitt.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
-#include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of_irq.h>
 #include <linux/of_gpio.h>
@@ -30,7 +29,7 @@
 #include <linux/delay.h>
 #include <linux/nfc.h>
 #include <linux/firmware.h>
-#include <linux/platform_data/st21nfca.h>
+
 #include <asm/unaligned.h>
 
 #include <net/nfc/hci.h>
@@ -60,6 +59,7 @@
 #define IS_START_OF_FRAME(buf) (buf[0] == ST21NFCA_SOF_EOF && \
 				buf[1] == 0)
 
+#define ST21NFCA_HCI_DRIVER_NAME "st21nfca_hci"
 #define ST21NFCA_HCI_I2C_DRIVER_NAME "st21nfca_hci_i2c"
 
 #define ST21NFCA_GPIO_NAME_EN "enable"
@@ -577,43 +577,10 @@ static int st21nfca_hci_i2c_of_request_resources(struct i2c_client *client)
 	return 0;
 }
 
-static int st21nfca_hci_i2c_request_resources(struct i2c_client *client)
-{
-	struct st21nfca_nfc_platform_data *pdata;
-	struct st21nfca_i2c_phy *phy = i2c_get_clientdata(client);
-	int r;
-
-	pdata = client->dev.platform_data;
-	if (pdata == NULL) {
-		nfc_err(&client->dev, "No platform data\n");
-		return -EINVAL;
-	}
-
-	/* store for later use */
-	phy->gpio_ena = pdata->gpio_ena;
-	phy->irq_polarity = pdata->irq_polarity;
-
-	if (phy->gpio_ena > 0) {
-		r = devm_gpio_request_one(&client->dev, phy->gpio_ena,
-					  GPIOF_OUT_INIT_HIGH,
-					  ST21NFCA_GPIO_NAME_EN);
-		if (r) {
-			pr_err("%s : ena gpio_request failed\n", __FILE__);
-			return r;
-		}
-	}
-
-	phy->se_status.is_ese_present = pdata->is_ese_present;
-	phy->se_status.is_uicc_present = pdata->is_uicc_present;
-
-	return 0;
-}
-
 static int st21nfca_hci_i2c_probe(struct i2c_client *client,
 				  const struct i2c_device_id *id)
 {
 	struct st21nfca_i2c_phy *phy;
-	struct st21nfca_nfc_platform_data *pdata;
 	int r;
 
 	dev_dbg(&client->dev, "%s\n", __func__);
@@ -639,19 +606,12 @@ static int st21nfca_hci_i2c_probe(struct i2c_client *client,
 	mutex_init(&phy->phy_lock);
 	i2c_set_clientdata(client, phy);
 
-	pdata = client->dev.platform_data;
-	if (!pdata && client->dev.of_node) {
+	if (client->dev.of_node) {
 		r = st21nfca_hci_i2c_of_request_resources(client);
 		if (r) {
 			nfc_err(&client->dev, "No platform data\n");
 			return r;
 		}
-	} else if (pdata) {
-		r = st21nfca_hci_i2c_request_resources(client);
-		if (r) {
-			nfc_err(&client->dev, "Cannot get platform resources\n");
-			return r;
-		}
 	} else if (ACPI_HANDLE(&client->dev)) {
 		r = st21nfca_hci_i2c_acpi_request_resources(client);
 		if (r) {
diff --git a/include/linux/platform_data/st21nfca.h b/include/linux/platform_data/st21nfca.h
deleted file mode 100644
index cc2bdafb0c69..000000000000
--- a/include/linux/platform_data/st21nfca.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Driver include for the ST21NFCA NFC chip.
- *
- * Copyright (C) 2014  STMicroelectronics SAS. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef _ST21NFCA_HCI_H_
-#define _ST21NFCA_HCI_H_
-
-#include <linux/i2c.h>
-
-#define ST21NFCA_HCI_DRIVER_NAME "st21nfca_hci"
-
-struct st21nfca_nfc_platform_data {
-	unsigned int gpio_ena;
-	unsigned int irq_polarity;
-	bool is_ese_present;
-	bool is_uicc_present;
-};
-
-#endif /* _ST21NFCA_HCI_H_ */
-- 
2.11.0

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

* [PATCH v2 3/5] NFC: st21nfca: Get rid of "interesting" use of interrupt polarity
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 2/5] NFC: st21nfca: Get rid of platform data Andy Shevchenko
@ 2017-03-07 10:25 ` Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 4/5] NFC: st21nfca: Covert to use GPIO descriptor Andy Shevchenko
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-07 10:25 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard
  Cc: Andy Shevchenko

I2C framework followed by IRQ framework does set interrupt polarity
correctly if it's properly specified in firmware (ACPI or DT).

Get rid of the redundant trick when requesting interrupt.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nfc/st21nfca/i2c.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
index 1eb558f00290..30ef330c9030 100644
--- a/drivers/nfc/st21nfca/i2c.c
+++ b/drivers/nfc/st21nfca/i2c.c
@@ -69,7 +69,6 @@ struct st21nfca_i2c_phy {
 	struct nfc_hci_dev *hdev;
 
 	unsigned int gpio_ena;
-	unsigned int irq_polarity;
 
 	struct st21nfca_se_status se_status;
 
@@ -521,8 +520,6 @@ static int st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
 
 	phy->gpio_ena = desc_to_gpio(gpiod_ena);
 
-	phy->irq_polarity = irq_get_trigger_type(client->irq);
-
 	phy->se_status.is_ese_present = false;
 	phy->se_status.is_uicc_present = false;
 
@@ -567,8 +564,6 @@ static int st21nfca_hci_i2c_of_request_resources(struct i2c_client *client)
 
 	phy->gpio_ena = gpio;
 
-	phy->irq_polarity = irq_get_trigger_type(client->irq);
-
 	phy->se_status.is_ese_present =
 				of_property_read_bool(pp, "ese-present");
 	phy->se_status.is_uicc_present =
@@ -631,7 +626,7 @@ static int st21nfca_hci_i2c_probe(struct i2c_client *client,
 
 	r = devm_request_threaded_irq(&client->dev, client->irq, NULL,
 				st21nfca_hci_irq_thread_fn,
-				phy->irq_polarity | IRQF_ONESHOT,
+				IRQF_ONESHOT,
 				ST21NFCA_HCI_DRIVER_NAME, phy);
 	if (r < 0) {
 		nfc_err(&client->dev, "Unable to register IRQ handler\n");
-- 
2.11.0

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

* [PATCH v2 4/5] NFC: st21nfca: Covert to use GPIO descriptor
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 2/5] NFC: st21nfca: Get rid of platform data Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 3/5] NFC: st21nfca: Get rid of "interesting" use of interrupt polarity Andy Shevchenko
@ 2017-03-07 10:25 ` Andy Shevchenko
  2017-03-07 10:25 ` [PATCH v2 5/5] NFC: st21nfca: Use unified device property API meaningfully Andy Shevchenko
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-07 10:25 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard
  Cc: Andy Shevchenko

Since we got rid of platform data, the driver may use GPIO descriptor
directly.

Looking deeply to the use of the GPIO pin it looks like it should be
a fixed voltage regulator rather than custom GPIO handling. But this
is out of scope of the change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nfc/st21nfca/i2c.c | 40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
index 30ef330c9030..941f6f61166e 100644
--- a/drivers/nfc/st21nfca/i2c.c
+++ b/drivers/nfc/st21nfca/i2c.c
@@ -68,8 +68,7 @@ struct st21nfca_i2c_phy {
 	struct i2c_client *i2c_dev;
 	struct nfc_hci_dev *hdev;
 
-	unsigned int gpio_ena;
-
+	struct gpio_desc *gpiod_ena;
 	struct st21nfca_se_status se_status;
 
 	struct sk_buff *pending_skb;
@@ -150,7 +149,7 @@ static int st21nfca_hci_i2c_enable(void *phy_id)
 {
 	struct st21nfca_i2c_phy *phy = phy_id;
 
-	gpio_set_value(phy->gpio_ena, 1);
+	gpiod_set_value(phy->gpiod_ena, 1);
 	phy->powered = 1;
 	phy->run_mode = ST21NFCA_HCI_MODE;
 
@@ -163,7 +162,7 @@ static void st21nfca_hci_i2c_disable(void *phy_id)
 {
 	struct st21nfca_i2c_phy *phy = phy_id;
 
-	gpio_set_value(phy->gpio_ena, 0);
+	gpiod_set_value(phy->gpiod_ena, 0);
 
 	phy->powered = 0;
 }
@@ -506,20 +505,17 @@ static struct nfc_phy_ops i2c_phy_ops = {
 static int st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
 {
 	struct st21nfca_i2c_phy *phy = i2c_get_clientdata(client);
-	struct gpio_desc *gpiod_ena;
 	struct device *dev = &client->dev;
 	u8 tmp;
 
 	/* Get EN GPIO from ACPI */
-	gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN, 1,
-					 GPIOD_OUT_LOW);
-	if (IS_ERR(gpiod_ena)) {
+	phy->gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN, 1,
+					      GPIOD_OUT_LOW);
+	if (IS_ERR(phy->gpiod_ena)) {
 		nfc_err(dev, "Unable to get ENABLE GPIO\n");
-		return PTR_ERR(gpiod_ena);
+		return PTR_ERR(phy->gpiod_ena);
 	}
 
-	phy->gpio_ena = desc_to_gpio(gpiod_ena);
-
 	phy->se_status.is_ese_present = false;
 	phy->se_status.is_uicc_present = false;
 
@@ -539,31 +535,21 @@ static int st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
 static int st21nfca_hci_i2c_of_request_resources(struct i2c_client *client)
 {
 	struct st21nfca_i2c_phy *phy = i2c_get_clientdata(client);
+	struct device *dev = &client->dev;
 	struct device_node *pp;
-	int gpio;
-	int r;
 
 	pp = client->dev.of_node;
 	if (!pp)
 		return -ENODEV;
 
 	/* Get GPIO from device tree */
-	gpio = of_get_named_gpio(pp, "enable-gpios", 0);
-	if (gpio < 0) {
-		nfc_err(&client->dev, "Failed to retrieve enable-gpios from device tree\n");
-		return gpio;
+	phy->gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN, 0,
+					      GPIOD_OUT_HIGH);
+	if (IS_ERR(phy->gpiod_ena)) {
+		nfc_err(dev, "Failed to request enable pin\n");
+		return PTR_ERR(phy->gpiod_ena);
 	}
 
-	/* GPIO request and configuration */
-	r = devm_gpio_request_one(&client->dev, gpio, GPIOF_OUT_INIT_HIGH,
-				  ST21NFCA_GPIO_NAME_EN);
-	if (r) {
-		nfc_err(&client->dev, "Failed to request enable pin\n");
-		return r;
-	}
-
-	phy->gpio_ena = gpio;
-
 	phy->se_status.is_ese_present =
 				of_property_read_bool(pp, "ese-present");
 	phy->se_status.is_uicc_present =
-- 
2.11.0

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

* [PATCH v2 5/5] NFC: st21nfca: Use unified device property API meaningfully
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
                   ` (2 preceding siblings ...)
  2017-03-07 10:25 ` [PATCH v2 4/5] NFC: st21nfca: Covert to use GPIO descriptor Andy Shevchenko
@ 2017-03-07 10:25 ` Andy Shevchenko
  2017-03-17 15:49 ` [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-07 10:25 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard
  Cc: Andy Shevchenko

Another place in the code that unveils non-tested at all ACPI case.

Use unified device property API in meaningful way.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nfc/st21nfca/i2c.c | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
index 941f6f61166e..d9e585ac0ee6 100644
--- a/drivers/nfc/st21nfca/i2c.c
+++ b/drivers/nfc/st21nfca/i2c.c
@@ -506,7 +506,6 @@ static int st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
 {
 	struct st21nfca_i2c_phy *phy = i2c_get_clientdata(client);
 	struct device *dev = &client->dev;
-	u8 tmp;
 
 	/* Get EN GPIO from ACPI */
 	phy->gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN, 1,
@@ -516,19 +515,6 @@ static int st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
 		return PTR_ERR(phy->gpiod_ena);
 	}
 
-	phy->se_status.is_ese_present = false;
-	phy->se_status.is_uicc_present = false;
-
-	if (device_property_present(dev, "ese-present")) {
-		device_property_read_u8(dev, "ese-present", &tmp);
-		phy->se_status.is_ese_present = tmp;
-	}
-
-	if (device_property_present(dev, "uicc-present")) {
-		device_property_read_u8(dev, "uicc-present", &tmp);
-		phy->se_status.is_uicc_present = tmp;
-	}
-
 	return 0;
 }
 
@@ -536,11 +522,6 @@ static int st21nfca_hci_i2c_of_request_resources(struct i2c_client *client)
 {
 	struct st21nfca_i2c_phy *phy = i2c_get_clientdata(client);
 	struct device *dev = &client->dev;
-	struct device_node *pp;
-
-	pp = client->dev.of_node;
-	if (!pp)
-		return -ENODEV;
 
 	/* Get GPIO from device tree */
 	phy->gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN, 0,
@@ -550,11 +531,6 @@ static int st21nfca_hci_i2c_of_request_resources(struct i2c_client *client)
 		return PTR_ERR(phy->gpiod_ena);
 	}
 
-	phy->se_status.is_ese_present =
-				of_property_read_bool(pp, "ese-present");
-	phy->se_status.is_uicc_present =
-				of_property_read_bool(pp, "uicc-present");
-
 	return 0;
 }
 
@@ -604,6 +580,11 @@ static int st21nfca_hci_i2c_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
+	phy->se_status.is_ese_present =
+			device_property_read_bool(&client->dev, "ese-present");
+	phy->se_status.is_uicc_present =
+			device_property_read_bool(&client->dev, "uicc-present");
+
 	r = st21nfca_hci_platform_init(phy);
 	if (r < 0) {
 		nfc_err(&client->dev, "Unable to reboot st21nfca\n");
-- 
2.11.0

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

* Re: [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
                   ` (3 preceding siblings ...)
  2017-03-07 10:25 ` [PATCH v2 5/5] NFC: st21nfca: Use unified device property API meaningfully Andy Shevchenko
@ 2017-03-17 15:49 ` Andy Shevchenko
  2017-03-21  2:29   ` Christophe Ricard
  2017-04-04 16:16 ` Andy Shevchenko
  2017-04-05  8:09 ` Samuel Ortiz
  6 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-17 15:49 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard

On Tue, 2017-03-07 at 12:25 +0200, Andy Shevchenko wrote:
> We return -ENODEV if ACPI provides a GPIO resource. Looks really
> wrong.
> If it has even been tested?

Any comments on this clean up?

Next patch which is dependent to this is related to ACPI enumeration.
After GPIO ACPI library gets stricter the driver wouldn't work without
ACPI related changes.

By the way, is this device have ever been enumerated via ACPI?

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/nfc/st21nfca/i2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
> index 5a82f553906c..737384d287aa 100644
> --- a/drivers/nfc/st21nfca/i2c.c
> +++ b/drivers/nfc/st21nfca/i2c.c
> @@ -514,9 +514,9 @@ static int
> st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
>  	/* Get EN GPIO from ACPI */
>  	gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN,
> 1,
>  					 GPIOD_OUT_LOW);
> -	if (!IS_ERR(gpiod_ena)) {
> +	if (IS_ERR(gpiod_ena)) {
>  		nfc_err(dev, "Unable to get ENABLE GPIO\n");
> -		return -ENODEV;
> +		return PTR_ERR(gpiod_ena);
>  	}
>  
>  	phy->gpio_ena = desc_to_gpio(gpiod_ena);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code
  2017-03-17 15:49 ` [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
@ 2017-03-21  2:29   ` Christophe Ricard
  2017-03-21 18:36     ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Ricard @ 2017-03-21  2:29 UTC (permalink / raw)
  To: Andy Shevchenko, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, linux-wireless

Hi Andy,

The patch looks good to me.

The device got enumerated via ACPI on development platforms for 
integration tests purposes.

Best Regards

Christophe

On 17/03/2017 08:49, Andy Shevchenko wrote:
> On Tue, 2017-03-07 at 12:25 +0200, Andy Shevchenko wrote:
>> We return -ENODEV if ACPI provides a GPIO resource. Looks really
>> wrong.
>> If it has even been tested?
> Any comments on this clean up?
>
> Next patch which is dependent to this is related to ACPI enumeration.
> After GPIO ACPI library gets stricter the driver wouldn't work without
> ACPI related changes.
>
> By the way, is this device have ever been enumerated via ACPI?
>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>   drivers/nfc/st21nfca/i2c.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
>> index 5a82f553906c..737384d287aa 100644
>> --- a/drivers/nfc/st21nfca/i2c.c
>> +++ b/drivers/nfc/st21nfca/i2c.c
>> @@ -514,9 +514,9 @@ static int
>> st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
>>   	/* Get EN GPIO from ACPI */
>>   	gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN,
>> 1,
>>   					 GPIOD_OUT_LOW);
>> -	if (!IS_ERR(gpiod_ena)) {
>> +	if (IS_ERR(gpiod_ena)) {
>>   		nfc_err(dev, "Unable to get ENABLE GPIO\n");
>> -		return -ENODEV;
>> +		return PTR_ERR(gpiod_ena);
>>   	}
>>   
>>   	phy->gpio_ena = desc_to_gpio(gpiod_ena);

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

* Re: [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code
  2017-03-21  2:29   ` Christophe Ricard
@ 2017-03-21 18:36     ` Andy Shevchenko
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-03-21 18:36 UTC (permalink / raw)
  To: Christophe Ricard, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, linux-wireless

On Mon, 2017-03-20 at 19:29 -0700, Christophe Ricard wrote:
> Hi Andy,
> 
> The patch looks good to me.

Thanks for the answer and looking into it.
Does it apply to the whole series?

> The device got enumerated via ACPI on development platforms for 
> integration tests purposes.

It would be ideally good if you can test this series for real.
I'm in doubt someone tested any further than looking into DSDT in ACPI
case or on other hand it might be enumerated but didn't work as supposed
(no enable GPIO, see below fix for the details).

> 
> Best Regards
> 
> Christophe
> 
> On 17/03/2017 08:49, Andy Shevchenko wrote:
> > On Tue, 2017-03-07 at 12:25 +0200, Andy Shevchenko wrote:
> > > We return -ENODEV if ACPI provides a GPIO resource. Looks really
> > > wrong.
> > > If it has even been tested?
> > 
> > Any comments on this clean up?
> > 
> > Next patch which is dependent to this is related to ACPI
> > enumeration.
> > After GPIO ACPI library gets stricter the driver wouldn't work
> > without
> > ACPI related changes.
> > 
> > By the way, is this device have ever been enumerated via ACPI?
> > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >   drivers/nfc/st21nfca/i2c.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/nfc/st21nfca/i2c.c
> > > b/drivers/nfc/st21nfca/i2c.c
> > > index 5a82f553906c..737384d287aa 100644
> > > --- a/drivers/nfc/st21nfca/i2c.c
> > > +++ b/drivers/nfc/st21nfca/i2c.c
> > > @@ -514,9 +514,9 @@ static int
> > > st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
> > >   	/* Get EN GPIO from ACPI */
> > >   	gpiod_ena = devm_gpiod_get_index(dev,
> > > ST21NFCA_GPIO_NAME_EN,
> > > 1,
> > >   					 GPIOD_OUT_LOW);
> > > -	if (!IS_ERR(gpiod_ena)) {
> > > +	if (IS_ERR(gpiod_ena)) {
> > >   		nfc_err(dev, "Unable to get ENABLE GPIO\n");
> > > -		return -ENODEV;
> > > +		return PTR_ERR(gpiod_ena);
> > >   	}
> > >   
> > >   	phy->gpio_ena = desc_to_gpio(gpiod_ena);
> 
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
                   ` (4 preceding siblings ...)
  2017-03-17 15:49 ` [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
@ 2017-04-04 16:16 ` Andy Shevchenko
  2017-04-05  8:09 ` Samuel Ortiz
  6 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-04-04 16:16 UTC (permalink / raw)
  To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, Christophe Ricard

On Tue, 2017-03-07 at 12:25 +0200, Andy Shevchenko wrote:
> We return -ENODEV if ACPI provides a GPIO resource. Looks really
> wrong.
> If it has even been tested?

Samuel, Christophe, anything I have to address?

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/nfc/st21nfca/i2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nfc/st21nfca/i2c.c b/drivers/nfc/st21nfca/i2c.c
> index 5a82f553906c..737384d287aa 100644
> --- a/drivers/nfc/st21nfca/i2c.c
> +++ b/drivers/nfc/st21nfca/i2c.c
> @@ -514,9 +514,9 @@ static int
> st21nfca_hci_i2c_acpi_request_resources(struct i2c_client *client)
>  	/* Get EN GPIO from ACPI */
>  	gpiod_ena = devm_gpiod_get_index(dev, ST21NFCA_GPIO_NAME_EN,
> 1,
>  					 GPIOD_OUT_LOW);
> -	if (!IS_ERR(gpiod_ena)) {
> +	if (IS_ERR(gpiod_ena)) {
>  		nfc_err(dev, "Unable to get ENABLE GPIO\n");
> -		return -ENODEV;
> +		return PTR_ERR(gpiod_ena);
>  	}
>  
>  	phy->gpio_ena = desc_to_gpio(gpiod_ena);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

* Re: [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code
  2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
                   ` (5 preceding siblings ...)
  2017-04-04 16:16 ` Andy Shevchenko
@ 2017-04-05  8:09 ` Samuel Ortiz
  6 siblings, 0 replies; 10+ messages in thread
From: Samuel Ortiz @ 2017-04-05  8:09 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-wireless, Christophe Ricard

Hi Andy,

On Tue, Mar 07, 2017 at 12:25:42PM +0200, Andy Shevchenko wrote:
> We return -ENODEV if ACPI provides a GPIO resource. Looks really wrong.
> If it has even been tested?
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/nfc/st21nfca/i2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
All 5 patches applied to nfc-next, thanks.

Cheers,
Samuel.

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

end of thread, other threads:[~2017-04-05  8:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-07 10:25 [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
2017-03-07 10:25 ` [PATCH v2 2/5] NFC: st21nfca: Get rid of platform data Andy Shevchenko
2017-03-07 10:25 ` [PATCH v2 3/5] NFC: st21nfca: Get rid of "interesting" use of interrupt polarity Andy Shevchenko
2017-03-07 10:25 ` [PATCH v2 4/5] NFC: st21nfca: Covert to use GPIO descriptor Andy Shevchenko
2017-03-07 10:25 ` [PATCH v2 5/5] NFC: st21nfca: Use unified device property API meaningfully Andy Shevchenko
2017-03-17 15:49 ` [PATCH v2 1/5] NFC: st21nfca: Fix obvious typo when check error code Andy Shevchenko
2017-03-21  2:29   ` Christophe Ricard
2017-03-21 18:36     ` Andy Shevchenko
2017-04-04 16:16 ` Andy Shevchenko
2017-04-05  8:09 ` Samuel Ortiz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.