[PATCH] AVR EXT Topic is solved

[utzig]

The else if of course for the low level mode(EXT_CH_MODE_LOW_LEVEL). May be I should use it explicitly, just want to know for next time.

Suppose there are new modes added in the future. The user than passes some unsupported mode, our code would run the low_level interrupt mode although that's not what should happen. So the correct behavior would be to be explicit in the test. I think the else should not exist there.

Would be happy to get a new patch for this!

[tfAteba]

You are totaly rigth , so this is a patch to correct that.

Thanks.

[utzig]

Awesome, applied!

[igor]

Hello,
sorry for not sharing this sooner, I made an attempt some time ago, but i did not have much time to follow up.

In my case I use the 328p, which has only a couple of real interrupts, but then supports PORTS, where 8 lines are grouped into a PORT and an IRQ is generated whenever one of the lines belonging to the port is activated.

The driver I wrote handles both types of HW lines: direct interrupt and port.
https://github.com/igor-stoppa/ChibiOS/ ... d09ce027f8

And a small fix, I found with Coverity:
https://github.com/igor-stoppa/ChibiOS/ ... c70bf93a58

I know I'm a bit late, but I would like to make an effort to get merged also the support for PORTS, since they are the vast majority of inputs I use.
BTW, it also allows for cheaper designs because one doesn't need to resort to bigger AVRs for having more than 2 interrupts.

Any advice?

[utzig]

I think it would be great having support for PCINTs. If you can provide a patch with the changes against the SVN head (or github mirror master HEAD) that would be great.

[igor]

I think it would be great having support for PCINTs. If you can provide a patch with the changes against the SVN head (or github mirror master HEAD) that would be great.

I would need some comments about my current patch vs the implementation you just merged:

* is it ok if i use my approach (macros) to handle similar code - ex: declare_pcint_isr(index)
* I am blurring a bit the line between interrupts with dedicated event and PCINTs - the user of the driver will still be aware of what is what, from their name, but they will be treated otherwise identically.
* what is the best way to deal with multiple AVR types? so far I have only cared about 328p, but now this EXT driver is for another, so it seems clear that there needs to be a certain amount of dependency on the AVR type - at least these 2

[utzig]

* is it ok if i use my approach (macros) to handle similar code - ex: declare_pcint_isr(index)

Without looking at the code I would say yes. If it gets simpler/shorter the better.

* I am blurring a bit the line between interrupts with dedicated event and PCINTs - the user of the driver will still be aware of what is what, from their name, but they will be treated otherwise identically.

Write some short doxygen docs about it inside the driver, should be enough.

* what is the best way to deal with multiple AVR types? so far I have only cared about 328p, but now this EXT driver is for another, so it seems clear that there needs to be a certain amount of dependency on the AVR type - at least these 2

Just go with 328p as "universal". I can update for 1280/2560/32u (the models I have) later. If someone needs support for other models that have different implementations they can send the patches. Btw, the way to deal with the different parts is most probably by using #ifdefs for the specific models.

Cheers,
Fabio Utzig

[tfAteba]

Hi all,

Sorry to be late, I just had the time to connect.

I was planning to provide some ameliorations of the EXT drivers such as:
- PCINTs modules
- Make the EXT driver more generic. Support of ATmega328p ext to start.

Happy to see that I'm not the only one to focus on it. But now I want to finish a project where I effectively need EXT, after what I will do those improvements. But if you have the time to do that before me it will also be great.

[igor]

Hi,
this is my version of the driver.
I got review from utzig about 1 year ago on a version where EXT and PCINT drivers were separate.
The comment was that the EXT and PCINT drivers should be merged.
So this version treats them almost transparently.
It should apply on top of trunk and for what I could test, it seems to work fine.

Please review.

[tfAteba]

Hi Igor,

I will take a look to your EXT patch this week.

for what I could test, it seems to work fine.

I supposed that you made your test with atmega328p mcu only?

If so, I will also made test with mega2560 to see if every things is fine.

Also for test, did you use the same test program under testhal/AVR/EXT withoug any modifications? If y made your own test application could you also add it.

Thanks.