[PATCH 0/3] NFC: Misc. kernel updates and trf7970a erratum handling

[PATCH 0/3] NFC: Misc. kernel updates and trf7970a erratum handling

[Mark Greer]

Add a couple minor fixups and remove the trf7970a erratum handling
in which the last byte in the response to a Type 5 Read Multiple Blocks
command is discarded.  That handling is moved to the neard daemon where
there is enough context to do it properly.

Mark Greer (3):
  NFC: digital: NFC-A SEL_RES must be one byte
  NFC: digital: NFC-DEP Target WT(nfcdep,max) is now 14
  Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB
    commands"

 .../devicetree/bindings/net/nfc/trf7970a.txt       |  4 ----
 drivers/nfc/trf7970a.c                             | 25 ++++------------------
 net/nfc/digital_dep.c                              |  2 +-
 net/nfc/digital_technology.c                       |  3 ++-
 4 files changed, 7 insertions(+), 27 deletions(-)

-- 
2.13.0

[PATCH 1/3] NFC: digital: NFC-A SEL_RES must be one byte

[Mark Greer]

Section 4.8.2 (SEL_RES Response) of NFC Forum's NFC Digital Protocol
Technical Specification dated 2010-11-17 clearly states that the size
of a SEL_RES Response is one byte.  Enforce this restriction in the
code.

Signed-off-by: Mark Greer <mgreer@animalcreek.com>
---
 net/nfc/digital_technology.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
index d9080dec5d27..06c4795ed647 100644
--- a/net/nfc/digital_technology.c
+++ b/net/nfc/digital_technology.c
@@ -27,6 +27,7 @@
 
 #define DIGITAL_SDD_RES_CT  0x88
 #define DIGITAL_SDD_RES_LEN 5
+#define DIGITAL_SEL_RES_LEN 1
 
 #define DIGITAL_SEL_RES_NFCID1_COMPLETE(sel_res) (!((sel_res) & 0x04))
 #define DIGITAL_SEL_RES_IS_T2T(sel_res) (!((sel_res) & 0x60))
@@ -299,7 +300,7 @@ static void digital_in_recv_sel_res(struct nfc_digital_dev *ddev, void *arg,
 		}
 	}
 
-	if (!resp->len) {
+	if (resp->len != DIGITAL_SEL_RES_LEN) {
 		rc = -EIO;
 		goto exit;
 	}
-- 
2.13.0

[PATCH 2/3] NFC: digital: NFC-DEP Target WT(nfcdep,max) is now 14

[Mark Greer]

Version 1.1 of the NFC Forum's NFC Digital Protocol Technical
Specification dated 2014-07-14 specifies that the NFC-DEP Protocol's
Target WT(nfcdep,max) value is 14.  In version 1.0 it was 8 so change
the value in the Linux NFC-DEP Protocol code accordingly.

Signed-off-by: Mark Greer <mgreer@animalcreek.com>
---
 net/nfc/digital_dep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfc/digital_dep.c b/net/nfc/digital_dep.c
index f864ce19e13d..5bb241bb3490 100644
--- a/net/nfc/digital_dep.c
+++ b/net/nfc/digital_dep.c
@@ -151,7 +151,7 @@ static const u8 digital_payload_bits_map[4] = {
  *  0 <= wt <= 14 (given by the target by the TO field of ATR_RES response)
  */
 #define DIGITAL_NFC_DEP_IN_MAX_WT 14
-#define DIGITAL_NFC_DEP_TG_MAX_WT 8
+#define DIGITAL_NFC_DEP_TG_MAX_WT 14
 static const u16 digital_rwt_map[DIGITAL_NFC_DEP_IN_MAX_WT + 1] = {
 	100,  101,  101,  102,  105,
 	110,  119,  139,  177,  255,
-- 
2.13.0

[PATCH 3/3] Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB commands"

[Mark Greer]

This reverts commit ab714817d7e891608d31f6996b1e4c43cf2bf342.

The original commit was designed to handle a bug in the trf7970a NFC
controller where an extra byte was returned in Read Multiple Blocks (RMB)
command responses.  However, it has become less clear whether it is a bug
in the trf7970a or in the tag.  In addition, it was assumed that the extra
byte was always returned but it turns out that is not always the case. The
result is that a byte of good data is trimmed off when the extra byte is
not present ultimately causing the neard deamon to fail the read.

Since the trf7970a driver does not have the context to know when to trim
the byte or not, remove the code from the trf7970a driver all together
(and move it up to the neard daemon).  This has the added benefit of
simplifying the kernel driver and putting the extra complexity into
userspace.

CC: Rob Herring <robh@kernel.org>
CC: devicetree@vger.kernel.org
Signed-off-by: Mark Greer <mgreer@animalcreek.com>
---
 .../devicetree/bindings/net/nfc/trf7970a.txt       |  4 ----
 drivers/nfc/trf7970a.c                             | 25 ++++------------------
 2 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index a24a93a4b010..60c833d62181 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -17,9 +17,6 @@ Optional SoC Specific Properties:
   "IRQ Status Read" erratum.
 - en2-rf-quirk: Specify that the trf7970a being used has the "EN2 RF"
   erratum.
-- 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
 
@@ -43,7 +40,6 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 		autosuspend-delay = <30000>;
 		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 28b942ea15fb..199ab77efa5c 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -150,7 +150,6 @@
  */
 #define TRF7970A_QUIRK_IRQ_STATUS_READ		BIT(0)
 #define TRF7970A_QUIRK_EN2_MUST_STAY_LOW	BIT(1)
-#define TRF7970A_QUIRK_T5T_RMB_EXTRA_BYTE	BIT(2)
 
 /* Direct commands */
 #define TRF7970A_CMD_IDLE			0x00
@@ -449,7 +448,6 @@ struct trf7970a {
 	u8				md_rf_tech;
 	u8				tx_cmd;
 	bool				issue_eof;
-	bool				adjust_resp_len;
 	struct gpio_desc		*en_gpiod;
 	struct gpio_desc		*en2_gpiod;
 	struct mutex			lock;
@@ -630,13 +628,6 @@ static void trf7970a_send_upstream(struct trf7970a *trf)
 		trf->aborting = false;
 	}
 
-	if (trf->adjust_resp_len) {
-		if (trf->rx_skb)
-			skb_trim(trf->rx_skb, trf->rx_skb->len - 1);
-
-		trf->adjust_resp_len = false;
-	}
-
 	trf->cb(trf->ddev, trf->cb_arg, trf->rx_skb);
 
 	trf->rx_skb = NULL;
@@ -1459,15 +1450,10 @@ static int trf7970a_per_cmd_config(struct trf7970a *trf, struct sk_buff *skb)
 			trf->iso_ctrl = iso_ctrl;
 		}
 
-		if (trf->framing == NFC_DIGITAL_FRAMING_ISO15693_T5T) {
-			if (trf7970a_is_iso15693_write_or_lock(req[1]) &&
-			    (req[0] & ISO15693_REQ_FLAG_OPTION))
-				trf->issue_eof = true;
-			else if ((trf->quirks &
-				  TRF7970A_QUIRK_T5T_RMB_EXTRA_BYTE) &&
-				 (req[1] == ISO15693_CMD_READ_MULTIPLE_BLOCK))
-				trf->adjust_resp_len = true;
-		}
+		if ((trf->framing == NFC_DIGITAL_FRAMING_ISO15693_T5T) &&
+		    trf7970a_is_iso15693_write_or_lock(req[1]) &&
+		    (req[0] & ISO15693_REQ_FLAG_OPTION))
+			trf->issue_eof = true;
 	}
 
 	return 0;
@@ -2032,9 +2018,6 @@ static int trf7970a_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	if (of_property_read_bool(np, "t5t-rmb-extra-byte-quirk"))
-		trf->quirks |= TRF7970A_QUIRK_T5T_RMB_EXTRA_BYTE;
-
 	if (of_property_read_bool(np, "irq-status-read-quirk"))
 		trf->quirks |= TRF7970A_QUIRK_IRQ_STATUS_READ;
 
-- 
2.13.0

[PATCH 3/3] Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB commands"

[Mark Greer]

This reverts commit ab714817d7e891608d31f6996b1e4c43cf2bf342.

The original commit was designed to handle a bug in the trf7970a NFC
controller where an extra byte was returned in Read Multiple Blocks (RMB)
command responses.  However, it has become less clear whether it is a bug
in the trf7970a or in the tag.  In addition, it was assumed that the extra
byte was always returned but it turns out that is not always the case. The
result is that a byte of good data is trimmed off when the extra byte is
not present ultimately causing the neard deamon to fail the read.

Since the trf7970a driver does not have the context to know when to trim
the byte or not, remove the code from the trf7970a driver all together
(and move it up to the neard daemon).  This has the added benefit of
simplifying the kernel driver and putting the extra complexity into
userspace.

CC: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Mark Greer <mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>
---
 .../devicetree/bindings/net/nfc/trf7970a.txt       |  4 ----
 drivers/nfc/trf7970a.c                             | 25 ++++------------------
 2 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index a24a93a4b010..60c833d62181 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -17,9 +17,6 @@ Optional SoC Specific Properties:
   "IRQ Status Read" erratum.
 - en2-rf-quirk: Specify that the trf7970a being used has the "EN2 RF"
   erratum.
-- 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
 
@@ -43,7 +40,6 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
 		autosuspend-delay = <30000>;
 		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 28b942ea15fb..199ab77efa5c 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -150,7 +150,6 @@
  */
 #define TRF7970A_QUIRK_IRQ_STATUS_READ		BIT(0)
 #define TRF7970A_QUIRK_EN2_MUST_STAY_LOW	BIT(1)
-#define TRF7970A_QUIRK_T5T_RMB_EXTRA_BYTE	BIT(2)
 
 /* Direct commands */
 #define TRF7970A_CMD_IDLE			0x00
@@ -449,7 +448,6 @@ struct trf7970a {
 	u8				md_rf_tech;
 	u8				tx_cmd;
 	bool				issue_eof;
-	bool				adjust_resp_len;
 	struct gpio_desc		*en_gpiod;
 	struct gpio_desc		*en2_gpiod;
 	struct mutex			lock;
@@ -630,13 +628,6 @@ static void trf7970a_send_upstream(struct trf7970a *trf)
 		trf->aborting = false;
 	}
 
-	if (trf->adjust_resp_len) {
-		if (trf->rx_skb)
-			skb_trim(trf->rx_skb, trf->rx_skb->len - 1);
-
-		trf->adjust_resp_len = false;
-	}
-
 	trf->cb(trf->ddev, trf->cb_arg, trf->rx_skb);
 
 	trf->rx_skb = NULL;
@@ -1459,15 +1450,10 @@ static int trf7970a_per_cmd_config(struct trf7970a *trf, struct sk_buff *skb)
 			trf->iso_ctrl = iso_ctrl;
 		}
 
-		if (trf->framing == NFC_DIGITAL_FRAMING_ISO15693_T5T) {
-			if (trf7970a_is_iso15693_write_or_lock(req[1]) &&
-			    (req[0] & ISO15693_REQ_FLAG_OPTION))
-				trf->issue_eof = true;
-			else if ((trf->quirks &
-				  TRF7970A_QUIRK_T5T_RMB_EXTRA_BYTE) &&
-				 (req[1] == ISO15693_CMD_READ_MULTIPLE_BLOCK))
-				trf->adjust_resp_len = true;
-		}
+		if ((trf->framing == NFC_DIGITAL_FRAMING_ISO15693_T5T) &&
+		    trf7970a_is_iso15693_write_or_lock(req[1]) &&
+		    (req[0] & ISO15693_REQ_FLAG_OPTION))
+			trf->issue_eof = true;
 	}
 
 	return 0;
@@ -2032,9 +2018,6 @@ static int trf7970a_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	if (of_property_read_bool(np, "t5t-rmb-extra-byte-quirk"))
-		trf->quirks |= TRF7970A_QUIRK_T5T_RMB_EXTRA_BYTE;

Re: [PATCH 0/3] NFC: Misc. kernel updates and trf7970a erratum handling

[Samuel Ortiz]

Hi Mark,

On Thu, Jun 15, 2017 at 10:46:14AM -0700, Mark Greer wrote:
> Add a couple minor fixups and remove the trf7970a erratum handling
> in which the last byte in the response to a Type 5 Read Multiple Blocks
> command is discarded.  That handling is moved to the neard daemon where
> there is enough context to do it properly.
> 
> Mark Greer (3):
>   NFC: digital: NFC-A SEL_RES must be one byte
>   NFC: digital: NFC-DEP Target WT(nfcdep,max) is now 14
>   Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB
>     commands"
All 3 patches applied, thanks.

Cheers,
Samuel.

Re: [PATCH 3/3] Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB commands"

[Rob Herring]

On Thu, Jun 15, 2017 at 10:46:17AM -0700, Mark Greer wrote:
> This reverts commit ab714817d7e891608d31f6996b1e4c43cf2bf342.
> 
> The original commit was designed to handle a bug in the trf7970a NFC
> controller where an extra byte was returned in Read Multiple Blocks (RMB)
> command responses.  However, it has become less clear whether it is a bug
> in the trf7970a or in the tag.  In addition, it was assumed that the extra
> byte was always returned but it turns out that is not always the case. The
> result is that a byte of good data is trimmed off when the extra byte is
> not present ultimately causing the neard deamon to fail the read.
> 
> Since the trf7970a driver does not have the context to know when to trim
> the byte or not, remove the code from the trf7970a driver all together
> (and move it up to the neard daemon).  This has the added benefit of
> simplifying the kernel driver and putting the extra complexity into
> userspace.
> 
> CC: Rob Herring <robh@kernel.org>
> CC: devicetree@vger.kernel.org
> Signed-off-by: Mark Greer <mgreer@animalcreek.com>
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  4 ----
>  drivers/nfc/trf7970a.c                             | 25 ++++------------------
>  2 files changed, 4 insertions(+), 25 deletions(-)

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

Re: [PATCH 3/3] Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB commands"

[Rob Herring]

On Thu, Jun 15, 2017 at 10:46:17AM -0700, Mark Greer wrote:
> This reverts commit ab714817d7e891608d31f6996b1e4c43cf2bf342.
> 
> The original commit was designed to handle a bug in the trf7970a NFC
> controller where an extra byte was returned in Read Multiple Blocks (RMB)
> command responses.  However, it has become less clear whether it is a bug
> in the trf7970a or in the tag.  In addition, it was assumed that the extra
> byte was always returned but it turns out that is not always the case. The
> result is that a byte of good data is trimmed off when the extra byte is
> not present ultimately causing the neard deamon to fail the read.
> 
> Since the trf7970a driver does not have the context to know when to trim
> the byte or not, remove the code from the trf7970a driver all together
> (and move it up to the neard daemon).  This has the added benefit of
> simplifying the kernel driver and putting the extra complexity into
> userspace.
> 
> CC: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Mark Greer <mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  4 ----
>  drivers/nfc/trf7970a.c                             | 25 ++++------------------
>  2 files changed, 4 insertions(+), 25 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html