sysfs-spi: invalid SPI KeepCS handling [was: can't set antenna for mfrc522 device]

[svenwoldt]

Hello jdevelop and maruel,
great to see this merge. pull/220
I am busy trying out the experimental mfrc522 code but it fails to set the antenna.

The pins follow the layout described here by jdevelop.
interrupt falling edge by jens18

So after initially running into this:

mfrc522: mfrc522: timeout waitinf for IRQ edge: 30s.

I used the following command which worked for the irq pin timeout but it would then run into this issue:

pi@raspberrypi:~/go/src/github.com/google/periph/experimental $ go run cmd/mfrc522/main.go -rs-pin 13 -irq-pin 11

mfrc522: mfrc522: can not set the bitmask for antenna.
exit status 1

Could anybody help me troubleshoot this? Running on a raspberry pi zero or 3 b
I am using this device joy-it/RFID_RC522 I can confirm that it works with this library mxgxw/MFRC522-python
Much thanks

[jdevelop]

Hi @svenwoldt !

Sorry for the late response. Did you manage to figure out what was wrong?

[jdevelop]

I see that in the Python code:

def AntennaOn(self):
    temp = self.Read_MFRC522(self.TxControlReg)
    if(~(temp & 0x03)):
self.SetBitMask(self.TxControlReg, 0x03)

they don't fail if the command register has no bits 0 and 1 set.

Perhaps you can try to replace the line https://github.com/google/periph/blob/HEAD/experimental/devices/mfrc522/mfrc522.go#L154 with

return nil
and let us know if this solves the problem

[vkorn]

I didn't have a chance to dig into the code yet, but it seems that it's not the only problem.
Something in device Init is wrong, basically blocking the receiver completely, only rpi reboot helps.

[vkorn]

@jdevelop

as per your recommendation trying to use this implementation instead of rpi-extras. However, I'm having troubles to make it work. Getting the same timeout as @svenwoldt

Moreover, device becomes totally unresponsive after irq pin setup in NewSPI

if err := irqPin.In(gpio.PullUp, gpio.FallingEdge); err != nil {
		return nil, err
	}

Basically after this call the only way to bring reader back is to reboot rpi.

I don't see any other devices using FallingEdge so it will take some time to understand whether it's a gpio pin implementation issue rather than mfrc522 driver, maybe @maruel has some insights on this.

P.S. One thing I don't understand so far, I can see that sysfs-gpio is used after host.Init(), if I'm looking into host/sysfs/gpio.go, In method always receives PullNoChange even though PullUp is explicitly used.

[jdevelop]

This happens on Raspberry Pi 3 Model B+, right?

[vkorn]

Yes

[jdevelop]

Okay, I will try to test that out in a couple of days, I think I have RPi 3 B+. I tested this code on Pi Zero / Pi Zero W

@maruel is there any possibility that there's something off in the GPIO drivers for these models?

[vkorn]

Ok, I don't think it has something to do with the device implementation itself tbh, simply running this locks the device:

package main

import (
	"periph.io/x/periph/conn/gpio"
	"periph.io/x/periph/conn/gpio/gpioreg"
	"periph.io/x/periph/conn/physic"
	"periph.io/x/periph/conn/spi"
	"periph.io/x/periph/conn/spi/spireg"
	"periph.io/x/periph/host"
)

func main() {
	_, err := host.Init()

	if err != nil {
		panic(err.Error())
	}

	spiDev, err := spireg.Open("0")
	if err != nil {
		panic(err.Error())
	}

	_, err = spiDev.Connect(10*physic.MegaHertz, spi.Mode0, 8)
	if err != nil {
		panic(err.Error())
	}

	resetPin := gpioreg.ByName("13")
	println(resetPin.String())
	err = resetPin.Out(gpio.High)
	if err != nil {
		panic(err.Error())
	}

	err = resetPin.Halt()
	if err != nil{
		panic(err.Error())
	}

	irqPin := gpioreg.ByName("11")
	err = irqPin.In(gpio.PullUp, gpio.FallingEdge)
	if err != nil {
		panic(err.Error())
	}

	err = irqPin.Halt()
	if err != nil{
		panic(err.Error())
	}

	err = spiDev.Close()
	if err != nil{
		panic(err.Error())
	}
}

Which seems to output correct values for the pins, but not for the pin mode (added debug output into sysfs.gpio)

13(GPIO13)
PullNoChange
11(GPIO11)

[jdevelop]

10*physic.MegaHertz does MFRC522 support 10 Mhz? I wonder if it's true, I usually do 1 MHz.

[vkorn]

This is exactly how current implementation is initializing the device:

periph/experimental/devices/mfrc522/mfrc522.go

Line 107 in b353903

spiDev, err := spiPort.Connect(10*physic.MegaHertz, spi.Mode0, 8)

I've googled today and it seems that it should support 10Mhz, not that it matters though -- changing frequency does nothing.

[vkorn]

Ok, so pull mode is implemented in bcm283, that's why it always NoChange in sysfs. But it seems that it's enough to simply call irqPin.In with any mode to lock the device.

[vkorn]

Sorry for spamming, but it's definitely not a device implementation issue.
With reverted periph.io version everything works as expected

 periph.io/x/periph v2.1.1-0.20180325234317-cc45cbc42835+incompatible

which corresponds to commit cc45cbc

Worth mentioning, that using 13 and 11 as an input pins is incorrect, documentation of gpioreg.ByName is not very clear, unfortunately. For 13 and 11 physical pins it should be GPIO27 and GPIO17 (for rpi) instead.

I was able to narrow down the problem to the range of commits between 78a6876 and 6e2faaa

Unfortunately there were a lot of changes related to the pin functions and I'm not completely familiar with periph.io core yet.

[jdevelop]

@maruel shall we create another issue for this new details or you / someone will be looking into that in scope of this one?

[maruel]

@vkorn Thanks a lot for the investigation.

https://github.com/google/periph/compare/78a68761ff34ae411e312b7553ce6009cf712f5a..6e2faaa5091f07b7ddd6e3d97919c85cdf881451 includes a lot of SPI driver changes, so I could have messed up there. That's one possibility.

The bcm283x driver registers the number aliases as the GPIO number as exposed by the CPU. This means that 13 == GPIO13. To get the rpi board numbers, you need to either use the rpi package or the form P1_13.

Background info that may have led to a bit of the confusion above:

• the sysfs driver cannot set pull resistor, because it's not implemented on linux.
• periph CPU drivers implement setting the pull resistor, but doesn't support edge detection.
  • so the CPU drivers will transparently leverage the sysfs driver for edge detection.

Any idea how to make this clearer for user with better doc is appreciated.

[maruel]

Re gpio number documentation, https://periph.io/device/gpio/ is clear about this. Maybe worth adding a link from within gpioreg package to this page?

[vkorn]

@maruel having a link would definitely help, this page is very useful.

So I give it a few more tries and it seems everything boils down to this commit 6c72da6 where spiConn was introduced.

[vkorn]

Ok, so the problem is here

periph/host/sysfs/spi.go

Lines 399 to 401 in 6c72da6

	if !s.noCS && !p[i].KeepCS {
	m[i].csChange = 1
	}

I'm not sure whether it was intended to change the behavior of Tx (since now all drivers, using Tx will have csChange == 1) but if yes, this should be fixed on driver's level: instead of Tx, TxPackets should be used. @maruel will need your input here.

Here

periph/experimental/devices/mfrc522/mfrc522.go

Lines 680 to 682 in 6c72da6

	func (r *Dev) devWrite(address int, data byte) error {
	newData := []byte{(byte(address) << 1) & 0x7E, data}
	return r.spiDev.Tx(newData, nil)

And here

periph/experimental/devices/mfrc522/mfrc522.go

Lines 685 to 688 in 6c72da6

	func (r *Dev) devRead(address int) (byte, error) {
	data := []byte{((byte(address) << 1) & 0x7E) | 0x80, 0}
	out := make([]byte, len(data))
	if err := r.spiDev.Tx(data, out); err != nil {

[maruel]

Thanks a lot for the analysis @vkorn !
Does https://github.com/google/periph/pull/new/fix_spi fix your problem? If so I'll have to debug with my oscilloscope now that I'm back home.

[vkorn]

@maruel yes, with this change everything is back to normal, thank you. I would assume all other devices still should work fine since previously this flag was not present while calling regular Tx() method.

[maruel]

Dang, thanks for diagnosing! I'll push this and will try to release soon.

[maruel]

I invite you to join the slack channel to discuss this more. I'm really looking forward to have someone move it out of experimental, albeit I need someone to write a page at https://periph.io/device/ first. Thanks!

[maruel]

While my fix worked for Tx(), it didn't for TxPacket(). I verified the behavior on the oscilloscope and sending a follow up fix. Sorry for the annoyance.

[maruel]

Here's screenshots of the oscilloscope with the fix be61657.

• Line 1 (yellow) is SPI_CS.
• Line 2 (cyan) is SPI_CLK.

3 transactions

Either

c.Tx(make([]byte, 1), nil)
c.Tx(make([]byte, 1), nil)
c.Tx(make([]byte, 1), nil)

or

c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: false}})
c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: false}})
c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: false}})

TxPackets with 3 packets

Either

p := []spi.Packet{
  {W: make([]byte, 1), KeepCS: true},
  {W: make([]byte, 1), KeepCS: true},
  {W: make([]byte, 1), KeepCS: false},
} 
c.TxPackets(p)

or

c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: true}})
c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: true}})
c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: false}})

TxPackets with KeepCS true

c.TxPackets([]spi.Packet{{W: make([]byte, 1), KeepCS: true}})

[jdevelop]

I built my app against the head, and at the moment IRQ doesn't work at all, e.g there is no signal on failing edge. I will do some more research on that, but generally speaking there should be a way to not break the existing functionality with the updates.

My hardware is Raspberry Pi Zero.

[maruel]

@jdevelop you probably want to star #323 and stick to v3.3.0 in the meantine.

[jdevelop]

@maruel thanks, tried v3.3.0 with the same result, there's no IRQ intercepted. I wonder whether there was some API change that requires doing something in order to get WaitForEdge working?

The pin is set up as

	if err := irqPin.In(gpio.PullUp, gpio.FallingEdge); err != nil {
		return nil, err
	}

in https://github.com/google/periph/blob/master/experimental/devices/mfrc522/commands/low_level.go#L46

Anything else to try out? As for now I'm stuck with the version that is 6 months old :(

[vkorn]

The 6-month version was the only one working prior to this fix on Pi3. Head is working on Pi3 correctly, so I would assume something is different in handling edges on Pi0 :(