Hi Samuel,
I think my last last answer:
"I believe a "complete regardless of the event" we are receiving (on the
loopback gate) may hide a CLF firmware bug and should not happen...
Are you in favour of hiding a potential bug ?"
Was not so smart from my side.
You are right considering a system should not be locked because of a
buggy NFC controller.
Also an error message will be enough to notify a potential bug...
I will apply your remark where possible.
Sorry for that.
Best Regards
Christophe
On 24/10/2015 14:19, Christophe Ricard wrote:
Hi Samuel,
On 24/10/2015 08:48, Samuel Ortiz wrote:
Hi Christophe,
Just a couple of nitpicks:
On Tue, Oct 20, 2015 at 11:48:09PM +0200, Christophe Ricard wrote:
diff --git a/drivers/nfc/st-nci/Makefile b/drivers/nfc/st-nci/Makefile
index 348ce76..60e569b 100644
--- a/drivers/nfc/st-nci/Makefile
+++ b/drivers/nfc/st-nci/Makefile
@@ -2,7 +2,7 @@
# Makefile for ST21NFCB NCI based NFC driver
#
-st-nci-objs = ndlc.o core.o st-nci_se.o
+st-nci-objs = ndlc.o core.o st-nci_se.o st-nci_vendor_cmds.o
Please rename that file to vendor_cmds.c.
I pushed a change to rename st-nci_se.c to se.c already, to keep the
file names consistent there.
I think your latest change introduced a build break :(. The log
message mention also MFC instead of NFC ;).
"In file included from drivers/nfc/st-nci/ndlc.c:23:0:
drivers/nfc/st-nci/st-nci.h:22:23: fatal error: st-nci_se.h: No such
file or directory
#include "st-nci_se.h""
I will send a fix in the v2 ;).
obj-$(CONFIG_NFC_ST_NCI) += st-nci.o
st-nci_i2c-objs = i2c.o
diff --git a/drivers/nfc/st-nci/core.c b/drivers/nfc/st-nci/core.c
index c419d39..fd2a5ca 100644
--- a/drivers/nfc/st-nci/core.c
+++ b/drivers/nfc/st-nci/core.c
@@ -25,6 +25,7 @@
#include "st-nci.h"
#include "st-nci_se.h"
+#include "st-nci_vendor_cmds.h"
Ditto: Please rename it to vendor_cmds.h.
Don't you think it would be better for headers to either:
- merge st-nci_se.h and st-nci_vendor_cmds.h in st-nci.h
- or keep st-nci_ prefix for header files only.
Which option do you prefer ?
I have no concern to remove the st-nci prefix and the st21nfca for .c
file in their respective directory.
+void st_nci_hci_loopback_event_received(struct nci_dev *ndev, u8
event,
+ struct sk_buff *skb)
+{
+ struct st_nci_info *info = nci_get_drvdata(ndev);
+
+ switch (event) {
+ case ST_NCI_EVT_POST_DATA:
+ info->vendor_info.rx_skb = skb;
+
+ complete(&info->vendor_info.req_completion);
+ break;
+ }
Wouldn't it make sense to complete regardless of the event you're
receiving ?
Since you're checking for rx_skb after the completion, it's safe and
would prevent you from being stuck waiting for the right event in the
below routine.
The only reason for receiving a EVT_POST_DATA from the CLF loopback
gate should come from a
previous EVT_POST_DATA sent from the Device Host.
The loopback gate is a standard hci echo test mechanism.
Also, according to etsi 102 622 and ST implementation, the loopback
gate only support EVT_POST_DATA.
I believe a "complete regardless of the event" we are receiving (on
the loopback gate) may hide a CLF firmware bug and should not happen...
Are you in favour of hiding a potential bug ?
[snip]
Best Regards
Christophe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
