SAM AFEC wrapping Atmel's ASF library [2 Attachments]

[[e-mail] [nuttx]]

Hello all.

I've implemented an AFEC driver for SAM34 by wrapping Atmel's ASF code. The idea
is to have common driver for all SAM chips supporting it. The implementation
needs more work, but the basic ADC example works.

Been working on this driver intermittently for a while now, but I realised it's
best to push it upstream in case someone else is interested in this development
and to have it taken in consideration in API changes (like the recent
adc_receive one). Also I would like some feedback on the direction taken.

My repo for this is here: https://github.com/orbifx/nuttx/tree/sam-afec (Note I
have purposely tracked the .config file until I'm done developing this). The
patch I'm attaching doesn't contain the .config of course.

## Changes

A summary of the files affected per directory.

### Arch

arch/arm/src/sam34/Make.defs
arch/arm/src/sam34/chip/sam_afec.h
arch/arm/src/sam34/sam_afec.c
arch/arm/src/sam34/sam_afec.h

The architecture specific code for the sam34. I think Atmel's driver supports
other architectures like SAMV and SAMS. I've only tested on SAM4E16E. If others
have other chips please give it a try.

There are some experimental functions (sam_afec_channel_*) which are not being
used by default and I would like some feedback on. From what I realised, an ADC
driver with multiple channels is supposed to use the channel feature of the ADC
interface. But can that allow for someone to open one of the channels as a
stream via a file descriptor? As an experiment I made a separate set of
functions for registering a channel as a file. I've pulled back from this idea
till I consider it some more. Any thoughts?


### Board

configs/sam4e-ek/Kconfig
configs/sam4e-ek/src/Makefile
configs/sam4e-ek/src/sam4e-ek.h
configs/sam4e-ek/src/sam_afec.c

The code required for the evaluation board I used for my development.


### Driver

drivers/analog/Kconfig
drivers/analog/Make.defs
drivers/analog/sam_afec.c

include/nuttx/analog/sam4e_afec.h
include/nuttx/analog/sam_afec.h

The generic SAM driver from Atmel's ASF. I've tried to avoid changing this as
much as I could. Any changes are limited to the top of any affected files so
that doing a diff against future releases by Atmel will be straightforward.


### Apps

There is a small patch for the ADC example, but I'd rather if there wasn't. If
anyone has ideas on this please share them. They are two IOCTL calls necessary
for configuring the driver.

[Sebastien Lorquet [e-mail] [nuttx]]

Hello

Why do we have to dump more and more atmel code in nuttx? This is not the first
time it happens, and every time, Greg is supposed to maintain this, but no one
on this list knows how the imported code is supposed to work.

At this pace why do we even bother to write drivers, we just have to randomly
throw every crap suppliers write and add wrappers when it does not fit? ST
publishes lots of STM32 HAL libs, why do we care to write drivers then?

External libs should be managed outside of the nuttx tree, such as uavcan and
others - Or am I wrong?

I am not a decider, but I am against such raw code imports.

The code style is very far from nuttx, pointers do not use FAR keywords, structs
are typedefed and their names do not end with _s, documentation uses doxygen
keywords, everything is so wrong here!

</rant>

Sebastien

Post by ***@orbitalfox.com [nuttx]
Hello all.
I've implemented an AFEC driver for SAM34 by wrapping Atmel's ASF code. The idea
is to have common driver for all SAM chips supporting it. The implementation
needs more work, but the basic ADC example works.
Been working on this driver intermittently for a while now, but I realised it's
best to push it upstream in case someone else is interested in this development
and to have it taken in consideration in API changes (like the recent
adc_receive one). Also I would like some feedback on the direction taken.
My repo for this is here: https://github.com/orbifx/nuttx/tree/sam-afec (Note I
have purposely tracked the .config file until I'm done developing this). The
patch I'm attaching doesn't contain the .config of course.
## Changes
A summary of the files affected per directory.
### Arch
arch/arm/src/sam34/Make.defs
arch/arm/src/sam34/chip/sam_afec.h
arch/arm/src/sam34/sam_afec.c
arch/arm/src/sam34/sam_afec.h
The architecture specific code for the sam34. I think Atmel's driver supports
other architectures like SAMV and SAMS. I've only tested on SAM4E16E. If others
have other chips please give it a try.
There are some experimental functions (sam_afec_channel_*) which are not being
used by default and I would like some feedback on. From what I realised, an ADC
driver with multiple channels is supposed to use the channel feature of the ADC
interface. But can that allow for someone to open one of the channels as a
stream via a file descriptor? As an experiment I made a separate set of
functions for registering a channel as a file. I've pulled back from this idea
till I consider it some more. Any thoughts?
### Board
configs/sam4e-ek/Kconfig
configs/sam4e-ek/src/Makefile
configs/sam4e-ek/src/sam4e-ek.h
configs/sam4e-ek/src/sam_afec.c
The code required for the evaluation board I used for my development.
### Driver
drivers/analog/Kconfig
drivers/analog/Make.defs
drivers/analog/sam_afec.c
include/nuttx/analog/sam4e_afec.h
include/nuttx/analog/sam_afec.h
The generic SAM driver from Atmel's ASF. I've tried to avoid changing this as
much as I could. Any changes are limited to the top of any affected files so
that doing a diff against future releases by Atmel will be straightforward.
### Apps
There is a small patch for the ADC example, but I'd rather if there wasn't. If
anyone has ideas on this please share them. They are two IOCTL calls necessary
for configuring the driver.

------------------------------------

------------------------------------


------------------------------------

Yahoo Groups Links

<*> To visit your group on the web, go to:
http://groups.yahoo.com/group/nuttx/

<*> Your email settings:
Individual Email | Traditional

<*> To change settings online go to:
http://groups.yahoo.com/group/nuttx/join
(Yahoo! ID required)

<*> To change settings via email:
nuttx-***@yahoogroups.com
nuttx-***@yahoogroups.com

<*> To unsubscribe from this group, send an email to:
nuttx-***@yahoogroups.com

<*> Your use of Yahoo Groups is subject to:
https://info.yahoo.com/legal/us/yahoo/utos/terms/

[[e-mail] [nuttx]]

Hello Sebastien.

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
Why do we have to dump more and more atmel code in nuttx? This is not the first
time it happens, and every time, Greg is supposed to maintain this, but no one
on this list knows how the imported code is supposed to work.

I never saw this as a "dump", I've actually put some time working on this to try
and do it "properly". It is precisely for reducing the maintenance cost that I
took this approach. I avoided duplication by adding a (hopefully) thin layer.

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
At this pace why do we even bother to write drivers, we just have to randomly
throw every crap suppliers write and add wrappers when it does not fit? ST
publishes lots of STM32 HAL libs, why do we care to write drivers then?

I don't know, maybe you have some beneficial reason? Nuttx doesn't necessarily
gain because something was written from scratch. The duplication of work tends
to result to lack of features and introduction of new bugs.

Also, it's a bit arbitrary to slant this as crap, just because.

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
The code style is very far from nuttx, pointers do not use FAR keywords, structs
are typedefed and their names do not end with _s, documentation uses doxygen
keywords, everything is so wrong here!

I agree it wasn't the files imported in `drivers/` don't fit in well, but I
thought that keeping them as close to the original will make transfer of changes
in the future easier.

If Greg prefers style over `diff` practicality then I can change the style of
those files. Would there be an issue with the licence?

[[e-mail] [nuttx]]

No, I do not accept third party code into NuttX, nor do I support any references to external third party code.

All code in the NuttX source must conform to the NuttX architecture and to the NuttX coding standard: In naming, indentation, file structure, etc. The Atmel code does not (neither does your code). If you wish to make code submissions, you code submissions you should look at the NuttX coding standard at http://www.nuttx.org/doku.php?id=documentation:codingstandard

You might also look at the SAMA5D ADC driver at arch/arm/src/sama5/sam_adc.c. The SAMA5D ADC is 80% compatible with the SAM4 AFEC.

Greg

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
No, I do not accept third party code into NuttX, nor do I support any references to external third party code.

Whilst this makes sense on its own, it doesn't make sense when recalling this:

http://thread.gmane.org/gmane.comp.embedded.nuttx/11423/focus=11427

and this code is only for the Atmel SAM chips.

Post by ***@yahoo.com [nuttx]
All code in the NuttX source must conform to the NuttX architecture and to the
NuttX coding standard: In naming, indentation, file structure, etc. The Atmel
code does not (neither does your code). If you wish to make code
submissions, you code submissions you should look at the NuttX coding standard
at http://www.nuttx.org/doku.php?id=documentation:codingstandard

Would further conformity by my code and "deriving" Atmel's be acceptible for
inclusing into the main code base?

Post by ***@yahoo.com [nuttx]
You might also look at the SAMA5D ADC driver at arch/arm/src/sama5/sam_adc.c. The SAMA5D ADC is 80% compatible with the SAM4 AFEC.

I have and I will continue to draw from that.

[[e-mail] [nuttx]]

Post by ***@orbitalfox.com [nuttx]

Post by ***@yahoo.com [nuttx]
No, I do not accept third party code into NuttX, nor do I support any references to external third party code.

http://thread.gmane.org/gmane.comp.embedded.nuttx/11423/focus=11427
and this code is only for the Atmel SAM chips.

You are right, I will accept third party code if it conforms 100% with the NuttX coding standard and has a BSD license. That reference is about deriving code from third party source. But I can't imagine any situation where you could just dump some foreign code into the OS without making it 100% compatible. That will probably never happen.

Post by ***@orbitalfox.com [nuttx]

Post by ***@yahoo.com [nuttx]
All code in the NuttX source must conform to the NuttX architecture and to the
NuttX coding standard: In naming, indentation, file structure, etc. The Atmel
code does not (neither does your code). If you wish to make code
submissions, you code submissions you should look at the NuttX coding standard
at http://www.nuttx.org/doku.php?id=documentation:codingstandard

Would further conformity by my code and "deriving" Atmel's be acceptible for
inclusing into the main code base?

Of course, several of the Atmel files derives from the ASF in some form. But all are 100% compitible with the coding standard and architecture. No Atmel specific interfaces or IOCTL commands.

Greg

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
You are right, I will accept third party code if it conforms 100% with the
NuttX coding standard and has a BSD license. That reference is about deriving
code from third party source. But I can't imagine any situation where you
could just dump some foreign code into the OS without making it 100%
compatible. That will probably never happen.

Ok. I'm bound on how much project time I can put towards this, but I'd like to
contribute back to the project. So I'd appreciate if we were specific on what
needs to happen to this patch for it to be acceptible.

The code I have written is given as BSD. The Atmel code is given as essentially
BSD when provided for Atmel's devices. Correct?

On the aspect of styling, I'd have to go over my code and restyle whatever I
have missed (I'll use the script) and also restyle Atmel's code. Is that
permissable by their licence? It won't be easy to run diff checks for future
versions, but I'm happy to do it.

Post by ***@yahoo.com [nuttx]
Of course, several of the Atmel files derives from the ASF in some form. But
all are 100% compitible with the coding standard and architecture. No Atmel
specific interfaces or IOCTL commands.

What do you mean by Atmel interfaces? It's all wrapped and intended to be used
by NuttX's interfaces.

What about IOCTL commands not existing in NuttX?

[[e-mail] [nuttx]]

If when you are all finished (1) the code follows the NuttX coding standard, (2) the code looks basically like every other NuttX ADC, and (2) no special interfaces or IOCTL calls are used, then it can be accepted. I will not accept less.

[SP [e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
If when you are all finished (1) the code follows the NuttX coding
standard, (2) the code looks basically like every other NuttX ADC, and (2)
no special interfaces or IOCTL calls are used, then it can be accepted. I
will not accept less.

Releasing this independently is probably the best way for this situation.
Not everything should have to be part of the main repository. Is there is a
convention for apps, libraries and drivers to be included into the file image
without copying the files manually into the build directories?

If anyone plans to or is using this driver and has some technical remarks,
like bugs, issues, ideas on improving this wrapper please let me know.

The current branch is here:
https://github.com/orbifx/nuttx-apps/tree/sam-afec

When I find the time I will change it to only contain the source
distribution of this driver with some kind of `make target` for
installing it.