Add support for GPIO line names via libgpiod by mhei · Pull Request #4 · EVerest/linux_libnfc-nci

mhei · 2026-04-29T15:52:09Z

The good old sysfs based user-space interface with GPIO numbers is obsolete since several years.

This PR add support for an alternative approach: instead of plain integers, user can configure the required GPIO lines using a GPIO line name as usually defined in the Device Tree for the board. For this, this PR relys on libgpiod. Support for using libgpiod can be switched on at compile time, to keep backwards compatiblity, it is disabled by default.

The new dependency is not pulled in automatically but is expected to be provided by the environment.

The tested libgpiod version is v2.0.1 - since this is currently the version that we (chargebyte) bundle in our kirkstone builds on our targets.

Even if the library is compiled with libgpiod support, then it is still possible to use integer numbers and the older sysfs interface.

Sidenote 1: I pulled in the commit from #3 - just wanted to have it included when testing. I can remove it once there is a decision/feedback in that PR.

Sidenote 2: I tested only on our Charge SOM platform so far and fixed a 64-bit issue from an older commit in this branch.

I'm unsure regarding the ADDITIONAL_CONTENT stuff in the packaging file - this is untested and I hope someone with more expertise can comment on it.

barsnick · 2026-05-19T08:00:05Z

@cburandt Friendly ping ;-)

mhei · 2026-06-08T17:23:54Z

Any comments, positive or negative, are welcome, as is regression testing on existing platforms.

mhei · 2026-06-11T14:58:06Z

I had problems to cross-compile the library and the PN7160TokenProvider using Yocto kirkstone, so I added a commit here to create a shared library. This requires also a small change in the CMake file of PN7160TokenProvider, you can find it here as example: https://github.com/chargebyte/everest-core/commits/feature/pn7160-as-shared-lib/

mhei · 2026-06-29T08:38:23Z

Just FYI: I'm already working on the next improvement step (dynmaic config via EVerest module config), see the branch here: https://github.com/chargebyte/linux_libnfc-nci/tree/feature/libgpiod-with-cfg-call I'm just wondering, whether I should merge this into this PR here? (corresponding branch for everest-core: https://github.com/chargebyte/everest-core/tree/feature/pn7160-as-shared-lib-with-dyn-cfg)

cburandt

Sorry for being unresponsive. I did a quick test with hardware and that required a small change in order to work (see below).

I will have another look at the ADDITIONAL_CONTENT and the libgpiod related things soon.

cburandt · 2026-07-16T16:43:22Z

  int value;
-  int isfound = GetNxpNumValue(name, &value, sizeof(&value));
+  int isfound = GetNxpNumValue(name, &value, sizeof(value));
  if (isfound > 0) {
    return value;
  }


Suggested change

int value;

int isfound = GetNxpNumValue(name, &value, sizeof(&value));

int isfound = GetNxpNumValue(name, &value, sizeof(value));

if (isfound > 0) {

return value;

}

// GetNxpNumValue only dispatches on sizeof(unsigned long long/short/char)

// (see above); a 4-byte int hits the default case and returns false. Use unsigned

// long so the length matches a supported case.

unsigned long value = 0;

int isfound = GetNxpNumValue(name, &value, sizeof(value));

if (isfound > 0) {

return (int)value;

}

Had to do this change in order to make it work on a test setup (RPi5+I2C-attached eval board).

Good catch! But for some reason, I cannot add your suggestion directly here with Github... will do with new commit and to be on the safe side for later, I'd add the "int" case to GetNxpNumValue, too.

lategoodbye · 2026-07-27T17:52:21Z

@cburandt Did you had the time to test Michael's changes?

cburandt · 2026-07-28T15:42:25Z

@cburandt Did you had the time to test Michael's changes?

Hello @lategoodbye , @mhei I started a review now (but will likely finish not today, but tomorrow).

cburandt

LGTM in general.

Find some comments below.
Don't consider the deleting of the "virtual" keywords a mandatory change; I prefer do have only one of virtual/override/final, but that's more a matter of taste (and accordance with the core guidelines: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final)

Concerning the "ADDITIONAL_CONTENT" in the CMakeLists.txt, I think it's not required.

cburandt · 2026-07-28T07:01:05Z

   **
   ****************************************************************************/
-  int NfccReset(void* pDevHandle, NfccResetType eType);
+  virtual int NfccReset(void* pDevHandle, NfccResetType eType) override;


Suggested change

virtual int NfccReset(void* pDevHandle, NfccResetType eType) override;

int NfccReset(void* pDevHandle, NfccResetType eType) override;

cburandt · 2026-07-28T07:01:16Z

   ** Returns          None
   ****************************************************************************/
-  void EnableFwDnldMode(bool mode);
+  virtual void EnableFwDnldMode(bool mode) override;


Suggested change

virtual void EnableFwDnldMode(bool mode) override;

void EnableFwDnldMode(bool mode) override;

cburandt · 2026-07-28T07:01:24Z

   ** Returns           Current mode download/NCI
   ****************************************************************************/
-  bool_t IsFwDnldModeEnabled(void);
+  virtual bool_t IsFwDnldModeEnabled(void) override;


Suggested change

virtual bool_t IsFwDnldModeEnabled(void) override;

bool_t IsFwDnldModeEnabled(void) override;

cburandt · 2026-07-28T07:04:47Z

-  int GetIrqState(void* pDevHandle);
-  int GetNfcState(void* pDevHandle);
+   ****************************************************************************/
+  virtual int GetIrqState(void* pDevHandle) override;


Suggested change

virtual int GetIrqState(void* pDevHandle) override;

int GetIrqState(void* pDevHandle) override;

cburandt · 2026-07-28T07:04:59Z

+   ** Returns          None
+   **
+   ****************************************************************************/
+  virtual void Close(void *pDevHandle) override;


Suggested change

virtual void Close(void *pDevHandle) override;

void Close(void *pDevHandle) override;

cburandt · 2026-07-28T07:14:07Z

  *****************************************************************************/
-  int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite);
-  void Close(void* pDevHandle);
+  virtual int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite) override;


Suggested change

virtual int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite) override;

int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite) override;

cburandt · 2026-07-28T07:15:11Z

   **
   ****************************************************************************/
-  NFCSTATUS OpenAndConfigure(pphTmlNfc_Config_t pConfig, void** pLinkHandle);
+  virtual NFCSTATUS OpenAndConfigure(pphTmlNfc_Config_t pConfig, void** pLinkHandle) override;


Suggested change

virtual NFCSTATUS OpenAndConfigure(pphTmlNfc_Config_t pConfig, void** pLinkHandle) override;

NFCSTATUS OpenAndConfigure(pphTmlNfc_Config_t pConfig, void** pLinkHandle) override;

cburandt · 2026-07-28T07:15:20Z

   **
   ****************************************************************************/
-  int Read(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToRead);
+  virtual int Read(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToRead) override;


Suggested change

virtual int Read(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToRead) override;

int Read(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToRead) override;

cburandt · 2026-07-28T07:15:29Z

  *****************************************************************************/
-  int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite);
-  void Close(void* pDevHandle);
+  virtual int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite) override;


Suggested change

virtual int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite) override;

int Write(void* pDevHandle, uint8_t* pBuffer, int nNbBytesToWrite) override;

cburandt · 2026-07-29T15:14:49Z

+            "set(LIBNFCNCI_LIBGPIOD @LIBNFCNCI_LIBGPIOD@)"
+            "if(LIBNFCNCI_LIBGPIOD)"
+            "    find_dependency(PkgConfig)"
+            "    pkg_search_module(LIBGPIOD REQUIRED libgpiodcxx)"
+            "endif()"


Did you find this to be required in testing? Pretty sure this is not: You switched to building a shared lib which depends only PRIVATE-ly on libgpiod. Provided libgpiod is a static lib, it will have become part of the nfc-nci lib already, so no need for the client to find it. Same for thread.
But I didn't test.

mhei · 2026-07-30T10:14:16Z

Thanks for your feedback. I will look into it and address the points after returning from holidays next week.