| View previous topic :: View next topic |
| Author |
Message |
prasad_21
Joined: 12 Nov 2014
Posts: 11
|
|
 Posted: Wed Mar 11, 2015 11:32 pm |
 |
|
@asmboy
Thanks for correcting me its actually SWD_PIN_LOW and a mistake by me while writing the code. |
|
 |
prasad_21
Joined: 12 Nov 2014
Posts: 11
|
|
| Posted: Wed Mar 11, 2015 11:43 pm |
|
|
@ckielstra
The code written above is for a software SPI as the module which i have has a hardware fixed which can't be changed. I was thinking of designing a new one so said that am using hardware SPI, but the code is written keeping in mind the Circuit which i have.
Will use the SPI Mode 0 as stated by you i.e. #define SPI_MODE_0 (SPI_L_TO_H | SPI_XMIT_L_TO_H) .
As far as the SPI configuration for ADF7021 is concerned, i have defined the register bits :
#define UART_MODE_EN (0x1<<28) //reg 0
#define TxRxCLK (0x7<<17) //reg 15
the only thing i have missed is calling these when i initialize the adf. Thanks.. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19590
|
|
| Posted: Thu Mar 12, 2015 2:56 am |
|
|
The point about the rotated declarations, is that they are dangerous.
They 'assume' that the compiler will realise that the value can't fit into the default integer size, and automatically use a larger type to hold them.
Also, if the default type is 'signed', the result is defined (in K&R), as 'implementation specific'. In other words 'no guarantees as to what will happen'....
If you instead physically directly declare the number, then such assumptions no longer have to be made. |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Thu Mar 12, 2015 3:08 am |
|
|
| prasad_21 wrote: | @asmboy
Thanks for correcting me its actually SWD_PIN_LOW and a mistake by me while writing the code. |
When you find one error, then always check for similar errors. For example when I look quickly there is also: | Code: | #define COUNTER RESET (0x1<<15)
...
#define Tx/Rx_SWITCH_ENABLE (0x1<<11)
|
Again, a missing '-', and the '/' character is not allowed in a #define.
Not a big problem as you are not using these defines now, but these are difficult to find problems when you start using them.
I can't find many source code examples on the internet. For your reference here a link to a project I found: https://github.com/satlab/bluebox/tree/master/software/firmware
Good for explaining that you are using a software SPI because you are using an existing hardware where these lines are connected wrong. Now we understand your design choices better.
But, then you have to remove the call to setup_spi() because that is configuring a hardware SPI and locks the use of pins B4, B6 and C7.
| Code: | | output_low( ADF_CLK); |
The ADF7021 is SPI master and generates the clock. This pin should be change to an input ! Now you have two outputs connected to each other which can cause a shortcut and destroy your chips.
Forget my remarks about the SPI mode. You have your own routines for a software SPI receiver and then my remarks for the hardware SPI don't care. Can you post your SPI code? |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Thu Mar 12, 2015 4:02 am |
|
|
| Ttelmah wrote: | | The point about the rotated declarations, is that they are dangerous. |
I've seen this notation style in other software as well and kind of like it. For example it is used in the professional ARM CMSIS libraries. (source link without login)
Once you get used to it, it is easy to read. For 8-bit values a raw hex value is easy to convert to individual bits, but for 32-bit constants the notation used here creates less errors.
| Quote: | | They 'assume' that the compiler will realise that the value can't fit into the default integer size, and automatically use a larger type to hold them. |
True
| Quote: | | Also, if the default type is 'signed', the result is defined (in K&R), as 'implementation specific'. In other words 'no guarantees as to what will happen'.... |
As far as I remember this problem is for right shift only. The left shift used here is guaranteed to shift in a zero.
Both problems can be prevented when you declare the constant as unsigned long by adding 'ULL': | Code: | Original:
#define TxINV_CLK_AND_DATA (0x3<<28)
Better:
#define TxINV_CLK_AND_DATA (0x3ULL<<28) |
Edit: Changed 'UL' to 'ULL' for 32-bit support as suggested by Ttelmah.
Last edited by ckielstra on Thu Mar 12, 2015 9:28 am; edited 1 time in total |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19590
|
|
| Posted: Thu Mar 12, 2015 5:01 am |
|
|
On the left shift, the behaviour is undefined, if you shift into the top bit. Otherwise OK.
I too quite like the shift notation, but if you are going to use it, you really need to force the compiler into a 'known mode'. As you say Ckielstra, Using UL, or ULL (if you are on a PIC16/18, a 'long' is only 16bits), or an explicit cast to UINT32, is the really safe way to do this. |
|
|
prasad_21
Joined: 12 Nov 2014
Posts: 11
|
|
| Posted: Tue Mar 17, 2015 12:59 am |
|
|
@ckielstra
I have gone through the code for missing '-', and the '/' character, i had missed many times in the defines. Thanks for highlighting the same. I am also going through the link you have provided. I didn't get what exactly you want to say about output_low( ADF_CLK); and the pin should be changed to input. I am writing the SPI code and post it once i am done with it. Also as said by Ttelmah and after looking at your comment i have changed 'UL' to 'ULL'. Thanks.. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
