Skip to content

Conversation

@Sizurka
Copy link
Contributor

@Sizurka Sizurka commented Apr 2, 2019

This adds support for the basic timer counter (TC) found on SAM0 series parts. This driver only supports running the counter in 32 bit wide mode. Since this mode explicitly slaves the odd counters to the even ones, only instances of the even ones are defined.

Tested with tests/drivers/counter/counter_basic_api on SAMD21.

Compile tested on SAMR21 and SAMD20. but requires #15104 (and a bit of rebasing if that's merged)

This will also require a trivial rebase if #14794 is merged (the parameter is already there, just currently set to a default).

@benpicco: I compile tested this against SAME54, but you'd need to generate a tc_fixup_samd5x.h in the spirit of the sercom one. I just used the static definition:

#define MCLK_TC0 (&MCLK->APBAMASK.reg)
#define MCLK_TC0_MASK (MCLK_APBAMASK_TC0)

@benpicco
Copy link
Contributor

benpicco commented Apr 2, 2019

Nice work!
On SAME54 the odd timer has to be enabled separately. Luckily both even and odd timers have to the same GCLK_ID (e.g. TC0_GCLK_ID == TC1_GCLK_ID) and they can be assumed to belong to the same APB group.

So all I had to do was to change .mclk_mask = MCLK_TC##n##_MASK to .mclk_pos = MCLK_TC##n##_Pos and add *cfg->mclk |= 1 << (cfg->mclk_pos + 1)

Thinking about it there is nothing that requires both timers to occupy consecutive bits in MCLK.
So no change to your code is needed, I just had to fixup my tc_fixup_samd5x.h - this should be a more robust solution.

With this, tests/drivers/counter/counter_basic_api runs successful on SAME54.

@zephyrbot
Copy link

zephyrbot commented Apr 11, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@Sizurka Sizurka force-pushed the sam0-tc branch 2 times, most recently from 4ebb5a3 to 136af90 Compare April 18, 2019 23:38
@galak galak added platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) area: Counter labels Apr 20, 2019
@Sizurka
Copy link
Contributor Author

Sizurka commented Apr 29, 2019

Note that the Kconfig error is due to token pasting (CONFIG_COUNTER_SAM0_TC32_##n##_DIVISOR) so it's spurious. Should I open a PR on the CI tools to add this symbol, even fore this is merged?

@Sizurka Sizurka requested a review from benpicco as a code owner May 8, 2019 12:47
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

tested both on samr21-xpro and same54-xpro

@benpicco benpicco requested a review from nordic-krch May 8, 2019 13:11
benpicco added a commit to benpicco/zephyr that referenced this pull request May 19, 2019
This makes use of the counter driver from zephyrproject-rtos#15105

tc_fixup_samd5x.h was generated by

for i in {0..6..2}; do
        for n in {A..D}; do
                echo "#ifdef MCLK_APB${n}MASK_TC${i}"
                echo "#define MCLK_TC${i} (&MCLK->APB${n}MASK.reg)"
                echo "#define MCLK_TC${i}_MASK "		\
		     "((1 << MCLK_APB${n}MASK_TC${i}_Pos)"	\
		     "| (1 << MCLK_APB${n}MASK_TC$(($i+1))_Pos))"
                echo "#endif"
        done
        echo ""
done

Signed-off-by: Benjamin Valentin <[email protected]>
benpicco added a commit to benpicco/zephyr that referenced this pull request May 23, 2019
This makes use of the counter driver from zephyrproject-rtos#15105

tc_fixup_samd5x.h was generated by

for i in {0..6..2}; do
        for n in {A..D}; do
                echo "#ifdef MCLK_APB${n}MASK_TC${i}"
                echo "#define MCLK_TC${i} (&MCLK->APB${n}MASK.reg)"
                echo "#define MCLK_TC${i}_MASK "		\
		     "((1 << MCLK_APB${n}MASK_TC${i}_Pos)"	\
		     "| (1 << MCLK_APB${n}MASK_TC$(($i+1))_Pos))"
                echo "#endif"
        done
        echo ""
done

Signed-off-by: Benjamin Valentin <[email protected]>
This adds support for the basic timer counter (TC) found on SAM0
series parts.  This driver only supports running the counter
in 32 bit wide mode.  Since this mode explicitly slaves the odd
counters to the even ones, only instances of the even ones are
defined.

Tested with tests/drivers/counter/counter_basic_api on SAMD21.

Signed-off-by: Derek Hageman <[email protected]>
@benpicco
Copy link
Contributor

I think the prescaler selection should go into the device tree - but I suppose you will touch up on this anyway once #15585 is merged.

@Sizurka
Copy link
Contributor Author

Sizurka commented May 27, 2019

Yeah, I'd do a clock-frequency property in the device tree for that. My original logic was that the frequency of the counter would be better as application controllable (vs defined by hardware), which makes it fit Kconfig better.

However, once the SoC clock tree is not an immutable thing, the prescaler should be configured the same way: meaning device tree specification and calculation derived from the requested frequency.

Copy link
Contributor

@nordic-krch nordic-krch left a comment

Choose a reason for hiding this comment

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

looks ok

@nashif nashif merged commit a4f3e62 into zephyrproject-rtos:master May 28, 2019
@Sizurka Sizurka deleted the sam0-tc branch May 28, 2019 15:23
benpicco added a commit to benpicco/zephyr that referenced this pull request Oct 14, 2019
This makes use of the counter driver from zephyrproject-rtos#15105

tc_fixup_samd5x.h was generated by

for i in {0..6..2}; do
        for n in {A..D}; do
                echo "#ifdef MCLK_APB${n}MASK_TC${i}"
                echo "#define MCLK_TC${i} (&MCLK->APB${n}MASK.reg)"
                echo "#define MCLK_TC${i}_MASK "		\
		     "((1 << MCLK_APB${n}MASK_TC${i}_Pos)"	\
		     "| (1 << MCLK_APB${n}MASK_TC$(($i+1))_Pos))"
                echo "#endif"
        done
        echo ""
done

Signed-off-by: Benjamin Valentin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Counter platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants