linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock
@ 2016-12-22  4:18 Geoff Lansberry
  2016-12-22  4:18 ` [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Geoff Lansberry @ 2016-12-22  4:18 UTC (permalink / raw)
  To: linux-wireless
  Cc: lauro.venancio, aloisio.almeida, sameo, robh+dt, mark.rutland,
	netdev, devicetree, linux-kernel, mgreer, justin,
	Geoff Lansberry

The TRF7970A has configuration options to support hardware designs
which use a 27.12MHz clock. This commit adds a device tree option
'clock-frequency' to support configuring the this chip for default
13.56MHz clock or the optional 27.12MHz clock.

Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
---
 .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 +
 drivers/nfc/trf7970a.c                             | 50 +++++++++++++++++-----
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 32b35a0..8b01fc81 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -21,6 +21,7 @@ Optional SoC Specific Properties:
 - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
   where an extra byte is returned by Read Multiple Block commands issued
   to Type 5 tags.
+- clock-frequency: Set to specify that the input frequency to the trf7970a is 13560000Hz or 27120000Hz
 
 Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 
@@ -43,6 +44,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 		irq-status-read-quirk;
 		en2-rf-quirk;
 		t5t-rmb-extra-byte-quirk;
+		clock-frequency = <27120000>;
 		status = "okay";
 	};
 };
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 26c9dbb..b1cd4ef 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -124,6 +124,9 @@
 		 NFC_PROTO_ISO15693_MASK | NFC_PROTO_NFC_DEP_MASK)
 
 #define TRF7970A_AUTOSUSPEND_DELAY		30000 /* 30 seconds */
+#define TRF7970A_13MHZ_CLOCK_FREQUENCY		13560000
+#define TRF7970A_27MHZ_CLOCK_FREQUENCY		27120000
+
 
 #define TRF7970A_RX_SKB_ALLOC_SIZE		256
 
@@ -1056,12 +1059,11 @@ static int trf7970a_init(struct trf7970a *trf)
 
 	trf->chip_status_ctrl &= ~TRF7970A_CHIP_STATUS_RF_ON;
 
-	ret = trf7970a_write(trf, TRF7970A_MODULATOR_SYS_CLK_CTRL, 0);
+	ret = trf7970a_write(trf, TRF7970A_MODULATOR_SYS_CLK_CTRL,
+			trf->modulator_sys_clk_ctrl);
 	if (ret)
 		goto err_out;
 
-	trf->modulator_sys_clk_ctrl = 0;
-
 	ret = trf7970a_write(trf, TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS,
 			TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS_WLH_96 |
 			TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS_WLL_32);
@@ -1181,27 +1183,37 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
 	switch (tech) {
 	case NFC_DIGITAL_RF_TECH_106A:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443A_106;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_OOK;
 		trf->guard_time = TRF7970A_GUARD_TIME_NFCA;
 		break;
 	case NFC_DIGITAL_RF_TECH_106B:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443B_106;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_ASK10;
 		trf->guard_time = TRF7970A_GUARD_TIME_NFCB;
 		break;
 	case NFC_DIGITAL_RF_TECH_212F:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_FELICA_212;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_ASK10;
 		trf->guard_time = TRF7970A_GUARD_TIME_NFCF;
 		break;
 	case NFC_DIGITAL_RF_TECH_424F:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_FELICA_424;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_ASK10;
 		trf->guard_time = TRF7970A_GUARD_TIME_NFCF;
 		break;
 	case NFC_DIGITAL_RF_TECH_ISO15693:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_15693_SGL_1OF4_2648;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_OOK;
 		trf->guard_time = TRF7970A_GUARD_TIME_15693;
 		break;
 	default:
@@ -1571,17 +1583,23 @@ static int trf7970a_tg_config_rf_tech(struct trf7970a *trf, int tech)
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
 			TRF7970A_ISO_CTRL_NFC_CE |
 			TRF7970A_ISO_CTRL_NFC_CE_14443A;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_OOK;
 		break;
 	case NFC_DIGITAL_RF_TECH_212F:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
 			TRF7970A_ISO_CTRL_NFC_NFCF_212;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_ASK10;
 		break;
 	case NFC_DIGITAL_RF_TECH_424F:
 		trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
 			TRF7970A_ISO_CTRL_NFC_NFCF_424;
-		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+		trf->modulator_sys_clk_ctrl =
+			(trf->modulator_sys_clk_ctrl & 0xf8) |
+			TRF7970A_MODULATOR_DEPTH_ASK10;
 		break;
 	default:
 		dev_dbg(trf->dev, "Unsupported rf technology: %d\n", tech);
@@ -1987,6 +2005,7 @@ static int trf7970a_probe(struct spi_device *spi)
 	struct device_node *np = spi->dev.of_node;
 	struct trf7970a *trf;
 	int uvolts, autosuspend_delay, ret;
+	u32 clk_freq = TRF7970A_13MHZ_CLOCK_FREQUENCY;
 
 	if (!np) {
 		dev_err(&spi->dev, "No Device Tree entry\n");
@@ -2043,6 +2062,15 @@ static int trf7970a_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	of_property_read_u32(np, "clock-frequency", &clk_freq);
+	if ((clk_freq != TRF7970A_27MHZ_CLOCK_FREQUENCY) ||
+		(clk_freq != TRF7970A_13MHZ_CLOCK_FREQUENCY)) {
+		dev_err(trf->dev,
+			"clock-frequency (%u Hz) unsupported\n",
+			clk_freq);
+		return -EINVAL;
+	}
+
 	if (of_property_read_bool(np, "en2-rf-quirk"))
 		trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
 
-- 
2.7.4

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

* [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
  2016-12-22  4:18 [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
@ 2016-12-22  4:18 ` Geoff Lansberry
  2016-12-22 23:02   ` Rob Herring
  2017-01-20 18:37   ` Mark Greer
  2016-12-22  4:18 ` [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel Geoff Lansberry
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Geoff Lansberry @ 2016-12-22  4:18 UTC (permalink / raw)
  To: linux-wireless
  Cc: lauro.venancio, aloisio.almeida, sameo, robh+dt, mark.rutland,
	netdev, devicetree, linux-kernel, mgreer, justin,
	Geoff Lansberry

The TRF7970A has configuration options for supporting hardware designs
with 1.8 Volt or 3.3 Volt IO.   This commit adds a device tree option,
using a fixed regulator binding, for setting the io voltage to match
the hardware configuration. If no option is supplied it defaults to
3.3 volt configuration.

Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
---
 .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 ++
 drivers/nfc/trf7970a.c                             | 26 +++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 8b01fc81..b5777d8 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -21,6 +21,7 @@ Optional SoC Specific Properties:
 - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
   where an extra byte is returned by Read Multiple Block commands issued
   to Type 5 tags.
+- vdd-io-supply: Regulator specifying voltage for vdd-io
 - clock-frequency: Set to specify that the input frequency to the trf7970a is 13560000Hz or 27120000Hz
 
 Example (for ARM-based BeagleBone with TRF7970A on SPI1):
@@ -40,6 +41,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 				  <&gpio2 5 GPIO_ACTIVE_LOW>;
 		vin-supply = <&ldo3_reg>;
 		vin-voltage-override = <5000000>;
+		vdd-io-supply = <&ldo2_reg>;
 		autosuspend-delay = <30000>;
 		irq-status-read-quirk;
 		en2-rf-quirk;
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index b1cd4ef..e3c72c6 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -444,6 +444,7 @@ struct trf7970a {
 	u8				iso_ctrl_tech;
 	u8				modulator_sys_clk_ctrl;
 	u8				special_fcn_reg1;
+	u8				io_ctrl;
 	unsigned int			guard_time;
 	int				technology;
 	int				framing;
@@ -1051,6 +1052,11 @@ static int trf7970a_init(struct trf7970a *trf)
 	if (ret)
 		goto err_out;
 
+	ret = trf7970a_write(trf, TRF7970A_REG_IO_CTRL,
+			trf->io_ctrl | TRF7970A_REG_IO_CTRL_VRS(0x1));
+	if (ret)
+		goto err_out;
+
 	ret = trf7970a_write(trf, TRF7970A_NFC_TARGET_LEVEL, 0);
 	if (ret)
 		goto err_out;
@@ -1767,7 +1773,7 @@ static int _trf7970a_tg_listen(struct nfc_digital_dev *ddev, u16 timeout,
 		goto out_err;
 
 	ret = trf7970a_write(trf, TRF7970A_REG_IO_CTRL,
-			TRF7970A_REG_IO_CTRL_VRS(0x1));
+			trf->io_ctrl | TRF7970A_REG_IO_CTRL_VRS(0x1));
 	if (ret)
 		goto out_err;
 
@@ -2105,6 +2111,24 @@ static int trf7970a_probe(struct spi_device *spi)
 	if (uvolts > 4000000)
 		trf->chip_status_ctrl = TRF7970A_CHIP_STATUS_VRS5_3;
 
+	trf->regulator = devm_regulator_get(&spi->dev, "vdd-io");
+	if (IS_ERR(trf->regulator)) {
+		ret = PTR_ERR(trf->regulator);
+		dev_err(trf->dev, "Can't get VDD_IO regulator: %d\n", ret);
+		goto err_destroy_lock;
+	}
+
+	ret = regulator_enable(trf->regulator);
+	if (ret) {
+		dev_err(trf->dev, "Can't enable VDD_IO: %d\n", ret);
+		goto err_destroy_lock;
+	}
+
+	if (regulator_get_voltage(trf->regulator) == 1800000) {
+		trf->io_ctrl = TRF7970A_REG_IO_CTRL_IO_LOW;
+		dev_dbg(trf->dev, "trf7970a config vdd_io to 1.8V\n");
+	}
+
 	trf->ddev = nfc_digital_allocate_device(&trf7970a_nfc_ops,
 			TRF7970A_SUPPORTED_PROTOCOLS,
 			NFC_DIGITAL_DRV_CAPS_IN_CRC |
-- 
2.7.4

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

* [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2016-12-22  4:18 [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
  2016-12-22  4:18 ` [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
@ 2016-12-22  4:18 ` Geoff Lansberry
  2016-12-24  6:01   ` Mark Greer
  2016-12-22 23:01 ` [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Rob Herring
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Geoff Lansberry @ 2016-12-22  4:18 UTC (permalink / raw)
  To: linux-wireless
  Cc: lauro.venancio, aloisio.almeida, sameo, robh+dt, mark.rutland,
	netdev, devicetree, linux-kernel, mgreer, justin, Jaret Cantu,
	Geoff Lansberry

From: Jaret Cantu <jaret.cantu@timesys.com>

Repeated polling attempts cause a NULL dereference error to occur.
This is because the state of the trf7970a is currently reading but
another request has been made to send a command before it has finished.

The solution is to properly kill the waiting reading (workqueue)
before failing on the send.

Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
---
 drivers/nfc/trf7970a.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index e3c72c6..ba5f9b8 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1496,6 +1496,10 @@ static int trf7970a_send_cmd(struct nfc_digital_dev *ddev,
 			(trf->state != TRF7970A_ST_IDLE_RX_BLOCKED)) {
 		dev_err(trf->dev, "%s - Bogus state: %d\n", __func__,
 				trf->state);
+		if (trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA ||
+		    trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA_CONT)
+			trf->ignore_timeout =
+				!cancel_delayed_work(&trf->timeout_work);
 		ret = -EIO;
 		goto out_err;
 	}
-- 
2.7.4

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

* Re: [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock
  2016-12-22  4:18 [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
  2016-12-22  4:18 ` [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
  2016-12-22  4:18 ` [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel Geoff Lansberry
@ 2016-12-22 23:01 ` Rob Herring
  2017-01-19 23:35 ` Mark Greer
  2017-04-05  9:06 ` Samuel Ortiz
  4 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2016-12-22 23:01 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
	mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin

On Wed, Dec 21, 2016 at 11:18:32PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options to support hardware designs
> which use a 27.12MHz clock. This commit adds a device tree option
> 'clock-frequency' to support configuring the this chip for default
> 13.56MHz clock or the optional 27.12MHz clock.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/nfc/trf7970a.c                             | 50 +++++++++++++++++-----
>  2 files changed, 41 insertions(+), 11 deletions(-)

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

* Re: [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
  2016-12-22  4:18 ` [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
@ 2016-12-22 23:02   ` Rob Herring
  2017-01-20 18:37   ` Mark Greer
  1 sibling, 0 replies; 16+ messages in thread
From: Rob Herring @ 2016-12-22 23:02 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
	mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin

On Wed, Dec 21, 2016 at 11:18:33PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options for supporting hardware designs
> with 1.8 Volt or 3.3 Volt IO.   This commit adds a device tree option,
> using a fixed regulator binding, for setting the io voltage to match
> the hardware configuration. If no option is supplied it defaults to
> 3.3 volt configuration.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 ++

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/nfc/trf7970a.c                             | 26 +++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2016-12-22  4:18 ` [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel Geoff Lansberry
@ 2016-12-24  6:01   ` Mark Greer
  2016-12-24 16:17     ` Geoff Lansberry
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Greer @ 2016-12-24  6:01 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
	mark.rutland, netdev, devicetree, linux-kernel, justin,
	Jaret Cantu

On Wed, Dec 21, 2016 at 11:18:34PM -0500, Geoff Lansberry wrote:
> From: Jaret Cantu <jaret.cantu@timesys.com>
> 
> Repeated polling attempts cause a NULL dereference error to occur.
> This is because the state of the trf7970a is currently reading but
> another request has been made to send a command before it has finished.
> 
> The solution is to properly kill the waiting reading (workqueue)
> before failing on the send.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---

You've still provided virtually no information on the actual problem(s)
nor justified why you think this is the best solution.  You're adding
code to a section of code that should _never_ be executed so the only
reasonable things I can infer is that there are, at least, two problems:

1) There is a bug causing execution to get into this block of code.

2) Once in this block of code, there is another bug.

You seem to be attempting to fix 2) and completely ignoring 1).
1) is the first bug that needs to be root-caused and fixed.

Also, what exactly is the "NULL dereference error" you mention?
Is this the neard crash you talked about in another thread or is
this a kernel crash?  If it is the kernel crash, please post the
relevant information.  If this is the neard crash - which seems
unlikely - then how can changing a section of kernel code that
shouldn't be executed in the first place fix that?

Mark
--

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2016-12-24  6:01   ` Mark Greer
@ 2016-12-24 16:17     ` Geoff Lansberry
  2016-12-24 17:24       ` Mark Greer
  0 siblings, 1 reply; 16+ messages in thread
From: Geoff Lansberry @ 2016-12-24 16:17 UTC (permalink / raw)
  To: Mark Greer
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

Mark - I'm sorry, but I did not write this code, and therefore was not
able to accurately describe it.   It is fixing a different issue, not
the neard segfault that we are still chasing. Last week Jaret Cantu
sent a separate email explaining the purpose of the code, which had
you copied, did you see that?   Does it explain why it was done to
your satisfaction?   I've asked him to join in on the effort to push
the change upstream, however he will not be available until the new
year.

I know you did suggest that we split off that change from the others,
and if now is the time to do that, let me know.   If you don't have
the email from Jaret, also please let me know and I will forward it to
you.

Geoff
Geoff Lansberry


Engineering Guy
Kuvée, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com



On Sat, Dec 24, 2016 at 1:01 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Wed, Dec 21, 2016 at 11:18:34PM -0500, Geoff Lansberry wrote:
>> From: Jaret Cantu <jaret.cantu@timesys.com>
>>
>> Repeated polling attempts cause a NULL dereference error to occur.
>> This is because the state of the trf7970a is currently reading but
>> another request has been made to send a command before it has finished.
>>
>> The solution is to properly kill the waiting reading (workqueue)
>> before failing on the send.
>>
>> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
>> ---
>
> You've still provided virtually no information on the actual problem(s)
> nor justified why you think this is the best solution.  You're adding
> code to a section of code that should _never_ be executed so the only
> reasonable things I can infer is that there are, at least, two problems:
>
> 1) There is a bug causing execution to get into this block of code.
>
> 2) Once in this block of code, there is another bug.
>
> You seem to be attempting to fix 2) and completely ignoring 1).
> 1) is the first bug that needs to be root-caused and fixed.
>
> Also, what exactly is the "NULL dereference error" you mention?
> Is this the neard crash you talked about in another thread or is
> this a kernel crash?  If it is the kernel crash, please post the
> relevant information.  If this is the neard crash - which seems
> unlikely - then how can changing a section of kernel code that
> shouldn't be executed in the first place fix that?
>
> Mark
> --

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2016-12-24 16:17     ` Geoff Lansberry
@ 2016-12-24 17:24       ` Mark Greer
  2016-12-27 14:18         ` Geoff Lansberry
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Greer @ 2016-12-24 17:24 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

On Sat, Dec 24, 2016 at 11:17:18AM -0500, Geoff Lansberry wrote:
> Mark - I'm sorry, but I did not write this code, and therefore was not
> able to accurately describe it.   It is fixing a different issue, not
> the neard segfault that we are still chasing. Last week Jaret Cantu
> sent a separate email explaining the purpose of the code, which had
> you copied, did you see that?

Hm, no, I didn't.  I received an email from Justin Bronder but not from
Jaret Cantu.  Justin's email did help but is still pretty high-level.
We need a clear understanding as to what is happening in the digital
layer and the driver to know how execution is getting into a block of
error handling code that should never be executed.  Once we understand
that we can start thinking about what the best fix is.

> Does it explain why it was done to
> your satisfaction?   I've asked him to join in on the effort to push
> the change upstream, however he will not be available until the new
> year.

I expect that it would help if he joins.  After the holidays is fine -
I think many people are taking it easy for the next week or so, anyway.

> I know you did suggest that we split off that change from the others,
> and if now is the time to do that, let me know.   If you don't have
> the email from Jaret, also please let me know and I will forward it to
> you.

I think it would help you if you split it off because the first two patches
have a good chance of being accepted but this one doesn't (yet).  If you
separate the them, it will make it easier for Samuel to take the first two
(or he may take the first two anyway but its always good to make it as
easy maintainers as you can).

Mark
--

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2016-12-24 17:24       ` Mark Greer
@ 2016-12-27 14:18         ` Geoff Lansberry
  2017-01-03 16:33           ` Mark Greer
  0 siblings, 1 reply; 16+ messages in thread
From: Geoff Lansberry @ 2016-12-27 14:18 UTC (permalink / raw)
  To: Mark Greer
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

Mark - I will split this off soon.

In the meantime - here is some more info about how we use it.

We do use NFC structures.    I did find an interesting clue in that
there are certain bottles that cause neard to segfault,  I'm not sure
what is different about them.  We write a string, like
"coppola_chardonnay_2015" to the bottles.  Come to think of it, I
haven't done anything special to make that an ndef record, just
assumed that it would happen by default, I'll look into this further.
  Also, I've been running neard with --plugin nfctype2. Just in case
the problem was happening due to cycling through other tag types.   It
didn't seem to make any difference, but I have not gone back to
default.

Geoff
Geoff Lansberry


Engineering Guy
Kuvée, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com



On Sat, Dec 24, 2016 at 12:24 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Sat, Dec 24, 2016 at 11:17:18AM -0500, Geoff Lansberry wrote:
>> Mark - I'm sorry, but I did not write this code, and therefore was not
>> able to accurately describe it.   It is fixing a different issue, not
>> the neard segfault that we are still chasing. Last week Jaret Cantu
>> sent a separate email explaining the purpose of the code, which had
>> you copied, did you see that?
>
> Hm, no, I didn't.  I received an email from Justin Bronder but not from
> Jaret Cantu.  Justin's email did help but is still pretty high-level.
> We need a clear understanding as to what is happening in the digital
> layer and the driver to know how execution is getting into a block of
> error handling code that should never be executed.  Once we understand
> that we can start thinking about what the best fix is.
>
>> Does it explain why it was done to
>> your satisfaction?   I've asked him to join in on the effort to push
>> the change upstream, however he will not be available until the new
>> year.
>
> I expect that it would help if he joins.  After the holidays is fine -
> I think many people are taking it easy for the next week or so, anyway.
>
>> I know you did suggest that we split off that change from the others,
>> and if now is the time to do that, let me know.   If you don't have
>> the email from Jaret, also please let me know and I will forward it to
>> you.
>
> I think it would help you if you split it off because the first two patches
> have a good chance of being accepted but this one doesn't (yet).  If you
> separate the them, it will make it easier for Samuel to take the first two
> (or he may take the first two anyway but its always good to make it as
> easy maintainers as you can).
>
> Mark
> --

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2016-12-27 14:18         ` Geoff Lansberry
@ 2017-01-03 16:33           ` Mark Greer
  2017-01-03 18:35             ` Geoff Lansberry
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Greer @ 2017-01-03 16:33 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

[Please stop top-posting.  Bottom-post only to these lists.]

Hi Geoff & happy new year.

On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
> Mark - I will split this off soon.

OK

> In the meantime - here is some more info about how we use it.
> 
> We do use NFC structures.    I did find an interesting clue in that
> there are certain bottles that cause neard to segfault,  I'm not sure
> what is different about them.  We write a string, like
> "coppola_chardonnay_2015" to the bottles.

Off the top of my head, it could be the length of the text.
It would be useful to compare the data that works to the data
that doesn't work.  Can you install NXP's 'TagInfo' app on a
smartphone and scan tags with working & non-working data?
You can email the data from the app to yourself, edit out
the cruft, and share here.

> Come to think of it, I
> haven't done anything special to make that an ndef record, just
> assumed that it would happen by default, I'll look into this further.

If you wrote the data using neard, it will be NDEF formatted.
Since it is working this well, it is virtually guaranteed that
the data is NDEF formatted.

>   Also, I've been running neard with --plugin nfctype2. Just in case
> the problem was happening due to cycling through other tag types.   It
> didn't seem to make any difference, but I have not gone back to
> default.

Good to know, thanks.

Mark
--

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2017-01-03 16:33           ` Mark Greer
@ 2017-01-03 18:35             ` Geoff Lansberry
  2017-01-03 21:21               ` Mark Greer
  0 siblings, 1 reply; 16+ messages in thread
From: Geoff Lansberry @ 2017-01-03 18:35 UTC (permalink / raw)
  To: Mark Greer
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

On Tue, Jan 3, 2017 at 11:33 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> [Please stop top-posting.  Bottom-post only to these lists.]

Sorry; gmail keeps baiting me to do it...

>
> Hi Geoff & happy new year.
>
> On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
>> Mark - I will split this off soon.
>
> OK

Thanks for the reminder!

>
>> In the meantime - here is some more info about how we use it.
>>
>> We do use NFC structures.    I did find an interesting clue in that
>> there are certain bottles that cause neard to segfault,  I'm not sure
>> what is different about them.  We write a string, like
>> "coppola_chardonnay_2015" to the bottles.
>
> Off the top of my head, it could be the length of the text.
> It would be useful to compare the data that works to the data
> that doesn't work.  Can you install NXP's 'TagInfo' app on a
> smartphone and scan tags with working & non-working data?
> You can email the data from the app to yourself, edit out
> the cruft, and share here.

The data is always the same - and the tags are all the same.  Only
difference is that the tag is physically different, and perhaps
orientation; distance from antenna to tag is fixed.  I can't even
write the tags at all, so reading them will show blank.   Also a minor
but significant detail, is that the tags are embedded in such a way
that the phone cannot get close enough to them to connect.

>
>> Come to think of it, I
>> haven't done anything special to make that an ndef record, just
>> assumed that it would happen by default, I'll look into this further.
>
> If you wrote the data using neard, it will be NDEF formatted.
> Since it is working this well, it is virtually guaranteed that
> the data is NDEF formatted.

OK, good.

>
>>   Also, I've been running neard with --plugin nfctype2. Just in case
>> the problem was happening due to cycling through other tag types.   It
>> didn't seem to make any difference, but I have not gone back to
>> default.
>
> Good to know, thanks.
>
> Mark
> --

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2017-01-03 18:35             ` Geoff Lansberry
@ 2017-01-03 21:21               ` Mark Greer
  2017-01-08 22:32                 ` Geoff Lansberry
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Greer @ 2017-01-03 21:21 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

On Tue, Jan 03, 2017 at 01:35:18PM -0500, Geoff Lansberry wrote:
> On Tue, Jan 3, 2017 at 11:33 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> > On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:

> >> In the meantime - here is some more info about how we use it.
> >>
> >> We do use NFC structures.    I did find an interesting clue in that
> >> there are certain bottles that cause neard to segfault,  I'm not sure
> >> what is different about them.  We write a string, like
> >> "coppola_chardonnay_2015" to the bottles.
> >
> > Off the top of my head, it could be the length of the text.
> > It would be useful to compare the data that works to the data
> > that doesn't work.  Can you install NXP's 'TagInfo' app on a
> > smartphone and scan tags with working & non-working data?
> > You can email the data from the app to yourself, edit out
> > the cruft, and share here.
> 
> The data is always the same - and the tags are all the same.  Only
> difference is that the tag is physically different, and perhaps
> orientation; distance from antenna to tag is fixed.

Interesting...  They're all type 2 tags, right?

> I can't even
> write the tags at all, so reading them will show blank.   Also a minor
> but significant detail, is that the tags are embedded in such a way
> that the phone cannot get close enough to them to connect.

This section had me completely confused for a couple minutes until I realized
that you mean that you can read & write the tags using the trf7970a with
an attached antenna but not with your phone.  Is that correct?

If so, try a tag that isn't embedded in something else and move it around
the back of the phone.  Try to find where it works best.  The phone
manufacturers are notorius for paying little attention to the NFC antenna
they put on their products.  For example, I have a Samsung S5 next to me
and it seems to work best around the center of the phone.  I've used others
where I had to use the upper-left or upper-right corner of the phone.

Mark
--

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

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
  2017-01-03 21:21               ` Mark Greer
@ 2017-01-08 22:32                 ` Geoff Lansberry
  0 siblings, 0 replies; 16+ messages in thread
From: Geoff Lansberry @ 2017-01-08 22:32 UTC (permalink / raw)
  To: Mark Greer
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, Justin Bronder, Jaret Cantu

On Tue, Jan 3, 2017 at 4:21 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Tue, Jan 03, 2017 at 01:35:18PM -0500, Geoff Lansberry wrote:
>> On Tue, Jan 3, 2017 at 11:33 AM, Mark Greer <mgreer@animalcreek.com> wrote:
>> > On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
>
>> >> In the meantime - here is some more info about how we use it.
>> >>
>> >> We do use NFC structures.    I did find an interesting clue in that
>> >> there are certain bottles that cause neard to segfault,  I'm not sure
>> >> what is different about them.  We write a string, like
>> >> "coppola_chardonnay_2015" to the bottles.
>> >
>> > Off the top of my head, it could be the length of the text.
>> > It would be useful to compare the data that works to the data
>> > that doesn't work.  Can you install NXP's 'TagInfo' app on a
>> > smartphone and scan tags with working & non-working data?
>> > You can email the data from the app to yourself, edit out
>> > the cruft, and share here.
>>
>> The data is always the same - and the tags are all the same.  Only
>> difference is that the tag is physically different, and perhaps
>> orientation; distance from antenna to tag is fixed.
>
> Interesting...  They're all type 2 tags, right?

Yes type 2.

>
>> I can't even
>> write the tags at all, so reading them will show blank.   Also a minor
>> but significant detail, is that the tags are embedded in such a way
>> that the phone cannot get close enough to them to connect.
>
> This section had me completely confused for a couple minutes until I realized
> that you mean that you can read & write the tags using the trf7970a with
> an attached antenna but not with your phone.  Is that correct?

Correct, due to the physical arrangement of the part the tag is embedded in.

>
> If so, try a tag that isn't embedded in something else and move it around
> the back of the phone.  Try to find where it works best.  The phone
> manufacturers are notorius for paying little attention to the NFC antenna
> they put on their products.  For example, I have a Samsung S5 next to me
> and it seems to work best around the center of the phone.  I've used others
> where I had to use the upper-left or upper-right corner of the phone.

I can borrow a phone and try, I do have some other tags.  This will
take me some time and
 I'm not optimistic that we will learn much, other than that the tag
was not programmed when
it does not work.   Don't wait on this answer.
>
> Mark
> --

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

* Re: [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock
  2016-12-22  4:18 [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
                   ` (2 preceding siblings ...)
  2016-12-22 23:01 ` [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Rob Herring
@ 2017-01-19 23:35 ` Mark Greer
  2017-04-05  9:06 ` Samuel Ortiz
  4 siblings, 0 replies; 16+ messages in thread
From: Mark Greer @ 2017-01-19 23:35 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
	mark.rutland, netdev, devicetree, linux-kernel, justin

On Wed, Dec 21, 2016 at 11:18:32PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options to support hardware designs
> which use a 27.12MHz clock. This commit adds a device tree option
> 'clock-frequency' to support configuring the this chip for default
> 13.56MHz clock or the optional 27.12MHz clock.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---

Acked-by: Mark Greer <mgreer@animalcreek.com>

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

* Re: [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
  2016-12-22  4:18 ` [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
  2016-12-22 23:02   ` Rob Herring
@ 2017-01-20 18:37   ` Mark Greer
  1 sibling, 0 replies; 16+ messages in thread
From: Mark Greer @ 2017-01-20 18:37 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
	mark.rutland, netdev, devicetree, linux-kernel, justin

On Wed, Dec 21, 2016 at 11:18:33PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options for supporting hardware designs
> with 1.8 Volt or 3.3 Volt IO.   This commit adds a device tree option,
> using a fixed regulator binding, for setting the io voltage to match
> the hardware configuration. If no option is supplied it defaults to
> 3.3 volt configuration.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 ++
>  drivers/nfc/trf7970a.c                             | 26 +++++++++++++++++++++-

Acked-by: Mark Greer <mgreer@animalcreek.com>

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

* Re: [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock
  2016-12-22  4:18 [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
                   ` (3 preceding siblings ...)
  2017-01-19 23:35 ` Mark Greer
@ 2017-04-05  9:06 ` Samuel Ortiz
  4 siblings, 0 replies; 16+ messages in thread
From: Samuel Ortiz @ 2017-04-05  9:06 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, robh+dt, mark.rutland, netdev, devicetree,
	linux-kernel, mgreer, justin

Hi Geoff,

On Wed, Dec 21, 2016 at 11:18:32PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options to support hardware designs
> which use a 27.12MHz clock. This commit adds a device tree option
> 'clock-frequency' to support configuring the this chip for default
> 13.56MHz clock or the optional 27.12MHz clock.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 +
>  drivers/nfc/trf7970a.c                             | 50 +++++++++++++++++-----
>  2 files changed, 41 insertions(+), 11 deletions(-)
Patches #1 and #2 applied to nfc-next. I'll wait for you to rework #3
before merging.

Cheers,
Samuel.

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

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

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-22  4:18 [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
2016-12-22  4:18 ` [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
2016-12-22 23:02   ` Rob Herring
2017-01-20 18:37   ` Mark Greer
2016-12-22  4:18 ` [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel Geoff Lansberry
2016-12-24  6:01   ` Mark Greer
2016-12-24 16:17     ` Geoff Lansberry
2016-12-24 17:24       ` Mark Greer
2016-12-27 14:18         ` Geoff Lansberry
2017-01-03 16:33           ` Mark Greer
2017-01-03 18:35             ` Geoff Lansberry
2017-01-03 21:21               ` Mark Greer
2017-01-08 22:32                 ` Geoff Lansberry
2016-12-22 23:01 ` [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock Rob Herring
2017-01-19 23:35 ` Mark Greer
2017-04-05  9:06 ` Samuel Ortiz

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).