Skip to content

Conversation

@benys
Copy link
Contributor

@benys benys commented Sep 20, 2022

Driver was based on can_sam.

Signed-off-by: Kamil Serwus [email protected]

Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

We will need a way of testing this - at least the compilation part. Can this be enabled on a board?

@benys
Copy link
Contributor Author

benys commented Sep 20, 2022

We will need a way of testing this - at least the compilation part. Can this be enabled on a board?

This CAN works only with SAMCAN21x. I have in my solution connected to board intruduced at #50384 PR.
In my opionion I should add connection to board/soc after merged #50384? Becouse only C21 implements this SAM0_CAN

BTW In my repo it fully works. You can check my old commit: benys@55559a8

@henrikbrixandersen
Copy link
Member

Okay. I suggest postponing this to after #50384 is merged, then. That was the initial feedback given in that PR as well.

@alexanderwachter
Copy link
Member

Please provide a meaningful commit message

@str4t0m str4t0m marked this pull request as draft September 20, 2022 17:18
@str4t0m
Copy link
Contributor

str4t0m commented Sep 20, 2022

Converted to a draft for now, as board files still need to be added after #50384 was merged.

@benys
Copy link
Contributor Author

benys commented Sep 26, 2022

@henrikbrixandersen i have a problem, that CAN can send messages (i receive messages by CAN usb stick), but when I send by CAN usb stick - i don't receive interrupt.
I set this filter:

    const struct can_filter change_led_filter = {
		.id_type = CAN_EXTENDED_IDENTIFIER,
		.rtr = CAN_DATAFRAME,
		.rtr_mask = 1,
		.id_mask = 0
	};

    ret = can_add_rx_filter_msgq(can_dev, &can_msgq, &change_led_filter);
	if (ret == -ENOSPC) {
		printk("Error, no filter available!\n");
		return;
	}

Do you have idea what I have to check?

@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented Sep 26, 2022

Do you have idea what I have to check?

First off, do the CAN API tests pass in loopback mode? Once the CAN API tests pass in loopback mode - and if the problem persists:

  • Have you tried using the CAN shell for testing?
  • Are the pinctrl settings correct?
  • Is the CAN interrupt enabled correctly?
  • Is the CAN interrupt triggered at all?
  • Have you tried using a debugger to examine what happens?

@benys
Copy link
Contributor Author

benys commented Sep 26, 2022

@henrikbrixandersen

  • Have you tried using the CAN shell for testing?

Not yet, but I run sam counter sample. And I extend it a litte bit. On loopback mode it fully works. With external CAN commands it not handle incoming CAN frames.

  • Are the pinctrl settings correct?

I tripled checked. I honestly I port my application which originaly used plain ASF4. So I checked pinctrls by comparison one-to-one. TXpin works becouse it send data. SO i believe that in in zephyr RXpin also is corecrly connected.

  • Is the CAN interrupt enabled correctly?

Yes, but I have idea that problem is that zephyr mcan driver uses two interrupts per device, but SAMC has only one line. I tried investigate, that maybe mcan driver set to use second interrupts for incoming data from external world.

  • Is the CAN interrupt triggered at all?

Yes for example:

LED ON
[00:00:05.004,000] <dbg> can_mcan: can_mcan_send: Sending 2 bytes. Id: 0x12345, ID type: extended   
[00:00:05.004,000] <dbg> can_mcan: can_mcan_send: Waiting for TX complete
[00:00:05.005,000] <dbg> can_mcan: can_mcan_line_1_isr: RX FIFO0 INT
[00:00:05.005,000] <dbg> can_mcan: can_mcan_get_message: Frame on filter 28, ID: 0x12345
Counter received: 2
[00:00:06.005,000] <dbg> can_mcan: can_mcan_send: Sending 1 bytes. Id: 0x10, ID type: standard   
[00:00:06.006,000] <dbg> can_mcan: can_mcan_line_1_isr: RX FIFO0 INT
[00:00:06.006,000] <dbg> can_mcan: can_mcan_get_message: Frame on filter 0, ID: 0x10
  • Have you tried using a debugger to examine what happens?

Yes :-) without debugger it is inposible to investigate :)

I will try check with second interupt for bosch IP. If you have some ideas please give me :)

@alexanderwachter
Copy link
Member

Looks like the loopback mode works.
That means it is likely that it is either pinctrl, or a problem wit the bus (baudrate, termination, issue with the transceiver, ...)

@benys
Copy link
Contributor Author

benys commented Sep 26, 2022

@henrikbrixandersen and @alexanderwachter i think that i found I receive some of messages.
I forget that "if you work with can you must found mcan datasheet" :)

#ifdef CONFIG_CAN_STM32FD
	can->ils = CAN_MCAN_ILS_RXFIFO0 | CAN_MCAN_ILS_RXFIFO1;
#else
	can->ils = 0;//CAN_MCAN_ILS_RF0N | CAN_MCAN_ILS_RF1N;
#endif
	can->ile = CAN_MCAN_ILE_EINT0; //| CAN_MCAN_ILE_EINT1;

in mcan driver was implemented that some interrupts was handled by second line.
So I have a question - you recomend add elif CONFIG_CAN_SAM0 ? with fixed declaration for SAMC?

BTW i need now investiage why have many bus_off state.

@henrikbrixandersen
Copy link
Member

So I have a question - you recomend add elif CONFIG_CAN_SAM0 ?

You can add that for now. I am working on getting rid of all platform specific code in the M_CAN backend driver, though.

@benys
Copy link
Contributor Author

benys commented Sep 27, 2022

@henrikbrixandersen @alexanderwachter i had last problem (i hope). NBTP computation.
I discovered that zephry compute this registry as:

generation from zephyr

  • ntseg2=3
  • ntseg1=26
  • nbrp=0

so

  • tq = 1
  • ntseq1+ntseg2+3=32
  • product = 32*1=32

my original sample

  • ntseg2=0
  • ntseg1=1
  • nbrp=7

so

  • tq = 8
  • ntseq1+ntseg2+3=4
  • product =4*8=32

But first sample - produces a lot of receive errors (counter is not zero) and my second samples works very well.
Did you have same problems in the past? How can I handle this problem?

@alexanderwachter
Copy link
Member

Well, you can set the sample point a bit earlier to meet your old values. However, the sample points used by Zephyr are recommended by the CiA

@fabiobaltieri
Copy link
Member

@benys can you rebase please?

@benys benys force-pushed the samc21can branch 2 times, most recently from bdf0893 to be71b32 Compare May 11, 2023 09:51
@benys
Copy link
Contributor Author

benys commented May 11, 2023

@benys can you rebase please?

I rebased, but I will it test +- tommorow at the office :) I will inform you.

@benys benys force-pushed the samc21can branch 3 times, most recently from 3f0a940 to 782c99f Compare May 11, 2023 10:42
@silverv
Copy link

silverv commented May 11, 2023

I tested the code before rebase and after rebase on my very reputable hardware (https://ww1.microchip.com/downloads/en/DeviceDoc/70005318A.pdf) and I have some observations.
The rebased code fails (didn't fail before rebase) if you do this on an astamc21n18a, running the samples/drivers/can/counter example code:

  1. Set loopback mode on the example off
  2. Set bitrate to match the can adapter
  3. Connect CAN to an adapter
  4. Observe how the can driver fails to start at line 215: printf("CAN: Device %s not ready.\n", can_dev->name);
  5. With logging enabled CONFIG_LOG=y, the mcan driver will report an init failure.
    Both CAN0 and CAN1 work before rebase on the previous 81afde13448808f71b12c4bacc18d9ef4f6649ec commit, I still have it locally and I am using it for another custom board.

@benys
Copy link
Contributor Author

benys commented May 14, 2023

@silverv @henrikbrixandersen i little fixed my PR, but after rebase still not works :-(
I am still looking reason...

EDIT:
I discovered that it brokes at commit "drivers: can: mcan: move message RAM configuration to front-end drivers"

EDIT2 @henrikbrixandersen
I discovered that for SAM0 can_mcan_configure_message_ram need be executed "can_mcan_enter_init_mode" before or first at can_mcan_configure_message_ram like:
image

I don't know that I can include it at my change? Or remove static attribute can_mcan_enter_init_mode to execute it form sam0 driver level

@benys benys force-pushed the samc21can branch 2 times, most recently from fa21a9c to 47b072e Compare May 14, 2023 19:28
@benys benys requested a review from henrikbrixandersen May 15, 2023 18:18
benys added 3 commits May 18, 2023 12:19
Driver was based on can_sam. SAMC21 has only 1 interrupt for one
can "output", so can interrupt has to executes two lines of
interrupts.
CAN is configured to use OSC48M clock via GLCK7. GLCK7 is set
by divider configured from dts.

Signed-off-by: Kamil Serwus <[email protected]>
Enable CAN driver sam0 in SAMC21 socs. CAN module exists only in
C21 socs.

Signed-off-by: Kamil Serwus <[email protected]>
Configure pinmux pins at samc21 xpro board. Boards has
hard-configured pins for CAN0 and CAN1 module.

Signed-off-by: Kamil Serwus <[email protected]>
SAM0 required can module was in init state before configure
pointers to ram which handle can frames.

Signed-off-by: Kamil Serwus <[email protected]>
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Thanks!

@fabiobaltieri fabiobaltieri merged commit b63a9af into zephyrproject-rtos:main May 22, 2023
@benys benys deleted the samc21can branch May 22, 2023 15:56
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 platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)

Projects

None yet

Development

Successfully merging this pull request may close these issues.