2016-12-15 22:30:53

by Geoff Lansberry

[permalink] [raw]
Subject: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock

From: Geoff Lansberry <[email protected]>

---
.../devicetree/bindings/net/nfc/trf7970a.txt | 3 ++
drivers/nfc/trf7970a.c | 42 ++++++++++++++++------
2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 32b35a0..9dda879 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -21,6 +21,8 @@ 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):

@@ -43,6 +45,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 26c9dbb..2d2a077 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1056,12 +1056,11 @@ static int trf7970a_init(struct trf7970a *trf)

trf->chip_status_ctrl &= ~TRF7970A_CHIP_STATUS_RF_ON;

- ret = trf7970a_write(trf, TRF7970A_MODULATOR_SYS_CLK_CTRL, 0);
+ ret = trf7970a_write(trf, TRF7970A_MODULATOR_SYS_CLK_CTRL,
+ trf->modulator_sys_clk_ctrl);
if (ret)
goto err_out;

- trf->modulator_sys_clk_ctrl = 0;
-
ret = trf7970a_write(trf, TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS,
TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS_WLH_96 |
TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS_WLL_32);
@@ -1181,27 +1180,37 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
switch (tech) {
case NFC_DIGITAL_RF_TECH_106A:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443A_106;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_OOK;
trf->guard_time = TRF7970A_GUARD_TIME_NFCA;
break;
case NFC_DIGITAL_RF_TECH_106B:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443B_106;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
trf->guard_time = TRF7970A_GUARD_TIME_NFCB;
break;
case NFC_DIGITAL_RF_TECH_212F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_FELICA_212;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
trf->guard_time = TRF7970A_GUARD_TIME_NFCF;
break;
case NFC_DIGITAL_RF_TECH_424F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_FELICA_424;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
trf->guard_time = TRF7970A_GUARD_TIME_NFCF;
break;
case NFC_DIGITAL_RF_TECH_ISO15693:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_15693_SGL_1OF4_2648;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_OOK;
trf->guard_time = TRF7970A_GUARD_TIME_15693;
break;
default:
@@ -1571,17 +1580,23 @@ static int trf7970a_tg_config_rf_tech(struct trf7970a *trf, int tech)
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
TRF7970A_ISO_CTRL_NFC_CE |
TRF7970A_ISO_CTRL_NFC_CE_14443A;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_OOK;
break;
case NFC_DIGITAL_RF_TECH_212F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
TRF7970A_ISO_CTRL_NFC_NFCF_212;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
break;
case NFC_DIGITAL_RF_TECH_424F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
TRF7970A_ISO_CTRL_NFC_NFCF_424;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
break;
default:
dev_dbg(trf->dev, "Unsupported rf technology: %d\n", tech);
@@ -2043,6 +2058,11 @@ 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;
+ dev_dbg(trf->dev, "trf7970a configure crystal_27mhz\n");
+ }
+
if (of_property_read_bool(np, "en2-rf-quirk"))
trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;

--
Signed-off-by: Geoff Lansberry <[email protected]>


2016-12-19 23:23:42

by Geoff Lansberry

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock

I can make that change, however, I worry that it may be a bit
misleading, since there are only two supported clock frequencies, but
a number like that to me implies that it could be set to any number
you want. I'm new at this, and so I'll go ahead and change it as you
request, but I'd like to hear your thoughts on my concern.

Thanks
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 Mon, Dec 19, 2016 at 5:31 PM, Rob Herring <[email protected]> wrote:
> On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
>> From: Geoff Lansberry <[email protected]>
>>
>> ---
>> .../devicetree/bindings/net/nfc/trf7970a.txt | 3 ++
>> drivers/nfc/trf7970a.c | 42 +++++++++++++++=
+------
>> 2 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Do=
cumentation/devicetree/bindings/net/nfc/trf7970a.txt
>> index 32b35a0..9dda879 100644
>> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> @@ -21,6 +21,8 @@ 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 issue=
d
>> to Type 5 tags.
>> +- crystal_27mhz: Set to specify that the input frequency to the trf7970=
a is 27.12MHz
>> +
>
> Can't you use 'clock-frequency =3D "27000000";'?
>
>>
>> Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>>
>> @@ -43,6 +45,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI=
1):
>> irq-status-read-quirk;
>> en2-rf-quirk;
>> t5t-rmb-extra-byte-quirk;
>> + crystal_27mhz;
>> status =3D "okay";
>> };
>> };

2016-12-16 01:14:12

by Mark Greer

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <[email protected]>

Missing commit description.

> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 2d2a077..b4c37ab 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c

> @@ -1048,6 +1049,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));

s/l|T/l | T/

Otherwise, looks good.

Mark
--

2016-12-16 01:06:15

by Mark Greer

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock

Hi Geoff.

On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <[email protected]>

Please add an informative commit description to all of your commits.
No matter how trivial this patch may seem to you now, it may not be
to others (or to you in a few years).

> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 26c9dbb..2d2a077 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c

> @@ -1181,27 +1180,37 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
> switch (tech) {
> case NFC_DIGITAL_RF_TECH_106A:
> trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443A_106;
> - trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
> + trf->modulator_sys_clk_ctrl =
> + (trf->modulator_sys_clk_ctrl & 0xF8) |

nit: s/0xF8/0xf8/ please (for consistency with the rest of the file.).

Otherwise, it looks good.

Thanks,

Mark

2016-12-19 22:35:07

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <[email protected]>
>
> ---
> Documentation/devicetree/bindings/net/nfc/trf7970a.txt | 2 ++
> drivers/nfc/trf7970a.c | 13 ++++++++++++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 9dda879..208f045 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,7 @@ Optional SoC Specific Properties:
> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
> where an extra byte is returned by Read Multiple Block commands issued
> to Type 5 tags.
> +- vdd_io_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V

Use the regulator binding and provide a fixed 1.8V supply.

> - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>
>
> @@ -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;
> + vdd_io_1v8;
> crystal_27mhz;
> status = "okay";
> };

2016-12-19 22:31:13

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock

On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <[email protected]>
>
> ---
> .../devicetree/bindings/net/nfc/trf7970a.txt | 3 ++
> drivers/nfc/trf7970a.c | 42 ++++++++++++++++------
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 32b35a0..9dda879 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,8 @@ 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
> +

Can't you use 'clock-frequency = "27000000";'?

>
> Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>
> @@ -43,6 +45,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";
> };
> };

2016-12-22 18:48:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock

On Mon, Dec 19, 2016 at 5:23 PM, Geoff Lansberry <[email protected]> wrote:
> I can make that change, however, I worry that it may be a bit
> misleading, since there are only two supported clock frequencies, but
> a number like that to me implies that it could be set to any number
> you want. I'm new at this, and so I'll go ahead and change it as you
> request, but I'd like to hear your thoughts on my concern.

Then the binding doc just needs to state what are the 2 valid frequencies.

Rob

2016-12-15 22:30:56

by Geoff Lansberry

[permalink] [raw]
Subject: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

From: Geoff Lansberry <[email protected]>

---
Documentation/devicetree/bindings/net/nfc/trf7970a.txt | 2 ++
drivers/nfc/trf7970a.c | 13 ++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 9dda879..208f045 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -21,6 +21,7 @@ Optional SoC Specific Properties:
- t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
where an extra byte is returned by Read Multiple Block commands issued
to Type 5 tags.
+- vdd_io_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V
- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz


@@ -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;
+ vdd_io_1v8;
crystal_27mhz;
status = "okay";
};
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 2d2a077..b4c37ab 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;
@@ -1048,6 +1049,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;
@@ -1764,7 +1770,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;

@@ -2058,6 +2064,11 @@ 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;
+ dev_dbg(trf->dev, "trf7970a config vdd_io_1v8\n");
+ }
+
if (of_property_read_bool(np, "crystal_27mhz")) {
trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_27MHZ;
dev_dbg(trf->dev, "trf7970a configure crystal_27mhz\n");
--
Signed-off-by: Geoff Lansberry <[email protected]>

2016-12-15 22:30:57

by Geoff Lansberry

[permalink] [raw]
Subject: [PATCH 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel

From: Jaret Cantu <[email protected]>

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

The solution is to properly kill the waiting reading (workqueue)
before failing on the send.
---
drivers/nfc/trf7970a.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index b4c37ab..f96a321 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1493,6 +1493,10 @@ static int trf7970a_send_cmd(struct nfc_digital_dev *ddev,
(trf->state != TRF7970A_ST_IDLE_RX_BLOCKED)) {
dev_err(trf->dev, "%s - Bogus state: %d\n", __func__,
trf->state);
+ if (trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA ||
+ trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA_CONT)
+ trf->ignore_timeout =
+ !cancel_delayed_work(&trf->timeout_work);
ret = -EIO;
goto out_err;
}
--
Signed-off-by: Geoff Lansberry <[email protected]>

2016-12-16 01:18:25

by Mark Greer

[permalink] [raw]
Subject: Re: [PATCH 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel

On Thu, Dec 15, 2016 at 05:30:44PM -0500, Geoff Lansberry wrote:
> From: Jaret Cantu <[email protected]>
>
> Repeated polling attempts cause a NULL dereference error to occur.
> This is because the curent state of the trf7970a is reading but
> a request has been made to send a command.
>
> The solution is to properly kill the waiting reading (workqueue)
> before failing on the send.

Maybe its just me but I find this description a little hard to grok.
Mind reworking it?

The patch itself looks fine.

Thanks,

Mark
--

2016-12-20 16:14:05

by Geoff Lansberry

[permalink] [raw]
Subject: Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage

On Mon, Dec 19, 2016 at 5:35 PM, Rob Herring <[email protected]> wrote:
> On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
>> From: Geoff Lansberry <[email protected]>
>>
>> ---
>> Documentation/devicetree/bindings/net/nfc/trf7970a.txt | 2 ++
>> drivers/nfc/trf7970a.c | 13 ++++++++++++-
>> 2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> index 9dda879..208f045 100644
>> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> @@ -21,6 +21,7 @@ Optional SoC Specific Properties:
>> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
>> where an extra byte is returned by Read Multiple Block commands issued
>> to Type 5 tags.
>> +- vdd_io_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V
>
> Use the regulator binding and provide a fixed 1.8V supply.
>
>> - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>>
>>
>> @@ -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;
>> + vdd_io_1v8;
>> crystal_27mhz;
>> status = "okay";
>> };

Rob - using the regulator binding is new to me, but I've given it a
shot and just sent you another set of patches for your inspection.
Please let me know if this is what you had in mind.

Geoff