Skip to content

Conversation

@henrikbrixandersen
Copy link
Member

Use the UPLLCK clock for the CAN controller as recommended by the Atmel SAM E70 data sheet.

Fixes: #45012

Signed-off-by: Henrik Brix Andersen [email protected]

@henrikbrixandersen henrikbrixandersen added bug The issue is a bug, or the PR is fixing a bug platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) area: CAN labels Apr 28, 2022
@henrikbrixandersen henrikbrixandersen self-assigned this Apr 28, 2022
@henrikbrixandersen
Copy link
Member Author

The can_sam_get_core_clock() function likely needs updating as well to reflect the change in CAN core clock frequency, but I do not have the hardware to test with. @nandojve Any ideas?

@nandojve
Copy link
Member

Hi @henrikbrixandersen ,
Let me check

@nandojve
Copy link
Member

nandojve commented May 1, 2022

Hi @henrikbrixandersen , Let me check

@henrikbrixandersen ,

May you check if below enables UPLLCK at 480 MHz:

diff --git a/soc/arm/atmel_sam/same70/soc.c b/soc/arm/atmel_sam/same70/soc.c
index 77a7fe0cf3..bd0e77c9ae 100644
--- a/soc/arm/atmel_sam/same70/soc.c
+++ b/soc/arm/atmel_sam/same70/soc.c
@@ -177,6 +177,15 @@ static ALWAYS_INLINE void clock_init(void)
                ;
        }
 
+       /* Setup UPLL */
+       PMC->CKGR_UCKR = CKGR_UCKR_UPLLCOUNT(0x3Fu);
+                      | CKGR_UCKR_UPLLEN;
+
+       /* Wait for PLL lock */
+       while (!(PMC->PMC_SR & PMC_SR_LOCKU)) {
+               ;
+       }
+
        /*
         * Final setup of the Master Clock
         */

@henrikbrixandersen
Copy link
Member Author

May you check if below enables UPLLCK at 480 MHz:

Thanks! I do not have access to the hardware, but perhaps @shaoanxli can check?

@shaoanxli
Copy link
Contributor

shaoanxli commented May 3, 2022

May you check if below enables UPLLCK at 480 MHz:

Thanks! I do not have access to the hardware, but perhaps @shaoanxli can check?

Is there any good way to get UPLLCK? It seems no register to get the UPLLCK.

From the SAM E70 data sheet, page 277:
image
And the UPLLCK clock frequency seems to be fixed.

@nandojve
Copy link
Member

nandojve commented May 3, 2022

Is there any good way to get UPLLCK?

The below is supposed to work.

diff --git a/soc/arm/atmel_sam/same70/soc.c b/soc/arm/atmel_sam/same70/soc.c
index 77a7fe0cf3..bd0e77c9ae 100644
--- a/soc/arm/atmel_sam/same70/soc.c
+++ b/soc/arm/atmel_sam/same70/soc.c
@@ -177,6 +177,15 @@ static ALWAYS_INLINE void clock_init(void)
                ;
        }
 
+       /* Setup UPLL */
+       PMC->CKGR_UCKR = CKGR_UCKR_UPLLCOUNT(0x3Fu);
+                      | CKGR_UCKR_UPLLEN;
+
+       /* Wait for PLL lock */
+       while (!(PMC->PMC_SR & PMC_SR_LOCKU)) {
+               ;
+       }
+
        /*
         * Final setup of the Master Clock
         */

@shaoanxli
Copy link
Contributor

The UPLLCK seems to setup successfully , but the test failed , logs and console output shows:

DEBUG   - DEVICE: *** Booting Zephyr OS build zephyr-v3.0.0-3206-gf7171e20b33e  ***
DEBUG   - DEVICE: testing on device CAN_0 @ 150000000 Hz
DEBUG   - DEVICE: Running test suite can_timing_tests
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_set_timing_min
DEBUG   - DEVICE: PASS - test_set_timing_min in 0.1 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_set_timing_max
DEBUG   - DEVICE: PASS - test_set_timing_max in 0.1 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_timing
DEBUG   - DEVICE: testing bitrate 20000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 130, phase_seg2 = 19, prescaler = 50 OK, sample point error 0.2%
DEBUG   - DEVICE: testing bitrate 50000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 174, phase_seg2 = 25, prescaler = 15 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 125000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 209, phase_seg2 = 30, prescaler = 5 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 250000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 174, phase_seg2 = 25, prescaler = 3 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 130, phase_seg2 = 19, prescaler = 2 OK, sample point error 0.2%
DEBUG   - DEVICE: testing bitrate 800000, sample point 80.0% (valid):
DEBUG   - DEVICE: Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/can/timing/src/main.c:194: test_timing_values: (sp_err >= 0 is false)
DEBUG   - DEVICE: unknown error -22
DEBUG   - DEVICE: FAIL - test_timing in 0.80 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_timing_data
DEBUG   - DEVICE: testing bitrate 500000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 25, phase_seg2 = 4, prescaler = 10 OK, sample point error 0.9%
DEBUG   - DEVICE: testing bitrate 1000000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 25, phase_seg2 = 4, prescaler = 5 OK, sample point error 0.9%
DEBUG   - DEVICE: testing bitrate 500000, sample point 90.0% (valid): prop_seg = 0, phase_seg1 = 26, phase_seg2 = 3, prescaler = 10 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 23, phase_seg2 = 6, prescaler = 10 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 100.0% (invalid): OK
DEBUG   - DEVICE: testing bitrate 8000001, sample point 75.0% (invalid): OK
DEBUG   - DEVICE: PASS - test_timing_data in 0.60 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: Test suite can_timing_tests failed.
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: RunID: 52876579eaf0bab6807fb28529187fe9
DEBUG   - DEVICE: PROJECT EXECUTION FAILED

@nandojve
Copy link
Member

nandojve commented May 5, 2022

The UPLLCK seems to setup successfully , but the test failed

Hi @shaoanxli,

Let me check!

@shaoanxli
Copy link
Contributor

shaoanxli commented May 5, 2022

The UPLLCK seems to setup successfully , but the test failed

Hi @shaoanxli,

Let me check!

This error seems to occur here(drivers/can/can_common.c) , when bitrate=800000 :

if (core_clock % (prescaler * bitrate)) {
	/* No integer ts */
	continue;
}

@henrikbrixandersen
Copy link
Member Author

The UPLLCK seems to setup successfully , but the test failed , logs and console output shows:

DEBUG   - DEVICE: *** Booting Zephyr OS build zephyr-v3.0.0-3206-gf7171e20b33e  ***
DEBUG   - DEVICE: testing on device CAN_0 @ 150000000 Hz

The can_sam_get_core_clock() function needs to be updated to report the correct clock frequency and you need to use one of the clock dividers recommended by the data sheet (see the linked issue).

Without those changes there are no changes to the CAN timing calculations.

@github-actions github-actions bot added area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding labels May 5, 2022
@henrikbrixandersen
Copy link
Member Author

@shaoanxli, @nandojve I have tried to incorporate the needed changes as good as I can from studying the datasheet and the suggested code from @nandojve. Please test - both the CAN timing test cases and the actual CAN bitrate as presented on the CAN bus.

@alexanderwachter You wrote the original driver - can you assist in testing?

@nandojve
Copy link
Member

nandojve commented May 7, 2022

@shaoanxli, @nandojve I have tried to incorporate the needed changes as good as I can from studying the datasheet and the suggested code from @nandojve. Please test - both the CAN timing test cases and the actual CAN bitrate as presented on the CAN bus.

@alexanderwachter You wrote the original driver - can you assist in testing?

Hi folks,

There is nothing wrong with this patch. You just need select correct divisor at devicetree. With 24 CAN clock will be 20MHz. Please, change it to 6 then clock will be 80MHz. See below result at SAMV71_XULT:

*** Booting Zephyr OS build zephyr-v3.0.0-3510-g9a8162aca685  ***
testing on device CAN_1 @ 80000000 Hz
Running test suite can_timing_tests
===================================================================
START - test_set_timing_min
 PASS - test_set_timing_min in 0.1 seconds
===================================================================
START - test_set_timing_max
 PASS - test_set_timing_max in 0.1 seconds
===================================================================
START - test_timing
testing bitrate 20000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 174, phase_seg2 = 25, prescaler = 20 OK, sample point error 0.0%
testing bitrate 50000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 174, phase_seg2 = 25, prescaler = 8 OK, sample point error 0.0%
testing bitrate 125000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 139, phase_seg2 = 20, prescaler = 4 OK, sample point error 0.0%
testing bitrate 250000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 139, phase_seg2 = 20, prescaler = 2 OK, sample point error 0.0%
testing bitrate 500000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 139, phase_seg2 = 20, prescaler = 1 OK, sample point error 0.0%
testing bitrate 800000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 79, phase_seg2 = 20, prescaler = 1 OK, sample point error 0.0%
testing bitrate 1000000, sample point 75.0% (valid): prop_seg = 0, phase_seg1 = 59, phase_seg2 = 20, prescaler = 1 OK, sample point error 0.0%
testing bitrate 125000, sample point 90.0% (valid): prop_seg = 0, phase_seg1 = 143, phase_seg2 = 16, prescaler = 4 OK, sample point error 0.0%
testing bitrate 125000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 255, phase_seg2 = 64, prescaler = 2 OK, sample point error 0.0%
testing bitrate 125000, sample point 100.0% (invalid): OK
testing bitrate 8000001, sample point 75.0% (invalid): OK
 PASS - test_timing in 0.123 seconds
===================================================================
START - test_timing_data
testing bitrate 500000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 27, phase_seg2 = 4, prescaler = 5 OK, sample point error 0.0%
testing bitrate 1000000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 13, phase_seg2 = 2, prescaler = 5 OK, sample point error 0.0%
testing bitrate 500000, sample point 90.0% (valid): prop_seg = 0, phase_seg1 = 17, phase_seg2 = 2, prescaler = 8 OK, sample point error 0.0%
testing bitrate 500000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 31, phase_seg2 = 8, prescaler = 4 OK, sample point error 0.0%
testing bitrate 500000, sample point 100.0% (invalid): OK
testing bitrate 8000001, sample point 75.0% (invalid): OK
 PASS - test_timing_data in 0.60 seconds
===================================================================
Test suite can_timing_tests succeeded
===================================================================
PROJECT EXECUTION SUCCESSFUL

BTW, don't forget to add soc: arm: atmel: same70: enable the UPLL clock to samv71.

@henrikbrixandersen
Copy link
Member Author

@nandojve Thank you so much for your help on co-authoring and verifying this fix :) I have marked it ready for review.

@nandojve
Copy link
Member

nandojve commented May 7, 2022

@nandojve Thank you so much for your help on co-authoring and verifying this fix :) I have marked it ready for review.

Hi @henrikbrixandersen , you are welcome!
BTW, it should be [email protected] in this case. Thank you!

Enable the UTMI PLL (UPLL) clock and add a static definition of its clock
frequency.

Signed-off-by: Henrik Brix Andersen <[email protected]>
Co-authored-by: Gerson Fernando Budke <[email protected]>
henrikbrixandersen and others added 3 commits May 7, 2022 20:11
Enable the UTMI PLL (UPLL) clock and add a static definition of its clock
frequency.

Signed-off-by: Henrik Brix Andersen <[email protected]>
Co-authored-by: Gerson Fernando Budke <[email protected]>
Use the UPLLCK clock for the CAN controller as recommended by the Atmel SAM
E70 data sheet.

Move the configuration of the clock prescaler from Kconfig to devicetree
and limit it to the values recommended by the SAM E70 datasheet.

Fixes: zephyrproject-rtos#45012

Signed-off-by: Henrik Brix Andersen <[email protected]>
Remove unnecessary #address-cells/#size-cells from the CAN devicetree
nodes.

Signed-off-by: Henrik Brix Andersen <[email protected]>
@henrikbrixandersen
Copy link
Member Author

Hi @henrikbrixandersen , you are welcome! BTW, it should be [email protected] in this case. Thank you!

Fixed.

Copy link
Member

@alexanderwachter alexanderwachter left a comment

Choose a reason for hiding this comment

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

Cannot get my hands on the HW at the moment. Did somebody test?

Edit: never mind. Test results were hidden in the APP 🤦‍♂️

@shaoanxli
Copy link
Contributor

Cannot get my hands on the HW at the moment. Did somebody test?

Edit: never mind. Test results were hidden in the APP 🤦‍♂️

I tried the test on the HW . The test pass.

DEBUG   - DEVICE: *** Booting Zephyr OS build zephyr-v3.0.0-3507-g6388778fb17d  ***
DEBUG   - DEVICE: testing on device CAN_0 @ 80000000 Hz
DEBUG   - DEVICE: Running test suite can_timing_tests
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_set_timing_min
DEBUG   - DEVICE: PASS - test_set_timing_min in 0.1 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_set_timing_max
DEBUG   - DEVICE: PASS - test_set_timing_max in 0.1 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_timing
DEBUG   - DEVICE: testing bitrate 20000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 174, phase_seg2 = 25, prescaler = 20 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 50000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 174, phase_seg2 = 25, prescaler = 8 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 125000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 139, phase_seg2 = 20, prescaler = 4 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 250000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 139, phase_seg2 = 20, prescaler = 2 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 139, phase_seg2 = 20, prescaler = 1 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 800000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 79, phase_seg2 = 20, prescaler = 1 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 1000000, sample point 75.0% (valid): prop_seg = 0, phase_seg1 = 59, phase_seg2 = 20, prescaler = 1 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 125000, sample point 90.0% (valid): prop_seg = 0, phase_seg1 = 143, phase_seg2 = 16, prescaler = 4 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 125000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 255, phase_seg2 = 64, prescaler = 2 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 125000, sample point 100.0% (invalid): OK
DEBUG   - DEVICE: testing bitrate 8000001, sample point 75.0% (invalid): OK
DEBUG   - DEVICE: PASS - test_timing in 0.123 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: START - test_timing_data
DEBUG   - DEVICE: testing bitrate 500000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 27, phase_seg2 = 4, prescaler = 5 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 1000000, sample point 87.5% (valid): prop_seg = 0, phase_seg1 = 13, phase_seg2 = 2, prescaler = 5 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 90.0% (valid): prop_seg = 0, phase_seg1 = 17, phase_seg2 = 2, prescaler = 8 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 80.0% (valid): prop_seg = 0, phase_seg1 = 31, phase_seg2 = 8, prescaler = 4 OK, sample point error 0.0%
DEBUG   - DEVICE: testing bitrate 500000, sample point 100.0% (invalid): OK
DEBUG   - DEVICE: testing bitrate 8000001, sample point 75.0% (invalid): OK
DEBUG   - DEVICE: PASS - test_timing_data in 0.60 seconds
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: Test suite can_timing_tests succeeded
DEBUG   - DEVICE: ===================================================================
DEBUG   - DEVICE: RunID: 63937deb897c652468f6c0e64355cba8
DEBUG   - DEVICE: PROJECT EXECUTION SUCCESSFUL

@carlescufi carlescufi merged commit 302e643 into zephyrproject-rtos:main May 9, 2022
@henrikbrixandersen henrikbrixandersen deleted the can_sam_core_clock branch May 9, 2022 15:12
@nandojve nandojve added this to the v3.1.0 milestone May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: CAN area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree bug The issue is a bug, or the PR is fixing a bug platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sam_e70b_xplained: failed to run test case tests/drivers/can/timing/drivers.can.timing

5 participants