[PATCH] AVR EXT Topic is solved

ChibiOS public support forum for topics related to the Atmel AVR family of micro-controllers.

Moderators: utzig, tfAteba

utzig
Posts: 355
Joined: Sat Jan 07, 2012 6:22 pm
Location: Brazil
Has thanked: 1 time
Been thanked: 19 times
Contact:

Re: [PATCH] AVR EXT

Postby utzig » Sun Oct 23, 2016 12:45 pm

Theodore ATEBA wrote: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! :)

User avatar
tfAteba
Posts: 526
Joined: Fri Oct 16, 2015 11:03 pm
Location: Chartres, France
Has thanked: 80 times
Been thanked: 46 times

Re: [PATCH] AVR EXT

Postby tfAteba » Sun Oct 23, 2016 1:35 pm

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

Thanks.
Attachments
0002-Correct-the-ext-low-level-trigger-mode.patch.zip
(813 Bytes) Downloaded 73 times
regards,

Theo.

utzig
Posts: 355
Joined: Sat Jan 07, 2012 6:22 pm
Location: Brazil
Has thanked: 1 time
Been thanked: 19 times
Contact:

Re: [PATCH] AVR EXT

Postby utzig » Sun Oct 23, 2016 1:41 pm

Awesome, applied!

User avatar
igor
Posts: 50
Joined: Sun Aug 16, 2015 7:24 pm
Location: Helsinki, Finland
Has thanked: 1 time

Re: [PATCH] AVR EXT

Postby igor » Tue Oct 25, 2016 4:02 pm

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
Posts: 355
Joined: Sat Jan 07, 2012 6:22 pm
Location: Brazil
Has thanked: 1 time
Been thanked: 19 times
Contact:

Re: [PATCH] AVR EXT

Postby utzig » Tue Oct 25, 2016 5:13 pm

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.

User avatar
igor
Posts: 50
Joined: Sun Aug 16, 2015 7:24 pm
Location: Helsinki, Finland
Has thanked: 1 time

Re: [PATCH] AVR EXT

Postby igor » Tue Oct 25, 2016 5:46 pm

utzig wrote: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
Posts: 355
Joined: Sat Jan 07, 2012 6:22 pm
Location: Brazil
Has thanked: 1 time
Been thanked: 19 times
Contact:

Re: [PATCH] AVR EXT

Postby utzig » Tue Oct 25, 2016 5:59 pm

igor wrote:* 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.

igor wrote:* 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.

igor wrote:* 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

User avatar
tfAteba
Posts: 526
Joined: Fri Oct 16, 2015 11:03 pm
Location: Chartres, France
Has thanked: 80 times
Been thanked: 46 times

Re: [PATCH] AVR EXT

Postby tfAteba » Tue Oct 25, 2016 7:09 pm

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.
regards,

Theo.

User avatar
igor
Posts: 50
Joined: Sun Aug 16, 2015 7:24 pm
Location: Helsinki, Finland
Has thanked: 1 time

Re: [PATCH] AVR EXT

Postby igor » Sun Nov 06, 2016 7:20 am

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.
Attachments
HAL_AVR_EXT_PCINT.zip
(5.36 KiB) Downloaded 68 times

User avatar
tfAteba
Posts: 526
Joined: Fri Oct 16, 2015 11:03 pm
Location: Chartres, France
Has thanked: 80 times
Been thanked: 46 times

Re: [PATCH] AVR EXT

Postby tfAteba » Sun Nov 06, 2016 4:01 pm

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.
regards,

Theo.


Return to “AVR Support”

Who is online

Users browsing this forum: No registered users and 1 guest