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

dts: stm32f3: Remap USB IRQ to avoid conflict with CAN #22373

Merged

Conversation

erwango
Copy link
Member

@erwango erwango commented Jan 31, 2020

On stm32f302/3 series, USB and CAN_1 share same IRQ lines.
To use USB and CAN_1 together, USB IRQ could be remap to other
line numbers, on which there is no conflict.
Remap the USB IRQ lines by default:
-Assign remap number in matching dtsi files
-Perform remap in usb driver init

Additionally, fix compilation issue in usb driver.

Fixes #22343

Signed-off-by: Erwan Gouriou [email protected]

@erwango erwango added bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 labels Jan 31, 2020
@erwango erwango requested a review from ydamigos January 31, 2020 13:27
@erwango erwango added the block: HW Test Testing on hardware required before merging label Jan 31, 2020
@erwango
Copy link
Member Author

erwango commented Jan 31, 2020

@giellamoswhard can you test and confirm this helps you enabling CAN_1 and USB at the same time?

@ghost
Copy link

ghost commented Jan 31, 2020

@erwang I've tried you pull request: now it builds without errors but I have runtime troubles: when I run it USB is not recognized anymore by linux.

[ 1262.105222] usb 1-6.4: Device not responding to setup address.
[ 1262.313225] usb 1-6.4: Device not responding to setup address.
[ 1262.521093] usb 1-6.4: device not accepting address 39, error -71
[ 1262.521191] usb 1-6-port4: unable to enumerate USB device

@@ -8,6 +8,11 @@

/ {
soc {
usb: usb@40005c00 {
/* Remap USB_LB IRQ to enable use with CAN_1 */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it USB_LP?

@@ -8,6 +8,11 @@

/ {
soc {
usb: usb@40005c00 {
/* Remap USB_LB IRQ to enable use with CAN_1 */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it USB_LP?

@erwango
Copy link
Member Author

erwango commented Jan 31, 2020

@giellamoswhard Thanks for quick feedback.

@erwang I've tried you pull request: now it builds without errors but I have runtime troubles: when I run it USB is not recognized anymore by linux.

Do you mean this was working fine previously with USB only?

@ghost
Copy link

ghost commented Jan 31, 2020

@erwango Thank you for the PR.

Yes if I build it with master it works (both usb and can)
Using the pull request it seems that:

  • USB and CAN can be built together
  • CAN alone works
  • USB alone works on cdc profile but fails on hid profile

[00:00:00.006,000] main.composite_pre_init: HID Device: dev 0x2004
[00:00:00.006,000] main.main: Starting application
[00:00:00.006,000] usb_dc_stm32: System Configuration Controller

@erwango
Copy link
Member Author

erwango commented Jan 31, 2020

@giellamoswhard ok. It is likely that something is missing in IRQ remap operation. I'll need to test and debug. I don't have hw with me, so it will have to wait next week.

@ghost
Copy link

ghost commented Jan 31, 2020

@erwango Ok, thanks for the support.

Don't know if it can be useful but datasheet says that setting USB_IT_RMP all three USB interrupts are remapped (to 74, 75, 76) but new DTS only has a configuration for USB_LP.

@ydamigos
Copy link
Collaborator

ydamigos commented Feb 1, 2020

@erwango I tried it on stm32f3_disco with samples/subsys/usb/cdc_acm without enabling CAN. I get the following error and the USB device is not recognized:

*** Booting Zephyr OS build zephyr-v2.1.0-1540-gc01c00b71b21  ***
[00:00:00.005,000] <err> usb_dc_stm32: System Configuration Controller clock is disabled. Unable to enable IRQ remapping.
[00:00:00.005,000] <inf> cdc_acm_echo: Wait for DTR

SYSCFG clock is not enabled. The following patch (I opened a PR in your repository), fixed the USB issue:

diff --git a/drivers/clock_control/clock_stm32f0_f3.c b/drivers/clock_control/clock_stm32f0_f3.c
index d35b03cee9..e1b68698d2 100644
--- a/drivers/clock_control/clock_stm32f0_f3.c
+++ b/drivers/clock_control/clock_stm32f0_f3.c
@@ -77,6 +77,11 @@ void config_enable_default_clocks(void)
        /* Enable System Configuration Controller clock. */
        LL_APB1_GRP2_EnableClock(LL_APB1_GRP2_PERIPH_SYSCFG);
 #endif
+#else
+#if defined(CONFIG_USB_DC_STM32)
+       /* Enable System Configuration Controller clock. */
+       LL_APB2_GRP1_EnableClock(LL_APB2_GRP1_PERIPH_SYSCFG);
+#endif
 #endif /* !CONFIG_SOC_SERIES_STM32F3X */
 }

Then, I enabled CAN. USB device is not recognized anymore and I don't get an error in logs. I don't have a external CAN bus transceiver to test CAN.

@erwango
Copy link
Member Author

erwango commented Feb 4, 2020

@ydamigos

SYSCFG clock is not enabled. The following patch (I opened a PR in your repository), fixed the USB issue:

Ok, I got my mistake. SYSCONFIG is indeed enabled in gpio_stm32_enable_int. But this one is not part of gpio_init, only happens if we configure a pin with interrupts (as the name suggests..).
Will fix that first and then check out the last issue.

@erwango
Copy link
Member Author

erwango commented Feb 4, 2020

@ydamigos , I'm seeing issue on stm32f3_disco on samples/subsys/usb/cdc_acm on master already.

*** Booting Zephyr OS build zephyr-v2.1.0-1657-g9897c3b0dda2  ***
[00:00:00.005,000] <inf> cdc_acm_echo: Wait for DTR
[00:00:00.008,000] <inf> usb_cdc_acm: USB device suspended

And nothing in dmesg.

Do you see the same?

@erwango erwango force-pushed the fix_f3_usb_can_it_conflict branch 2 times, most recently from a4edd30 to d8d41d5 Compare February 4, 2020 10:12
@zephyrbot
Copy link
Collaborator

zephyrbot commented Feb 4, 2020

All checks passed.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@ydamigos
Copy link
Collaborator

ydamigos commented Feb 4, 2020

@ydamigos , I'm seeing issue on stm32f3_disco on samples/subsys/usb/cdc_acm on master already.

*** Booting Zephyr OS build zephyr-v2.1.0-1657-g9897c3b0dda2  ***
[00:00:00.005,000] <inf> cdc_acm_echo: Wait for DTR
[00:00:00.008,000] <inf> usb_cdc_acm: USB device suspended

And nothing in dmesg.

Do you see the same?

It works fine with master for me. A common mistake I do with stm32f3_disco is that I always forget to plug a usb cable to USB USER port. So I have to ask, did you plug a cable to USB USER port?

@erwango
Copy link
Member Author

erwango commented Feb 5, 2020

It works fine with master for me. A common mistake I do with stm32f3_disco is that I always forget to plug a usb cable to USB USER port. So I have to ask, did you plug a cable to USB USER port?

Yes I did, but this is a fair question :-)!

@erwango
Copy link
Member Author

erwango commented Feb 10, 2020

@giellamoswhard, @ydamigos can you have a new try ?
Change is working on my side now (tested with CAN enabled).

Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

@erwango Something is wrong with the commits. I see 2 commits with similar message.

@@ -303,6 +303,18 @@ static int usb_dc_stm32_init(void)
HAL_StatusTypeDef status;
unsigned int i;

#ifdef SYSCFG_CFGR1_USB_IT_RMP
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we move it to usb_dc_attach() where we do also the pin remapping?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

On stm32f302/3 series, USB and CAN_1 share same IRQ lines.
To use USB and CAN_1 together, USB IRQ could be remap to other
line numbers, on which there is no conflict.
Remap the USB IRQ lines by default:
-Assign remap number in matching dtsi files
-Perform remap before usb driver init

Additionally, fix compilation issue in usb driver.

Fixes zephyrproject-rtos#22343

Signed-off-by: Erwan Gouriou <[email protected]>
Signed-off-by: Yannis Damigos <[email protected]>
Signed-off-by: Erwan Gouriou <[email protected]>
@ghost
Copy link

ghost commented Feb 11, 2020

@erwango HI, I'm testing on my custom board (stm32f303rc) with both can and usb configured:

  • Can example build and works
  • cdc_acm example seems to work but my linux refuses to enumerate the board

[ 3366.273290] usb 1-5.4: new full-speed USB device number 59 using xhci_hcd
[ 3366.473173] usb 1-5.4: device descriptor read/64, error -32
[ 3366.785158] usb 1-5.4: device descriptor read/64, error -32
[ 3366.893273] debugfs: Directory '34' with parent 'devices' already present!
[ 3367.073163] usb 1-5.4: new full-speed USB device number 60 using xhci_hcd
[ 3367.273099] usb 1-5.4: device descriptor read/64, error -32

@erwango
Copy link
Member Author

erwango commented Feb 11, 2020

@giellamoswhard can you try to boot and then plug/unplug ?

@ghost
Copy link

ghost commented Feb 11, 2020

@erwango
I did it but the same result, even if I disable CAN drivers.
But now it seems I get the same result from master so maybe it is another problem (a bit strange: last week I was able to use USB)

@ghost
Copy link

ghost commented Feb 11, 2020

@erwango It seems something I have something hw broken on the USB side, but my build and can test works.

@erwango
Copy link
Member Author

erwango commented Feb 11, 2020

@erwango It seems something I have something hw broken on the USB side, but my build and can test works.

@giellamoswhard opposite on my side: Things are working now. Maybe @ydamigos can give his status?

@ghost
Copy link

ghost commented Feb 11, 2020

@erwango It works now ! Sorry but doing doing the test I deleted the pinmuxes of USB.

@erwango erwango removed the block: HW Test Testing on hardware required before merging label Feb 11, 2020
Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

Test on stm32f3_disco with samples/subsys/usb/cdc_acm enabling CAN and it works fine.

@erwango erwango requested a review from galak February 14, 2020 13:05
@galak galak added this to the v2.2.0 milestone Feb 14, 2020
@jhedberg jhedberg merged commit 557d263 into zephyrproject-rtos:master Feb 18, 2020
@erwango erwango deleted the fix_f3_usb_can_it_conflict branch June 5, 2020 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stm32f303 - irq conflict between CAN and USB
7 participants