linux_libnfc-nci: Question About Deadlock Commit in API

[tduong]

I'm using PN7160 with linux_libnfc-nci library. 

I noticed that the master branch of linux_libnfc-nci has this commit that supposedly prevents deadlock during MIFARE Classic formatting operation.

I noticed that this change does not exist for the NCI2.0_PN7160 branch (line 416-418).  Is this intentional? 

[danielchen]

Hi Tduong:

According to Linux porting guider AN13287,  clone the Linux libnfc-nci stack repository

In file nativeNfcManager.cpp , I can see this change.  existed.

Thanks for your feedback.

Regards

Daniel

[danielchen]

This file moved to src/nxp_nci_hal_nfc/nci/jni

[tduong]

Thanks for the quick reply.  Yes, I am aware that the file appears to have moved, but my point is that the "fix" to the deadlock on the master branch was to remove the "if (gIsSelectingRfInterface)" check.  It now always invokes nativeNfcTag_doCconnectStatus(false) whenenver eventData->status != NFA_STATUS_OK:

Master:

NCI2.0_PN7160:

[danielchen]

Hi Tduong:

Do you see any functionality or performance loss at your side?

Regards

Daniel

[tduong]

Yes, I'm currently troubleshooting a situation where PN7160 randomly stops detecting any cards, and our user-space application requires a restart in order to get things back into a working state again.  Our customers have run into this problem, but this is not easily reproducible so there aren't logs to help troubleshoot, but randomness suggests it could be some kind of race condition that causes a deadlock, which led me to look through the NCI code, and this particular commit caught my attention because all our customers running into this are using MiFare Classics. 

[danielchen]

Hi @tduong :

Could you please   add the "if (gIsSelectingRfInterface)" check, and test it on your side to see the result?

Regards

Daniel

[tduong]

Hey Daniel. Thanks for the reply, but that would be opposite of what the fix on the master branch did.  The fix on the master branch *removed* the "if" check.  The PN7160 branch still contains this "if" check.  I can just as easily remove it on the PN7160 branch, but seeing as how this is maintained by NXP, I'm asking if this change is appropriate for the PN7160 branch. One problem with race conditions (and deadlocks that can happen as a result) is that - by their nature - we're not going to be able to easily replicate them.  So if I make this change, and tested a few times, I wouldn't know if I introduced or fixed any problems.  This is why I'm posting in this forum to get a second, hopefully more expert, opinion. 

Appreciate your time!

[danielchen]

The master branch is for PN7150.

I will forward your concern to our develop team.   If there is any feedback, I will let you know

Regards

Daniel