Created Fri, 23 Dec 2011 23:52:04 +0000 by bperrybap
Fri, 23 Dec 2011 23:52:04 +0000
If I understand the LAT register correctly, digitalWrite() in the PIC32 core currently does not seem to properly to ensure atomicity of the setting/clearing of the data bits.
It drops down to ANDing or Oring of the bitmaskt using the LAT register.
*latchPort &= ~bit;
and
*latchPort |= bit;
I'm just starting to get familiar with the PIC32 registers but I'm assuming that these operations on the LAT registers are interruptible. The arduino guys suffered from many subtle bugs until they made this operation atomic because there are some libraries that do call digitalWrite() from an ISR context. In the case of the PIC32, there is no need to mask interrupts like was needed on the AVR since the CLR and SET registers on the PIC32 can be used instead.
The code could be updated to use the CLR and SET registers in stead of the LAT but that will require two additional macro wrappers and to additional pointer tables to map from the port# to the output address for SET and CLR registers.
It is pretty simple change, and would seem to also add a slight performance increase as the final operation reduces to a single write vs a read/OR/write or Read/AND/write.
The digital write function becomes something like:
void digitalWrite(uint8_t pin, uint8_t val)
{
volatile uint32_t *outPort;
uint8_t port;
uint16_t bit;
uint8_t timer;
port = digitalPinToPort(pin);
bit = digitalPinToBitMask(pin);
timer = digitalPinToTimer(pin);
if (port == NOT_A_PIN) return;
// If the pin that support PWM output, we need to turn it off
// before doing a digital write.
if (timer != NOT_ON_TIMER) turnOffPWM(timer);
if (val == LOW)
{
outPort = portOutputRegisterCLR(port);
}
else
{
outPort = portOutputRegisterSET(port);
}
*outPort = bit; // atomically set or clear bit
}
--- bill
Tue, 27 Dec 2011 13:03:24 +0000
Bill,
This seems like great suggestion keep them coming, however, the repository looks like something like this has already been implemented. Which version of the compiler are you using?
https://github.com/chipKIT32/chipKIT32-MAX
Jacob
Tue, 27 Dec 2011 17:00:16 +0000
You are right. I was using the "official release". From Aug 22 which has a wiring_digital.c which goes back to Aug 18. wiring_digital.c was updated in November.
Just looked, looks there is a newer "official release" from just a few days ago.
oops, Guess I really need to be following the repository as things are still a bit fluid.
--- bill
Tue, 27 Dec 2011 20:00:35 +0000
As part of the general rework to remove board dependent code from the core and add the ability to plug board definitions into the system without modifying code, I rewrote the functions in wiring_digital to use the LATSET and LATCLR registers as you suggest.
As part of this, I defined a structure (p32_regset) that provides a generic map to most PIC32 peripheral registers, and the code has been modified to use a pointer to one of these structures for all peripheral register access. There was already a pin mapping table that contained the base address of a peripheral port register set, so no new pin mapping table was needed.
Gene Apperson Digilent
Wed, 28 Dec 2011 00:20:54 +0000
I saw that. (After seeing Jacob's comment I went to look at the code) Very nicely done.
I have so wished for having simple set/clear registers in the AVR for this very type of thing. With the AVR you can only get atomic register bit modification when you use constants and the register lands in the low memory space where cbi/sbi works, which isn't all of the port bits on the larger AVRs.
--- bill
Thu, 29 Dec 2011 18:56:20 +0000
A question related to this topic.
Currently there are Arduino macros that can return the bitmask and port address. However there is no way to get the address of the bit set and bit clear port addresses.
If a library wants to do higher speed direct bit access by using the port address directly, (vs calling digitalWrite()) it needs to use the bit set/clear registers to ensure that the access is atomic.
Is there already a way to do this? If not, would it make sense to extend the macros to include something like:
#define portOutputRegisterCLR(P)
#define portOutputRegisterSET(P)
So that way library could get access to the CLR and SET registers and then be able to modify bits without having to go through the additional overhead of digitalWrite().
This would allow a library to have fast access to ports and still allow the user to configure things run time as the library could save the port addresses and bit masks during its setup code.
--- bill
Sat, 31 Dec 2011 11:20:48 +0000
They are at a fixed offset from the port address
Fri, 06 Jan 2012 01:50:21 +0000
Since any use of the SET and CLR registers is going to be PIC32 specific, I wouldn't worry about trying to make the code portable. I would do something like this:
#include <p32_defs.h>
p32_ioport port; uint16_t bit;
port = digitalPinToPort(pin); bit = digitalPinToBitMask(pin);
port->lat.set = bit; //set the output high port->lat.clr = bit; //set the output low
I've provided definitions for all of the PIC32 peripherals in p32_defs.h (a new header file in the cores folder).
If port and bit are global variables that are initalized in setup(), then setting and clearing the bits will be extremely fast and atomic. (the code generated would be: load the pointer, add a constant offset, write a constant indirect through the resulting pointer).
If you have a macro do the pin-to-port mapping and the pin-to-bitmask mapping as part of the operation, it adds two additional load pointer, load indirect through pointer operations to each bit set or clr.
digitalPinToPort and digitalPinToBitMask are macros that are already defined and are part of the generally accepted Arduino interface. They aren't explicitly documented to be part of the interface, but their use is so prevalent, they are a de facto part of the interface.
I would still use pinMode to set the pin direction to output rather than write directly to the TRIS register, as there are a couple of additional things that need to be done in some cases for setting pin direction besides just writing to the TRIS register.
Gene Apperson Digilent
Gene Apperson Digilent
Wed, 11 Dec 2013 00:53:29 +0000
Hi y'all,
I'm a bit of an amateur here, so please bear with me if this is a dumb question, but is there a way to bang a whole byte into a i/o port at one time, effectively setting the on/off state of the corresponding 8 pins all at once? I've managed to figure out the technique mentioned in this thread, involving masks and set/clr, but am wondering if there's an easier way when I have a byte representing the 8 pin values.
Andy
Wed, 11 Dec 2013 01:26:20 +0000
Not really, as the ports are all 16-bit devices, not 8 bits.
You can mask out and then write to the port, which is the closest you get:
LATB &= 0xFF00;
LATB |= (myVal & 0xFF);
That would control the lowest 8 bits of port B in one go. You could do other subsets with shifting, too:
LATB &= 0xF00F;
LATB |= (myVal & 0xFF) << 4;
That would be the middle 8 bits.
Wed, 11 Dec 2013 15:14:48 +0000
Hmm, thanks, that helped.
I'm trying to do this within MPIDE.
LATF &= 0xFF00;
LATF |= (myVal & 0xFF);
Didn't work, but using portRegisters(digitalPinToPort(30))->lat.reg instead of LATF did. (I'm trying to set PORTF pins)
I presume there must be a more straightforward macro to get the latch register, but couldn't figure out what it is. My only include is pins_arduino.h - tried ports.h, but got "no such file or directory".