helena-project / imix

imix Low-Power IoT Research Platform
32 stars 6 forks source link

ADC pin labels #16

Closed brghena closed 7 years ago

brghena commented 7 years ago

The issue @phil-levis commented on in the ADC driver seems to be fixed in Imix now. https://github.com/helena-project/tock/blob/master/chips/sam4l/src/adc.rs#L11 (@shaneleonard let me know if that's right and I'll remove the comment)

However, the pins are still mapped with AD0 -> ADCIFE1, AD1 -> ADCIFE2, etc. Is there any reason that they shouldn't be AD0 -> ADCIFE0, AD1 -> ADCIFE1, etc. instead so that the world makes sense?

shaneleonard commented 7 years ago
screen shot 2017-03-08 at 12 12 52 pm

In v1.3 they are mapped sensibly, although they should be that way for at least v1.2 as well.

shaneleonard commented 7 years ago

I just checked and every recent revision has the sane mapping, in fact I'm pretty sure it's been mapped like this on every board. I explicitly remember identifying the weird mapping as a major issue with Firestorm, so there's no reason that I can recall where I wouldn't have fixed it from the beginning.

brghena commented 7 years ago

Thanks @shaneleonard, I'll remove the issue from the ADC driver.

However, the pins still aren't exactly labeled sensibly. The net ADCIFE0 connects the PA05 on the SAM4L, which per the datasheet is actually ADCIFE AD1.

image

image

shaneleonard commented 7 years ago

Hmm. This is starting to sound familiar. I think I must have gotten into a corner with the pin assignments where I had to use PA04 for the light sensor interrupt, and thus ADCIFE1-6 were a better option than ADCIFE0-5. However, it was silly of me to number the pins this way in the schematic, I should have kept them true to their values in the datasheet.

Firestorm's numbering was worse in the sense that they actually numbered ADCIFE0-ADCIFE5 to A5-A0 (in reverse), which is where Phil's comment came from.

After looking back through the datasheet, there are luckily a couple of ways to fix this:

(1) Just rename the schematic to be more accurate (ADCIFE1->A0, etc) or (2) Reassign some pins so that LIGHT_INT goes to PC25 (still EXTINT2), GPIO8 gets bumped to any accessible pin, I use ADCIFE0-5 for A0-5, and I use the newly free ADCIFE6 as the analog sense pin for the TRNG. This would also take care of #8.

shaneleonard commented 7 years ago

Looking at the layout, option 2 actually shouldn't be too difficult. I can do it for v1.3 but then unfortunately people will have to be aware of this change until everyone on 1.2 moves to 1.3

alevy commented 7 years ago

@shaneleonard I wouldn't worry about that.

shaneleonard commented 7 years ago

@alevy By 'that' you mean people being aware of this right?

alevy commented 7 years ago

Yes, sorry :) To reiterate: breaking changes are fine at this stage.

shaneleonard commented 7 years ago

Great. I'll change the pins on v1.3 and update the schematics to be more accurate.