drivers/pn532: adapted to SPI interface changes by haukepetersen · Pull Request #6478 · RIOT-OS/RIOT

haukepetersen · 2017-01-26T09:05:06Z

missed in #4780: driver needed adaption to SPI interface changes for its SPI mode.

haukepetersen · 2017-01-26T09:05:57Z

build the test-app with PN532_MODE=spi BOARD=xxx make ...

cgundogan · 2017-01-26T10:00:19Z

drivers/pn532/pn532.c

    }
    else {
 #ifdef PN532_SUPPORT_SPI
-        ret = spi_init_master(dev->conf->spi, SPI_CONF_FIRST_RISING,


ret is initially set to -1 and this function returns ret => pn532_init always returns -1 this way and thusly fails

cgundogan · 2017-01-26T10:48:47Z

cf. haukepetersen#36 some fixes to get the pn532 driver working again (SPI_MODE)

haukepetersen · 2017-01-26T11:03:33Z

fixed init function and removed debug output from test makefile

cgundogan · 2017-01-26T11:04:42Z

@haukepetersen please look at haukepetersen#36 there is another bug

cgundogan · 2017-01-26T11:29:11Z

ACK works as expected now. Please squash 👍

haukepetersen · 2017-01-26T11:36:26Z

squashed.

cgundogan · 2017-01-26T11:41:03Z

drivers/pn532/pn532.c

-        }
+
+        buff[0] = 0x80;
+        reverse(buff, ret);


no idea what happened, but this fix didn't survive the squash operation (: please replace ret with len.

should be ok now.

lebrush

Changes look good except these two small comments. Thanks for the effort @haukepetersen :-) and thanks @cgundogan for testing !!!

lebrush · 2017-01-26T12:33:04Z

tests/driver_pn532/main.c

+#elif defined(PN532_SUPPORT_SPI)
+    ret = pn532_init_spi(&pn532, &pn532_conf[0]);
+#else
+  #error "Error: bus mode not defined"


This check is already included in pn532.h :-)

lebrush · 2017-01-26T12:36:19Z

drivers/pn532/pn532.c

    else {
 #ifdef PN532_SUPPORT_SPI
-        spi_acquire(dev->conf->spi);
+        spi_acquire(dev->conf->spi, GPIO_UNDEF, SPI_MODE, SPI_CLK);


Documentation in spi.h says cs shall be set to SPI_CS_UNDEF instead of GPIO_UNDEF

(also in the other places)

lebrush · 2017-01-26T12:39:21Z

Btw, I really like that the test application now supports spi too :-)

haukepetersen · 2017-01-26T13:47:50Z

Comments are addressed.

(also in the other places)

you are absolutely right, will propose a fix.

cgundogan

ACK from my side. @lebrush do you have any further comments on this one?

lebrush · 2017-01-26T14:27:09Z

Nope codewise looks fine :-) but now some boards are not happy about SPI... ~~maybe make i2c or spi the default one for the test application to solve this~~ this is already the case

/data/riotbuild/drivers/include/pn532.h:33:2: error: #error Please define PN532_SUPPORT_I2C and/or PN532_SUPPORT_SPI to enable the functionality on this device
 #error Please define PN532_SUPPORT_I2C and/or PN532_SUPPORT_SPI to enable \
  ^~~~~
/data/riotbuild/Makefile.base:64: recipe for target '/data/riotbuild/tests/driver_pn532/bin/calliope-mini/tests_driver_pn532/main.o' failed
make[1]: *** [/data/riotbuild/tests/driver_pn532/bin/calliope-mini/tests_driver_pn532/main.o] Error 1
make[1]: *** Waiting for unfinished jobs....
In file included from /data/riotbuild/drivers/pn532/pn532.c:26:0:
/data/riotbuild/drivers/include/pn532.h:33:2: error: #error Please define PN532_SUPPORT_I2C and/or PN532_SUPPORT_SPI to enable the functionality on this device
 #error Please define PN532_SUPPORT_I2C and/or PN532_SUPPORT_SPI to enable \
  ^~~~~
/data/riotbuild/Makefile.base:64: recipe for target '/data/riotbuild/tests/driver_pn532/bin/calliope-mini/pn532/pn532.o' failed
make[3]: *** [/data/riotbuild/tests/driver_pn532/bin/calliope-mini/pn532/pn532.o] Error 1
/data/riotbuild/Makefile.base:20: recipe for target 'ALL--/data/riotbuild/drivers/pn532' failed
make[2]: *** [ALL--/data/riotbuild/drivers/pn532] Error 2
/data/riotbuild/Makefile.base:20: recipe for target 'ALL--/data/riotbuild/drivers' failed
make[1]: *** [ALL--/data/riotbuild/drivers] Error 2
/data/riotbuild/Makefile.include:275: recipe for target 'all' failed
make: *** [all] Error 2

cgundogan · 2017-01-26T14:29:17Z

tests/driver_pn532/Makefile

+
+# select if you want to build the SPI or the I2C version of the driver:
+# set PN532_MODE to `i2c` or to `spi`
+TPN532_MODE ?= i2c


s/TPN532_MODE/PN532_MODE

haukepetersen · 2017-01-27T07:30:45Z

hm, that capital T must have accidentally slipped in. Buildtest now runs fine again locally...

cgundogan · 2017-01-27T09:52:44Z

tested again with my local setup. looks fine and both CIs agree => GO

PeterKietzmann · 2017-01-27T09:57:13Z

@lebrush can you please provide a backport to the 2017.01 release branch?

lebrush · 2017-01-27T10:44:22Z

👍 will do

Backport #6478

haukepetersen added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers labels Jan 26, 2017

haukepetersen added this to the Release 2017.01 milestone Jan 26, 2017

haukepetersen assigned lebrush and PeterKietzmann Jan 26, 2017

haukepetersen mentioned this pull request Jan 26, 2017

periph/spi: Leftovers from SPI rework in #4780 #6437

Closed

23 tasks

cgundogan self-assigned this Jan 26, 2017

cgundogan reviewed Jan 26, 2017

View reviewed changes

cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 26, 2017

haukepetersen force-pushed the fix_pm532_spiadapt branch from 3d80e72 to d32cf28 Compare January 26, 2017 11:36

cgundogan reviewed Jan 26, 2017

View reviewed changes

haukepetersen force-pushed the fix_pm532_spiadapt branch from d32cf28 to 678a97e Compare January 26, 2017 12:34

lebrush suggested changes Jan 26, 2017

View reviewed changes

haukepetersen force-pushed the fix_pm532_spiadapt branch from 678a97e to 3527173 Compare January 26, 2017 13:47

cgundogan approved these changes Jan 26, 2017

View reviewed changes

lebrush approved these changes Jan 26, 2017

View reviewed changes

cgundogan reviewed Jan 26, 2017

View reviewed changes

PeterKietzmann added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 26, 2017

haukepetersen added 2 commits January 27, 2017 08:30

drivers/pn532: enabled test app to select SPI mode

db048f2

drivers/pn532: adapted to SPI interface changes

1ae0ad2

haukepetersen force-pushed the fix_pm532_spiadapt branch from 3527173 to 1ae0ad2 Compare January 27, 2017 07:30

cgundogan merged commit 9f1721a into RIOT-OS:master Jan 27, 2017

haukepetersen deleted the fix_pm532_spiadapt branch January 27, 2017 10:19

PeterKietzmann added a commit that referenced this pull request Jan 27, 2017

Merge pull request #6496 from RIOT-OS/master

f3f403e

Backport #6478

PeterKietzmann removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 27, 2017

Conversation

haukepetersen commented Jan 26, 2017

Uh oh!

haukepetersen commented Jan 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgundogan commented Jan 26, 2017

Uh oh!

haukepetersen commented Jan 26, 2017

Uh oh!

cgundogan commented Jan 26, 2017

Uh oh!

cgundogan commented Jan 26, 2017

Uh oh!

haukepetersen commented Jan 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lebrush left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lebrush commented Jan 26, 2017

Uh oh!

haukepetersen commented Jan 26, 2017

Uh oh!

cgundogan left a comment

Choose a reason for hiding this comment

Uh oh!

lebrush commented Jan 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Jan 27, 2017

Uh oh!

cgundogan commented Jan 27, 2017

Uh oh!

PeterKietzmann commented Jan 27, 2017

Uh oh!

lebrush commented Jan 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

haukepetersen commented Jan 26, 2017 •

edited

Loading

lebrush commented Jan 26, 2017 •

edited

Loading