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

STM32: CAN Reset() #4396

Closed
chrissnow opened this issue May 26, 2017 · 8 comments
Closed

STM32: CAN Reset() #4396

chrissnow opened this issue May 26, 2017 · 8 comments

Comments

@chrissnow
Copy link
Contributor

Description

  • Type: Bug

Bug

Target
STM32 (device not important)

Toolchain:
ARM

A call to reset() triggers the CAN peripheral reset bit, however the peripheral itself seems to set the sleep bit when this happens.

This means that CAN cannot be used after a call to reset without manually clearing the sleep bit in the MCR register (CAN_MCR_SLEEP) which the user has no easy way of doing.

Should can_reset() be changed to clear the sleep bit after the reset?

cc @thomasonw

@0xc0170
Copy link
Contributor

0xc0170 commented May 26, 2017

@thomasonw
Copy link

A comment in support,
-- @ POR the STM32 CAN devices are placed in sleep by default (CAN_MCR Reset value: 0x0001 0002).
-- MBED libs stm32f?xx_hal_can.c, function HAL_CAN_Init() clears this bit.
-- STM32F man states software reset will re-enable sleep mode

There may be other configs as well which need cleaning up? Would it be appropriate for can_reset() to resolve to HAL_CAN_Init() ?

@LMESTM
Copy link
Contributor

LMESTM commented Aug 17, 2017

@chrissnow @thomasonw thanks for reporting this.
We can indeed consider the idea of applying the init phase again to restore registers - I will look into it and propose a patch, keeping @adustm in cc for review

@LMESTM
Copy link
Contributor

LMESTM commented Aug 17, 2017

@thomasonw @chrissnow settings like CAN mode() and monitor() would probably be lost as well after reset. Would you expect them to be restored as well or would application set them again ?
Restoring them means storing them in a first place and I just want to avoid wasting memory

@chrissnow
Copy link
Contributor Author

@LMESTM I guess it depends what the API is meant to do, If the intention is to clear some sort of error then I would expect the configuration to survive, if it's meant to be like a hardware reset I'd expect everything to be lost.

I didn't check what config(if any) was lost, but it worked fine after clearing standby.

@chrissnow
Copy link
Contributor Author

Here it is,

  /** Reset CAN interface.
  *
  * To use after error overflow.
  */
  void reset();

So calling it should maintain config.

@LMESTM
Copy link
Contributor

LMESTM commented Aug 17, 2017

@chrissnow I've read those lines as well but let's be honest, that doesn't help much ;-)
Anyway I think that I can avoid other settings to be lost without storing too much data. While doing the changes, I've done other clean-ups, especially for proper multiple instantiation of the driver.
I've got a branch available (https://github.com/LMESTM/mbed/tree/can_reset). I'll try to run some tests and check the registers are ok. Then when I think this is ok, would you have also have a setup and some time to run a test to check the patch is fine ?

@LMESTM
Copy link
Contributor

LMESTM commented Aug 18, 2017

I tested the proposed patch set and have sent the pull requet in #4932
I ended with more modifications than I'd first expected - so would be great if @adustm can review :-)
Also maybe @chrissnow could run some test ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants