ADC initialization mistake in ADC.h #1418

murmuring · 2013-05-16T11:43:51Z

As has been noted by "http://www.djerickson.com/arduino/" (all credits to that person).
It seems there may be a mistake in the ADC initialization which results in a relatively slow analogRead, for a full description of his finding go to the link, but the summary of the problem (quoted):

Here are the #defines for the STARTUP field from
\arduino\sam\system\libsam\include\adc.h

/* The normal adc startup time*/
#define ADC_STARTUP_NORM     40
/* The fast adc startup time*/
#define ADC_STARTUP_FAST     12

This may be a bug. STARTUP is a four bit field so 12 (0xC) works, but 40 (0x28) won't work. And 12 sets the value to 768 which is why the ADC is so slow. So I suspect that the programmer confused the value with the hardware settings. The table of values vs. register settings is in the SAM3S processor manual.

The text was updated successfully, but these errors were encountered:

jtouch · 2013-06-06T18:30:38Z

The values 40 and 12 accurately describe the number of microseconds required, but there is no equation that goes from those times to the values set in the STARTUP bitfield; it's more of a map that correlates bitfield values to numbers of ADC clock cycles:
0 SUT0 0 periods of ADCClock
1 SUT8 8 periods of ADCClock
2 SUT16 16 periods of ADCClock
3 SUT24 24 periods of ADCClock
4 SUT64 64 periods of ADCClock
5 SUT80 80 periods of ADCClock
6 SUT96 96 periods of ADCClock
7 SUT112 112 periods of ADCClock
8 SUT512 512 periods of ADCClock
9 SUT576 576 periods of ADCClock
10 SUT640 640 periods of ADCClock
11 SUT704 704 periods of ADCClock
12 SUT768 768 periods of ADCClock
13 SUT832 832 periods of ADCClock
14 SUT896 896 periods of ADCClock
15 SUT960 960 periods of ADCClock

For the Due, the ADC clock is determined by a prescalar from MCK, and it's available in the code as a function. The code in adc.c for the function adc_init appears to lack this table above, and instead uses the values ADC_STARTUP_NORM directly.

…by a factor x10) Discussion: http://www.djerickson.com/arduino/ http://forum.arduino.cc/index.php?topic=156849 http://forum.arduino.cc/index.php?topic=187693 Fixes #1418

cmaglie · 2013-09-25T23:25:13Z

The hardware specification state that 20 ADCClock cycles are required for each conversion, so the minimum safe value is "3" => 24 ADCClock.

Thanks!
C

jtouch · 2013-09-26T08:49:47Z

Hi, Cristian,

I could not determine the correct safe value because the number of
cycles depends on the prescale factor:

ADCClock = MCK / ( (PRESCAL+1) * 2 )

(spec sheet page 1346)

Can you clarify why 20 ADCClock cycles is always correct?

Joe

On 9/25/2013 4:25 PM, Cristian Maglie wrote:

The hardware specification state that 20 ADCClock cycles are required
for each conversion, so the minimum safe value is "3" => 24 ADCClock.

Thanks!
C

—
Reply to this email directly or view it on GitHub
#1418 (comment)
Bug from MailScannerWebBug

cmaglie · 2013-09-26T09:19:26Z

Well, it took some time for me to decode, from the following table:

We deduce that ADCClock should be <= 20MHZ. The CPU on the Due runs at 84MHz so we are forced to use PRESCALER=2 that gives 84/(2+1)_2=14Mhz. Using PRESCALER=1 gives 84/(1+1)_2=21 that is 1Mhz off (the ADC will probably runs fine also with a small overclock like this, but that its the final user's choice, we can't deliver code that don't operate in the safe area).

From the following table:

we see that conversion time is always 20 ADCClock (well, its 20 T_cp_adc that is 1/f_adc).

This is my understating of the Datasheet, I'll be glad if someone can confirm if is correct.

Thanks!
C

jtouch · 2013-09-26T10:41:31Z

Hi,

On 9/26/2013 2:19 AM, Cristian Maglie wrote:

Well, it took some time for me to decode, from the following table:

sam3-1
https://f.cloud.github.com/assets/423515/1216435/9eb1047a-268a-11e3-8640-def9ae2e27ad.png

We deduce that ADCClock should be <= 20MHZ.

I agree.

The CPU on the Due runs at
84MHz

The CPU runs at HCLK rates.

The ADC is derived from MCK rates. MCK is derived from the external
crystal tied to XIN/XOUT, which is 12 MHz here, which can be scaled down
further.

I'm not sure what the scale for MCK is. However, assuming no scale, MCK
is at most 12 MHz.

so we are forced to use PRESCALER=2 that gives 84/(2+1)*2=14Mhz.

ADCClock = MCK / ( (PRESCAL+1) * 2 )

MCK = 12 MHz (let's assume)

Using PRESCALER =1 (is that what it is?) then ADCClock is 3 MHz. Even
though it can be faster, it isn't here.

Using PRESCALER=1 gives 84/(1+1)*2=21 that is 1Mhz off (the ADC will
probably runs fine also with a small overclock like this, but that its
the final user's choice, we can't deliver code that don't operate in the
safe area).

From the following table:

sam3-2
https://f.cloud.github.com/assets/423515/1216436/a2553830-268a-11e3-824a-031747e335a0.png

we see that conversion time is always 20 ADCClock (well, its 20 T_cp_adc
that is 1/f_adc).

I don't understand that. STARTUP varies from 0 to 960 periods of the
ADCClock. There would be no point in having that table if the answer was
always "20 clocks" - i.e., if so, there would never be a reason to set
STARTUP to a value other than 3.

So I don't think this is correct.

I think the answer in the code should be:

given ADC_STARTUP_NORM  (which we have)
given PRESCAL (which we need to get somehow from elsewhere in
    the code??)
given MCK = 12 MHz (which we know from the Due schematic)

ADCClock = ADCClock = MCK / ( (PRESCAL+1) * 2 )

STARTUP = lookup_in_table(ADC_STARTUP_NORM / ADCClock)

(where "lookup_in_table" finds the STARTUP value that corresponds to the
number of ADC clock cycles needed as per the table I posted earlier)

I cannot see a way to avoid including the table and a lookup function. I
also have no way to figure out what PRESCAL is.

Joe

This is my understating of the Datasheet, I'll be glad if someone can
confirm if is correct.

Joe

Thanks!
C

—
Reply to this email directly or view it on GitHub
#1418 (comment)
Bug from MailScannerWebBug

cmaglie · 2013-09-26T12:48:41Z

Ok, let's face one piece at a time.

The 12MHz external oscillator goes through PLLA that scales clock to 84Mhz. The Due Board runs at 84Mhz there is no doubt about that, so MCK is 84M.

Setup code here:
https://github.com/arduino/Arduino/blob/ide-1.5.x/hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/source/system_sam3xa.c#L28

Internal schematics here:

MCK==84M means that we must set the PRESCAL value to 2, because

ADCClock = MCK / ( (PRESCAL+1) * 2 ) = 84M / ((2+1)*2 = 14M

using 1 we get:

ADCClock = MCK / ( (PRESCAL+1) * 2 ) = 84M / ((1+1)*2 = 21M > 20M (as per datasheet specification)

do you agree until now?

jtouch · 2013-09-26T13:24:39Z

Hi, Christian,

On 9/26/2013 5:49 AM, Cristian Maglie wrote:

Ok, let's face one piece at a time.

The 12MHz external oscillator goes through PLLA that scales clock to
84Mhz.

PLLA is a divider (increases frequency), so you're saying DIV is set to
7. AOK.

The Due Board runs at 84Mhz there is no doubt about that, so MCK
is 84M.

Setup code here:
https://github.com/arduino/Arduino/blob/ide-1.5.x/hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/source/system_sam3xa.c#L28

Internal schematics here:
sam3-3
https://f.cloud.github.com/assets/423515/1217403/a85268f0-26a5-11e3-98dc-bf232391a2c5.png

AOK. Got it ;-)

MCK==84M means that we must set the PRESCAL value to 2, because

ADCClock = MCK / ( (PRESCAL+1) * 2 ) = 84M / ((2+1)*2 = 14M

using 1 we get:

ADCClock = MCK / ( (PRESCAL+1) * 2 ) = 84M / ((1+1)*2 = 21M > 20M (as
per datasheet specification)

do you agree until now?

Yes. I see now.

I agree that the conversion time is 20 ADC clocks, but that's not
relevant to the STARTUP time.

If ADCClock is 14MHz (I agree now), then:

STARTUP = lookup_in_table(ADC_STARTUP_NORM / ADCClock_cycle)

ADC_STARTUP_NORM is 40 (microseconds)
in hardware/arduino/sam/system/libsam/include/adc.h
also as per max time from "off" in the specs on Table 46-28. Channel
Conversion Time and ADC Clock

ADCClock_cycle = 1 / ADCClock

i.e., 71.143 ns

so we should lookup the appropriate value for (40 microsec / 71.143 ns) or

562.25

So STARTUP should be "9" - i.e., 576 cycles.

8 is too small (512 cycles), and 3 (24 cycles) is way too small.

AFAICT ;-)

Joe

jtouch · 2013-09-26T13:43:40Z

PS - this also explains why the original value of writing
ADC_STARTUP_NORM into the STARTUP register worked.

Writing 40 effectively writes 12 (because the register uses only the
lower 4 bits). 12 represents 768 clock cycles, which is more than enough
for the max of 560 needed (14 MHz * 40 microseconds).

Note that the max is 40 microseconds only when going from off to normal
mode. The max is 12 when going from Standby, where the voltage reference
is already on. In that case, 14 MHz * 12 microseconds would be 168
cycles, which corresponds to a STARTUP value of 8 (512 cycles).

If we went with typical values rather than max (worst case), we'd get a
lot better performance especailly for the latter case (from Standby),
e.g., 112 cycles (exactly a STARTUP of 8), which is 4x faster.

This all matters only when going from OFF to running, or standby to
running. In the usual case, when everything is up, the 20 cycle delay
you cited earlier would apply - i.e., 700,000 samples/second if
everything else could keep up.

Joe

On 9/26/2013 6:24 AM, Joe Touch wrote:

Hi, Christian,

On 9/26/2013 5:49 AM, Cristian Maglie wrote:

Ok, let's face one piece at a time.

The 12MHz external oscillator goes through PLLA that scales clock to
84Mhz.

PLLA is a divider (increases frequency), so you're saying DIV is set to
7. AOK.

The Due Board runs at 84Mhz there is no doubt about that, so MCK
is 84M.

Setup code here:
https://github.com/arduino/Arduino/blob/ide-1.5.x/hardware/arduino/sam/system/CMSIS/Device/ATMEL/sam3xa/source/system_sam3xa.c#L28

Internal schematics here:
sam3-3
https://f.cloud.github.com/assets/423515/1217403/a85268f0-26a5-11e3-98dc-bf232391a2c5.png

AOK. Got it ;-)

MCK==84M means that we must set the PRESCAL value to 2, because

ADCClock = MCK / ( (PRESCAL+1) * 2 ) = 84M / ((2+1)*2 = 14M

using 1 we get:

ADCClock = MCK / ( (PRESCAL+1) * 2 ) = 84M / ((1+1)*2 = 21M > 20M (as
per datasheet specification)

do you agree until now?

Yes. I see now.

I agree that the conversion time is 20 ADC clocks, but that's not
relevant to the STARTUP time.

If ADCClock is 14MHz (I agree now), then:
 STARTUP = lookup_in_table(ADC_STARTUP_NORM / ADCClock_cycle)
ADC_STARTUP_NORM is 40 (microseconds)
in hardware/arduino/sam/system/libsam/include/adc.h
also as per max time from "off" in the specs on Table 46-28. Channel
Conversion Time and ADC Clock
 ADCClock_cycle = 1 / ADCClock
i.e., 71.143 ns

so we should lookup the appropriate value for (40 microsec / 71.143 ns) or
 562.25
So STARTUP should be "9" - i.e., 576 cycles.

8 is too small (512 cycles), and 3 (24 cycles) is way too small.

AFAICT ;-)

Joe

cmaglie · 2013-09-26T14:38:04Z

Joe,

you're right, I misinterpreted that part of the datasheet and totally agree with you now.

Looking at the adc_init function I see also that is buggy and it fails calculations:

https://github.com/arduino/Arduino/blob/ide-1.5.x/hardware/arduino/sam/system/libsam/source/adc.c#L84

uint32_t adc_init(Adc *p_adc, const uint32_t ul_mck, const uint32_t ul_adc_clock,
   const uint32_t ul_startuptime)
{
   uint32_t ul_prescal, ul_startup;
   p_adc->ADC_CR = ADC_CR_SWRST;

   /* Reset Mode Register. */
   p_adc->ADC_MR = 0;

   /* Reset PDC transfer. */
   p_adc->ADC_PTCR = (ADC_PTCR_RXTDIS | ADC_PTCR_TXTDIS);
   p_adc->ADC_RCR = 0;
   p_adc->ADC_RNCR = 0;
   ul_prescal = ul_mck / (2 * ul_adc_clock) - 1;
   ul_startup = ((ul_adc_clock / 1000000) * ul_startuptime / 8) - 1;
   p_adc->ADC_MR |= ADC_MR_PRESCAL(ul_prescal) |
         ((ul_startup << ADC_MR_STARTUP_Pos) &
         ADC_MR_STARTUP_Msk);
   return 0;
}

in our case:

adc_init(ADC, SystemCoreClock, ADC_FREQ_MAX, ADC_STARTUP_FAST);

it will set ul_precal to = ul_mck / (2 * ul_adc_clock) - 1 = 84M / (2 * 20M) -1 = 1
where it should be 2, and also the ul_startup will be wrong because it uses a linear scale of 8 instead of the conversion table... ARGH.

Besides that, remains the last question: if this is the "startup" time, it should be applied only to the first ADC conversion, while the following should run at the maximum speed allowing a theoretical conversion rate of 700K in our case right?

C

jtouch · 2013-09-26T14:45:19Z

Hi, Cristian,

On 9/26/2013 7:38 AM, Cristian Maglie wrote:

Joe,

you're right, I misinterpreted that part of the datasheet and totally
agree with you now.

We both learned a lot in this process, IMO ;-)

Looking at the adc_init function I see also that is buggy and it fails
calculations:

https://github.com/arduino/Arduino/blob/ide-1.5.x/hardware/arduino/sam/system/libsam/source/adc.c#L84

|uint32_t adc_init(Adc *p_adc, const uint32_t ul_mck, const uint32_t ul_adc_clock,
const uint32_t ul_startuptime)
{
uint32_t ul_prescal, ul_startup;
p_adc->ADC_CR = ADC_CR_SWRST;
/* Reset Mode Register. */
p_adc->ADC_MR = 0;

/* Reset PDC transfer. */
p_adc->ADC_PTCR = (ADC_PTCR_RXTDIS | ADC_PTCR_TXTDIS);
p_adc->ADC_RCR = 0;
p_adc->ADC_RNCR = 0;
ul_prescal = ul_mck / (2 * ul_adc_clock) - 1;
ul_startup = ((ul_adc_clock / 1000000) * ul_startuptime / 8) - 1;
p_adc->ADC_MR |= ADC_MR_PRESCAL(ul_prescal) |
      ((ul_startup << ADC_MR_STARTUP_Pos) &
      ADC_MR_STARTUP_Msk);
return 0;
}
|

in our case:

|adc_init(ADC, SystemCoreClock, ADC_FREQ_MAX, ADC_STARTUP_FAST);
|

it will set ul_precal to = ul_mck / (2 * ul_adc_clock) - 1 = 84M / (2 *
20M) -1 = 1
where it should be 2, and also the ul_startup will be wrong because it
uses a linear scale of 8 instead of the conversion table... ARGH.

Right; that's what threw up a red flag to me; I expected the table.

Besides that, remains the last question: if this is the "startup" time,
it should be applied only to the first ADC conversion, while the
following should run at the maximum speed allowing a theoretical
conversion rate of 700K in our case right?

AFAICT, yes - and there should be one of two different startups
depending on whether it's a cold start or wakeup from sleep, but I don't
suspect the sleep function is typically used much.

Joe

C

—
Reply to this email directly or view it on GitHub
#1418 (comment)
Bug from MailScannerWebBug

See #1418

cmaglie · 2013-10-09T14:02:23Z

I've pushed a fix to the adc_init() function, now the register configuration is correct, but the gain in ADC performance is gone (it runs almost at the same speed as before).

I see that the analogRead() function enable and disable the ADC each time is called, may this force the ADC to always wait the startup time causing the poor conversion speed?

C

jtouch · 2013-10-09T17:27:16Z

Hi,

On 10/9/2013 7:02 AM, Cristian Maglie wrote:

I've pushed a fix to the adc_init() function, now the register
configuration is corret, but there gain in ADC performance is gone (it
runs almost at the same speed as before).

Right; that was what I expected. The number used before was basically
close to what we ended up figuring out.

I see that the analogRead() function enable and disable the ADC each
time is called, may this force the ADC to always wait the startup time
causing the poor conversion speed?

The setting we need depends on whether the system is in sleep mode or
fast wakeup mode. We calculated it from sleep, the more conservative
number. Fast-sleep is supposed to speed things up, but given our clock
rates it won't:

set FWUP = 1 (faster wakeup, but worse power consumption)

12 microsec / 71.143 ns = 169 clocks

STARTUP would be: 8 = 512 periods of ADCClock
(vs. 9 = 576 periods)

Fast wakeup isn't worth it.

It might be useful to do something to allow the user to say "don't sleep
ADC" vs "sleep ADC". In the don't-sleep mode, power goes up, but the
rate of conversions would be very fast - close to 1M samples/sec
according to the specs.

The specs claim the DAC costs 12.34 microamps/Mhz, so running the ADC
all the time might consume 1 milliamp of additional power. I don't know
for sure.

Joe

C

—
Reply to this email directly or view it on GitHub
#1418 (comment)
Bug from MailScannerWebBug

See arduino#1418

cmaglie · 2013-10-21T10:45:17Z

@jtouch and anyone else,

would you like to review the pull request #1634 and check for any problems?
C

jtouch · 2013-10-21T18:37:01Z

I just did; the cost is 5-9mA vs. using sleep mode. I think it's worth the benefit to the ADC.

cmaglie · 2013-10-22T09:48:41Z

Thanks.
I'm closing this issue since the initialization is now fixed.
I'll leave open only the pull request for a while.

C

seb46 · 2014-02-23T10:08:07Z

En français...
J'ai eu un problème avec cette nouvelle modification.
Je développe un programme avec un Arduino Due qui lit alternativement une photorésistante et un l'écran tactile. J'utilise l'entrée analogie A0, A10 et A11. Si je ne fait pas un adc_disable_channel, La lecture les entrées analogiques est incorrecte... J'ai donc modifé le code de la façon suivant :

google translation in English
I had a problem with this new change. I develop a program that reads an Arduino Due alternately a photoresist and a touch screen. I use the analogy A0, A10 and A11 entry. If I do not make a adc_disable_channel, Reading analog inputs is incorrect ... So I rotated the code following way:

File 'wiring_analog.c' line 152 now :

// Enable the corresponding channel
if (ulChannel != latestSelectedChannel) {
  adc_disable_channel(ADC,latestSelectedChannel);
  adc_enable_channel( ADC, ulChannel );
  latestSelectedChannel = ulChannel;
}

File 'wiring_analog.c' line 152 before :

// Enable the corresponding channel
if (ulChannel != latestSelectedChannel) {
  adc_enable_channel( ADC, ulChannel );
  latestSelectedChannel = ulChannel;
}

cmaglie · 2014-02-23T19:14:57Z

Hi @seb46,
have you tried with IDE 1.5.6-r2?
http://arduino.cc/en/Main/Software#toc3

seb46 · 2014-02-25T00:15:14Z

In IDE 1.5.5-r2 with the modification that I had brought my program run. I compiled my sources with 1.5.6-r2 IDE. But for some reason I have not yet found the analog output provided me a bad value and fixed. I would look better tonight.

seb46 · 2014-02-25T00:24:41Z

I found ... It was simple. He just put faillait 'adc_disable_channel' before 'adc_enable_channel'

File 'wiring_analog.c' line 150 now :

// Enable the corresponding channel
if (ulChannel != latestSelectedChannel) {
    if ( latestSelectedChannel != -1 )
        adc_disable_channel( ADC, latestSelectedChannel );
    adc_enable_channel( ADC, ulChannel );
    latestSelectedChannel = ulChannel;
}

File 'wiring_analog.c' line 150 before :

// Enable the corresponding channel
if (ulChannel != latestSelectedChannel) {
    adc_enable_channel( ADC, ulChannel );
    if ( latestSelectedChannel != -1 )
        adc_disable_channel( ADC, latestSelectedChannel );
    latestSelectedChannel = ulChannel;
}

cmaglie · 2014-02-25T08:54:12Z

@seb46
thanks for the report!
the wrong readings may be due to high impedance of the analog sources: after the latest updates analogRead achieves A/D conversions at very high speed but the input source impedance may affect the precision.

May I ask you to try to "double" the readings, for example in your sketch where you do:

data = analogRead(1);

you should do that twice (or even three times) and keep only the last reading:

data = analogRead(1);
data = analogRead(1); // Do it again to allow sampling-capacitor to charge up

and see if the thing go better?

seb46 · 2014-02-25T17:21:14Z

If the file wiring_analog.c I leave:

if (ulChannel != latestSelectedChannel) {
  adc_enable_channel( ADC, ulChannel );
  if ( latestSelectedChannel != -1 )
    adc_disable_channel( ADC, latestSelectedChannel );
  latestSelectedChannel = ulChannel;
}

The more analogRead(1) is great, the better the result. But after 10 analogRead(1), it is still not quite right.

If, against the file 'wiring_analog.c' I reverse:

if (ulChannel != latestSelectedChannel) {
  if ( latestSelectedChannel != -1 )
    adc_disable_channel( ADC, latestSelectedChannel );
  adc_enable_channel( ADC, ulChannel );
  latestSelectedChannel = ulChannel;
}

From the first analogRead (1), the result is good. On the analog input of the photoresist and the analog input of the touch screen.

cmaglie · 2014-02-26T12:41:50Z

Hi @seb46

may you give me some more details on your test circuit (for example the photoresistor and the additional resistors values) so I can try to reproduce the problem?
How much is the difference from the good and the bad readings?

Thank you!

seb46 · 2014-02-26T18:51:37Z

In simple terms, here's a tip from one end of the circuit and code

first of all :
PIN 3.3V <-> PIN AREF

Photoresistor :
GND <-> R=10K ohm <-> PIN 54 (A0) <-> Photoresistor 24~2850 K ohm <-> 3.3V

Touchscreen with TFT ILI9328 :
PIN XP <-> PIN 30
PIN YP Ana <-> PIN 65 (A11)
PIN XM Ana <-> PIN 64 (A10)
PIN YM <-> PIN 33

void setup(void)
{
  ...
  analogReadResolution(12);
  ...
  //For Photoresistor
  ...
 pinMode(54,OUTPUT);
 ...
 //For Touchscreen
  //Pin_xp=30
  //Pin_yp=65
  //Pin_xm=64
  //Pin_ym=33
  ...
  Bit_xp=digitalPinToBitMask(Pin_xp);
  Port_xp=digitalPinToPort(Pin_xp);
  Bit_yp=digitalPinToBitMask(Pin_yp_ana);
  Port_yp=digitalPinToPort(Pin_yp_ana);
  Bit_xm=digitalPinToBitMask(Pin_xm_ana);
  Port_xm=digitalPinToPort(Pin_xm_ana);
  Bit_ym=digitalPinToBitMask(Pin_ym);
  Port_ym=digitalPinToPort(Pin_ym);
  Port_xp->PIO_IDR=Bit_xp;
  Port_xp->PIO_PUDR=Bit_xp;
  Port_xp->PIO_MDER=Bit_xp;
  Port_xp->PIO_IFDR=Bit_xp;
  Port_xp->PIO_OER=Bit_xp;
  PMC->PMC_PCER0=1<<g_APinDescription[Pin_xp].ulPeripheralId;
  Port_xp->PIO_CODR=Bit_xp;
  Port_xp->PIO_OER=Bit_xp;
  Port_xp->PIO_PER=Bit_xp;
  Port_yp->PIO_IDR=Bit_yp;
  Port_yp->PIO_PUDR=Bit_yp;
  Port_yp->PIO_MDER=Bit_yp;
  Port_yp->PIO_IFDR=Bit_yp;
  Port_yp->PIO_OER=Bit_yp;
  PMC->PMC_PCER0=1<<g_APinDescription[Pin_yp_ana].ulPeripheralId;
  Port_yp->PIO_CODR=Bit_yp;
  Port_yp->PIO_OER=Bit_yp;
  Port_yp->PIO_PER=Bit_yp;
  Port_xm->PIO_IDR=Bit_xm;
  Port_xm->PIO_PUDR=Bit_xm;
  Port_xm->PIO_MDER=Bit_xm;
  Port_xm->PIO_IFDR=Bit_xm;
  Port_xm->PIO_OER=Bit_xm;
  PMC->PMC_PCER0=1<<g_APinDescription[Pin_xm_ana].ulPeripheralId;
  Port_xm->PIO_CODR=Bit_xm;
  Port_xm->PIO_OER=Bit_xm;
  Port_xm->PIO_PER=Bit_xm;
  Port_ym->PIO_IDR=Bit_ym;
  Port_ym->PIO_PUDR=Bit_ym;
  Port_ym->PIO_MDER=Bit_ym;
  Port_ym->PIO_IFDR=Bit_ym;
  Port_ym->PIO_OER=Bit_ym;
  PMC->PMC_PCER0=1<<g_APinDescription[Pin_ym].ulPeripheralId;
  Port_ym->PIO_CODR=Bit_ym;
  Port_ym->PIO_OER=Bit_ym;
  Port_ym->PIO_PER=Bit_ym;
  ...
}

void loop(void)
{
  //For Photoresistor
  ...
  XXX=analogRead(54):
  ...
  //For Touchscreen
  ...
  Port_xp->PIO_CODR=Bit_xp; //digitalWrite(Pin_xp,LOW);
  Port_ym->PIO_SODR=Bit_ym; //digitalWrite(Pin_ym,HIGH);
  Port_xp->PIO_OER=Bit_xp; //pinMode(Pin_xp,OUTPUT);
  Port_ym->PIO_OER=Bit_ym; //pinMode(Pin_ym,OUTPUT);
  Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
  Port_yp->PIO_CODR=Bit_yp; //digitalWrite(Pin_yp,LOW);
  Port_xm->PIO_ODR=Bit_xm; //pinMode(Pin_xm,INPUT);
  Port_yp->PIO_ODR=Bit_yp; //pinMode(Pin_yp,INPUT);
  z1=analogRead(Pin_xm);
  z2=analogRead(Pin_yp);
  ...
  Port_xp->PIO_SODR=Bit_xp; //digitalWrite(Pin_xp,HIGH);
  Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
  Port_xp->PIO_OER=Bit_xp; //pinMode(Pin_xp,OUTPUT);
  Port_xm->PIO_OER=Bit_xm; //pinMode(Pin_xm,OUTPUT);
  Port_yp->PIO_CODR=Bit_yp; //digitalWrite(Pin_yp,LOW);
  Port_ym->PIO_CODR=Bit_ym; //digitalWrite(Pin_ym,LOW);
  Port_yp->PIO_ODR=Bit_yp; //pinMode(Pin_yp,INPUT);
  Port_ym->PIO_ODR=Bit_ym; //pinMode(Pin_ym,INPUT);
  for (cpt=0;cpt\<9;cpt++) {
    ListeX[cpt]=analogRead(Pin_yp);
    ...
  } //for
  ...
  Port_yp->PIO_SODR=Bit_yp; //digitalWrite(Pin_yp,HIGH);
  Port_ym->PIO_CODR=Bit_ym; //digitalWrite(Pin_ym,LOW);
  Port_yp->PIO_OER=Bit_yp; //pinMode(Pin_yp,OUTPUT);
  Port_ym->PIO_OER=Bit_ym; //pinMode(Pin_ym,OUTPUT);
  Port_xp->PIO_CODR=Bit_xp; //digitalWrite(Pin_xp,LOW);
  Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
  Port_xp->PIO_ODR=Bit_xp; //pinMode(Pin_xp,INPUT);
  Port_xm->PIO_ODR=Bit_xm; //pinMode(Pin_xm,INPUT);
  for(cpt=0;cpt\<9;cpt++) {
    ListeY[cpt]=analogRead(Pin_xm);
    ...
  } //for
  ...
  Port_xp->PIO_CODR=Bit_xp; //digitalWrite(Pin_xp,LOW);
  Port_ym->PIO_SODR=Bit_ym; //digitalWrite(Pin_ym,HIGH);
  Port_xp->PIO_OER=Bit_xp; //pinMode(Pin_xp,OUTPUT);
  Port_ym->PIO_OER=Bit_ym; //pinMode(Pin_ym,OUTPUT);
  Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
  Port_yp->PIO_CODR=Bit_yp; //digitalWrite(Pin_yp,LOW);
  Port_xm->PIO_ODR=Bit_xm; //pinMode(Pin_xm,INPUT);
  Port_yp->PIO_ODR=Bit_yp; //pinMode(Pin_yp,INPUT);
  z1=analogRead(Pin_xm);
  z2=analogRead(Pin_yp);
  ;...
}

jtouch · 2014-02-26T19:53:57Z

On 2/26/2014 10:51 AM, seb46 wrote:

In simple terms, here's a tip from one end of the circuit and code
first of all :
PIN 3.3V <-> PIN AREF

Photoresistor :
GND <-> R=10K ohm <-> PIN 54 (A0) <-> Photoresistor 24~2850 K ohm <-> 3.3V

Touchscreen with TFT ILI9328 :
PIN XP <-> PIN 30
PIN YP Ana <-> PIN 65 (A11)
PIN XM Ana <-> PIN 64 (A10)
PIN YM <-> PIN 33

It might be preferable to use analog pins that aren't overloaded with
other functions, e.g., presuming this is a Due (from the "pin numbers"
above), maybe try using three from A1..A7 instead of A0, A10, and A11.

|
|

{
...
analogReadResolution(12);
...
//For Photoresistor
...
pinMode(54,OUTPUT);

The above is an error; it should be INPUT.

...
//For Touchscreen
//Pin_xp=30
//Pin_yp=65
//Pin_xm=64
//Pin_ym=33

Why are the above commented out, when you use symbolic names in the rest
of this?

...
Bit_xp=digitalPinToBitMask(Pin_xp);
Port_xp=digitalPinToPort(Pin_xp);

Not sure what you're doing below here: these are analog pins.

Bit_yp=digitalPinToBitMask(Pin_yp_ana);
Port_yp=digitalPinToPort(Pin_yp_ana);
Bit_xm=digitalPinToBitMask(Pin_xm_ana);
Port_xm=digitalPinToPort(Pin_xm_ana);

Bit_ym=digitalPinToBitMask(Pin_ym);
Port_ym=digitalPinToPort(Pin_ym);
Port_xp->PIO_IDR=Bit_xp;
Port_xp->PIO_PUDR=Bit_xp;
Port_xp->PIO_MDER=Bit_xp;
Port_xp->PIO_IFDR=Bit_xp;
Port_xp->PIO_OER=Bit_xp;
PMC->PMC_PCER0=1< Port_xp->PIO_CODR=Bit_xp;
Port_xp->PIO_OER=Bit_xp;
Port_xp->PIO_PER=Bit_xp;
Port_yp->PIO_IDR=Bit_yp;
Port_yp->PIO_PUDR=Bit_yp;
Port_yp->PIO_MDER=Bit_yp;
Port_yp->PIO_IFDR=Bit_yp;
Port_yp->PIO_OER=Bit_yp;
PMC->PMC_PCER0=1< Port_yp->PIO_CODR=Bit_yp;
Port_yp->PIO_OER=Bit_yp;
Port_yp->PIO_PER=Bit_yp;
Port_xm->PIO_IDR=Bit_xm;
Port_xm->PIO_PUDR=Bit_xm;
Port_xm->PIO_MDER=Bit_xm;
Port_xm->PIO_IFDR=Bit_xm;
Port_xm->PIO_OER=Bit_xm;
PMC->PMC_PCER0=1< Port_xm->PIO_CODR=Bit_xm;
Port_xm->PIO_OER=Bit_xm;
Port_xm->PIO_PER=Bit_xm;
Port_ym->PIO_IDR=Bit_ym;
Port_ym->PIO_PUDR=Bit_ym;
Port_ym->PIO_MDER=Bit_ym;
Port_ym->PIO_IFDR=Bit_ym;
Port_ym->PIO_OER=Bit_ym;
PMC->PMC_PCER0=1< Port_ym->PIO_CODR=Bit_ym;
Port_ym->PIO_OER=Bit_ym;
Port_ym->PIO_PER=Bit_ym;
...
}

void loop(void)
{
//For Photoresistor
....
XXX=analogRead(54):

Why are you referring to this by number, vs. using "A0"?

And why not refer to it by name (photo_in), where you could say:

#define photo_in A0

....
//For Touchscreen
...
Port_xp->PIO_CODR=Bit_xp; //digitalWrite(Pin_xp,LOW);
Port_ym->PIO_SODR=Bit_ym; //digitalWrite(Pin_ym,HIGH);
Port_xp->PIO_OER=Bit_xp; //pinMode(Pin_xp,OUTPUT);
Port_ym->PIO_OER=Bit_ym; //pinMode(Pin_ym,OUTPUT);
Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
Port_yp->PIO_CODR=Bit_yp; //digitalWrite(Pin_yp,LOW);
Port_xm->PIO_ODR=Bit_xm; //pinMode(Pin_xm,INPUT);
Port_yp->PIO_ODR=Bit_yp; //pinMode(Pin_yp,INPUT);
z1=analogRead(Pin_xm);
z2=analogRead(Pin_yp);
....
Port_xp->PIO_SODR=Bit_xp; //digitalWrite(Pin_xp,HIGH);
Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
Port_xp->PIO_OER=Bit_xp; //pinMode(Pin_xp,OUTPUT);
Port_xm->PIO_OER=Bit_xm; //pinMode(Pin_xm,OUTPUT);
Port_yp->PIO_CODR=Bit_yp; //digitalWrite(Pin_yp,LOW);
Port_ym->PIO_CODR=Bit_ym; //digitalWrite(Pin_ym,LOW);
Port_yp->PIO_ODR=Bit_yp; //pinMode(Pin_yp,INPUT);
Port_ym->PIO_ODR=Bit_ym; //pinMode(Pin_ym,INPUT);
for (cpt=0;cpt<9;cpt++) {
ListeX[cpt]=analogRead(Pin_yp);
...
} //for
...
Port_yp->PIO_SODR=Bit_yp; //digitalWrite(Pin_yp,HIGH);
Port_ym->PIO_CODR=Bit_ym; //digitalWrite(Pin_ym,LOW);
Port_yp->PIO_OER=Bit_yp; //pinMode(Pin_yp,OUTPUT);
Port_ym->PIO_OER=Bit_ym; //pinMode(Pin_ym,OUTPUT);
Port_xp->PIO_CODR=Bit_xp; //digitalWrite(Pin_xp,LOW);
Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
Port_xp->PIO_ODR=Bit_xp; //pinMode(Pin_xp,INPUT);
Port_xm->PIO_ODR=Bit_xm; //pinMode(Pin_xm,INPUT);
for(cpt=0;cpt<9;cpt++) {
ListeY[cpt]=analogRead(Pin_xm);
...
} //for
...
Port_xp->PIO_CODR=Bit_xp; //digitalWrite(Pin_xp,LOW);
Port_ym->PIO_SODR=Bit_ym; //digitalWrite(Pin_ym,HIGH);
Port_xp->PIO_OER=Bit_xp; //pinMode(Pin_xp,OUTPUT);
Port_ym->PIO_OER=Bit_ym; //pinMode(Pin_ym,OUTPUT);
Port_xm->PIO_CODR=Bit_xm; //digitalWrite(Pin_xm,LOW);
Port_yp->PIO_CODR=Bit_yp; //digitalWrite(Pin_yp,LOW);
Port_xm->PIO_ODR=Bit_xm; //pinMode(Pin_xm,INPUT);
Port_yp->PIO_ODR=Bit_yp; //pinMode(Pin_yp,INPUT);
z1=analogRead(Pin_xm);
z2=analogRead(Pin_yp);
...
}
|

—
Reply to this email directly or view it on GitHub
#1418 (comment)
Bug from MailScannerWebBug

seb46 · 2014-03-06T12:10:39Z

The code I put, is only a small piece of that 155KB program. This is the reason Laquel I did a shortcut in the code that I put.
Indeed, we must pinMode(54,INPUT) instead of pinMode (54,OUTPUT).
To go faster, I directly use registers to certain operation. Hence the following line:

Bit_XX=digitalPinToBitMask(Pin_XX);
Port_XX=digitalPinToPort(Pin_XX);
-- for setup --
Port_XX->PIO_IDR=Bit_XX;
Port_XX->PIO_PUDR=Bit_XX;
Port_XX->PIO_MDER=Bit_XX;
Port_XX->PIO_IFDR=Bit_XX;
Port_XX->PIO_OER=Bit_XX;
PMC->PMC_PCER0=1<Port_XX->PIO_CODR=Bit_XX;
Port_XX->PIO_OER=Bit_xp;
Port_XX->PIO_PER=Bit_xp;
-- and for --
Port_XX->PIO_SODR=Bit_XX; //digitalWrite(Pin_XX,HIGH);
Port_XX->PIO_CODR=Bit_XX; //digitalWrite(Pin_XX,LOW);
Port_XX->PIO_OER=Bit_XX; //pinMode(Pin_XX,OUTPUT);
Port_XX->PIO_ODR=Bit_XX; //pinMode(Pin_XX,INPUT)

But in any case, if the file 'wiring_analog.c' I put:

if (ulChannel != latestSelectedChannel) {
  if ( latestSelectedChannel != -1 )
    adc_disable_channel( ADC, latestSelectedChannel );
  adc_enable_channel( ADC, ulChannel );
  latestSelectedChannel = ulChannel;
}

From the first analogRead(X), the result is good.

cmaglie · 2014-03-16T15:06:18Z

Hi @seb46

Your analog sources have a very high impedance (on the MOhm magnitude) and the internal sample-hold capacitor is probably affecting your signal during the first read (it would be cool to have a scope reading to confirm this).

Now, swapping the adc_enable/disable statements, as you did, will slow down the ADC sample rate (because it adds the ADC startup-time every time) and probably this helps the capacitor to fully charge-up before the conversion. Another guy had a similar problem and solved it by placing an Op-Amp between the analog source and the ADC input pin.

seb46 · 2014-03-17T23:07:21Z

I use a screen TFT 2.8'' of Adafruit (https://www.adafruit.com/products/335) with ILI9328.
The touchscreen have 640 ohm max in Y and 320 Ohm max in X. Between X and Y 1040 Ohm max.
For the Photoresistor : Resistance=10K ohm + Photoresistor 24~2850 K ohm.
With this information, do you have any idea what's wrong?

…by a factor x10) Discussion: http://www.djerickson.com/arduino/ http://forum.arduino.cc/index.php?topic=156849 http://forum.arduino.cc/index.php?topic=187693 Fixes arduino#1418

See arduino#1418

cmaglie closed this as completed Sep 25, 2013

cmaglie reopened this Sep 30, 2013

cmaglie added a commit that referenced this issue Oct 9, 2013

Fixed adc_init() function.

40dce96

See #1418

cmaglie added a commit to cmaglie/Arduino that referenced this issue Oct 21, 2013

[sam] Improved analogRead speed.

1fc54f5

See arduino#1418

cmaglie mentioned this issue Oct 21, 2013

Improved ADC speed on Arduino Due #1634

Merged

cmaglie closed this as completed Oct 22, 2013

cmaglie mentioned this issue May 5, 2015

Bug: analogRead() for Arduino Due should disable previous ADC channel before enabling new one #3064

Closed

ollie1400 pushed a commit to ollie1400/Arduino that referenced this issue May 2, 2022

Fixed adc_init() function.

b80a5ec

See arduino#1418

ollie1400 pushed a commit to ollie1400/Arduino that referenced this issue May 2, 2022

[sam] Improved analogRead speed.

320ddcb

See arduino#1418

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADC initialization mistake in ADC.h #1418

ADC initialization mistake in ADC.h #1418

murmuring commented May 16, 2013

jtouch commented Jun 6, 2013

cmaglie commented Sep 25, 2013

jtouch commented Sep 26, 2013

cmaglie commented Sep 26, 2013

jtouch commented Sep 26, 2013

cmaglie commented Sep 26, 2013

jtouch commented Sep 26, 2013

jtouch commented Sep 26, 2013

cmaglie commented Sep 26, 2013

jtouch commented Sep 26, 2013

cmaglie commented Oct 9, 2013

jtouch commented Oct 9, 2013

cmaglie commented Oct 21, 2013

jtouch commented Oct 21, 2013

cmaglie commented Oct 22, 2013

seb46 commented Feb 23, 2014

cmaglie commented Feb 23, 2014

seb46 commented Feb 25, 2014

seb46 commented Feb 25, 2014

cmaglie commented Feb 25, 2014

seb46 commented Feb 25, 2014

cmaglie commented Feb 26, 2014

seb46 commented Feb 26, 2014

jtouch commented Feb 26, 2014

seb46 commented Mar 6, 2014

cmaglie commented Mar 16, 2014

seb46 commented Mar 17, 2014

ADC initialization mistake in ADC.h #1418

ADC initialization mistake in ADC.h #1418

Comments

murmuring commented May 16, 2013

jtouch commented Jun 6, 2013

cmaglie commented Sep 25, 2013

jtouch commented Sep 26, 2013

cmaglie commented Sep 26, 2013

jtouch commented Sep 26, 2013

cmaglie commented Sep 26, 2013

jtouch commented Sep 26, 2013

jtouch commented Sep 26, 2013

cmaglie commented Sep 26, 2013

jtouch commented Sep 26, 2013

cmaglie commented Oct 9, 2013

jtouch commented Oct 9, 2013

cmaglie commented Oct 21, 2013

jtouch commented Oct 21, 2013

cmaglie commented Oct 22, 2013

seb46 commented Feb 23, 2014

cmaglie commented Feb 23, 2014

seb46 commented Feb 25, 2014

seb46 commented Feb 25, 2014

cmaglie commented Feb 25, 2014

seb46 commented Feb 25, 2014

cmaglie commented Feb 26, 2014

seb46 commented Feb 26, 2014

jtouch commented Feb 26, 2014

seb46 commented Mar 6, 2014

cmaglie commented Mar 16, 2014

seb46 commented Mar 17, 2014