Pages: [1] 2   Go Down
Author Topic: Arduino.h and wiring_digital.c problem.  (Read 6733 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi.

While trying to blink the led with this program:

Code:
#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:
Code:
#define digitalPinToPort(P)        ( g_APinDescription[P]->pPort )
#define digitalPinToBitMask(P)     ( g_APinDescription[P]->ulPin )

with this:
Code:
#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:

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

The functions ends up like this:

Code:
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?
« Last Edit: November 04, 2012, 02:42:01 pm by alvesjc » Logged


Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

alvesjc,

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

https://github.com/arduino/Arduino/commit/73649c2f60d0a006061382b0966a06e6d0fda7fe

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
Logged

C.

0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Ok, Thank you.

Regards,

Joao
Logged


0
Offline Offline
Newbie
*
Karma: 0
Posts: 41
Arduino DUE rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Check all my projects based on Atmel/Arduino -> www.aqualed-light.com

Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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 smiley)

Code:
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.
Logged

C.

0
Offline Offline
Newbie
*
Karma: 0
Posts: 41
Arduino DUE rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:

Code:
#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:

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

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

Logged

Check all my projects based on Atmel/Arduino -> www.aqualed-light.com

Offline Offline
God Member
*****
Karma: 32
Posts: 507
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
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.
Logged


0
Offline Offline
Newbie
*
Karma: 0
Posts: 41
Arduino DUE rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Check all my projects based on Atmel/Arduino -> www.aqualed-light.com

Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Code:
#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:

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

C.

0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi cmaglie.

Yes, it has been tested that way also, in fact, that is the original code.
Code:
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:

Code:
#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:

Code:
#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...
Logged


Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

alvesjc,

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

may you try this:

Code:
P_LED = &portOutputRegister(digitalPinToPort(13));

using the & to obtain the address of portOutputRegister.

BTW the definition for RwReg is:

Code:
typedef volatile uint32_t RwReg;
Logged

C.

0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Ok, I'll try that in evening.

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

Thank you.
Logged


Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Here all the definitions in CMSIS:

file hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/include/sam3x8e.h, line 47
Code:
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
Code:
/** \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
Code:
#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...)
Code:
#define portOutputRegister(port)   ( port->PIO_ODSR )
#define portInputRegister(port)    ( port->PIO_PDSR )

to:

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

I'm waiting the results of your test! :-)
« Last Edit: November 05, 2012, 07:02:31 am by cmaglie » Logged

C.

0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
« Last Edit: November 05, 2012, 08:22:31 am by alvesjc » Logged


Forum Administrator
Milano, Italy
Offline Offline
Sr. Member
*****
Karma: 23
Posts: 292
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

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:

Quote
(*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:

Quote
PIOA->PIO_PDSR

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

C.

Pages: [1] 2   Go Up
Jump to: