Arduino.h and wiring_digital.c problem.

Hi.

While trying to blink the led with this program:

#include "Arduino.h"

void setup()
{
  pinMode(13,OUTPUT);
  //REG_PIOB_OWER=g_APinDescription[13].ulPin;
}
void loop()
{
  portOutputRegister(digitalPinToPort(13)) = 0xffffffff & g_APinDescription[13].ulPin;
  delay(1000);
  portOutputRegister(digitalPinToPort(13)) = 0x0 & g_APinDescription[13].ulPin;
  delay(1000);
}

I've found two problems.

The first one, it wouldn't compile unless I would change the sintaxe in Arduino.h in lines 56 and 57.

After replacing this:

#define digitalPinToPort(P)        ( g_APinDescription[P]->pPort )
#define digitalPinToBitMask(P)     ( g_APinDescription[P]->ulPin )

with this:

#define digitalPinToPort(P)        ( g_APinDescription[P].pPort )
#define digitalPinToBitMask(P)     ( g_APinDescription[P].ulPin )

It compiled without errors.

But even after this, the led wouldn't blink.

I've found that after enabling the OWER register int setup it works.

To avoid setting pinMode and also OWER, I've added this to wiring_digital.c in pinMode function for case OUTPUT:

g_APinDescription[ulPin].pPort -> PIO_OWER = g_APinDescription[ulPin].ulPin;

The functions ends up like this:

extern void pinMode( uint32_t ulPin, uint32_t ulMode )
{
	if ( g_APinDescription[ulPin].ulPinType == PIO_NOT_A_PIN )
    {
        return ;
    }

	switch ( ulMode )
    {
        case INPUT:
            /* Enable peripheral for clocking input */
            pmc_enable_periph_clk( g_APinDescription[ulPin].ulPeripheralId ) ;
            PIO_Configure(
            	g_APinDescription[ulPin].pPort,
            	PIO_INPUT,
            	g_APinDescription[ulPin].ulPin,
            	0 ) ;
        break ;

        case INPUT_PULLUP:
            /* Enable peripheral for clocking input */
            pmc_enable_periph_clk( g_APinDescription[ulPin].ulPeripheralId ) ;
            PIO_Configure(
            	g_APinDescription[ulPin].pPort,
            	PIO_INPUT,
            	g_APinDescription[ulPin].ulPin,
            	PIO_PULLUP ) ;
        break ;

        case OUTPUT:
            PIO_Configure(
            	g_APinDescription[ulPin].pPort,
            	PIO_OUTPUT_1,
            	g_APinDescription[ulPin].ulPin,
            	g_APinDescription[ulPin].ulPinConfiguration ) ;
			g_APinDescription[ulPin].pPort -> PIO_OWER = g_APinDescription[ulPin].ulPin;

            /* if all pins are output, disable PIO Controller clocking, reduce power consumption */
            if ( g_APinDescription[ulPin].pPort->PIO_OSR == 0xffffffff )
            {
                pmc_disable_periph_clk( g_APinDescription[ulPin].ulPeripheralId ) ;
            }
        break ;

        default:
        break ;
    }
}

Is there any inconvenient to have the pinMode function also setting the OWER register?

alvesjc,

your patch to the macros digitalPinToPort and digitalPinToBitMask are correct, I fixed it upstream:

Also your use of OWER register is correct. I guess we can set it to 0xFFFFFFFF during board initialization in all PIO{A,B,C} without side effects. I'll check this tomorrow, hope to do it in time to include it in 1.5.1.

C

Ok, Thank you.

Regards,

Joao

Yes, I also had see that on Arduino.h, but the problem now is on the portOutputRegister and portInputRegister, since I can't use what it returns if this is correct.

I have this definition as a test sketch:

#define sbi(reg, bitmask) reg = 0xffffffff & bitmask

and the folowing vars:

volatile RwReg P_LED;
uint32_t B_LED;
P_LED = portOutputRegister(digitalPinToPort(13));
B_LED = digitalPinToBitMask(13);

using this test sketch to light the led on the pin 13 is not working:

sbi(P_LED,B_LED);
delay(1000);
cbi(P_LED,B_LED);
delay(1000);

Only if I go directly like this it works:

portOutputRegister(digitalPinToPort(13)) = 0xffffffff & digitalPinToBitMask(13);
delay(1000);
portOutputRegister(digitalPinToPort(13)) = 0x0 & digitalPinToBitMask(13);
delay(1000);

Looks like thar reg on sbi or cbi is not passed correct, any help, coments on this?

What sort of var should I declared so it works?

Baltasar,

what are you missing is to enable Synchronous Data Output (look at page 646 of SAM3X datasheet for more info on that).
The following snippet of code should solve your issue (warning: totally untested :))

REG_PIOA_OWER = 0xFFFFFFFF;
REG_PIOB_OWER = 0xFFFFFFFF;
REG_PIOC_OWER = 0xFFFFFFFF;

I'm thinking about adding this as a default initialization in Arduino Core.

Thanks cmaglie, but we (me and alvesjc) allready had that included on the changed pinMode version.

The problem is like I said, using directly like this works:

#define sbi(reg, bitmask) portOutputRegister(digitalPinToPort(13)) = 0xffffffff & bitmask

B_LED = digitalPinToBitMask(13);
sbi(P_LED,B_LED);

passing the reg var not !

Here is the not working code code:

#define sbi(reg, bitmask) reg = 0xffffffff & bitmask

P_LED = portOutputRegister(digitalPinToPort(13));
B_LED = digitalPinToBitMask(13);
sbi(P_LED,B_LED);
P_LED = portOutputRegister(digitalPinToPort(13));

This line reads the current value of the port output register and stores it in P_LED. This is almost certainly not what you wanted to do.

stimmer , but using that inside de sbi macro works and the led on pin 13 blinks on/off alternating, only does not work if I pass the P_LED.

What's the intended behaviour of sbi? you defined it as:

#define sbi(reg, bitmask) reg = 0xffffffff & bitmask

Actually (0xffffffff & bitmask) equals to "bitmask", i don't see why you do this bitwise-AND.

probably you want to do something like that:

#define sbi(reg, mask) (reg |= mask)
#define cbi(reg, mask) (reg &= ~mask)

Hi cmaglie.

Yes, it has been tested that way also, in fact, that is the original code.

stimmer:

P_LED = portOutputRegister(digitalPinToPort(13));

This line reads the current value of the port output register and stores it in P_LED. This is almost certainly not what you wanted to do.

Stimmer, I think I understand what you mean.

What we are trying to do is to get the port address.

This code in mega works this way:

#define sbi(reg, mask) *reg |= mask
#define cbi(reg, mask) *reg &= ~mask

volatile uint8_t * P_LED;
uint16_t B_LED;

void(loop)
{
P_LED = portOutputRegister(digitalPinToPort(13));
B_LED = digitalPinToBitMask(13);
sbi(P_LED,B_LED);
cbi(P_LED,B_LED);
}

But when we ported this code to Due like this:

#define sbi(reg, mask) *reg |= mask
#define cbi(reg, mask) *reg &= ~mask

volatile uint32_t * P_LED;
uint32_t B_LED;

void(loop)
{
P_LED = portOutputRegister(digitalPinToPort(13));
B_LED = digitalPinToBitMask(13);
sbi(P_LED,B_LED);
cbi(P_LED,B_LED);
}

It doens't compile complaining that RwREG is not compatible with volatile uint32_t*.

Can you give us a clue why?

I confess, i'm very bad with pointers...

alvesjc,

the portOutputRegister(..) currently is returning the actual value of the port register instead of its address.

may you try this:

P_LED = &portOutputRegister(digitalPinToPort(13));

using the & to obtain the address of portOutputRegister.

BTW the definition for RwReg is:

typedef volatile uint32_t RwReg;

Ok, I'll try that in evening.

Meanwhile I'll get some reading for better understanding of your suggestion.

Thank you.

Here all the definitions in CMSIS:

file hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/include/sam3x8e.h, line 47

typedef volatile const uint32_t RoReg; /**< Read only 32-bit register (volatile const unsigned int) */
#else
typedef volatile       uint32_t RoReg; /**< Read only 32-bit register (volatile const unsigned int) */
#endif
typedef volatile       uint32_t WoReg; /**< Write only 32-bit register (volatile unsigned int) */
typedef volatile       uint32_t RwReg; /**< Read-Write 32-bit register (volatile unsigned int) */

file hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/include/component/component_pio.h, line 40

/** \brief Pio hardware registers */
typedef struct {
  WoReg PIO_PER;       /**< \brief (Pio Offset: 0x0000) PIO Enable Register */
  WoReg PIO_PDR;       /**< \brief (Pio Offset: 0x0004) PIO Disable Register */
  RoReg PIO_PSR;       /**< \brief (Pio Offset: 0x0008) PIO Status Register */
  RoReg Reserved1[1];
  WoReg PIO_OER;       /**< \brief (Pio Offset: 0x0010) Output Enable Register */
  WoReg PIO_ODR;       /**< \brief (Pio Offset: 0x0014) Output Disable Register */
  RoReg PIO_OSR;       /**< \brief (Pio Offset: 0x0018) Output Status Register */
  RoReg Reserved2[1];
  WoReg PIO_IFER;      /**< \brief (Pio Offset: 0x0020) Glitch Input Filter Enable Register */
  WoReg PIO_IFDR;      /**< \brief (Pio Offset: 0x0024) Glitch Input Filter Disable Register */
  RoReg PIO_IFSR;      /**< \brief (Pio Offset: 0x0028) Glitch Input Filter Status Register */
  RoReg Reserved3[1];
  WoReg PIO_SODR;      /**< \brief (Pio Offset: 0x0030) Set Output Data Register */
  WoReg PIO_CODR;      /**< \brief (Pio Offset: 0x0034) Clear Output Data Register */
  RwReg PIO_ODSR;      /**< \brief (Pio Offset: 0x0038) Output Data Status Register */
  RoReg PIO_PDSR;      /**< \brief (Pio Offset: 0x003C) Pin Data Status Register */
  WoReg PIO_IER;       /**< \brief (Pio Offset: 0x0040) Interrupt Enable Register */
  WoReg PIO_IDR;       /**< \brief (Pio Offset: 0x0044) Interrupt Disable Register */
  RoReg PIO_IMR;       /**< \brief (Pio Offset: 0x0048) Interrupt Mask Register */
  RoReg PIO_ISR;       /**< \brief (Pio Offset: 0x004C) Interrupt Status Register */
  WoReg PIO_MDER;      /**< \brief (Pio Offset: 0x0050) Multi-driver Enable Register */
  WoReg PIO_MDDR;      /**< \brief (Pio Offset: 0x0054) Multi-driver Disable Register */
  RoReg PIO_MDSR;      /**< \brief (Pio Offset: 0x0058) Multi-driver Status Register */
  RoReg Reserved4[1];
  WoReg PIO_PUDR;      /**< \brief (Pio Offset: 0x0060) Pull-up Disable Register */
  WoReg PIO_PUER;      /**< \brief (Pio Offset: 0x0064) Pull-up Enable Register */
  RoReg PIO_PUSR;      /**< \brief (Pio Offset: 0x0068) Pad Pull-up Status Register */
  RoReg Reserved5[1];
  RwReg PIO_ABSR;      /**< \brief (Pio Offset: 0x0070) Peripheral AB Select Register */
  RoReg Reserved6[3];
  WoReg PIO_SCIFSR;    /**< \brief (Pio Offset: 0x0080) System Clock Glitch Input Filter Select Register */
  WoReg PIO_DIFSR;     /**< \brief (Pio Offset: 0x0084) Debouncing Input Filter Select Register */
  RoReg PIO_IFDGSR;    /**< \brief (Pio Offset: 0x0088) Glitch or Debouncing Input Filter Clock Selection Status Register */
  RwReg PIO_SCDR;      /**< \brief (Pio Offset: 0x008C) Slow Clock Divider Debouncing Register */
  RoReg Reserved7[4];
  WoReg PIO_OWER;      /**< \brief (Pio Offset: 0x00A0) Output Write Enable */
  WoReg PIO_OWDR;      /**< \brief (Pio Offset: 0x00A4) Output Write Disable */
  RoReg PIO_OWSR;      /**< \brief (Pio Offset: 0x00A8) Output Write Status Register */
  RoReg Reserved8[1];
  WoReg PIO_AIMER;     /**< \brief (Pio Offset: 0x00B0) Additional Interrupt Modes Enable Register */
  WoReg PIO_AIMDR;     /**< \brief (Pio Offset: 0x00B4) Additional Interrupt Modes Disables Register */
  RoReg PIO_AIMMR;     /**< \brief (Pio Offset: 0x00B8) Additional Interrupt Modes Mask Register */
  RoReg Reserved9[1];
  WoReg PIO_ESR;       /**< \brief (Pio Offset: 0x00C0) Edge Select Register */
  WoReg PIO_LSR;       /**< \brief (Pio Offset: 0x00C4) Level Select Register */
  RoReg PIO_ELSR;      /**< \brief (Pio Offset: 0x00C8) Edge/Level Status Register */
  RoReg Reserved10[1];
  WoReg PIO_FELLSR;    /**< \brief (Pio Offset: 0x00D0) Falling Edge/Low Level Select Register */
  WoReg PIO_REHLSR;    /**< \brief (Pio Offset: 0x00D4) Rising Edge/ High Level Select Register */
  RoReg PIO_FRLHSR;    /**< \brief (Pio Offset: 0x00D8) Fall/Rise - Low/High Status Register */
  RoReg Reserved11[1];
  RoReg PIO_LOCKSR;    /**< \brief (Pio Offset: 0x00E0) Lock Status */
  RwReg PIO_WPMR;      /**< \brief (Pio Offset: 0x00E4) Write Protect Mode Register */
  RoReg PIO_WPSR;      /**< \brief (Pio Offset: 0x00E8) Write Protect Status Register */
} Pio;

file hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/include/sam3x8e.h, line 494

#define PIOA       ((Pio    *)0x400E0E00U) /**< \brief (PIOA      ) Base Address */
#define PIOB       ((Pio    *)0x400E1000U) /**< \brief (PIOB      ) Base Address */
#define PIOC       ((Pio    *)0x400E1200U) /**< \brief (PIOC      ) Base Address */
#define PIOD       ((Pio    *)0x400E1400U) /**< \brief (PIOD      ) Base Address */

PIOA, for example, is a pointer to a Pio structure located exactly over the memory mapped registers, this way we can access, say, ODSR using PIOA->PIO_ODSR (access to a structure member located exactly over the register), and its what the macro portOutputRegister actually does.

Said that, the original meaning of the macro portOutputRegister(..) was to give the address of that register, so the define in Arduino.h is wrong and it should be changed from:

(...aehm..edited...)

#define portOutputRegister(port)   ( port->PIO_ODSR )
#define portInputRegister(port)    ( port->PIO_PDSR )

to:

#define portOutputRegister(port)   ( &(port->PIO_ODSR) )
#define portInputRegister(port)    ( &(port->PIO_PDSR) )

I'm waiting the results of your test! :slight_smile:

Ok ok ok, if I get it right, that means that I'll not need to add the ampersand to "portOutputRegister(digitalPinToPort(13))" as your previous suggestion, because "( &(port->PIO_PDSR) )" will already return that pointer, right?

By the way, can you explain me the meaning of "->"? Or point me to some place to read about it?

I'm not finding any reference to it.

Thank you.

BR,

Joao

alvesjc:
Ok ok ok, if I get it right, that means that I'll not need to add the ampersand to "portOutputRegister(digitalPinToPort(13))" as your previous suggestion, because "( &(port->PIO_PDSR) )" will already return that pointer, right?

exactly.

alvesjc:
By the way, can you explain me the meaning of "->"? Or point me to some place to read about it?

when you have a pointer to a structure the "->" operator is used to access a member of the pointed structure. Without this operator you should write something like:

(*PIOA).PIO_PDSR

whete *PIOA is the structure instance pointed by PIOA and .PIO_PDSR is the member selector. You need also the parenthesis because . has higher priority versus *. Since this kind of access is used very often, the C language defines the -> operator so you can write a more concise:

PIOA->PIO_PDSR

if you need more info a google search for "pointer to structure" will give you a lot of articles.

Thanks again cmaglie, your fix on the Arduino.h puts everything working on the right spot. :slight_smile:
I hope it will sent to GIT so the last version comes out fixed with that.

Now all code works like a charme, I wonder now if we still can not optimize it a bit more since we are still using on the ports writing routines the REG_PIOD_SODR register, do you think this can also be changed for ODSR?

The problem with ODSR is when you write something there what was before gets cleared, so I think I need to do the proper logic just to change what´s new and maintain what was previous, no?

cmaglie:

alvesjc:
Ok ok ok, if I get it right, that means that I'll not need to add the ampersand to "portOutputRegister(digitalPinToPort(13))" as your previous suggestion, because "( &(port->PIO_PDSR) )" will already return that pointer, right?

exactly.

alvesjc:
By the way, can you explain me the meaning of "->"? Or point me to some place to read about it?

when you have a pointer to a structure the "->" operator is used to access a member of the pointed structure. Without this operator you should write something like:

(*PIOA).PIO_PDSR

whete *PIOA is the structure instance pointed by PIOA and .PIO_PDSR is the member selector. You need also the parenthesis because . has higher priority versus *. Since this kind of access is used very often, the C language defines the -> operator so you can write a more concise:

PIOA->PIO_PDSR

if you need more info a google search for "pointer to structure" will give you a lot of articles.

Ok, thank you for your time explaining. :wink:

BR,

Joao

cmaglie:
I'm waiting the results of your test! :slight_smile:

Ok, working fine now, thanks for your help!

With direct port communication with SODR and CODR for data pins we have gained +-13s in UTFT Demo code loop, a HUGE difference!!!

Regards,

Joao

Hello alvesjc

Can you put the sample code you already it works.
I refer to the set of sbi, cbi.

It would be far better that you put the modified UTFT to download.

I have cast a write methods for UTFT ports that I have not been able to apply, but I set the display directly and go very fast, I would like to adapt the library and send it to the developer to UTFT but I'm missing that part.

Thanks

After doing some tests, I see that it works by putting the code which says cmaglie.

REG_PIOA_OWER = 0xFFFFFFFF;
REG_PIOB_OWER = 0xFFFFFFFF;
REG_PIOC_OWER = 0xFFFFFFFF;

then hit the code.

#define sbi(reg, mask) *reg |= mask
#define cbi(reg, mask) *reg &= ~mask

int led = 13;
volatile uint32_t * Registro;
uint32_t Mascara;

// the setup routine runs once when you press reset:
void setup() {                
  REG_PIOA_OWER = 0xFFFFFFFF;
  REG_PIOB_OWER = 0xFFFFFFFF;
  REG_PIOC_OWER = 0xFFFFFFFF;
  Registro	= portOutputRegister(digitalPinToPort(led));
  Mascara	= digitalPinToBitMask(led);
  // initialize the digital pin as an output.
  pinMode(led, OUTPUT);    
  
  
}

// the loop routine runs over and over again forever:
void loop() {
  sbi(Registro,Mascara);
  delay(1000);               // wait for a second
  cbi(Registro,Mascara);
  delay(1000);               // wait for a second
}