[PATCH net-next v4 1/2] nfc: Add a virtual nci device driver

[Bongsu Jeon]

From: Bongsu Jeon 

NCI virtual device simulates a NCI device to the user. It can be used to
validate the NCI module and applications. This driver supports
communication between the virtual NCI device and NCI module.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/Kconfig  |  11 ++
 drivers/nfc/Makefile |   1 +
 drivers/nfc/virtual_ncidev.c | 215 +++
 3 files changed, 227 insertions(+)
 create mode 100644 drivers/nfc/virtual_ncidev.c

diff --git a/drivers/nfc/Kconfig b/drivers/nfc/Kconfig
index 75c65d339018..288c6f1c6979 100644
--- a/drivers/nfc/Kconfig
+++ b/drivers/nfc/Kconfig
@@ -49,6 +49,17 @@ config NFC_PORT100
 
  If unsure, say N.
 
+config NFC_VIRTUAL_NCI
+   tristate "NCI device simulator driver"
+   depends on NFC_NCI
+   help
+ NCI virtual device simulates a NCI device to the user.
+ It can be used to validate the NCI module and applications.
+ This driver supports communication between the virtual NCI device and
+ module.
+
+ If unsure, say N.
+
 source "drivers/nfc/fdp/Kconfig"
 source "drivers/nfc/pn544/Kconfig"
 source "drivers/nfc/pn533/Kconfig"
diff --git a/drivers/nfc/Makefile b/drivers/nfc/Makefile
index 5393ba59b17d..7b1bfde1d971 100644
--- a/drivers/nfc/Makefile
+++ b/drivers/nfc/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_NFC_ST_NCI)  += st-nci/
 obj-$(CONFIG_NFC_NXP_NCI)  += nxp-nci/
 obj-$(CONFIG_NFC_S3FWRN5)  += s3fwrn5/
 obj-$(CONFIG_NFC_ST95HF)   += st95hf/
+obj-$(CONFIG_NFC_VIRTUAL_NCI)  += virtual_ncidev.o
diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
new file mode 100644
index ..f73ee0bf3593
--- /dev/null
+++ b/drivers/nfc/virtual_ncidev.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual NCI device simulation driver
+ *
+ * Copyright (C) 2020 Samsung Electrnoics
+ * Bongsu Jeon 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+enum virtual_ncidev_mode {
+   virtual_ncidev_enabled,
+   virtual_ncidev_disabled,
+   virtual_ncidev_disabling,
+};
+
+#define IOCTL_GET_NCIDEV_IDX0
+#define VIRTUAL_NFC_PROTOCOLS  (NFC_PROTO_JEWEL_MASK | \
+NFC_PROTO_MIFARE_MASK | \
+NFC_PROTO_FELICA_MASK | \
+NFC_PROTO_ISO14443_MASK | \
+NFC_PROTO_ISO14443_B_MASK | \
+NFC_PROTO_ISO15693_MASK)
+
+static enum virtual_ncidev_mode state;
+static struct miscdevice miscdev;
+static struct sk_buff *send_buff;
+static struct nci_dev *ndev;
+static DEFINE_MUTEX(nci_mutex);
+
+static int virtual_nci_open(struct nci_dev *ndev)
+{
+   return 0;
+}
+
+static int virtual_nci_close(struct nci_dev *ndev)
+{
+   mutex_lock(_mutex);
+   kfree_skb(send_buff);
+   send_buff = NULL;
+   mutex_unlock(_mutex);
+
+   return 0;
+}
+
+static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
+{
+   mutex_lock(_mutex);
+   if (state != virtual_ncidev_enabled) {
+   mutex_unlock(_mutex);
+   return 0;
+   }
+
+   if (send_buff) {
+   mutex_unlock(_mutex);
+   return -1;
+   }
+   send_buff = skb_copy(skb, GFP_KERNEL);
+   mutex_unlock(_mutex);
+
+   return 0;
+}
+
+static struct nci_ops virtual_nci_ops = {
+   .open = virtual_nci_open,
+   .close = virtual_nci_close,
+   .send = virtual_nci_send
+};
+
+static ssize_t virtual_ncidev_read(struct file *file, char __user *buf,
+  size_t count, loff_t *ppos)
+{
+   size_t actual_len;
+
+   mutex_lock(_mutex);
+   if (!send_buff) {
+   mutex_unlock(_mutex);
+   return 0;
+   }
+
+   actual_len = min_t(size_t, count, send_buff->len);
+
+   if (copy_to_user(buf, send_buff->data, actual_len)) {
+   mutex_unlock(_mutex);
+   return -EFAULT;
+   }
+
+   skb_pull(send_buff, actual_len);
+   if (send_buff->len == 0) {
+   consume_skb(send_buff);
+   send_buff = NULL;
+   }
+   mutex_unlock(_mutex);
+
+   return actual_len;
+}
+
+static ssize_t virtual_ncidev_write(struct file *file,
+   const char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   struct sk_buff *skb;
+
+   skb = alloc_skb(count, GFP_KERNEL);
+   if (!skb)
+   return -ENOMEM;
+
+   if (copy_from_user(skb_put(skb, count), buf, count)) {
+   kfree_skb(skb);
+   return -EFAULT;
+   }
+
+   nci_recv_frame(ndev, skb);
+   return count;
+}
+
+static int virtual_ncidev_open(struct inode *inode, struct file *file)
+{
+   int ret = 0;
+
+   mutex_lock(_mutex);
+   if (state != virtual_ncide

[PATCH net-next v4 2/2] selftests: Add nci suite

[Bongsu Jeon]

From: Bongsu Jeon 

This is the NCI test suite. It tests the NFC/NCI module using virtual NCI
device. Test cases consist of making the virtual NCI device on/off and
controlling the device's polling for NCI1.0 and NCI2.0 version.

Signed-off-by: Bongsu Jeon 
---
 MAINTAINERS   |   8 +
 tools/testing/selftests/Makefile  |   1 +
 tools/testing/selftests/nci/Makefile  |   6 +
 tools/testing/selftests/nci/config|   3 +
 tools/testing/selftests/nci/nci_dev.c | 599 ++
 5 files changed, 617 insertions(+)
 create mode 100644 tools/testing/selftests/nci/Makefile
 create mode 100644 tools/testing/selftests/nci/config
 create mode 100644 tools/testing/selftests/nci/nci_dev.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 096b584e7fed..b62832e39e2a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12531,6 +12531,14 @@ F: include/net/nfc/
 F: include/uapi/linux/nfc.h
 F: net/nfc/
 
+NFC VIRTUAL NCI DEVICE DRIVER
+M: Bongsu Jeon 
+L: net...@vger.kernel.org
+L: linux-...@lists.01.org (moderated for non-subscribers)
+S: Supported
+F: drivers/nfc/virtual_ncidev.c
+F: tools/testing/selftests/nci/
+
 NFS, SUNRPC, AND LOCKD CLIENTS
 M: Trond Myklebust 
 M: Anna Schumaker 
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 8a917cb4426a..c42aacec5038 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -34,6 +34,7 @@ TARGETS += memory-hotplug
 TARGETS += mincore
 TARGETS += mount
 TARGETS += mqueue
+TARGETS += nci
 TARGETS += net
 TARGETS += net/forwarding
 TARGETS += net/mptcp
diff --git a/tools/testing/selftests/nci/Makefile 
b/tools/testing/selftests/nci/Makefile
new file mode 100644
index ..47669a1d6a59
--- /dev/null
+++ b/tools/testing/selftests/nci/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -Wl,-no-as-needed -Wall
+LDFLAGS += -lpthread
+
+TEST_GEN_PROGS := nci_dev
+include ../lib.mk
diff --git a/tools/testing/selftests/nci/config 
b/tools/testing/selftests/nci/config
new file mode 100644
index ..b084e78276be
--- /dev/null
+++ b/tools/testing/selftests/nci/config
@@ -0,0 +1,3 @@
+CONFIG_NFC=y
+CONFIG_NFC_NCI=y
+CONFIG_NFC_VIRTUAL_NCI=y
diff --git a/tools/testing/selftests/nci/nci_dev.c 
b/tools/testing/selftests/nci/nci_dev.c
new file mode 100644
index ..57b505cb1561
--- /dev/null
+++ b/tools/testing/selftests/nci/nci_dev.c
@@ -0,0 +1,599 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 Samsung Electrnoics
+ * Bongsu Jeon 
+ *
+ * Test code for nci
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../kselftest_harness.h"
+
+#define GENLMSG_DATA(glh)  ((void *)(NLMSG_DATA(glh) + GENL_HDRLEN))
+#define GENLMSG_PAYLOAD(glh)   (NLMSG_PAYLOAD(glh, 0) - GENL_HDRLEN)
+#define NLA_DATA(na)   ((void *)((char *)(na) + NLA_HDRLEN))
+#define NLA_PAYLOAD(len)   ((len) - NLA_HDRLEN)
+
+#define MAX_MSG_SIZE   1024
+
+#define IOCTL_GET_NCIDEV_IDX   0
+#define VIRTUAL_NFC_PROTOCOLS  (NFC_PROTO_JEWEL_MASK | \
+NFC_PROTO_MIFARE_MASK | \
+NFC_PROTO_FELICA_MASK | \
+NFC_PROTO_ISO14443_MASK | \
+NFC_PROTO_ISO14443_B_MASK | \
+NFC_PROTO_ISO15693_MASK)
+
+const __u8 nci_reset_cmd[] = {0x20, 0x00, 0x01, 0x01};
+const __u8 nci_init_cmd[] = {0x20, 0x01, 0x00};
+const __u8 nci_rf_discovery_cmd[] = {0x21, 0x03, 0x09, 0x04, 0x00, 0x01,
+ 0x01, 0x01, 0x02, 0x01, 0x06, 0x01};
+const __u8 nci_init_cmd_v2[] = {0x20, 0x01, 0x02, 0x00, 0x00};
+const __u8 nci_rf_disc_map_cmd[] = {0x21, 0x00, 0x07, 0x02, 0x04, 0x03,
+0x02, 0x05, 0x03, 0x03};
+const __u8 nci_rf_deact_cmd[] = {0x21, 0x06, 0x01, 0x00};
+const __u8 nci_reset_rsp[] = {0x40, 0x00, 0x03, 0x00, 0x10, 0x01};
+const __u8 nci_reset_rsp_v2[] = {0x40, 0x00, 0x01, 0x00};
+const __u8 nci_reset_ntf[] = {0x60, 0x00, 0x09, 0x02, 0x01, 0x20, 0x0e,
+  0x04, 0x61, 0x00, 0x04, 0x02};
+const __u8 nci_init_rsp[] = {0x40, 0x01, 0x14, 0x00, 0x02, 0x0e, 0x02,
+ 0x00, 0x03, 0x01, 0x02, 0x03, 0x02, 0xc8,
+ 0x00, 0xff, 0x10, 0x00, 0x0e, 0x12, 0x00,
+ 0x00, 0x04};
+const __u8 nci_init_rsp_v2[] = {0x40, 0x01, 0x1c, 0x00, 0x1a, 0x7e, 0x06,
+0x00, 0x02, 0x92, 0x04, 0xff, 0xff, 0x01,
+0x00, 0x40, 0x06, 0x00, 0x00, 0x01, 0x01,
+0x00, 0x02, 0x00, 0x03, 0x01, 0x01, 0x06,
+0x00, 0x80, 0x00};
+const __u8 nci_rf_disc_map_rsp[] = {0x41, 0x00, 0x01, 0x00};
+const __u8 nci_rf_disc_rsp[] = {0x41, 0x03, 0x01, 0x00};
+

[PATCH net-next v4 0/2] Add nci suit and virtual nci device driver

[Bongsu Jeon]

From: Bongsu Jeon 

1/2 is the Virtual NCI device driver.
2/2 is the NCI selftest suite

v4:
 1/2
 - flip the condition for the ioctl.
 - refactor some code.
 - remove the unused function after refactoring.
v3:
 1/2
 - change the Kconfig help comment.
 - remove the mutex init code.
 - remove the unnecessary mutex(nci_send_mutex).
 - remove the full_txbuff.
 - add the code to release skb at error case.
 - refactor some code.
v2:
 1/2
 - change the permission of the Virtual NCI device.
 - add the ioctl to find the nci device index.
 2/2
 - add the NCI selftest suite.

Bongsu Jeon (2):
  nfc: Add a virtual nci device driver
  selftests: Add nci suite

 MAINTAINERS   |   8 +
 drivers/nfc/Kconfig   |  11 +
 drivers/nfc/Makefile  |   1 +
 drivers/nfc/virtual_ncidev.c  | 215 +
 tools/testing/selftests/Makefile  |   1 +
 tools/testing/selftests/nci/Makefile  |   6 +
 tools/testing/selftests/nci/config|   3 +
 tools/testing/selftests/nci/nci_dev.c | 599 ++
 8 files changed, 844 insertions(+)
 create mode 100644 drivers/nfc/virtual_ncidev.c
 create mode 100644 tools/testing/selftests/nci/Makefile
 create mode 100644 tools/testing/selftests/nci/config
 create mode 100644 tools/testing/selftests/nci/nci_dev.c

-- 
2.25.1

Re: [PATCH net] net: nfc: nci: fix the wrong NCI_CORE_INIT parameters

[Bongsu Jeon]

On Wed, Jan 20, 2021 at 10:00 AM  wrote:
>
> Hello:
>
> This patch was applied to netdev/net.git (refs/heads/master):
>
> On Tue, 19 Jan 2021 05:55:22 +0900 you wrote:
> > From: Bongsu Jeon 
> >
> > Fix the code because NCI_CORE_INIT_CMD includes two parameters in NCI2.0
> > but there is no parameters in NCI1.x.
> >
> > Signed-off-by: Bongsu Jeon 
> >
> > [...]
>
> Here is the summary with links:
>   - [net] net: nfc: nci: fix the wrong NCI_CORE_INIT parameters
> https://git.kernel.org/netdev/net/c/4964e5a1e080
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>

Could you merge this patch to net-next repo??
NCI selftest that i will send will fail if this patch isn't merged.

Re: [PATCH net] net: nfc: nci: fix the wrong NCI_CORE_INIT parameters

[Bongsu Jeon]

On Tue, Jan 19, 2021 at 6:01 AM Jakub Kicinski  wrote:
>
> On Tue, 19 Jan 2021 05:55:22 +0900 Bongsu Jeon wrote:
> > From: Bongsu Jeon 
> >
> > Fix the code because NCI_CORE_INIT_CMD includes two parameters in NCI2.0
> > but there is no parameters in NCI1.x.
> >
> > Signed-off-by: Bongsu Jeon 
>
> What's the Fixes tag for this change?
Sorry to miss the Fixes tag.
This is the Fixes tag ( Fixes: bcd684aace34 ("net/nfc/nci: Support NCI
2.x initial sequence") )
Could I resend this patch after adding that tag?

[PATCH net] net: nfc: nci: fix the wrong NCI_CORE_INIT parameters

[Bongsu Jeon]

From: Bongsu Jeon 

Fix the code because NCI_CORE_INIT_CMD includes two parameters in NCI2.0
but there is no parameters in NCI1.x.

Signed-off-by: Bongsu Jeon 
---
 net/nfc/nci/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index e64727e1a72f..02a1f13f0798 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -508,7 +508,7 @@ static int nci_open_device(struct nci_dev *ndev)
};
unsigned long opt = 0;
 
-   if (!(ndev->nci_ver & NCI_VER_2_MASK))
+   if (ndev->nci_ver & NCI_VER_2_MASK)
opt = (unsigned long)_init_v2_cmd;
 
rc = __nci_request(ndev, nci_init_req, opt,
-- 
2.25.1

Re: [PATCH net-next] nfc: Add a virtual nci device driver

[Bongsu Jeon]

On Thu, Jan 7, 2021 at 2:01 AM Jakub Kicinski  wrote:
>
> On Wed, 6 Jan 2021 08:16:47 +0900 Bongsu Jeon wrote:
> > On Tue, Jan 5, 2021 at 4:48 AM Jakub Kicinski  wrote:
> > > > thank you for your answer.
> > > > I think that neard(NFC deamon) is necessary to test the NCI subsystem
> > > > meaningfully.
> > > > The NCI virtual device in user space can communicate with neard
> > > > through this driver.
> > > > Is it enough to make NCI virtual device at tools/nfc for some test?
> > >
> > > I'm not sure if I understand. Are you asking if it's okay for the test
> > > or have a dependency on neard?
> >
> > Sorry for confusing you.
> > There is no dependency between neard and a NCI virtual device.
> > But, To test the NCI module, it is necessary to make an application like 
> > neard.
> > Is it okay to just make a NCI virtual device as a tool at tools/nfc
> > without the application?
>
> Meaning the device will be created but there will be no test cases in
> the tree?

yes.

>
> What we'd like to see is some form of a test which would exercise the
> NFC-related kernel code on such a device and can signal success /
> failure. It doesn't have to be very complex.
>
> You can build a more complex user space applications and tests
> separately.

okay. I understand it. I will try to make it.

Re: [PATCH net-next] nfc: Add a virtual nci device driver

[Bongsu Jeon]

On Tue, Jan 5, 2021 at 4:48 AM Jakub Kicinski  wrote:
>
> On Thu, 31 Dec 2020 14:22:45 +0900 Bongsu Jeon wrote:
> > On Tue, Dec 29, 2020 at 6:16 AM Jakub Kicinski  wrote:
> > >
> > > On Mon, 28 Dec 2020 18:45:07 +0900 Bongsu Jeon wrote:
> > > > From: Bongsu Jeon 
> > > >
> > > > A NCI virtual device can be made to simulate a NCI device in user space.
> > > > Using the virtual NCI device, The NCI module and application can be
> > > > validated. This driver supports to communicate between the virtual NCI
> > > > device and NCI module.
> > > >
> > > > Signed-off-by: Bongsu Jeon 
> > >
> > > net-next is still closed:
> > >
> > > http://vger.kernel.org/~davem/net-next.html
> > >
> > > Please repost in a few days.
> > >
> > > As far as the patch goes - please include some tests for the NCI/NFC
> > > subsystem based on this virtual device, best if they live in tree under
> > > tools/testing/selftest.
> >
> > thank you for your answer.
> > I think that neard(NFC deamon) is necessary to test the NCI subsystem
> > meaningfully.
> > The NCI virtual device in user space can communicate with neard
> > through this driver.
> > Is it enough to make NCI virtual device at tools/nfc for some test?
>
> I'm not sure if I understand. Are you asking if it's okay for the test
> or have a dependency on neard?

Sorry for confusing you.
There is no dependency between neard and a NCI virtual device.
But, To test the NCI module, it is necessary to make an application like neard.
Is it okay to just make a NCI virtual device as a tool at tools/nfc
without the application?

Re: [PATCH net-next] nfc: Add a virtual nci device driver

[Bongsu Jeon]

On Tue, Dec 29, 2020 at 6:16 AM Jakub Kicinski  wrote:
>
> On Mon, 28 Dec 2020 18:45:07 +0900 Bongsu Jeon wrote:
> > From: Bongsu Jeon 
> >
> > A NCI virtual device can be made to simulate a NCI device in user space.
> > Using the virtual NCI device, The NCI module and application can be
> > validated. This driver supports to communicate between the virtual NCI
> > device and NCI module.
> >
> > Signed-off-by: Bongsu Jeon 
>
> net-next is still closed:
>
> http://vger.kernel.org/~davem/net-next.html
>
> Please repost in a few days.
>
> As far as the patch goes - please include some tests for the NCI/NFC
> subsystem based on this virtual device, best if they live in tree under
> tools/testing/selftest.

thank you for your answer.
I think that neard(NFC deamon) is necessary to test the NCI subsystem
meaningfully.
The NCI virtual device in user space can communicate with neard
through this driver.
Is it enough to make NCI virtual device at tools/nfc for some test?

[PATCH net-next v2] net: nfc: nci: Change the NCI close sequence

[Bongsu Jeon]

From: Bongsu Jeon 

If there is a NCI command in work queue after closing the NCI device at 
nci_unregister_device, The NCI command timer starts at flush_workqueue
function and then NCI command timeout handler would be called 5 second 
after flushing the NCI command work queue and destroying the queue.
At that time, the timeout handler would try to use NCI command work queue
that is destroyed already. it will causes the problem. To avoid this 
abnormal situation, change the sequence to prevent the NCI command timeout
handler from being called after destroying the NCI command work queue.

Signed-off-by: Bongsu Jeon 
---

Changes in v2:
 - Change the commit message.

 net/nfc/nci/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index e64727e1a72f..79bebf4b0796 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -579,11 +579,11 @@ static int nci_close_device(struct nci_dev *ndev)
 
clear_bit(NCI_INIT, >flags);
 
-   del_timer_sync(>cmd_timer);
-
/* Flush cmd wq */
flush_workqueue(ndev->cmd_wq);
 
+   del_timer_sync(>cmd_timer);
+
/* Clear flags */
ndev->flags = 0;
 
-- 
2.17.1

[PATCH net-next] nfc: Add a virtual nci device driver

[Bongsu Jeon]

From: Bongsu Jeon 

A NCI virtual device can be made to simulate a NCI device in user space.
Using the virtual NCI device, The NCI module and application can be
validated. This driver supports to communicate between the virtual NCI
device and NCI module.

Signed-off-by: Bongsu Jeon 
---
 MAINTAINERS  |   7 ++
 drivers/nfc/Kconfig  |  11 ++
 drivers/nfc/Makefile |   1 +
 drivers/nfc/virtual_ncidev.c | 216 +++
 4 files changed, 235 insertions(+)
 create mode 100644 drivers/nfc/virtual_ncidev.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a355db292486..6479a4754a1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12431,6 +12431,13 @@ F: include/net/nfc/
 F: include/uapi/linux/nfc.h
 F: net/nfc/
 
+NFC VIRTUAL NCI DEVICE DRIVER
+M: Bongsu Jeon 
+L: net...@vger.kernel.org
+L: linux-...@lists.01.org (moderated for non-subscribers)
+S: Supported
+F: drivers/nfc/virtual_ncidev.c
+
 NFS, SUNRPC, AND LOCKD CLIENTS
 M: Trond Myklebust 
 M: Anna Schumaker 
diff --git a/drivers/nfc/Kconfig b/drivers/nfc/Kconfig
index 75c65d339018..d32c3a8937ed 100644
--- a/drivers/nfc/Kconfig
+++ b/drivers/nfc/Kconfig
@@ -49,6 +49,17 @@ config NFC_PORT100
 
  If unsure, say N.
 
+config NFC_VIRTUAL_NCI
+   tristate "NCI device simulator driver"
+   depends on NFC_NCI
+   help
+ A NCI virtual device can be made to simulate a NCI device in user
+ level. Using the virtual NCI device, The NCI module and application
+ can be validated. This driver supports to communicate between the
+ virtual NCI device and NCI module.
+
+ If unsure, say N.
+
 source "drivers/nfc/fdp/Kconfig"
 source "drivers/nfc/pn544/Kconfig"
 source "drivers/nfc/pn533/Kconfig"
diff --git a/drivers/nfc/Makefile b/drivers/nfc/Makefile
index 5393ba59b17d..7b1bfde1d971 100644
--- a/drivers/nfc/Makefile
+++ b/drivers/nfc/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_NFC_ST_NCI)  += st-nci/
 obj-$(CONFIG_NFC_NXP_NCI)  += nxp-nci/
 obj-$(CONFIG_NFC_S3FWRN5)  += s3fwrn5/
 obj-$(CONFIG_NFC_ST95HF)   += st95hf/
+obj-$(CONFIG_NFC_VIRTUAL_NCI)  += virtual_ncidev.o
diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
new file mode 100644
index ..163d0b2dda2e
--- /dev/null
+++ b/drivers/nfc/virtual_ncidev.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual NCI device simulation driver
+ *
+ * Copyright (C) 2020 Samsung Electrnoics
+ * Bongsu Jeon 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+enum virtual_ncidev_mode {
+   virtual_ncidev_enabled,
+   virtual_ncidev_disabled,
+   virtual_ncidev_disabling,
+};
+
+#define VIRTUAL_NFC_PROTOCOLS  (NFC_PROTO_JEWEL_MASK | \
+NFC_PROTO_MIFARE_MASK | \
+NFC_PROTO_FELICA_MASK | \
+NFC_PROTO_ISO14443_MASK | \
+NFC_PROTO_ISO14443_B_MASK | \
+NFC_PROTO_ISO15693_MASK)
+
+static enum virtual_ncidev_mode state;
+static struct mutex nci_send_mutex;
+static struct miscdevice miscdev;
+static struct sk_buff *send_buff;
+static struct mutex nci_mutex;
+static struct nci_dev *ndev;
+static bool full_txbuff;
+
+static bool virtual_ncidev_check_enabled(void)
+{
+   bool ret = true;
+
+   mutex_lock(_mutex);
+   if (state != virtual_ncidev_enabled)
+   ret = false;
+   mutex_unlock(_mutex);
+
+   return ret;
+}
+
+static int virtual_nci_open(struct nci_dev *ndev)
+{
+   return 0;
+}
+
+static int virtual_nci_close(struct nci_dev *ndev)
+{
+   mutex_lock(_send_mutex);
+   if (full_txbuff)
+   kfree_skb(send_buff);
+   full_txbuff = false;
+   mutex_unlock(_send_mutex);
+
+   return 0;
+}
+
+static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
+{
+   if (virtual_ncidev_check_enabled() == false)
+   return 0;
+
+   mutex_lock(_send_mutex);
+   if (full_txbuff) {
+   mutex_unlock(_send_mutex);
+   return -1;
+   }
+   send_buff = skb_copy(skb, GFP_KERNEL);
+   full_txbuff = true;
+   mutex_unlock(_send_mutex);
+
+   return 0;
+}
+
+static struct nci_ops virtual_nci_ops = {
+   .open = virtual_nci_open,
+   .close = virtual_nci_close,
+   .send = virtual_nci_send
+};
+
+static ssize_t virtual_ncidev_read(struct file *file, char __user *buf,
+  size_t count, loff_t *ppos)
+{
+   size_t actual_len;
+
+   mutex_lock(_send_mutex);
+   if (!full_txbuff) {
+   mutex_unlock(_send_mutex);
+   return 0;
+   }
+
+   actual_len = count > send_buff->len ? send_buff->len : count;
+
+   if (copy_to_user(buf, send_buff->data, actual_len)) {
+   mutex

[PATCH net-next] net: nfc: nci: Change the NCI close sequence

[Bongsu Jeon]

From: Bongsu Jeon 

Change the NCI close sequence because the NCI Command timer should be
deleted after flushing the NCI command work queue.

Signed-off-by: Bongsu Jeon 
---
 net/nfc/nci/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index e64727e1a72f..79bebf4b0796 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -579,11 +579,11 @@ static int nci_close_device(struct nci_dev *ndev)
 
clear_bit(NCI_INIT, >flags);
 
-   del_timer_sync(>cmd_timer);
-
/* Flush cmd wq */
flush_workqueue(ndev->cmd_wq);
 
+   del_timer_sync(>cmd_timer);
+
/* Clear flags */
ndev->flags = 0;
 
-- 
2.17.1

[PATCH v2 net-next 2/2] nfc: s3fwrn5: Remove unused NCI prop commands

[Bongsu Jeon]

From: Bongsu Jeon 

Remove the unused NCI prop commands that s3fwrn5 driver doesn't use.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/nci.c | 25 -
 drivers/nfc/s3fwrn5/nci.h | 22 --
 2 files changed, 47 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/nci.c b/drivers/nfc/s3fwrn5/nci.c
index 103bf5c92bdc..f042d3eaf8f6 100644
--- a/drivers/nfc/s3fwrn5/nci.c
+++ b/drivers/nfc/s3fwrn5/nci.c
@@ -21,31 +21,11 @@ static int s3fwrn5_nci_prop_rsp(struct nci_dev *ndev, 
struct sk_buff *skb)
 }
 
 static struct nci_driver_ops s3fwrn5_nci_prop_ops[] = {
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_AGAIN),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_GET_RFREG),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
{
.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
NCI_PROP_SET_RFREG),
.rsp = s3fwrn5_nci_prop_rsp,
},
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_GET_RFREG_VER),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_SET_RFREG_VER),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
{
.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
NCI_PROP_START_RFREG),
@@ -61,11 +41,6 @@ static struct nci_driver_ops s3fwrn5_nci_prop_ops[] = {
NCI_PROP_FW_CFG),
.rsp = s3fwrn5_nci_prop_rsp,
},
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_WR_RESET),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
 };
 
 void s3fwrn5_nci_get_prop_ops(struct nci_driver_ops **ops, size_t *n)
diff --git a/drivers/nfc/s3fwrn5/nci.h b/drivers/nfc/s3fwrn5/nci.h
index 23c0b28f247a..a80f0fb082a8 100644
--- a/drivers/nfc/s3fwrn5/nci.h
+++ b/drivers/nfc/s3fwrn5/nci.h
@@ -11,9 +11,6 @@
 
 #include "s3fwrn5.h"
 
-#define NCI_PROP_AGAIN 0x01
-
-#define NCI_PROP_GET_RFREG 0x21
 #define NCI_PROP_SET_RFREG 0x22
 
 struct nci_prop_set_rfreg_cmd {
@@ -25,23 +22,6 @@ struct nci_prop_set_rfreg_rsp {
__u8 status;
 };
 
-#define NCI_PROP_GET_RFREG_VER 0x24
-
-struct nci_prop_get_rfreg_ver_rsp {
-   __u8 status;
-   __u8 data[8];
-};
-
-#define NCI_PROP_SET_RFREG_VER 0x25
-
-struct nci_prop_set_rfreg_ver_cmd {
-   __u8 data[8];
-};
-
-struct nci_prop_set_rfreg_ver_rsp {
-   __u8 status;
-};
-
 #define NCI_PROP_START_RFREG   0x26
 
 struct nci_prop_start_rfreg_rsp {
@@ -70,8 +50,6 @@ struct nci_prop_fw_cfg_rsp {
__u8 status;
 };
 
-#define NCI_PROP_WR_RESET  0x2f
-
 void s3fwrn5_nci_get_prop_ops(struct nci_driver_ops **ops, size_t *n);
 int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name);
 
-- 
2.17.1

[PATCH v2 net-next 1/2] nfc: s3fwrn5: Remove the delay for NFC sleep

[Bongsu Jeon]

From: Bongsu Jeon 

Remove the delay for NFC sleep because the delay is only needed to
guarantee that the NFC is awake.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/phy_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
index 497b02b30ae7..81318478d5fd 100644
--- a/drivers/nfc/s3fwrn5/phy_common.c
+++ b/drivers/nfc/s3fwrn5/phy_common.c
@@ -20,7 +20,8 @@ void s3fwrn5_phy_set_wake(void *phy_id, bool wake)
 
mutex_lock(>mutex);
gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME);
+   if (wake)
+   msleep(S3FWRN5_EN_WAIT_TIME);
mutex_unlock(>mutex);
 }
 EXPORT_SYMBOL(s3fwrn5_phy_set_wake);
-- 
2.17.1

[PATCH v2 net-next 0/2] nfc: s3fwrn5: Refactor the s3fwrn5 module

[Bongsu Jeon]

From: Bongsu Jeon 

Refactor the s3fwrn5 module.

1/2 is to remove the unneeded delay for NFC sleep.
2/2 is to remove the unused NCI prop commands.

ChangeLog:
 v2:
  - Update the commit messages.

Bongsu Jeon (2):
  nfc: s3fwrn5: Remove the delay for NFC sleep
  nfc: s3fwrn5: Remove unused NCI prop commands

 drivers/nfc/s3fwrn5/nci.c| 25 -
 drivers/nfc/s3fwrn5/nci.h| 22 --
 drivers/nfc/s3fwrn5/phy_common.c |  3 ++-
 3 files changed, 2 insertions(+), 48 deletions(-)

-- 
2.17.1

Re: [PATCH net-next] nfc: s3fwrn5: Remove unused nci prop commands

[Bongsu Jeon]

On Tue, Dec 15, 2020 at 1:02 AM Krzysztof Kozlowski  wrote:
>
> On Mon, Dec 14, 2020 at 08:46:58PM +0900, Bongsu Jeon wrote:
> > From: Bongsu Jeon 
> >
> > remove the unused nci prop commands that samsung driver doesn't use.
>
> Don't send patches one-by-one, but group them in a patchset.
>
> Previous comments apply here as well - NCI is acronym, start with
> capital letter.
>
> Best regards,
> Krzysztof
>
I will update the comments and send the patches in a series.

Re: [linux-nfc] [PATCH net-next] MAINTAINERS: Update maintainer for SAMSUNG S3FWRN5 NFC

[Bongsu Jeon]

On Tue, Dec 15, 2020 at 12:44 AM Krzysztof Kozlowski  wrote:
>
> On Mon, Dec 14, 2020 at 09:28:23PM +0900, Bongsu Jeon wrote:
> > From: Bongsu Jeon 
> >
> > add an email to look after the SAMSUNG NFC driver.
>
> Hi Bongsu,
>
> Review and testing is always appreciated. However before adding an entry
> to Maintainers, I would prefer to see some activity in maintainer-like
> tasks. So far there are none:
> https://lore.kernel.org/lkml/?q=f%3A%22Bongsu+Jeon%22
>
> Contributing patches is not the same as maintenance. Please subscribe to
> relevant mailing lists and devote effort for improving other people
> code.
>
> We had too many maintainers from many companies which did not perform
> actual maintainership for long time and clearly that's not the point.
>
> Best regards,
> Krzysztof

Ok, I  understand it.

[PATCH net-next] MAINTAINERS: Update maintainer for SAMSUNG S3FWRN5 NFC

[Bongsu Jeon]

From: Bongsu Jeon 

add an email to look after the SAMSUNG NFC driver.

Signed-off-by: Bongsu Jeon 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c1a6ba5ef26..cb1634eb010d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15425,6 +15425,7 @@ F:  include/media/drv-intf/s3c_camif.h
 SAMSUNG S3FWRN5 NFC DRIVER
 M: Krzysztof Kozlowski 
 M: Krzysztof Opasiak 
+M: Bongsu Jeon 
 L: linux-...@lists.01.org (moderated for non-subscribers)
 S: Maintained
 F: Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
-- 
2.17.1

[PATCH net-next] nfc: s3fwrn5: Remove unused nci prop commands

[Bongsu Jeon]

From: Bongsu Jeon 

remove the unused nci prop commands that samsung driver doesn't use.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/nci.c | 25 -
 drivers/nfc/s3fwrn5/nci.h | 22 --
 2 files changed, 47 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/nci.c b/drivers/nfc/s3fwrn5/nci.c
index 103bf5c92bdc..f042d3eaf8f6 100644
--- a/drivers/nfc/s3fwrn5/nci.c
+++ b/drivers/nfc/s3fwrn5/nci.c
@@ -21,31 +21,11 @@ static int s3fwrn5_nci_prop_rsp(struct nci_dev *ndev, 
struct sk_buff *skb)
 }
 
 static struct nci_driver_ops s3fwrn5_nci_prop_ops[] = {
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_AGAIN),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_GET_RFREG),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
{
.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
NCI_PROP_SET_RFREG),
.rsp = s3fwrn5_nci_prop_rsp,
},
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_GET_RFREG_VER),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_SET_RFREG_VER),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
{
.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
NCI_PROP_START_RFREG),
@@ -61,11 +41,6 @@ static struct nci_driver_ops s3fwrn5_nci_prop_ops[] = {
NCI_PROP_FW_CFG),
.rsp = s3fwrn5_nci_prop_rsp,
},
-   {
-   .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
-   NCI_PROP_WR_RESET),
-   .rsp = s3fwrn5_nci_prop_rsp,
-   },
 };
 
 void s3fwrn5_nci_get_prop_ops(struct nci_driver_ops **ops, size_t *n)
diff --git a/drivers/nfc/s3fwrn5/nci.h b/drivers/nfc/s3fwrn5/nci.h
index 23c0b28f247a..a80f0fb082a8 100644
--- a/drivers/nfc/s3fwrn5/nci.h
+++ b/drivers/nfc/s3fwrn5/nci.h
@@ -11,9 +11,6 @@
 
 #include "s3fwrn5.h"
 
-#define NCI_PROP_AGAIN 0x01
-
-#define NCI_PROP_GET_RFREG 0x21
 #define NCI_PROP_SET_RFREG 0x22
 
 struct nci_prop_set_rfreg_cmd {
@@ -25,23 +22,6 @@ struct nci_prop_set_rfreg_rsp {
__u8 status;
 };
 
-#define NCI_PROP_GET_RFREG_VER 0x24
-
-struct nci_prop_get_rfreg_ver_rsp {
-   __u8 status;
-   __u8 data[8];
-};
-
-#define NCI_PROP_SET_RFREG_VER 0x25
-
-struct nci_prop_set_rfreg_ver_cmd {
-   __u8 data[8];
-};
-
-struct nci_prop_set_rfreg_ver_rsp {
-   __u8 status;
-};
-
 #define NCI_PROP_START_RFREG   0x26
 
 struct nci_prop_start_rfreg_rsp {
@@ -70,8 +50,6 @@ struct nci_prop_fw_cfg_rsp {
__u8 status;
 };
 
-#define NCI_PROP_WR_RESET  0x2f
-
 void s3fwrn5_nci_get_prop_ops(struct nci_driver_ops **ops, size_t *n);
 int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name);
 
-- 
2.17.1

[PATCH net-next] nfc: s3fwrn5: Remove the delay for nfc sleep

[Bongsu Jeon]

From: Bongsu Jeon 

remove the delay for nfc sleep because nfc doesn't need the sleep delay.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/phy_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
index 497b02b30ae7..81318478d5fd 100644
--- a/drivers/nfc/s3fwrn5/phy_common.c
+++ b/drivers/nfc/s3fwrn5/phy_common.c
@@ -20,7 +20,8 @@ void s3fwrn5_phy_set_wake(void *phy_id, bool wake)
 
mutex_lock(>mutex);
gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME);
+   if (wake)
+   msleep(S3FWRN5_EN_WAIT_TIME);
mutex_unlock(>mutex);
 }
 EXPORT_SYMBOL(s3fwrn5_phy_set_wake);
-- 
2.17.1

[PATCH net-next] nfc: s3fwrn5: Release the nfc firmware

[Bongsu Jeon]

From: Bongsu Jeon 

add the code to release the nfc firmware when the firmware image size is
wrong.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/firmware.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/firmware.c b/drivers/nfc/s3fwrn5/firmware.c
index 4b5352e2b915..b70737b3070e 100644
--- a/drivers/nfc/s3fwrn5/firmware.c
+++ b/drivers/nfc/s3fwrn5/firmware.c
@@ -293,8 +293,10 @@ int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info 
*fw_info)
if (ret < 0)
return ret;
 
-   if (fw->fw->size < S3FWRN5_FW_IMAGE_HEADER_SIZE)
+   if (fw->fw->size < S3FWRN5_FW_IMAGE_HEADER_SIZE) {
+   release_firmware(fw->fw);
return -EINVAL;
+   }
 
memcpy(fw->date, fw->fw->data + 0x00, 12);
fw->date[12] = '\0';
-- 
2.17.1

[PATCH v2 net-next 2/2] nfc: s3fwrn5: Remove hard coded interrupt trigger type from the i2c module

[Bongsu Jeon]

From: Bongsu Jeon 

For the flexible control of interrupt trigger type, remove the hard coded
interrupt trigger type in the i2c module. The trigger type will be loaded
 from a dts.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/i2c.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index e1bdde105f24..42f1f610ac2c 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -179,6 +179,8 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
  const struct i2c_device_id *id)
 {
struct s3fwrn5_i2c_phy *phy;
+   struct irq_data *irq_data;
+   unsigned long irqflags;
int ret;
 
phy = devm_kzalloc(>dev, sizeof(*phy), GFP_KERNEL);
@@ -212,8 +214,11 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
if (ret < 0)
return ret;
 
+   irq_data = irq_get_irq_data(client->irq);
+   irqflags = irqd_get_trigger_type(irq_data) | IRQF_ONESHOT;
+
ret = devm_request_threaded_irq(>dev, phy->i2c_dev->irq, NULL,
-   s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+   s3fwrn5_i2c_irq_thread_fn, irqflags,
S3FWRN5_I2C_DRIVER_NAME, phy);
if (ret)
s3fwrn5_remove(phy->common.ndev);
-- 
2.17.1

[PATCH v2 net-next 1/2] dt-bindings: net: nfc: s3fwrn5: Change I2C interrupt trigger type

[Bongsu Jeon]

From: Bongsu Jeon 

Change interrupt trigger from IRQ_TYPE_LEVEL_HIGH to IRQ_TYPE_EDGE_RISING
 for stable NFC I2C interrupt handling.
Samsung's NFC Firmware sends an i2c frame as below.
1. NFC Firmware sets the GPIO(interrupt pin) high when there is an i2c
 frame to send.
2. If the CPU's I2C master has received the i2c frame, NFC F/W sets the
GPIO low.
NFC driver's i2c interrupt handler would be called in the abnormal case
as the NFC FW task of number 2 is delayed because of other high priority
tasks.
In that case, NFC driver will try to receive the i2c frame but there isn't
 any i2c frame to send in NFC.
It would cause an I2C communication problem. This case would hardly happen.
But, I changed the interrupt as a defense code.
If Driver uses the TRIGGER_RISING instead of the LEVEL trigger,
there would be no problem even if the NFC FW task is delayed.

Signed-off-by: Bongsu Jeon 
---
 Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml 
b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index ca3904bf90e0..477066e2b821 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -76,7 +76,7 @@ examples:
 reg = <0x27>;
 
 interrupt-parent = <>;
-interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
+interrupts = <3 IRQ_TYPE_EDGE_RISING>;
 
 en-gpios = < 4 GPIO_ACTIVE_HIGH>;
 wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
-- 
2.17.1

[PATCH v2 net-next 0/2] nfc: s3fwrn5: Change I2C interrupt trigger to EDGE_RISING

[Bongsu Jeon]

From: Bongsu Jeon 

For stable Samsung's I2C interrupt handling, I changed the interrupt 
trigger from IRQ_TYPE_LEVEL_HIGH to IRQ_TYPE_EDGE_RISING and removed 
the hard coded interrupt trigger type in the i2c module for the flexible 
control.

1/2 is the changed dt binding for the edge rising trigger.
2/2 is to remove the hard coded interrupt trigger type in the i2c module.

ChangeLog:
 v2:
  2/2
   - remove the hard coded interrupt trigger type.

Bongsu Jeon (2):
  dt-bindings: net: nfc: s3fwrn5: Change I2C interrupt trigger to
EDGE_RISING
  nfc: s3fwrn5: Remove hard coded interrupt trigger type from the i2c
module

 .../devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml  | 2 +-
 drivers/nfc/s3fwrn5/i2c.c | 8 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.17.1

Re: [PATCH net-next] nfc: s3fwrn5: Change irqflags

[Bongsu Jeon]

On Mon, Dec 7, 2020 at 11:13 PM Krzysztof Kozlowski  wrote:
>
> On Mon, Dec 07, 2020 at 10:39:01PM +0900, Bongsu Jeon wrote:
> > On Mon, Dec 7, 2020 at 8:51 PM Krzysztof Kozlowski  wrote:
> > >
> > > On Mon, Dec 07, 2020 at 08:38:27PM +0900, Bongsu Jeon wrote:
> > > > From: Bongsu Jeon 
> > > >
> > > > change irqflags from IRQF_TRIGGER_HIGH to IRQF_TRIGGER_RISING for stable
> > > > Samsung's nfc interrupt handling.
> > >
> > > 1. Describe in commit title/subject the change. Just a word "change 
> > > irqflags" is
> > >not enough.
> > >
> > Ok. I'll update it.
> >
> > > 2. Describe in commit message what you are trying to fix. Before was not
> > >stable? The "for stable interrupt handling" is a little bit vauge.
> > >
> > Usually, Samsung's NFC Firmware sends an i2c frame as below.
> >
> > 1. NFC Firmware sets the gpio(interrupt pin) high when there is an i2c
> > frame to send.
> > 2. If the CPU's I2C master has received the i2c frame, NFC F/W sets
> > the gpio low.
> >
> > NFC driver's i2c interrupt handler would be called in the abnormal case
> > as the NFC F/W task of number 2 is delayed because of other high
> > priority tasks.
> > In that case, NFC driver will try to receive the i2c frame but there
> > isn't any i2c frame
> > to send in NFC. It would cause an I2C communication problem.
> > This case would hardly happen.
> > But, I changed the interrupt as a defense code.
> > If Driver uses the TRIGGER_RISING not LEVEL trigger, there would be no 
> > problem
> > even if the NFC F/W task is delayed.
>
> All this should be explained in commit message, not in the email.
>
Okay.  I will

> >
> > > 3. This is contradictory to the bindings and current DTS. I think the
> > >driver should not force the specific trigger type because I could
> > >imagine some configuration that the actual interrupt to the CPU is
> > >routed differently.
> > >
> > >Instead, how about removing the trigger flags here and fixing the DTS
> > >and bindings example?
> > >
> >
> > As I mentioned before,
> > I changed this code because of Samsung NFC's I2C Communication way.
> > So, I think that it is okay for the nfc driver to force the specific
> > trigger type( EDGE_RISING).
> >
> > What do you think about it?
>
> Some different chip or some different hardware implementation could have
> the signal inverted, e.g. edge falling, not rising. This is rather
> a theoretical scenario but still such change makes the code more
> generic, configurable with DTS. Therefore trigger mode should be
> configured via DTS, not enforced by the driver.
>
Okay. I understand it.

> Best regards,
> Krzysztof

Re: [PATCH net-next] nfc: s3fwrn5: Change irqflags

[Bongsu Jeon]

On Mon, Dec 7, 2020 at 8:51 PM Krzysztof Kozlowski  wrote:
>
> On Mon, Dec 07, 2020 at 08:38:27PM +0900, Bongsu Jeon wrote:
> > From: Bongsu Jeon 
> >
> > change irqflags from IRQF_TRIGGER_HIGH to IRQF_TRIGGER_RISING for stable
> > Samsung's nfc interrupt handling.
>
> 1. Describe in commit title/subject the change. Just a word "change irqflags" 
> is
>not enough.
>
Ok. I'll update it.

> 2. Describe in commit message what you are trying to fix. Before was not
>stable? The "for stable interrupt handling" is a little bit vauge.
>
Usually, Samsung's NFC Firmware sends an i2c frame as below.

1. NFC Firmware sets the gpio(interrupt pin) high when there is an i2c
frame to send.
2. If the CPU's I2C master has received the i2c frame, NFC F/W sets
the gpio low.

NFC driver's i2c interrupt handler would be called in the abnormal case
as the NFC F/W task of number 2 is delayed because of other high
priority tasks.
In that case, NFC driver will try to receive the i2c frame but there
isn't any i2c frame
to send in NFC. It would cause an I2C communication problem.
This case would hardly happen.
But, I changed the interrupt as a defense code.
If Driver uses the TRIGGER_RISING not LEVEL trigger, there would be no problem
even if the NFC F/W task is delayed.

> 3. This is contradictory to the bindings and current DTS. I think the
>driver should not force the specific trigger type because I could
>imagine some configuration that the actual interrupt to the CPU is
>routed differently.
>
>Instead, how about removing the trigger flags here and fixing the DTS
>and bindings example?
>

As I mentioned before,
I changed this code because of Samsung NFC's I2C Communication way.
So, I think that it is okay for the nfc driver to force the specific
trigger type( EDGE_RISING).

What do you think about it?

> Best regards,
> Krzysztof
>
> >
> > Signed-off-by: Bongsu Jeon 
> > ---
> >  drivers/nfc/s3fwrn5/i2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> > index e1bdde105f24..016f6b6df849 100644
> > --- a/drivers/nfc/s3fwrn5/i2c.c
> > +++ b/drivers/nfc/s3fwrn5/i2c.c
> > @@ -213,7 +213,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
> >   return ret;
> >
> >   ret = devm_request_threaded_irq(>dev, phy->i2c_dev->irq, NULL,
> > - s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > + s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> >   S3FWRN5_I2C_DRIVER_NAME, phy);
> >   if (ret)
> >   s3fwrn5_remove(phy->common.ndev);
> > --
> > 2.17.1
> >

[PATCH net-next] nfc: s3fwrn5: Change irqflags

[Bongsu Jeon]

From: Bongsu Jeon 

change irqflags from IRQF_TRIGGER_HIGH to IRQF_TRIGGER_RISING for stable
Samsung's nfc interrupt handling.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index e1bdde105f24..016f6b6df849 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -213,7 +213,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
return ret;
 
ret = devm_request_threaded_irq(>dev, phy->i2c_dev->irq, NULL,
-   s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+   s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_RISING | IRQF_ONESHOT,
S3FWRN5_I2C_DRIVER_NAME, phy);
if (ret)
s3fwrn5_remove(phy->common.ndev);
-- 
2.17.1

[PATCH v2 net-next] nfc: s3fwrn5: skip the NFC bootloader mode

[Bongsu Jeon]

From: Bongsu Jeon 

If there isn't a proper NFC firmware image, Bootloader mode will be
skipped.

Signed-off-by: Bongsu Jeon 
---

 ChangeLog:
  v2:
   - change the commit message.
   - change the skip handling code.

 drivers/nfc/s3fwrn5/core.c | 23 +--
 drivers/nfc/s3fwrn5/firmware.c | 11 +--
 drivers/nfc/s3fwrn5/firmware.h |  1 +
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index f8e5d78d9078..c00b7a07c3ee 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -20,13 +20,26 @@
NFC_PROTO_ISO14443_B_MASK | \
NFC_PROTO_ISO15693_MASK)
 
+static int s3fwrn5_firmware_init(struct s3fwrn5_info *info)
+{
+   struct s3fwrn5_fw_info *fw_info = >fw_info;
+   int ret;
+
+   s3fwrn5_fw_init(fw_info, "sec_s3fwrn5_firmware.bin");
+
+   /* Get firmware data */
+   ret = s3fwrn5_fw_request_firmware(fw_info);
+   if (ret < 0)
+   dev_err(_info->ndev->nfc_dev->dev,
+   "Failed to get fw file, ret=%02x\n", ret);
+   return ret;
+}
+
 static int s3fwrn5_firmware_update(struct s3fwrn5_info *info)
 {
bool need_update;
int ret;
 
-   s3fwrn5_fw_init(>fw_info, "sec_s3fwrn5_firmware.bin");
-
/* Update firmware */
 
s3fwrn5_set_wake(info, false);
@@ -109,6 +122,12 @@ static int s3fwrn5_nci_post_setup(struct nci_dev *ndev)
struct s3fwrn5_info *info = nci_get_drvdata(ndev);
int ret;
 
+   if (s3fwrn5_firmware_init(info)) {
+   //skip bootloader mode
+   ret = 0;
+   goto out;
+   }
+
ret = s3fwrn5_firmware_update(info);
if (ret < 0)
goto out;
diff --git a/drivers/nfc/s3fwrn5/firmware.c b/drivers/nfc/s3fwrn5/firmware.c
index 4cde6dd5c019..4b5352e2b915 100644
--- a/drivers/nfc/s3fwrn5/firmware.c
+++ b/drivers/nfc/s3fwrn5/firmware.c
@@ -280,7 +280,7 @@ static int s3fwrn5_fw_complete_update_mode(struct 
s3fwrn5_fw_info *fw_info)
 
 #define S3FWRN5_FW_IMAGE_HEADER_SIZE 44
 
-static int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info *fw_info)
+int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info *fw_info)
 {
struct s3fwrn5_fw_image *fw = _info->fw;
u32 sig_off;
@@ -358,15 +358,6 @@ int s3fwrn5_fw_setup(struct s3fwrn5_fw_info *fw_info)
struct s3fwrn5_fw_cmd_get_bootinfo_rsp bootinfo;
int ret;
 
-   /* Get firmware data */
-
-   ret = s3fwrn5_fw_request_firmware(fw_info);
-   if (ret < 0) {
-   dev_err(_info->ndev->nfc_dev->dev,
-   "Failed to get fw file, ret=%02x\n", ret);
-   return ret;
-   }
-
/* Get bootloader info */
 
ret = s3fwrn5_fw_get_bootinfo(fw_info, );
diff --git a/drivers/nfc/s3fwrn5/firmware.h b/drivers/nfc/s3fwrn5/firmware.h
index 3c83e6730d30..3a82ce5837fb 100644
--- a/drivers/nfc/s3fwrn5/firmware.h
+++ b/drivers/nfc/s3fwrn5/firmware.h
@@ -89,6 +89,7 @@ struct s3fwrn5_fw_info {
char parity;
 };
 
+int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info *fw_info);
 void s3fwrn5_fw_init(struct s3fwrn5_fw_info *fw_info, const char *fw_name);
 int s3fwrn5_fw_setup(struct s3fwrn5_fw_info *fw_info);
 bool s3fwrn5_fw_check_version(const struct s3fwrn5_fw_info *fw_info, u32 
version);
-- 
2.17.1

[PATCH net-next] nfc: s3fwrn5: skip the NFC bootloader mode

[Bongsu Jeon]

From: Bongsu Jeon 

If there isn't proper NFC firmware image,
Bootloader mode will be skipped.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/core.c | 44 --
 drivers/nfc/s3fwrn5/firmware.c | 11 +
 drivers/nfc/s3fwrn5/firmware.h |  1 +
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index f8e5d78d9078..df89bc5d7338 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -20,13 +20,26 @@
NFC_PROTO_ISO14443_B_MASK | \
NFC_PROTO_ISO15693_MASK)
 
+static int s3fwrn5_firmware_init(struct s3fwrn5_info *info)
+{
+   struct s3fwrn5_fw_info *fw_info = >fw_info;
+   int ret;
+
+   s3fwrn5_fw_init(fw_info, "sec_s3fwrn5_firmware.bin");
+
+   /* Get firmware data */
+   ret = s3fwrn5_fw_request_firmware(fw_info);
+   if (ret < 0)
+   dev_err(_info->ndev->nfc_dev->dev,
+   "Failed to get fw file, ret=%02x\n", ret);
+   return ret;
+}
+
 static int s3fwrn5_firmware_update(struct s3fwrn5_info *info)
 {
bool need_update;
int ret;
 
-   s3fwrn5_fw_init(>fw_info, "sec_s3fwrn5_firmware.bin");
-
/* Update firmware */
 
s3fwrn5_set_wake(info, false);
@@ -109,21 +122,26 @@ static int s3fwrn5_nci_post_setup(struct nci_dev *ndev)
struct s3fwrn5_info *info = nci_get_drvdata(ndev);
int ret;
 
-   ret = s3fwrn5_firmware_update(info);
-   if (ret < 0)
-   goto out;
+   if (s3fwrn5_firmware_init(info) == 0) {
+   ret = s3fwrn5_firmware_update(info);
+   if (ret < 0)
+   goto out;
 
-   /* NCI core reset */
-
-   s3fwrn5_set_mode(info, S3FWRN5_MODE_NCI);
-   s3fwrn5_set_wake(info, true);
+   /* NCI core reset */
 
-   ret = nci_core_reset(info->ndev);
-   if (ret < 0)
-   goto out;
+   s3fwrn5_set_mode(info, S3FWRN5_MODE_NCI);
+   s3fwrn5_set_wake(info, true);
 
-   ret = nci_core_init(info->ndev);
+   ret = nci_core_reset(info->ndev);
+   if (ret < 0)
+   goto out;
 
+   ret = nci_core_init(info->ndev);
+   } else {
+   dev_info(>ndev->nfc_dev->dev,
+"skip bootloader mode\n");
+   ret = 0;
+   }
 out:
return ret;
 }
diff --git a/drivers/nfc/s3fwrn5/firmware.c b/drivers/nfc/s3fwrn5/firmware.c
index 4cde6dd5c019..4b5352e2b915 100644
--- a/drivers/nfc/s3fwrn5/firmware.c
+++ b/drivers/nfc/s3fwrn5/firmware.c
@@ -280,7 +280,7 @@ static int s3fwrn5_fw_complete_update_mode(struct 
s3fwrn5_fw_info *fw_info)
 
 #define S3FWRN5_FW_IMAGE_HEADER_SIZE 44
 
-static int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info *fw_info)
+int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info *fw_info)
 {
struct s3fwrn5_fw_image *fw = _info->fw;
u32 sig_off;
@@ -358,15 +358,6 @@ int s3fwrn5_fw_setup(struct s3fwrn5_fw_info *fw_info)
struct s3fwrn5_fw_cmd_get_bootinfo_rsp bootinfo;
int ret;
 
-   /* Get firmware data */
-
-   ret = s3fwrn5_fw_request_firmware(fw_info);
-   if (ret < 0) {
-   dev_err(_info->ndev->nfc_dev->dev,
-   "Failed to get fw file, ret=%02x\n", ret);
-   return ret;
-   }
-
/* Get bootloader info */
 
ret = s3fwrn5_fw_get_bootinfo(fw_info, );
diff --git a/drivers/nfc/s3fwrn5/firmware.h b/drivers/nfc/s3fwrn5/firmware.h
index 3c83e6730d30..3a82ce5837fb 100644
--- a/drivers/nfc/s3fwrn5/firmware.h
+++ b/drivers/nfc/s3fwrn5/firmware.h
@@ -89,6 +89,7 @@ struct s3fwrn5_fw_info {
char parity;
 };
 
+int s3fwrn5_fw_request_firmware(struct s3fwrn5_fw_info *fw_info);
 void s3fwrn5_fw_init(struct s3fwrn5_fw_info *fw_info, const char *fw_name);
 int s3fwrn5_fw_setup(struct s3fwrn5_fw_info *fw_info);
 bool s3fwrn5_fw_check_version(const struct s3fwrn5_fw_info *fw_info, u32 
version);
-- 
2.17.1

[PATCH net-next v4] net/nfc/nci: Support NCI 2.x initial sequence

[Bongsu Jeon]

From: Bongsu Jeon 

implement the NCI 2.x initial sequence to support NCI 2.x NFCC.
Since NCI 2.0, CORE_RESET and CORE_INIT sequence have been changed.
If NFCEE supports NCI 2.x, then NCI 2.x initial sequence will work.

In NCI 1.0, Initial sequence and payloads are as below:
(DH) (NFCC)
 |  -- CORE_RESET_CMD --> |
 |  <-- CORE_RESET_RSP -- |
 |  -- CORE_INIT_CMD -->  |
 |  <-- CORE_INIT_RSP --  |
 CORE_RESET_RSP payloads are Status, NCI version, Configuration Status.
 CORE_INIT_CMD payloads are empty.
 CORE_INIT_RSP payloads are Status, NFCC Features,
Number of Supported RF Interfaces, Supported RF Interface,
Max Logical Connections, Max Routing table Size,
Max Control Packet Payload Size, Max Size for Large Parameters,
Manufacturer ID, Manufacturer Specific Information.

In NCI 2.0, Initial Sequence and Parameters are as below:
(DH) (NFCC)
 |  -- CORE_RESET_CMD --> |
 |  <-- CORE_RESET_RSP -- |
 |  <-- CORE_RESET_NTF -- |
 |  -- CORE_INIT_CMD -->  |
 |  <-- CORE_INIT_RSP --  |
 CORE_RESET_RSP payloads are Status.
 CORE_RESET_NTF payloads are Reset Trigger,
Configuration Status, NCI Version, Manufacturer ID,
Manufacturer Specific Information Length,
Manufacturer Specific Information.
 CORE_INIT_CMD payloads are Feature1, Feature2.
 CORE_INIT_RSP payloads are Status, NFCC Features,
Max Logical Connections, Max Routing Table Size,
Max Control Packet Payload Size,
Max Data Packet Payload Size of the Static HCI Connection,
Number of Credits of the Static HCI Connection,
Max NFC-V RF Frame Size, Number of Supported RF Interfaces,
Supported RF Interfaces.

Signed-off-by: Bongsu Jeon 
---

 Changes in v4:
  - remove the unnecessary empty line.
  - pull out the common code.
  - change the unsigned char to u8.
  - order the variable declarations longest to shortest.

 Changes in v3:
  - remove the unused struct nci_core_reset_rsp_nci_ver2.
  - remove the __packed because of no need.
  - remove the unnecessary brackets.
  - change the asignment code for ndev->num_supported_rf_interfaces.

 Changes in v2:
  - fix the warning of type casting.
  - changed the __u8 type to unsigned char.

 include/net/nfc/nci.h | 34 ++
 net/nfc/nci/core.c| 18 --
 net/nfc/nci/ntf.c | 21 +++
 net/nfc/nci/rsp.c | 81 +++
 4 files changed, 138 insertions(+), 16 deletions(-)

diff --git a/include/net/nfc/nci.h b/include/net/nfc/nci.h
index 0550e0380b8d..e82f55f543bb 100644
--- a/include/net/nfc/nci.h
+++ b/include/net/nfc/nci.h
@@ -25,6 +25,8 @@
 #define NCI_MAX_PARAM_LEN  251
 #define NCI_MAX_PAYLOAD_SIZE   255
 #define NCI_MAX_PACKET_SIZE258
+#define NCI_MAX_LARGE_PARAMS_NCI_v215
+#define NCI_VER_2_MASK 0x20
 
 /* NCI Status Codes */
 #define NCI_STATUS_OK  0x00
@@ -131,6 +133,9 @@
 #define NCI_LF_CON_BITR_F_212  0x02
 #define NCI_LF_CON_BITR_F_424  0x04
 
+/* NCI 2.x Feature Enable Bit */
+#define NCI_FEATURE_DISABLE0x00
+
 /* NCI Reset types */
 #define NCI_RESET_TYPE_KEEP_CONFIG 0x00
 #define NCI_RESET_TYPE_RESET_CONFIG0x01
@@ -220,6 +225,11 @@ struct nci_core_reset_cmd {
 } __packed;
 
 #define NCI_OP_CORE_INIT_CMD   nci_opcode_pack(NCI_GID_CORE, 0x01)
+/* To support NCI 2.x */
+struct nci_core_init_v2_cmd {
+   u8  feature1;
+   u8  feature2;
+};
 
 #define NCI_OP_CORE_SET_CONFIG_CMD nci_opcode_pack(NCI_GID_CORE, 0x02)
 struct set_config_param {
@@ -334,6 +344,20 @@ struct nci_core_init_rsp_2 {
__le32  manufact_specific_info;
 } __packed;
 
+/* To support NCI ver 2.x */
+struct nci_core_init_rsp_nci_ver2 {
+   u8  status;
+   __le32  nfcc_features;
+   u8  max_logical_connections;
+   __le16  max_routing_table_size;
+   u8  max_ctrl_pkt_payload_len;
+   u8  max_data_pkt_hci_payload_len;
+   u8  number_of_hci_credit;
+   __le16  max_nfc_v_frame_size;
+   u8  num_supported_rf_interfaces;
+   u8  supported_rf_interfaces[];
+} __packed;
+
 #define NCI_OP_CORE_SET_CONFIG_RSP nci_opcode_pack(NCI_GID_CORE, 0x02)
 struct nci_core_set_config_rsp {
__u8status;
@@ -372,6 +396,16 @@ struct nci_nfcee_discover_rsp {
 /* --- */
 /*  NCI Notifications  */
 /* --- */
+#define NCI_OP_CORE_RESET_NTF  nci_opcode_pack(NCI_GID_CORE, 0x00)
+struct nci_core_reset_ntf {
+   u8  reset_trigger;
+   u8  config_status;
+   u8  nci_ver;
+   u8  manufact_id;
+   u8  manufacture

Re: [PATCH net-next v3] net/nfc/nci: Support NCI 2.x initial sequence

[Bongsu Jeon]

On Tue, Dec 1, 2020 at 11:48 AM Jakub Kicinski  wrote:
>
> On Fri, 27 Nov 2020 22:36:31 +0900 bongsu.je...@gmail.com wrote:
> > From: Bongsu Jeon 
> >
> > implement the NCI 2.x initial sequence to support NCI 2.x NFCC.
> > Since NCI 2.0, CORE_RESET and CORE_INIT sequence have been changed.
> > If NFCEE supports NCI 2.x, then NCI 2.x initial sequence will work.
> >
> > In NCI 1.0, Initial sequence and payloads are as below:
> > (DH) (NFCC)
> >  |  -- CORE_RESET_CMD --> |
> >  |  <-- CORE_RESET_RSP -- |
> >  |  -- CORE_INIT_CMD -->  |
> >  |  <-- CORE_INIT_RSP --  |
> >  CORE_RESET_RSP payloads are Status, NCI version, Configuration Status.
> >  CORE_INIT_CMD payloads are empty.
> >  CORE_INIT_RSP payloads are Status, NFCC Features,
> > Number of Supported RF Interfaces, Supported RF Interface,
> > Max Logical Connections, Max Routing table Size,
> > Max Control Packet Payload Size, Max Size for Large Parameters,
> > Manufacturer ID, Manufacturer Specific Information.
> >
> > In NCI 2.0, Initial Sequence and Parameters are as below:
> > (DH) (NFCC)
> >  |  -- CORE_RESET_CMD --> |
> >  |  <-- CORE_RESET_RSP -- |
> >  |  <-- CORE_RESET_NTF -- |
> >  |  -- CORE_INIT_CMD -->  |
> >  |  <-- CORE_INIT_RSP --  |
> >  CORE_RESET_RSP payloads are Status.
> >  CORE_RESET_NTF payloads are Reset Trigger,
> > Configuration Status, NCI Version, Manufacturer ID,
> > Manufacturer Specific Information Length,
> > Manufacturer Specific Information.
> >  CORE_INIT_CMD payloads are Feature1, Feature2.
> >  CORE_INIT_RSP payloads are Status, NFCC Features,
> > Max Logical Connections, Max Routing Table Size,
> > Max Control Packet Payload Size,
> > Max Data Packet Payload Size of the Static HCI Connection,
> > Number of Credits of the Static HCI Connection,
> > Max NFC-V RF Frame Size, Number of Supported RF Interfaces,
> > Supported RF Interfaces.
> >
> > Signed-off-by: Bongsu Jeon 
>
> >  static void nci_init_req(struct nci_dev *ndev, unsigned long opt)
> >  {
> > - nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, 0, NULL);
> > + struct nci_core_init_v2_cmd *cmd = (struct nci_core_init_v2_cmd *)opt;
> > +
> > + if (!cmd)
> > + nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, 0, NULL);
> > + else
> > + /* if nci version is 2.0, then use the feature parameters */
> > + nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD,
> > +  sizeof(struct nci_core_init_v2_cmd), cmd);
>
> This would be better written as:
>
> u8 plen = 0;
>
> if (opt)
> plen = sizeof(struct nci_core_init_v2_cmd);
>
> nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, plen, (void *)opt);
>
> > +
>
> unnecessary empty line
>
> >  }
> >
> >  static void nci_init_complete_req(struct nci_dev *ndev, unsigned long opt)
> > @@ -497,8 +505,18 @@ static int nci_open_device(struct nci_dev *ndev)
> >   }
> >
> >   if (!rc) {
> > - rc = __nci_request(ndev, nci_init_req, 0,
> > -msecs_to_jiffies(NCI_INIT_TIMEOUT));
> > + if (!(ndev->nci_ver & NCI_VER_2_MASK)) {
> > + rc = __nci_request(ndev, nci_init_req, 0,
> > +
> > msecs_to_jiffies(NCI_INIT_TIMEOUT));
> > + } else {
> > + struct nci_core_init_v2_cmd nci_init_v2_cmd;
> > +
> > + nci_init_v2_cmd.feature1 = NCI_FEATURE_DISABLE;
> > + nci_init_v2_cmd.feature2 = NCI_FEATURE_DISABLE;
> > +
> > + rc = __nci_request(ndev, nci_init_req, (unsigned 
> > long)_init_v2_cmd,
> > +
> > msecs_to_jiffies(NCI_INIT_TIMEOUT));
> > + }
>
> again please try to pull out the common code:
>
> struct nci_core_init_v2_cmd nci_init_v2_cmd = {
> .feature1 = NCI_FEATURE_DISABLE;
> .feature2 = NCI_FEATURE_DISABLE;
> };
> unsigned long opt = 0;
>
> if (ndev->nci_ver & NCI_VER_2_MASK)
> opt = (unsigned long)_init_v2_cmd;
>
> rc = __nci_request(ndev, nci_init_req, opt,
>msecs_to_jiffies(NCI_INIT_TIMEOUT));
>
>
> >   }
>
> > -static void nci_core_init_rsp_packet(struct nci_dev 

Re: [PATCH v5 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

On Thu, Dec 3, 2020 at 2:16 AM Krzysztof Kozlowski  wrote:
>
> On Wed, Dec 02, 2020 at 08:47:38PM +0900, Bongsu Jeon wrote:
> > From: Bongsu Jeon 
> >
> > Since S3FWRN82 NFC Chip, The UART interface can be used.
> > S3FWRN82 supports I2C and UART interface.
> >
> > Signed-off-by: Bongsu Jeon 
> > ---
> >  .../bindings/net/nfc/samsung,s3fwrn5.yaml  | 31 
> > +++---
> >  1 file changed, 28 insertions(+), 3 deletions(-)
> >
>
> Reviewed-by: Krzysztof Kozlowski 
>
> Best regards,
> Krzysztof

Thanks a lot for advising and reviewing my patches.

[PATCH v5 net-next 4/4] nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 uses NCI protocol and supports I2C and UART interface.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/Kconfig  |  12 +++
 drivers/nfc/s3fwrn5/Makefile |   2 +
 drivers/nfc/s3fwrn5/phy_common.c |  12 +++
 drivers/nfc/s3fwrn5/phy_common.h |   1 +
 drivers/nfc/s3fwrn5/uart.c   | 196 +++
 5 files changed, 223 insertions(+)
 create mode 100644 drivers/nfc/s3fwrn5/uart.c

diff --git a/drivers/nfc/s3fwrn5/Kconfig b/drivers/nfc/s3fwrn5/Kconfig
index 3f8b6da..8a6b1a7 100644
--- a/drivers/nfc/s3fwrn5/Kconfig
+++ b/drivers/nfc/s3fwrn5/Kconfig
@@ -20,3 +20,15 @@ config NFC_S3FWRN5_I2C
  To compile this driver as a module, choose m here. The module will
  be called s3fwrn5_i2c.ko.
  Say N if unsure.
+
+config NFC_S3FWRN82_UART
+tristate "Samsung S3FWRN82 UART support"
+depends on NFC_NCI && SERIAL_DEV_BUS
+select NFC_S3FWRN5
+help
+  This module adds support for a UART interface to the S3FWRN82 chip.
+  Select this if your platform is using the UART bus.
+
+  To compile this driver as a module, choose m here. The module will
+  be called s3fwrn82_uart.ko.
+  Say N if unsure.
diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index 6b6f52d..7da827a 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -5,6 +5,8 @@
 
 s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
 s3fwrn5_i2c-objs = i2c.o
+s3fwrn82_uart-objs = uart.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
 obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
+obj-$(CONFIG_NFC_S3FWRN82_UART) += s3fwrn82_uart.o
diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
index 5cad1f4..497b02b 100644
--- a/drivers/nfc/s3fwrn5/phy_common.c
+++ b/drivers/nfc/s3fwrn5/phy_common.c
@@ -47,6 +47,18 @@ bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum 
s3fwrn5_mode mode)
 }
 EXPORT_SYMBOL(s3fwrn5_phy_power_ctrl);
 
+void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode)
+{
+   struct phy_common *phy = phy_id;
+
+   mutex_lock(>mutex);
+
+   s3fwrn5_phy_power_ctrl(phy, mode);
+
+   mutex_unlock(>mutex);
+}
+EXPORT_SYMBOL(s3fwrn5_phy_set_mode);
+
 enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id)
 {
struct phy_common *phy = phy_id;
diff --git a/drivers/nfc/s3fwrn5/phy_common.h b/drivers/nfc/s3fwrn5/phy_common.h
index b98531d..99749c9 100644
--- a/drivers/nfc/s3fwrn5/phy_common.h
+++ b/drivers/nfc/s3fwrn5/phy_common.h
@@ -31,6 +31,7 @@ struct phy_common {
 
 void s3fwrn5_phy_set_wake(void *phy_id, bool wake);
 bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum s3fwrn5_mode mode);
+void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode);
 enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id);
 
 #endif /* __NFC_S3FWRN5_PHY_COMMON_H */
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
new file mode 100644
index 000..82ea35d
--- /dev/null
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UART Link Layer for S3FWRN82 NCI based Driver
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ * Robert Baldyga 
+ * Copyright (C) 2020 Samsung Electronics
+ * Bongsu Jeon 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "phy_common.h"
+
+#define S3FWRN82_NCI_HEADER 3
+#define S3FWRN82_NCI_IDX 2
+#define NCI_SKB_BUFF_LEN 258
+
+struct s3fwrn82_uart_phy {
+   struct phy_common common;
+   struct serdev_device *ser_dev;
+   struct sk_buff *recv_skb;
+};
+
+static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+   int err;
+
+   err = serdev_device_write(phy->ser_dev,
+ out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+   if (err < 0)
+   return err;
+
+   return 0;
+}
+
+static const struct s3fwrn5_phy_ops uart_phy_ops = {
+   .set_wake = s3fwrn5_phy_set_wake,
+   .set_mode = s3fwrn5_phy_set_mode,
+   .get_mode = s3fwrn5_phy_get_mode,
+   .write = s3fwrn82_uart_write,
+};
+
+static int s3fwrn82_uart_read(struct serdev_device *serdev,
+ const unsigned char *data,
+ size_t count)
+{
+   struct s3fwrn82_uart_phy *phy = serdev_device_get_drvdata(serdev);
+   size_t i;
+
+   for (i = 0; i < count; i++) {
+   skb_put_u8(phy->recv_skb, *data++);
+
+   if (phy->recv_skb->len < S3FWRN82_NCI_HEADER)
+   continue;
+
+   if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
+   < p

[PATCH v5 net-next 3/4] nfc: s3fwrn5: extract the common phy blocks

[Bongsu Jeon]

From: Bongsu Jeon 

Extract the common phy blocks to reuse it.
The UART module will use the common blocks.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/Makefile |   2 +-
 drivers/nfc/s3fwrn5/i2c.c| 117 +--
 drivers/nfc/s3fwrn5/phy_common.c |  63 +
 drivers/nfc/s3fwrn5/phy_common.h |  36 
 4 files changed, 139 insertions(+), 79 deletions(-)
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.h

diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index d0ffa35..6b6f52d 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -3,7 +3,7 @@
 # Makefile for Samsung S3FWRN5 NFC driver
 #
 
-s3fwrn5-objs = core.o firmware.o nci.o
+s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
 s3fwrn5_i2c-objs = i2c.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index 9a64eea..e1bdde1 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -15,75 +15,30 @@
 
 #include 
 
-#include "s3fwrn5.h"
+#include "phy_common.h"
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_EN_WAIT_TIME 20
-
 struct s3fwrn5_i2c_phy {
+   struct phy_common common;
struct i2c_client *i2c_dev;
-   struct nci_dev *ndev;
-
-   int gpio_en;
-   int gpio_fw_wake;
-
-   struct mutex mutex;
 
-   enum s3fwrn5_mode mode;
unsigned int irq_skip:1;
 };
 
-static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
-{
-   struct s3fwrn5_i2c_phy *phy = phy_id;
-
-   mutex_lock(>mutex);
-   gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   mutex_unlock(>mutex);
-}
-
 static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
 {
struct s3fwrn5_i2c_phy *phy = phy_id;
 
-   mutex_lock(>mutex);
+   mutex_lock(>common.mutex);
 
-   if (phy->mode == mode)
+   if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
goto out;
 
-   phy->mode = mode;
-
-   gpio_set_value(phy->gpio_en, 1);
-   gpio_set_value(phy->gpio_fw_wake, 0);
-   if (mode == S3FWRN5_MODE_FW)
-   gpio_set_value(phy->gpio_fw_wake, 1);
-
-   if (mode != S3FWRN5_MODE_COLD) {
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   gpio_set_value(phy->gpio_en, 0);
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   }
-
phy->irq_skip = true;
 
 out:
-   mutex_unlock(>mutex);
-}
-
-static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
-{
-   struct s3fwrn5_i2c_phy *phy = phy_id;
-   enum s3fwrn5_mode mode;
-
-   mutex_lock(>mutex);
-
-   mode = phy->mode;
-
-   mutex_unlock(>mutex);
-
-   return mode;
+   mutex_unlock(>common.mutex);
 }
 
 static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
@@ -91,7 +46,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
struct s3fwrn5_i2c_phy *phy = phy_id;
int ret;
 
-   mutex_lock(>mutex);
+   mutex_lock(>common.mutex);
 
phy->irq_skip = false;
 
@@ -102,7 +57,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
ret  = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
}
 
-   mutex_unlock(>mutex);
+   mutex_unlock(>common.mutex);
 
if (ret < 0)
return ret;
@@ -114,9 +69,9 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
 }
 
 static const struct s3fwrn5_phy_ops i2c_phy_ops = {
-   .set_wake = s3fwrn5_i2c_set_wake,
+   .set_wake = s3fwrn5_phy_set_wake,
.set_mode = s3fwrn5_i2c_set_mode,
-   .get_mode = s3fwrn5_i2c_get_mode,
+   .get_mode = s3fwrn5_phy_get_mode,
.write = s3fwrn5_i2c_write,
 };
 
@@ -128,7 +83,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
char hdr[4];
int ret;
 
-   hdr_size = (phy->mode == S3FWRN5_MODE_NCI) ?
+   hdr_size = (phy->common.mode == S3FWRN5_MODE_NCI) ?
NCI_CTRL_HDR_SIZE : S3FWRN5_FW_HDR_SIZE;
ret = i2c_master_recv(phy->i2c_dev, hdr, hdr_size);
if (ret < 0)
@@ -137,7 +92,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
if (ret < hdr_size)
return -EBADMSG;
 
-   data_len = (phy->mode == S3FWRN5_MODE_NCI) ?
+   data_len = (phy->common.mode == S3FWRN5_MODE_NCI) ?
((struct nci_ctrl_hdr *)hdr)->plen :
((struct s3fwrn5_fw_header *)hdr)->len;
 
@@ -157,24 +112,24 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
}
 
 out:
-   return s3fwrn5_recv_frame(phy->ndev, skb, phy->mode);
+   return s3fwrn5_recv_frame(phy->common.ndev

[PATCH v5 net-next 2/4] nfc: s3fwrn5: reduce the EN_WAIT_TIME

[Bongsu Jeon]

From: Bongsu Jeon 

The delay of 20ms is enough to enable and
wake up the Samsung's nfc chip.

Acked-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/i2c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index ae26594..9a64eea 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -19,7 +19,7 @@
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_EN_WAIT_TIME 150
+#define S3FWRN5_EN_WAIT_TIME 20
 
 struct s3fwrn5_i2c_phy {
struct i2c_client *i2c_dev;
@@ -40,7 +40,7 @@ static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
 
mutex_lock(>mutex);
gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME/2);
+   msleep(S3FWRN5_EN_WAIT_TIME);
mutex_unlock(>mutex);
 }
 
@@ -63,7 +63,7 @@ static void s3fwrn5_i2c_set_mode(void *phy_id, enum 
s3fwrn5_mode mode)
if (mode != S3FWRN5_MODE_COLD) {
msleep(S3FWRN5_EN_WAIT_TIME);
gpio_set_value(phy->gpio_en, 0);
-   msleep(S3FWRN5_EN_WAIT_TIME/2);
+   msleep(S3FWRN5_EN_WAIT_TIME);
}
 
phy->irq_skip = true;
-- 
1.9.1

[PATCH v5 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---
 .../bindings/net/nfc/samsung,s3fwrn5.yaml  | 31 +++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml 
b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index cb0b8a5..ca3904b 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -12,7 +12,9 @@ maintainers:
 
 properties:
   compatible:
-const: samsung,s3fwrn5-i2c
+enum:
+  - samsung,s3fwrn5-i2c
+  - samsung,s3fwrn82
 
   en-gpios:
 maxItems: 1
@@ -47,10 +49,19 @@ additionalProperties: false
 required:
   - compatible
   - en-gpios
-  - interrupts
-  - reg
   - wake-gpios
 
+allOf:
+  - if:
+  properties:
+compatible:
+  contains:
+const: samsung,s3fwrn5-i2c
+then:
+  required:
+- interrupts
+- reg
+
 examples:
   - |
 #include 
@@ -71,3 +82,17 @@ examples:
 wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
 };
 };
+  # UART example on Raspberry Pi
+  - |
+uart0 {
+status = "okay";
+
+nfc {
+compatible = "samsung,s3fwrn82";
+
+en-gpios = < 20 GPIO_ACTIVE_HIGH>;
+wake-gpios = < 16 GPIO_ACTIVE_HIGH>;
+
+status = "okay";
+};
+};
-- 
1.9.1

[PATCH v5 net-next 0/4] nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

S3FWRN82 is the Samsung's NFC chip that supports the UART communication.
Before adding the UART driver module, I did refactoring the s3fwrn5_i2c module 
to reuse the common blocks.

1/4 is the dt bindings for the RN82 UART interface.
2/4..3/4 are refactoring the s3fwrn5_i2c module.
4/4 is the UART driver module implementation.

ChangeLog:
 v5:
   1/4
- remove the 'items' of the compatible property.
- change the GPIO flags.
 v4:
   1/4
- change 'oneOf' to 'items'.
- fix the indentation.
   2/4
- add the ACK tag.
   4/4
- remove the of_match_ptr macro.
 v3:
   3/4
- move the phy_common object to s3fwrn.ko to avoid duplication.
- include the header files to include everything which is used inside.
- wrap the lines.
   4/4
- remove the kfree(phy) because of duplicated free.
- use the phy_common blocks.
- wrap lines properly.
 v2:
   1/4
- change the compatible name.
- change the const to enum for compatible.
- change the node name to nfc.
   3/4
- remove the common function's definition in common header file.
- make the common phy_common.c file to define the common function.
- wrap the lines.
- change the Header guard.
- remove the unused common function.

Bongsu Jeon (4):
  dt-bindings: net: nfc: s3fwrn5: Support a UART interface
  nfc: s3fwrn5: reduce the EN_WAIT_TIME
  nfc: s3fwrn5: extract the common phy blocks
  nfc: s3fwrn5: Support a UART interface

 .../bindings/net/nfc/samsung,s3fwrn5.yaml  |  31 +++-
 drivers/nfc/s3fwrn5/Kconfig|  12 ++
 drivers/nfc/s3fwrn5/Makefile   |   4 +-
 drivers/nfc/s3fwrn5/i2c.c  | 117 
 drivers/nfc/s3fwrn5/phy_common.c   |  75 
 drivers/nfc/s3fwrn5/phy_common.h   |  37 
 drivers/nfc/s3fwrn5/uart.c | 196 +
 7 files changed, 390 insertions(+), 82 deletions(-)
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
 create mode 100644 drivers/nfc/s3fwrn5/uart.c

-- 
1.9.1

[PATCH v4 net-next 3/4] nfc: s3fwrn5: extract the common phy blocks

[Bongsu Jeon]

From: Bongsu Jeon 

Extract the common phy blocks to reuse it.
The UART module will use the common blocks.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/Makefile |   2 +-
 drivers/nfc/s3fwrn5/i2c.c| 117 +--
 drivers/nfc/s3fwrn5/phy_common.c |  63 +
 drivers/nfc/s3fwrn5/phy_common.h |  36 
 4 files changed, 139 insertions(+), 79 deletions(-)
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.h

diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index d0ffa35..6b6f52d 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -3,7 +3,7 @@
 # Makefile for Samsung S3FWRN5 NFC driver
 #
 
-s3fwrn5-objs = core.o firmware.o nci.o
+s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
 s3fwrn5_i2c-objs = i2c.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index 9a64eea..e1bdde1 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -15,75 +15,30 @@
 
 #include 
 
-#include "s3fwrn5.h"
+#include "phy_common.h"
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_EN_WAIT_TIME 20
-
 struct s3fwrn5_i2c_phy {
+   struct phy_common common;
struct i2c_client *i2c_dev;
-   struct nci_dev *ndev;
-
-   int gpio_en;
-   int gpio_fw_wake;
-
-   struct mutex mutex;
 
-   enum s3fwrn5_mode mode;
unsigned int irq_skip:1;
 };
 
-static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
-{
-   struct s3fwrn5_i2c_phy *phy = phy_id;
-
-   mutex_lock(>mutex);
-   gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   mutex_unlock(>mutex);
-}
-
 static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
 {
struct s3fwrn5_i2c_phy *phy = phy_id;
 
-   mutex_lock(>mutex);
+   mutex_lock(>common.mutex);
 
-   if (phy->mode == mode)
+   if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
goto out;
 
-   phy->mode = mode;
-
-   gpio_set_value(phy->gpio_en, 1);
-   gpio_set_value(phy->gpio_fw_wake, 0);
-   if (mode == S3FWRN5_MODE_FW)
-   gpio_set_value(phy->gpio_fw_wake, 1);
-
-   if (mode != S3FWRN5_MODE_COLD) {
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   gpio_set_value(phy->gpio_en, 0);
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   }
-
phy->irq_skip = true;
 
 out:
-   mutex_unlock(>mutex);
-}
-
-static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
-{
-   struct s3fwrn5_i2c_phy *phy = phy_id;
-   enum s3fwrn5_mode mode;
-
-   mutex_lock(>mutex);
-
-   mode = phy->mode;
-
-   mutex_unlock(>mutex);
-
-   return mode;
+   mutex_unlock(>common.mutex);
 }
 
 static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
@@ -91,7 +46,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
struct s3fwrn5_i2c_phy *phy = phy_id;
int ret;
 
-   mutex_lock(>mutex);
+   mutex_lock(>common.mutex);
 
phy->irq_skip = false;
 
@@ -102,7 +57,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
ret  = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
}
 
-   mutex_unlock(>mutex);
+   mutex_unlock(>common.mutex);
 
if (ret < 0)
return ret;
@@ -114,9 +69,9 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
 }
 
 static const struct s3fwrn5_phy_ops i2c_phy_ops = {
-   .set_wake = s3fwrn5_i2c_set_wake,
+   .set_wake = s3fwrn5_phy_set_wake,
.set_mode = s3fwrn5_i2c_set_mode,
-   .get_mode = s3fwrn5_i2c_get_mode,
+   .get_mode = s3fwrn5_phy_get_mode,
.write = s3fwrn5_i2c_write,
 };
 
@@ -128,7 +83,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
char hdr[4];
int ret;
 
-   hdr_size = (phy->mode == S3FWRN5_MODE_NCI) ?
+   hdr_size = (phy->common.mode == S3FWRN5_MODE_NCI) ?
NCI_CTRL_HDR_SIZE : S3FWRN5_FW_HDR_SIZE;
ret = i2c_master_recv(phy->i2c_dev, hdr, hdr_size);
if (ret < 0)
@@ -137,7 +92,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
if (ret < hdr_size)
return -EBADMSG;
 
-   data_len = (phy->mode == S3FWRN5_MODE_NCI) ?
+   data_len = (phy->common.mode == S3FWRN5_MODE_NCI) ?
((struct nci_ctrl_hdr *)hdr)->plen :
((struct s3fwrn5_fw_header *)hdr)->len;
 
@@ -157,24 +112,24 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
}
 
 out:
-   return s3fwrn5_recv_frame(phy->ndev, skb, phy->mode);
+   return s3fwrn5_recv_frame(phy->common.ndev, skb, phy->common.mode);
 }
 

[PATCH v4 net-next 4/4] nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 uses NCI protocol and supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/Kconfig  |  12 +++
 drivers/nfc/s3fwrn5/Makefile |   2 +
 drivers/nfc/s3fwrn5/phy_common.c |  12 +++
 drivers/nfc/s3fwrn5/phy_common.h |   1 +
 drivers/nfc/s3fwrn5/uart.c   | 196 +++
 5 files changed, 223 insertions(+)
 create mode 100644 drivers/nfc/s3fwrn5/uart.c

diff --git a/drivers/nfc/s3fwrn5/Kconfig b/drivers/nfc/s3fwrn5/Kconfig
index 3f8b6da..8a6b1a7 100644
--- a/drivers/nfc/s3fwrn5/Kconfig
+++ b/drivers/nfc/s3fwrn5/Kconfig
@@ -20,3 +20,15 @@ config NFC_S3FWRN5_I2C
  To compile this driver as a module, choose m here. The module will
  be called s3fwrn5_i2c.ko.
  Say N if unsure.
+
+config NFC_S3FWRN82_UART
+tristate "Samsung S3FWRN82 UART support"
+depends on NFC_NCI && SERIAL_DEV_BUS
+select NFC_S3FWRN5
+help
+  This module adds support for a UART interface to the S3FWRN82 chip.
+  Select this if your platform is using the UART bus.
+
+  To compile this driver as a module, choose m here. The module will
+  be called s3fwrn82_uart.ko.
+  Say N if unsure.
diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index 6b6f52d..7da827a 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -5,6 +5,8 @@
 
 s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
 s3fwrn5_i2c-objs = i2c.o
+s3fwrn82_uart-objs = uart.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
 obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
+obj-$(CONFIG_NFC_S3FWRN82_UART) += s3fwrn82_uart.o
diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
index 5cad1f4..497b02b 100644
--- a/drivers/nfc/s3fwrn5/phy_common.c
+++ b/drivers/nfc/s3fwrn5/phy_common.c
@@ -47,6 +47,18 @@ bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum 
s3fwrn5_mode mode)
 }
 EXPORT_SYMBOL(s3fwrn5_phy_power_ctrl);
 
+void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode)
+{
+   struct phy_common *phy = phy_id;
+
+   mutex_lock(>mutex);
+
+   s3fwrn5_phy_power_ctrl(phy, mode);
+
+   mutex_unlock(>mutex);
+}
+EXPORT_SYMBOL(s3fwrn5_phy_set_mode);
+
 enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id)
 {
struct phy_common *phy = phy_id;
diff --git a/drivers/nfc/s3fwrn5/phy_common.h b/drivers/nfc/s3fwrn5/phy_common.h
index b98531d..99749c9 100644
--- a/drivers/nfc/s3fwrn5/phy_common.h
+++ b/drivers/nfc/s3fwrn5/phy_common.h
@@ -31,6 +31,7 @@ struct phy_common {
 
 void s3fwrn5_phy_set_wake(void *phy_id, bool wake);
 bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum s3fwrn5_mode mode);
+void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode);
 enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id);
 
 #endif /* __NFC_S3FWRN5_PHY_COMMON_H */
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
new file mode 100644
index 000..82ea35d
--- /dev/null
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UART Link Layer for S3FWRN82 NCI based Driver
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ * Robert Baldyga 
+ * Copyright (C) 2020 Samsung Electronics
+ * Bongsu Jeon 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "phy_common.h"
+
+#define S3FWRN82_NCI_HEADER 3
+#define S3FWRN82_NCI_IDX 2
+#define NCI_SKB_BUFF_LEN 258
+
+struct s3fwrn82_uart_phy {
+   struct phy_common common;
+   struct serdev_device *ser_dev;
+   struct sk_buff *recv_skb;
+};
+
+static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+   int err;
+
+   err = serdev_device_write(phy->ser_dev,
+ out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+   if (err < 0)
+   return err;
+
+   return 0;
+}
+
+static const struct s3fwrn5_phy_ops uart_phy_ops = {
+   .set_wake = s3fwrn5_phy_set_wake,
+   .set_mode = s3fwrn5_phy_set_mode,
+   .get_mode = s3fwrn5_phy_get_mode,
+   .write = s3fwrn82_uart_write,
+};
+
+static int s3fwrn82_uart_read(struct serdev_device *serdev,
+ const unsigned char *data,
+ size_t count)
+{
+   struct s3fwrn82_uart_phy *phy = serdev_device_get_drvdata(serdev);
+   size_t i;
+
+   for (i = 0; i < count; i++) {
+   skb_put_u8(phy->recv_skb, *data++);
+
+   if (phy->recv_skb->len < S3FWRN82_NCI_HEADER)
+   continue;
+
+   if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
+   < phy->recv_skb->data[S3FWRN82_NCI_IDX]

[PATCH v4 net-next 2/4] nfc: s3fwrn5: reduce the EN_WAIT_TIME

[Bongsu Jeon]

From: Bongsu Jeon 

The delay of 20ms is enough to enable and
wake up the Samsung's nfc chip.

Acked-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/i2c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index ae26594..9a64eea 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -19,7 +19,7 @@
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_EN_WAIT_TIME 150
+#define S3FWRN5_EN_WAIT_TIME 20
 
 struct s3fwrn5_i2c_phy {
struct i2c_client *i2c_dev;
@@ -40,7 +40,7 @@ static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
 
mutex_lock(>mutex);
gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME/2);
+   msleep(S3FWRN5_EN_WAIT_TIME);
mutex_unlock(>mutex);
 }
 
@@ -63,7 +63,7 @@ static void s3fwrn5_i2c_set_mode(void *phy_id, enum 
s3fwrn5_mode mode)
if (mode != S3FWRN5_MODE_COLD) {
msleep(S3FWRN5_EN_WAIT_TIME);
gpio_set_value(phy->gpio_en, 0);
-   msleep(S3FWRN5_EN_WAIT_TIME/2);
+   msleep(S3FWRN5_EN_WAIT_TIME);
}
 
phy->irq_skip = true;
-- 
1.9.1

[PATCH v4 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---
 .../bindings/net/nfc/samsung,s3fwrn5.yaml  | 32 --
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml 
b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index cb0b8a5..cc5f9a1 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -12,7 +12,10 @@ maintainers:
 
 properties:
   compatible:
-const: samsung,s3fwrn5-i2c
+items:
+  - enum:
+  - samsung,s3fwrn5-i2c
+  - samsung,s3fwrn82
 
   en-gpios:
 maxItems: 1
@@ -47,10 +50,19 @@ additionalProperties: false
 required:
   - compatible
   - en-gpios
-  - interrupts
-  - reg
   - wake-gpios
 
+allOf:
+  - if:
+  properties:
+compatible:
+  contains:
+const: samsung,s3fwrn5-i2c
+then:
+  required:
+- interrupts
+- reg
+
 examples:
   - |
 #include 
@@ -71,3 +83,17 @@ examples:
 wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
 };
 };
+  # UART example on Raspberry Pi
+  - |
+uart0 {
+status = "okay";
+
+nfc {
+compatible = "samsung,s3fwrn82";
+
+en-gpios = < 20 0>;
+wake-gpios = < 16 0>;
+
+status = "okay";
+};
+};
-- 
1.9.1

[PATCH v4 net-next 0/4] nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

S3FWRN82 is the Samsung's NFC chip that supports the UART communication.
Before adding the UART driver module, I did refactoring the s3fwrn5_i2c module 
to reuse the common blocks.

1/4 is the dt bindings for the RN82 UART interface.
2/4..3/4 are refactoring the s3fwrn5_i2c module.
4/4 is the UART driver module implementation.

ChangeLog:
 v4:
   1/4
- change 'oneOf' to 'items'.
- fix the indentation.
   2/4
- add the ACK tag.
   4/4
- remove the of_match_ptr macro.
 v3:
   3/4
- move the phy_common object to s3fwrn.ko to avoid duplication.
- include the header files to include everything which is used inside.
- wrap the lines.
   4/4
- remove the kfree(phy) because of duplicated free.
- use the phy_common blocks.
- wrap lines properly.
 v2:
   1/4
- change the compatible name.
- change the const to enum for compatible.
- change the node name to nfc.
   3/4
- remove the common function's definition in common header file.
- make the common phy_common.c file to define the common function.
- wrap the lines.
- change the Header guard.
- remove the unused common function.


Bongsu Jeon (4):
  dt-bindings: net: nfc: s3fwrn5: Support a UART interface
  nfc: s3fwrn5: reduce the EN_WAIT_TIME
  nfc: s3fwrn5: extract the common phy blocks
  nfc: s3fwrn5: Support a UART interface

 .../bindings/net/nfc/samsung,s3fwrn5.yaml  |  32 +++-
 drivers/nfc/s3fwrn5/Kconfig|  12 ++
 drivers/nfc/s3fwrn5/Makefile   |   4 +-
 drivers/nfc/s3fwrn5/i2c.c  | 117 
 drivers/nfc/s3fwrn5/phy_common.c   |  75 
 drivers/nfc/s3fwrn5/phy_common.h   |  37 
 drivers/nfc/s3fwrn5/uart.c | 196 +
 7 files changed, 391 insertions(+), 82 deletions(-)
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
 create mode 100644 drivers/nfc/s3fwrn5/uart.c

-- 
1.9.1

[PATCH v2 net-next 4/4] net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

From: Bongsu Jeon 

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 uses NCI protocol and supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---

 Changes in v2:
   - remove the kfree(phy) because of duplicated free.
   - use the phy_common blocks.
   - wrap lines properly.

 drivers/nfc/s3fwrn5/Kconfig  |  12 +++
 drivers/nfc/s3fwrn5/Makefile |   2 +
 drivers/nfc/s3fwrn5/phy_common.c |  12 +++
 drivers/nfc/s3fwrn5/phy_common.h |   1 +
 drivers/nfc/s3fwrn5/uart.c   | 197 +++
 5 files changed, 224 insertions(+)
 create mode 100644 drivers/nfc/s3fwrn5/uart.c

diff --git a/drivers/nfc/s3fwrn5/Kconfig b/drivers/nfc/s3fwrn5/Kconfig
index 3f8b6da..8a6b1a7 100644
--- a/drivers/nfc/s3fwrn5/Kconfig
+++ b/drivers/nfc/s3fwrn5/Kconfig
@@ -20,3 +20,15 @@ config NFC_S3FWRN5_I2C
  To compile this driver as a module, choose m here. The module will
  be called s3fwrn5_i2c.ko.
  Say N if unsure.
+
+config NFC_S3FWRN82_UART
+tristate "Samsung S3FWRN82 UART support"
+depends on NFC_NCI && SERIAL_DEV_BUS
+select NFC_S3FWRN5
+help
+  This module adds support for a UART interface to the S3FWRN82 chip.
+  Select this if your platform is using the UART bus.
+
+  To compile this driver as a module, choose m here. The module will
+  be called s3fwrn82_uart.ko.
+  Say N if unsure.
diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index 6b6f52d..7da827a 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -5,6 +5,8 @@
 
 s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
 s3fwrn5_i2c-objs = i2c.o
+s3fwrn82_uart-objs = uart.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
 obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
+obj-$(CONFIG_NFC_S3FWRN82_UART) += s3fwrn82_uart.o
diff --git a/drivers/nfc/s3fwrn5/phy_common.c b/drivers/nfc/s3fwrn5/phy_common.c
index 5cad1f4..497b02b 100644
--- a/drivers/nfc/s3fwrn5/phy_common.c
+++ b/drivers/nfc/s3fwrn5/phy_common.c
@@ -47,6 +47,18 @@ bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum 
s3fwrn5_mode mode)
 }
 EXPORT_SYMBOL(s3fwrn5_phy_power_ctrl);
 
+void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode)
+{
+   struct phy_common *phy = phy_id;
+
+   mutex_lock(>mutex);
+
+   s3fwrn5_phy_power_ctrl(phy, mode);
+
+   mutex_unlock(>mutex);
+}
+EXPORT_SYMBOL(s3fwrn5_phy_set_mode);
+
 enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id)
 {
struct phy_common *phy = phy_id;
diff --git a/drivers/nfc/s3fwrn5/phy_common.h b/drivers/nfc/s3fwrn5/phy_common.h
index b98531d..99749c9 100644
--- a/drivers/nfc/s3fwrn5/phy_common.h
+++ b/drivers/nfc/s3fwrn5/phy_common.h
@@ -31,6 +31,7 @@ struct phy_common {
 
 void s3fwrn5_phy_set_wake(void *phy_id, bool wake);
 bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum s3fwrn5_mode mode);
+void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode);
 enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id);
 
 #endif /* __NFC_S3FWRN5_PHY_COMMON_H */
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
new file mode 100644
index 000..f5ac017
--- /dev/null
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UART Link Layer for S3FWRN82 NCI based Driver
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ * Robert Baldyga 
+ * Copyright (C) 2020 Samsung Electronics
+ * Bongsu Jeon 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "phy_common.h"
+
+#define S3FWRN82_NCI_HEADER 3
+#define S3FWRN82_NCI_IDX 2
+#define NCI_SKB_BUFF_LEN 258
+
+struct s3fwrn82_uart_phy {
+   struct phy_common common;
+   struct serdev_device *ser_dev;
+   struct sk_buff *recv_skb;
+};
+
+static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+   int err;
+
+   err = serdev_device_write(phy->ser_dev,
+ out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+   if (err < 0)
+   return err;
+
+   return 0;
+}
+
+static const struct s3fwrn5_phy_ops uart_phy_ops = {
+   .set_wake = s3fwrn5_phy_set_wake,
+   .set_mode = s3fwrn5_phy_set_mode,
+   .get_mode = s3fwrn5_phy_get_mode,
+   .write = s3fwrn82_uart_write,
+};
+
+static int s3fwrn82_uart_read(struct serdev_device *serdev,
+ const unsigned char *data,
+ size_t count)
+{
+   struct s3fwrn82_uart_phy *phy = serdev_device_get_drvdata(serdev);
+   size_t i;
+
+   for (i = 0; i < count; i++) {
+   skb_put_u8(phy->recv_skb, *data++);
+
+   if (phy->recv_skb->len < S3FWRN82_NCI_HEADER)
+   continue;
+

[PATCH v3 net-next 3/4] nfc: s3fwrn5: extract the common phy blocks

[Bongsu Jeon]

From: Bongsu Jeon 

Extract the common phy blocks to reuse it.
The UART module will use the common blocks.

Signed-off-by: Bongsu Jeon 
---
 Changes in v3:
   - move the phy_common object to s3fwrn.ko to avoid duplication.
   - include the header files to include everything which is used inside.
   - wrap the lines.

 Changes in v2:
   - remove the common function's definition in common header file.
   - make the common phy_common.c file to define the common function.
   - wrap the lines.
   - change the Header guard.
   - remove the unused common function.

 drivers/nfc/s3fwrn5/Makefile |   2 +-
 drivers/nfc/s3fwrn5/i2c.c| 117 +--
 drivers/nfc/s3fwrn5/phy_common.c |  63 +
 drivers/nfc/s3fwrn5/phy_common.h |  36 
 4 files changed, 139 insertions(+), 79 deletions(-)
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
 create mode 100644 drivers/nfc/s3fwrn5/phy_common.h

diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index d0ffa35..6b6f52d 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -3,7 +3,7 @@
 # Makefile for Samsung S3FWRN5 NFC driver
 #
 
-s3fwrn5-objs = core.o firmware.o nci.o
+s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
 s3fwrn5_i2c-objs = i2c.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index 9a64eea..e1bdde1 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -15,75 +15,30 @@
 
 #include 
 
-#include "s3fwrn5.h"
+#include "phy_common.h"
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_EN_WAIT_TIME 20
-
 struct s3fwrn5_i2c_phy {
+   struct phy_common common;
struct i2c_client *i2c_dev;
-   struct nci_dev *ndev;
-
-   int gpio_en;
-   int gpio_fw_wake;
-
-   struct mutex mutex;
 
-   enum s3fwrn5_mode mode;
unsigned int irq_skip:1;
 };
 
-static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
-{
-   struct s3fwrn5_i2c_phy *phy = phy_id;
-
-   mutex_lock(>mutex);
-   gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   mutex_unlock(>mutex);
-}
-
 static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
 {
struct s3fwrn5_i2c_phy *phy = phy_id;
 
-   mutex_lock(>mutex);
+   mutex_lock(>common.mutex);
 
-   if (phy->mode == mode)
+   if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
goto out;
 
-   phy->mode = mode;
-
-   gpio_set_value(phy->gpio_en, 1);
-   gpio_set_value(phy->gpio_fw_wake, 0);
-   if (mode == S3FWRN5_MODE_FW)
-   gpio_set_value(phy->gpio_fw_wake, 1);
-
-   if (mode != S3FWRN5_MODE_COLD) {
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   gpio_set_value(phy->gpio_en, 0);
-   msleep(S3FWRN5_EN_WAIT_TIME);
-   }
-
phy->irq_skip = true;
 
 out:
-   mutex_unlock(>mutex);
-}
-
-static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
-{
-   struct s3fwrn5_i2c_phy *phy = phy_id;
-   enum s3fwrn5_mode mode;
-
-   mutex_lock(>mutex);
-
-   mode = phy->mode;
-
-   mutex_unlock(>mutex);
-
-   return mode;
+   mutex_unlock(>common.mutex);
 }
 
 static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
@@ -91,7 +46,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
struct s3fwrn5_i2c_phy *phy = phy_id;
int ret;
 
-   mutex_lock(>mutex);
+   mutex_lock(>common.mutex);
 
phy->irq_skip = false;
 
@@ -102,7 +57,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
ret  = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
}
 
-   mutex_unlock(>mutex);
+   mutex_unlock(>common.mutex);
 
if (ret < 0)
return ret;
@@ -114,9 +69,9 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
*skb)
 }
 
 static const struct s3fwrn5_phy_ops i2c_phy_ops = {
-   .set_wake = s3fwrn5_i2c_set_wake,
+   .set_wake = s3fwrn5_phy_set_wake,
.set_mode = s3fwrn5_i2c_set_mode,
-   .get_mode = s3fwrn5_i2c_get_mode,
+   .get_mode = s3fwrn5_phy_get_mode,
.write = s3fwrn5_i2c_write,
 };
 
@@ -128,7 +83,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
char hdr[4];
int ret;
 
-   hdr_size = (phy->mode == S3FWRN5_MODE_NCI) ?
+   hdr_size = (phy->common.mode == S3FWRN5_MODE_NCI) ?
NCI_CTRL_HDR_SIZE : S3FWRN5_FW_HDR_SIZE;
ret = i2c_master_recv(phy->i2c_dev, hdr, hdr_size);
if (ret < 0)
@@ -137,7 +92,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
if (ret < hdr_size)
return -EBADMSG;
 
-   data_len = (phy->mode == S3FWRN5_MODE_NCI) ?
+   

[PATCH net-next 2/4] nfc: s3fwrn5: reduce the EN_WAIT_TIME

[Bongsu Jeon]

From: Bongsu Jeon 

The delay of 20ms is enough to enable and
wake up the Samsung's nfc chip.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/i2c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index ae26594..9a64eea 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -19,7 +19,7 @@
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_EN_WAIT_TIME 150
+#define S3FWRN5_EN_WAIT_TIME 20
 
 struct s3fwrn5_i2c_phy {
struct i2c_client *i2c_dev;
@@ -40,7 +40,7 @@ static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
 
mutex_lock(>mutex);
gpio_set_value(phy->gpio_fw_wake, wake);
-   msleep(S3FWRN5_EN_WAIT_TIME/2);
+   msleep(S3FWRN5_EN_WAIT_TIME);
mutex_unlock(>mutex);
 }
 
@@ -63,7 +63,7 @@ static void s3fwrn5_i2c_set_mode(void *phy_id, enum 
s3fwrn5_mode mode)
if (mode != S3FWRN5_MODE_COLD) {
msleep(S3FWRN5_EN_WAIT_TIME);
gpio_set_value(phy->gpio_en, 0);
-   msleep(S3FWRN5_EN_WAIT_TIME/2);
+   msleep(S3FWRN5_EN_WAIT_TIME);
}
 
phy->irq_skip = true;
-- 
1.9.1

[PATCH v2 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu jeon]

From: Bongsu Jeon 

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---

Changes in v2:
 -change the compatible name.
 -change the const to enum for compatible.
 -change the node name to nfc.

 .../bindings/net/nfc/samsung,s3fwrn5.yaml  | 32 --
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml 
b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index cb0b8a5..481bbcc 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -12,7 +12,10 @@ maintainers:
 
 properties:
   compatible:
-const: samsung,s3fwrn5-i2c
+oneOf:
+  - enum:
+- samsung,s3fwrn5-i2c
+- samsung,s3fwrn82
 
   en-gpios:
 maxItems: 1
@@ -47,10 +50,19 @@ additionalProperties: false
 required:
   - compatible
   - en-gpios
-  - interrupts
-  - reg
   - wake-gpios
 
+allOf:
+  - if:
+  properties:
+compatible:
+  contains:
+const: samsung,s3fwrn5-i2c
+then:
+  required:
+- interrupts
+- reg
+
 examples:
   - |
 #include 
@@ -71,3 +83,17 @@ examples:
 wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
 };
 };
+  # UART example on Raspberry Pi
+  - |
+uart0 {
+status = "okay";
+
+nfc {
+compatible = "samsung,s3fwrn82";
+
+en-gpios = < 20 0>;
+wake-gpios = < 16 0>;
+
+status = "okay";
+};
+};
-- 
1.9.1

Re: [PATCH v2 net-next 3/3] nfc: s3fwrn5: extract the common phy blocks

[Bongsu Jeon]

On Sat, Nov 28, 2020 at 9:49 PM Krzysztof Kozlowski  wrote:
>
> On Fri, Nov 27, 2020 at 08:22:18PM +0900, bongsu.je...@gmail.com wrote:
> > From: Bongsu Jeon 
> >
> > Extract the common phy blocks to reuse it.
> > The UART module will use the common blocks.
> >
> > Signed-off-by: Bongsu Jeon 
> > ---
> > Changes in v2:
> >  - remove the common function's definition in common header file.
> >  - make the common phy_common.c file to define the common function.
> >  - wrap the lines.
> >  - change the Header guard.
> >  - remove the unused common function.
> >
> >  drivers/nfc/s3fwrn5/Makefile |   2 +-
> >  drivers/nfc/s3fwrn5/i2c.c| 114 
> > +--
> >  drivers/nfc/s3fwrn5/phy_common.c |  60 +
> >  drivers/nfc/s3fwrn5/phy_common.h |  31 +++
> >  4 files changed, 129 insertions(+), 78 deletions(-)
> >  create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
> >  create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
> >
> > diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
> > index d0ffa35..a5279c6 100644
> > --- a/drivers/nfc/s3fwrn5/Makefile
> > +++ b/drivers/nfc/s3fwrn5/Makefile
> > @@ -4,7 +4,7 @@
> >  #
> >
> >  s3fwrn5-objs = core.o firmware.o nci.o
> > -s3fwrn5_i2c-objs = i2c.o
> > +s3fwrn5_i2c-objs = i2c.o phy_common.o
>
> Thanks for the changes.
>
> Shouldn't this be part of s3fwrn5.ko? Otherwise you would duplicate the
> objects in two modules.
>

Okay. It could be better to avoid the duplication.
I will try to move phy_common.c to s3fwrn5.ko.

> >
> >  obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
> >  obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
> > diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> > index 9a64eea..207f970 100644
> > --- a/drivers/nfc/s3fwrn5/i2c.c
> > +++ b/drivers/nfc/s3fwrn5/i2c.c
> > @@ -16,74 +16,30 @@
> >  #include 
> >
> >  #include "s3fwrn5.h"
> > +#include "phy_common.h"
> >
> >  #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
> >
> > -#define S3FWRN5_EN_WAIT_TIME 20
> > -
> >  struct s3fwrn5_i2c_phy {
> > + struct phy_common common;
> >   struct i2c_client *i2c_dev;
> > - struct nci_dev *ndev;
> > -
> > - int gpio_en;
> > - int gpio_fw_wake;
> > -
> > - struct mutex mutex;
> >
> > - enum s3fwrn5_mode mode;
> >   unsigned int irq_skip:1;
> >  };
> >
> > -static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
> > -{
> > - struct s3fwrn5_i2c_phy *phy = phy_id;
> > -
> > - mutex_lock(>mutex);
> > - gpio_set_value(phy->gpio_fw_wake, wake);
> > - msleep(S3FWRN5_EN_WAIT_TIME);
> > - mutex_unlock(>mutex);
> > -}
> > -
> >  static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> >  {
> >   struct s3fwrn5_i2c_phy *phy = phy_id;
> >
> > - mutex_lock(>mutex);
> > + mutex_lock(>common.mutex);
> >
> > - if (phy->mode == mode)
> > + if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
> >   goto out;
> >
> > - phy->mode = mode;
> > -
> > - gpio_set_value(phy->gpio_en, 1);
> > - gpio_set_value(phy->gpio_fw_wake, 0);
> > - if (mode == S3FWRN5_MODE_FW)
> > - gpio_set_value(phy->gpio_fw_wake, 1);
> > -
> > - if (mode != S3FWRN5_MODE_COLD) {
> > - msleep(S3FWRN5_EN_WAIT_TIME);
> > - gpio_set_value(phy->gpio_en, 0);
> > - msleep(S3FWRN5_EN_WAIT_TIME);
> > - }
> > -
> >   phy->irq_skip = true;
> >
> >  out:
> > - mutex_unlock(>mutex);
> > -}
> > -
> > -static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
> > -{
> > - struct s3fwrn5_i2c_phy *phy = phy_id;
> > - enum s3fwrn5_mode mode;
> > -
> > - mutex_lock(>mutex);
> > -
> > - mode = phy->mode;
> > -
> > - mutex_unlock(>mutex);
> > -
> > - return mode;
> > + mutex_unlock(>common.mutex);
> >  }
> >
> >  static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
> > @@ -91,7 +47,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
> > *skb)
> >   struct s3fwrn5_i2c_phy *phy = phy_id;
> >   int ret;
> >
> > - mutex_lock(>mutex);
> > +

Re: [PATCH net-next 1/3] nfc: s3fwrn5: use signed integer for parsing GPIO numbers

[Bongsu Jeon]

On 11/27/20, Bongsu Jeon  wrote:
> On Fri, Nov 27, 2020 at 2:06 AM Krzysztof Kozlowski 
> wrote:
>>
>> On Fri, Nov 27, 2020 at 12:33:37AM +0900, bongsu.je...@gmail.com wrote:
>> > From: Krzysztof Kozlowski 
>> >
>> > GPIOs - as returned by of_get_named_gpio() and used by the gpiolib -
>> > are
>> > signed integers, where negative number indicates error.  The return
>> > value of of_get_named_gpio() should not be assigned to an unsigned int
>> > because in case of !CONFIG_GPIOLIB such number would be a valid GPIO.
>> >
>> > Fixes: c04c674fadeb ("nfc: s3fwrn5: Add driver for Samsung S3FWRN5 NFC
>> > Chip")
>> > Cc: 
>> > Signed-off-by: Krzysztof Kozlowski 
>>
>> Why do you send my patch?
>>
>
> I think that your patch should be applied before refactoring for this
> driver.
> So, I applied your patch to net-next branch and included your patch at
> my patch list.
> Is this the wrong process?
>

Sorry to confuse you.
I found your patch when i updated my workspace using git pull.

>> Best regards,
>> Krzysztof
>

Re: [PATCH net-next 3/3] nfc: s3fwrn5: extract the common phy blocks

[Bongsu Jeon]

On Fri, Nov 27, 2020 at 2:13 AM Krzysztof Kozlowski  wrote:
>
> On Fri, Nov 27, 2020 at 12:33:39AM +0900, bongsu.je...@gmail.com wrote:
> > From: Bongsu Jeon 
> >
> > Extract the common phy blocks to reuse it.
> > The UART module will use the common blocks.
>
>
> Hi,
>
> Thanks for the patch. Few comments below.
>
> > Signed-off-by: Bongsu Jeon 
> > ---
> >  drivers/nfc/s3fwrn5/i2c.c| 111 
> > ---
> >  drivers/nfc/s3fwrn5/phy_common.h |  86 ++
> >  2 files changed, 119 insertions(+), 78 deletions(-)
> >  create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
> >
> > diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> > index 9a64eea..cd1b2a7 100644
> > --- a/drivers/nfc/s3fwrn5/i2c.c
> > +++ b/drivers/nfc/s3fwrn5/i2c.c
> > @@ -15,75 +15,30 @@
> >
> >  #include 
> >
> > -#include "s3fwrn5.h"
> > +#include "phy_common.h"
> >
> >  #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
> >
> > -#define S3FWRN5_EN_WAIT_TIME 20
> > -
> >  struct s3fwrn5_i2c_phy {
> > + struct phy_common common;
> >   struct i2c_client *i2c_dev;
> > - struct nci_dev *ndev;
> > -
> > - int gpio_en;
> > - int gpio_fw_wake;
> > -
> > - struct mutex mutex;
> >
> > - enum s3fwrn5_mode mode;
> >   unsigned int irq_skip:1;
> >  };
> >
> > -static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
> > -{
> > - struct s3fwrn5_i2c_phy *phy = phy_id;
> > -
> > - mutex_lock(>mutex);
> > - gpio_set_value(phy->gpio_fw_wake, wake);
> > - msleep(S3FWRN5_EN_WAIT_TIME);
> > - mutex_unlock(>mutex);
> > -}
> > -
> >  static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> >  {
> >   struct s3fwrn5_i2c_phy *phy = phy_id;
> >
> > - mutex_lock(>mutex);
> > + mutex_lock(>common.mutex);
> >
> > - if (phy->mode == mode)
> > + if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
> >   goto out;
> >
> > - phy->mode = mode;
> > -
> > - gpio_set_value(phy->gpio_en, 1);
> > - gpio_set_value(phy->gpio_fw_wake, 0);
> > - if (mode == S3FWRN5_MODE_FW)
> > - gpio_set_value(phy->gpio_fw_wake, 1);
> > -
> > - if (mode != S3FWRN5_MODE_COLD) {
> > - msleep(S3FWRN5_EN_WAIT_TIME);
> > - gpio_set_value(phy->gpio_en, 0);
> > - msleep(S3FWRN5_EN_WAIT_TIME);
> > - }
> > -
> >   phy->irq_skip = true;
> >
> >  out:
> > - mutex_unlock(>mutex);
> > -}
> > -
> > -static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
> > -{
> > - struct s3fwrn5_i2c_phy *phy = phy_id;
> > - enum s3fwrn5_mode mode;
> > -
> > - mutex_lock(>mutex);
> > -
> > - mode = phy->mode;
> > -
> > - mutex_unlock(>mutex);
> > -
> > - return mode;
> > + mutex_unlock(>common.mutex);
> >  }
> >
> >  static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
> > @@ -91,7 +46,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff 
> > *skb)
> >   struct s3fwrn5_i2c_phy *phy = phy_id;
> >   int ret;
> >
> > - mutex_lock(>mutex);
> > + mutex_lock(>common.mutex);
> >
> >   phy->irq_skip = false;
> >
> > @@ -102,7 +57,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct 
> > sk_buff *skb)
> >   ret  = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
> >   }
> >
> > - mutex_unlock(>mutex);
> > + mutex_unlock(>common.mutex);
> >
> >   if (ret < 0)
> >   return ret;
> > @@ -114,9 +69,9 @@ static int s3fwrn5_i2c_write(void *phy_id, struct 
> > sk_buff *skb)
> >  }
> >
> >  static const struct s3fwrn5_phy_ops i2c_phy_ops = {
> > - .set_wake = s3fwrn5_i2c_set_wake,
> > + .set_wake = s3fwrn5_phy_set_wake,
> >   .set_mode = s3fwrn5_i2c_set_mode,
> > - .get_mode = s3fwrn5_i2c_get_mode,
> > + .get_mode = s3fwrn5_phy_get_mode,
> >   .write = s3fwrn5_i2c_write,
> >  };
> >
> > @@ -128,7 +83,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
> >   char hdr[4];
> >   int ret;
> >
> > - hdr_size = (p

Re: [PATCH net-next 1/3] nfc: s3fwrn5: use signed integer for parsing GPIO numbers

[Bongsu Jeon]

On Fri, Nov 27, 2020 at 2:06 AM Krzysztof Kozlowski  wrote:
>
> On Fri, Nov 27, 2020 at 12:33:37AM +0900, bongsu.je...@gmail.com wrote:
> > From: Krzysztof Kozlowski 
> >
> > GPIOs - as returned by of_get_named_gpio() and used by the gpiolib - are
> > signed integers, where negative number indicates error.  The return
> > value of of_get_named_gpio() should not be assigned to an unsigned int
> > because in case of !CONFIG_GPIOLIB such number would be a valid GPIO.
> >
> > Fixes: c04c674fadeb ("nfc: s3fwrn5: Add driver for Samsung S3FWRN5 NFC 
> > Chip")
> > Cc: 
> > Signed-off-by: Krzysztof Kozlowski 
>
> Why do you send my patch?
>

I think that your patch should be applied before refactoring for this driver.
So, I applied your patch to net-next branch and included your patch at
my patch list.
Is this the wrong process?

> Best regards,
> Krzysztof

Re: [PATCH net-next 2/2] net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

On 11/25/20, Bongsu Jeon  wrote:
> On 11/24/20, k...@kernel.org  wrote:
>> On Tue, Nov 24, 2020 at 09:05:52PM +0900, Bongsu Jeon wrote:
>>> On Mon, Nov 23, 2020 at 5:55 PM k...@kernel.org  wrote:
>>> > > +static enum s3fwrn5_mode s3fwrn82_uart_get_mode(void *phy_id)
>>> > > +{
>>> > > + struct s3fwrn82_uart_phy *phy = phy_id;
>>> > > + enum s3fwrn5_mode mode;
>>> > > +
>>> > > + mutex_lock(>mutex);
>>> > > + mode = phy->mode;
>>> > > + mutex_unlock(>mutex);
>>> > > + return mode;
>>> > > +}
>>> >
>>> > All this duplicates I2C version. You need to start either reusing
>>> > common
>>> > blocks.
>>> >
>>>
>>> Okay. I will do refactoring on i2c.c and uart.c to make common blocks.
>>>  is it okay to separate a patch for it?
>>
>> Yes, that would be the best - refactor the driver to split some common
>> methods and then in next patch add new s3fwrn82 UART driver.
>>
>>> > > +
>>> > > +static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
>>> > > +{
>>> > > + struct s3fwrn82_uart_phy *phy = phy_id;
>>> > > + int err;
>>> > > +
>>> > > + err = serdev_device_write(phy->ser_dev,
>>> > > +   out->data, out->len,
>>> > > +   MAX_SCHEDULE_TIMEOUT);
>>> > > + if (err < 0)
>>> > > + return err;
>>> > > +
>>> > > + return 0;
>>> > > +}
>>> > > +
>>> > > +static const struct s3fwrn5_phy_ops uart_phy_ops = {
>>> > > + .set_wake = s3fwrn82_uart_set_wake,
>>> > > + .set_mode = s3fwrn82_uart_set_mode,
>>> > > + .get_mode = s3fwrn82_uart_get_mode,
>>> > > + .write = s3fwrn82_uart_write,
>>> > > +};
>>> > > +
>>> > > +static int s3fwrn82_uart_read(struct serdev_device *serdev,
>>> > > +   const unsigned char *data,
>>> > > +   size_t count)
>>> > > +{
>>> > > + struct s3fwrn82_uart_phy *phy =
>>> > > serdev_device_get_drvdata(serdev);
>>> > > + size_t i;
>>> > > +
>>> > > + for (i = 0; i < count; i++) {
>>> > > + skb_put_u8(phy->recv_skb, *data++);
>>> > > +
>>> > > + if (phy->recv_skb->len < S3FWRN82_NCI_HEADER)
>>> > > + continue;
>>> > > +
>>> > > + if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
>>> > > + <
>>> > > phy->recv_skb->data[S3FWRN82_NCI_IDX])
>>> > > + continue;
>>> > > +
>>> > > + s3fwrn5_recv_frame(phy->ndev, phy->recv_skb,
>>> > > phy->mode);
>>> > > + phy->recv_skb = alloc_skb(NCI_SKB_BUFF_LEN,
>>> > > GFP_KERNEL);
>>> > > + if (!phy->recv_skb)
>>> > > + return 0;
>>> > > + }
>>> > > +
>>> > > + return i;
>>> > > +}
>>> > > +
>>> > > +static struct serdev_device_ops s3fwrn82_serdev_ops = {
>>> >
>>> > const
>>> >
>>> > > + .receive_buf = s3fwrn82_uart_read,
>>> > > + .write_wakeup = serdev_device_write_wakeup,
>>> > > +};
>>> > > +
>>> > > +static const struct of_device_id s3fwrn82_uart_of_match[] = {
>>> > > + { .compatible = "samsung,s3fwrn82-uart", },
>>> > > + {},
>>> > > +};
>>> > > +MODULE_DEVICE_TABLE(of, s3fwrn82_uart_of_match);
>>> > > +
>>> > > +static int s3fwrn82_uart_parse_dt(struct serdev_device *serdev)
>>> > > +{
>>> > > + struct s3fwrn82_uart_phy *phy =
>>> > > serdev_device_get_drvdata(serdev);
>>> > > + struct device_node *np = serdev->dev.of_node;
>>> > > +
>>> > > + if (!np)
>>> > > + return -ENODEV;
>>> > > +
>>> > > +   

Re: [PATCH net-next 2/2] net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

On 11/24/20, k...@kernel.org  wrote:
> On Tue, Nov 24, 2020 at 09:05:52PM +0900, Bongsu Jeon wrote:
>> On Mon, Nov 23, 2020 at 5:55 PM k...@kernel.org  wrote:
>> > > +static enum s3fwrn5_mode s3fwrn82_uart_get_mode(void *phy_id)
>> > > +{
>> > > + struct s3fwrn82_uart_phy *phy = phy_id;
>> > > + enum s3fwrn5_mode mode;
>> > > +
>> > > + mutex_lock(>mutex);
>> > > + mode = phy->mode;
>> > > + mutex_unlock(>mutex);
>> > > + return mode;
>> > > +}
>> >
>> > All this duplicates I2C version. You need to start either reusing
>> > common
>> > blocks.
>> >
>>
>> Okay. I will do refactoring on i2c.c and uart.c to make common blocks.
>>  is it okay to separate a patch for it?
>
> Yes, that would be the best - refactor the driver to split some common
> methods and then in next patch add new s3fwrn82 UART driver.
>
>> > > +
>> > > +static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
>> > > +{
>> > > + struct s3fwrn82_uart_phy *phy = phy_id;
>> > > + int err;
>> > > +
>> > > + err = serdev_device_write(phy->ser_dev,
>> > > +   out->data, out->len,
>> > > +   MAX_SCHEDULE_TIMEOUT);
>> > > + if (err < 0)
>> > > + return err;
>> > > +
>> > > + return 0;
>> > > +}
>> > > +
>> > > +static const struct s3fwrn5_phy_ops uart_phy_ops = {
>> > > + .set_wake = s3fwrn82_uart_set_wake,
>> > > + .set_mode = s3fwrn82_uart_set_mode,
>> > > + .get_mode = s3fwrn82_uart_get_mode,
>> > > + .write = s3fwrn82_uart_write,
>> > > +};
>> > > +
>> > > +static int s3fwrn82_uart_read(struct serdev_device *serdev,
>> > > +   const unsigned char *data,
>> > > +   size_t count)
>> > > +{
>> > > + struct s3fwrn82_uart_phy *phy =
>> > > serdev_device_get_drvdata(serdev);
>> > > + size_t i;
>> > > +
>> > > + for (i = 0; i < count; i++) {
>> > > + skb_put_u8(phy->recv_skb, *data++);
>> > > +
>> > > + if (phy->recv_skb->len < S3FWRN82_NCI_HEADER)
>> > > + continue;
>> > > +
>> > > + if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
>> > > + <
>> > > phy->recv_skb->data[S3FWRN82_NCI_IDX])
>> > > + continue;
>> > > +
>> > > + s3fwrn5_recv_frame(phy->ndev, phy->recv_skb,
>> > > phy->mode);
>> > > + phy->recv_skb = alloc_skb(NCI_SKB_BUFF_LEN,
>> > > GFP_KERNEL);
>> > > + if (!phy->recv_skb)
>> > > + return 0;
>> > > + }
>> > > +
>> > > + return i;
>> > > +}
>> > > +
>> > > +static struct serdev_device_ops s3fwrn82_serdev_ops = {
>> >
>> > const
>> >
>> > > + .receive_buf = s3fwrn82_uart_read,
>> > > + .write_wakeup = serdev_device_write_wakeup,
>> > > +};
>> > > +
>> > > +static const struct of_device_id s3fwrn82_uart_of_match[] = {
>> > > + { .compatible = "samsung,s3fwrn82-uart", },
>> > > + {},
>> > > +};
>> > > +MODULE_DEVICE_TABLE(of, s3fwrn82_uart_of_match);
>> > > +
>> > > +static int s3fwrn82_uart_parse_dt(struct serdev_device *serdev)
>> > > +{
>> > > + struct s3fwrn82_uart_phy *phy =
>> > > serdev_device_get_drvdata(serdev);
>> > > + struct device_node *np = serdev->dev.of_node;
>> > > +
>> > > + if (!np)
>> > > + return -ENODEV;
>> > > +
>> > > + phy->gpio_en = of_get_named_gpio(np, "en-gpios", 0);
>> > > + if (!gpio_is_valid(phy->gpio_en))
>> > > + return -ENODEV;
>> > > +
>> > > + phy->gpio_fw_wake = of_get_named_gpio(np, "wake-gpios", 0);
>> >
>> > You should not cast it it unsigned int. I'll fix the s3fwrn5 from which
>> > you copied this apparen

Re: [PATCH net-next 1/2] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

On 11/24/20, k...@kernel.org  wrote:
> On Tue, Nov 24, 2020 at 08:39:40PM +0900, Bongsu Jeon wrote:
>> On Mon, Nov 23, 2020 at 5:02 PM k...@kernel.org  wrote:
>> >
>> > On Mon, Nov 23, 2020 at 04:55:26PM +0900, Bongsu Jeon wrote:
>  > >  examples:
>> > >- |
>> > >  #include 
>> > > @@ -71,3 +81,17 @@ examples:
>> > >  wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
>> > >  };
>> > >  };
>> > > +  # UART example on Raspberry Pi
>> > > +  - |
>> > > + {
>> > > +status = "okay";
>> > > +
>> > > +s3fwrn82_uart {
>> >
>> > Just "bluetooth" to follow Devicetree specification.
>> Sorry. I don't understand this comment.
>> Could you explain it?
>> Does it mean i need to refer to the net/broadcom-bluetooth.txt?
>
> The node name should be "bluetooth", not "s3fwrn82_uart", because of
> Devicetree naming convention - node names should represent generic class
> of a device.
>
Actually, RN82 is the nfc device.
So, is it okay to use the name as nfc instead of Bluetooth?

> Best regards,
> Krzysztof
>
>

Re: [PATCH net-next 2/2] net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

On Mon, Nov 23, 2020 at 5:55 PM k...@kernel.org  wrote:
>
> On Mon, Nov 23, 2020 at 04:56:58PM +0900, Bongsu Jeon wrote:
> > Since S3FWRN82 NFC Chip, The UART interface can be used.
> > S3FWRN82 uses NCI protocol and supports I2C and UART interface.
> >
> > Signed-off-by: Bongsu Jeon 
>
> Please start sending emails properly, e.g. with git send-email, so all
> your patches in the patchset are referencing the first patch.
>
Ok. I will do that.

> > ---
> >  drivers/nfc/s3fwrn5/Kconfig  |  12 ++
> >  drivers/nfc/s3fwrn5/Makefile |   2 +
> >  drivers/nfc/s3fwrn5/uart.c   | 250 +++
> >  3 files changed, 264 insertions(+)
> >  create mode 100644 drivers/nfc/s3fwrn5/uart.c
> >
> > diff --git a/drivers/nfc/s3fwrn5/Kconfig b/drivers/nfc/s3fwrn5/Kconfig
> > index 3f8b6da58280..6f88737769e1 100644
> > --- a/drivers/nfc/s3fwrn5/Kconfig
> > +++ b/drivers/nfc/s3fwrn5/Kconfig
> > @@ -20,3 +20,15 @@ config NFC_S3FWRN5_I2C
> > To compile this driver as a module, choose m here. The module will
> > be called s3fwrn5_i2c.ko.
> > Say N if unsure.
> > +
> > +config NFC_S3FWRN82_UART
> > + tristate "Samsung S3FWRN82 UART support"
> > + depends on NFC_NCI && SERIAL_DEV_BUS
>
> What about SERIAL_DEV_BUS as module? Shouldn't this be
> SERIAL_DEV_BUS || !SERIAL_DEV_BUS?
>

SERIAL_DEV_BUS is okay even if SERIAL_DEV_BUS is defined as module.

> > + select NFC_S3FWRN5
> > + help
> > +   This module adds support for a UART interface to the S3FWRN82 chip.
> > +   Select this if your platform is using the UART bus.
> > +
> > +   To compile this driver as a module, choose m here. The module will
> > +   be called s3fwrn82_uart.ko.
> > +   Say N if unsure.
> > diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
> > index d0ffa35f50e8..d1902102060b 100644
> > --- a/drivers/nfc/s3fwrn5/Makefile
> > +++ b/drivers/nfc/s3fwrn5/Makefile
> > @@ -5,6 +5,8 @@
> >
> >  s3fwrn5-objs = core.o firmware.o nci.o
> >  s3fwrn5_i2c-objs = i2c.o
> > +s3fwrn82_uart-objs = uart.o
> >
> >  obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
> >  obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
> > +obj-$(CONFIG_NFC_S3FWRN82_UART) += s3fwrn82_uart.o
> > diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
> > new file mode 100644
> > index ..b3c36a5b28d3
> > --- /dev/null
> > +++ b/drivers/nfc/s3fwrn5/uart.c
> > @@ -0,0 +1,250 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * UART Link Layer for S3FWRN82 NCI based Driver
> > + *
> > + * Copyright (C) 2020 Samsung Electronics
> > + * Author: Bongsu Jeon 
>
> You copied a lot from existing i2c.c. Please keep also the original
> copyrights.
>

Okay. I will keep also the original copyrights.

> > + * All rights reserved.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "s3fwrn5.h"
> > +
> > +#define S3FWRN82_UART_DRIVER_NAME "s3fwrn82_uart"
>
> Remove the define, it is used only once.
>
> > +#define S3FWRN82_NCI_HEADER 3
> > +#define S3FWRN82_NCI_IDX 2
> > +#define S3FWRN82_EN_WAIT_TIME 20
> > +#define NCI_SKB_BUFF_LEN 258
> > +
> > +struct s3fwrn82_uart_phy {
> > + struct serdev_device *ser_dev;
> > + struct nci_dev *ndev;
> > + struct sk_buff *recv_skb;
> > +
> > + unsigned int gpio_en;
> > + unsigned int gpio_fw_wake;
> > +
> > + /* mutex is used to synchronize */
>
> Please do not write obvious comments. Mutex is always used to
> synchronize, what else is it for? Instead you must describe what exactly
> is protected with mutex.
>

I understand it. I will fix it.

> > + struct mutex mutex;
> > + enum s3fwrn5_mode mode;
> > +};
> > +
> > +static void s3fwrn82_uart_set_wake(void *phy_id, bool wake)
> > +{
> > + struct s3fwrn82_uart_phy *phy = phy_id;
> > +
> > + mutex_lock(>mutex);
> > + gpio_set_value(phy->gpio_fw_wake, wake);
> > + msleep(S3FWRN82_EN_WAIT_TIME);
> > + mutex_unlock(>mutex);
> > +}
> > +
> > +static void s3fwrn82_uart_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> > +{
> > + struct s3fwrn82_uart_phy *phy = phy_id;
> > +
> > + mutex_lock(>mutex);
&g

Re: [PATCH net-next 1/2] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

On Mon, Nov 23, 2020 at 5:02 PM k...@kernel.org  wrote:
>
> On Mon, Nov 23, 2020 at 04:55:26PM +0900, Bongsu Jeon wrote:
> > Since S3FWRN82 NFC Chip, The UART interface can be used.
> > S3FWRN82 supports I2C and UART interface.
> >
> > Signed-off-by: Bongsu Jeon 
> > ---
> >  .../bindings/net/nfc/samsung,s3fwrn5.yaml | 28 +--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml 
> > b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> > index cb0b8a560282..37b3e5ae5681 100644
> > --- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> > +++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> > @@ -13,6 +13,7 @@ maintainers:
> >  properties:
> >compatible:
> >  const: samsung,s3fwrn5-i2c
> > +const: samsung,s3fwrn82-uart
>
> This does not work, you need to use enum. Did you run at least
> dt_bindings_check?
>
Sorry. I didn't. I fixed it as below and ran dt_bindings_check.
compatible:
   oneOf:
   - enum:
   - samsung,s3fwrn5-i2c
   - samsung,s3fwrn82


> The compatible should be just "samsung,s3fwrn82". I think it was a
> mistake in the first s3fwrn5 submission to add a interface to
> compatible.
>
Ok. I will change the name.

> >
> >en-gpios:
> >  maxItems: 1
> > @@ -47,10 +48,19 @@ additionalProperties: false
> >  required:
> >- compatible
> >- en-gpios
> > -  - interrupts
> > -  - reg
> >- wake-gpios
> >
> > +allOf:
> > +  - if:
> > +  properties:
> > +compatible:
> > +  contains:
> > +const: samsung,s3fwrn5-i2c
> > +then:
> > +  required:
> > +- interrupts
> > +- reg
> > +
> >  examples:
> >- |
> >  #include 
> > @@ -71,3 +81,17 @@ examples:
> >  wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
> >  };
> >  };
> > +  # UART example on Raspberry Pi
> > +  - |
> > + {
> > +status = "okay";
> > +
> > +s3fwrn82_uart {
>
> Just "bluetooth" to follow Devicetree specification.
Sorry. I don't understand this comment.
Could you explain it?
Does it mean i need to refer to the net/broadcom-bluetooth.txt?

>
> Best regards,
> Krzysztof

[PATCH net-next v2] net/nfc/nci: Support NCI 2.x initial sequence

[Bongsu Jeon]

implement the NCI 2.x initial sequence to support NCI 2.x NFCC.
Since NCI 2.0, CORE_RESET and CORE_INIT sequence have been changed.
If NFCEE supports NCI 2.x, then NCI 2.x initial sequence will work.

In NCI 1.0, Initial sequence and payloads are as below:
(DH) (NFCC)
 |  -- CORE_RESET_CMD --> |
 |  <-- CORE_RESET_RSP -- |
 |  -- CORE_INIT_CMD -->  |
 |  <-- CORE_INIT_RSP --  |
 CORE_RESET_RSP payloads are Status, NCI version, Configuration Status.
 CORE_INIT_CMD payloads are empty.
 CORE_INIT_RSP payloads are Status, NFCC Features,
Number of Supported RF Interfaces, Supported RF Interface,
Max Logical Connections, Max Routing table Size,
Max Control Packet Payload Size, Max Size for Large Parameters,
Manufacturer ID, Manufacturer Specific Information.

In NCI 2.0, Initial Sequence and Parameters are as below:
(DH) (NFCC)
 |  -- CORE_RESET_CMD --> |
 |  <-- CORE_RESET_RSP -- |
 |  <-- CORE_RESET_NTF -- |
 |  -- CORE_INIT_CMD -->  |
 |  <-- CORE_INIT_RSP --  |
 CORE_RESET_RSP payloads are Status.
 CORE_RESET_NTF payloads are Reset Trigger,
Configuration Status, NCI Version, Manufacturer ID,
Manufacturer Specific Information Length,
Manufacturer Specific Information.
 CORE_INIT_CMD payloads are Feature1, Feature2.
 CORE_INIT_RSP payloads are Status, NFCC Features,
Max Logical Connections, Max Routing Table Size,
Max Control Packet Payload Size,
Max Data Packet Payload Size of the Static HCI Connection,
Number of Credits of the Static HCI Connection,
Max NFC-V RF Frame Size, Number of Supported RF Interfaces,
Supported RF Interfaces.

Signed-off-by: Bongsu Jeon 
---
 Changes in v2:
  - fix the warning of type casting.
  - changed the __u8 type to unsigned char.

 include/net/nfc/nci.h | 39 ++
 net/nfc/nci/core.c| 23 +++--
 net/nfc/nci/ntf.c | 21 
 net/nfc/nci/rsp.c | 75 +--
 4 files changed, 146 insertions(+), 12 deletions(-)

diff --git a/include/net/nfc/nci.h b/include/net/nfc/nci.h
index 0550e0380b8d..decc89803d4b 100644
--- a/include/net/nfc/nci.h
+++ b/include/net/nfc/nci.h
@@ -25,6 +25,8 @@
 #define NCI_MAX_PARAM_LEN  251
 #define NCI_MAX_PAYLOAD_SIZE   255
 #define NCI_MAX_PACKET_SIZE258
+#define NCI_MAX_LARGE_PARAMS_NCI_v215
+#define NCI_VER_2_MASK 0x20
 
 /* NCI Status Codes */
 #define NCI_STATUS_OK  0x00
@@ -131,6 +133,9 @@
 #define NCI_LF_CON_BITR_F_212  0x02
 #define NCI_LF_CON_BITR_F_424  0x04
 
+/* NCI 2.x Feature Enable Bit */
+#define NCI_FEATURE_DISABLE0x00
+
 /* NCI Reset types */
 #define NCI_RESET_TYPE_KEEP_CONFIG 0x00
 #define NCI_RESET_TYPE_RESET_CONFIG0x01
@@ -220,6 +225,11 @@ struct nci_core_reset_cmd {
 } __packed;
 
 #define NCI_OP_CORE_INIT_CMD   nci_opcode_pack(NCI_GID_CORE, 0x01)
+/* To support NCI 2.x */
+struct nci_core_init_v2_cmd {
+   unsigned char   feature1;
+   unsigned char   feature2;
+} __packed;
 
 #define NCI_OP_CORE_SET_CONFIG_CMD nci_opcode_pack(NCI_GID_CORE, 0x02)
 struct set_config_param {
@@ -316,6 +326,11 @@ struct nci_core_reset_rsp {
__u8config_status;
 } __packed;
 
+/* To support NCI ver 2.x */
+struct nci_core_reset_rsp_nci_ver2 {
+   unsigned char   status;
+} __packed;
+
 #define NCI_OP_CORE_INIT_RSP   nci_opcode_pack(NCI_GID_CORE, 0x01)
 struct nci_core_init_rsp_1 {
__u8status;
@@ -334,6 +349,20 @@ struct nci_core_init_rsp_2 {
__le32  manufact_specific_info;
 } __packed;
 
+/* To support NCI ver 2.x */
+struct nci_core_init_rsp_nci_ver2 {
+   unsigned char   status;
+   __le32  nfcc_features;
+   unsigned char   max_logical_connections;
+   __le16  max_routing_table_size;
+   unsigned char   max_ctrl_pkt_payload_len;
+   unsigned char   max_data_pkt_hci_payload_len;
+   unsigned char   number_of_hci_credit;
+   __le16  max_nfc_v_frame_size;
+   unsigned char   num_supported_rf_interfaces;
+   unsigned char   supported_rf_interfaces[];
+} __packed;
+
 #define NCI_OP_CORE_SET_CONFIG_RSP nci_opcode_pack(NCI_GID_CORE, 0x02)
 struct nci_core_set_config_rsp {
__u8status;
@@ -372,6 +401,16 @@ struct nci_nfcee_discover_rsp {
 /* --- */
 /*  NCI Notifications  */
 /* --- */
+#define NCI_OP_CORE_RESET_NTF  nci_opcode_pack(NCI_GID_CORE, 0x00)
+struct nci_core_reset_ntf {
+   unsigned char   reset_trigger;
+   unsigned char   config_status;
+   unsigned char   nci_ver;
+   unsigned char   manufac

[PATCH net-next 1/2] dt-bindings: net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---
 .../bindings/net/nfc/samsung,s3fwrn5.yaml | 28 +--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml 
b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index cb0b8a560282..37b3e5ae5681 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -13,6 +13,7 @@ maintainers:
 properties:
   compatible:
 const: samsung,s3fwrn5-i2c
+const: samsung,s3fwrn82-uart
 
   en-gpios:
 maxItems: 1
@@ -47,10 +48,19 @@ additionalProperties: false
 required:
   - compatible
   - en-gpios
-  - interrupts
-  - reg
   - wake-gpios
 
+allOf:
+  - if:
+  properties:
+compatible:
+  contains:
+const: samsung,s3fwrn5-i2c
+then:
+  required:
+- interrupts
+- reg
+
 examples:
   - |
 #include 
@@ -71,3 +81,17 @@ examples:
 wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
 };
 };
+  # UART example on Raspberry Pi
+  - |
+ {
+status = "okay";
+
+s3fwrn82_uart {
+compatible = "samsung,s3fwrn82-uart";
+
+en-gpios = < 20 0>;
+wake-gpios = < 16 0>;
+
+status = "okay";
+};
+};
-- 
2.17.1

[PATCH net-next 2/2] net: nfc: s3fwrn5: Support a UART interface

[Bongsu Jeon]

Since S3FWRN82 NFC Chip, The UART interface can be used.
S3FWRN82 uses NCI protocol and supports I2C and UART interface.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/Kconfig  |  12 ++
 drivers/nfc/s3fwrn5/Makefile |   2 +
 drivers/nfc/s3fwrn5/uart.c   | 250 +++
 3 files changed, 264 insertions(+)
 create mode 100644 drivers/nfc/s3fwrn5/uart.c

diff --git a/drivers/nfc/s3fwrn5/Kconfig b/drivers/nfc/s3fwrn5/Kconfig
index 3f8b6da58280..6f88737769e1 100644
--- a/drivers/nfc/s3fwrn5/Kconfig
+++ b/drivers/nfc/s3fwrn5/Kconfig
@@ -20,3 +20,15 @@ config NFC_S3FWRN5_I2C
  To compile this driver as a module, choose m here. The module will
  be called s3fwrn5_i2c.ko.
  Say N if unsure.
+
+config NFC_S3FWRN82_UART
+   tristate "Samsung S3FWRN82 UART support"
+   depends on NFC_NCI && SERIAL_DEV_BUS
+   select NFC_S3FWRN5
+   help
+ This module adds support for a UART interface to the S3FWRN82 chip.
+ Select this if your platform is using the UART bus.
+
+ To compile this driver as a module, choose m here. The module will
+ be called s3fwrn82_uart.ko.
+ Say N if unsure.
diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
index d0ffa35f50e8..d1902102060b 100644
--- a/drivers/nfc/s3fwrn5/Makefile
+++ b/drivers/nfc/s3fwrn5/Makefile
@@ -5,6 +5,8 @@
 
 s3fwrn5-objs = core.o firmware.o nci.o
 s3fwrn5_i2c-objs = i2c.o
+s3fwrn82_uart-objs = uart.o
 
 obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
 obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
+obj-$(CONFIG_NFC_S3FWRN82_UART) += s3fwrn82_uart.o
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
new file mode 100644
index ..b3c36a5b28d3
--- /dev/null
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UART Link Layer for S3FWRN82 NCI based Driver
+ *
+ * Copyright (C) 2020 Samsung Electronics
+ * Author: Bongsu Jeon 
+ * All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "s3fwrn5.h"
+
+#define S3FWRN82_UART_DRIVER_NAME "s3fwrn82_uart"
+#define S3FWRN82_NCI_HEADER 3
+#define S3FWRN82_NCI_IDX 2
+#define S3FWRN82_EN_WAIT_TIME 20
+#define NCI_SKB_BUFF_LEN 258
+
+struct s3fwrn82_uart_phy {
+   struct serdev_device *ser_dev;
+   struct nci_dev *ndev;
+   struct sk_buff *recv_skb;
+
+   unsigned int gpio_en;
+   unsigned int gpio_fw_wake;
+
+   /* mutex is used to synchronize */
+   struct mutex mutex;
+   enum s3fwrn5_mode mode;
+};
+
+static void s3fwrn82_uart_set_wake(void *phy_id, bool wake)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+
+   mutex_lock(>mutex);
+   gpio_set_value(phy->gpio_fw_wake, wake);
+   msleep(S3FWRN82_EN_WAIT_TIME);
+   mutex_unlock(>mutex);
+}
+
+static void s3fwrn82_uart_set_mode(void *phy_id, enum s3fwrn5_mode mode)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+
+   mutex_lock(>mutex);
+   if (phy->mode == mode)
+   goto out;
+   phy->mode = mode;
+   gpio_set_value(phy->gpio_en, 1);
+   gpio_set_value(phy->gpio_fw_wake, 0);
+   if (mode == S3FWRN5_MODE_FW)
+   gpio_set_value(phy->gpio_fw_wake, 1);
+   if (mode != S3FWRN5_MODE_COLD) {
+   msleep(S3FWRN82_EN_WAIT_TIME);
+   gpio_set_value(phy->gpio_en, 0);
+   msleep(S3FWRN82_EN_WAIT_TIME);
+   }
+out:
+   mutex_unlock(>mutex);
+}
+
+static enum s3fwrn5_mode s3fwrn82_uart_get_mode(void *phy_id)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+   enum s3fwrn5_mode mode;
+
+   mutex_lock(>mutex);
+   mode = phy->mode;
+   mutex_unlock(>mutex);
+   return mode;
+}
+
+static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
+{
+   struct s3fwrn82_uart_phy *phy = phy_id;
+   int err;
+
+   err = serdev_device_write(phy->ser_dev,
+ out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+   if (err < 0)
+   return err;
+
+   return 0;
+}
+
+static const struct s3fwrn5_phy_ops uart_phy_ops = {
+   .set_wake = s3fwrn82_uart_set_wake,
+   .set_mode = s3fwrn82_uart_set_mode,
+   .get_mode = s3fwrn82_uart_get_mode,
+   .write = s3fwrn82_uart_write,
+};
+
+static int s3fwrn82_uart_read(struct serdev_device *serdev,
+ const unsigned char *data,
+ size_t count)
+{
+   struct s3fwrn82_uart_phy *phy = serdev_device_get_drvdata(serdev);
+   size_t i;
+
+   for (i = 0; i < count; i++) {
+   skb_put_u8(phy->recv_skb, *data++);
+
+   if (phy->recv_skb->len < S3FWRN82_NCI_HEADER)
+   continue;
+
+   if ((phy-&

Re: [PATCH net-next] net/nfc/nci: Support NCI 2.x initial sequence

[Bongsu Jeon]

On 11/18/20, Jakub Kicinski  wrote:
> On Tue, 17 Nov 2020 14:37:59 +0900 Bongsu Jeon wrote:
>> implement the NCI 2.x initial sequence to support NCI 2.x NFCC.
>> Since NCI 2.0, CORE_RESET and CORE_INIT sequence have been changed.
>> If NFCEE supports NCI 2.x, then NCI 2.x initial sequence will work.
>>
>> In NCI 1.0, Initial sequence and payloads are as below:
>> (DH) (NFCC)
>>  |  -- CORE_RESET_CMD --> |
>>  |  <-- CORE_RESET_RSP -- |
>>  |  -- CORE_INIT_CMD -->  |
>>  |  <-- CORE_INIT_RSP --  |
>>  CORE_RESET_RSP payloads are Status, NCI version, Configuration Status.
>>  CORE_INIT_CMD payloads are empty.
>>  CORE_INIT_RSP payloads are Status, NFCC Features,
>> Number of Supported RF Interfaces, Supported RF Interface,
>> Max Logical Connections, Max Routing table Size,
>> Max Control Packet Payload Size, Max Size for Large Parameters,
>> Manufacturer ID, Manufacturer Specific Information.
>>
>> In NCI 2.0, Initial Sequence and Parameters are as below:
>> (DH) (NFCC)
>>  |  -- CORE_RESET_CMD --> |
>>  |  <-- CORE_RESET_RSP -- |
>>  |  <-- CORE_RESET_NTF -- |
>>  |  -- CORE_INIT_CMD -->  |
>>  |  <-- CORE_INIT_RSP --  |
>>  CORE_RESET_RSP payloads are Status.
>>  CORE_RESET_NTF payloads are Reset Trigger,
>> Configuration Status, NCI Version, Manufacturer ID,
>> Manufacturer Specific Information Length,
>> Manufacturer Specific Information.
>>  CORE_INIT_CMD payloads are Feature1, Feature2.
>>  CORE_INIT_RSP payloads are Status, NFCC Features,
>> Max Logical Connections, Max Routing Table Size,
>> Max Control Packet Payload Size,
>> Max Data Packet Payload Size of the Static HCI Connection,
>> Number of Credits of the Static HCI Connection,
>> Max NFC-V RF Frame Size, Number of Supported RF Interfaces,
>> Supported RF Interfaces.
>>
>> Signed-off-by: Bongsu Jeon 
>
> Please fix the following sparse (build with C=1) warning:
>
> net/nfc/nci/ntf.c:42:17: warning: cast to restricted __le32
>
>> +__u8 status = 0;
>
> Please don't use the __u types in the normal kernel code, those are
> types for user space ABI.
>

Thanks for reviewing my patch.
I will change the code to fix it
and then resend my patch with version2.

Re: [PATCH net-next v2 1/3] nfc: s3fwrn5: Remove the max_payload

[Bongsu Jeon]

On Tue, Nov 17, 2020 at 5:39 PM Krzysztof Kozlowski  wrote:
>
> On Tue, 17 Nov 2020 at 09:14, Bongsu Jeon  wrote:
> >
> > 2020-11-17 16:42 GMT+09:00, k...@kernel.org :
> > > On Tue, Nov 17, 2020 at 10:16:11AM +0900, Bongsu Jeon wrote:
> > >> max_payload is unused.
> > >
> > > Why did you resend the patch ignoring my review? I already provided you
> > > with a tag, so you should include it.
> > >
> > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> > >
> > > Reviewed-by: Krzysztof Kozlowski 
> > >
> > > Best regards,
> > > Krzysztof
> > >
> >
> > Sorry about that. I included the tag.
>
> You need to reduce the rate of sending new patches. You sent v1. Then
> you sent again v1, which I reviewed. Then you send v2 without my
> review. So I provided a review. Then you sent again a v2 with my
> reviewed tags. So there are two v1 patches and two v2. Since I
> provided you the review tags for v2, no need to send v2 again. It
> confuses.
>
> Best regards,
> Krzysztof

Sorry to confuse you.
I made a mistake because I thought that you asked me
 to resend the patches with the new version(v2).
I think you intended that I need to version the patches and
describe changes when I update the patches next time.

Thanks a lot for reviewing my patches.

Re: [PATCH net-next v2 1/3] nfc: s3fwrn5: Remove the max_payload

[Bongsu Jeon]

2020-11-17 16:42 GMT+09:00, k...@kernel.org :
> On Tue, Nov 17, 2020 at 10:16:11AM +0900, Bongsu Jeon wrote:
>> max_payload is unused.
>
> Why did you resend the patch ignoring my review? I already provided you
> with a tag, so you should include it.
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> Reviewed-by: Krzysztof Kozlowski 
>
> Best regards,
> Krzysztof
>

Sorry about that. I included the tag.

[PATCH net-next v2 3/3] nfc: s3fwrn5: Change the error code

[Bongsu Jeon]

ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/s3fwrn5.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index 9d5f34759225..bb8f936d13a2 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -44,7 +44,7 @@ static inline int s3fwrn5_set_mode(struct s3fwrn5_info *info,
enum s3fwrn5_mode mode)
 {
if (!info->phy_ops->set_mode)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
info->phy_ops->set_mode(info->phy_id, mode);
 
@@ -54,7 +54,7 @@ static inline int s3fwrn5_set_mode(struct s3fwrn5_info *info,
 static inline enum s3fwrn5_mode s3fwrn5_get_mode(struct s3fwrn5_info *info)
 {
if (!info->phy_ops->get_mode)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
return info->phy_ops->get_mode(info->phy_id);
 }
@@ -62,7 +62,7 @@ static inline enum s3fwrn5_mode s3fwrn5_get_mode(struct 
s3fwrn5_info *info)
 static inline int s3fwrn5_set_wake(struct s3fwrn5_info *info, bool wake)
 {
if (!info->phy_ops->set_wake)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
info->phy_ops->set_wake(info->phy_id, wake);
 
@@ -72,7 +72,7 @@ static inline int s3fwrn5_set_wake(struct s3fwrn5_info *info, 
bool wake)
 static inline int s3fwrn5_write(struct s3fwrn5_info *info, struct sk_buff *skb)
 {
if (!info->phy_ops->write)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
return info->phy_ops->write(info->phy_id, skb);
 }
-- 
2.17.1

[PATCH net-next v2 2/3] nfc: s3fwrn5: Fix the misspelling in a comment

[Bongsu Jeon]

stucture should be replaced by structure.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/firmware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/firmware.c b/drivers/nfc/s3fwrn5/firmware.c
index ec930ee2c847..4cde6dd5c019 100644
--- a/drivers/nfc/s3fwrn5/firmware.c
+++ b/drivers/nfc/s3fwrn5/firmware.c
@@ -266,7 +266,7 @@ static int s3fwrn5_fw_complete_update_mode(struct 
s3fwrn5_fw_info *fw_info)
 }
 
 /*
- * Firmware header stucture:
+ * Firmware header structure:
  *
  * 0x00 - 0x0B : Date and time string (w/o NUL termination)
  * 0x10 - 0x13 : Firmware version
-- 
2.17.1

[PATCH net-next v2 1/3] nfc: s3fwrn5: Remove the max_payload

[Bongsu Jeon]

max_payload is unused.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/core.c| 3 +--
 drivers/nfc/s3fwrn5/i2c.c | 4 +---
 drivers/nfc/s3fwrn5/s3fwrn5.h | 3 +--
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index ba6c486d6465..f8e5d78d9078 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -136,7 +136,7 @@ static struct nci_ops s3fwrn5_nci_ops = {
 };
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-   const struct s3fwrn5_phy_ops *phy_ops, unsigned int max_payload)
+   const struct s3fwrn5_phy_ops *phy_ops)
 {
struct s3fwrn5_info *info;
int ret;
@@ -148,7 +148,6 @@ int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, 
struct device *pdev,
info->phy_id = phy_id;
info->pdev = pdev;
info->phy_ops = phy_ops;
-   info->max_payload = max_payload;
mutex_init(>mutex);
 
s3fwrn5_set_mode(info, S3FWRN5_MODE_COLD);
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index dc995286be84..0ffa389066a0 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -19,7 +19,6 @@
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_I2C_MAX_PAYLOAD 32
 #define S3FWRN5_EN_WAIT_TIME 150
 
 struct s3fwrn5_i2c_phy {
@@ -248,8 +247,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
if (ret < 0)
return ret;
 
-   ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev, _phy_ops,
-   S3FWRN5_I2C_MAX_PAYLOAD);
+   ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev, _phy_ops);
if (ret < 0)
return ret;
 
diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index ede68bb5eeae..9d5f34759225 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -34,7 +34,6 @@ struct s3fwrn5_info {
struct device *pdev;
 
const struct s3fwrn5_phy_ops *phy_ops;
-   unsigned int max_payload;
 
struct s3fwrn5_fw_info fw_info;
 
@@ -79,7 +78,7 @@ static inline int s3fwrn5_write(struct s3fwrn5_info *info, 
struct sk_buff *skb)
 }
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-   const struct s3fwrn5_phy_ops *phy_ops, unsigned int max_payload);
+   const struct s3fwrn5_phy_ops *phy_ops);
 void s3fwrn5_remove(struct nci_dev *ndev);
 
 int s3fwrn5_recv_frame(struct nci_dev *ndev, struct sk_buff *skb,
-- 
2.17.1

[PATCH net-next] net/nfc/nci: Support NCI 2.x initial sequence

[Bongsu Jeon]

implement the NCI 2.x initial sequence to support NCI 2.x NFCC.
Since NCI 2.0, CORE_RESET and CORE_INIT sequence have been changed.
If NFCEE supports NCI 2.x, then NCI 2.x initial sequence will work.

In NCI 1.0, Initial sequence and payloads are as below:
(DH) (NFCC)
 |  -- CORE_RESET_CMD --> |
 |  <-- CORE_RESET_RSP -- |
 |  -- CORE_INIT_CMD -->  |
 |  <-- CORE_INIT_RSP --  |
 CORE_RESET_RSP payloads are Status, NCI version, Configuration Status.
 CORE_INIT_CMD payloads are empty.
 CORE_INIT_RSP payloads are Status, NFCC Features,
Number of Supported RF Interfaces, Supported RF Interface,
Max Logical Connections, Max Routing table Size,
Max Control Packet Payload Size, Max Size for Large Parameters,
Manufacturer ID, Manufacturer Specific Information.

In NCI 2.0, Initial Sequence and Parameters are as below:
(DH) (NFCC)
 |  -- CORE_RESET_CMD --> |
 |  <-- CORE_RESET_RSP -- |
 |  <-- CORE_RESET_NTF -- |
 |  -- CORE_INIT_CMD -->  |
 |  <-- CORE_INIT_RSP --  |
 CORE_RESET_RSP payloads are Status.
 CORE_RESET_NTF payloads are Reset Trigger,
Configuration Status, NCI Version, Manufacturer ID,
Manufacturer Specific Information Length,
Manufacturer Specific Information.
 CORE_INIT_CMD payloads are Feature1, Feature2.
 CORE_INIT_RSP payloads are Status, NFCC Features,
Max Logical Connections, Max Routing Table Size,
Max Control Packet Payload Size,
Max Data Packet Payload Size of the Static HCI Connection,
Number of Credits of the Static HCI Connection,
Max NFC-V RF Frame Size, Number of Supported RF Interfaces,
Supported RF Interfaces.

Signed-off-by: Bongsu Jeon 
---
 include/net/nfc/nci.h | 39 ++
 net/nfc/nci/core.c| 23 +++--
 net/nfc/nci/ntf.c | 21 
 net/nfc/nci/rsp.c | 75 +--
 4 files changed, 146 insertions(+), 12 deletions(-)

diff --git a/include/net/nfc/nci.h b/include/net/nfc/nci.h
index 0550e0380b8d..b434f984bc8c 100644
--- a/include/net/nfc/nci.h
+++ b/include/net/nfc/nci.h
@@ -25,6 +25,8 @@
 #define NCI_MAX_PARAM_LEN  251
 #define NCI_MAX_PAYLOAD_SIZE   255
 #define NCI_MAX_PACKET_SIZE258
+#define NCI_MAX_LARGE_PARAMS_NCI_v215
+#define NCI_VER_2_MASK 0x20
 
 /* NCI Status Codes */
 #define NCI_STATUS_OK  0x00
@@ -131,6 +133,9 @@
 #define NCI_LF_CON_BITR_F_212  0x02
 #define NCI_LF_CON_BITR_F_424  0x04
 
+/* NCI 2.x Feature Enable Bit */
+#define NCI_FEATURE_DISABLE0x00
+
 /* NCI Reset types */
 #define NCI_RESET_TYPE_KEEP_CONFIG 0x00
 #define NCI_RESET_TYPE_RESET_CONFIG0x01
@@ -220,6 +225,11 @@ struct nci_core_reset_cmd {
 } __packed;
 
 #define NCI_OP_CORE_INIT_CMD   nci_opcode_pack(NCI_GID_CORE, 0x01)
+/* To support NCI 2.x */
+struct nci_core_init_v2_cmd {
+   __u8feature1;
+   __u8feature2;
+} __packed;
 
 #define NCI_OP_CORE_SET_CONFIG_CMD nci_opcode_pack(NCI_GID_CORE, 0x02)
 struct set_config_param {
@@ -316,6 +326,11 @@ struct nci_core_reset_rsp {
__u8config_status;
 } __packed;
 
+/* To support NCI ver 2.x */
+struct nci_core_reset_rsp_nci_ver2 {
+   __u8status;
+} __packed;
+
 #define NCI_OP_CORE_INIT_RSP   nci_opcode_pack(NCI_GID_CORE, 0x01)
 struct nci_core_init_rsp_1 {
__u8status;
@@ -334,6 +349,20 @@ struct nci_core_init_rsp_2 {
__le32  manufact_specific_info;
 } __packed;
 
+/* To support NCI ver 2.x */
+struct nci_core_init_rsp_nci_ver2 {
+   __u8status;
+   __le32  nfcc_features;
+   __u8max_logical_connections;
+   __le16  max_routing_table_size;
+   __u8max_ctrl_pkt_payload_len;
+   __u8max_data_pkt_hci_payload_len;
+   __u8number_of_hci_credit;
+   __le16  max_nfc_v_frame_size;
+   __u8num_supported_rf_interfaces;
+   __u8supported_rf_interfaces[];
+} __packed;
+
 #define NCI_OP_CORE_SET_CONFIG_RSP nci_opcode_pack(NCI_GID_CORE, 0x02)
 struct nci_core_set_config_rsp {
__u8status;
@@ -372,6 +401,16 @@ struct nci_nfcee_discover_rsp {
 /* --- */
 /*  NCI Notifications  */
 /* --- */
+#define NCI_OP_CORE_RESET_NTF  nci_opcode_pack(NCI_GID_CORE, 0x00)
+struct nci_core_reset_ntf {
+   __u8reset_trigger;
+   __u8config_status;
+   __u8nci_ver;
+   __u8manufact_id;
+   __u8manufacturer_specific_len;
+   __u32   manufact_specific_info;
+} __packed;
+
 #define NCI_OP_CORE_CONN_CREDITS_NTF   nci_opcode_pack(NCI_GID_CORE, 0x06)
 struct conn_credit_entry {
  

[PATCH net-next v2 3/3] nfc: s3fwrn5: Change the error code

[Bongsu Jeon]

ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/s3fwrn5.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index 9d5f34759225..bb8f936d13a2 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -44,7 +44,7 @@ static inline int s3fwrn5_set_mode(struct s3fwrn5_info *info,
enum s3fwrn5_mode mode)
 {
if (!info->phy_ops->set_mode)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
info->phy_ops->set_mode(info->phy_id, mode);
 
@@ -54,7 +54,7 @@ static inline int s3fwrn5_set_mode(struct s3fwrn5_info *info,
 static inline enum s3fwrn5_mode s3fwrn5_get_mode(struct s3fwrn5_info *info)
 {
if (!info->phy_ops->get_mode)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
return info->phy_ops->get_mode(info->phy_id);
 }
@@ -62,7 +62,7 @@ static inline enum s3fwrn5_mode s3fwrn5_get_mode(struct 
s3fwrn5_info *info)
 static inline int s3fwrn5_set_wake(struct s3fwrn5_info *info, bool wake)
 {
if (!info->phy_ops->set_wake)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
info->phy_ops->set_wake(info->phy_id, wake);
 
@@ -72,7 +72,7 @@ static inline int s3fwrn5_set_wake(struct s3fwrn5_info *info, 
bool wake)
 static inline int s3fwrn5_write(struct s3fwrn5_info *info, struct sk_buff *skb)
 {
if (!info->phy_ops->write)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
return info->phy_ops->write(info->phy_id, skb);
 }
-- 
2.17.1

[PATCH net-next v2 2/3] nfc: s3fwrn5: Fix the misspelling in a comment

[Bongsu Jeon]

stucture should be replaced by structure.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/firmware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/firmware.c b/drivers/nfc/s3fwrn5/firmware.c
index ec930ee2c847..4cde6dd5c019 100644
--- a/drivers/nfc/s3fwrn5/firmware.c
+++ b/drivers/nfc/s3fwrn5/firmware.c
@@ -266,7 +266,7 @@ static int s3fwrn5_fw_complete_update_mode(struct 
s3fwrn5_fw_info *fw_info)
 }
 
 /*
- * Firmware header stucture:
+ * Firmware header structure:
  *
  * 0x00 - 0x0B : Date and time string (w/o NUL termination)
  * 0x10 - 0x13 : Firmware version
-- 
2.17.1

[PATCH net-next v2 1/3] nfc: s3fwrn5: Remove the max_payload

[Bongsu Jeon]

max_payload is unused.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/core.c| 3 +--
 drivers/nfc/s3fwrn5/i2c.c | 4 +---
 drivers/nfc/s3fwrn5/s3fwrn5.h | 3 +--
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index ba6c486d6465..f8e5d78d9078 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -136,7 +136,7 @@ static struct nci_ops s3fwrn5_nci_ops = {
 };
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-   const struct s3fwrn5_phy_ops *phy_ops, unsigned int max_payload)
+   const struct s3fwrn5_phy_ops *phy_ops)
 {
struct s3fwrn5_info *info;
int ret;
@@ -148,7 +148,6 @@ int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, 
struct device *pdev,
info->phy_id = phy_id;
info->pdev = pdev;
info->phy_ops = phy_ops;
-   info->max_payload = max_payload;
mutex_init(>mutex);
 
s3fwrn5_set_mode(info, S3FWRN5_MODE_COLD);
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index dc995286be84..0ffa389066a0 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -19,7 +19,6 @@
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_I2C_MAX_PAYLOAD 32
 #define S3FWRN5_EN_WAIT_TIME 150
 
 struct s3fwrn5_i2c_phy {
@@ -248,8 +247,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
if (ret < 0)
return ret;
 
-   ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev, _phy_ops,
-   S3FWRN5_I2C_MAX_PAYLOAD);
+   ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev, _phy_ops);
if (ret < 0)
return ret;
 
diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index ede68bb5eeae..9d5f34759225 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -34,7 +34,6 @@ struct s3fwrn5_info {
struct device *pdev;
 
const struct s3fwrn5_phy_ops *phy_ops;
-   unsigned int max_payload;
 
struct s3fwrn5_fw_info fw_info;
 
@@ -79,7 +78,7 @@ static inline int s3fwrn5_write(struct s3fwrn5_info *info, 
struct sk_buff *skb)
 }
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-   const struct s3fwrn5_phy_ops *phy_ops, unsigned int max_payload);
+   const struct s3fwrn5_phy_ops *phy_ops);
 void s3fwrn5_remove(struct nci_dev *ndev);
 
 int s3fwrn5_recv_frame(struct nci_dev *ndev, struct sk_buff *skb,
-- 
2.17.1

[PATCH net-next v2 0/3] nfc: s3fwrn5: Refactor the s3fwrn5 driver

[Bongsu Jeon]

Changes from v1:
- Remove the trailing dot from subject.
- Remove an empty line at beginning of commit message.
- Add a commit message.

Bongsu Jeon (3):
  nfc: s3fwrn5: Remove the max_payload
  nfc: s3fwrn5: Fix the misspelling in a comment
  nfc: s3fwrn5: Change the error code

 drivers/nfc/s3fwrn5/core.c |  3 +--
 drivers/nfc/s3fwrn5/firmware.c |  2 +-
 drivers/nfc/s3fwrn5/i2c.c  |  4 +---
 drivers/nfc/s3fwrn5/s3fwrn5.h  | 11 +--
 4 files changed, 8 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH net-next 3/3] nfc: s3fwrn5: Change the error code

[Bongsu Jeon]

ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/s3fwrn5.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index 9d5f34759225..bb8f936d13a2 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -44,7 +44,7 @@ static inline int s3fwrn5_set_mode(struct s3fwrn5_info *info,
enum s3fwrn5_mode mode)
 {
if (!info->phy_ops->set_mode)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
info->phy_ops->set_mode(info->phy_id, mode);
 
@@ -54,7 +54,7 @@ static inline int s3fwrn5_set_mode(struct s3fwrn5_info *info,
 static inline enum s3fwrn5_mode s3fwrn5_get_mode(struct s3fwrn5_info *info)
 {
if (!info->phy_ops->get_mode)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
return info->phy_ops->get_mode(info->phy_id);
 }
@@ -62,7 +62,7 @@ static inline enum s3fwrn5_mode s3fwrn5_get_mode(struct 
s3fwrn5_info *info)
 static inline int s3fwrn5_set_wake(struct s3fwrn5_info *info, bool wake)
 {
if (!info->phy_ops->set_wake)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
info->phy_ops->set_wake(info->phy_id, wake);
 
@@ -72,7 +72,7 @@ static inline int s3fwrn5_set_wake(struct s3fwrn5_info *info, 
bool wake)
 static inline int s3fwrn5_write(struct s3fwrn5_info *info, struct sk_buff *skb)
 {
if (!info->phy_ops->write)
-   return -ENOTSUPP;
+   return -EOPNOTSUPP;
 
return info->phy_ops->write(info->phy_id, skb);
 }
-- 
2.17.1

[PATCH net-next 2/3] nfc: s3fwrn5: Fix the misspelling in a comment

[Bongsu Jeon]

stucture should be replaced by structure.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/firmware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/firmware.c b/drivers/nfc/s3fwrn5/firmware.c
index ec930ee2c847..4cde6dd5c019 100644
--- a/drivers/nfc/s3fwrn5/firmware.c
+++ b/drivers/nfc/s3fwrn5/firmware.c
@@ -266,7 +266,7 @@ static int s3fwrn5_fw_complete_update_mode(struct 
s3fwrn5_fw_info *fw_info)
 }
 
 /*
- * Firmware header stucture:
+ * Firmware header structure:
  *
  * 0x00 - 0x0B : Date and time string (w/o NUL termination)
  * 0x10 - 0x13 : Firmware version
-- 
2.17.1

[PATCH net-next 1/3] nfc: s3fwrn5: Remove the max_payload

[Bongsu Jeon]

max_payload is unused.

Signed-off-by: Bongsu Jeon 
---
 drivers/nfc/s3fwrn5/core.c| 3 +--
 drivers/nfc/s3fwrn5/i2c.c | 4 +---
 drivers/nfc/s3fwrn5/s3fwrn5.h | 3 +--
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index ba6c486d6465..f8e5d78d9078 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -136,7 +136,7 @@ static struct nci_ops s3fwrn5_nci_ops = {
 };
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-   const struct s3fwrn5_phy_ops *phy_ops, unsigned int max_payload)
+   const struct s3fwrn5_phy_ops *phy_ops)
 {
struct s3fwrn5_info *info;
int ret;
@@ -148,7 +148,6 @@ int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, 
struct device *pdev,
info->phy_id = phy_id;
info->pdev = pdev;
info->phy_ops = phy_ops;
-   info->max_payload = max_payload;
mutex_init(>mutex);
 
s3fwrn5_set_mode(info, S3FWRN5_MODE_COLD);
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index dc995286be84..0ffa389066a0 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -19,7 +19,6 @@
 
 #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
 
-#define S3FWRN5_I2C_MAX_PAYLOAD 32
 #define S3FWRN5_EN_WAIT_TIME 150
 
 struct s3fwrn5_i2c_phy {
@@ -248,8 +247,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client,
if (ret < 0)
return ret;
 
-   ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev, _phy_ops,
-   S3FWRN5_I2C_MAX_PAYLOAD);
+   ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev, _phy_ops);
if (ret < 0)
return ret;
 
diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index ede68bb5eeae..9d5f34759225 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -34,7 +34,6 @@ struct s3fwrn5_info {
struct device *pdev;
 
const struct s3fwrn5_phy_ops *phy_ops;
-   unsigned int max_payload;
 
struct s3fwrn5_fw_info fw_info;
 
@@ -79,7 +78,7 @@ static inline int s3fwrn5_write(struct s3fwrn5_info *info, 
struct sk_buff *skb)
 }
 
 int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
-   const struct s3fwrn5_phy_ops *phy_ops, unsigned int max_payload);
+   const struct s3fwrn5_phy_ops *phy_ops);
 void s3fwrn5_remove(struct nci_dev *ndev);
 
 int s3fwrn5_recv_frame(struct nci_dev *ndev, struct sk_buff *skb,
-- 
2.17.1