Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial import of low-level timer driver interface #614

Merged
merged 1 commit into from
Apr 10, 2014

Conversation

haukepetersen
Copy link
Contributor

Proposal for a low-level timer driver.

This driver gives a hardware independent access to hardware timers. This driver is mainly intended to be used by the hwtimer, as the hwtimer implementation is quite trivial using this driver.

*
* This file is subject to the terms and conditions of the LGPLv2 License.
* See the file LICENSE in the top level directory for more details.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need Copyright headers in h files.

@OlegHahm OlegHahm assigned OlegHahm and StefanPfeiffer and unassigned OlegHahm Feb 5, 2014
@OlegHahm OlegHahm added this to the Release NEXT MAJOR milestone Feb 5, 2014
@OlegHahm
Copy link
Member

OlegHahm commented Feb 5, 2014

Apart from the comments: ack.

@haukepetersen
Copy link
Contributor Author

I intentionally didn't name it timer_now(), as I wanted to keep a clear separation between the (hardware) base driver and the v- and hwtimers. But I guess its just my personal opinion...

@OlegHahm
Copy link
Member

OlegHahm commented Feb 5, 2014

I'm also fine with that. Just a thought.

@thomaseichinger
Copy link
Member

ACK from my side. Deployed on the iot-lab_M3 for openwsn integration and it works 😉 👍

* @param timeout timeout in ticks after that the registered callback is executed
* @return 1 on success, -1 on error
*/
int timer_set(tim_t dev, int channel, int timeout);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int might not be a good choice for timeout as it can not take negative values and the maximum value is not so large, especially on 16bit platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remember, we are talking about a low-level interface, the given timeout will be written into a capture/compare register -> and these are normally not larger then 32 or 16 bit, so int should be sufficient for 99% of the platforms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the un/signed though?

@haukepetersen
Copy link
Contributor Author

small adjustments:

  • rebased
  • changed timeouts to unsigned int
  • adjusted meaning of return value of init()

@thomaseichinger could you maybe test this also with your implementation?

@thomaseichinger
Copy link
Member

works for me, ACK holds

@OlegHahm
Copy link
Member

OlegHahm commented Apr 1, 2014

For clarification: This should be the interface for the arch dependent hwtimer implementation?

@miri64
Copy link
Member

miri64 commented Apr 1, 2014

Think about direction tags in @param documentation

@haukepetersen
Copy link
Contributor Author

done

*/
typedef enum {
#if TIMER_0_EN
TIMER_0 = 0, /*< 1st timer */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think doxygen needs /**< 1st timer */ here.

@haukepetersen
Copy link
Contributor Author

@thomaseichinger fixed.

@OlegHahm
Copy link
Member

OlegHahm commented Apr 8, 2014

Could need some squashing, otherwise ready to be merged, I guess?

@haukepetersen
Copy link
Contributor Author

almost ready, one final ACK missing

@miri64
Copy link
Member

miri64 commented Apr 9, 2014

ACK when squashed.

@haukepetersen
Copy link
Contributor Author

rebased, squashed and go

haukepetersen added a commit that referenced this pull request Apr 10, 2014
Initial import of low-level timer driver interface
@haukepetersen haukepetersen merged commit f17efea into RIOT-OS:master Apr 10, 2014
@haukepetersen haukepetersen deleted the periph_driver_timer branch April 10, 2014 12:56
Fixed spelling

drivers: adjustments to low-level timer driver IF

drivers: added [in|out] to @param documentation

drivers: fixed doxygen for tim_t typedef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants