[Patch] NFC: trf7970a:

[Patch] NFC: trf7970a:

[Geoff Lansberry]

The current version of the trf7970a driver is missing support for several features that we needed to operate a custom board. 
We feel that these features will be useful to others as well, and we want to share them.

1: Support for using a gpio as Slave-Select. Our processor has several devices on the spi bus, and we ran out of ss lines.  This patch gives TRF7970A the ability to
 drive the ss line of the chip from a gpio that is defined in the device tree.

2. When reviewing problems we were having in our implementation with TI support staff, they recommended that during initialization, address 0x18 should be written to zero.  This patch adds that change

3. This existing version of the driver assumes that the crystal driving the trf7970a is 13.56 MHz, because there are several places in the driver code where the rel
ated register is re-written, there is clean way to change to 27.12 MHz.  This patch adds a device tree option for 27 MHz and properly or's in changes in locations w
here the register is changed.

4. the existing version of the driver assumes that 3.3 volt io is used. The trf7970a has a special register where you can configure it for 1.8 volt io.  This patch
adds a device tree option to select that this setting should be made.

 [PATCH 1/4] NFC: trf7970a: Add support for gpio as SS
 [PATCH 2/4] NFC: trf7970a: add TI recommended write of zero to
 [PATCH 3/4] NFC: trf7970a: add device tree option for 27MHz clock
 [PATCH 4/4] NFC: trf7970a: Add device tree option of 1.8 Volt IO

[PATCH 1/4] NFC: trf7970a: Add support for gpio as SS

[Geoff Lansberry]

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

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 32b35a0..09c5056 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -7,6 +7,7 @@ Required properties:
 - interrupts: A single interrupt specifier.
 - ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the
   TRF7970A.
+- ti,ss-gpio: GPIO entry used for active low SS (spi slave select) on the TRF7970A
 - vin-supply: Regulator for supply voltage to VIN pin
 
 Optional SoC Specific Properties:
@@ -37,6 +38,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 		interrupts = <14 0>;
 		ti,enable-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>,
 				  <&gpio2 5 GPIO_ACTIVE_LOW>;
+		ti,ss-gpio = <&gpio2 4 GPIO_ACTIVE_HIGH>;
 		vin-supply = <&ldo3_reg>;
 		vin-voltage-override = <5000000>;
 		autosuspend-delay = <30000>;
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 10842b7..2c3530a 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -450,6 +450,7 @@ struct trf7970a {
 	bool				adjust_resp_len;
 	int				en2_gpio;
 	int				en_gpio;
+	int				ss_gpio;
 	struct mutex			lock;
 	unsigned int			timeout;
 	bool				ignore_timeout;
@@ -462,9 +463,11 @@ static int trf7970a_cmd(struct trf7970a *trf, u8 opcode)
 	u8 cmd = TRF7970A_CMD_BIT_CTRL | TRF7970A_CMD_BIT_OPCODE(opcode);
 	int ret;
 
+	gpio_set_value(trf->ss_gpio, 0);
 	dev_dbg(trf->dev, "cmd: 0x%x\n", cmd);
 
 	ret = spi_write(trf->spi, &cmd, 1);
+	gpio_set_value(trf->ss_gpio, 1);
 	if (ret)
 		dev_err(trf->dev, "%s - cmd: 0x%x, ret: %d\n", __func__, cmd,
 				ret);
@@ -476,7 +479,9 @@ static int trf7970a_read(struct trf7970a *trf, u8 reg, u8 *val)
 	u8 addr = TRF7970A_CMD_BIT_RW | reg;
 	int ret;
 
+	gpio_set_value(trf->ss_gpio, 0);
 	ret = spi_write_then_read(trf->spi, &addr, 1, val, 1);
+	gpio_set_value(trf->ss_gpio, 1);
 	if (ret)
 		dev_err(trf->dev, "%s - addr: 0x%x, ret: %d\n", __func__, addr,
 				ret);
@@ -493,6 +498,7 @@ static int trf7970a_read_cont(struct trf7970a *trf, u8 reg, u8 *buf, size_t len)
 	struct spi_message m;
 	int ret;
 
+	gpio_set_value(trf->ss_gpio, 0);
 	dev_dbg(trf->dev, "read_cont(0x%x, %zd)\n", addr, len);
 
 	spi_message_init(&m);
@@ -508,6 +514,7 @@ static int trf7970a_read_cont(struct trf7970a *trf, u8 reg, u8 *buf, size_t len)
 	spi_message_add_tail(&t[1], &m);
 
 	ret = spi_sync(trf->spi, &m);
+	gpio_set_value(trf->ss_gpio, 1);
 	if (ret)
 		dev_err(trf->dev, "%s - addr: 0x%x, ret: %d\n", __func__, addr,
 				ret);
@@ -519,9 +526,11 @@ static int trf7970a_write(struct trf7970a *trf, u8 reg, u8 val)
 	u8 buf[2] = { reg, val };
 	int ret;
 
+	gpio_set_value(trf->ss_gpio, 0);
 	dev_dbg(trf->dev, "write(0x%x): 0x%x\n", reg, val);
 
 	ret = spi_write(trf->spi, buf, 2);
+	gpio_set_value(trf->ss_gpio, 1);
 	if (ret)
 		dev_err(trf->dev, "%s - write: 0x%x 0x%x, ret: %d\n", __func__,
 				buf[0], buf[1], ret);
@@ -535,6 +544,7 @@ static int trf7970a_read_irqstatus(struct trf7970a *trf, u8 *status)
 	u8 buf[2];
 	u8 addr;
 
+	gpio_set_value(trf->ss_gpio, 0);
 	addr = TRF7970A_IRQ_STATUS | TRF7970A_CMD_BIT_RW;
 
 	if (trf->quirks & TRF7970A_QUIRK_IRQ_STATUS_READ) {
@@ -544,6 +554,7 @@ static int trf7970a_read_irqstatus(struct trf7970a *trf, u8 *status)
 		ret = spi_write_then_read(trf->spi, &addr, 1, buf, 1);
 	}
 
+	gpio_set_value(trf->ss_gpio, 1);
 	if (ret)
 		dev_err(trf->dev, "%s - irqstatus: Status read failed: %d\n",
 				__func__, ret);
@@ -559,6 +570,7 @@ static int trf7970a_read_target_proto(struct trf7970a *trf, u8 *target_proto)
 	u8 buf[2];
 	u8 addr;
 
+	gpio_set_value(trf->ss_gpio, 0);
 	addr = TRF79070A_NFC_TARGET_PROTOCOL | TRF7970A_CMD_BIT_RW |
 		TRF7970A_CMD_BIT_CONTINUOUS;
 
@@ -569,6 +581,7 @@ static int trf7970a_read_target_proto(struct trf7970a *trf, u8 *target_proto)
 	else
 		*target_proto = buf[0];
 
+	gpio_set_value(trf->ss_gpio, 1);
 	return ret;
 }
 
@@ -663,6 +676,7 @@ static int trf7970a_transmit(struct trf7970a *trf, struct sk_buff *skb,
 	print_hex_dump_debug("trf7970a tx data: ", DUMP_PREFIX_NONE,
 			16, 1, skb->data, len, false);
 
+	gpio_set_value(trf->ss_gpio, 0);
 	spi_message_init(&m);
 
 	memset(&t, 0, sizeof(t));
@@ -679,7 +693,7 @@ static int trf7970a_transmit(struct trf7970a *trf, struct sk_buff *skb,
 	if (ret) {
 		dev_err(trf->dev, "%s - Can't send tx data: %d\n", __func__,
 				ret);
-		return ret;
+		goto out_err;
 	}
 
 	skb_pull(skb, len);
@@ -706,7 +720,9 @@ static int trf7970a_transmit(struct trf7970a *trf, struct sk_buff *skb,
 
 	schedule_delayed_work(&trf->timeout_work, msecs_to_jiffies(timeout));
 
-	return 0;
+out_err:
+	gpio_set_value(trf->ss_gpio, 1);
+	return ret;
 }
 
 static void trf7970a_fill_fifo(struct trf7970a *trf)
@@ -2039,6 +2055,19 @@ static int trf7970a_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	trf->ss_gpio = of_get_named_gpio(np, "ti,ss-gpio", 0);
+	if (!gpio_is_valid(trf->ss_gpio)) {
+		dev_err(trf->dev, "No SS GPIO property\n");
+		return trf->ss_gpio;
+	}
+
+	ret = devm_gpio_request_one(trf->dev, trf->ss_gpio,
+			GPIOF_DIR_OUT | GPIOF_INIT_HIGH, "trf7970a SS");
+	if (ret) {
+		dev_err(trf->dev, "Can't request SS GPIO: %d\n", ret);
+		return ret;
+	}
+
 	if (of_property_read_bool(np, "en2-rf-quirk"))
 		trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
 
-- 
1.9.1

[PATCH 2/4] NFC: trf7970a: add TI recommended write of zero to Register 0x18

[Geoff Lansberry]

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 2c3530a..447b6c9 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1064,6 +1064,10 @@ static int trf7970a_init(struct trf7970a *trf)
 	if (ret)
 		goto err_out;
 
+	ret = trf7970a_write(trf, TRF7970A_NFC_TARGET_LEVEL, 0);
+	if (ret)
+		goto err_out;
+
 	usleep_range(1000, 2000);
 
 	trf->chip_status_ctrl &= ~TRF7970A_CHIP_STATUS_RF_ON;
-- 
1.9.1

[PATCH 3/4] NFC: trf7970a: add device tree option for 27MHz clock

[Geoff Lansberry]

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

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 09c5056..bf25f39 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -22,6 +22,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.
+- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
 
 Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 
@@ -45,6 +46,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 		irq-status-read-quirk;
 		en2-rf-quirk;
 		t5t-rmb-extra-byte-quirk;
+		crystal_27mhz;
 		status = "okay";
 	};
 };
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 447b6c9..74210f9 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1072,12 +1072,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);
@@ -1194,30 +1193,32 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
 
 	dev_dbg(trf->dev, "rf technology: %d\n", tech);
 
+	trf->modulator_sys_clk_ctrl = (trf->modulator_sys_clk_ctrl&0xF8);
+
 	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 |= 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 |= 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 |= 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 |= 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 |= TRF7970A_MODULATOR_DEPTH_OOK;
 		trf->guard_time = TRF7970A_GUARD_TIME_15693;
 		break;
 	default:
@@ -1582,22 +1583,24 @@ static int trf7970a_tg_config_rf_tech(struct trf7970a *trf, int tech)
 
 	dev_dbg(trf->dev, "rf technology: %d\n", tech);
 
+	trf->modulator_sys_clk_ctrl = (trf->modulator_sys_clk_ctrl&0xF8);
+
 	switch (tech) {
 	case NFC_DIGITAL_RF_TECH_106A:
 		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 |= 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 |= 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 |= TRF7970A_MODULATOR_DEPTH_ASK10;
 		break;
 	default:
 		dev_dbg(trf->dev, "Unsupported rf technology: %d\n", tech);
@@ -2072,6 +2075,9 @@ static int trf7970a_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	if (of_property_read_bool(np, "crystal_27MHz"))
+		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_27MHZ;
+
 	if (of_property_read_bool(np, "en2-rf-quirk"))
 		trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
 
-- 
1.9.1

[PATCH 4/4] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

[Geoff Lansberry]

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

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index bf25f39..e605ebd 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -19,9 +19,13 @@ Optional SoC Specific Properties:
   "IRQ Status Read" erratum.
 - en2-rf-quirk: Specify that the trf7970a being used has the "EN2 RF"
   erratum.
+<<<<<<< HEAD
 - 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_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V
+>>>>>>> e7ea4dd... NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
 - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
 
 Example (for ARM-based BeagleBone with TRF7970A on SPI1):
@@ -45,7 +49,11 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 		autosuspend-delay = <30000>;
 		irq-status-read-quirk;
 		en2-rf-quirk;
+<<<<<<< HEAD
 		t5t-rmb-extra-byte-quirk;
+=======
+		vdd_io_1v8;
+>>>>>>> e7ea4dd... NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
 		crystal_27mhz;
 		status = "okay";
 	};
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 74210f9..be56897 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -441,6 +441,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;
@@ -1064,6 +1065,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;
@@ -1768,7 +1774,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;
 
@@ -2075,6 +2081,9 @@ static int trf7970a_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	if (of_property_read_bool(np, "vdd_io_1v8"))
+		trf->io_ctrl = TRF7970A_REG_IO_CTRL_IO_LOW;
+
 	if (of_property_read_bool(np, "crystal_27MHz"))
 		trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_27MHZ;
 
-- 
1.9.1

Re: [PATCH 1/4] NFC: trf7970a: Add support for gpio as SS

[Julian Calaby]

Hi Geoff,

On Tue, Apr 19, 2016 at 5:48 AM, Geoff Lansberry <geoff@kuvee.com> wrote:
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>

Can't you just use the cs-gpios property? This should require no driver changes.

This is described here:

https://www.kernel.org/doc/Documentation/devicetree/bindings/spi/spi-bus.txt

Thanks,

Julian Calaby


> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 ++
>  drivers/nfc/trf7970a.c                             | 33 ++++++++++++++++++++--
>  2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 32b35a0..09c5056 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -7,6 +7,7 @@ Required properties:
>  - interrupts: A single interrupt specifier.
>  - ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the
>    TRF7970A.
> +- ti,ss-gpio: GPIO entry used for active low SS (spi slave select) on the TRF7970A
>  - vin-supply: Regulator for supply voltage to VIN pin
>
>  Optional SoC Specific Properties:
> @@ -37,6 +38,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>                 interrupts = <14 0>;
>                 ti,enable-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>,
>                                   <&gpio2 5 GPIO_ACTIVE_LOW>;
> +               ti,ss-gpio = <&gpio2 4 GPIO_ACTIVE_HIGH>;
>                 vin-supply = <&ldo3_reg>;
>                 vin-voltage-override = <5000000>;
>                 autosuspend-delay = <30000>;
> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 10842b7..2c3530a 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c
> @@ -450,6 +450,7 @@ struct trf7970a {
>         bool                            adjust_resp_len;
>         int                             en2_gpio;
>         int                             en_gpio;
> +       int                             ss_gpio;
>         struct mutex                    lock;
>         unsigned int                    timeout;
>         bool                            ignore_timeout;
> @@ -462,9 +463,11 @@ static int trf7970a_cmd(struct trf7970a *trf, u8 opcode)
>         u8 cmd = TRF7970A_CMD_BIT_CTRL | TRF7970A_CMD_BIT_OPCODE(opcode);
>         int ret;
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         dev_dbg(trf->dev, "cmd: 0x%x\n", cmd);
>
>         ret = spi_write(trf->spi, &cmd, 1);
> +       gpio_set_value(trf->ss_gpio, 1);
>         if (ret)
>                 dev_err(trf->dev, "%s - cmd: 0x%x, ret: %d\n", __func__, cmd,
>                                 ret);
> @@ -476,7 +479,9 @@ static int trf7970a_read(struct trf7970a *trf, u8 reg, u8 *val)
>         u8 addr = TRF7970A_CMD_BIT_RW | reg;
>         int ret;
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         ret = spi_write_then_read(trf->spi, &addr, 1, val, 1);
> +       gpio_set_value(trf->ss_gpio, 1);
>         if (ret)
>                 dev_err(trf->dev, "%s - addr: 0x%x, ret: %d\n", __func__, addr,
>                                 ret);
> @@ -493,6 +498,7 @@ static int trf7970a_read_cont(struct trf7970a *trf, u8 reg, u8 *buf, size_t len)
>         struct spi_message m;
>         int ret;
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         dev_dbg(trf->dev, "read_cont(0x%x, %zd)\n", addr, len);
>
>         spi_message_init(&m);
> @@ -508,6 +514,7 @@ static int trf7970a_read_cont(struct trf7970a *trf, u8 reg, u8 *buf, size_t len)
>         spi_message_add_tail(&t[1], &m);
>
>         ret = spi_sync(trf->spi, &m);
> +       gpio_set_value(trf->ss_gpio, 1);
>         if (ret)
>                 dev_err(trf->dev, "%s - addr: 0x%x, ret: %d\n", __func__, addr,
>                                 ret);
> @@ -519,9 +526,11 @@ static int trf7970a_write(struct trf7970a *trf, u8 reg, u8 val)
>         u8 buf[2] = { reg, val };
>         int ret;
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         dev_dbg(trf->dev, "write(0x%x): 0x%x\n", reg, val);
>
>         ret = spi_write(trf->spi, buf, 2);
> +       gpio_set_value(trf->ss_gpio, 1);
>         if (ret)
>                 dev_err(trf->dev, "%s - write: 0x%x 0x%x, ret: %d\n", __func__,
>                                 buf[0], buf[1], ret);
> @@ -535,6 +544,7 @@ static int trf7970a_read_irqstatus(struct trf7970a *trf, u8 *status)
>         u8 buf[2];
>         u8 addr;
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         addr = TRF7970A_IRQ_STATUS | TRF7970A_CMD_BIT_RW;
>
>         if (trf->quirks & TRF7970A_QUIRK_IRQ_STATUS_READ) {
> @@ -544,6 +554,7 @@ static int trf7970a_read_irqstatus(struct trf7970a *trf, u8 *status)
>                 ret = spi_write_then_read(trf->spi, &addr, 1, buf, 1);
>         }
>
> +       gpio_set_value(trf->ss_gpio, 1);
>         if (ret)
>                 dev_err(trf->dev, "%s - irqstatus: Status read failed: %d\n",
>                                 __func__, ret);
> @@ -559,6 +570,7 @@ static int trf7970a_read_target_proto(struct trf7970a *trf, u8 *target_proto)
>         u8 buf[2];
>         u8 addr;
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         addr = TRF79070A_NFC_TARGET_PROTOCOL | TRF7970A_CMD_BIT_RW |
>                 TRF7970A_CMD_BIT_CONTINUOUS;
>
> @@ -569,6 +581,7 @@ static int trf7970a_read_target_proto(struct trf7970a *trf, u8 *target_proto)
>         else
>                 *target_proto = buf[0];
>
> +       gpio_set_value(trf->ss_gpio, 1);
>         return ret;
>  }
>
> @@ -663,6 +676,7 @@ static int trf7970a_transmit(struct trf7970a *trf, struct sk_buff *skb,
>         print_hex_dump_debug("trf7970a tx data: ", DUMP_PREFIX_NONE,
>                         16, 1, skb->data, len, false);
>
> +       gpio_set_value(trf->ss_gpio, 0);
>         spi_message_init(&m);
>
>         memset(&t, 0, sizeof(t));
> @@ -679,7 +693,7 @@ static int trf7970a_transmit(struct trf7970a *trf, struct sk_buff *skb,
>         if (ret) {
>                 dev_err(trf->dev, "%s - Can't send tx data: %d\n", __func__,
>                                 ret);
> -               return ret;
> +               goto out_err;
>         }
>
>         skb_pull(skb, len);
> @@ -706,7 +720,9 @@ static int trf7970a_transmit(struct trf7970a *trf, struct sk_buff *skb,
>
>         schedule_delayed_work(&trf->timeout_work, msecs_to_jiffies(timeout));
>
> -       return 0;
> +out_err:
> +       gpio_set_value(trf->ss_gpio, 1);
> +       return ret;
>  }
>
>  static void trf7970a_fill_fifo(struct trf7970a *trf)
> @@ -2039,6 +2055,19 @@ static int trf7970a_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> +       trf->ss_gpio = of_get_named_gpio(np, "ti,ss-gpio", 0);
> +       if (!gpio_is_valid(trf->ss_gpio)) {
> +               dev_err(trf->dev, "No SS GPIO property\n");
> +               return trf->ss_gpio;
> +       }
> +
> +       ret = devm_gpio_request_one(trf->dev, trf->ss_gpio,
> +                       GPIOF_DIR_OUT | GPIOF_INIT_HIGH, "trf7970a SS");
> +       if (ret) {
> +               dev_err(trf->dev, "Can't request SS GPIO: %d\n", ret);
> +               return ret;
> +       }
> +
>         if (of_property_read_bool(np, "en2-rf-quirk"))
>                 trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 3/4] NFC: trf7970a: add device tree option for 27MHz clock

[Julian Calaby]

Hi Geoff,

On Tue, Apr 19, 2016 at 5:48 AM, Geoff Lansberry <geoff@kuvee.com> wrote:
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>

You should add the description you had in your cover letter to the patches also.

> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  2 ++
>  drivers/nfc/trf7970a.c                             | 28 +++++++++++++---------
>  2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 09c5056..bf25f39 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -22,6 +22,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.
> +- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz

Does the trf7970a only operate at the default or 27.12MHz, or can it
operate at any frequency? If it's the latter, would it make sense to
specify this using the clock framework and some fixed-factor clock?

Thanks,

Julian Calaby


>  Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>
> @@ -45,6 +46,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>                 irq-status-read-quirk;
>                 en2-rf-quirk;
>                 t5t-rmb-extra-byte-quirk;
> +               crystal_27mhz;
>                 status = "okay";
>         };
>  };
> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 447b6c9..74210f9 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c
> @@ -1072,12 +1072,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);
> @@ -1194,30 +1193,32 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
>
>         dev_dbg(trf->dev, "rf technology: %d\n", tech);
>
> +       trf->modulator_sys_clk_ctrl = (trf->modulator_sys_clk_ctrl&0xF8);
> +
>         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 |= 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 |= 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 |= 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 |= 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 |= TRF7970A_MODULATOR_DEPTH_OOK;
>                 trf->guard_time = TRF7970A_GUARD_TIME_15693;
>                 break;
>         default:
> @@ -1582,22 +1583,24 @@ static int trf7970a_tg_config_rf_tech(struct trf7970a *trf, int tech)
>
>         dev_dbg(trf->dev, "rf technology: %d\n", tech);
>
> +       trf->modulator_sys_clk_ctrl = (trf->modulator_sys_clk_ctrl&0xF8);
> +
>         switch (tech) {
>         case NFC_DIGITAL_RF_TECH_106A:
>                 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 |= 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 |= 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 |= TRF7970A_MODULATOR_DEPTH_ASK10;
>                 break;
>         default:
>                 dev_dbg(trf->dev, "Unsupported rf technology: %d\n", tech);
> @@ -2072,6 +2075,9 @@ static int trf7970a_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> +       if (of_property_read_bool(np, "crystal_27MHz"))
> +               trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_27MHZ;
> +
>         if (of_property_read_bool(np, "en2-rf-quirk"))
>                 trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 4/4] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

[Julian Calaby]

Hi Geoff,

On Tue, Apr 19, 2016 at 5:48 AM, Geoff Lansberry <geoff@kuvee.com> wrote:
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
>  Documentation/devicetree/bindings/net/nfc/trf7970a.txt |  8 ++++++++
>  drivers/nfc/trf7970a.c                                 | 11 ++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index bf25f39..e605ebd 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -19,9 +19,13 @@ Optional SoC Specific Properties:
>    "IRQ Status Read" erratum.
>  - en2-rf-quirk: Specify that the trf7970a being used has the "EN2 RF"
>    erratum.
> +<<<<<<< HEAD
>  - 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_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V
> +>>>>>>> e7ea4dd... NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

Your patch is mangled. Please fix the merge errors before submitting.

Thanks,

Julian Calaby


>  - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>
>  Example (for ARM-based BeagleBone with TRF7970A on SPI1):
> @@ -45,7 +49,11 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>                 autosuspend-delay = <30000>;
>                 irq-status-read-quirk;
>                 en2-rf-quirk;
> +<<<<<<< HEAD
>                 t5t-rmb-extra-byte-quirk;
> +=======
> +               vdd_io_1v8;
> +>>>>>>> e7ea4dd... NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
>                 crystal_27mhz;
>                 status = "okay";
>         };
> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 74210f9..be56897 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c
> @@ -441,6 +441,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;
> @@ -1064,6 +1065,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;
> @@ -1768,7 +1774,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;
>
> @@ -2075,6 +2081,9 @@ static int trf7970a_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> +       if (of_property_read_bool(np, "vdd_io_1v8"))
> +               trf->io_ctrl = TRF7970A_REG_IO_CTRL_IO_LOW;
> +
>         if (of_property_read_bool(np, "crystal_27MHz"))
>                 trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_27MHZ;
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 1/4] NFC: trf7970a: Add support for gpio as SS

[Geoffrey Lansberry]

As Julian suggests, this is handled by the cs-gpios property, so we will 
drop this patch, but keep the other three active.

Thanks for the suggestion.
>
> On Mon, Apr 18, 2016 at 8:07 PM, Julian Calaby 
> <julian.calaby@gmail.com <mailto:julian.calaby@gmail.com>> wrote:
>
>     Hi Geoff,
>
>     On Tue, Apr 19, 2016 at 5:48 AM, Geoff Lansberry <geoff@kuvee.com
>     <mailto:geoff@kuvee.com>> wrote:
>     > Signed-off-by: Geoff Lansberry <geoff@kuvee.com
>     <mailto:geoff@kuvee.com>>
>
>     Can't you just use the cs-gpios property? This should require no
>     driver changes.
>
>     This is described here:
>
>     https://www.kernel.org/doc/Documentation/devicetree/bindings/spi/spi-bus.txt
>
>     Thanks,
>
>     Julian Calaby
>
>
>     > ---
>     >  .../devicetree/bindings/net/nfc/trf7970a.txt  |  2 ++
>     >  drivers/nfc/trf7970a.c  | 33 ++++++++++++++++++++--
>     >  2 files changed, 33 insertions(+), 2 deletions(-)
>     >
>     > diff --git
>     a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>     b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>     > index 32b35a0..09c5056 100644
>     > --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>     > +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>     > @@ -7,6 +7,7 @@ Required properties:
>     >  - interrupts: A single interrupt specifier.
>     >  - ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2'
>     pins on the
>     >    TRF7970A.
>     > +- ti,ss-gpio: GPIO entry used for active low SS (spi slave
>     select) on the TRF7970A
>     >  - vin-supply: Regulator for supply voltage to VIN pin
>     >
>     >  Optional SoC Specific Properties:
>     > @@ -37,6 +38,7 @@ Example (for ARM-based BeagleBone with
>     TRF7970A on SPI1):
>     >                 interrupts = <14 0>;
>     >                 ti,enable-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>,
>     >                                   <&gpio2 5 GPIO_ACTIVE_LOW>;
>     > +               ti,ss-gpio = <&gpio2 4 GPIO_ACTIVE_HIGH>;
>     >                 vin-supply = <&ldo3_reg>;
>     >                 vin-voltage-override = <5000000>;
>     >                 autosuspend-delay = <30000>;
>     > diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
>     > index 10842b7..2c3530a 100644
>     > --- a/drivers/nfc/trf7970a.c
>     > +++ b/drivers/nfc/trf7970a.c
>     > @@ -450,6 +450,7 @@ struct trf7970a {
>     >         bool adjust_resp_len;
>     >         int                             en2_gpio;
>     >         int                             en_gpio;
>     > +       int                             ss_gpio;
>     >         struct mutex                    lock;
>     >         unsigned int                    timeout;
>     >         bool ignore_timeout;
>     > @@ -462,9 +463,11 @@ static int trf7970a_cmd(struct trf7970a
>     *trf, u8 opcode)
>     >         u8 cmd = TRF7970A_CMD_BIT_CTRL |
>     TRF7970A_CMD_BIT_OPCODE(opcode);
>     >         int ret;
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         dev_dbg(trf->dev, "cmd: 0x%x\n", cmd);
>     >
>     >         ret = spi_write(trf->spi, &cmd, 1);
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     >         if (ret)
>     >                 dev_err(trf->dev, "%s - cmd: 0x%x, ret: %d\n",
>     __func__, cmd,
>     >                                 ret);
>     > @@ -476,7 +479,9 @@ static int trf7970a_read(struct trf7970a
>     *trf, u8 reg, u8 *val)
>     >         u8 addr = TRF7970A_CMD_BIT_RW | reg;
>     >         int ret;
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         ret = spi_write_then_read(trf->spi, &addr, 1, val, 1);
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     >         if (ret)
>     >                 dev_err(trf->dev, "%s - addr: 0x%x, ret: %d\n",
>     __func__, addr,
>     >                                 ret);
>     > @@ -493,6 +498,7 @@ static int trf7970a_read_cont(struct
>     trf7970a *trf, u8 reg, u8 *buf, size_t len)
>     >         struct spi_message m;
>     >         int ret;
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         dev_dbg(trf->dev, "read_cont(0x%x, %zd)\n", addr, len);
>     >
>     >         spi_message_init(&m);
>     > @@ -508,6 +514,7 @@ static int trf7970a_read_cont(struct
>     trf7970a *trf, u8 reg, u8 *buf, size_t len)
>     >         spi_message_add_tail(&t[1], &m);
>     >
>     >         ret = spi_sync(trf->spi, &m);
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     >         if (ret)
>     >                 dev_err(trf->dev, "%s - addr: 0x%x, ret: %d\n",
>     __func__, addr,
>     >                                 ret);
>     > @@ -519,9 +526,11 @@ static int trf7970a_write(struct trf7970a
>     *trf, u8 reg, u8 val)
>     >         u8 buf[2] = { reg, val };
>     >         int ret;
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         dev_dbg(trf->dev, "write(0x%x): 0x%x\n", reg, val);
>     >
>     >         ret = spi_write(trf->spi, buf, 2);
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     >         if (ret)
>     >                 dev_err(trf->dev, "%s - write: 0x%x 0x%x, ret:
>     %d\n", __func__,
>     >                                 buf[0], buf[1], ret);
>     > @@ -535,6 +544,7 @@ static int trf7970a_read_irqstatus(struct
>     trf7970a *trf, u8 *status)
>     >         u8 buf[2];
>     >         u8 addr;
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         addr = TRF7970A_IRQ_STATUS | TRF7970A_CMD_BIT_RW;
>     >
>     >         if (trf->quirks & TRF7970A_QUIRK_IRQ_STATUS_READ) {
>     > @@ -544,6 +554,7 @@ static int trf7970a_read_irqstatus(struct
>     trf7970a *trf, u8 *status)
>     >                 ret = spi_write_then_read(trf->spi, &addr, 1,
>     buf, 1);
>     >         }
>     >
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     >         if (ret)
>     >                 dev_err(trf->dev, "%s - irqstatus: Status read
>     failed: %d\n",
>     >                                 __func__, ret);
>     > @@ -559,6 +570,7 @@ static int trf7970a_read_target_proto(struct
>     trf7970a *trf, u8 *target_proto)
>     >         u8 buf[2];
>     >         u8 addr;
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         addr = TRF79070A_NFC_TARGET_PROTOCOL | TRF7970A_CMD_BIT_RW |
>     >                 TRF7970A_CMD_BIT_CONTINUOUS;
>     >
>     > @@ -569,6 +581,7 @@ static int trf7970a_read_target_proto(struct
>     trf7970a *trf, u8 *target_proto)
>     >         else
>     >                 *target_proto = buf[0];
>     >
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     >         return ret;
>     >  }
>     >
>     > @@ -663,6 +676,7 @@ static int trf7970a_transmit(struct trf7970a
>     *trf, struct sk_buff *skb,
>     >         print_hex_dump_debug("trf7970a tx data: ", DUMP_PREFIX_NONE,
>     >                         16, 1, skb->data, len, false);
>     >
>     > +       gpio_set_value(trf->ss_gpio, 0);
>     >         spi_message_init(&m);
>     >
>     >         memset(&t, 0, sizeof(t));
>     > @@ -679,7 +693,7 @@ static int trf7970a_transmit(struct trf7970a
>     *trf, struct sk_buff *skb,
>     >         if (ret) {
>     >                 dev_err(trf->dev, "%s - Can't send tx data:
>     %d\n", __func__,
>     >                                 ret);
>     > -               return ret;
>     > +               goto out_err;
>     >         }
>     >
>     >         skb_pull(skb, len);
>     > @@ -706,7 +720,9 @@ static int trf7970a_transmit(struct trf7970a
>     *trf, struct sk_buff *skb,
>     >
>     >  schedule_delayed_work(&trf->timeout_work,
>     msecs_to_jiffies(timeout));
>     >
>     > -       return 0;
>     > +out_err:
>     > +       gpio_set_value(trf->ss_gpio, 1);
>     > +       return ret;
>     >  }
>     >
>     >  static void trf7970a_fill_fifo(struct trf7970a *trf)
>     > @@ -2039,6 +2055,19 @@ static int trf7970a_probe(struct
>     spi_device *spi)
>     >                 return ret;
>     >         }
>     >
>     > +       trf->ss_gpio = of_get_named_gpio(np, "ti,ss-gpio", 0);
>     > +       if (!gpio_is_valid(trf->ss_gpio)) {
>     > +               dev_err(trf->dev, "No SS GPIO property\n");
>     > +               return trf->ss_gpio;
>     > +       }
>     > +
>     > +       ret = devm_gpio_request_one(trf->dev, trf->ss_gpio,
>     > +                       GPIOF_DIR_OUT | GPIOF_INIT_HIGH,
>     "trf7970a SS");
>     > +       if (ret) {
>     > +               dev_err(trf->dev, "Can't request SS GPIO: %d\n",
>     ret);
>     > +               return ret;
>     > +       }
>     > +
>     >         if (of_property_read_bool(np, "en2-rf-quirk"))
>     >                 trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
>     >
>     > --
>     > 1.9.1
>     >
>     > --
>     > To unsubscribe from this list: send the line "unsubscribe
>     linux-wireless" in
>     > the body of a message to majordomo@vger.kernel.org
>     <mailto:majordomo@vger.kernel.org>
>     > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
>     --
>     Julian Calaby
>
>     Email: julian.calaby@gmail.com <mailto:julian.calaby@gmail.com>
>     Profile: http://www.google.com/profiles/julian.calaby/
>
>
>
>
> -- 
> 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
> <http://www.kuvee.com>
>

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Mon, Apr 18, 2016 at 03:48:37PM -0400, Geoff Lansberry wrote:

Hi Geoff.

> The current version of the trf7970a driver is missing support for several features that we needed to operate a custom board. 
> We feel that these features will be useful to others as well, and we want to share them.
> 
> 1: Support for using a gpio as Slave-Select. Our processor has several devices on the spi bus, and we ran out of ss lines.  This patch gives TRF7970A the ability to
>  drive the ss line of the chip from a gpio that is defined in the device tree.
> 
> 2. When reviewing problems we were having in our implementation with TI support staff, they recommended that during initialization, address 0x18 should be written to zero.  This patch adds that change
> 
> 3. This existing version of the driver assumes that the crystal driving the trf7970a is 13.56 MHz, because there are several places in the driver code where the rel
> ated register is re-written, there is clean way to change to 27.12 MHz.  This patch adds a device tree option for 27 MHz and properly or's in changes in locations w
> here the register is changed.
> 
> 4. the existing version of the driver assumes that 3.3 volt io is used. The trf7970a has a special register where you can configure it for 1.8 volt io.  This patch
> adds a device tree option to select that this setting should be made.
> 
>  [PATCH 1/4] NFC: trf7970a: Add support for gpio as SS
>  [PATCH 2/4] NFC: trf7970a: add TI recommended write of zero to
>  [PATCH 3/4] NFC: trf7970a: add device tree option for 27MHz clock
>  [PATCH 4/4] NFC: trf7970a: Add device tree option of 1.8 Volt IO

I'm on vacation this week but will be back next week.  I'll take a
look once I'm back.

In the meantime, for emails sent to public (techie) list always keep
the lines less than 80 characters and always bottom-post (i.e., put
your text *underneath* the text that you are responding to).  Also,
when you change one or more patches in a series, re-submit the entire
series with the version number incremented (.e.g., v2, v3, ...) even
when you change only one of them.  It is a easier for others to know
what the latest versions are that way.

Thanks,

Mark
--

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Thu, Apr 21, 2016 at 05:01:19PM -0700, Mark Greer wrote:
> On Mon, Apr 18, 2016 at 03:48:37PM -0400, Geoff Lansberry wrote:
> 
> Hi Geoff.
> 
> > The current version of the trf7970a driver is missing support for several features that we needed to operate a custom board. 
> > We feel that these features will be useful to others as well, and we want to share them.
> > 
> > 1: Support for using a gpio as Slave-Select. Our processor has several devices on the spi bus, and we ran out of ss lines.  This patch gives TRF7970A the ability to
> >  drive the ss line of the chip from a gpio that is defined in the device tree.
> > 
> > 2. When reviewing problems we were having in our implementation with TI support staff, they recommended that during initialization, address 0x18 should be written to zero.  This patch adds that change
> > 
> > 3. This existing version of the driver assumes that the crystal driving the trf7970a is 13.56 MHz, because there are several places in the driver code where the rel
> > ated register is re-written, there is clean way to change to 27.12 MHz.  This patch adds a device tree option for 27 MHz and properly or's in changes in locations w
> > here the register is changed.
> > 
> > 4. the existing version of the driver assumes that 3.3 volt io is used. The trf7970a has a special register where you can configure it for 1.8 volt io.  This patch
> > adds a device tree option to select that this setting should be made.
> > 
> >  [PATCH 1/4] NFC: trf7970a: Add support for gpio as SS
> >  [PATCH 2/4] NFC: trf7970a: add TI recommended write of zero to
> >  [PATCH 3/4] NFC: trf7970a: add device tree option for 27MHz clock
> >  [PATCH 4/4] NFC: trf7970a: Add device tree option of 1.8 Volt IO
> 
> I'm on vacation this week but will be back next week.  I'll take a
> look once I'm back.
> 
> In the meantime, for emails sent to public (techie) list always keep
> the lines less than 80 characters and always bottom-post (i.e., put
> your text *underneath* the text that you are responding to).  Also,
> when you change one or more patches in a series, re-submit the entire
> series with the version number incremented (.e.g., v2, v3, ...) even
> when you change only one of them.  It is a easier for others to know
> what the latest versions are that way.

Hi Geoff.

I know its been a ridiculously long time since I said I would look at
your patches but I have time now.  Do you have an updated version of
your patch series?

Mark
--

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Tue, Dec 13, 2016 at 08:50:04PM -0500, Geoff Lansberry wrote:
> Hi Mark -  Thanks for getting back to me.   It's funny that you ask,
> because we are currently chasing a segfault that is happening in neard, but
> may end up back in the trf7970a driver.   Have you ever heard on anyone
> having segfault problems related to the trf7970a hardware drivers?

No.  Mind sharing more info on that segfault?

> I'll get you an update later tonight or tomorrow.

Okay, thanks.

Mark
--

Re: [Patch] NFC: trf7970a:

[Geoff Lansberry]

On Wed, Dec 14, 2016 at 10:57 AM, Mark Greer <mgreer@animalcreek.com> wrote:
>
> On Tue, Dec 13, 2016 at 08:50:04PM -0500, Geoff Lansberry wrote:
> > Hi Mark -  Thanks for getting back to me.   It's funny that you ask,
> > because we are currently chasing a segfault that is happening in neard, but
> > may end up back in the trf7970a driver.   Have you ever heard on anyone
> > having segfault problems related to the trf7970a hardware drivers?
>
> No.  Mind sharing more info on that segfault?
>
> > I'll get you an update later tonight or tomorrow.
>
> Okay, thanks.
>
> Mark
> --

Mark - The segfault issue is only happening on writing, The work on
the segfault is being done by a consultant, but here is his statement
on how to recreate it on our build:

I am able to reliably force neard to segfault by flooding it with
write requests. I have attached a python script called flood.py that
can be used to do this. The script uses utilities that ship with
neard.

The segfault does not appear deterministic. It usually happens within
1000 writes, but the time can varying greatly. The logs output from
neard are inconsistent between crashes, which suggests this may be a
timing or race condition related issue.

I have been running neard manually to obtain the log information and a
core file for debugging (attached). I run neard as,

  $ /usr/lib/neard/nfc/neard -d -n

In a separate terminal I run,

  $ python flood.py

And the resulting core file provides the following backtrace,

(gdb) bt
#0  0xb6caed64 in ?? ()
#1  0x0001ed7c in data_recv (resp=0x5bd90 "", length=17, data=0x58348)
at plugins/nfctype2.c:156
#2  0x00024ecc in execute_recv_cb (user_data=0x5bd88) at src/adapter.c:979
#3  0xb6e70d60 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb)

The line at nfctype2.c:156 contains a memcpy operation.


Below is the flood.py script:
#!/usr/bin/python

import neardutils
import dbus
import time

bus = dbus.SystemBus()
DURATION = 0.05


def write():
    # Get an adapter interface
    objects = neardutils.get_managed_objects()
    for path, interfaces in objects.iteritems():
        if "org.neard.Adapter" in interfaces:
            break

    else:
        raise Exception("Unable to find adapter")

    print("adapter object path: " + path)
    adapter = dbus.Interface(bus.get_object("org.neard", path),
"org.freedesktop.DBus.Properties")

    # power cycle
    try:
        adapter.Set("org.neard.Adapter", "Powered", dbus.Boolean(0))
        time.sleep(DURATION)
    except:
        pass

    try:
        adapter.Set("org.neard.Adapter", "Powered", dbus.Boolean(1))
        time.sleep(DURATION)
    except:
        pass

    # Set polling
    adapter = dbus.Interface(bus.get_object("org.neard", path),
"org.neard.Adapter")
    adapter.StartPollLoop("Initiator")

    time.sleep(DURATION)

    # write tag
    objects = neardutils.get_managed_objects()
    for path, interfaces in objects.iteritems():
        if "org.neard.Tag" in interfaces:
            break
    else:
        raise Exception("Unable to find tag")

    print("tag object path: " + path)

    time.sleep(DURATION)

    tag = dbus.Interface(bus.get_object("org.neard", path), "org.neard.Tag")
    tag.Write(({
        "Type": "Text",
        "Encoding": "UTF-8",
        "Language": "en",
        "Representation": "omen_red_2014",
        }))

    time.sleep(DURATION)

    objects = neardutils.get_managed_objects()
    for path, interfaces in objects.iteritems():
        if "org.neard.Record" in interfaces:
            break
    else:
        raise Exception("Unable to read record")

    print("record object path: " + path)

    time.sleep(DURATION)

    record = dbus.Interface(bus.get_object("org.neard", path),
"org.freedesktop.DBus.Properties")
    print("representation: " + record.Get("org.neard.Record", "Representation"))


def main():
    for iteration in range(1000):
        try:
            print("==================================================")
            print("iteration: " + str(iteration))
            write()
            print("SUCCESS")

        except Exception,e:
            print(str(e))
            print("FAILURE")


if __name__ == "__main__":
    main()
-----

If we find the source of the problem, we will share it upstream.   If
you have any thoughts on where to look, please share.

Geoff Lansberry

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Wed, Dec 14, 2016 at 11:17:33AM -0500, Geoff Lansberry wrote:
> On Wed, Dec 14, 2016 at 10:57 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> >
> > On Tue, Dec 13, 2016 at 08:50:04PM -0500, Geoff Lansberry wrote:
> > > Hi Mark -  Thanks for getting back to me.   It's funny that you ask,
> > > because we are currently chasing a segfault that is happening in neard, but
> > > may end up back in the trf7970a driver.   Have you ever heard on anyone
> > > having segfault problems related to the trf7970a hardware drivers?
> >
> > No.  Mind sharing more info on that segfault?
> >
> > > I'll get you an update later tonight or tomorrow.
> >
> > Okay, thanks.
> >
> > Mark
> > --
> 
> Mark - The segfault issue is only happening on writing, The work on
> the segfault is being done by a consultant, but here is his statement
> on how to recreate it on our build:
> 
> I am able to reliably force neard to segfault by flooding it with
> write requests. I have attached a python script called flood.py that
> can be used to do this. The script uses utilities that ship with
> neard.
> 
> The segfault does not appear deterministic. It usually happens within
> 1000 writes, but the time can varying greatly. The logs output from
> neard are inconsistent between crashes, which suggests this may be a
> timing or race condition related issue.
> 
> I have been running neard manually to obtain the log information and a
> core file for debugging (attached). I run neard as,
> 
>   $ /usr/lib/neard/nfc/neard -d -n
> 
> In a separate terminal I run,
> 
>   $ python flood.py
> 
> And the resulting core file provides the following backtrace,
> 
> (gdb) bt
> #0  0xb6caed64 in ?? ()
> #1  0x0001ed7c in data_recv (resp=0x5bd90 "", length=17, data=0x58348)
> at plugins/nfctype2.c:156
> #2  0x00024ecc in execute_recv_cb (user_data=0x5bd88) at src/adapter.c:979
> #3  0xb6e70d60 in ?? ()
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> (gdb)
> 
> The line at nfctype2.c:156 contains a memcpy operation.

Thanks Geoff.

What are the values of the arguments to memcpy()?

I will look at it later today/tomorrow but if you have another NFC device
to test with, it would help isolate whether it is neard or the trf7970a
driver.  The driver shouldn't be able to make neard crash like this but
who knows.

You could also try testing older versions of neard to see if they also
fail and if not, start bisecting from there.  Maybe test a different
tag type too.

Mark
--

Re: [Patch] NFC: trf7970a:

[Geoff Lansberry]

On Wed, Dec 14, 2016 at 12:10 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Wed, Dec 14, 2016 at 11:17:33AM -0500, Geoff Lansberry wrote:
>> On Wed, Dec 14, 2016 at 10:57 AM, Mark Greer <mgreer@animalcreek.com> wrote:
>> >
>> > On Tue, Dec 13, 2016 at 08:50:04PM -0500, Geoff Lansberry wrote:
>> > > Hi Mark -  Thanks for getting back to me.   It's funny that you ask,
>> > > because we are currently chasing a segfault that is happening in neard, but
>> > > may end up back in the trf7970a driver.   Have you ever heard on anyone
>> > > having segfault problems related to the trf7970a hardware drivers?
>> >
>> > No.  Mind sharing more info on that segfault?
>> >
>> > > I'll get you an update later tonight or tomorrow.
>> >
>> > Okay, thanks.
>> >
>> > Mark
>> > --
>>
>> Mark - The segfault issue is only happening on writing, The work on
>> the segfault is being done by a consultant, but here is his statement
>> on how to recreate it on our build:
>>
>> I am able to reliably force neard to segfault by flooding it with
>> write requests. I have attached a python script called flood.py that
>> can be used to do this. The script uses utilities that ship with
>> neard.
>>
>> The segfault does not appear deterministic. It usually happens within
>> 1000 writes, but the time can varying greatly. The logs output from
>> neard are inconsistent between crashes, which suggests this may be a
>> timing or race condition related issue.
>>
>> I have been running neard manually to obtain the log information and a
>> core file for debugging (attached). I run neard as,
>>
>>   $ /usr/lib/neard/nfc/neard -d -n
>>
>> In a separate terminal I run,
>>
>>   $ python flood.py
>>
>> And the resulting core file provides the following backtrace,
>>
>> (gdb) bt
>> #0  0xb6caed64 in ?? ()
>> #1  0x0001ed7c in data_recv (resp=0x5bd90 "", length=17, data=0x58348)
>> at plugins/nfctype2.c:156
>> #2  0x00024ecc in execute_recv_cb (user_data=0x5bd88) at src/adapter.c:979
>> #3  0xb6e70d60 in ?? ()
>> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>> (gdb)
>>
>> The line at nfctype2.c:156 contains a memcpy operation.
>
> Thanks Geoff.
>
> What are the values of the arguments to memcpy()?
>
> I will look at it later today/tomorrow but if you have another NFC device
> to test with, it would help isolate whether it is neard or the trf7970a
> driver.  The driver shouldn't be able to make neard crash like this but
> who knows.
>
> You could also try testing older versions of neard to see if they also
> fail and if not, start bisecting from there.  Maybe test a different
> tag type too.
>
> Mark
> --
Mark - We can't seem to get gdb to run on our board, so we can't see
the exact arguments.  Here is what our consultant has to say about
your question:


The backtrace seems to indicate that the error is occurring in neard,
not the driver.

Since the driver is built as a module, your kernel won't crash if
there is a problem in it, but you should be told that the error is
originating in the module.

It is also possible that the NFC driver does have a non-fatal problem
in it (such as returning unexpected data) that is propagating to neard
and causing the error there.


Of course, it is also worth noting:

Backtrace stopped: previous frame identical to this frame (corrupt stack?)

and the same address appearing twice -- what I would assume to be your
memcpy address, since that is the last call made on a given source
line. If the stack is corrupt, then the error could very well
originate in the driver and not neard.

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Wed, Dec 14, 2016 at 01:35:03PM -0500, Geoff Lansberry wrote:
> On Wed, Dec 14, 2016 at 12:10 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> > On Wed, Dec 14, 2016 at 11:17:33AM -0500, Geoff Lansberry wrote:
> >> On Wed, Dec 14, 2016 at 10:57 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> >> >
> >> > On Tue, Dec 13, 2016 at 08:50:04PM -0500, Geoff Lansberry wrote:
> >> > > Hi Mark -  Thanks for getting back to me.   It's funny that you ask,
> >> > > because we are currently chasing a segfault that is happening in neard, but
> >> > > may end up back in the trf7970a driver.   Have you ever heard on anyone
> >> > > having segfault problems related to the trf7970a hardware drivers?
> >> >
> >> > No.  Mind sharing more info on that segfault?
> >> >
> >> > > I'll get you an update later tonight or tomorrow.
> >> >
> >> > Okay, thanks.
> >> >
> >> > Mark
> >> > --
> >>
> >> Mark - The segfault issue is only happening on writing, The work on
> >> the segfault is being done by a consultant, but here is his statement
> >> on how to recreate it on our build:
> >>
> >> I am able to reliably force neard to segfault by flooding it with
> >> write requests. I have attached a python script called flood.py that
> >> can be used to do this. The script uses utilities that ship with
> >> neard.
> >>
> >> The segfault does not appear deterministic. It usually happens within
> >> 1000 writes, but the time can varying greatly. The logs output from
> >> neard are inconsistent between crashes, which suggests this may be a
> >> timing or race condition related issue.
> >>
> >> I have been running neard manually to obtain the log information and a
> >> core file for debugging (attached). I run neard as,
> >>
> >>   $ /usr/lib/neard/nfc/neard -d -n
> >>
> >> In a separate terminal I run,
> >>
> >>   $ python flood.py
> >>
> >> And the resulting core file provides the following backtrace,
> >>
> >> (gdb) bt
> >> #0  0xb6caed64 in ?? ()
> >> #1  0x0001ed7c in data_recv (resp=0x5bd90 "", length=17, data=0x58348)
> >> at plugins/nfctype2.c:156
> >> #2  0x00024ecc in execute_recv_cb (user_data=0x5bd88) at src/adapter.c:979
> >> #3  0xb6e70d60 in ?? ()
> >> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> >> (gdb)
> >>
> >> The line at nfctype2.c:156 contains a memcpy operation.
> >
> > Thanks Geoff.
> >
> > What are the values of the arguments to memcpy()?
> >
> > I will look at it later today/tomorrow but if you have another NFC device
> > to test with, it would help isolate whether it is neard or the trf7970a
> > driver.  The driver shouldn't be able to make neard crash like this but
> > who knows.
> >
> > You could also try testing older versions of neard to see if they also
> > fail and if not, start bisecting from there.  Maybe test a different
> > tag type too.
> >
> > Mark
> > --
> Mark - We can't seem to get gdb to run on our board, so we can't see
> the exact arguments.

:(

> Here is what our consultant has to say about
> your question:
> 
> 
> The backtrace seems to indicate that the error is occurring in neard,
> not the driver.

Yep.

> Since the driver is built as a module, your kernel won't crash if
> there is a problem in it,

Not true.  A driver driver can happily crash the kernel even when it
is dynamically loaded/linked.  I expect the fact that it is dynamicaly
loaded to be irrelevant to this issue.

> but you should be told that the error is
> originating in the module.
>
> It is also possible that the NFC driver does have a non-fatal problem
> in it (such as returning unexpected data) that is propagating to neard
> and causing the error there.

I agree, it is possible that the driver is returning bad data but that
still shouldn't crash neard.  So there is almost certainly one neard
issue and potentially more.  There could also be driver issues too,
of course.

> Of course, it is also worth noting:
> 
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> and the same address appearing twice -- what I would assume to be your
> memcpy address, since that is the last call made on a given source
> line. If the stack is corrupt, then the error could very well
> originate in the driver and not neard.

Lots of things are possible but that doesn't make them so.  Let's be
methodical and follow where the data takes us.  I'll start on this
tonight but won't likely get far until tomorrow.  In the meantime,
if you and/or your contractor make progress, please share.

Thanks,

Mark
--

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Wed, Dec 14, 2016 at 03:31:23PM -0700, Mark Greer wrote:
> I'll start on this
> tonight but won't likely get far until tomorrow.  In the meantime,
> if you and/or your contractor make progress, please share.

Geoff,

Which version of neard are you using?  0.16?

Mark
--

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Thu, Dec 15, 2016 at 09:52:10PM -0700, Mark Greer wrote:
> On Wed, Dec 14, 2016 at 03:31:23PM -0700, Mark Greer wrote:
> > I'll start on this
> > tonight but won't likely get far until tomorrow.  In the meantime,
> > if you and/or your contractor make progress, please share.
> 
> Geoff,
> 
> Which version of neard are you using?  0.16?

Also, the flood.py script doesn't work well at all for me.  At best,
it works successfully for one iteration and then fails continually for
all other iterations.  This is true when using the trf7970a and pn533
drivers.

I've tweaked it a but but still no success.  I haven't looked all that
closely at it but since you said you were persuing this, I'll wait to
hear more from you.

Mark
--

Re: [Patch] NFC: trf7970a:

[Geoff Lansberry]

Mark,   from our consultant:

It isn't important whether the flood script is successful in writing
or not. The point of it is to force a segfault by making many
requests. It needs to run for several hundred iterations (successful
or not) in order to generate the segfault.

Geoff
Geoff Lansberry


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



On Fri, Dec 16, 2016 at 3:35 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Thu, Dec 15, 2016 at 09:52:10PM -0700, Mark Greer wrote:
>> On Wed, Dec 14, 2016 at 03:31:23PM -0700, Mark Greer wrote:
>> > I'll start on this
>> > tonight but won't likely get far until tomorrow.  In the meantime,
>> > if you and/or your contractor make progress, please share.
>>
>> Geoff,
>>
>> Which version of neard are you using?  0.16?
>
> Also, the flood.py script doesn't work well at all for me.  At best,
> it works successfully for one iteration and then fails continually for
> all other iterations.  This is true when using the trf7970a and pn533
> drivers.
>
> I've tweaked it a but but still no success.  I haven't looked all that
> closely at it but since you said you were persuing this, I'll wait to
> hear more from you.
>
> Mark
> --

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> Mark,   from our consultant:
> 
> It isn't important whether the flood script is successful in writing
> or not. The point of it is to force a segfault by making many
> requests. It needs to run for several hundred iterations (successful
> or not) in order to generate the segfault.

So neard crashes even when the write fails??  Okay, I'll let it run for
a while tomorrow (Monday).

Thanks,

Mark
--

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Sun, Dec 18, 2016 at 08:07:33PM -0700, Mark Greer wrote:
> On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> > Mark,   from our consultant:
> > 
> > It isn't important whether the flood script is successful in writing
> > or not. The point of it is to force a segfault by making many
> > requests. It needs to run for several hundred iterations (successful
> > or not) in order to generate the segfault.
> 
> So neard crashes even when the write fails??  Okay, I'll let it run for
> a while tomorrow (Monday).

[Okay, so not exactly "tomorrow" but I did get back to this.]

Geoff, a few things:

1) Any update on these issues?

2) Do you have all of the NFC-related patches from the nfc-next master
branch?  In particular, do you have all of Thierry's patches to
net/nfc/digial_*.c dated around June-July 2016?  Without those patches,
I see a panic; with them, I don't.

3) Assuming you have all of those patches, please revert the one with the
summary line of, "NFC: digital: Set the command pending flag", and tell me
if that stops the "Bogus state" messages.  I don't know which repo/branch
you're using so I can't provide a commit id.

Thanks,

Mark
--

Re: [Patch] NFC: trf7970a:

[Mark Greer]

I just realized that the linux-nfc is not CC'd so adding it.

On Wed, Feb 08, 2017 at 03:53:39PM -0700, Mark Greer wrote:
> On Sun, Dec 18, 2016 at 08:07:33PM -0700, Mark Greer wrote:
> > On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> > > Mark,   from our consultant:
> > > 
> > > It isn't important whether the flood script is successful in writing
> > > or not. The point of it is to force a segfault by making many
> > > requests. It needs to run for several hundred iterations (successful
> > > or not) in order to generate the segfault.
> > 
> > So neard crashes even when the write fails??  Okay, I'll let it run for
> > a while tomorrow (Monday).
> 
> [Okay, so not exactly "tomorrow" but I did get back to this.]
> 
> Geoff, a few things:
> 
> 1) Any update on these issues?
> 
> 2) Do you have all of the NFC-related patches from the nfc-next master
> branch?  In particular, do you have all of Thierry's patches to
> net/nfc/digial_*.c dated around June-July 2016?  Without those patches,
> I see a panic; with them, I don't.
> 
> 3) Assuming you have all of those patches, please revert the one with the
> summary line of, "NFC: digital: Set the command pending flag", and tell me
> if that stops the "Bogus state" messages.  I don't know which repo/branch
> you're using so I can't provide a commit id.
> 
> Thanks,
> 
> Mark
> --

Re: [Patch] NFC: trf7970a:

[Geoff Lansberry]

On Wed, Feb 8, 2017 at 5:56 PM, Mark Greer <mgreer@animalcreek.com> wrote:
>
> I just realized that the linux-nfc is not CC'd so adding it.
>
> On Wed, Feb 08, 2017 at 03:53:39PM -0700, Mark Greer wrote:
> > On Sun, Dec 18, 2016 at 08:07:33PM -0700, Mark Greer wrote:
> > > On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> > > > Mark,   from our consultant:
> > > >
> > > > It isn't important whether the flood script is successful in writing
> > > > or not. The point of it is to force a segfault by making many
> > > > requests. It needs to run for several hundred iterations (successful
> > > > or not) in order to generate the segfault.
> > >
> > > So neard crashes even when the write fails??  Okay, I'll let it run for
> > > a while tomorrow (Monday).
> >
> > [Okay, so not exactly "tomorrow" but I did get back to this.]
> >
> > Geoff, a few things:
> >
> > 1) Any update on these issues?

Yes - I've discovered the primary trigger of my problems.  I'm using
some very small tags from Murata, and they don't have a strong signal.
Sometimes when writing, the tags can get corrupted data written to
them. When the tag is verified by re-reading it, neard bails out
because it finds non-utf8 characters.  So far I have found it
impossible to rewrite a tag once it has been mal-formed.

> >
> > 2) Do you have all of the NFC-related patches from the nfc-next master
> > branch?  In particular, do you have all of Thierry's patches to
> > net/nfc/digial_*.c dated around June-July 2016?  Without those patches,
> > I see a panic; with them, I don't.

No, we don't.  the last patch I've got in our kernel version is yours
from 7-21-2014.  I'm inquiring with more experienced people to see how
to address that.
> >
> > 3) Assuming you have all of those patches, please revert the one with the
> > summary line of, "NFC: digital: Set the command pending flag", and tell me
> > if that stops the "Bogus state" messages.  I don't know which repo/branch
> > you're using so I can't provide a commit id.

No to the patches, but I found the commit you are talking about and
will look at it.   Maybe possible to cherry pick it.
> >
> > Thanks,
> >
> > Mark
> > --

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Thu, Feb 09, 2017 at 10:54:08AM -0500, Geoff Lansberry wrote:
> On Wed, Feb 8, 2017 at 5:56 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> >
> > I just realized that the linux-nfc is not CC'd so adding it.
> >
> > On Wed, Feb 08, 2017 at 03:53:39PM -0700, Mark Greer wrote:
> > > On Sun, Dec 18, 2016 at 08:07:33PM -0700, Mark Greer wrote:
> > > > On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> > > > > Mark,   from our consultant:
> > > > >
> > > > > It isn't important whether the flood script is successful in writing
> > > > > or not. The point of it is to force a segfault by making many
> > > > > requests. It needs to run for several hundred iterations (successful
> > > > > or not) in order to generate the segfault.
> > > >
> > > > So neard crashes even when the write fails??  Okay, I'll let it run for
> > > > a while tomorrow (Monday).
> > >
> > > [Okay, so not exactly "tomorrow" but I did get back to this.]
> > >
> > > Geoff, a few things:
> > >
> > > 1) Any update on these issues?
> 
> Yes - I've discovered the primary trigger of my problems.  I'm using
> some very small tags from Murata, and they don't have a strong signal.
> Sometimes when writing, the tags can get corrupted data written to
> them. When the tag is verified by re-reading it, neard bails out
> because it finds non-utf8 characters.  So far I have found it
> impossible to rewrite a tag once it has been mal-formed.

Okay, that's interesting.  neard still shouldn't bail out so we'll
have to look at that.

> > > 2) Do you have all of the NFC-related patches from the nfc-next master
> > > branch?  In particular, do you have all of Thierry's patches to
> > > net/nfc/digial_*.c dated around June-July 2016?  Without those patches,
> > > I see a panic; with them, I don't.
> 
> No, we don't.  the last patch I've got in our kernel version is yours
> from 7-21-2014.  I'm inquiring with more experienced people to see how
> to address that.

Oh... Then you really need to update.  Look at the master branch in the
nfc-next repo (git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git)

Which kernel version are you using?  If its old, you should update to the
most recent version that you can.  If you *absolutely* can't use a newer
kernel, let me know as I may be able to help.

Also, be sure to use the latest version of neard (currently 0.16).

> > > 3) Assuming you have all of those patches, please revert the one with the
> > > summary line of, "NFC: digital: Set the command pending flag", and tell me
> > > if that stops the "Bogus state" messages.  I don't know which repo/branch
> > > you're using so I can't provide a commit id.
> 
> No to the patches, but I found the commit you are talking about and
> will look at it.   Maybe possible to cherry pick it.

No, don't cherry-pick it.  I was asking you to revert that commit.
Ignore this request for now because the commit is fine, it just seemed
to cover up the issue so I wanted to see if it did for you too.  I will
dig into it more once I finish setting up a new SDCard for my test system
(and get some time to look at it).

Mark
--

Re: [Patch] NFC: trf7970a:

[Geoff Lansberry]

On Thu, Feb 9, 2017 at 4:27 PM, Mark Greer <mgreer@animalcreek.com> wrote:
>
> On Thu, Feb 09, 2017 at 10:54:08AM -0500, Geoff Lansberry wrote:
> > On Wed, Feb 8, 2017 at 5:56 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> > >
> > > I just realized that the linux-nfc is not CC'd so adding it.
> > >
> > > On Wed, Feb 08, 2017 at 03:53:39PM -0700, Mark Greer wrote:
> > > > On Sun, Dec 18, 2016 at 08:07:33PM -0700, Mark Greer wrote:
> > > > > On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> > > > > > Mark,   from our consultant:
> > > > > >
> > > > > > It isn't important whether the flood script is successful in writing
> > > > > > or not. The point of it is to force a segfault by making many
> > > > > > requests. It needs to run for several hundred iterations (successful
> > > > > > or not) in order to generate the segfault.
> > > > >
> > > > > So neard crashes even when the write fails??  Okay, I'll let it run for
> > > > > a while tomorrow (Monday).
> > > >
> > > > [Okay, so not exactly "tomorrow" but I did get back to this.]
> > > >
> > > > Geoff, a few things:
> > > >
> > > > 1) Any update on these issues?
> >
> > Yes - I've discovered the primary trigger of my problems.  I'm using
> > some very small tags from Murata, and they don't have a strong signal.
> > Sometimes when writing, the tags can get corrupted data written to
> > them. When the tag is verified by re-reading it, neard bails out
> > because it finds non-utf8 characters.  So far I have found it
> > impossible to rewrite a tag once it has been mal-formed.
>
> Okay, that's interesting.  neard still shouldn't bail out so we'll
> have to look at that.

This is the most critical thing for us right now.  Can you make a
malformed tag and try it to see if you have the same issue?  What I
typically see is 0xFE character too early in the string, or similarly
the length being too long for the string.  If you can do that, then
try to write or read it via neard.  If you can confirm the quitting
behavior, that will help a lot.  I can live with the corruption,
because I can detect it and re-write if it happens.
>
> > > > 2) Do you have all of the NFC-related patches from the nfc-next master
> > > > branch?  In particular, do you have all of Thierry's patches to
> > > > net/nfc/digial_*.c dated around June-July 2016?  Without those patches,
> > > > I see a panic; with them, I don't.
> >
> > No, we don't.  the last patch I've got in our kernel version is yours
> > from 7-21-2014.  I'm inquiring with more experienced people to see how
> > to address that.
>
> Oh... Then you really need to update.  Look at the master branch in the
> nfc-next repo (git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git)

Those commits are in the latest ti build,
https://git.ti.com/ti-linux-kernel/ti-linux-kernel/commits/ti-linux-4.9.y,
 so I've built that up.  Unfortunately bricked the board and don't
have the dev tools to fix it with me.  Bummer, should be back into it
tomorrow.
>
> Which kernel version are you using?  If its old, you should update to the
> most recent version that you can.  If you *absolutely* can't use a newer
> kernel, let me know as I may be able to help.
>
> Also, be sure to use the latest version of neard (currently 0.16).
>
> > > > 3) Assuming you have all of those patches, please revert the one with the
> > > > summary line of, "NFC: digital: Set the command pending flag", and tell me
> > > > if that stops the "Bogus state" messages.  I don't know which repo/branch
> > > > you're using so I can't provide a commit id.
> >
> > No to the patches, but I found the commit you are talking about and
> > will look at it.   Maybe possible to cherry pick it.
>
> No, don't cherry-pick it.  I was asking you to revert that commit.
> Ignore this request for now because the commit is fine, it just seemed
> to cover up the issue so I wanted to see if it did for you too.  I will
> dig into it more once I finish setting up a new SDCard for my test system
> (and get some time to look at it).
>
> Mark
> --

Re: [Patch] NFC: trf7970a:

[Mark Greer]

On Thu, Feb 09, 2017 at 07:41:11PM -0500, Geoff Lansberry wrote:
> On Thu, Feb 9, 2017 at 4:27 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> >
> > On Thu, Feb 09, 2017 at 10:54:08AM -0500, Geoff Lansberry wrote:
> > > On Wed, Feb 8, 2017 at 5:56 PM, Mark Greer <mgreer@animalcreek.com> wrote:
> > > >
> > > > I just realized that the linux-nfc is not CC'd so adding it.
> > > >
> > > > On Wed, Feb 08, 2017 at 03:53:39PM -0700, Mark Greer wrote:
> > > > > On Sun, Dec 18, 2016 at 08:07:33PM -0700, Mark Greer wrote:
> > > > > > On Sat, Dec 17, 2016 at 04:19:04PM -0500, Geoff Lansberry wrote:
> > > > > > > Mark,   from our consultant:
> > > > > > >
> > > > > > > It isn't important whether the flood script is successful in writing
> > > > > > > or not. The point of it is to force a segfault by making many
> > > > > > > requests. It needs to run for several hundred iterations (successful
> > > > > > > or not) in order to generate the segfault.
> > > > > >
> > > > > > So neard crashes even when the write fails??  Okay, I'll let it run for
> > > > > > a while tomorrow (Monday).
> > > > >
> > > > > [Okay, so not exactly "tomorrow" but I did get back to this.]
> > > > >
> > > > > Geoff, a few things:
> > > > >
> > > > > 1) Any update on these issues?
> > >
> > > Yes - I've discovered the primary trigger of my problems.  I'm using
> > > some very small tags from Murata, and they don't have a strong signal.
> > > Sometimes when writing, the tags can get corrupted data written to
> > > them. When the tag is verified by re-reading it, neard bails out
> > > because it finds non-utf8 characters.  So far I have found it
> > > impossible to rewrite a tag once it has been mal-formed.
> >
> > Okay, that's interesting.  neard still shouldn't bail out so we'll
> > have to look at that.
> 
> This is the most critical thing for us right now.  Can you make a
> malformed tag and try it to see if you have the same issue?  What I
> typically see is 0xFE character too early in the string, or similarly
> the length being too long for the string.  If you can do that, then
> try to write or read it via neard.  If you can confirm the quitting
> behavior, that will help a lot.  I can live with the corruption,
> because I can detect it and re-write if it happens.

I don't have such a thing but I can always hack something to write bogus
data.  I may not get to this until next week, though.

> > > > > 2) Do you have all of the NFC-related patches from the nfc-next master
> > > > > branch?  In particular, do you have all of Thierry's patches to
> > > > > net/nfc/digial_*.c dated around June-July 2016?  Without those patches,
> > > > > I see a panic; with them, I don't.
> > >
> > > No, we don't.  the last patch I've got in our kernel version is yours
> > > from 7-21-2014.  I'm inquiring with more experienced people to see how
> > > to address that.
> >
> > Oh... Then you really need to update.  Look at the master branch in the
> > nfc-next repo (git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git)
> 
> Those commits are in the latest ti build,
> https://git.ti.com/ti-linux-kernel/ti-linux-kernel/commits/ti-linux-4.9.y,
>  so I've built that up.

Yes, v4.9.y has everything so that's a good one to work from.

> Unfortunately bricked the board and don't
> have the dev tools to fix it with me.  Bummer, should be back into it
> tomorrow.

:(